update docker to debian latest #40
Conversation
shaulk
force-pushed
the
bugfix/oded/fix-docker
branch
from
0ba9194 to
9bc875e
Compare
| @@ -50,7 +52,8 @@ class RemoteConfigFetcher: | |||
| organizations (which is not secure). | |||
| """ | |||
| DEFAULT_RETRY_CONFIG = { | |||
There was a problem hiding this comment.
@orishavit please review it
| RUN apk update | ||
| FROM python:3.8-slim-bullseye as BuildStage | ||
| # update apt | ||
| RUN apt-get update | ||
| # TODO: remove this when upgrading to a new alpine version | ||
| # more details: https://github.com/pyca/cryptography/issues/5771 | ||
| ENV CRYPTOGRAPHY_DONT_BUILD_RUST=1 |
There was a problem hiding this comment.
CRYPTOGRAPHY_DONT_BUILD_RUST looks like an alpine thing
| FROM python:3.8-alpine3.11 as BuildStage | ||
| # update apk cache | ||
| RUN apk update | ||
| FROM python:3.8-slim-bullseye as BuildStage |
There was a problem hiding this comment.
No need for slim in the BuildStage
There was a problem hiding this comment.
Why not Python 3.10 ?
(I mean you probably want it fast for Riskified, but would be good to upgrade in the near future)
There was a problem hiding this comment.
We should test our code for 3.10, I would be glad to know that we can move to 3.10 @asafc, can we?
| # TODO: remove this when upgrading to a new alpine version | ||
| # more details: https://github.com/pyca/cryptography/issues/5771 | ||
| ENV CRYPTOGRAPHY_DONT_BUILD_RUST=1 | ||
| # install linux libraries necessary to compile some python packages | ||
| RUN apk add --update --no-cache --virtual .build-deps gcc git build-base alpine-sdk python3-dev musl-dev postgresql-dev libffi-dev libressl-dev | ||
| # TODO ask asaf about postgres 11 | ||
| RUN apt-get install --fix-missing -y gcc git make python3-dev libpq-dev libffi-dev libssl-dev g++ |
There was a problem hiding this comment.
Do this together with apt-get update
There was a problem hiding this comment.
& apt-get cleanto reduce image size.- Also - if you take @orishavit 's advice and use
python3.8without the slim, it almost certainly already has you covered in terms of deps and no real need to install anything else (that's the purpose of that image, and that was my experience withopal) - That would speed up the image build which is always good
There was a problem hiding this comment.
wouldn't using the non-slim image will make the image larger?
There was a problem hiding this comment.
If you use the slim image and then all install deps yourself, you'll get the same size but longer setup times. Slim might actually be better, but I would test this.
There was a problem hiding this comment.
Couple of things:
- Actually Python's official rule of thumb is go with the regular. It's bigger in size but many images are using the underlying
buildpack-deps- therefor it "compresses" better per machine (docker stores the layer only once for many images). - The
BuildStageis a temporary thing just used to build the pkgs, the final image can be still based on the slim flavor.
| RUN apk add g++ python3-dev linux-headers | ||
| RUN apt-get -y install --fix-missing gcc g++ python3-dev | ||
| # install newer pcre2 to resolve CVE-2022-1586 | ||
| RUN apt-get -y install -t testing libpcre2-8-0 |
There was a problem hiding this comment.
Merge all apt-gets to one step
| @@ -50,7 +52,8 @@ class RemoteConfigFetcher: | |||
| organizations (which is not secure). | |||
| """ | |||
| DEFAULT_RETRY_CONFIG = { | |||
| @@ -0,0 +1 @@ | |||
| deb http://deb.debian.org/debian bookworm main | |||
There was a problem hiding this comment.
Why is this needed? Might cause collisions between versions (not unlike what we have with tenacity). If debian bullseye has versions too old for what you need, consider using Ubuntu.
There was a problem hiding this comment.
This is a very new version, with a patch for critical vlun
https://security.snyk.io/vuln/SNYK-DEBIAN11-PCRE2-2808697
Check out the date
obsd
force-pushed
the
bugfix/oded/fix-docker
branch
from
f5afd19 to
3cdd0bd
Compare
| CMD ["/start.sh"] |
There was a problem hiding this comment.
(can't comment on lines that haven't change)
regarding wait-for-it.sh... I found that it uses nc which isn't installed by default. (apt-get install netcat).
I think u don't use it here anyway...
| # needed for rookout | ||
| RUN apk add g++ python3-dev linux-headers | ||
| RUN apt-get -y install --fix-missing gcc g++ python3-dev |
There was a problem hiding this comment.
On my tests seems like rookout is installable & importable without that line. (wasn't the case on alpine)
No description provided.