From b13aa3d24877c4b892a6e8aded6f6dbc6724dea7 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 30 Jan 2023 12:02:58 +0000 Subject: [PATCH] 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 }{ {