Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Use --pull flag when building images for releases #1293

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

NimJay
Copy link
Collaborator

@NimJay NimJay commented Nov 22, 2022

Background

  • When I first created the v0.4.2 release, emailservice:v0.4.2 still contained the same # of CVEs as emailservice:v0.4.1.
  • But we expected CVE-2022-43680 to be fixed in emailservice:v0.4.2.
  • The reason it wasn't being fixed was because CVE-2022-43680 was being sourced from the emailservice's base Docker images — which my local machine was caching when I initially built emailservice:v0.4.2.
  • The --pull flag is explained here. It makes sure the base Docker image is pulled (not from local cache but from the registry) when we docker build.

Change Summary

  • Use the --pull flag to ensure that the base Docker images is pulled from the registry (not my local cache) when container images are built.

Testing Procedure

  • I have already tested this when I created the images for v0.4.2.
  • If you look at the emailservice on our Google Container Registry, you'll see that v0.4.2 has one less vulnerability than v0.4.1, which means the base images was pulled (not from cache) when I build emailservice on my local machine.

@NimJay NimJay added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Nov 22, 2022
@github-actions
Copy link

🚲 PR staged at http://34.27.45.93

@NimJay
Copy link
Collaborator Author

NimJay commented Nov 22, 2022

Do not merge until Online Boutique v0.4.2 has been released!
Online Boutique v0.4.2 has been released!

@NimJay NimJay removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Nov 22, 2022
@NimJay NimJay marked this pull request as ready for review November 22, 2022 18:46
@NimJay NimJay requested a review from a team as a code owner November 22, 2022 18:46

log "Pushing: ${image}"
docker push "${image}"

if [ $svcname != "frontend" ] && [ $svcname != "loadgenerator" ]
then
log "Building: ${image}-native-grpc-probes"
docker build -t "${image}-native-grpc-probes" . --target without-grpc-health-probe-bin
docker build --pull -t "${image}-native-grpc-probes" . --target without-grpc-health-probe-bin
log "Pushing: ${image}-native-grpc-probes"
docker push "${image}-native-grpc-probes"
fi
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Context:

@bourgeoisor
Copy link
Member

I'm a little bit confused how that could've happened -- is the implication that the base image tag we're using (python:3.9.15-slim) was overwritten with a new image by the python team to fix a CVE instead of releasing a new tag altogether?

@NimJay
Copy link
Collaborator Author

NimJay commented Nov 22, 2022

is the implication that the base image tag we're using (python:3.9.15-slim) was overwritten with a new image by the python team to fix a CVE instead of releasing a new tag altogether

Great question, @bourgeoisor!
And yes. That is our understanding.
The python:3.9.15-slim image was last pushed 6 days ago.

Copy link
Member

@bourgeoisor bourgeoisor left a comment

Choose a reason for hiding this comment

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

LGTM. Ideally this wouldn't be needed, but we can't control external Docker image tags getting overwritten 😅

@mathieu-benoit
Copy link
Contributor

LGTM. Ideally this wouldn't be needed, but we can't control external Docker image tags getting overwritten 😅

@bourgeoisor @NimJay in the future, we may want to use hash instead of tag to avoid this kind of issue. No rush on this, but maybe something for the future :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants