Skip to content

Commit b004f8a

Browse files
committed
cli/command: remove ManifestStore from CLI interface
This method was a shallow wrapper around manifeststore.NewStore, but 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. Thie patch: - Removes the ManifestStore method from the interface - Inlines the code where needed, skipping the wrapper - Define a local interface for some tests where a dummy store was used. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1 parent 1e9ed3a commit b004f8a

7 files changed

Lines changed: 28 additions & 18 deletions

File tree

cli/command/cli.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"fmt"
99
"io"
1010
"os"
11-
"path/filepath"
1211
"runtime"
1312
"strconv"
1413
"strings"
@@ -22,7 +21,6 @@ import (
2221
"github.com/docker/cli/cli/context/store"
2322
"github.com/docker/cli/cli/debug"
2423
cliflags "github.com/docker/cli/cli/flags"
25-
manifeststore "github.com/docker/cli/cli/manifest/store"
2624
registryclient "github.com/docker/cli/cli/registry/client"
2725
"github.com/docker/cli/cli/streams"
2826
"github.com/docker/cli/cli/version"
@@ -56,7 +54,6 @@ type Cli interface {
5654
ServerInfo() ServerInfo
5755
DefaultVersion() string
5856
CurrentVersion() string
59-
ManifestStore() manifeststore.Store
6057
RegistryClient(bool) registryclient.RegistryClient
6158
ContentTrustEnabled() bool
6259
BuildKitEnabled() (bool, error)
@@ -228,12 +225,6 @@ func (cli *DockerCli) HooksEnabled() bool {
228225
return false
229226
}
230227

231-
// ManifestStore returns a store for local manifests
232-
func (*DockerCli) ManifestStore() manifeststore.Store {
233-
// TODO: support override default location from config file
234-
return manifeststore.NewStore(filepath.Join(config.Dir(), "manifests"))
235-
}
236-
237228
// RegistryClient returns a client for communicating with a Docker distribution
238229
// registry
239230
func (cli *DockerCli) RegistryClient(allowInsecure bool) registryclient.RegistryClient {

cli/command/manifest/annotate.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ package manifest
22

33
import (
44
"fmt"
5+
"path/filepath"
56

67
"github.com/docker/cli/cli"
78
"github.com/docker/cli/cli/command"
9+
"github.com/docker/cli/cli/config"
810
"github.com/docker/cli/cli/manifest/store"
911
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
1012
"github.com/pkg/errors"
@@ -21,6 +23,23 @@ type annotateOptions struct {
2123
osVersion string
2224
}
2325

26+
// manifestStoreProvider is used in tests to provide a dummy store.
27+
type manifestStoreProvider interface {
28+
// ManifestStore returns a store for local manifests
29+
ManifestStore() store.Store
30+
}
31+
32+
// newManifestStore returns a store for local manifests
33+
func newManifestStore(dockerCLI command.Cli) store.Store {
34+
if msp, ok := dockerCLI.(manifestStoreProvider); ok {
35+
// manifestStoreProvider is used in tests to provide a dummy store.
36+
return msp.ManifestStore()
37+
}
38+
39+
// TODO: support override default location from config file
40+
return store.NewStore(filepath.Join(config.Dir(), "manifests"))
41+
}
42+
2443
// NewAnnotateCommand creates a new `docker manifest annotate` command
2544
func newAnnotateCommand(dockerCli command.Cli) *cobra.Command {
2645
var opts annotateOptions
@@ -47,7 +66,7 @@ func newAnnotateCommand(dockerCli command.Cli) *cobra.Command {
4766
return cmd
4867
}
4968

50-
func runManifestAnnotate(dockerCli command.Cli, opts annotateOptions) error {
69+
func runManifestAnnotate(dockerCLI command.Cli, opts annotateOptions) error {
5170
targetRef, err := normalizeReference(opts.target)
5271
if err != nil {
5372
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 {
5776
return errors.Wrapf(err, "annotate: error parsing name for manifest %s", opts.image)
5877
}
5978

60-
manifestStore := dockerCli.ManifestStore()
79+
manifestStore := newManifestStore(dockerCLI)
6180
imageManifest, err := manifestStore.Get(targetRef, imgRef)
6281
switch {
6382
case store.IsNotFound(err):

cli/command/manifest/create_list.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func createManifestList(ctx context.Context, dockerCLI command.Cli, args []strin
4141
return errors.Wrapf(err, "error parsing name for manifest list %s", newRef)
4242
}
4343

44-
manifestStore := dockerCLI.ManifestStore()
44+
manifestStore := newManifestStore(dockerCLI)
4545
_, err = manifestStore.GetList(targetRef)
4646
switch {
4747
case store.IsNotFound(err):

cli/command/manifest/inspect.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,15 @@ func runInspect(ctx context.Context, dockerCli command.Cli, opts inspectOptions)
6161
return err
6262
}
6363

64-
imageManifest, err := dockerCli.ManifestStore().Get(listRef, namedRef)
64+
imageManifest, err := newManifestStore(dockerCli).Get(listRef, namedRef)
6565
if err != nil {
6666
return err
6767
}
6868
return printManifest(dockerCli, imageManifest, opts)
6969
}
7070

7171
// Try a local manifest list first
72-
localManifestList, err := dockerCli.ManifestStore().GetList(namedRef)
72+
localManifestList, err := newManifestStore(dockerCli).GetList(namedRef)
7373
if err == nil {
7474
return printManifestList(dockerCli, namedRef, localManifestList, opts)
7575
}

cli/command/manifest/push.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func runPush(ctx context.Context, dockerCli command.Cli, opts pushOpts) error {
6868
return err
6969
}
7070

71-
manifests, err := dockerCli.ManifestStore().GetList(targetRef)
71+
manifests, err := newManifestStore(dockerCli).GetList(targetRef)
7272
if err != nil {
7373
return err
7474
}
@@ -85,7 +85,7 @@ func runPush(ctx context.Context, dockerCli command.Cli, opts pushOpts) error {
8585
return err
8686
}
8787
if opts.purge {
88-
return dockerCli.ManifestStore().Remove(targetRef)
88+
return newManifestStore(dockerCli).Remove(targetRef)
8989
}
9090
return nil
9191
}

cli/command/manifest/rm.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ func newRmManifestListCommand(dockerCLI command.Cli) *cobra.Command {
1616
Short: "Delete one or more manifest lists from local storage",
1717
Args: cli.RequiresMinArgs(1),
1818
RunE: func(cmd *cobra.Command, args []string) error {
19-
return runRemove(cmd.Context(), dockerCLI.ManifestStore(), args)
19+
return runRemove(cmd.Context(), newManifestStore(dockerCLI), args)
2020
},
2121
}
2222

cli/command/manifest/util.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func normalizeReference(ref string) (reference.Named, error) {
7070
// getManifest from the local store, and fallback to the remote registry if it
7171
// doesn't exist locally
7272
func getManifest(ctx context.Context, dockerCli command.Cli, listRef, namedRef reference.Named, insecure bool) (types.ImageManifest, error) {
73-
data, err := dockerCli.ManifestStore().Get(listRef, namedRef)
73+
data, err := newManifestStore(dockerCli).Get(listRef, namedRef)
7474
switch {
7575
case store.IsNotFound(err):
7676
return dockerCli.RegistryClient(insecure).GetManifest(ctx, namedRef)

0 commit comments

Comments
 (0)