fix: digest divergence on dual-push buildx builds - #764
Open
bthuilot wants to merge 2 commits into
Open
Conversation
Signed-off-by: Bryce Thuilot <bryce@crashoverride.com>
Signed-off-by: Bryce Thuilot <bryce@crashoverride.com>
bthuilot
force-pushed
the
bthuilot/fix/digest-divergence
branch
from
September 2, 2026 18:20
ccb31b9 to
90d1618
Compare
bthuilot
marked this pull request as ready for review
September 2, 2026 18:47
drraid
reviewed
Sep 10, 2026
| for param in DIGEST_OUTPUT_PARAMS: | ||
| if param in output: | ||
| result &= param & "=" & output[param] & "," | ||
| break |
Contributor
There was a problem hiding this comment.
I'm reviewing this completely without context, so this comment might not make sense, but is it intended that there will be only one match in the larger outer loop? Because on the first match in the outer loop will unconditionally break at the end. Since there is more than one possibly matching term ("image" or "registry") in the conditional if/continue block, and because the result string appends a "," to the end of its construction, is this meant to service more than one match?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix divergent container digests when running dual-push (chalkular mode) via buildx
claude summary of changes
## DescriptionWhen a build pushes via
docker buildx build --output type=image,...with no-ttag, and chalk is also configured to push the image to a registry of its own, the invocation ends up with two image exporters. That single fact causes four separate defects, fixed here.buildkit merges the response of every exporter into the single
--metadata-file, with the last exporter winning duplicated keys (solver/llbsolver/export.go, which still carries a// TODO: separate these out). chalk appended its exporter last, soimage.name,containerimage.digest,containerimage.config.digestandcontainerimage.descriptorall came back describing chalk's copy rather than the image the build pushed.--iidfileis written from the same merged map (getImageID(resp.ExporterResponse)in buildx), so it was wrong too.The user-visible effect: every downstream step that reads the digest out of
--metadata-filetargeted an image that was never pushed to its own repository.docker tag,docker pull,aws ecr put-imageand any Helm/Argo reference built from that value fail with a not-found error, even though the build exits 0 and pushes correctly. chalk hit the same 404 itself while trying to record the image, which is why affected images showed up with no digest.The fixes:
src/docker/build.nim— newaddOutput. chalk's--outputis now inserted before any exporter the build already had, instead of appended, so the build's own exporter wins the metadata merge. Exporter order follows--outputorder on the command line, so this is sufficient. Falls back to appending when the build has no--outputof its own.src/docker/build.nim— newDIGEST_OUTPUT_PARAMS/getDigestOutputParams. chalk's exporter now mirrorscompression,compression-level,force-compressionandoci-mediatypesfrom the build's image exporter. buildkit derives the manifest digest from the exported layer blobs, so without this the two copies got different digests and neither digest described both images.src/docker/cmdline.nim+collectAfterBuild— stop inventing a:latesttag forpush-by-digest=truebuilds. buildkit publishes the manifest by digest only and never creates a tag, so the tag-less name is now reported without one. This was being fabricated in two places:extractOutputs, and again fromimage.nameincollectAfterBuild.src/docker/collect.nim— tolerate one image under several OCI indexes. Even with fix 2, the two copies do not end up under the same index: buildkit attaches a provenance attestation per exporter and the attestation records the image name, so each copy gets its own index digest over an identical image manifest.collectImageFromresolved a repository's tags withfetchListOrImageManifest, which returns the index when the image is under one, and then required that digest to be already known. chalk's own index digest never was, so the guard discarded the tag with acould not match docker image tagwarning — dropping chalk's tag from_REPO_TAGSand its registry from_REPO_LIST_DIGESTS. Tags are now also matched on the platform image manifest, and the newly discovered index digest is recorded.This branch fires only when the index digest is otherwise unknown, so single-exporter builds keep the original code path. That is deliberate:
test_base_imagesasserts_REPO_LIST_DIGESTSstrictly, includingMISSINGexpectations, and unconditionally recording resolved index digests would break it.Note that giving chalk a separate metadata file for its own bookkeeping is not implementable —
buildx buildaccepts exactly one--metadata-fileand buildkit writes one merged response per invocation. Exporter ordering is what delivers the same outcome._REPO_DIGESTSwas not affected by the index divergence, for what it is worth:collectImageManifestre-stamps every repository with the platform image manifest digest rather than the index digest, and fix 2 makes that manifest identical across both copies, so it already resolved in both.Testing
Two new functional tests in
tests/functional/test_docker.py:test_push_by_digest_metadata_file— covers fixes 1–3. Kept atprovenance=False; with attestations on, its "one digest describes both copies" assertion is false by construction.test_push_by_digest_provenance_indexes— covers fix 4, withprovenance=Trueso the two copies genuinely land under different indexes.Supporting harness changes:
tests/functional/utils/docker.py—build_cmd/buildgainoutputsandmetadata_file; newmanifest_exists()helper (usescrane manifest, since a manifest digest is not a blob digest and the existingblob_existswould 404 on it).--outputnow raises if combined with--load.tests/functional/chalk/runner.py—docker_buildforwards both params.tests/functional/data/configs/docker_push_by_digest.c4m— new. Adds the second push destination. Deliberately notdocker_wrap.c4m, whose mirror registry (:5048) requires auth and so cannot be queried by digest without credentials; both copies go to the same insecure registry instead.All new parameters default to
None, so existing callers are unaffected.Fixes 1–3
Asserted against the build's own repository and chalk's mirror:
image.nameis the build's repoDOCKER_TAGSrecorded[".../together-web:latest"]Three of the four fail on the unfixed binary. The third only has value in combination with the second: both can hold simultaneously only if the reported digest belongs to the build and is byte-identical to chalk's copy, which is what fix 2 buys.
Fix 4
Measured on both storage drivers, since
collectAfterBuildbranches onisDockerOverlayFS()and CI defaults to the containerd snapshotter:_REPO_DIGESTS_REPO_LIST_DIGESTS_REPO_TAGSEvery reported digest was checked against the registries: the shared image manifest returns 200 in both repositories; each index returns 200 only in its own repository and 404 in the other; the reported tag resolves to chalk's index. No
could not match docker image tagwarnings remain. A single-exporter-tbuild was re-run on both drivers as a regression check and is unchanged.Manual verification
Also verified outside the suite against two local registries, with binaries built from identical trees except the fix, using the reporter's exact command line (
push-by-digest=true,name-canonical=true,push=true,compression=zstd,force-compression=true,oci-mediatypes=true):image.name= chalk's registry; its digest 404s in the build's repository. Layers in chalk's copy weretar+gzip(buildx default, ignoring the requestedcompression=zstd).image.name= the build's repository, digest resolves there, and both registries share one identical image manifest withtar+zstdlayers.Worth knowing when reproducing:
push-by-digestis rejected on the defaultdockerdriver ("push-by-digest is currently not implemented for docker driver"), so this code path only runs under adocker-containeror remote builder. The suite'sinsecure_builderis one.