Skip to content

fix: digest divergence on dual-push buildx builds - #764

Open
bthuilot wants to merge 2 commits into
mainfrom
bthuilot/fix/digest-divergence
Open

fix: digest divergence on dual-push buildx builds#764
bthuilot wants to merge 2 commits into
mainfrom
bthuilot/fix/digest-divergence

Conversation

@bthuilot

@bthuilot bthuilot commented Sep 2, 2026

Copy link
Copy Markdown

Fix divergent container digests when running dual-push (chalkular mode) via buildx

claude summary of changes ## Description

When a build pushes via docker buildx build --output type=image,... with no -t tag, 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, so image.name, containerimage.digest, containerimage.config.digest and containerimage.descriptor all came back describing chalk's copy rather than the image the build pushed. --iidfile is 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-file targeted an image that was never pushed to its own repository. docker tag, docker pull, aws ecr put-image and 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:

  1. src/docker/build.nim — new addOutput. chalk's --output is 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 --output order on the command line, so this is sufficient. Falls back to appending when the build has no --output of its own.

  2. src/docker/build.nim — new DIGEST_OUTPUT_PARAMS / getDigestOutputParams. chalk's exporter now mirrors compression, compression-level, force-compression and oci-mediatypes from 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.

  3. src/docker/cmdline.nim + collectAfterBuild — stop inventing a :latest tag for push-by-digest=true builds. 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 from image.name in collectAfterBuild.

  4. 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. collectImageFrom resolved a repository's tags with fetchListOrImageManifest, 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 a could not match docker image tag warning — dropping chalk's tag from _REPO_TAGS and 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_images asserts _REPO_LIST_DIGESTS strictly, including MISSING expectations, 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 build accepts exactly one --metadata-file and buildkit writes one merged response per invocation. Exporter ordering is what delivers the same outcome.

_REPO_DIGESTS was not affected by the index divergence, for what it is worth: collectImageManifest re-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:

make tests args="-k push_by_digest"
  • test_push_by_digest_metadata_file — covers fixes 1–3. Kept at provenance=False; with attestations on, its "one digest describes both copies" assertion is false by construction.
  • test_push_by_digest_provenance_indexes — covers fix 4, with provenance=True so the two copies genuinely land under different indexes.

Supporting harness changes:

  • tests/functional/utils/docker.pybuild_cmd/build gain outputs and metadata_file; new manifest_exists() helper (uses crane manifest, since a manifest digest is not a blob digest and the existing blob_exists would 404 on it). --output now raises if combined with --load.
  • tests/functional/chalk/runner.pydocker_build forwards both params.
  • tests/functional/data/configs/docker_push_by_digest.c4m — new. Adds the second push destination. Deliberately not docker_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:

assertion before after
image.name is the build's repo fails — reports chalk's registry passes
reported digest resolves in the build's repo fails — HTTP 404 passes — HTTP 200
same digest resolves in chalk's repo passes passes
no DOCKER_TAGS recorded fails — [".../together-web:latest"] passes

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 collectAfterBuild branches on isDockerOverlayFS() and CI defaults to the containerd snapshotter:

_REPO_DIGESTS _REPO_LIST_DIGESTS _REPO_TAGS
before (overlay2 / overlayfs) both repos build's repo only empty
after (overlay2 / overlayfs) both repos both repos chalk's tag present

Every 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 tag warnings remain. A single-exporter -t build 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):

  • before: image.name = chalk's registry; its digest 404s in the build's repository. Layers in chalk's copy were tar+gzip (buildx default, ignoring the requested compression=zstd).
  • after: image.name = the build's repository, digest resolves there, and both registries share one identical image manifest with tar+zstd layers.

Worth knowing when reproducing: push-by-digest is rejected on the default docker driver ("push-by-digest is currently not implemented for docker driver"), so this code path only runs under a docker-container or remote builder. The suite's insecure_builder is one.

Signed-off-by: Bryce Thuilot <bryce@crashoverride.com>
Signed-off-by: Bryce Thuilot <bryce@crashoverride.com>
@bthuilot
bthuilot force-pushed the bthuilot/fix/digest-divergence branch from ccb31b9 to 90d1618 Compare September 2, 2026 18:20
@bthuilot
bthuilot marked this pull request as ready for review September 2, 2026 18:47
@bthuilot
bthuilot requested a review from viega as a code owner September 2, 2026 18:47
Comment thread src/docker/build.nim
for param in DIGEST_OUTPUT_PARAMS:
if param in output:
result &= param & "=" & output[param] & ","
break

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants