-
Notifications
You must be signed in to change notification settings - Fork 58
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
chore(hermetic-build): update final base image and owlbot dependency #3385
Conversation
@@ -64,6 +62,9 @@ ENV OS_ARCHITECTURE="linux-x86_64" | |||
# install OS tools | |||
RUN apk update && apk add unzip curl rsync openjdk11 jq bash nodejs npm git | |||
|
|||
# Remove unnecessary cross-spawn from npm to resolve CVE-2024-21538 | |||
RUN rm -rf /usr/lib/node_modules/npm/node_modules/cross-spawn/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the latest npm includes a fix for it? If not, I think we should wait for it rather than this verify customized fix. Removing it could resolve the vulnerability, but based on the description of the vulnerability, I don't think it would affect hermetic build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cross-spawn
comes from npm CLI, the CLI updated the dependency in v9.9.4. However, the latest version of npm
in Alpine Package Keeper (apk) is still 8.10.0.
The waiting time is out of our control so how about I remove npm
afterwards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the SLO of fixing this issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before 2025/1/29
Quality Gate passed for 'gapic-generator-java-root'Issues Measures |
Quality Gate passed for 'java_showcase_integration_tests'Issues Measures |
@@ -126,6 +124,7 @@ RUN git checkout "${OWLBOT_CLI_COMMITTISH}" | |||
RUN npm i && npm run compile && npm link | |||
RUN owl-bot copy-code --version | |||
RUN chmod o+rx $(which owl-bot) | |||
RUN apk del -r npm && apk cache clean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this approach! We don't need npm
in our image as long as owl-bot
package is already built.
@@ -27,7 +27,7 @@ jobs: | |||
git checkout -b "${head_ref}" fork/${head_ref} | |||
changed_directories="$(git diff --name-only "fork/${head_ref}" "origin/${base_ref}")" | |||
fi | |||
if [[ ${changed_directories} =~ "hermetic_build/" ]]; then | |||
if [[ ${changed_directories} =~ "hermetic_build/" ]] || [[ ${changed_directories} =~ ".cloudbuild/library_generation/" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change related?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change aims to run hermetic build test if Dockerfile is changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, so it is not directly related to the vulnerability fix. Can you please mention it in the PR description?
…oogleapis#3385) Updates the _final_ base image to the latest as well as the owlbot CLI dependency. --------- Co-authored-by: Joe Wang <[email protected]>
In this PR:
Dockerfile
changed