From 08580b47b0d1924c776f4f43abb822d1c2e318c9 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 3 Aug 2026 18:40:04 -0500 Subject: [PATCH 1/3] ci: skip container rebuild when the image already exists The container jobs ran on every CI run and rebuilt both arch images from registry cache. Even on a full cache hit that costs a checkout, buildx setup, cache pull and re-push per arch, plus three manifest pushes -- all on the critical path ahead of every depends, build and test job. Measured on this branch, that phase drops from 351s to 12s. Add a check job that derives a content key for the image and looks for the corresponding multi-arch manifest in ghcr. When it is present both build jobs and the manifest job are skipped. The manifest is pushed last, so its presence proves both arch builds completed; any error inspecting it falls through to a rebuild. The key covers everything that decides what gets built: every file in the build context, since ci.Dockerfile pulls in ci-slim.Dockerfile via dockerfile-x and hashing one file alone would let a sibling edit reuse a stale image; which Dockerfile the caller asked for, since both images share a context and would otherwise key alike; this workflow, because build-args, target and platforms live in the build step rather than in the context; and the manifest digest of every external image the Dockerfiles reference, since ubuntu:noble, debian:bookworm-slim and ghcr.io/astral-sh/uv:latest are floating tags that BuildKit re-resolves per build. Those references are found by parsing rather than listed by hand, so a newly added FROM cannot silently escape the key, and the step fails if the parser finds none. Unpinned apt packages and git refs that move under a fixed name are deliberately not covered. Adding them would achieve nothing: the key only names the image, their RUN command strings are unchanged, and a rebuild would restore byte-identical layers from cache. Pinning them in the Dockerfile, as CTCACHE_COMMIT already does, is what makes them move. Also switch the workflow output from the branch tag to the content key. Under pull_request_target GITHUB_REF is refs/pull/N/merge, so every open PR was sharing the mutable tag ':merge'. The build jobs guard on success() as well as the skip condition, since a bare custom 'if' drops the implicit requirement that needs succeeded, and the hash pipeline sets pipefail so a failing stage cannot yield a well-formed but wrong key. --- .github/workflows/build-container.yml | 202 ++++++++++++++++++++++---- 1 file changed, 177 insertions(+), 25 deletions(-) diff --git a/.github/workflows/build-container.yml b/.github/workflows/build-container.yml index 078a3e84a5ec..0ced21565bba 100644 --- a/.github/workflows/build-container.yml +++ b/.github/workflows/build-container.yml @@ -26,16 +26,17 @@ on: outputs: path: description: "Path to built container" - value: ghcr.io/${{ jobs.build-amd64.outputs.repo }}/${{ inputs.name }}:${{ jobs.build-amd64.outputs.tag }} + value: ghcr.io/${{ jobs.check.outputs.repo }}/${{ inputs.name }}:${{ jobs.check.outputs.hash-tag }} jobs: - build-amd64: - name: Build container (amd64) - runs-on: ${{ inputs.runs-on-amd64 }} + check: + name: Check for existing container + runs-on: ${{ inputs.runs-on-arm64 }} outputs: tag: ${{ steps.prepare.outputs.tag }} repo: ${{ steps.prepare.outputs.repo }} - digest: ${{ steps.build.outputs.digest }} + hash-tag: ${{ steps.prepare.outputs.hash-tag }} + exists: ${{ steps.exists.outputs.exists }} steps: - name: Checkout code uses: actions/checkout@v6 @@ -44,13 +45,167 @@ jobs: allow-unsafe-pr-checkout: true persist-credentials: false + # Must precede the digest lookups in "Prepare variables", which may need + # credentials to resolve a private image reference. + - name: Login to GitHub Container Registry + uses: docker/login-action@v4 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Prepare variables id: prepare + env: + CONTEXT: ${{ inputs.context }} + DOCKERFILE: ${{ inputs.file }} run: | + # Without pipefail a failing stage mid-pipeline still yields a + # well-formed hash, which would silently pin us to a wrong image. + set -o pipefail BRANCH_NAME=$(echo "${GITHUB_REF##*/}" | tr '[:upper:]' '[:lower:]') REPO_NAME=$(echo "${{ github.repository }}" | tr '[:upper:]' '[:lower:]') - echo "tag=${BRANCH_NAME}" >> "$GITHUB_OUTPUT" - echo "repo=${REPO_NAME}" >> "$GITHUB_OUTPUT" + + if [ -z "$(find "${CONTEXT}" -type f -print -quit)" ]; then + echo "::error::Build context '${CONTEXT}' contains no files" + exit 1 + fi + # Anything the image is built from has to live inside the hashed + # context, or edits to it would not invalidate the tag. + CTX_ABS=$(cd "${CONTEXT}" && pwd -P) + DF_ABS="$(cd "$(dirname "${DOCKERFILE}")" && pwd -P)/$(basename "${DOCKERFILE}")" + case "${DF_ABS}" in + "${CTX_ABS}"/*) ;; + *) echo "::error::Dockerfile '${DOCKERFILE}' is outside the hashed context '${CONTEXT}'" + exit 1 ;; + esac + + # Hash the whole context, not just the named Dockerfile: ci.Dockerfile + # pulls in ci-slim.Dockerfile via dockerfile-x, so hashing one file + # alone would let a sibling change reuse a stale image. + CONTEXT_SUM=$(find "${CONTEXT}" -type f -print0 | LC_ALL=C sort -z \ + | xargs -0 sha256sum) + + # BuildKit re-resolves every external image on a real build, so an + # upstream push to any of them changes the result. They must be in the + # key or that push is silently ignored. Discovered by parsing rather + # than listed by hand, so a newly added FROM cannot be forgotten. + echo "Resolving external image references:" + # Scans every file in the context, not a *.Dockerfile glob, so a + # differently named Dockerfile cannot quietly lose coverage. Skipping + # leading flags handles "FROM --platform=x img" and + # "COPY --chown=u:g --from=img"; taking the first non-flag token after + # FROM ignores the "AS stage" alias. Local dockerfile-x includes + # (./foo.Dockerfile) and stage names are then dropped, leaving only + # things resolvable from a registry. + # shellcheck disable=SC2016 # $1/$i are awk fields, not shell vars + EXTERNAL_REFS=$(find "${CONTEXT}" -type f -print0 | LC_ALL=C sort -z \ + | xargs -0 awk ' + toupper($1) == "FROM" { + for (i = 2; i <= NF; i++) + if ($i !~ /^--/) { print $i; break } + } + toupper($1) == "COPY" { + for (i = 2; i <= NF; i++) + if ($i ~ /^--from=/) { sub(/^--from=/, "", $i); print $i; break } + }' \ + | grep -vE '^\.' | grep -E '[./:]' | LC_ALL=C sort -u) + if [ -z "${EXTERNAL_REFS}" ]; then + echo "::error::Found no external image references; the parser is broken and drift in base images would go undetected" + exit 1 + fi + IMAGE_SUM="" + # Here-string, not a pipe: a piped `while` runs in a subshell and + # would discard IMAGE_SUM. + while IFS= read -r REF; do + [ -n "${REF}" ] || continue + RAW=$(docker buildx imagetools inspect --raw "${REF}" 2>/dev/null || true) + if [ -n "${RAW}" ]; then + REF_DIGEST=$(printf '%s' "${RAW}" | sha256sum | cut -d' ' -f1) + else + # Rate limit or outage. A distinct marker means we rebuild under a + # separate tag rather than silently reusing a possibly stale one; + # the normal tag is reused again once lookups recover. + REF_DIGEST="unresolved" + fi + echo " ${REF} -> ${REF_DIGEST}" + IMAGE_SUM="${IMAGE_SUM}${REF}=${REF_DIGEST}"$'\n' + done <<< "${EXTERNAL_REFS}" + + # Note that unpinned apt packages and git refs that move under a fixed + # name (IWYU's clang_NN branch, dash_hash's tag) are deliberately not + # covered. Adding them would achieve nothing: the key only names the + # image, their RUN command strings are unchanged, and a rebuild would + # restore byte-identical layers from cache. Pin them in the Dockerfile + # if they need to move, the way CTCACHE_COMMIT already does. + # + # The Dockerfile we were told to build is part of the key too. Both + # images share this context, so hashing only the directory gives them + # the same key, and repointing one image's file: input would otherwise + # silently reuse the image built from the old one. + # + # This workflow is hashed as well: build-args, target and platforms + # all change the image without touching a Dockerfile, and they live + # in the build step below rather than in the context. Note this covers + # settings written here, not values a caller passes in. Anything added + # to workflow_call.inputs that reaches the build step -- a build-args + # or target passthrough, say -- has to be added to this key too, or + # changing it in build.yml will silently reuse the old image. + WORKFLOW_FILE=".github/workflows/build-container.yml" + if [ ! -f "${WORKFLOW_FILE}" ]; then + echo "::error::${WORKFLOW_FILE} not found; the key would silently stop covering build settings" + exit 1 + fi + HASH_TAG=$(printf '%s\n%sdockerfile=%s\n%s\n' \ + "${CONTEXT_SUM}" "${IMAGE_SUM}" "${DF_ABS#"${CTX_ABS}/"}" \ + "$(sha256sum "${WORKFLOW_FILE}")" | sha256sum | cut -d' ' -f1) + echo "Content key: ${HASH_TAG}" + { + echo "tag=${BRANCH_NAME}" + echo "repo=${REPO_NAME}" + echo "hash-tag=${HASH_TAG}" + } >> "$GITHUB_OUTPUT" + + - name: Check whether the image was already built + id: exists + env: + REF: ghcr.io/${{ steps.prepare.outputs.repo }}/${{ inputs.name }}:${{ steps.prepare.outputs.hash-tag }} + run: | + # The multi-arch manifest is pushed last, so its presence means both + # arch-specific builds completed. Any failure here falls through to a + # rebuild, which is correct (just slower). + RAW=$(docker buildx imagetools inspect --raw "${REF}" 2>/dev/null || true) + COMPLETE=$(jq -r ' + [(.manifests // [])[] + | select(.platform.os == "linux") + | .platform.architecture] as $arch + | (($arch | index("amd64")) != null) and (($arch | index("arm64")) != null) + ' <<<"${RAW}" 2>/dev/null || true) + if [ "${COMPLETE}" = "true" ]; then + echo "Reusing existing image ${REF}" + echo "exists=true" >> "$GITHUB_OUTPUT" + else + echo "No complete multi-arch image at ${REF}, building" + echo "exists=false" >> "$GITHUB_OUTPUT" + fi + + build-amd64: + name: Build container (amd64) + needs: [check] + # success() is implicit for an `if` with no status function, so this is + # explicit rather than load-bearing: a failed check skips the build either + # way. Only a status function (always(), !cancelled()) would change that. + if: ${{ success() && needs.check.outputs.exists != 'true' }} + runs-on: ${{ inputs.runs-on-amd64 }} + outputs: + digest: ${{ steps.build.outputs.digest }} + steps: + - name: Checkout code + uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.head.sha }} + allow-unsafe-pr-checkout: true + persist-credentials: false - name: Set up Docker Buildx uses: docker/setup-buildx-action@v4 @@ -71,14 +226,19 @@ jobs: push: true platforms: linux/amd64 tags: | - ghcr.io/${{ steps.prepare.outputs.repo }}/${{ inputs.name }}:${{ hashFiles(inputs.file) }}-amd64 + ghcr.io/${{ needs.check.outputs.repo }}/${{ inputs.name }}:${{ needs.check.outputs.hash-tag }}-amd64 cache-from: | - type=registry,ref=ghcr.io/${{ steps.prepare.outputs.repo }}/${{ inputs.name }}:${{ hashFiles(inputs.file) }}-amd64 - type=registry,ref=ghcr.io/${{ steps.prepare.outputs.repo }}/${{ inputs.name }}:${{ steps.prepare.outputs.tag }} + type=registry,ref=ghcr.io/${{ needs.check.outputs.repo }}/${{ inputs.name }}:${{ needs.check.outputs.hash-tag }}-amd64 + type=registry,ref=ghcr.io/${{ needs.check.outputs.repo }}/${{ inputs.name }}:${{ needs.check.outputs.tag }} cache-to: type=inline build-arm64: name: Build container (arm64) + needs: [check] + # success() is implicit for an `if` with no status function, so this is + # explicit rather than load-bearing: a failed check skips the build either + # way. Only a status function (always(), !cancelled()) would change that. + if: ${{ success() && needs.check.outputs.exists != 'true' }} runs-on: ${{ inputs.runs-on-arm64 }} outputs: digest: ${{ steps.build.outputs.digest }} @@ -90,14 +250,6 @@ jobs: allow-unsafe-pr-checkout: true persist-credentials: false - - name: Prepare variables - id: prepare - run: | - BRANCH_NAME=$(echo "${GITHUB_REF##*/}" | tr '[:upper:]' '[:lower:]') - REPO_NAME=$(echo "${{ github.repository }}" | tr '[:upper:]' '[:lower:]') - echo "tag=${BRANCH_NAME}" >> "$GITHUB_OUTPUT" - echo "repo=${REPO_NAME}" >> "$GITHUB_OUTPUT" - - name: Set up Docker Buildx uses: docker/setup-buildx-action@v4 @@ -117,16 +269,16 @@ jobs: push: true platforms: linux/arm64 tags: | - ghcr.io/${{ steps.prepare.outputs.repo }}/${{ inputs.name }}:${{ hashFiles(inputs.file) }}-arm64 + ghcr.io/${{ needs.check.outputs.repo }}/${{ inputs.name }}:${{ needs.check.outputs.hash-tag }}-arm64 cache-from: | - type=registry,ref=ghcr.io/${{ steps.prepare.outputs.repo }}/${{ inputs.name }}:${{ hashFiles(inputs.file) }}-arm64 - type=registry,ref=ghcr.io/${{ steps.prepare.outputs.repo }}/${{ inputs.name }}:${{ steps.prepare.outputs.tag }} + type=registry,ref=ghcr.io/${{ needs.check.outputs.repo }}/${{ inputs.name }}:${{ needs.check.outputs.hash-tag }}-arm64 + type=registry,ref=ghcr.io/${{ needs.check.outputs.repo }}/${{ inputs.name }}:${{ needs.check.outputs.tag }} cache-to: type=inline create-manifest: name: Create multi-arch manifest runs-on: ${{ inputs.runs-on-arm64 }} - needs: [build-amd64, build-arm64] + needs: [check, build-amd64, build-arm64] steps: - name: Checkout code uses: actions/checkout@v6 @@ -147,9 +299,9 @@ jobs: - name: Create and push multi-arch manifest run: | - REPO="ghcr.io/${{ needs.build-amd64.outputs.repo }}/${{ inputs.name }}" - TAG="${{ needs.build-amd64.outputs.tag }}" - HASH_TAG="${{ hashFiles(inputs.file) }}" + REPO="ghcr.io/${{ needs.check.outputs.repo }}/${{ inputs.name }}" + TAG="${{ needs.check.outputs.tag }}" + HASH_TAG="${{ needs.check.outputs.hash-tag }}" # Create manifest from arch-specific images docker buildx imagetools create -t "${REPO}:${HASH_TAG}" \ From a6af3575ab91aa69af192670bbe13814a2fbcfb9 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 4 Aug 2026 00:42:34 -0500 Subject: [PATCH 2/3] fix(ci): harden container content-key checks from review feedback Address valid review findings without expanding the PR's scope: - hash the pull_request_target-executing workflow, not the PR-head copy - include # syntax frontend digests in the external-image key - force a rebuild when any external digest lookup is unresolved - pass shell inputs via env to avoid template injection - set up buildx in the check job before imagetools inspect --- .github/workflows/build-container.yml | 89 ++++++++++++++++++++++----- 1 file changed, 74 insertions(+), 15 deletions(-) diff --git a/.github/workflows/build-container.yml b/.github/workflows/build-container.yml index 0ced21565bba..575e4369f9e0 100644 --- a/.github/workflows/build-container.yml +++ b/.github/workflows/build-container.yml @@ -45,6 +45,25 @@ jobs: allow-unsafe-pr-checkout: true persist-credentials: false + # pull_request_target executes the base-branch workflow while the main + # checkout is the PR head. Grab the executing copy so the content key + # hashes what actually runs (see WORKFLOW_SUM below). + - name: Checkout executing workflow file + if: ${{ github.event_name == 'pull_request_target' }} + uses: actions/checkout@v6 + with: + ref: ${{ github.sha }} + sparse-checkout: | + .github/workflows/build-container.yml + sparse-checkout-cone-mode: false + path: .executing-workflow + persist-credentials: false + + # imagetools inspect is used below; ensure buildx is present on all + # runner images (stock GHA and custom labels). + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v4 + # Must precede the digest lookups in "Prepare variables", which may need # credentials to resolve a private image reference. - name: Login to GitHub Container Registry @@ -59,12 +78,13 @@ jobs: env: CONTEXT: ${{ inputs.context }} DOCKERFILE: ${{ inputs.file }} + REPOSITORY: ${{ github.repository }} run: | # Without pipefail a failing stage mid-pipeline still yields a # well-formed hash, which would silently pin us to a wrong image. set -o pipefail BRANCH_NAME=$(echo "${GITHUB_REF##*/}" | tr '[:upper:]' '[:lower:]') - REPO_NAME=$(echo "${{ github.repository }}" | tr '[:upper:]' '[:lower:]') + REPO_NAME=$(echo "${REPOSITORY}" | tr '[:upper:]' '[:lower:]') if [ -z "$(find "${CONTEXT}" -type f -print -quit)" ]; then echo "::error::Build context '${CONTEXT}' contains no files" @@ -95,12 +115,18 @@ jobs: # differently named Dockerfile cannot quietly lose coverage. Skipping # leading flags handles "FROM --platform=x img" and # "COPY --chown=u:g --from=img"; taking the first non-flag token after - # FROM ignores the "AS stage" alias. Local dockerfile-x includes - # (./foo.Dockerfile) and stage names are then dropped, leaving only - # things resolvable from a registry. + # FROM ignores the "AS stage" alias. "# syntax = frontend" is collected + # too: BuildKit fetches that floating frontend on every build. + # Local dockerfile-x includes (./foo.Dockerfile) and stage names are + # then dropped, leaving only things resolvable from a registry. # shellcheck disable=SC2016 # $1/$i are awk fields, not shell vars EXTERNAL_REFS=$(find "${CONTEXT}" -type f -print0 | LC_ALL=C sort -z \ | xargs -0 awk ' + /^#[[:space:]]*syntax[[:space:]]*=/ { + sub(/^#[[:space:]]*syntax[[:space:]]*=[[:space:]]*/, "") + print $1 + next + } toupper($1) == "FROM" { for (i = 2; i <= NF; i++) if ($i !~ /^--/) { print $i; break } @@ -115,18 +141,21 @@ jobs: exit 1 fi IMAGE_SUM="" + ANY_UNRESOLVED=false # Here-string, not a pipe: a piped `while` runs in a subshell and - # would discard IMAGE_SUM. + # would discard IMAGE_SUM / ANY_UNRESOLVED. while IFS= read -r REF; do [ -n "${REF}" ] || continue RAW=$(docker buildx imagetools inspect --raw "${REF}" 2>/dev/null || true) if [ -n "${RAW}" ]; then REF_DIGEST=$(printf '%s' "${RAW}" | sha256sum | cut -d' ' -f1) else - # Rate limit or outage. A distinct marker means we rebuild under a - # separate tag rather than silently reusing a possibly stale one; - # the normal tag is reused again once lookups recover. + # Rate limit or outage. Mark the key and force a rebuild for this + # run: reusing a prior "unresolved" image can hide base-image + # drift that happened between outages. Once lookups recover the + # digest-keyed path is used again. REF_DIGEST="unresolved" + ANY_UNRESOLVED=true fi echo " ${REF} -> ${REF_DIGEST}" IMAGE_SUM="${IMAGE_SUM}${REF}=${REF_DIGEST}"$'\n' @@ -151,26 +180,51 @@ jobs: # to workflow_call.inputs that reaches the build step -- a build-args # or target passthrough, say -- has to be added to this key too, or # changing it in build.yml will silently reuse the old image. + # + # Under pull_request_target the executing workflow is the base-branch + # copy (GITHUB_SHA), while the working tree is the PR head. Hash the + # version that actually runs so a PR cannot pre-seed a key for build + # settings it did not execute. WORKFLOW_FILE=".github/workflows/build-container.yml" - if [ ! -f "${WORKFLOW_FILE}" ]; then - echo "::error::${WORKFLOW_FILE} not found; the key would silently stop covering build settings" - exit 1 + if [ "${GITHUB_EVENT_NAME}" = "pull_request_target" ]; then + EXEC_WF=".executing-workflow/${WORKFLOW_FILE}" + if [ ! -f "${EXEC_WF}" ]; then + echo "::error::${EXEC_WF} not found; the key would silently stop covering build settings" + exit 1 + fi + WORKFLOW_SUM=$(sha256sum "${EXEC_WF}") + else + if [ ! -f "${WORKFLOW_FILE}" ]; then + echo "::error::${WORKFLOW_FILE} not found; the key would silently stop covering build settings" + exit 1 + fi + WORKFLOW_SUM=$(sha256sum "${WORKFLOW_FILE}") fi HASH_TAG=$(printf '%s\n%sdockerfile=%s\n%s\n' \ "${CONTEXT_SUM}" "${IMAGE_SUM}" "${DF_ABS#"${CTX_ABS}/"}" \ - "$(sha256sum "${WORKFLOW_FILE}")" | sha256sum | cut -d' ' -f1) + "${WORKFLOW_SUM}" | sha256sum | cut -d' ' -f1) echo "Content key: ${HASH_TAG}" { echo "tag=${BRANCH_NAME}" echo "repo=${REPO_NAME}" echo "hash-tag=${HASH_TAG}" + echo "unresolved=${ANY_UNRESOLVED}" } >> "$GITHUB_OUTPUT" - name: Check whether the image was already built id: exists env: REF: ghcr.io/${{ steps.prepare.outputs.repo }}/${{ inputs.name }}:${{ steps.prepare.outputs.hash-tag }} + UNRESOLVED: ${{ steps.prepare.outputs.unresolved }} run: | + # If any external digest lookup failed, force a rebuild. A prior + # image published under the same "unresolved" marker may predate a + # base-image change that we could not observe during the outage. + if [ "${UNRESOLVED}" = "true" ]; then + echo "External image lookup was unresolved; rebuilding rather than reusing ${REF}" + echo "exists=false" >> "$GITHUB_OUTPUT" + exit 0 + fi # The multi-arch manifest is pushed last, so its presence means both # arch-specific builds completed. Any failure here falls through to a # rebuild, which is correct (just slower). @@ -298,10 +352,15 @@ jobs: password: ${{ secrets.GITHUB_TOKEN }} - name: Create and push multi-arch manifest + env: + CHECK_REPO: ${{ needs.check.outputs.repo }} + IMAGE_NAME: ${{ inputs.name }} + CHECK_TAG: ${{ needs.check.outputs.tag }} + CHECK_HASH_TAG: ${{ needs.check.outputs.hash-tag }} run: | - REPO="ghcr.io/${{ needs.check.outputs.repo }}/${{ inputs.name }}" - TAG="${{ needs.check.outputs.tag }}" - HASH_TAG="${{ needs.check.outputs.hash-tag }}" + REPO="ghcr.io/${CHECK_REPO}/${IMAGE_NAME}" + TAG="${CHECK_TAG}" + HASH_TAG="${CHECK_HASH_TAG}" # Create manifest from arch-specific images docker buildx imagetools create -t "${REPO}:${HASH_TAG}" \ From 7f16d31f6aba20a84eac2b0da40dbbad423598ef Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 7 Aug 2026 13:43:02 -0500 Subject: [PATCH 3/3] fix(ci): convert container content-key scripts to Python Review feedback: replace the check job's shell scripts with Python (knst) so hashing and discovery failures fail the step closed without pipefail/errexit care in every pipeline; track FROM ... AS stage aliases so bare external references like 'FROM alpine' stay in the key instead of being dropped by a punctuation filter; hash only the workflow file's digest, never its checkout path, so a pull_request_target run and the post-merge push run of identical workflow bytes share one content key. --- .github/workflows/build-container.yml | 240 +++++++++++++------------- 1 file changed, 118 insertions(+), 122 deletions(-) diff --git a/.github/workflows/build-container.yml b/.github/workflows/build-container.yml index 575e4369f9e0..57a947176119 100644 --- a/.github/workflows/build-container.yml +++ b/.github/workflows/build-container.yml @@ -73,93 +73,92 @@ jobs: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} + # Python rather than shell (review request): an unexpected failure in + # any hashing or discovery stage raises and fails the step closed, with + # no pipefail/errexit subtleties. - name: Prepare variables id: prepare env: CONTEXT: ${{ inputs.context }} DOCKERFILE: ${{ inputs.file }} REPOSITORY: ${{ github.repository }} + shell: python3 {0} run: | - # Without pipefail a failing stage mid-pipeline still yields a - # well-formed hash, which would silently pin us to a wrong image. - set -o pipefail - BRANCH_NAME=$(echo "${GITHUB_REF##*/}" | tr '[:upper:]' '[:lower:]') - REPO_NAME=$(echo "${REPOSITORY}" | tr '[:upper:]' '[:lower:]') - - if [ -z "$(find "${CONTEXT}" -type f -print -quit)" ]; then - echo "::error::Build context '${CONTEXT}' contains no files" - exit 1 - fi + import hashlib, os, re, subprocess, sys + + def die(message): + print(f"::error::{message}") + sys.exit(1) + + context = os.path.realpath(os.environ["CONTEXT"]) # Anything the image is built from has to live inside the hashed # context, or edits to it would not invalidate the tag. - CTX_ABS=$(cd "${CONTEXT}" && pwd -P) - DF_ABS="$(cd "$(dirname "${DOCKERFILE}")" && pwd -P)/$(basename "${DOCKERFILE}")" - case "${DF_ABS}" in - "${CTX_ABS}"/*) ;; - *) echo "::error::Dockerfile '${DOCKERFILE}' is outside the hashed context '${CONTEXT}'" - exit 1 ;; - esac + dockerfile = os.path.relpath(os.path.realpath(os.environ["DOCKERFILE"]), context) + if dockerfile.startswith(".."): + die(f"Dockerfile '{os.environ['DOCKERFILE']}' is outside the hashed context") + + files = sorted(os.path.join(root, name) + for root, _, names in os.walk(context) for name in names) + if not files: + die(f"Build context '{os.environ['CONTEXT']}' contains no files") # Hash the whole context, not just the named Dockerfile: ci.Dockerfile # pulls in ci-slim.Dockerfile via dockerfile-x, so hashing one file # alone would let a sibling change reuse a stale image. - CONTEXT_SUM=$(find "${CONTEXT}" -type f -print0 | LC_ALL=C sort -z \ - | xargs -0 sha256sum) + key = hashlib.sha256() + for path in files: + with open(path, "rb") as fh: + content = hashlib.sha256(fh.read()).hexdigest() + key.update(f"{os.path.relpath(path, context)} {content}\n".encode()) # BuildKit re-resolves every external image on a real build, so an # upstream push to any of them changes the result. They must be in the # key or that push is silently ignored. Discovered by parsing rather - # than listed by hand, so a newly added FROM cannot be forgotten. - echo "Resolving external image references:" - # Scans every file in the context, not a *.Dockerfile glob, so a - # differently named Dockerfile cannot quietly lose coverage. Skipping - # leading flags handles "FROM --platform=x img" and - # "COPY --chown=u:g --from=img"; taking the first non-flag token after - # FROM ignores the "AS stage" alias. "# syntax = frontend" is collected - # too: BuildKit fetches that floating frontend on every build. - # Local dockerfile-x includes (./foo.Dockerfile) and stage names are - # then dropped, leaving only things resolvable from a registry. - # shellcheck disable=SC2016 # $1/$i are awk fields, not shell vars - EXTERNAL_REFS=$(find "${CONTEXT}" -type f -print0 | LC_ALL=C sort -z \ - | xargs -0 awk ' - /^#[[:space:]]*syntax[[:space:]]*=/ { - sub(/^#[[:space:]]*syntax[[:space:]]*=[[:space:]]*/, "") - print $1 - next - } - toupper($1) == "FROM" { - for (i = 2; i <= NF; i++) - if ($i !~ /^--/) { print $i; break } - } - toupper($1) == "COPY" { - for (i = 2; i <= NF; i++) - if ($i ~ /^--from=/) { sub(/^--from=/, "", $i); print $i; break } - }' \ - | grep -vE '^\.' | grep -E '[./:]' | LC_ALL=C sort -u) - if [ -z "${EXTERNAL_REFS}" ]; then - echo "::error::Found no external image references; the parser is broken and drift in base images would go undetected" - exit 1 - fi - IMAGE_SUM="" - ANY_UNRESOLVED=false - # Here-string, not a pipe: a piped `while` runs in a subshell and - # would discard IMAGE_SUM / ANY_UNRESOLVED. - while IFS= read -r REF; do - [ -n "${REF}" ] || continue - RAW=$(docker buildx imagetools inspect --raw "${REF}" 2>/dev/null || true) - if [ -n "${RAW}" ]; then - REF_DIGEST=$(printf '%s' "${RAW}" | sha256sum | cut -d' ' -f1) - else - # Rate limit or outage. Mark the key and force a rebuild for this - # run: reusing a prior "unresolved" image can hide base-image - # drift that happened between outages. Once lookups recover the - # digest-keyed path is used again. - REF_DIGEST="unresolved" - ANY_UNRESOLVED=true - fi - echo " ${REF} -> ${REF_DIGEST}" - IMAGE_SUM="${IMAGE_SUM}${REF}=${REF_DIGEST}"$'\n' - done <<< "${EXTERNAL_REFS}" + # than listed by hand, so a newly added FROM cannot be forgotten; + # "# syntax = frontend" counts too, BuildKit fetches that floating + # frontend on every build. Build stages, scratch, numeric stage + # indexes and local dockerfile-x includes (./foo.Dockerfile) are not + # registry images; everything else is, including bare names such as + # "FROM alpine". + refs, stages = set(), set() + for path in files: + with open(path, encoding="utf-8", errors="replace") as fh: + for line in fh: + tokens = line.split() + syntax = re.match(r"#\s*syntax\s*=\s*(\S+)", line.strip()) + if syntax: + refs.add(syntax.group(1)) + elif tokens and tokens[0].upper() == "FROM": + words = [t for t in tokens[1:] if not t.startswith("--")] + if words: + refs.add(words[0]) + if len(words) >= 3 and words[1].upper() == "AS": + stages.add(words[2]) + elif tokens and tokens[0].upper() == "COPY": + refs.update(t[len("--from="):] for t in tokens[1:] + if t.startswith("--from=")) + refs = {r for r in refs - stages + if not r.startswith((".", "/")) and r != "scratch" and not r.isdigit()} + if not refs: + die("Found no external image references; the parser is broken " + "and drift in base images would go undetected") + + print("Resolving external image references:") + unresolved = False + for ref in sorted(refs): + result = subprocess.run( + ["docker", "buildx", "imagetools", "inspect", "--raw", ref], + capture_output=True) + if result.returncode == 0 and result.stdout: + digest = hashlib.sha256(result.stdout).hexdigest() + else: + # Rate limit or outage. Mark the key and force a rebuild for + # this run: reusing a prior "unresolved" image can hide + # base-image drift that happened between outages. Once + # lookups recover the digest-keyed path is used again. + digest, unresolved = "unresolved", True + print(f" {ref} -> {digest}") + key.update(f"{ref}={digest}\n".encode()) # Note that unpinned apt packages and git refs that move under a fixed # name (IWYU's clang_NN branch, dash_hash's tag) are deliberately not @@ -172,7 +171,8 @@ jobs: # images share this context, so hashing only the directory gives them # the same key, and repointing one image's file: input would otherwise # silently reuse the image built from the old one. - # + key.update(f"dockerfile={dockerfile}\n".encode()) + # This workflow is hashed as well: build-args, target and platforms # all change the image without touching a Dockerfile, and they live # in the build step below rather than in the context. Note this covers @@ -184,64 +184,60 @@ jobs: # Under pull_request_target the executing workflow is the base-branch # copy (GITHUB_SHA), while the working tree is the PR head. Hash the # version that actually runs so a PR cannot pre-seed a key for build - # settings it did not execute. - WORKFLOW_FILE=".github/workflows/build-container.yml" - if [ "${GITHUB_EVENT_NAME}" = "pull_request_target" ]; then - EXEC_WF=".executing-workflow/${WORKFLOW_FILE}" - if [ ! -f "${EXEC_WF}" ]; then - echo "::error::${EXEC_WF} not found; the key would silently stop covering build settings" - exit 1 - fi - WORKFLOW_SUM=$(sha256sum "${EXEC_WF}") - else - if [ ! -f "${WORKFLOW_FILE}" ]; then - echo "::error::${WORKFLOW_FILE} not found; the key would silently stop covering build settings" - exit 1 - fi - WORKFLOW_SUM=$(sha256sum "${WORKFLOW_FILE}") - fi - HASH_TAG=$(printf '%s\n%sdockerfile=%s\n%s\n' \ - "${CONTEXT_SUM}" "${IMAGE_SUM}" "${DF_ABS#"${CTX_ABS}/"}" \ - "${WORKFLOW_SUM}" | sha256sum | cut -d' ' -f1) - echo "Content key: ${HASH_TAG}" - { - echo "tag=${BRANCH_NAME}" - echo "repo=${REPO_NAME}" - echo "hash-tag=${HASH_TAG}" - echo "unresolved=${ANY_UNRESOLVED}" - } >> "$GITHUB_OUTPUT" + # settings it did not execute. Only the file's digest goes into the + # key, never its path, so a PR run and the post-merge push run of + # identical workflow bytes share one key and reuse one image. + workflow = ".github/workflows/build-container.yml" + if os.environ.get("GITHUB_EVENT_NAME") == "pull_request_target": + workflow = os.path.join(".executing-workflow", workflow) + try: + with open(workflow, "rb") as fh: + key.update(hashlib.sha256(fh.read()).hexdigest().encode()) + except OSError: + die(f"{workflow} not found; the key would silently stop covering build settings") + + hash_tag = key.hexdigest() + print(f"Content key: {hash_tag}") + with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as out: + out.write(f"tag={os.environ['GITHUB_REF'].rsplit('/', 1)[-1].lower()}\n") + out.write(f"repo={os.environ['REPOSITORY'].lower()}\n") + out.write(f"hash-tag={hash_tag}\n") + out.write(f"unresolved={str(unresolved).lower()}\n") - name: Check whether the image was already built id: exists env: REF: ghcr.io/${{ steps.prepare.outputs.repo }}/${{ inputs.name }}:${{ steps.prepare.outputs.hash-tag }} UNRESOLVED: ${{ steps.prepare.outputs.unresolved }} + shell: python3 {0} run: | - # If any external digest lookup failed, force a rebuild. A prior - # image published under the same "unresolved" marker may predate a - # base-image change that we could not observe during the outage. - if [ "${UNRESOLVED}" = "true" ]; then - echo "External image lookup was unresolved; rebuilding rather than reusing ${REF}" - echo "exists=false" >> "$GITHUB_OUTPUT" - exit 0 - fi - # The multi-arch manifest is pushed last, so its presence means both - # arch-specific builds completed. Any failure here falls through to a - # rebuild, which is correct (just slower). - RAW=$(docker buildx imagetools inspect --raw "${REF}" 2>/dev/null || true) - COMPLETE=$(jq -r ' - [(.manifests // [])[] - | select(.platform.os == "linux") - | .platform.architecture] as $arch - | (($arch | index("amd64")) != null) and (($arch | index("arm64")) != null) - ' <<<"${RAW}" 2>/dev/null || true) - if [ "${COMPLETE}" = "true" ]; then - echo "Reusing existing image ${REF}" - echo "exists=true" >> "$GITHUB_OUTPUT" - else - echo "No complete multi-arch image at ${REF}, building" - echo "exists=false" >> "$GITHUB_OUTPUT" - fi + import json, os, subprocess + + ref = os.environ["REF"] + exists = False + if os.environ["UNRESOLVED"] == "true": + # If any external digest lookup failed, force a rebuild. A prior + # image published under the same "unresolved" marker may predate + # a base-image change we could not observe during the outage. + print(f"External image lookup was unresolved; rebuilding rather than reusing {ref}") + else: + # The multi-arch manifest is pushed last, so its presence means + # both arch-specific builds completed. Any failure here falls + # through to a rebuild, which is correct (just slower). + result = subprocess.run( + ["docker", "buildx", "imagetools", "inspect", "--raw", ref], + capture_output=True) + try: + platforms = {(m["platform"]["os"], m["platform"]["architecture"]) + for m in json.loads(result.stdout)["manifests"]} + exists = {("linux", "amd64"), ("linux", "arm64")} <= platforms + except Exception: + exists = False + print(f"Reusing existing image {ref}" if exists + else f"No complete multi-arch image at {ref}, building") + + with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as out: + out.write(f"exists={str(exists).lower()}\n") build-amd64: name: Build container (amd64)