Skip to content

Commit

Permalink
Merge pull request #5889 from thaJeztah/internalize_manifest
Browse files Browse the repository at this point in the history
cli/command: deprecate Cli.ManifestStore, Cli.RegistryClient
  • Loading branch information
thaJeztah authored Mar 4, 2025
2 parents 7bcbe08 + 8ad0721 commit ea1f10b
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 35 deletions.
22 changes: 1 addition & 21 deletions cli/command/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"fmt"
"io"
"os"
"path/filepath"
"runtime"
"strconv"
"sync"
Expand All @@ -21,14 +20,11 @@ import (
"github.com/docker/cli/cli/context/store"
"github.com/docker/cli/cli/debug"
cliflags "github.com/docker/cli/cli/flags"
manifeststore "github.com/docker/cli/cli/manifest/store"
registryclient "github.com/docker/cli/cli/registry/client"
"github.com/docker/cli/cli/streams"
"github.com/docker/cli/cli/version"
dopts "github.com/docker/cli/opts"
"github.com/docker/docker/api"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/registry"
"github.com/docker/docker/api/types/swarm"
"github.com/docker/docker/client"
"github.com/pkg/errors"
Expand All @@ -54,15 +50,14 @@ type Cli interface {
ServerInfo() ServerInfo
DefaultVersion() string
CurrentVersion() string
ManifestStore() manifeststore.Store
RegistryClient(bool) registryclient.RegistryClient
ContentTrustEnabled() bool
BuildKitEnabled() (bool, error)
ContextStore() store.Store
CurrentContext() string
DockerEndpoint() docker.Endpoint
TelemetryClient
DeprecatedNotaryClient
DeprecatedManifestClient
}

// DockerCli is an instance the docker command line client.
Expand Down Expand Up @@ -227,21 +222,6 @@ func (cli *DockerCli) HooksEnabled() bool {
return false
}

// ManifestStore returns a store for local manifests
func (*DockerCli) ManifestStore() manifeststore.Store {
// TODO: support override default location from config file
return manifeststore.NewStore(filepath.Join(config.Dir(), "manifests"))
}

// RegistryClient returns a client for communicating with a Docker distribution
// registry
func (cli *DockerCli) RegistryClient(allowInsecure bool) registryclient.RegistryClient {
resolver := func(ctx context.Context, index *registry.IndexInfo) registry.AuthConfig {
return ResolveAuthConfig(cli.ConfigFile(), index)
}
return registryclient.NewRegistryClient(resolver, UserAgent(), allowInsecure)
}

// WithInitializeClient is passed to DockerCli.Initialize by callers who wish to set a particular API Client for use by the CLI.
func WithInitializeClient(makeClient func(dockerCli *DockerCli) (client.APIClient, error)) CLIOption {
return func(dockerCli *DockerCli) error {
Expand Down
38 changes: 38 additions & 0 deletions cli/command/cli_deprecated.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
package command

import (
"context"
"path/filepath"

"github.com/docker/cli/cli/config"
manifeststore "github.com/docker/cli/cli/manifest/store"
registryclient "github.com/docker/cli/cli/registry/client"
"github.com/docker/cli/cli/trust"
"github.com/docker/docker/api/types/registry"
notaryclient "github.com/theupdateframework/notary/client"
)

Expand All @@ -12,7 +19,38 @@ type DeprecatedNotaryClient interface {
NotaryClient(imgRefAndAuth trust.ImageRefAndAuth, actions []string) (notaryclient.Repository, error)
}

type DeprecatedManifestClient interface {
// ManifestStore returns a store for local manifests
//
// Deprecated: use [manifeststore.NewStore] instead. This method is no longer used and will be removed in the next release.
ManifestStore() manifeststore.Store

// RegistryClient returns a client for communicating with a Docker distribution
// registry.
//
// Deprecated: use [registryclient.NewRegistryClient]. This method is no longer used and will be removed in the next release.
RegistryClient(bool) registryclient.RegistryClient
}

// NotaryClient provides a Notary Repository to interact with signed metadata for an image
func (cli *DockerCli) NotaryClient(imgRefAndAuth trust.ImageRefAndAuth, actions []string) (notaryclient.Repository, error) {
return trust.GetNotaryRepository(cli.In(), cli.Out(), UserAgent(), imgRefAndAuth.RepoInfo(), imgRefAndAuth.AuthConfig(), actions...)
}

// ManifestStore returns a store for local manifests
//
// Deprecated: use [manifeststore.NewStore] instead. This method is no longer used and will be removed in the next release.
func (*DockerCli) ManifestStore() manifeststore.Store {
return manifeststore.NewStore(filepath.Join(config.Dir(), "manifests"))
}

// RegistryClient returns a client for communicating with a Docker distribution
// registry
//
// Deprecated: use [registryclient.NewRegistryClient]. This method is no longer used and will be removed in the next release.
func (cli *DockerCli) RegistryClient(allowInsecure bool) registryclient.RegistryClient {
resolver := func(ctx context.Context, index *registry.IndexInfo) registry.AuthConfig {
return ResolveAuthConfig(cli.ConfigFile(), index)
}
return registryclient.NewRegistryClient(resolver, UserAgent(), allowInsecure)
}
40 changes: 38 additions & 2 deletions cli/command/manifest/annotate.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
package manifest

import (
"context"
"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"
registryclient "github.com/docker/cli/cli/registry/client"
"github.com/docker/docker/api/types/registry"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand All @@ -21,6 +26,37 @@ 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
RegistryClient(bool) registryclient.RegistryClient
}

// 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"))
}

// newRegistryClient returns a client for communicating with a Docker distribution
// registry
func newRegistryClient(dockerCLI command.Cli, allowInsecure bool) registryclient.RegistryClient {
if msp, ok := dockerCLI.(manifestStoreProvider); ok {
// manifestStoreProvider is used in tests to provide a dummy store.
return msp.RegistryClient(allowInsecure)
}
resolver := func(ctx context.Context, index *registry.IndexInfo) registry.AuthConfig {
return command.ResolveAuthConfig(dockerCLI.ConfigFile(), index)
}
return registryclient.NewRegistryClient(resolver, command.UserAgent(), allowInsecure)
}

// NewAnnotateCommand creates a new `docker manifest annotate` command
func newAnnotateCommand(dockerCli command.Cli) *cobra.Command {
var opts annotateOptions
Expand All @@ -47,7 +83,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)
Expand All @@ -57,7 +93,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):
Expand Down
2 changes: 1 addition & 1 deletion cli/command/manifest/create_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
6 changes: 3 additions & 3 deletions cli/command/manifest/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,21 @@ 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
}
return printManifest(dockerCli, imageManifest, opts)
}

