From 6167efd54e54895633723db990b7913db2f9a3c7 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Fri, 28 Aug 2026 14:29:56 +0200 Subject: [PATCH] api: DownOptions.Images becomes a typed ImagePruneMode, validated upfront MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legal values of DownOptions.Images lived in pkg/compose (image_pruner.go) while the field itself was a bare string in pkg/api, and the only validation fired inside ImagesToPrune — after down had already removed the project's containers, leaving the teardown half done on a typo. The CLI validated its --rmi flag separately, so only SDK callers were exposed. ImagePruneMode and its three values now live in pkg/api next to the field they constrain (type-aliased in pkg/compose for existing consumers), down() rejects any other value before touching a single resource, and the CLI check reuses the same definition. A unit test runs Down with expectation-free mocks: one daemon call would fail it. Epic #14074, B.6. Signed-off-by: Nicolas De Loof --- cmd/compose/down.go | 8 +++----- pkg/api/api.go | 32 ++++++++++++++++++++++++++++++-- pkg/compose/down.go | 10 ++++++++-- pkg/compose/down_test.go | 18 ++++++++++++++++++ pkg/compose/image_pruner.go | 13 +++++++------ 5 files changed, 66 insertions(+), 15 deletions(-) diff --git a/cmd/compose/down.go b/cmd/compose/down.go index d74c8175292..0b0e424b010 100644 --- a/cmd/compose/down.go +++ b/cmd/compose/down.go @@ -50,10 +50,8 @@ func downCommand(p *ProjectOptions, dockerCli command.Cli, backendOptions *Backe Short: "Stop and remove containers, networks", PreRunE: AdaptCmd(func(ctx context.Context, cmd *cobra.Command, args []string) error { opts.timeChanged = cmd.Flags().Changed("timeout") - if opts.images != "" { - if opts.images != "all" && opts.images != "local" { - return fmt.Errorf("invalid value for --rmi: %q", opts.images) - } + if !api.ImagePruneMode(opts.images).Valid() { + return fmt.Errorf("invalid value for --rmi: %q (legal values are %q, %q)", opts.images, api.ImagePruneLocal, api.ImagePruneAll) } return nil }), @@ -97,7 +95,7 @@ func runDown(ctx context.Context, dockerCli command.Cli, backendOptions *Backend RemoveOrphans: opts.removeOrphans, Project: project, Timeout: timeout, - Images: opts.images, + Images: api.ImagePruneMode(opts.images), Volumes: opts.volumes, Services: services, }) diff --git a/pkg/api/api.go b/pkg/api/api.go index a8c21a17c9e..dec87ce39ff 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -377,6 +377,31 @@ type UpOptions struct { Start StartOptions } +// ImagePruneMode controls how aggressively `down` removes the images +// associated with the project. +type ImagePruneMode string + +const ( + // ImagePruneNone keeps all project images. + ImagePruneNone ImagePruneMode = "" + // ImagePruneLocal removes only the images built locally by Compose + // (no custom tag). + ImagePruneLocal ImagePruneMode = "local" + // ImagePruneAll removes every project-associated image, remote ones + // included. + ImagePruneAll ImagePruneMode = "all" +) + +// Valid reports whether the mode is one of the declared values. +func (m ImagePruneMode) Valid() bool { + switch m { + case ImagePruneNone, ImagePruneLocal, ImagePruneAll: + return true + default: + return false + } +} + // DownOptions group options of the Down API type DownOptions struct { // RemoveOrphans will cleanup containers that are not declared on the compose model but own the same labels @@ -385,8 +410,11 @@ type DownOptions struct { Project *types.Project // Timeout override container stop timeout Timeout *time.Duration - // Images remove image used by services. 'all': Remove all images. 'local': Remove only images that don't have a tag - Images string + // Images removes images used by the services: ImagePruneAll removes + // every project image, ImagePruneLocal only those without a custom tag, + // ImagePruneNone (the zero value) keeps them all. Down rejects any other + // value before touching a single resource. + Images ImagePruneMode // Volumes remove volumes, both declared in the `volumes` section and anonymous ones Volumes bool // Services passed in the command line to be stopped diff --git a/pkg/compose/down.go b/pkg/compose/down.go index e327b091458..aeaa7b7913d 100644 --- a/pkg/compose/down.go +++ b/pkg/compose/down.go @@ -42,6 +42,12 @@ func (s *composeService) Down(ctx context.Context, projectName string, options a } func (s *composeService) down(ctx context.Context, projectName string, options api.DownOptions) error { + // validate before touching anything: failing on a bad image prune mode + // after containers are already removed would leave the teardown half done + if !options.Images.Valid() { + return fmt.Errorf("invalid image prune mode %q: legal values are %q, %q", options.Images, api.ImagePruneLocal, api.ImagePruneAll) + } + resourceToRemove := false include := oneOffExclude @@ -113,7 +119,7 @@ func (s *composeService) down(ctx context.Context, projectName string, options a ops := s.ensureNetworksDown(ctx, project) - if options.Images != "" { + if options.Images != api.ImagePruneNone { imgOps, err := s.ensureImagesDown(ctx, project, options) if err != nil { return err @@ -154,7 +160,7 @@ func (s *composeService) ensureVolumesDown(ctx context.Context, project *types.P func (s *composeService) ensureImagesDown(ctx context.Context, project *types.Project, options api.DownOptions) ([]downOp, error) { imagePruner := NewImagePruner(s.apiClient(), project) pruneOpts := ImagePruneOptions{ - Mode: ImagePruneMode(options.Images), + Mode: options.Images, RemoveOrphans: options.RemoveOrphans, } images, err := imagePruner.ImagesToPrune(ctx, pruneOpts) diff --git a/pkg/compose/down_test.go b/pkg/compose/down_test.go index eea13b9d231..334d6ad78b8 100644 --- a/pkg/compose/down_test.go +++ b/pkg/compose/down_test.go @@ -37,6 +37,24 @@ import ( "github.com/docker/compose/v5/pkg/mocks" ) +// An invalid image prune mode must be rejected before any resource is +// touched: the mocks carry no expectation, so a single daemon call would +// fail the test. Guards the down() precondition — validating mid-down, +// after containers were removed, would leave the teardown half done. +func TestDownRejectsInvalidImagePruneModeUpfront(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + _, cli := prepareMocks(mockCtrl) + tested, err := NewComposeService(cli) + assert.NilError(t, err) + + err = tested.Down(t.Context(), strings.ToLower(testProject), compose.DownOptions{ + Images: "bogus", + }) + assert.ErrorContains(t, err, `invalid image prune mode "bogus"`) +} + func TestDown(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() diff --git a/pkg/compose/image_pruner.go b/pkg/compose/image_pruner.go index 7016fb7ef4f..45de915c7b2 100644 --- a/pkg/compose/image_pruner.go +++ b/pkg/compose/image_pruner.go @@ -32,19 +32,20 @@ import ( "github.com/docker/compose/v5/pkg/api" ) -// ImagePruneMode controls how aggressively images associated with the project -// are removed from the engine. -type ImagePruneMode string +// ImagePruneMode aliases the pkg/api definition: the legal values of +// DownOptions.Images belong to the SDK surface, kept aliased here for the +// existing consumers of this package. +type ImagePruneMode = api.ImagePruneMode const ( // ImagePruneNone indicates that no project images should be removed. - ImagePruneNone ImagePruneMode = "" + ImagePruneNone = api.ImagePruneNone // ImagePruneLocal indicates that only images built locally by Compose // should be removed. - ImagePruneLocal ImagePruneMode = "local" + ImagePruneLocal = api.ImagePruneLocal // ImagePruneAll indicates that all project-associated images, including // remote images should be removed. - ImagePruneAll ImagePruneMode = "all" + ImagePruneAll = api.ImagePruneAll ) // ImagePruneOptions controls the behavior of image pruning.