From 5e2d7efd82bd7f2cd7c66b4d0af54e7c060d1f87 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 21 May 2019 11:00:31 -0700 Subject: [PATCH] Use a single custom annotation for export Remove annotation prefix and add multiple index records for manifests with multiple image names. This makes the custom annotation more consistent with the OCI image annotation. Additionally, ensure the OCI image annotation always represents the tag (partial image name) as recommended by the specification. The containerd image name annotation will always contain the full image name. Signed-off-by: Derek McGowan --- images/annotations.go | 5 - images/archive/exporter.go | 224 ++++++++++++++++-------------------- images/archive/importer.go | 3 +- images/archive/reference.go | 26 ++++- import.go | 41 ++----- import_test.go | 2 +- 6 files changed, 137 insertions(+), 164 deletions(-) diff --git a/images/annotations.go b/images/annotations.go index bf9df7984d5e..47d92104cddc 100644 --- a/images/annotations.go +++ b/images/annotations.go @@ -20,9 +20,4 @@ const ( // AnnotationImageName is an annotation on a Descriptor in an index.json // containing the `Name` value as used by an `Image` struct AnnotationImageName = "io.containerd.image.name" - - // AnnotationImageNamePrefix is used the same way as AnnotationImageName - // but may be used to refer to additional names in the annotation map - // using user-defined suffixes (i.e. "extra.1") - AnnotationImageNamePrefix = AnnotationImageName + "." ) diff --git a/images/archive/exporter.go b/images/archive/exporter.go index b7a9d495f354..5440c0d1b91d 100644 --- a/images/archive/exporter.go +++ b/images/archive/exporter.go @@ -20,11 +20,9 @@ import ( "archive/tar" "context" "encoding/json" - "fmt" "io" "path" "sort" - "strings" "github.com/containerd/containerd/content" "github.com/containerd/containerd/errdefs" @@ -83,9 +81,8 @@ func WithImage(is images.Store, name string) ExportOpt { return err } - var i int - o.manifests, i = appendDescriptor(o.manifests, img.Target) - o.manifests[i].Annotations = addNameAnnotation(name, o.manifests[i].Annotations) + img.Target.Annotations = addNameAnnotation(name, img.Target.Annotations) + o.manifests = append(o.manifests, img.Target) return nil } @@ -96,9 +93,7 @@ func WithImage(is images.Store, name string) ExportOpt { // descriptor if needed. func WithManifest(manifest ocispec.Descriptor) ExportOpt { return func(ctx context.Context, o *exportOptions) error { - var i int - o.manifests, i = appendDescriptor(o.manifests, manifest) - o.manifests[i].Annotations = manifest.Annotations + o.manifests = append(o.manifests, manifest) return nil } } @@ -107,49 +102,23 @@ func WithManifest(manifest ocispec.Descriptor) ExportOpt { // with the provided names. func WithNamedManifest(manifest ocispec.Descriptor, names ...string) ExportOpt { return func(ctx context.Context, o *exportOptions) error { - var i int - o.manifests, i = appendDescriptor(o.manifests, manifest) for _, name := range names { - o.manifests[i].Annotations = addNameAnnotation(name, o.manifests[i].Annotations) + manifest.Annotations = addNameAnnotation(name, manifest.Annotations) + o.manifests = append(o.manifests, manifest) } return nil } } -func appendDescriptor(descs []ocispec.Descriptor, desc ocispec.Descriptor) ([]ocispec.Descriptor, int) { - i := 0 - for i < len(descs) { - if descs[i].Digest == desc.Digest { - return descs, i - } - i++ - } - return append(descs, desc), i -} - func addNameAnnotation(name string, annotations map[string]string) map[string]string { if annotations == nil { annotations = map[string]string{} } - i := 0 - for { - key := images.AnnotationImageName - if i > 0 { - key = fmt.Sprintf("%sextra.%d", images.AnnotationImageNamePrefix, i) - } - i++ + annotations[images.AnnotationImageName] = name + annotations[ocispec.AnnotationRefName] = ociReferenceName(name) - if val, ok := annotations[key]; ok { - if val != name { - continue - } - } else { - annotations[key] = name - } - break - } return annotations } @@ -168,78 +137,100 @@ func Export(ctx context.Context, store content.Provider, writer io.Writer, opts } algorithms := map[string]struct{}{} - manifestTags := map[string]ocispec.Descriptor{} + dManifests := map[digest.Digest]*exportManifest{} + resolvedIndex := map[digest.Digest]digest.Digest{} for _, desc := range eo.manifests { switch desc.MediaType { case images.MediaTypeDockerSchema2Manifest, ocispec.MediaTypeImageManifest: - r, err := getRecords(ctx, store, desc, algorithms) - if err != nil { - return err - } - records = append(records, r...) - - for _, name := range imageNames(desc.Annotations) { - manifestTags[name] = desc - } - case images.MediaTypeDockerSchema2ManifestList, ocispec.MediaTypeImageIndex: - records = append(records, blobRecord(store, desc)) + mt, ok := dManifests[desc.Digest] + if !ok { + // TODO(containerd): Skip if already added + r, err := getRecords(ctx, store, desc, algorithms) + if err != nil { + return err + } + records = append(records, r...) - p, err := content.ReadBlob(ctx, store, desc) - if err != nil { - return err + mt = &exportManifest{ + manifest: desc, + } + dManifests[desc.Digest] = mt } - var index ocispec.Index - if err := json.Unmarshal(p, &index); err != nil { - return err + name := desc.Annotations[images.AnnotationImageName] + if name != "" && !eo.skipDockerManifest { + mt.names = append(mt.names, name) } + case images.MediaTypeDockerSchema2ManifestList, ocispec.MediaTypeImageIndex: + d, ok := resolvedIndex[desc.Digest] + if !ok { + records = append(records, blobRecord(store, desc)) - names := imageNames(desc.Annotations) - var manifests []ocispec.Descriptor - for _, m := range index.Manifests { - if eo.platform != nil { - if m.Platform == nil || eo.platform.Match(*m.Platform) { - manifests = append(manifests, m) - } else if !eo.allPlatforms { - continue - } + p, err := content.ReadBlob(ctx, store, desc) + if err != nil { + return err } - r, err := getRecords(ctx, store, m, algorithms) - if err != nil { + var index ocispec.Index + if err := json.Unmarshal(p, &index); err != nil { return err } - records = append(records, r...) - } + var manifests []ocispec.Descriptor + for _, m := range index.Manifests { + if eo.platform != nil { + if m.Platform == nil || eo.platform.Match(*m.Platform) { + manifests = append(manifests, m) + } else if !eo.allPlatforms { + continue + } + } - if len(names) > 0 && !eo.skipDockerManifest { - if len(manifests) >= 1 { - if len(manifests) > 1 { - sort.SliceStable(manifests, func(i, j int) bool { - if manifests[i].Platform == nil { - return false - } - if manifests[j].Platform == nil { - return true - } - return eo.platform.Less(*manifests[i].Platform, *manifests[j].Platform) - }) + r, err := getRecords(ctx, store, m, algorithms) + if err != nil { + return err } - for _, name := range names { - manifestTags[name] = manifests[0] + + records = append(records, r...) + } + + if !eo.skipDockerManifest { + if len(manifests) >= 1 { + if len(manifests) > 1 { + sort.SliceStable(manifests, func(i, j int) bool { + if manifests[i].Platform == nil { + return false + } + if manifests[j].Platform == nil { + return true + } + return eo.platform.Less(*manifests[i].Platform, *manifests[j].Platform) + }) + } + d = manifests[0].Digest + dManifests[d] = &exportManifest{ + manifest: manifests[0], + } + } else if eo.platform != nil { + return errors.Wrap(errdefs.ErrNotFound, "no manifest found for platform") } - } else if eo.platform != nil { - return errors.Wrap(errdefs.ErrNotFound, "no manifest found for platform") } + resolvedIndex[desc.Digest] = d + } + if d != "" { + if name := desc.Annotations[images.AnnotationImageName]; name != "" { + mt := dManifests[d] + mt.names = append(mt.names, name) + } + } default: return errors.Wrap(errdefs.ErrInvalidArgument, "only manifests may be exported") } } - if len(manifestTags) > 0 { - tr, err := manifestsRecord(ctx, store, manifestTags) + if len(dManifests) > 0 { + tr, err := manifestsRecord(ctx, store, dManifests) if err != nil { return errors.Wrap(err, "unable to create manifests file") } @@ -259,16 +250,6 @@ func Export(ctx context.Context, store content.Provider, writer io.Writer, opts return writeTar(ctx, tw, records) } -func imageNames(annotations map[string]string) []string { - var names []string - for k, v := range annotations { - if k == images.AnnotationImageName || strings.HasPrefix(k, images.AnnotationImageName) { - names = append(names, v) - } - } - return names -} - func getRecords(ctx context.Context, store content.Provider, desc ocispec.Descriptor, algorithms map[string]struct{}) ([]tarRecord, error) { var records []tarRecord exportHandler := func(ctx context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) { @@ -394,16 +375,21 @@ func ociIndexRecord(manifests []ocispec.Descriptor) tarRecord { } } -func manifestsRecord(ctx context.Context, store content.Provider, manifests map[string]ocispec.Descriptor) (tarRecord, error) { - type mfst struct { +type exportManifest struct { + manifest ocispec.Descriptor + names []string +} + +func manifestsRecord(ctx context.Context, store content.Provider, manifests map[digest.Digest]*exportManifest) (tarRecord, error) { + mfsts := make([]struct { Config string RepoTags []string Layers []string - } + }, len(manifests)) - images := map[digest.Digest]mfst{} - for name, m := range manifests { - p, err := content.ReadBlob(ctx, store, m) + var i int + for _, m := range manifests { + p, err := content.ReadBlob(ctx, store, m.manifest) if err != nil { return tarRecord{}, err } @@ -413,32 +399,26 @@ func manifestsRecord(ctx context.Context, store content.Provider, manifests map[ return tarRecord{}, err } if err := manifest.Config.Digest.Validate(); err != nil { - return tarRecord{}, errors.Wrapf(err, "invalid manifest %q", m.Digest) - } - - nname, err := familiarizeReference(name) - if err != nil { - return tarRecord{}, err + return tarRecord{}, errors.Wrapf(err, "invalid manifest %q", m.manifest.Digest) } dgst := manifest.Config.Digest - mf, ok := images[dgst] - if !ok { - mf.Config = path.Join("blobs", dgst.Algorithm().String(), dgst.Encoded()) - for _, l := range manifest.Layers { - path := path.Join("blobs", l.Digest.Algorithm().String(), l.Digest.Encoded()) - mf.Layers = append(mf.Layers, path) - } + mfsts[i].Config = path.Join("blobs", dgst.Algorithm().String(), dgst.Encoded()) + for _, l := range manifest.Layers { + path := path.Join("blobs", l.Digest.Algorithm().String(), l.Digest.Encoded()) + mfsts[i].Layers = append(mfsts[i].Layers, path) } - mf.RepoTags = append(mf.RepoTags, nname) + for _, name := range m.names { + nname, err := familiarizeReference(name) + if err != nil { + return tarRecord{}, err + } - images[dgst] = mf - } + mfsts[i].RepoTags = append(mfsts[i].RepoTags, nname) + } - var mfsts []mfst - for _, mf := range images { - mfsts = append(mfsts, mf) + i++ } b, err := json.Marshal(mfsts) diff --git a/images/archive/importer.go b/images/archive/importer.go index 692c76b1ff01..0e64159112f5 100644 --- a/images/archive/importer.go +++ b/images/archive/importer.go @@ -181,7 +181,8 @@ func ImportIndex(ctx context.Context, store content.Store, reader io.Reader) (oc } mfstdesc.Annotations = map[string]string{ - ocispec.AnnotationRefName: normalized, + images.AnnotationImageName: normalized, + ocispec.AnnotationRefName: ociReferenceName(normalized), } idx.Manifests = append(idx.Manifests, mfstdesc) diff --git a/images/archive/reference.go b/images/archive/reference.go index 5ac3e11b00ff..19f419eaa2af 100644 --- a/images/archive/reference.go +++ b/images/archive/reference.go @@ -19,7 +19,8 @@ package archive import ( "strings" - "github.com/docker/distribution/reference" + "github.com/containerd/containerd/reference" + distref "github.com/docker/distribution/reference" "github.com/opencontainers/go-digest" "github.com/pkg/errors" ) @@ -69,7 +70,7 @@ func isImagePrefix(s, prefix string) bool { func normalizeReference(ref string) (string, error) { // TODO: Replace this function to not depend on reference package - normalized, err := reference.ParseDockerRef(ref) + normalized, err := distref.ParseDockerRef(ref) if err != nil { return "", errors.Wrapf(err, "normalize image ref %q", ref) } @@ -78,13 +79,28 @@ func normalizeReference(ref string) (string, error) { } func familiarizeReference(ref string) (string, error) { - named, err := reference.ParseNormalizedNamed(ref) + named, err := distref.ParseNormalizedNamed(ref) if err != nil { return "", errors.Wrapf(err, "failed to parse %q", ref) } - named = reference.TagNameOnly(named) + named = distref.TagNameOnly(named) - return reference.FamiliarString(named), nil + return distref.FamiliarString(named), nil +} + +func ociReferenceName(name string) string { + // OCI defines the reference name as only a tag excluding the + // repository. The containerd annotation contains the full image name + // since the tag is insufficent for correctly naming and referring to an + // image + var ociRef string + if spec, err := reference.Parse(name); err == nil { + ociRef = spec.Object + } else { + ociRef = name + } + + return ociRef } // DigestTranslator creates a digest reference by adding the diff --git a/import.go b/import.go index 87c87f6d5738..0e1531ab4dab 100644 --- a/import.go +++ b/import.go @@ -20,7 +20,6 @@ import ( "context" "encoding/json" "io" - "strings" "github.com/containerd/containerd/content" "github.com/containerd/containerd/errdefs" @@ -131,8 +130,8 @@ func (c *Client) Import(ctx context.Context, reader io.Reader, opts ...ImportOpt } for _, m := range idx.Manifests { - names := imageNames(m.Annotations, iopts.imageRefT) - for _, name := range names { + name := imageName(m.Annotations, iopts.imageRefT) + if name != "" { imgs = append(imgs, images.Image{ Name: name, Target: m, @@ -176,34 +175,16 @@ func (c *Client) Import(ctx context.Context, reader io.Reader, opts ...ImportOpt return imgs, nil } -func imageNames(annotations map[string]string, ociCleanup func(string) string) []string { - var names []string - for k, v := range annotations { - if k == ocispec.AnnotationRefName { - if ociCleanup != nil { - v = ociCleanup(v) - } - if v != "" { - names = appendSorted(names, v) - } - } else if k == images.AnnotationImageName || strings.HasPrefix(k, images.AnnotationImageNamePrefix) { - names = appendSorted(names, v) - - } +func imageName(annotations map[string]string, ociCleanup func(string) string) string { + name := annotations[images.AnnotationImageName] + if name != "" { + return name } - return names -} - -func appendSorted(arr []string, s string) []string { - for i, c := range arr { - if s < c { - arr = append(arr, "") - copy(arr[i+1:], arr[i:]) - arr[i] = s - return arr - } else if s == c { - return arr + name = annotations[ocispec.AnnotationRefName] + if name != "" { + if ociCleanup != nil { + name = ociCleanup(name) } } - return append(arr, s) + return name } diff --git a/import_test.go b/import_test.go index b435f79a911b..621dcde4a350 100644 --- a/import_test.go +++ b/import_test.go @@ -224,7 +224,7 @@ func TestImport(t *testing.T) { }, }, { - Name: "OCIPrefixName", + Name: "OCIPrefixName2", Writer: tartest.TarAll( tc.Dir("blobs", 0755), tc.Dir("blobs/sha256", 0755),