From 3b5dff27832e5a0124198fa0024c0dedd46bf2a0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Mar 2025 02:33:48 +0100 Subject: [PATCH] cli/command: internalize constructing ManifestStore The CLI.ManifestStore method is a shallow wrapper around manifeststore.NewStore and has no dependency on the CLI itself. However, due to its signature resulted in various dependencies becoming a dependency of the "command" package. Consequence of this was that cli-plugins, which need the cli/command package, would also get those dependencies. - This patch inlines the code to produce the store, skipping the wrapper. - Define a local interface for some tests where a dummy store was used. Signed-off-by: Sebastiaan van Stijn --- cli/command/manifest/annotate.go | 23 +++++++++++++++++++++-- cli/command/manifest/create_list.go | 2 +- cli/command/manifest/inspect.go | 4 ++-- cli/command/manifest/push.go | 4 ++-- cli/command/manifest/rm.go | 2 +- cli/command/manifest/util.go | 2 +- 6 files changed, 28 insertions(+), 9 deletions(-) diff --git a/cli/command/manifest/annotate.go b/cli/command/manifest/annotate.go index c12b69d823..2a0a49893b 100644 --- a/cli/command/manifest/annotate.go +++ b/cli/command/manifest/annotate.go @@ -2,9 +2,11 @@ package manifest import ( "fmt" + "path/filepath" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/manifest/store" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -21,6 +23,23 @@ type annotateOptions struct { osVersion string } +// manifestStoreProvider is used in tests to provide a dummy store. +type manifestStoreProvider interface { + // ManifestStore returns a store for local manifests + ManifestStore() store.Store +} + +// newManifestStore returns a store for local manifests +func newManifestStore(dockerCLI command.Cli) store.Store { + if msp, ok := dockerCLI.(manifestStoreProvider); ok { + // manifestStoreProvider is used in tests to provide a dummy store. + return msp.ManifestStore() + } + + // TODO: support override default location from config file + return store.NewStore(filepath.Join(config.Dir(), "manifests")) +} + // NewAnnotateCommand creates a new `docker manifest annotate` command func newAnnotateCommand(dockerCli command.Cli) *cobra.Command { var opts annotateOptions @@ -47,7 +66,7 @@ func newAnnotateCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runManifestAnnotate(dockerCli command.Cli, opts annotateOptions) error { +func runManifestAnnotate(dockerCLI command.Cli, opts annotateOptions) error { targetRef, err := normalizeReference(opts.target) if err != nil { return errors.Wrapf(err, "annotate: error parsing name for manifest list %s", opts.target) @@ -57,7 +76,7 @@ func runManifestAnnotate(dockerCli command.Cli, opts annotateOptions) error { return errors.Wrapf(err, "annotate: error parsing name for manifest %s", opts.image) } - manifestStore := dockerCli.ManifestStore() + manifestStore := newManifestStore(dockerCLI) imageManifest, err := manifestStore.Get(targetRef, imgRef) switch { case store.IsNotFound(err): diff --git a/cli/command/manifest/create_list.go b/cli/command/manifest/create_list.go index 5a399d5758..58c18124df 100644 --- a/cli/command/manifest/create_list.go +++ b/cli/command/manifest/create_list.go @@ -41,7 +41,7 @@ func createManifestList(ctx context.Context, dockerCLI command.Cli, args []strin return errors.Wrapf(err, "error parsing name for manifest list %s", newRef) } - manifestStore := dockerCLI.ManifestStore() + manifestStore := newManifestStore(dockerCLI) _, err = manifestStore.GetList(targetRef) switch { case store.IsNotFound(err): diff --git a/cli/command/manifest/inspect.go b/cli/command/manifest/inspect.go index 23ea2e69c6..ee68acf3c9 100644 --- a/cli/command/manifest/inspect.go +++ b/cli/command/manifest/inspect.go @@ -61,7 +61,7 @@ func runInspect(ctx context.Context, dockerCli command.Cli, opts inspectOptions) return err } - imageManifest, err := dockerCli.ManifestStore().Get(listRef, namedRef) + imageManifest, err := newManifestStore(dockerCli).Get(listRef, namedRef) if err != nil { return err } @@ -69,7 +69,7 @@ func runInspect(ctx context.Context, dockerCli command.Cli, opts inspectOptions) } // Try a local manifest list first - localManifestList, err := dockerCli.ManifestStore().GetList(namedRef) + localManifestList, err := newManifestStore(dockerCli).GetList(namedRef) if err == nil { return printManifestList(dockerCli, namedRef, localManifestList, opts) } diff --git a/cli/command/manifest/push.go b/cli/command/manifest/push.go index 9b214eeaa7..97bdab6cbc 100644 --- a/cli/command/manifest/push.go +++ b/cli/command/manifest/push.go @@ -68,7 +68,7 @@ func runPush(ctx context.Context, dockerCli command.Cli, opts pushOpts) error { return err } - manifests, err := dockerCli.ManifestStore().GetList(targetRef) + manifests, err := newManifestStore(dockerCli).GetList(targetRef) if err != nil { return err } @@ -85,7 +85,7 @@ func runPush(ctx context.Context, dockerCli command.Cli, opts pushOpts) error { return err } if opts.purge { - return dockerCli.ManifestStore().Remove(targetRef) + return newManifestStore(dockerCli).Remove(targetRef) } return nil } diff --git a/cli/command/manifest/rm.go b/cli/command/manifest/rm.go index e59b716cd3..dda4e3e489 100644 --- a/cli/command/manifest/rm.go +++ b/cli/command/manifest/rm.go @@ -16,7 +16,7 @@ func newRmManifestListCommand(dockerCLI command.Cli) *cobra.Command { Short: "Delete one or more manifest lists from local storage", Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runRemove(cmd.Context(), dockerCLI.ManifestStore(), args) + return runRemove(cmd.Context(), newManifestStore(dockerCLI), args) }, } diff --git a/cli/command/manifest/util.go b/cli/command/manifest/util.go index f89116d96c..929f646bcc 100644 --- a/cli/command/manifest/util.go +++ b/cli/command/manifest/util.go @@ -70,7 +70,7 @@ func normalizeReference(ref string) (reference.Named, error) { // getManifest from the local store, and fallback to the remote registry if it // doesn't exist locally func getManifest(ctx context.Context, dockerCli command.Cli, listRef, namedRef reference.Named, insecure bool) (types.ImageManifest, error) { - data, err := dockerCli.ManifestStore().Get(listRef, namedRef) + data, err := newManifestStore(dockerCli).Get(listRef, namedRef) switch { case store.IsNotFound(err): return dockerCli.RegistryClient(insecure).GetManifest(ctx, namedRef)