// 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)
}

// Next try a remote manifest
registryClient := dockerCli.RegistryClient(opts.insecure)
registryClient := newRegistryClient(dockerCli, opts.insecure)
imageManifest, err := registryClient.GetManifest(ctx, namedRef)
if err == nil {
return printManifest(dockerCli, imageManifest, opts)
Expand Down
6 changes: 3 additions & 3 deletions cli/command/manifest/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -248,7 +248,7 @@ func buildPutManifestRequest(imageManifest types.ImageManifest, targetRef refere
}

func pushList(ctx context.Context, dockerCLI command.Cli, req pushRequest) error {
rclient := dockerCLI.RegistryClient(req.insecure)
rclient := newRegistryClient(dockerCLI, req.insecure)

if err := mountBlobs(ctx, rclient, req.targetRef, req.manifestBlobs); err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cli/command/manifest/rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
}

Expand Down
8 changes: 4 additions & 4 deletions cli/command/manifest/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,15 @@ 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)
func getManifest(ctx context.Context, dockerCLI command.Cli, listRef, namedRef reference.Named, insecure bool) (types.ImageManifest, error) {
data, err := newManifestStore(dockerCLI).Get(listRef, namedRef)
switch {
case store.IsNotFound(err):
return dockerCli.RegistryClient(insecure).GetManifest(ctx, namedRef)
return newRegistryClient(dockerCLI, insecure).GetManifest(ctx, namedRef)
case err != nil:
return types.ImageManifest{}, err
case len(data.Raw) == 0:
return dockerCli.RegistryClient(insecure).GetManifest(ctx, namedRef)
return newRegistryClient(dockerCLI, insecure).GetManifest(ctx, namedRef)
default:
return data, nil
}
Expand Down

0 comments on commit ea1f10b

Please sign in to comment.