Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Revert "Upgrade pre-installed packages in docker images" and Pin docker images by digest #2430

Conversation

triarius
Copy link
Contributor

@triarius triarius commented Oct 16, 2023

In #2410 I responded to a report that the system packages in our docker images were out of date by upgrading the system packages at build time. However, this has a notable disadvantage - the version in an image is now determined both by the commit of the build and the time that the build ran. So we have lost some reproducibility. So in this PR, we revert that.

However, we can do a bit better than the situation before. Previously, we only pinned the tags of the base image, for example we pinned the ubuntu image to the tag 22.04. Dependabot would only update the image when a newer tag emerged, which is very infrequent in the case of ubuntu. However, maintainers of the base image regularly push newer images to the tag that we have pinned. This mutates the "digest" of the image, and dependabot is capable of detecting updates to the digest. So in this PR, we pin all docker base images by digest.

Notably, this is MORE reproducible than the original situation, as it now two builds at the same commit will always use the same base image.

Reverts #2410

@linear
Copy link

linear bot commented Oct 16, 2023

@triarius triarius force-pushed the revert-2410-pdp-1799-upgrade-packages-installed-by-default-on-docker-images branch from 9d26b6a to de43c1c Compare October 19, 2023 00:52
@triarius triarius changed the title Revert "Upgrade pre-installed packages in docker images" Revert "Upgrade pre-installed packages in docker images" and pin images to digests Oct 19, 2023
@triarius triarius force-pushed the revert-2410-pdp-1799-upgrade-packages-installed-by-default-on-docker-images branch from de43c1c to b5f8c5e Compare October 19, 2023 02:47
@triarius triarius marked this pull request as ready for review October 19, 2023 02:47
@triarius triarius requested a review from a team October 19, 2023 02:47
Copy link
Contributor

@DrJosh9000 DrJosh9000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🥦

I gather that pinning versions of the packages installed with apk, apt, etc and then having Dependabot update those is not possible, otherwise we could do the same and close another hole in reproducibility.

@@ -1,4 +1,4 @@
FROM golang:1.20.10@sha256:098d628490c97d4419ed44a23d893f37b764f3bea06e0827183e8af4120e19be
FROM public.ecr.aws/docker/library/golang:1.20.10@sha256:098d628490c97d4419ed44a23d893f37b764f3bea06e0827183e8af4120e19be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch here

@triarius
Copy link
Contributor Author

LGTM 🥦

I gather that pinning versions of the packages installed with apk, apt, etc and then having Dependabot update those is not possible, otherwise we could do the same and close another hole in reproducibility.

Yeah, the story is a bit murky here. As noted in: dependabot/dependabot-core#2129 (comment), if we pinned the versions of apk/apt packages, the build will break after the package is upgraded in the repo as these repos typically only hold one version of each package. There may be some ways around this with caching docker layers, but that won't work consistently either with the way we currently keep the docker build cache local to each instance the builder agents run on.

In theory, dependabot could detect the update and create a PR that builds successfully. But everyone's branches will be broken until they incorporate the changes in the dependabot PR. And dependabot will never create the new PR immediately, there is always going to be some lag between when packages are updated in the repos and dependabot runs. I think this will be a bad development experience.

@triarius triarius changed the title Revert "Upgrade pre-installed packages in docker images" and pin images to digests Revert "Upgrade pre-installed packages in docker images" and Pin docker images by digest Oct 19, 2023
@triarius triarius merged commit 2d79c32 into main Oct 19, 2023
@triarius triarius deleted the revert-2410-pdp-1799-upgrade-packages-installed-by-default-on-docker-images branch October 19, 2023 03:25
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants