-
Notifications
You must be signed in to change notification settings - Fork 81
Make docker images fully reproducible [DI-741] #1236
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
Changes from 24 commits
918eeab
88a74fc
8fa13c8
53a7a51
fabbc3f
b8000b2
728a75f
ecb6e3b
a64a377
5be04e2
b9177f0
4971efd
d6e0f65
561f06b
5044170
d440eb1
6dac833
0d527f4
c88b7db
551173a
4647ec0
4137118
b9f9f16
45b9319
f8dd562
4f9642e
c2917a6
04736eb
3ffa221
bd779ad
3297ad1
35fe78b
f1b5010
0ab9ebc
7f5d06c
7e9e78e
5f4e379
a5b5005
c5489d1
0adde40
5e77e2e
6d82fdd
a71e3a6
1e2615c
8fe36e8
a770adc
ad29259
01bb1e5
d48e0be
bdcee60
e1a6496
78287a4
a3fe046
f3ed167
7b686f6
a70f24a
2b2aa91
901b162
24258a9
3057fe4
d26cabd
591b144
1ad8b4e
5d77c97
20549f0
314680e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. General comments:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I've run it, but you won't see
Locally on Macos, and Linux in GH Actions |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| set -o errexit -o nounset -o pipefail ${RUNNER_DEBUG:+-x} | ||
|
|
||
| # Verifies Docker image build reproducibility by building twice and comparing layer digests. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit - standard is to have Usage() ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. something like this |
||
| # Exits 0 if all layers are identical across both builds, 1 if any differ. | ||
|
JackPGreen marked this conversation as resolved.
Outdated
|
||
| # | ||
| # Usage: | ||
| # verify-layer-reproducibility.sh -f <dockerfile> [-- <extra buildx build args...>] | ||
| # | ||
| # Example: | ||
| # verify-layer-reproducibility.sh -f hazelcast-oss/Dockerfile -- hazelcast-oss/ | ||
|
|
||
| RANDOM_SUFFIX="$(head -c 8 /dev/urandom | od -An -tx1 | tr -d ' \n')" | ||
|
nishaatr marked this conversation as resolved.
Outdated
|
||
| readonly RANDOM_SUFFIX | ||
| readonly TAG_A="repro-check-a-${RANDOM_SUFFIX}" | ||
| readonly TAG_B="repro-check-b-${RANDOM_SUFFIX}" | ||
|
|
||
| cleanup() { | ||
| docker rmi "${TAG_A}" "${TAG_B}" 2>/dev/null || true | ||
| } | ||
| trap cleanup EXIT | ||
|
|
||
| dockerfile="" | ||
| while [[ $# -gt 0 ]]; do | ||
|
nishaatr marked this conversation as resolved.
Outdated
|
||
| case "$1" in | ||
| -f) dockerfile="$2"; shift 2 ;; | ||
| --) shift; break ;; | ||
| *) echo "Unknown option: $1" >&2; exit 1 ;; | ||
| esac | ||
| done | ||
|
|
||
| if [[ -z "${dockerfile}" ]]; then | ||
|
nishaatr marked this conversation as resolved.
Outdated
|
||
| echo "Error: -f <dockerfile> is required" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| extra_args=("$@") | ||
|
|
||
| build_image() { | ||
| local tag="$1" | ||
| echo "==> Building image '${tag}'..." | ||
| docker buildx build \ | ||
| --no-cache \ | ||
| --load \ | ||
| -f "${dockerfile}" \ | ||
| -t "${tag}" \ | ||
| "${extra_args[@]}" | ||
| } | ||
|
|
||
| get_layers() { | ||
| docker inspect --format '{{json .RootFS.Layers}}' "$1" | ||
| } | ||
|
|
||
| build_image "${TAG_A}" | ||
| echo "" | ||
| build_image "${TAG_B}" | ||
| echo "" | ||
|
|
||
| layers_a=$(get_layers "${TAG_A}") | ||
| layers_b=$(get_layers "${TAG_B}") | ||
|
|
||
| count_a=$(echo "${layers_a}" | jq 'length') | ||
| count_b=$(echo "${layers_b}" | jq 'length') | ||
|
ldziedziul marked this conversation as resolved.
Outdated
JackPGreen marked this conversation as resolved.
Outdated
|
||
|
|
||
| echo "=== Layer Reproducibility Report ===" | ||
| echo "" | ||
|
|
||
| if [[ "${count_a}" -ne "${count_b}" ]]; then | ||
| echo "FAIL: Layer count mismatch (${count_a} vs ${count_b})" | ||
| exit 1 | ||
|
nishaatr marked this conversation as resolved.
Outdated
|
||
| fi | ||
|
|
||
| has_diff=false | ||
| for i in $(seq 0 $((count_a - 1))); do | ||
| digest_a=$(echo "${layers_a}" | jq -r ".[$i]") | ||
| digest_b=$(echo "${layers_b}" | jq -r ".[$i]") | ||
| if [[ "${digest_a}" == "${digest_b}" ]]; then | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not very readable IMHO: vs |
||
| echo " Layer $((i + 1))/${count_a}: MATCH" | ||
| else | ||
| echo " Layer $((i + 1))/${count_a}: DIFFER" | ||
| echo " A: ${digest_a}" | ||
| echo " B: ${digest_b}" | ||
| has_diff=true | ||
| fi | ||
| done | ||
|
|
||
| echo "" | ||
| if [[ "${has_diff}" == "true" ]]; then | ||
| echo "RESULT: FAIL - some layers differ between builds" | ||
|
JackPGreen marked this conversation as resolved.
Outdated
|
||
| exit 1 | ||
| fi | ||
|
|
||
| echo "RESULT: PASS - all ${count_a} layers are identical" | ||
|
JackPGreen marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| name: Verify layer reproducibility | ||
|
|
||
| on: | ||
| workflow_call: | ||
| inputs: | ||
| SOURCE_REF: | ||
| description: 'The hazelcast-docker branch to verify' | ||
| required: true | ||
| type: string | ||
|
Comment on lines
+6
to
+9
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to pass this? Isn't it implicit via the github context?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean? SOURCE_REF not needed?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for other workflows we pass it, I want to have a unified approach |
||
|
|
||
| jobs: | ||
| verify: | ||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| include: | ||
| - context: hazelcast-oss | ||
| dist-file: hazelcast-distribution.zip | ||
| - context: hazelcast-enterprise | ||
| dist-file: hazelcast-enterprise-distribution.zip | ||
| steps: | ||
| - name: Checkout Code | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ inputs.SOURCE_REF }} | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 | ||
|
JackPGreen marked this conversation as resolved.
Outdated
|
||
|
|
||
| - name: Create test distribution | ||
| run: | | ||
| dist_dir=$(mktemp -d) | ||
| mkdir -p "$dist_dir/hazelcast-0.0.0/bin" "$dist_dir/hazelcast-0.0.0/lib" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. whats the motivation of using temp zip as opposed to HZ snapshot?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. smaller and faster, we already have similar thing in this repo (which I integrate into a single code)
nishaatr marked this conversation as resolved.
Outdated
|
||
| printf '#!/bin/bash\n' > "$dist_dir/hazelcast-0.0.0/bin/hz" | ||
| printf '#!/bin/bash\n' > "$dist_dir/hazelcast-0.0.0/bin/hz-healthcheck" | ||
| chmod +x "$dist_dir/hazelcast-0.0.0/bin/"* | ||
| touch "$dist_dir/hazelcast-0.0.0/lib/placeholder" | ||
| (cd "$dist_dir" && zip -qr "$GITHUB_WORKSPACE/${{ matrix.context }}/${{ matrix.dist-file }}" hazelcast-0.0.0/) | ||
| rm -rf "$dist_dir" | ||
|
JackPGreen marked this conversation as resolved.
Outdated
|
||
|
|
||
| - name: Verify layer reproducibility | ||
| run: | | ||
| .github/scripts/verify-layer-reproducibility.sh \ | ||
| -f ${{ matrix.context }}/Dockerfile \ | ||
| -- ${{ matrix.context }}/ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| # syntax=docker/dockerfile:1.7 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. apparently
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed: 3297ad1
ldziedziul marked this conversation as resolved.
Outdated
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this require
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems it requires buildkit
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That's annoying, but not unexpected. I think this time around were in a better situation to progress though. Now we've nailed down that external users extend out images rather than building their own, which means buildx is an exclusively internal requirement - so ensuring our test environments are up to spec can be in scope as part of this. And we have convincing justification where it adds real customer value, vs last time where it didn't and wasn't worth pushing. |
||
|
|
||
| # Used for image metadata only | ||
| # Describes the version of the Dockerfile, *not* the version of the bundled Hazelcast binary as this is/can be controlled externally | ||
| # Dockerfile needs some concept of versioning so that the release pipeline can tag/archive with an appropriate label | ||
|
|
@@ -8,40 +10,47 @@ ARG HZ_HOME="/opt/hazelcast" | |
| ARG USER_NAME="hazelcast" | ||
| ARG USER_GROUP="hazelcast" | ||
| ARG JDK_VERSION="21" | ||
| ARG SOURCE_DATE_EPOCH=1688223600 | ||
|
JackPGreen marked this conversation as resolved.
Outdated
nishaatr marked this conversation as resolved.
Outdated
JackPGreen marked this conversation as resolved.
Outdated
|
||
| ARG BUILD_ROOT=/build_root | ||
|
|
||
| FROM alpine:3 AS get-distribution | ||
|
|
||
| ARG HZ_HOME | ||
| ARG SOURCE_DATE_EPOCH | ||
| ARG HAZELCAST_ZIP_FILE_NAME="hazelcast-enterprise-distribution.zip" | ||
| ARG BUILD_ROOT | ||
|
|
||
| # Expects distribution ZIP to be on the local filesystem within the build context | ||
| COPY ${HAZELCAST_ZIP_FILE_NAME} /tmp/ | ||
|
|
||
| RUN mkdir -p ${HZ_HOME} \ | ||
| RUN mkdir -p ${BUILD_ROOT}${HZ_HOME} \ | ||
| && apk add --no-cache unzip \ | ||
| && unzip -qq /tmp/${HAZELCAST_ZIP_FILE_NAME} -d /tmp/hz \ | ||
| # Distribution ZIP structure is a single folder (e.g. "hazelcast-5.4.0-slim") containing the content | ||
| # Move all the content up a level so that the path contains no version and is constant | ||
| && mv /tmp/hz/*/* ${HZ_HOME}/ \ | ||
| && mv /tmp/hz/*/* ${BUILD_ROOT}${HZ_HOME}/ \ | ||
| && apk del unzip \ | ||
| && rm -rf /tmp/* \ | ||
| && echo "Setting Pardot ID to 'docker'" \ | ||
| && echo 'hazelcastDownloadId=docker' > "${HZ_HOME}/lib/hazelcast-download.properties" \ | ||
| && echo "Granting read permission to ${HZ_HOME}" \ | ||
| && chmod -R +r ${HZ_HOME} \ | ||
| && echo 'hazelcastDownloadId=docker' > "${BUILD_ROOT}${HZ_HOME}/lib/hazelcast-download.properties" \ | ||
| && echo "Granting read permission to ${BUILD_ROOT}${HZ_HOME}" \ | ||
| && chmod -R +r ${BUILD_ROOT}${HZ_HOME} \ | ||
| && echo "Grant execute permission to scripts in order to address the issue of permissions not being accurately propagated on Windows OS" \ | ||
| && chmod +x ${HZ_HOME}/bin/* | ||
| && chmod +x ${BUILD_ROOT}${HZ_HOME}/bin/* | ||
|
|
||
| COPY log4j2.properties log4j2-json.properties jmx_agent_config.yaml ${HZ_HOME}/config/ | ||
| COPY log4j2.properties log4j2-json.properties jmx_agent_config.yaml ${BUILD_ROOT}${HZ_HOME}/config/ | ||
|
|
||
| # Normalize timestamps for reproducible layer | ||
| RUN find ${BUILD_ROOT} -exec touch -h -d "@$SOURCE_DATE_EPOCH" {} + | ||
|
|
||
| FROM redhat/ubi9-minimal:9.7 | ||
| FROM eclipse-temurin:${JDK_VERSION}-jre-ubi9-minimal | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we used to use JDK
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I compared the installed packages in However - I'm not sure how many of those are because of the different image, or because were changing JDK vendors. E.G. you "need" Ideally we'd change JDK vendor, then base image seperately to reproducible builds...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's actually for good spot, there's a lot of differences I haven't expected, especially python presence. I think it could be quite breaking for customers. Taking this into account I've reverted the original base image: 7f5d06c
JackPGreen marked this conversation as resolved.
Outdated
JackPGreen marked this conversation as resolved.
Outdated
|
||
|
|
||
| ARG HZ_VERSION | ||
| ARG HZ_HOME | ||
| ARG HZ_VERSION | ||
| ARG USER_NAME | ||
| ARG USER_GROUP | ||
| ARG JDK_VERSION | ||
| ARG SOURCE_DATE_EPOCH | ||
| ARG BUILD_ROOT | ||
|
|
||
| # Runtime variables | ||
| ENV HZ_HOME="${HZ_HOME}" \ | ||
|
|
@@ -53,7 +62,8 @@ ENV HZ_HOME="${HZ_HOME}" \ | |
| JAVA_OPTS="" \ | ||
| HAZELCAST_CONFIG=config/hazelcast-docker.xml \ | ||
| LANG=C.UTF-8 \ | ||
| PATH=${HZ_HOME}/bin:$PATH | ||
| PATH=${HZ_HOME}/bin:$PATH \ | ||
| SOURCE_DATE_EPOCH=${SOURCE_DATE_EPOCH} | ||
|
|
||
| LABEL name="Hazelcast Enterprise" \ | ||
| maintainer="info@hazelcast.com" \ | ||
|
|
@@ -67,27 +77,29 @@ LABEL name="Hazelcast Enterprise" \ | |
| EXPOSE 5701 | ||
|
|
||
| COPY licenses /licenses | ||
| COPY --from=get-distribution ${HZ_HOME} ${HZ_HOME}/ | ||
|
|
||
| # Install | ||
| RUN echo "Upgrading packages" \ | ||
| COPY --link --from=get-distribution ${BUILD_ROOT}/ / | ||
|
|
||
| RUN touch /tmp/.timestamp-marker && sleep 1 \ | ||
| && echo "Adding non-root user" \ | ||
| && groupadd --system ${USER_GROUP} \ | ||
| && useradd --no-log-init --system --gid ${USER_GROUP} --create-home ${USER_NAME} \ | ||
| && chage --lastday 0 ${USER_NAME} \ | ||
| && echo "Upgrading packages" \ | ||
| && microdnf -y update --nodocs \ | ||
| && echo "Installing new packages" \ | ||
| && microdnf -y --nodocs --disablerepo=* --enablerepo=ubi-9-appstream-rpms --enablerepo=ubi-9-baseos-rpms \ | ||
| --disableplugin=subscription-manager install \ | ||
| java-${JDK_VERSION}-openjdk-headless \ | ||
| shadow-utils \ | ||
| tar \ | ||
| tzdata-java \ | ||
| util-linux \ | ||
| && echo "Removing unnecessary packages and redundant files/folders" \ | ||
| && microdnf -y clean all | ||
| && microdnf -y clean all \ | ||
| && truncate --size 0 /var/cache/ldconfig/aux-cache \ | ||
| && truncate --size 0 /var/lib/dnf/history.sqlite* \ | ||
| && echo "Fixing non-deterministic files for reproducibility" \ | ||
| && find / -newer /tmp/.timestamp-marker -xdev \ | ||
| -exec touch -h -d "@$SOURCE_DATE_EPOCH" {} + 2>/dev/null \ | ||
| && rm -f /tmp/.timestamp-marker \ | ||
| && touch -d "@${SOURCE_DATE_EPOCH}" /tmp | ||
|
|
||
|
|
||
| WORKDIR ${HZ_HOME} | ||
|
|
||
| RUN echo "Adding non-root user" \ | ||
| && groupadd --system ${USER_GROUP} \ | ||
| && useradd --no-log-init --system --gid ${USER_GROUP} --create-home ${USER_NAME} | ||
| USER ${USER_NAME} | ||
|
|
||
| HEALTHCHECK CMD ["hz-healthcheck"] | ||
|
|
||





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.
Have you considered about writing this in something other than
bash?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 but not here ;)