From ebdc66051a57e21d8caa1c00f0e6543ba950f4c2 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Fri, 28 Aug 2026 14:59:11 +0200 Subject: [PATCH] fix(dry-run): commit no longer creates a real image; the interception set becomes declarative MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #14074, B.7. Signed-off-by: Nicolas De Loof --- pkg/dryrun/dryrunclient.go | 131 ++++++++++++++++++--------- pkg/dryrun/dryrunclient_test.go | 156 ++++++++++++++++++++++++++++++++ pkg/e2e/checks.go | 15 +++ pkg/e2e/commit_test.go | 3 + 4 files changed, 264 insertions(+), 41 deletions(-) create mode 100644 pkg/dryrun/dryrunclient_test.go diff --git a/pkg/dryrun/dryrunclient.go b/pkg/dryrun/dryrunclient.go index cf8f7ba438..5f81c557e3 100644 --- a/pkg/dryrun/dryrunclient.go +++ b/pkg/dryrun/dryrunclient.go @@ -100,7 +100,17 @@ func getCallingFunction() string { return fullName[strings.LastIndex(fullName, ".")+1:] } -// All methods and functions which need to be overridden for dry run. +// errDryRunForbidden refuses a mutating operation compose itself never +// issues: delegating it would let future code mutate the engine under +// --dry-run silently. The full classification of every APIClient method +// (read-only delegated / faked / refused) is pinned by +// TestDryRunClientClassifiesEveryMethod. +func errDryRunForbidden(operation string) error { + return fmt.Errorf("%s is not allowed in dry-run mode", operation) +} + +// Methods below either fake the operation for dry-run or delegate the +// read-only ones to the real client. func (d *DryRunClient) ContainerAttach(ctx context.Context, container string, options client.ContainerAttachOptions) (client.ContainerAttachResult, error) { return client.ContainerAttachResult{}, errors.New("interactive run is not supported in dry-run mode") @@ -329,19 +339,22 @@ func (d *DryRunClient) ConfigInspect(ctx context.Context, name string, options c } func (d *DryRunClient) ConfigCreate(ctx context.Context, options client.ConfigCreateOptions) (client.ConfigCreateResult, error) { - return d.apiClient.ConfigCreate(ctx, options) + var zero client.ConfigCreateResult + return zero, errDryRunForbidden("ConfigCreate") } func (d *DryRunClient) ConfigRemove(ctx context.Context, id string, options client.ConfigRemoveOptions) (client.ConfigRemoveResult, error) { - return d.apiClient.ConfigRemove(ctx, id, options) + var zero client.ConfigRemoveResult + return zero, errDryRunForbidden("ConfigRemove") } func (d *DryRunClient) ConfigUpdate(ctx context.Context, id string, options client.ConfigUpdateOptions) (client.ConfigUpdateResult, error) { - return d.apiClient.ConfigUpdate(ctx, id, options) + var zero client.ConfigUpdateResult + return zero, errDryRunForbidden("ConfigUpdate") } func (d *DryRunClient) ContainerCommit(ctx context.Context, container string, options client.ContainerCommitOptions) (client.ContainerCommitResult, error) { - return d.apiClient.ContainerCommit(ctx, container, options) + return client.ContainerCommitResult{ID: "dryRunId"}, nil } func (d *DryRunClient) ContainerDiff(ctx context.Context, container string, options client.ContainerDiffOptions) (client.ContainerDiffResult, error) { @@ -357,7 +370,8 @@ func (d *DryRunClient) ExecInspect(ctx context.Context, execID string, options c } func (d *DryRunClient) ExecResize(ctx context.Context, execID string, options client.ExecResizeOptions) (client.ExecResizeResult, error) { - return d.apiClient.ExecResize(ctx, execID, options) + var zero client.ExecResizeResult + return zero, errDryRunForbidden("ExecResize") } func (d *DryRunClient) ContainerExport(ctx context.Context, container string, options client.ContainerExportOptions) (client.ContainerExportResult, error) { @@ -369,7 +383,8 @@ func (d *DryRunClient) ContainerLogs(ctx context.Context, container string, opti } func (d *DryRunClient) ContainerResize(ctx context.Context, container string, options client.ContainerResizeOptions) (client.ContainerResizeResult, error) { - return d.apiClient.ContainerResize(ctx, container, options) + var zero client.ContainerResizeResult + return zero, errDryRunForbidden("ContainerResize") } func (d *DryRunClient) ContainerStatPath(ctx context.Context, container string, options client.ContainerStatPathOptions) (client.ContainerStatPathResult, error) { @@ -385,7 +400,8 @@ func (d *DryRunClient) ContainerTop(ctx context.Context, container string, optio } func (d *DryRunClient) ContainerUpdate(ctx context.Context, container string, options client.ContainerUpdateOptions) (client.ContainerUpdateResult, error) { - return d.apiClient.ContainerUpdate(ctx, container, options) + var zero client.ContainerUpdateResult + return zero, errDryRunForbidden("ContainerUpdate") } func (d *DryRunClient) ContainerWait(ctx context.Context, container string, options client.ContainerWaitOptions) client.ContainerWaitResult { @@ -393,7 +409,8 @@ func (d *DryRunClient) ContainerWait(ctx context.Context, container string, opti } func (d *DryRunClient) ContainerPrune(ctx context.Context, options client.ContainerPruneOptions) (client.ContainerPruneResult, error) { - return d.apiClient.ContainerPrune(ctx, options) + var zero client.ContainerPruneResult + return zero, errDryRunForbidden("ContainerPrune") } func (d *DryRunClient) DistributionInspect(ctx context.Context, imageName string, options client.DistributionInspectOptions) (client.DistributionInspectResult, error) { @@ -401,11 +418,13 @@ func (d *DryRunClient) DistributionInspect(ctx context.Context, imageName string } func (d *DryRunClient) BuildCachePrune(ctx context.Context, opts client.BuildCachePruneOptions) (client.BuildCachePruneResult, error) { - return d.apiClient.BuildCachePrune(ctx, opts) + var zero client.BuildCachePruneResult + return zero, errDryRunForbidden("BuildCachePrune") } func (d *DryRunClient) BuildCancel(ctx context.Context, id string, opts client.BuildCancelOptions) (client.BuildCancelResult, error) { - return d.apiClient.BuildCancel(ctx, id, opts) + var zero client.BuildCancelResult + return zero, errDryRunForbidden("BuildCancel") } func (d *DryRunClient) ImageHistory(ctx context.Context, imageName string, options ...client.ImageHistoryOption) (client.ImageHistoryResult, error) { @@ -417,7 +436,8 @@ func (d *DryRunClient) ImageAttestations(ctx context.Context, imageName string, } func (d *DryRunClient) ImageImport(ctx context.Context, source client.ImageImportSource, ref string, options client.ImageImportOptions) (client.ImageImportResult, error) { - return d.apiClient.ImageImport(ctx, source, ref, options) + var zero client.ImageImportResult + return zero, errDryRunForbidden("ImageImport") } func (d *DryRunClient) ImageList(ctx context.Context, options client.ImageListOptions) (client.ImageListResult, error) { @@ -425,7 +445,8 @@ func (d *DryRunClient) ImageList(ctx context.Context, options client.ImageListOp } func (d *DryRunClient) ImageLoad(ctx context.Context, input io.Reader, options ...client.ImageLoadOption) (client.ImageLoadResult, error) { - return d.apiClient.ImageLoad(ctx, input, options...) + var zero client.ImageLoadResult + return zero, errDryRunForbidden("ImageLoad") } func (d *DryRunClient) ImageSearch(ctx context.Context, term string, options client.ImageSearchOptions) (client.ImageSearchResult, error) { @@ -437,11 +458,13 @@ func (d *DryRunClient) ImageSave(ctx context.Context, images []string, options . } func (d *DryRunClient) ImageTag(ctx context.Context, options client.ImageTagOptions) (client.ImageTagResult, error) { - return d.apiClient.ImageTag(ctx, options) + var zero client.ImageTagResult + return zero, errDryRunForbidden("ImageTag") } func (d *DryRunClient) ImagePrune(ctx context.Context, options client.ImagePruneOptions) (client.ImagePruneResult, error) { - return d.apiClient.ImagePrune(ctx, options) + var zero client.ImagePruneResult + return zero, errDryRunForbidden("ImagePrune") } func (d *DryRunClient) NodeInspect(ctx context.Context, nodeID string, options client.NodeInspectOptions) (client.NodeInspectResult, error) { @@ -453,11 +476,13 @@ func (d *DryRunClient) NodeList(ctx context.Context, options client.NodeListOpti } func (d *DryRunClient) NodeRemove(ctx context.Context, nodeID string, options client.NodeRemoveOptions) (client.NodeRemoveResult, error) { - return d.apiClient.NodeRemove(ctx, nodeID, options) + var zero client.NodeRemoveResult + return zero, errDryRunForbidden("NodeRemove") } func (d *DryRunClient) NodeUpdate(ctx context.Context, nodeID string, options client.NodeUpdateOptions) (client.NodeUpdateResult, error) { - return d.apiClient.NodeUpdate(ctx, nodeID, options) + var zero client.NodeUpdateResult + return zero, errDryRunForbidden("NodeUpdate") } func (d *DryRunClient) NetworkInspect(ctx context.Context, networkName string, options client.NetworkInspectOptions) (client.NetworkInspectResult, error) { @@ -469,7 +494,8 @@ func (d *DryRunClient) NetworkList(ctx context.Context, options client.NetworkLi } func (d *DryRunClient) NetworkPrune(ctx context.Context, options client.NetworkPruneOptions) (client.NetworkPruneResult, error) { - return d.apiClient.NetworkPrune(ctx, options) + var zero client.NetworkPruneResult + return zero, errDryRunForbidden("NetworkPrune") } func (d *DryRunClient) PluginList(ctx context.Context, options client.PluginListOptions) (client.PluginListResult, error) { @@ -477,31 +503,38 @@ func (d *DryRunClient) PluginList(ctx context.Context, options client.PluginList } func (d *DryRunClient) PluginRemove(ctx context.Context, name string, options client.PluginRemoveOptions) (client.PluginRemoveResult, error) { - return d.apiClient.PluginRemove(ctx, name, options) + var zero client.PluginRemoveResult + return zero, errDryRunForbidden("PluginRemove") } func (d *DryRunClient) PluginEnable(ctx context.Context, name string, options client.PluginEnableOptions) (client.PluginEnableResult, error) { - return d.apiClient.PluginEnable(ctx, name, options) + var zero client.PluginEnableResult + return zero, errDryRunForbidden("PluginEnable") } func (d *DryRunClient) PluginDisable(ctx context.Context, name string, options client.PluginDisableOptions) (client.PluginDisableResult, error) { - return d.apiClient.PluginDisable(ctx, name, options) + var zero client.PluginDisableResult + return zero, errDryRunForbidden("PluginDisable") } func (d *DryRunClient) PluginInstall(ctx context.Context, name string, options client.PluginInstallOptions) (client.PluginInstallResult, error) { - return d.apiClient.PluginInstall(ctx, name, options) + var zero client.PluginInstallResult + return zero, errDryRunForbidden("PluginInstall") } func (d *DryRunClient) PluginUpgrade(ctx context.Context, name string, options client.PluginUpgradeOptions) (client.PluginUpgradeResult, error) { - return d.apiClient.PluginUpgrade(ctx, name, options) + var zero client.PluginUpgradeResult + return zero, errDryRunForbidden("PluginUpgrade") } func (d *DryRunClient) PluginPush(ctx context.Context, name string, options client.PluginPushOptions) (client.PluginPushResult, error) { - return d.apiClient.PluginPush(ctx, name, options) + var zero client.PluginPushResult + return zero, errDryRunForbidden("PluginPush") } func (d *DryRunClient) PluginSet(ctx context.Context, name string, options client.PluginSetOptions) (client.PluginSetResult, error) { - return d.apiClient.PluginSet(ctx, name, options) + var zero client.PluginSetResult + return zero, errDryRunForbidden("PluginSet") } func (d *DryRunClient) PluginInspect(ctx context.Context, name string, options client.PluginInspectOptions) (client.PluginInspectResult, error) { @@ -509,11 +542,13 @@ func (d *DryRunClient) PluginInspect(ctx context.Context, name string, options c } func (d *DryRunClient) PluginCreate(ctx context.Context, createContext io.Reader, options client.PluginCreateOptions) (client.PluginCreateResult, error) { - return d.apiClient.PluginCreate(ctx, createContext, options) + var zero client.PluginCreateResult + return zero, errDryRunForbidden("PluginCreate") } func (d *DryRunClient) ServiceCreate(ctx context.Context, options client.ServiceCreateOptions) (client.ServiceCreateResult, error) { - return d.apiClient.ServiceCreate(ctx, options) + var zero client.ServiceCreateResult + return zero, errDryRunForbidden("ServiceCreate") } func (d *DryRunClient) ServiceInspect(ctx context.Context, serviceID string, options client.ServiceInspectOptions) (client.ServiceInspectResult, error) { @@ -525,11 +560,13 @@ func (d *DryRunClient) ServiceList(ctx context.Context, options client.ServiceLi } func (d *DryRunClient) ServiceRemove(ctx context.Context, serviceID string, options client.ServiceRemoveOptions) (client.ServiceRemoveResult, error) { - return d.apiClient.ServiceRemove(ctx, serviceID, options) + var zero client.ServiceRemoveResult + return zero, errDryRunForbidden("ServiceRemove") } func (d *DryRunClient) ServiceUpdate(ctx context.Context, serviceID string, options client.ServiceUpdateOptions) (client.ServiceUpdateResult, error) { - return d.apiClient.ServiceUpdate(ctx, serviceID, options) + var zero client.ServiceUpdateResult + return zero, errDryRunForbidden("ServiceUpdate") } func (d *DryRunClient) ServiceLogs(ctx context.Context, serviceID string, options client.ServiceLogsOptions) (client.ServiceLogsResult, error) { @@ -549,11 +586,13 @@ func (d *DryRunClient) TaskList(ctx context.Context, options client.TaskListOpti } func (d *DryRunClient) SwarmInit(ctx context.Context, options client.SwarmInitOptions) (client.SwarmInitResult, error) { - return d.apiClient.SwarmInit(ctx, options) + var zero client.SwarmInitResult + return zero, errDryRunForbidden("SwarmInit") } func (d *DryRunClient) SwarmJoin(ctx context.Context, options client.SwarmJoinOptions) (client.SwarmJoinResult, error) { - return d.apiClient.SwarmJoin(ctx, options) + var zero client.SwarmJoinResult + return zero, errDryRunForbidden("SwarmJoin") } func (d *DryRunClient) SwarmGetUnlockKey(ctx context.Context) (client.SwarmGetUnlockKeyResult, error) { @@ -561,11 +600,13 @@ func (d *DryRunClient) SwarmGetUnlockKey(ctx context.Context) (client.SwarmGetUn } func (d *DryRunClient) SwarmUnlock(ctx context.Context, options client.SwarmUnlockOptions) (client.SwarmUnlockResult, error) { - return d.apiClient.SwarmUnlock(ctx, options) + var zero client.SwarmUnlockResult + return zero, errDryRunForbidden("SwarmUnlock") } func (d *DryRunClient) SwarmLeave(ctx context.Context, options client.SwarmLeaveOptions) (client.SwarmLeaveResult, error) { - return d.apiClient.SwarmLeave(ctx, options) + var zero client.SwarmLeaveResult + return zero, errDryRunForbidden("SwarmLeave") } func (d *DryRunClient) SwarmInspect(ctx context.Context, options client.SwarmInspectOptions) (client.SwarmInspectResult, error) { @@ -573,7 +614,8 @@ func (d *DryRunClient) SwarmInspect(ctx context.Context, options client.SwarmIns } func (d *DryRunClient) SwarmUpdate(ctx context.Context, options client.SwarmUpdateOptions) (client.SwarmUpdateResult, error) { - return d.apiClient.SwarmUpdate(ctx, options) + var zero client.SwarmUpdateResult + return zero, errDryRunForbidden("SwarmUpdate") } func (d *DryRunClient) SecretList(ctx context.Context, options client.SecretListOptions) (client.SecretListResult, error) { @@ -581,11 +623,13 @@ func (d *DryRunClient) SecretList(ctx context.Context, options client.SecretList } func (d *DryRunClient) SecretCreate(ctx context.Context, options client.SecretCreateOptions) (client.SecretCreateResult, error) { - return d.apiClient.SecretCreate(ctx, options) + var zero client.SecretCreateResult + return zero, errDryRunForbidden("SecretCreate") } func (d *DryRunClient) SecretRemove(ctx context.Context, id string, options client.SecretRemoveOptions) (client.SecretRemoveResult, error) { - return d.apiClient.SecretRemove(ctx, id, options) + var zero client.SecretRemoveResult + return zero, errDryRunForbidden("SecretRemove") } func (d *DryRunClient) SecretInspect(ctx context.Context, name string, options client.SecretInspectOptions) (client.SecretInspectResult, error) { @@ -593,7 +637,8 @@ func (d *DryRunClient) SecretInspect(ctx context.Context, name string, options c } func (d *DryRunClient) SecretUpdate(ctx context.Context, id string, options client.SecretUpdateOptions) (client.SecretUpdateResult, error) { - return d.apiClient.SecretUpdate(ctx, id, options) + var zero client.SecretUpdateResult + return zero, errDryRunForbidden("SecretUpdate") } func (d *DryRunClient) Events(ctx context.Context, options client.EventsListOptions) client.EventsResult { @@ -625,11 +670,13 @@ func (d *DryRunClient) VolumeList(ctx context.Context, opts client.VolumeListOpt } func (d *DryRunClient) VolumePrune(ctx context.Context, options client.VolumePruneOptions) (client.VolumePruneResult, error) { - return d.apiClient.VolumePrune(ctx, options) + var zero client.VolumePruneResult + return zero, errDryRunForbidden("VolumePrune") } func (d *DryRunClient) VolumeUpdate(ctx context.Context, volumeID string, options client.VolumeUpdateOptions) (client.VolumeUpdateResult, error) { - return d.apiClient.VolumeUpdate(ctx, volumeID, options) + var zero client.VolumeUpdateResult + return zero, errDryRunForbidden("VolumeUpdate") } func (d *DryRunClient) ClientVersion() string { @@ -657,11 +704,13 @@ func (d *DryRunClient) Close() error { } func (d *DryRunClient) CheckpointCreate(ctx context.Context, container string, options client.CheckpointCreateOptions) (client.CheckpointCreateResult, error) { - return d.apiClient.CheckpointCreate(ctx, container, options) + var zero client.CheckpointCreateResult + return zero, errDryRunForbidden("CheckpointCreate") } func (d *DryRunClient) CheckpointRemove(ctx context.Context, container string, options client.CheckpointRemoveOptions) (client.CheckpointRemoveResult, error) { - return d.apiClient.CheckpointRemove(ctx, container, options) + var zero client.CheckpointRemoveResult + return zero, errDryRunForbidden("CheckpointRemove") } func (d *DryRunClient) CheckpointList(ctx context.Context, container string, options client.CheckpointListOptions) (client.CheckpointListResult, error) { diff --git a/pkg/dryrun/dryrunclient_test.go b/pkg/dryrun/dryrunclient_test.go new file mode 100644 index 0000000000..b13d7db5b7 --- /dev/null +++ b/pkg/dryrun/dryrunclient_test.go @@ -0,0 +1,156 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dryrun + +import ( + "go/ast" + "go/parser" + "go/token" + "regexp" + "strings" + "testing" + + "gotest.tools/v3/assert" +) + +// The dry-run guarantee is a classification of every APIClient method into +// exactly one of three sets, declared here rather than implied by a method's +// position in the file: +// +// - readOnlyDelegated: reaches the real daemon, must not mutate anything; +// - faked: intercepted with a plausible in-memory result, so compose +// operations proceed under --dry-run without touching the engine; +// - refused: mutating operations compose itself never issues — they +// return an error instead of silently mutating if future code calls one. +// +// The test parses dryrunclient.go and fails on any method that is missing, +// misclassified, or new and unclassified: adding a method to APIClient +// requires an explicit decision here. +var ( + readOnlyDelegated = set( + "CheckpointList", "ClientVersion", "Close", "ConfigInspect", "ConfigList", "ContainerDiff", + "ContainerExport", "ContainerInspect", "ContainerList", "ContainerLogs", "ContainerStatPath", "ContainerStats", + "ContainerTop", "ContainerWait", "DaemonHost", "DialHijack", "Dialer", "DiskUsage", + "DistributionInspect", "Events", "ExecInspect", "ImageAttestations", "ImageHistory", "ImageInspect", + "ImageList", "ImageSave", "ImageSearch", "Info", "NetworkInspect", "NetworkList", + "NodeInspect", "NodeList", "Ping", "PluginInspect", "PluginList", "RegistryLogin", + "SecretInspect", "SecretList", "ServerVersion", "ServiceInspect", "ServiceList", "ServiceLogs", + "SwarmGetUnlockKey", "SwarmInspect", "TaskInspect", "TaskList", "TaskLogs", "VolumeInspect", + "VolumeList", + ) + + faked = set( + "ContainerAttach", "ContainerCommit", "ContainerCreate", "ContainerKill", "ContainerPause", "ContainerRemove", + "ContainerRename", "ContainerRestart", "ContainerStart", "ContainerStop", "ContainerUnpause", "CopyFromContainer", + "CopyToContainer", "ExecAttach", "ExecCreate", "ExecStart", "ImageBuild", "ImagePull", + "ImagePush", "ImageRemove", "NetworkConnect", "NetworkCreate", "NetworkDisconnect", "NetworkRemove", + "VolumeCreate", "VolumeRemove", + ) + + refused = set( + "BuildCachePrune", "BuildCancel", "CheckpointCreate", "CheckpointRemove", "ConfigCreate", "ConfigRemove", + "ConfigUpdate", "ContainerPrune", "ContainerResize", "ContainerUpdate", "ExecResize", "ImageImport", + "ImageLoad", "ImagePrune", "ImageTag", "NetworkPrune", "NodeRemove", "NodeUpdate", + "PluginCreate", "PluginDisable", "PluginEnable", "PluginInstall", "PluginPush", "PluginRemove", + "PluginSet", "PluginUpgrade", "SecretCreate", "SecretRemove", "SecretUpdate", "ServiceCreate", + "ServiceRemove", "ServiceUpdate", "SwarmInit", "SwarmJoin", "SwarmLeave", "SwarmUnlock", + "SwarmUpdate", "VolumePrune", "VolumeUpdate", + ) +) + +func set(names ...string) map[string]bool { + m := map[string]bool{} + for _, n := range names { + m[n] = true + } + return m +} + +func TestDryRunClientClassifiesEveryMethod(t *testing.T) { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "dryrunclient.go", nil, 0) + assert.NilError(t, err) + + classified := map[string]string{} + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Recv == nil || fn.Body == nil { + continue + } + recv, ok := fn.Recv.List[0].Type.(*ast.StarExpr) + if !ok { + continue + } + if ident, ok := recv.X.(*ast.Ident); !ok || ident.Name != "DryRunClient" { + continue + } + name := fn.Name.Name + if !ast.IsExported(name) { + continue + } + classified[name] = classify(fset, fn, name) + } + + for name, got := range classified { + var want string + switch { + case readOnlyDelegated[name]: + want = "delegated" + case faked[name]: + want = "faked" + case refused[name]: + want = "refused" + default: + t.Errorf("method %s is not classified: decide whether it is read-only (delegate), mutating-but-needed (fake it) or mutating-and-unused (refuse it), and add it to the matching list", name) + continue + } + assert.Equal(t, got, want, "method %s", name) + } + for _, list := range []map[string]bool{readOnlyDelegated, faked, refused} { + for name := range list { + _, found := classified[name] + assert.Assert(t, found, "classified method %s no longer exists on DryRunClient", name) + } + } +} + +// classify reads a method body: calling d.apiClient. means delegated, +// calling errDryRunForbidden means refused, anything else is a fake. +func classify(fset *token.FileSet, fn *ast.FuncDecl, name string) string { + var src strings.Builder + ast.Inspect(fn.Body, func(n ast.Node) bool { + if sel, ok := n.(*ast.SelectorExpr); ok { + if inner, ok := sel.X.(*ast.SelectorExpr); ok { + if id, ok := inner.X.(*ast.Ident); ok && id.Name == "d" && inner.Sel.Name == "apiClient" { + src.WriteString("DELEGATE:" + sel.Sel.Name + ";") + } + } + } + if id, ok := n.(*ast.Ident); ok && id.Name == "errDryRunForbidden" { + src.WriteString("FORBID;") + } + return true + }) + _ = fset + if regexp.MustCompile(`DELEGATE:` + name + `;`).MatchString(src.String()) { + return "delegated" + } + if strings.Contains(src.String(), "FORBID;") { + return "refused" + } + return "faked" +} diff --git a/pkg/e2e/checks.go b/pkg/e2e/checks.go index 518485eb51..0d2b460ed3 100644 --- a/pkg/e2e/checks.go +++ b/pkg/e2e/checks.go @@ -453,6 +453,21 @@ func ImageExists(ref string) Check { } } +// ImageAbsent expects no image with the given reference in the local store — +// the observable proof that a dry-run left the engine untouched. +func ImageAbsent(ref string) Check { + return Check{ + name: fmt.Sprintf("image %q is absent", ref), + fn: func(ctx *CheckContext) error { + res := icmd.RunCmd(ctx.scenario.cli.NewDockerCmd(ctx.scenario.t, "image", "inspect", "--format", "{{.Id}}", ref)) + if res.ExitCode == 0 { + return fmt.Errorf("image %s exists", ref) + } + return nil + }, + } +} + // FileExists expects a non-empty file at the given host path, e.g. the output // of an export command. func FileExists(path string) Check { diff --git a/pkg/e2e/commit_test.go b/pkg/e2e/commit_test.go index c0e3217ac5..da3093feee 100644 --- a/pkg/e2e/commit_test.go +++ b/pkg/e2e/commit_test.go @@ -29,6 +29,9 @@ func TestCommit(t *testing.T) { Step("up starts the service", ComposeCmd("up", "-d", "service"), ServiceState("service", "running")). + Step("commit --dry-run must not create the image", + ComposeCmd("--dry-run", "commit", "service", image), + ImageAbsent(image)). Step("commit turns the container into a tagged image", ComposeCmd("commit", "-a", "John Hannibal Smith ",