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 <derek@mcgstyle.net>

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 + "."
 )

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
+		img.Target.Annotations = addNameAnnotation(name, img.Target.Annotations)
-		o.manifests, i = appendDescriptor(o.manifests, img.Target)
+		o.manifests = append(o.manifests, img.Target)
-		o.manifests[i].Annotations = addNameAnnotation(name, o.manifests[i].Annotations)
 
 		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 = append(o.manifests, manifest)
-		o.manifests, i = appendDescriptor(o.manifests, manifest)
-		o.manifests[i].Annotations = manifest.Annotations
 		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
+	annotations[images.AnnotationImageName] = name
-	for {
+	annotations[ocispec.AnnotationRefName] = ociReferenceName(name)
-		key := images.AnnotationImageName
-		if i > 0 {
-			key = fmt.Sprintf("%sextra.%d", images.AnnotationImageNamePrefix, i)
-		}
-		i++
 
-		if val, ok := annotations[key]; ok {
-			if val != name {
-				continue
-			}
-		} else {
-			annotations[key] = name
-		}
-		break
-	}
 	return annotations
 }
 
@@ -168,20 +137,33 @@ 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:
+			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...)
 
-			for _, name := range imageNames(desc.Annotations) {
+				mt = &exportManifest{
-				manifestTags[name] = desc
+					manifest: desc,
+				}
+				dManifests[desc.Digest] = mt
+			}
+
+			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))
 
 				p, err := content.ReadBlob(ctx, store, desc)
@@ -194,7 +176,6 @@ func Export(ctx context.Context, store content.Provider, writer io.Writer, opts
 					return err
 				}
 
-			names := imageNames(desc.Annotations)
 				var manifests []ocispec.Descriptor
 				for _, m := range index.Manifests {
 					if eo.platform != nil {
@@ -213,7 +194,7 @@ func Export(ctx context.Context, store content.Provider, writer io.Writer, opts
 					records = append(records, r...)
 				}
 
-			if len(names) > 0 && !eo.skipDockerManifest {
+				if !eo.skipDockerManifest {
 					if len(manifests) >= 1 {
 						if len(manifests) > 1 {
 							sort.SliceStable(manifests, func(i, j int) bool {
@@ -226,20 +207,30 @@ func Export(ctx context.Context, store content.Provider, writer io.Writer, opts
 								return eo.platform.Less(*manifests[i].Platform, *manifests[j].Platform)
 							})
 						}
-					for _, name := range names {
+						d = manifests[0].Digest
-						manifestTags[name] = manifests[0]
+						dManifests[d] = &exportManifest{
+							manifest: manifests[0],
 						}
 					} 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 {
+	if len(dManifests) > 0 {
-		tr, err := manifestsRecord(ctx, store, manifestTags)
+		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 exportManifest struct {
-	type mfst 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{}
+	var i int
-	for name, m := range manifests {
+	for _, m := range manifests {
-		p, err := content.ReadBlob(ctx, store, m)
+		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)
+			return tarRecord{}, errors.Wrapf(err, "invalid manifest %q", m.manifest.Digest)
 		}
 
+		dgst := manifest.Config.Digest
+		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)
+		}
+
+		for _, name := range m.names {
 			nname, err := familiarizeReference(name)
 			if err != nil {
 				return tarRecord{}, err
 			}
 
-		dgst := manifest.Config.Digest
+			mfsts[i].RepoTags = append(mfsts[i].RepoTags, nname)
-		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)
-			}
 		}
 
-		mf.RepoTags = append(mf.RepoTags, nname)
+		i++
-
-		images[dgst] = mf
-	}
-
-	var mfsts []mfst
-	for _, mf := range images {
-		mfsts = append(mfsts, mf)
 	}
 
 	b, err := json.Marshal(mfsts)

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)

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

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)
+			name := imageName(m.Annotations, iopts.imageRefT)
-			for _, name := range names {
+			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 {
+func imageName(annotations map[string]string, ociCleanup func(string) string) string {
-	var names []string
+	name := annotations[images.AnnotationImageName]
-	for k, v := range annotations {
+	if name != "" {
-		if k == ocispec.AnnotationRefName {
+		return name
+	}
+	name = annotations[ocispec.AnnotationRefName]
+	if name != "" {
 		if ociCleanup != nil {
-				v = ociCleanup(v)
+			name = ociCleanup(name)
-			}
-			if v != "" {
-				names = appendSorted(names, v)
-			}
-		} else if k == images.AnnotationImageName || strings.HasPrefix(k, images.AnnotationImageNamePrefix) {
-			names = appendSorted(names, v)
-
 		}
 	}
-	return names
+	return name
-}
-
-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
-		}
-	}
-	return append(arr, s)
 }

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),