fix(build): use platform image-manifest digest, not attested index#13949
Conversation
With the containerd image store and BuildKit provenance attestations (the default), a built image is stored as an attested index whose top-level digest also covers the attestation manifest. That digest churns on every build even when the runnable content is unchanged, so compose recreated containers on every `up --build`. Compare the digest of the "image" kind manifest instead, selected for the target platform and restricted to locally available manifests, so it is deterministic and reflects only config + layers. Both the build and up sides of the staleness check go through the same selection, and registry-only images keep the Bake-reported digest. Fixes docker#13636 Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The fix correctly replaces the attested-index top-level digest with the platform-specific content-manifest digest to prevent spurious container recreation on repeated builds.
What was verified:
- The
contentDigestfunction'slen(available)==1fallback is intentional and safe: when only one platform manifest is available locally but doesn't match the service platform,getLocalImagesDigestsdetects the mismatch via a secondaryImageInspectcall and removes the entry — so the digest is never written toImageDigestLabel. - The use of
platforms.Default()ingetImageSummariesis correct for all realistic scenarios: single-platform cross-platform builds return the right manifest anyway (via the lone-available fallback), and multi-platform mismatches are caught by the downstream platform guard. manifestsSupportedcallingRuntimeAPIVersionis not a new failure mode: the result is cached after the first call (which happens in many earlier code paths duringcompose up), and daemon unavailability would fail the subsequentImageInspectcalls regardless.- The
contentDigestfunction and its six test cases comprehensively cover the important paths (no manifests, attested index, single-platform non-matching, multi-platform with match, unavailable manifests, and ambiguous no-match fallback).
There was a problem hiding this comment.
Pull request overview
This PR fixes unnecessary container recreation on docker compose up --build when using the containerd image store with BuildKit provenance attestations enabled by switching staleness detection from the top-level attested index digest to a deterministic, platform-relevant runnable-content digest (config + layers).
Changes:
- Add
contentDigest()selection logic to derive a stable per-platform image manifest digest and use it in local image summary resolution. - Use
ImageInspectWithManifests(true)(when supported) soImageInspectreturns per-manifest details needed for selecting the runnable-content digest. - Post-process Bake build results to substitute the Bake-reported attested index digest with the stable local content digest when the built image is available locally.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/compose/images.go | Adds manifest-support detection and contentDigest(); updates image summary IDs to use the selected content digest. |
| pkg/compose/images_test.go | Adds unit tests for contentDigest() and getImageSummaries() behavior across engine versions and missing images. |
| pkg/compose/build.go | Adjusts local-platform inspection to use the image name (since ImageSummary.ID now stores a content-manifest digest). |
| pkg/compose/build_bake.go | Rewrites Bake digests to stable local content digests for images loaded into the engine (best-effort). |
| // inspect by name, not img.ID: img.ID now holds a content-manifest | ||
| // digest (see contentDigest) which is not necessarily inspectable, | ||
| // whereas the image name always resolves. | ||
| inspect, err := s.apiClient().ImageInspect(ctx, imgName) |
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The PR correctly fixes the attested-index digest churn problem. The new contentDigest function is well-designed and handles all key scenarios:
- Single-platform attested images (primary fix): filters for
ManifestKindImage+Availablemanifests, then platform-matches → returns stable content digest instead of churning index digest. - Cross-platform services (
service.platform: linux/arm64on amd64 host): single-manifest fallback (len(available) == 1) returns the correct platform digest regardless of the host-platform matcher. - Multi-platform images (multiple available manifests, no host-platform match): falls back to
inspect.ID, which is intentional — the PR comment explicitly documents this case. - Legacy engines (API < 1.48):
manifestsSupportedgates theWithManifestsoption;contentDigestwith emptyManifestsreturnsinspect.ID(the plain image ID, correct for old stores). - Registry-only/push images:
errdefs.IsNotFoundingetImageSummariesskips them, preserving the Bake-reported digest.
The build.go fix (inspect by imgName instead of img.ID) correctly avoids inspecting a manifest digest directly, which may not be addressable the same way as a config ID on the containerd store.
Test coverage in images_test.go is thorough, covering the normal attested case, single-manifest fallback, multi-platform matching, unavailable manifests, attestation-only images, and the no-manifests legacy fallback.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
What I did
With the containerd image store and BuildKit provenance attestations (the default), a built image is stored as an attested index whose top-level digest also covers the attestation manifest. That digest churns on every build even when the runnable content is unchanged, so compose recreated containers on every
up --build.Compare the digest of the "image" kind manifest instead, selected for the target platform and restricted to locally available manifests, so it is deterministic and reflects only config + layers. Both the build and up sides of the staleness check go through the same selection, and registry-only images keep the Bake-reported digest.
Related issue
Fixes #13636
(not mandatory) A picture of a cute animal, if possible in relation to what you did