Repository navigation
ci(docker): support arm64 builds and multi-arch manifest assembly - #2132
Conversation
|
🔍 OpenCodeReview found 8 issue(s) in this PR.
📄
|
70bd008 to
3b3867e
Compare
693ebf8 to
ca5dff9
Compare
build.py gains --platform (amd64|arm64) and --tag_suffix: tags are suffixed inside build/push (with a local alias kept unsuffixed so app image builds can still reference the base images), and the version input stays unsuffixed to avoid double-suffixing. release_docker.yml becomes platform-aware via workflow_dispatch inputs (version, platform, tag_suffix, runner) so arm64 images can be built natively (ubuntu-24.04-arm) and pushed as '<tag>-arm64' suffixed tags. The new assemble_manifests input runs a registry-side job that turns per-arch suffixed tags into multi-arch manifest lists under the unsuffixed tags (docker buildx imagetools create; nothing rebuilt, idempotent snapshot of the historical amd64 layout under explicit '-amd64' tags first). Kept in the same workflow because GitHub validates workflow registrations against the default branch, so a separate dispatch-only workflow could not be used from a release branch. Note: single-arch release builds (workflow_call) behave exactly as before.
ca5dff9 to
10473d0
Compare
nullPointerEnjoyer
left a comment
There was a problem hiding this comment.
Approved: OCR review converged to 0 findings over 5 cycles, full check suite green, all 12 review threads resolved (permissions least-privilege, runner allowlist, tag_suffix validation, non-amd64 push guard, atomic assembly pre-flight, multi-arch verification, action SHA pins, timeouts, concurrency serialization).
- workflow_dispatch builds and assembly now require a protected ref (master or release-*), so ad-hoc runs from feature branches cannot push versioned or latest tags with the repo Docker Hub credentials - build and assembly jobs serialize behind fixed concurrency groups - latest is stamped only for semver versions; SHA builds (the dispatch default) keep their own tags only, and anything else fails early - version validation matches the whole value (same rule as the tag suffix), so a multi-line input cannot expand into extra tag arguments - assembly logs both source digests before creating a manifest list - build.py skips the local base-image alias when no tags were passed
| elif [[ -n "$INPUT_VERSION" ]]; then | ||
| VERSION="$INPUT_VERSION" | ||
| validate "$VERSION" | ||
| STAMP_LATEST=true |
There was a problem hiding this comment.
For a workflow_dispatch on a protected ref, the 'version' input is accepted as any semver string without being tied to the checked-out ref (unlike the tag path, which derives VERSION from GITHUB_REF). Since builds push '', 'v' and, with --latest, the shared 'latest' tag, an operator (or a compromised token with dispatch rights) can repoint already-released version tags with different content. Consider requiring, for dispatches, that the version matches the ref (tag dispatches) or at least warning/gating when the target tags already exist in the registry.
| echo "amd64 digest of $repo:$tag-amd64:" | ||
| docker buildx imagetools inspect "$repo:$tag-amd64" | grep '^Digest:' | ||
| echo "arm64 digest of $repo:$tag-arm64:" | ||
| docker buildx imagetools inspect "$repo:$tag-arm64" | grep '^Digest:' | ||
| docker buildx imagetools create -t "$repo:$tag" "$repo:$tag-amd64" "$repo:$tag-arm64" |
There was a problem hiding this comment.
The assemble() function combines whatever 'latest-amd64'/'latest-arm64' manifests currently exist in the registry into the shared production 'latest' and 'v' tags, without verifying they were built from the same commit/version. Two independently dispatched per-arch builds from different commits (as the inline provenance comment acknowledges) can be published as one multi-arch release. The digest logging is post-hoc audit only, not a gate. Consider recording the source digest (e.g. as a workflow output or build-time attestation/label) during each per-arch build and comparing them here before running the final 'imagetools create', or pinning 'create' to the exact digests captured during pre-flight.
Suggestion:
| echo "amd64 digest of $repo:$tag-amd64:" | |
| docker buildx imagetools inspect "$repo:$tag-amd64" | grep '^Digest:' | |
| echo "arm64 digest of $repo:$tag-arm64:" | |
| docker buildx imagetools inspect "$repo:$tag-arm64" | grep '^Digest:' | |
| docker buildx imagetools create -t "$repo:$tag" "$repo:$tag-amd64" "$repo:$tag-arm64" | |
| amd64_digest="$(docker buildx imagetools inspect "$repo:$tag-amd64" | grep '^Digest:' | awk '{print $2}')" | |
| arm64_digest="$(docker buildx imagetools inspect "$repo:$tag-arm64" | grep '^Digest:' | awk '{print $2}') | |
| # Compare against the digests recorded by the per-arch builds before repointing the shared tag. | |
| check_digests_match "$repo" "$tag" "$amd64_digest" "$arm64_digest" | |
| docker buildx imagetools create -t "$repo:$tag" "$repo:$tag-amd64@$amd64_digest" "$repo:$tag-arm64@$arm64_digest" |
Register the arm64 docker build support on the default branch.
--platform(amd64|arm64) and--tag_suffix— tags are suffixed inside build/push, a local unsuffixed alias keeps app-image FROM references working, and the version stays unsuffixed (no double-suffixing).<tag>-arm64suffixed tags; plus anassemble_manifestsdispatch mode whose job stitches per-arch tags into multi-arch manifest lists registry-side (docker buildx imagetools create, nothing rebuilt, idempotent amd64 snapshot). Kept inside the same workflow because GitHub validates workflow registrations against the default branch.Proven on the release branch: amd64 path untouched (regression dispatch), arm64 build+push dispatch green for v1.4.1 (all 8 images arm64 in registry). Rebased onto the current tip after the upstream history rewrite; the release-only commits (checkpoints, changelogs, arch smoke fixes) are intentionally NOT included here.