Replace cimg base image with ubuntu:latest #95
Conversation
| pip3 install awscli | ||
| pip3 install requests requests-unixsocket2 | ||
| apt-get update | ||
| pip3 install --break-system-packages awscli requests requests-unixsocket2 |
There was a problem hiding this comment.
used --break-system-packages to avoid issues with installing packages alongside the system's (blog ref)
There was a problem hiding this comment.
Interesting! Wasn't aware of this param :)
There was a problem hiding this comment.
I believe the built image is missing a few tools used in the scripts here and there in dd-trace-java.
Got a few questions / remarks though, that may be addressed later.
-
I noticed the versioning is based on year + month
dd-trace-java-docker-build/build
Line 212 in 3c26d22
Since this is like a major change shall we label this differently? That said this could affect other part of the build pipelines.
-
Circle CI image appear to use the
circleciuser, this requires commands to be run as sudo. Should we reproduce this behavior?
Dockerfile
Outdated
| RUN apt-get update && \ | ||
| apt-get install -y curl tar apt-transport-https ca-certificates gnupg \ | ||
| socat less debian-goodies autossh ca-certificates-java python3-pip && \ | ||
| apt-get clean && \ | ||
| rm -rf /var/lib/apt/lists/* && \ | ||
| mkdir -p /usr/local/lib/docker/cli-plugins /usr/local/bin |
There was a problem hiding this comment.
I believe this is not enough, we'll miss other executables such as jq, yq, git, gh (the github cli), docker among others.
Another thing that may bites us is the locale environment variables, we may want to have LANG=en_US.UTF-8, LC_ALL=en_US.UTF-8 (likely doable via this apt-get install -y locales && locale-gen en_US.UTF-8 and setting the envs ENV LANG='en_US.UTF-8' LANGUAGE='en_US:en' LC_ALL='en_US.UTF-8'.
There was a problem hiding this comment.
Added locale environment variables! How could we get a comprehensive list of all of the executables we'd need to add? By parsing through dd-trace-java..? 😅
There was a problem hiding this comment.
Yeah I guess, or maybe dive into the cimg layer.
You can do so using dive or IntelliJ IDEA docker support.
sarahchen6
force-pushed
the
sarahchen6/change-base-image
branch
from
3763033 to
bdee917
Compare
@bric3 What do you mean by labelling differently? From this PR, it seems like the intention is to label each image by when it was last pulled, so year/month seems to make sense 🤔 |
@bric3 Good point! Looking into this, it seems like it is good security practice for the Dockerfile to run as a non-root user (one ref). However, we can still install everything at the root without using the |
There was a problem hiding this comment.
Looking good.
Few comments about cleaning it up further:
- Is the
yq,dockeranddocker-composestill needed? I used to install them manually to override the outdated version from the CircleCI image but I don't know if they are used. - Similarly I don't know if autoforward is still used 🤷
- And I ask to @smola what
ubuntu17is for and if it is still relevant
Do you know how to test the image from your PR in the CI to test it?
| sudo cp -rf --remove-destination /etc/java-17-openjdk/* /usr/lib/jvm/ubuntu17/lib/ | ||
| sudo cp -f --remove-destination /etc/java-17-openjdk/jvm-amd64.cfg /usr/lib/jvm/ubuntu17/lib/ | ||
| apt-get update | ||
| apt-get install -y openjdk-17-jdk |
| pip3 install awscli | ||
| pip3 install requests requests-unixsocket2 | ||
| apt-get update | ||
| pip3 install --break-system-packages awscli requests requests-unixsocket2 |
There was a problem hiding this comment.
Interesting! Wasn't aware of this param :)
|
You might want to revisit the cron time too: |
sarahchen6
changed the title
| @@ -143,6 +175,9 @@ COPY --from=all-jdk /usr/lib/jvm/ubuntu17 /usr/lib/jvm/ubuntu17 | |||
| COPY --from=all-jdk /usr/lib/jvm/graalvm17 /usr/lib/jvm/graalvm17 | |||
| COPY --from=all-jdk /usr/lib/jvm/graalvm21 /usr/lib/jvm/graalvm21 | |||
|
|
|||
| # Switch to non-root user during runtime for security | |||
| USER non-root-user | |||
There was a problem hiding this comment.
suggestion: Do we want a workdir in these images as well ?
E.g. the non-root-user's home, that can be created via --create-home in the useradd above commands
There was a problem hiding this comment.
Yes! -m has the same functionality as --create-home (ref). I did not set WORKDIR to this created directory though, so let me do that.
| apt-get install -y curl tar apt-transport-https ca-certificates gnupg \ | ||
| socat less debian-goodies autossh ca-certificates-java python3-pip locales |
There was a problem hiding this comment.
note: Re on the need for jq, git I think, and if they come as dependencies, I believe they deserve to be explicitly asked to apt-get. The github cli gh is definitely used in the pipelines, but I'm not sure where iot is installed.
I also wonder if docker needs to be installed as well, I believe the build is using test-containers which can make use of it. And just raising this I realise I have no idea how do work docker in docker.
With our transition from CircleCI to Gitlab, we want to replace the
cimgbase image with the smaller, faster, saferubuntu:24.04image.