-
Notifications
You must be signed in to change notification settings - Fork 53
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): use secure base image hosted in docker.com #3324
Conversation
@@ -14,22 +14,45 @@ | |||
|
|||
# install gapic-generator-java in a separate layer so we don't overload the image | |||
# with the transferred source code and jars | |||
FROM gcr.io/cloud-devrel-public-resources/java21@sha256:2ceff5eeea72260258df56d42e44ed413e52ee421c1b77393c5f2c9c4d7c41da AS ggj-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.
Where is gcr.io/cloud-devrel-public-resources/java21
coming from? Basically I'm trying to understand what is the base image for it eventually.
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.
Confirmed it's based on eclipse-temurin-21
https://github.com/googleapis/testing-infra-docker/blob/main/java/java21/Dockerfile#L15
https://github.com/googleapis/testing-infra-docker/blob/main/java/cloudbuild.yaml#L163
WORKDIR /home | ||
|
||
# Install compatibility layer to run glibc-based programs (such as the | ||
# grpc plugin). |
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 guess protoc/gapic plugins as well?
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.
protoc
can run without the compatibility layer. The java runtime is enabled via apk add
, so the necessary shared objects (if any) will be brought by the package manager.
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.
At least only the grpc
plugin gave me trouble due to missing shared objects.
# downloadable distribution of the grpc plugin that is Alpine (musl) compatible. | ||
# This is one of the recommended approaches to ensure glibc-compatibility | ||
# as per https://wiki.alpinelinux.org/wiki/Running_glibc_programs | ||
RUN git clone https://gitlab.com/manoel-linux1/GlibMus-HQ.git |
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.
Where were these installed before? In one of the base images?
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.
Yes, most images come with binaries that were compiled with glibc. However Alpine is made with BusyBox + "musl libc", which cannot run certain linux binaries that were compiled with glibc (i.e. missing shared objects).
@@ -85,25 +119,14 @@ RUN python -m pip install src/common | |||
RUN python -m pip install --require-hashes -r src/library_generation/requirements.txt | |||
RUN python -m pip install src/library_generation | |||
|
|||
# Install nvm with node and npm |
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.
Why Node is not needed anymore? I thought we still need it for running owlbot cli?
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.
Oh, that's because Alpine can install nodejs
+ npm
via apk
sdk-platform-java/.cloudbuild/library_generation/library_generation.Dockerfile
Lines 65 to 66 in 10b176b
# install OS tools | |
RUN apk update && apk add unzip curl rsync maven jq bash nodejs npm git |
# {x-version-update-end} | ||
|
||
RUN mvn install -B -ntp -DskipTests -Dclirr.skip -Dcheckstyle.skip | ||
RUN cp "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" \ | ||
RUN --mount=type=cache,target=/root/.m2 mvn dependency:go-offline -B -ntp -DskipTests -Dclirr.skip -Dcheckstyle.skip |
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.
Why do we need mvn dependency
?
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.
It doesn't have any direct effect on the image but just to download the dependencies in a separate step. This is because we can leverage caching to prevent downloading the external dependencies every time we build the image.
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.
How much time it would save by caching the external dependencies? In general, I think it's OK to always download it because we don't build image often. Caching is nice but sometimes we may run into issues where new dependencies are not downloaded.
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.
We removed this just to have a consistent behavior in local and prod (thanks @blakeli0)
RUN apt-get update && apt-get install -y \ | ||
unzip openjdk-17-jdk rsync maven jq \ | ||
&& apt-get clean | ||
RUN apk update && apk add unzip curl rsync maven jq bash nodejs npm git |
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.
Do we need maven
in the final image?
As for java formatter, we've changed to download the jar so Java runtime is suffice.
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 is because there is no candidate in APK to download the runtime directly (apk list jdk
). I'll double check this.
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.
Oh, well, apk list
doesn't support partial string matches. I found openjdk11
by searching that exact string. Thanks for checking on this.
Quality Gate passed for 'gapic-generator-java-root'Issues Measures |
Quality Gate passed for 'java_showcase_integration_tests'Issues Measures |
This PR switches the base images of all the Hermetic Build Docker image stages to publicly hosted docker images.
The chosen images are mirrored by Airlock or are at least in process of incorporation.
The choice of OS family is
alpine
for its security-oriented setup. 0 vulnerabilities were found in the public image scan reports (example).Although several optimizations are possible (see #3196 and its intermediate commits), this PR is restricted to the minimal changes to have a secure base image.
Notes:
-d
flag in therm
command is not supported. This is why we removed its usage in the scripts