fix(config): pin type:image volume sources and pre_start hook images#13956
fix(config): pin type:image volume sources and pre_start hook images#13956glours wants to merge 1 commit into
Conversation
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
One medium-confidence finding in the new --no-interpolate code path.
89dd5fc to
b5e4ddd
Compare
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
Two medium-confidence findings in the new --no-interpolate image-digest resolution path. Both are in the newly added lockModel and resolveImageDigests functions in cmd/compose/config.go.
b5e4ddd to
fcd9344
Compare
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The PR cleanly delegates image resolution for type: image volume sources and pre_start hook images to WithImagesResolved rather than duplicating logic CLI-side. The new resolveImageDigests, preStartHooks, and imageVolumes helpers in the --no-interpolate path are well-structured: both population and writeback use the same deterministic filtering functions, keeping slice indices aligned. The lockModel function correctly limits --lock-image-digests output to image/type-image-volume fields. Test coverage in config_test.go and publish_test.go covers the main scenarios.
Rely on compose-go WithImagesResolved, which now resolves dependent images — `type: image` volume sources and pre_start hook images — with its already-digested guard, per-call memoization and sibling-service detection (compose-spec/compose-go#894, docker#899), rather than duplicating resolution logic CLI-side. The interpolated path gets this for free; --no-interpolate maps the raw model onto a pseudo-project keyed by service names to reuse the same resolution, and --lock-image-digests keeps type:image volumes in its output. pre_start hooks can't be carried into the lock override (hook lists are appended on merge), so generating a lock warns that hook images stay unpinned there. As a side effect, `compose publish` now fails fast on unresolvable dependent images. Fixes docker#13827 Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
fcd9344 to
1fa3397
Compare
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The diff refactors cmd/compose/config.go to delegate image digest resolution for type: image volume sources and pre_start hook images to compose-go's WithImagesResolved, rather than duplicating that logic CLI-side. Key correctness properties verified:
- Index alignment between pseudo-project construction and write-back loops is guaranteed — both use the same raw service map and helper functions with unchanged length.
- Map reference semantics are correctly used; the removed
services[name] = servicereassignment was redundant. - Hook filtering by
preStartHooksis consistent between construction and write-back, preserving index alignment even for non-map entries. - Tests are comprehensive and cover the new code paths.
No bugs introduced by this PR.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
What I did
Rely on compose-go WithImagesResolved, which now resolves dependent images —
type: imagevolume sources and pre_start hook images — with its already-digested guard, per-call memoization and sibling-service detection (compose-spec/compose-go#894, #899), rather than duplicating resolution logic CLI-side. The interpolated path gets this for free; --no-interpolate maps the raw model onto a pseudo-project keyed by service names to reuse the same resolution, and --lock-image-digests keepstype: imagevolumes in its output. As a side effect,compose publishnow fails fast on unresolvable dependent images.Related issue
Fixes #13827
(not mandatory) A picture of a cute animal, if possible in relation to what you did
