fix(dry-run): commit no longer creates a real image; the interception set becomes declarative - #14150
fix(dry-run): commit no longer creates a real image; the interception set becomes declarative#14150ndeloof wants to merge 1 commit into
Conversation
… set becomes declarative compose commit --dry-run delegated ContainerCommit to the real client and actually created the image. It now returns a dry-run ID without touching the engine. Auditing the rest of the client surface: 39 more mutating operations were delegated — none that compose itself issues (verified caller by caller), but any future code path reaching one would mutate the engine under --dry-run silently. They now refuse with an explicit error. The classification — read-only delegated / faked / refused — was only implied by a method's position under a comment; it is now declared in dryrunclient_test.go, which parses the client and fails on any method missing, misclassified, or new and unclassified: extending APIClient requires an explicit dry-run decision. The commit e2e scenario gains a --dry-run step asserting the image is NOT created (new ImageAbsent check). Epic docker#14074, B.7. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The changes are mechanically correct and well-structured. The 39 new refused stubs follow a consistent, uniform pattern. The AST-based classification test (TestDryRunClientClassifiesEveryMethod) provides strong bidirectional coverage — it catches both unclassified new methods and classified names that no longer exist on DryRunClient. ContainerCommit now returns a synthetic dry-run ID instead of delegating to the real engine (defense-in-depth, since pkg/compose/commit.go already short-circuits on s.dryRun). The new ImageAbsent check and the e2e dry-run commit step are logically sound.
2 participants
Epic #14074, B.7.
compose commit --dry-rundelegatedContainerCommitto the real client and actually created the image; it now returns a dry-run ID without touching the engine (e2e step added, with a newImageAbsentcheck).Auditing the rest of the surface: 39 more mutating operations were delegated — none that compose itself issues today (verified caller by caller), but any future code path reaching one would mutate the engine under
--dry-runsilently. They now refuse with an explicit<op> is not allowed in dry-run mode.The interception set was only implied by a method's position under a comment; it is now declared in
dryrunclient_test.go(read-only delegated / faked / refused), with an AST-based test that fails on any method missing, misclassified, or new and unclassified — extendingAPIClientrequires an explicit dry-run decision.🤖 Generated with Claude Code