From bde9d521df9d9c710b36fa47fd008eae34a33f61 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 30 Jan 2023 12:02:43 +0000 Subject: [PATCH 1/2] manifest: rework error handling This patch reworks and updates the error handling in the docker manifest command to be more inline with modern go principles. Specifically, we use the new errors.Is function, along with errors.Wrap, to preserve error hierarchy instead of using manual string operations. This allows us to simplify various testing components, as well as simplifying the implementation of manifest merging later. Signed-off-by: Justin Chadwell --- cli/command/manifest/annotate.go | 4 ++-- cli/command/manifest/create_list.go | 4 ++-- cli/command/manifest/create_test.go | 6 ++--- cli/command/manifest/inspect_test.go | 10 ++++----- cli/command/manifest/rm_test.go | 8 +++---- cli/command/manifest/util.go | 33 ++++++++++++++++++---------- cli/manifest/store/store.go | 30 ++----------------------- cli/manifest/store/store_test.go | 15 ++++++------- cli/manifest/types/types.go | 10 +++++++++ cli/registry/client/fetcher.go | 29 +++++++----------------- 10 files changed, 65 insertions(+), 84 deletions(-) diff --git a/cli/command/manifest/annotate.go b/cli/command/manifest/annotate.go index c12b69d823e9..226de3033c25 100644 --- a/cli/command/manifest/annotate.go +++ b/cli/command/manifest/annotate.go @@ -5,7 +5,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" - "github.com/docker/cli/cli/manifest/store" + "github.com/docker/cli/cli/manifest/types" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -60,7 +60,7 @@ func runManifestAnnotate(dockerCli command.Cli, opts annotateOptions) error { manifestStore := dockerCli.ManifestStore() imageManifest, err := manifestStore.Get(targetRef, imgRef) switch { - case store.IsNotFound(err): + case errors.Is(err, types.ErrManifestNotFound): return fmt.Errorf("manifest for image %s does not exist in %s", opts.image, opts.target) case err != nil: return err diff --git a/cli/command/manifest/create_list.go b/cli/command/manifest/create_list.go index f2e54dcff6d9..7dcee98b86bd 100644 --- a/cli/command/manifest/create_list.go +++ b/cli/command/manifest/create_list.go @@ -6,7 +6,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" - "github.com/docker/cli/cli/manifest/store" + "github.com/docker/cli/cli/manifest/types" "github.com/docker/docker/registry" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -50,7 +50,7 @@ func createManifestList(dockerCli command.Cli, args []string, opts createOpts) e manifestStore := dockerCli.ManifestStore() _, err = manifestStore.GetList(targetRef) switch { - case store.IsNotFound(err): + case errors.Is(err, types.ErrManifestNotFound): // New manifest list case err != nil: return err diff --git a/cli/command/manifest/create_test.go b/cli/command/manifest/create_test.go index e1092ee3d71e..1041360d38d1 100644 --- a/cli/command/manifest/create_test.go +++ b/cli/command/manifest/create_test.go @@ -99,10 +99,10 @@ func TestManifestCreateNoManifest(t *testing.T) { cli.SetManifestStore(store) cli.SetRegistryClient(&fakeRegistryClient{ getManifestFunc: func(_ context.Context, ref reference.Named) (manifesttypes.ImageManifest, error) { - return manifesttypes.ImageManifest{}, errors.Errorf("No such image: %v", ref) + return manifesttypes.ImageManifest{}, errors.Wrapf(manifesttypes.ErrManifestNotFound, "%q does not exist", ref) }, getManifestListFunc: func(ctx context.Context, ref reference.Named) ([]manifesttypes.ImageManifest, error) { - return nil, errors.Errorf("No such manifest: %s", ref) + return nil, errors.Wrapf(manifesttypes.ErrManifestNotFound, "%q does not exist", ref) }, }) @@ -110,5 +110,5 @@ func TestManifestCreateNoManifest(t *testing.T) { cmd.SetArgs([]string{"example.com/list:v1", "example.com/alpine:3.0"}) cmd.SetOut(io.Discard) err := cmd.Execute() - assert.Error(t, err, "No such image: example.com/alpine:3.0") + assert.Error(t, err, `"example.com/alpine:3.0" does not exist: manifest not found`) } diff --git a/cli/command/manifest/inspect_test.go b/cli/command/manifest/inspect_test.go index 9f4139e08cb5..0490f07e2593 100644 --- a/cli/command/manifest/inspect_test.go +++ b/cli/command/manifest/inspect_test.go @@ -70,7 +70,7 @@ func TestInspectCommandLocalManifestNotFound(t *testing.T) { cmd.SetOut(io.Discard) cmd.SetArgs([]string{"example.com/list:v1", "example.com/alpine:3.0"}) err := cmd.Execute() - assert.Error(t, err, "No such manifest: example.com/alpine:3.0") + assert.Error(t, err, `"example.com/alpine:3.0" does not exist: manifest not found`) } func TestInspectCommandNotFound(t *testing.T) { @@ -79,11 +79,11 @@ func TestInspectCommandNotFound(t *testing.T) { cli := test.NewFakeCli(nil) cli.SetManifestStore(store) cli.SetRegistryClient(&fakeRegistryClient{ - getManifestFunc: func(_ context.Context, _ reference.Named) (types.ImageManifest, error) { - return types.ImageManifest{}, errors.New("missing") + getManifestFunc: func(ctx context.Context, ref reference.Named) (types.ImageManifest, error) { + return types.ImageManifest{}, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", ref) }, getManifestListFunc: func(ctx context.Context, ref reference.Named) ([]types.ImageManifest, error) { - return nil, errors.Errorf("No such manifest: %s", ref) + return nil, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", ref) }, }) @@ -91,7 +91,7 @@ func TestInspectCommandNotFound(t *testing.T) { cmd.SetOut(io.Discard) cmd.SetArgs([]string{"example.com/alpine:3.0"}) err := cmd.Execute() - assert.Error(t, err, "No such manifest: example.com/alpine:3.0") + assert.Error(t, err, `"example.com/alpine:3.0" does not exist: manifest not found`) } func TestInspectCommandLocalManifest(t *testing.T) { diff --git a/cli/command/manifest/rm_test.go b/cli/command/manifest/rm_test.go index d76f85a507f7..1fbf88c42dec 100644 --- a/cli/command/manifest/rm_test.go +++ b/cli/command/manifest/rm_test.go @@ -37,8 +37,8 @@ func TestRmSeveralManifests(t *testing.T) { _, search1 := cli.ManifestStore().GetList(list1) _, search2 := cli.ManifestStore().GetList(list2) - assert.Error(t, search1, "No such manifest: example.com/first:1") - assert.Error(t, search2, "No such manifest: example.com/second:2") + assert.Error(t, search1, `"example.com/first:1" does not exist: manifest not found`) + assert.Error(t, search2, `"example.com/second:2" does not exist: manifest not found`) } // attempt to remove a manifest list which was never created @@ -57,8 +57,8 @@ func TestRmManifestNotCreated(t *testing.T) { cmd.SetArgs([]string{"example.com/first:1", "example.com/second:2"}) cmd.SetOut(io.Discard) err = cmd.Execute() - assert.Error(t, err, "No such manifest: example.com/first:1") + assert.Error(t, err, `"example.com/first:1" does not exist: manifest not found`) _, err = cli.ManifestStore().GetList(list2) - assert.Error(t, err, "No such manifest: example.com/second:2") + assert.Error(t, err, `"example.com/second:2" does not exist: manifest not found`) } diff --git a/cli/command/manifest/util.go b/cli/command/manifest/util.go index 16aba15c7488..398589cab9d5 100644 --- a/cli/command/manifest/util.go +++ b/cli/command/manifest/util.go @@ -4,9 +4,9 @@ import ( "context" "github.com/docker/cli/cli/command" - "github.com/docker/cli/cli/manifest/store" "github.com/docker/cli/cli/manifest/types" "github.com/docker/distribution/reference" + "github.com/pkg/errors" ) type osArch struct { @@ -70,15 +70,26 @@ 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) - switch { - case store.IsNotFound(err): - return dockerCli.RegistryClient(insecure).GetManifest(ctx, namedRef) - case err != nil: - return types.ImageManifest{}, err - case len(data.Raw) == 0: - return dockerCli.RegistryClient(insecure).GetManifest(ctx, namedRef) - default: - return data, nil + // load from the local store + if listRef != nil { + data, err := dockerCli.ManifestStore().Get(listRef, namedRef) + if err == nil { + return data, nil + } else if !errors.Is(err, types.ErrManifestNotFound) { + return types.ImageManifest{}, err + } } + + // load from the remote registry + client := dockerCli.RegistryClient(insecure) + if client != nil { + data, err := client.GetManifest(ctx, namedRef) + if err == nil { + return data, nil + } else if !errors.Is(err, types.ErrManifestNotFound) { + return types.ImageManifest{}, err + } + } + + return types.ImageManifest{}, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", namedRef) } diff --git a/cli/manifest/store/store.go b/cli/manifest/store/store.go index 39e576f6b2db..b0f951adbdee 100644 --- a/cli/manifest/store/store.go +++ b/cli/manifest/store/store.go @@ -2,7 +2,6 @@ package store import ( "encoding/json" - "fmt" "os" "path/filepath" "strings" @@ -49,7 +48,7 @@ func (s *fsStore) getFromFilename(ref reference.Reference, filename string) (typ bytes, err := os.ReadFile(filename) switch { case os.IsNotExist(err): - return types.ImageManifest{}, newNotFoundError(ref.String()) + return types.ImageManifest{}, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", ref.String()) case err != nil: return types.ImageManifest{}, err } @@ -93,7 +92,7 @@ func (s *fsStore) GetList(listRef reference.Reference) ([]types.ImageManifest, e case err != nil: return nil, err case filenames == nil: - return nil, newNotFoundError(listRef.String()) + return nil, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", listRef.String()) } manifests := []types.ImageManifest{} @@ -152,28 +151,3 @@ func makeFilesafeName(ref string) string { fileName := strings.Replace(ref, ":", "-", -1) return strings.Replace(fileName, "/", "_", -1) } - -type notFoundError struct { - object string -} - -func newNotFoundError(ref string) *notFoundError { - return ¬FoundError{object: ref} -} - -func (n *notFoundError) Error() string { - return fmt.Sprintf("No such manifest: %s", n.object) -} - -// NotFound interface -func (n *notFoundError) NotFound() {} - -// IsNotFound returns true if the error is a not found error -func IsNotFound(err error) bool { - _, ok := err.(notFound) - return ok -} - -type notFound interface { - NotFound() -} diff --git a/cli/manifest/store/store_test.go b/cli/manifest/store/store_test.go index cea0005f2f17..19fd30684d75 100644 --- a/cli/manifest/store/store_test.go +++ b/cli/manifest/store/store_test.go @@ -7,6 +7,7 @@ import ( "github.com/docker/cli/cli/manifest/types" "github.com/docker/distribution/reference" "github.com/google/go-cmp/cmp" + "github.com/pkg/errors" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -62,7 +63,7 @@ func TestStoreSaveAndGet(t *testing.T) { listRef reference.Reference manifestRef reference.Reference expected types.ImageManifest - expectedErr string + expectedErr error }{ { listRef: listRef, @@ -72,12 +73,12 @@ func TestStoreSaveAndGet(t *testing.T) { { listRef: listRef, manifestRef: ref("exist:does-not"), - expectedErr: "No such manifest: exist:does-not", + expectedErr: types.ErrManifestNotFound, }, { listRef: ref("list:does-not-exist"), manifestRef: ref("manifest:does-not-exist"), - expectedErr: "No such manifest: manifest:does-not-exist", + expectedErr: types.ErrManifestNotFound, }, } @@ -85,9 +86,8 @@ func TestStoreSaveAndGet(t *testing.T) { testcase := testcase t.Run(testcase.manifestRef.String(), func(t *testing.T) { actual, err := store.Get(testcase.listRef, testcase.manifestRef) - if testcase.expectedErr != "" { - assert.Error(t, err, testcase.expectedErr) - assert.Check(t, IsNotFound(err)) + if testcase.expectedErr != nil { + assert.Check(t, errors.Is(err, types.ErrManifestNotFound)) return } assert.NilError(t, err) @@ -117,6 +117,5 @@ func TestStoreGetListDoesNotExist(t *testing.T) { store := NewStore(t.TempDir()) listRef := ref("list") _, err := store.GetList(listRef) - assert.Error(t, err, "No such manifest: list") - assert.Check(t, IsNotFound(err)) + assert.Check(t, errors.Is(err, types.ErrManifestNotFound)) } diff --git a/cli/manifest/types/types.go b/cli/manifest/types/types.go index ca2a3e7866a4..8560d88a9490 100644 --- a/cli/manifest/types/types.go +++ b/cli/manifest/types/types.go @@ -13,6 +13,16 @@ import ( "github.com/pkg/errors" ) +var ( + // ErrManifestNotFound is an error returned when a requested manifest or + // manifest list cannot be found in the store or the registry + ErrManifestNotFound = errors.New("manifest not found") + + // ErrUnknownType is an error returned when a manifest was found, but it + // was of an unrecognized type + ErrUnknownType = errors.New("unknown manifest type") +) + // ImageManifest contains info to output for a manifest object. type ImageManifest struct { Ref *SerializableNamed diff --git a/cli/registry/client/fetcher.go b/cli/registry/client/fetcher.go index acae274a4431..de100a65868f 100644 --- a/cli/registry/client/fetcher.go +++ b/cli/registry/client/fetcher.go @@ -43,9 +43,9 @@ func fetchManifest(ctx context.Context, repo distribution.Repository, ref refere } return imageManifest, nil case *manifestlist.DeserializedManifestList: - return types.ImageManifest{}, errors.Errorf("%s is a manifest list", ref) + return types.ImageManifest{}, errors.Wrapf(types.ErrManifestNotFound, "%q is a manifest list", ref) } - return types.ImageManifest{}, errors.Errorf("%s is not a manifest", ref) + return types.ImageManifest{}, errors.Wrapf(types.ErrUnknownType, "%q does not exist", ref) } func fetchList(ctx context.Context, repo distribution.Repository, ref reference.Named) ([]types.ImageManifest, error) { @@ -55,6 +55,8 @@ func fetchList(ctx context.Context, repo distribution.Repository, ref reference. } switch v := manifest.(type) { + case *schema2.DeserializedManifest, *ocischema.DeserializedManifest: + return nil, errors.Wrapf(types.ErrManifestNotFound, "%q is not a manifest list", ref) case *manifestlist.DeserializedManifestList: imageManifests, err := pullManifestList(ctx, ref, repo, *v) if err != nil { @@ -62,7 +64,7 @@ func fetchList(ctx context.Context, repo distribution.Repository, ref reference. } return imageManifests, nil default: - return nil, errors.Errorf("unsupported manifest format: %v", v) + return nil, types.ErrUnknownType } } @@ -74,7 +76,7 @@ func getManifest(ctx context.Context, repo distribution.Repository, ref referenc dgst, opts, err := getManifestOptionsFromReference(ref) if err != nil { - return nil, errors.Errorf("image manifest for %q does not exist", ref) + return nil, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", ref) } return manSvc.Get(ctx, dgst, opts...) } @@ -195,7 +197,7 @@ func pullManifestList(ctx context.Context, ref reference.Named, repo distributio case *ocischema.DeserializedManifest: imageManifest, err = pullManifestOCISchema(ctx, manifestRef, repo, *v) default: - err = errors.Errorf("unsupported manifest type: %T", manifest) + err = types.ErrUnknownType } if err != nil { return nil, err @@ -287,7 +289,7 @@ func (c *client) iterateEndpoints(ctx context.Context, namedRef reference.Named, return nil } } - return newNotFoundError(namedRef.String()) + return errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", namedRef) } // allEndpoints returns a list of endpoints ordered by priority (v2, https, v1). @@ -310,18 +312,3 @@ func allEndpoints(namedRef reference.Named, insecure bool) ([]registry.APIEndpoi logrus.Debugf("endpoints for %s: %v", namedRef, endpoints) return endpoints, err } - -func newNotFoundError(ref string) *notFoundError { - return ¬FoundError{err: errors.New("no such manifest: " + ref)} -} - -type notFoundError struct { - err error -} - -func (n *notFoundError) Error() string { - return n.err.Error() -} - -// NotFound satisfies interface github.com/docker/docker/errdefs.ErrNotFound -func (n *notFoundError) NotFound() {} From 577dc075f87bacb8200008efb236a177c5b4d887 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 30 Jan 2023 12:02:58 +0000 Subject: [PATCH 2/2] manifest: merge manifest lists Previously, merging multiple manifest lists together was not supported as a use case. However, with new versions of buildkit, manifest lists will be created by default. To support these new manifest lists, we should support merging manifest lists, just as we support this with manifests today. To do this, we refactor the manifest store to save and load groups of manifests, instead of only one. On disk, these manifests take the form of suffixed indexes, e.g. "_2", "_3", etc (note that we ignore "_1", which is permitted to allow a backwards-compatible change with the previous disk format). Then, only the commands need small updates - "create" now looks up multiple manifests and saves multiple manifests, while "annotate" now annotates all the matching manifests. Signed-off-by: Justin Chadwell --- cli/command/manifest/annotate.go | 50 +++++++++++++------------- cli/command/manifest/create_list.go | 4 +-- cli/command/manifest/inspect.go | 36 +++++++------------ cli/command/manifest/util.go | 25 +++++++++---- cli/manifest/store/store.go | 54 +++++++++++++++++++++++++---- cli/manifest/store/store_test.go | 6 ++-- 6 files changed, 109 insertions(+), 66 deletions(-) diff --git a/cli/command/manifest/annotate.go b/cli/command/manifest/annotate.go index 226de3033c25..7753a3094788 100644 --- a/cli/command/manifest/annotate.go +++ b/cli/command/manifest/annotate.go @@ -58,7 +58,7 @@ func runManifestAnnotate(dockerCli command.Cli, opts annotateOptions) error { } manifestStore := dockerCli.ManifestStore() - imageManifest, err := manifestStore.Get(targetRef, imgRef) + imageManifests, err := manifestStore.Get(targetRef, imgRef) switch { case errors.Is(err, types.ErrManifestNotFound): return fmt.Errorf("manifest for image %s does not exist in %s", opts.image, opts.target) @@ -66,30 +66,32 @@ func runManifestAnnotate(dockerCli command.Cli, opts annotateOptions) error { return err } - // Update the mf - if imageManifest.Descriptor.Platform == nil { - imageManifest.Descriptor.Platform = new(ocispec.Platform) - } - if opts.os != "" { - imageManifest.Descriptor.Platform.OS = opts.os - } - if opts.arch != "" { - imageManifest.Descriptor.Platform.Architecture = opts.arch - } - for _, osFeature := range opts.osFeatures { - imageManifest.Descriptor.Platform.OSFeatures = appendIfUnique(imageManifest.Descriptor.Platform.OSFeatures, osFeature) - } - if opts.variant != "" { - imageManifest.Descriptor.Platform.Variant = opts.variant - } - if opts.osVersion != "" { - imageManifest.Descriptor.Platform.OSVersion = opts.osVersion - } - - if !isValidOSArch(imageManifest.Descriptor.Platform.OS, imageManifest.Descriptor.Platform.Architecture) { - return errors.Errorf("manifest entry for image has unsupported os/arch combination: %s/%s", opts.os, opts.arch) + for i, imageManifest := range imageManifests { + // Update the mf + if imageManifest.Descriptor.Platform == nil { + imageManifest.Descriptor.Platform = new(ocispec.Platform) + } + if opts.os != "" { + imageManifest.Descriptor.Platform.OS = opts.os + } + if opts.arch != "" { + imageManifest.Descriptor.Platform.Architecture = opts.arch + } + for _, osFeature := range opts.osFeatures { + imageManifest.Descriptor.Platform.OSFeatures = appendIfUnique(imageManifest.Descriptor.Platform.OSFeatures, osFeature) + } + if opts.variant != "" { + imageManifest.Descriptor.Platform.Variant = opts.variant + } + if opts.osVersion != "" { + imageManifest.Descriptor.Platform.OSVersion = opts.osVersion + } + if !isValidOSArch(imageManifest.Descriptor.Platform.OS, imageManifest.Descriptor.Platform.Architecture) { + return errors.Errorf("manifest entry for image has unsupported os/arch combination: %s/%s", opts.os, opts.arch) + } + imageManifests[i] = imageManifest } - return manifestStore.Save(targetRef, imgRef, imageManifest) + return manifestStore.Save(targetRef, imgRef, imageManifests...) } func appendIfUnique(list []string, str string) []string { diff --git a/cli/command/manifest/create_list.go b/cli/command/manifest/create_list.go index 7dcee98b86bd..ed78fdde586c 100644 --- a/cli/command/manifest/create_list.go +++ b/cli/command/manifest/create_list.go @@ -69,11 +69,11 @@ func createManifestList(dockerCli command.Cli, args []string, opts createOpts) e return err } - manifest, err := getManifest(ctx, dockerCli, targetRef, namedRef, opts.insecure) + manifests, err := getManifests(ctx, dockerCli, targetRef, namedRef, opts.insecure) if err != nil { return err } - if err := manifestStore.Save(targetRef, namedRef, manifest); err != nil { + if err := manifestStore.Save(targetRef, namedRef, manifests...); err != nil { return err } } diff --git a/cli/command/manifest/inspect.go b/cli/command/manifest/inspect.go index c270ee53beb7..a2f0aea8c763 100644 --- a/cli/command/manifest/inspect.go +++ b/cli/command/manifest/inspect.go @@ -55,40 +55,28 @@ func runInspect(dockerCli command.Cli, opts inspectOptions) error { return err } - // If list reference is provided, display the local manifest in a list + var listRef reference.Named if opts.list != "" { - listRef, err := normalizeReference(opts.list) + listRef, err = normalizeReference(opts.list) if err != nil { return err } - - imageManifest, err := dockerCli.ManifestStore().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) - if err == nil { - return printManifestList(dockerCli, namedRef, localManifestList, opts) } - // Next try a remote manifest ctx := context.Background() - registryClient := dockerCli.RegistryClient(opts.insecure) - imageManifest, err := registryClient.GetManifest(ctx, namedRef) - if err == nil { - return printManifest(dockerCli, imageManifest, opts) - } - - // Finally try a remote manifest list - manifestList, err := registryClient.GetManifestList(ctx, namedRef) + manifests, err := getManifests(ctx, dockerCli, listRef, namedRef, opts.insecure) if err != nil { return err } - return printManifestList(dockerCli, namedRef, manifestList, opts) + + switch len(manifests) { + case 0: + return nil + case 1: + return printManifest(dockerCli, manifests[0], opts) + default: + return printManifestList(dockerCli, namedRef, manifests, opts) + } } func printManifest(dockerCli command.Cli, manifest types.ImageManifest, opts inspectOptions) error { diff --git a/cli/command/manifest/util.go b/cli/command/manifest/util.go index 398589cab9d5..6a3d59a1a4d1 100644 --- a/cli/command/manifest/util.go +++ b/cli/command/manifest/util.go @@ -67,29 +67,42 @@ func normalizeReference(ref string) (reference.Named, error) { return namedRef, nil } -// getManifest from the local store, and fallback to the remote registry if it +// getManifests 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) { +func getManifests(ctx context.Context, dockerCli command.Cli, listRef, namedRef reference.Named, insecure bool) ([]types.ImageManifest, error) { // load from the local store if listRef != nil { data, err := dockerCli.ManifestStore().Get(listRef, namedRef) if err == nil { return data, nil } else if !errors.Is(err, types.ErrManifestNotFound) { - return types.ImageManifest{}, err + return nil, err } } + datas, err := dockerCli.ManifestStore().GetList(namedRef) + if err == nil { + return datas, nil + } else if !errors.Is(err, types.ErrManifestNotFound) { + return nil, err + } // load from the remote registry client := dockerCli.RegistryClient(insecure) if client != nil { data, err := client.GetManifest(ctx, namedRef) if err == nil { - return data, nil + return []types.ImageManifest{data}, nil + } else if !errors.Is(err, types.ErrManifestNotFound) { + return nil, err + } + + datas, err = client.GetManifestList(ctx, namedRef) + if err == nil { + return datas, nil } else if !errors.Is(err, types.ErrManifestNotFound) { - return types.ImageManifest{}, err + return nil, err } } - return types.ImageManifest{}, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", namedRef) + return nil, errors.Wrapf(types.ErrManifestNotFound, "%q does not exist", namedRef) } diff --git a/cli/manifest/store/store.go b/cli/manifest/store/store.go index b0f951adbdee..25c35e5e2927 100644 --- a/cli/manifest/store/store.go +++ b/cli/manifest/store/store.go @@ -2,6 +2,7 @@ package store import ( "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -17,9 +18,9 @@ import ( // Store manages local storage of image distribution manifests type Store interface { Remove(listRef reference.Reference) error - Get(listRef reference.Reference, manifest reference.Reference) (types.ImageManifest, error) + Get(listRef reference.Reference, manifest reference.Reference) ([]types.ImageManifest, error) GetList(listRef reference.Reference) ([]types.ImageManifest, error) - Save(listRef reference.Reference, manifest reference.Reference, image types.ImageManifest) error + Save(listRef reference.Reference, manifest reference.Reference, image ...types.ImageManifest) error } // fsStore manages manifest files stored on the local filesystem @@ -39,9 +40,30 @@ func (s *fsStore) Remove(listRef reference.Reference) error { } // Get returns the local manifest -func (s *fsStore) Get(listRef reference.Reference, manifest reference.Reference) (types.ImageManifest, error) { +func (s *fsStore) Get(listRef reference.Reference, manifest reference.Reference) ([]types.ImageManifest, error) { + var imgs []types.ImageManifest filename := manifestToFilename(s.root, listRef.String(), manifest.String()) - return s.getFromFilename(manifest, filename) + + img, err := s.getFromFilename(manifest, filename) + if err != nil { + return nil, err + } + imgs = append(imgs, img) + + i := 2 + for { + img, err := s.getFromFilename(manifest, fmt.Sprintf("%s_%d", filename, i)) + if errors.Is(err, types.ErrManifestNotFound) { + break + } else if err != nil { + return nil, err + } + imgs = append(imgs, img) + + i++ + } + + return imgs, nil } func (s *fsStore) getFromFilename(ref reference.Reference, filename string) (types.ImageManifest, error) { @@ -126,16 +148,34 @@ func (s *fsStore) listManifests(transaction string) ([]string, error) { } // Save a manifest as part of a local manifest list -func (s *fsStore) Save(listRef reference.Reference, manifest reference.Reference, image types.ImageManifest) error { +func (s *fsStore) Save(listRef reference.Reference, manifest reference.Reference, images ...types.ImageManifest) error { if err := s.createManifestListDirectory(listRef.String()); err != nil { return err } + if len(images) == 0 { + return nil + } + filename := manifestToFilename(s.root, listRef.String(), manifest.String()) - bytes, err := json.Marshal(image) + bytes, err := json.Marshal(images[0]) if err != nil { return err } - return os.WriteFile(filename, bytes, 0o644) + if err := os.WriteFile(filename, bytes, 0o644); err != nil { + return err + } + + for i, image := range images[1:] { + bytes, err := json.Marshal(image) + if err != nil { + return err + } + if err := os.WriteFile(fmt.Sprintf("%s_%d", filename, i+2), bytes, 0o644); err != nil { + return err + } + } + + return nil } func (s *fsStore) createManifestListDirectory(transaction string) error { diff --git a/cli/manifest/store/store_test.go b/cli/manifest/store/store_test.go index 19fd30684d75..e68702680263 100644 --- a/cli/manifest/store/store_test.go +++ b/cli/manifest/store/store_test.go @@ -55,14 +55,14 @@ func TestStoreRemove(t *testing.T) { func TestStoreSaveAndGet(t *testing.T) { store := NewStore(t.TempDir()) listRef := ref("list") - data := types.ImageManifest{Ref: sref(t, "abcdef")} - err := store.Save(listRef, ref("exists"), data) + data := []types.ImageManifest{{Ref: sref(t, "abcdef")}} + err := store.Save(listRef, ref("exists"), data...) assert.NilError(t, err) testcases := []struct { listRef reference.Reference manifestRef reference.Reference - expected types.ImageManifest + expected []types.ImageManifest expectedErr error }{ {