Refactor image importer

Allow customization of reference creation.
Add option for digest references.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>

cmd/ctr/commands/images/import.go

@@ -20,9 +20,10 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"time"
 
+	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/cmd/ctr/commands"
-	"github.com/containerd/containerd/images"
 	oci "github.com/containerd/containerd/images/oci"
 	"github.com/containerd/containerd/log"
 	"github.com/urfave/cli"
@@ -53,23 +54,34 @@ If foobar.tar contains an OCI ref named "latest" and anonymous ref "sha256:deadb
 			Usage: "image format. See DESCRIPTION.",
 		},
 		cli.StringFlag{
-			Name:  "oci-name",
+			Name:  "prefix,oci-name",
-			Value: "unknown/unknown",
+			Value: "",
-			Usage: "prefix added to either oci.v1 ref annotation or digest",
+			Usage: "prefix image name for added images",
+		},
+		cli.BoolFlag{
+			Name:  "digests",
+			Usage: "whether to create digest images",
 		},
-		// TODO(AkihiroSuda): support commands.LabelFlag (for all children objects)
 	}, commands.SnapshotterFlags...),
 
 	Action: func(context *cli.Context) error {
 		var (
 			in   = context.Args().First()
-			imageImporter images.Importer
+			opts []containerd.ImportOpt
 		)
 
 		switch format := context.String("format"); format {
 		case "oci.v1":
-			imageImporter = &oci.V1Importer{
+			opts = append(opts, containerd.WithImporter(&oci.V1Importer{}))
-				ImageName: context.String("oci-name"),
+
+			prefix := context.String("prefix")
+			if prefix == "" {
+				prefix = fmt.Sprintf("import-%s", time.Now().Format("2006-01-02"))
+			}
+
+			opts = append(opts, containerd.WithImageRefTranslator(oci.RefTranslator(prefix)))
+			if context.Bool("digests") {
+				opts = append(opts, containerd.WithDigestRef(oci.DigestTranslator(prefix)))
 			}
 		default:
 			return fmt.Errorf("unknown format %s", format)
@@ -90,20 +102,24 @@ If foobar.tar contains an OCI ref named "latest" and anonymous ref "sha256:deadb
 				return err
 			}
 		}
-		imgs, err := client.Import(ctx, imageImporter, r)
+		imgs, err := client.Import(ctx, r, opts...)
+		closeErr := r.Close()
 		if err != nil {
 			return err
 		}
-		if err = r.Close(); err != nil {
+		if closeErr != nil {
-			return err
+			return closeErr
 		}
 
 		log.G(ctx).Debugf("unpacking %d images", len(imgs))
 
 		for _, img := range imgs {
+			// TODO: Allow configuration of the platform
+			image := containerd.NewImage(client, img)
+
 			// TODO: Show unpack status
-			fmt.Printf("unpacking %s (%s)...", img.Name(), img.Target().Digest)
+			fmt.Printf("unpacking %s (%s)...", img.Name, img.Target.Digest)
-			err = img.Unpack(ctx, context.String("snapshotter"))
+			err = image.Unpack(ctx, context.String("snapshotter"))
 			if err != nil {
 				return err
 			}

images/importexport.go

@@ -27,7 +27,7 @@ import (
 // Importer is the interface for image importer.
 type Importer interface {
 	// Import imports an image from a tar stream.
-	Import(ctx context.Context, store content.Store, reader io.Reader) ([]Image, error)
+	Import(ctx context.Context, store content.Store, reader io.Reader) (ocispec.Descriptor, error)
 }
 
 // Exporter is the interface for image exporter.

images/oci/importer.go

@@ -19,114 +19,89 @@ package oci
 
 import (
 	"archive/tar"
+	"bytes"
 	"context"
-	"encoding/json"
-	"fmt"
 	"io"
 	"io/ioutil"
 	"path"
 	"strings"
 
 	"github.com/containerd/containerd/content"
-	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
+	"github.com/containerd/containerd/log"
 	digest "github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 )
 
 // V1Importer implements OCI Image Spec v1.
-type V1Importer struct {
+type V1Importer struct{}
-	// ImageName is preprended to either `:` + OCI ref name or `@` + digest (for anonymous refs).
-	// This field is mandatory atm, but may change in the future. maybe ref map[string]string as in moby/moby#33355
-	ImageName string
-}
 
 var _ images.Importer = &V1Importer{}
 
 // Import implements Importer.
-func (oi *V1Importer) Import(ctx context.Context, store content.Store, reader io.Reader) ([]images.Image, error) {
+func (oi *V1Importer) Import(ctx context.Context, store content.Store, reader io.Reader) (ocispec.Descriptor, error) {
-	if oi.ImageName == "" {
+	var (
-		return nil, errors.New("ImageName not set")
+		desc ocispec.Descriptor
-	}
+		tr   = tar.NewReader(reader)
-	tr := tar.NewReader(reader)
+	)
-	var imgrecs []images.Image
-	foundIndexJSON := false
 	for {
 		hdr, err := tr.Next()
 		if err == io.EOF {
 			break
 		}
 		if err != nil {
-			return nil, err
+			return ocispec.Descriptor{}, err
 		}
 		if hdr.Typeflag != tar.TypeReg && hdr.Typeflag != tar.TypeRegA {
+			log.G(ctx).WithField("file", hdr.Name).Debug("file type ignored")
 			continue
 		}
 		hdrName := path.Clean(hdr.Name)
 		if hdrName == "index.json" {
-			if foundIndexJSON {
+			if desc.Digest != "" {
-				return nil, errors.New("duplicated index.json")
+				return ocispec.Descriptor{}, errors.New("duplicated index.json")
 			}
-			foundIndexJSON = true
+			desc, err = onUntarIndexJSON(ctx, tr, store, hdr.Size)
-			imgrecs, err = onUntarIndexJSON(tr, oi.ImageName)
 			if err != nil {
-				return nil, err
+				return ocispec.Descriptor{}, err
 			}
-			continue
+		} else if strings.HasPrefix(hdrName, "blobs/") {
-		}
-		if strings.HasPrefix(hdrName, "blobs/") {
 			if err := onUntarBlob(ctx, tr, store, hdrName, hdr.Size); err != nil {
-				return nil, err
+				return ocispec.Descriptor{}, err
+			}
+		} else if hdrName == ocispec.ImageLayoutFile {
+			// TODO Validate
+		} else {
+			log.G(ctx).WithField("file", hdr.Name).Debug("unknown file ignored")
 		}
 	}
-	}
+	if desc.Digest == "" {
-	if !foundIndexJSON {
+		return ocispec.Descriptor{}, errors.New("no index.json found")
-		return nil, errors.New("no index.json found")
-	}
-	for _, img := range imgrecs {
-		err := setGCRefContentLabels(ctx, store, img.Target)
-		if err != nil {
-			return imgrecs, err
-		}
-	}
-	// FIXME(AkihiroSuda): set GC labels for unreferrenced blobs (i.e. with unknown media types)?
-	return imgrecs, nil
 	}
 
-func onUntarIndexJSON(r io.Reader, imageName string) ([]images.Image, error) {
+	return desc, nil
+}
+
+func onUntarIndexJSON(ctx context.Context, r io.Reader, store content.Ingester, size int64) (ocispec.Descriptor, error) {
 	b, err := ioutil.ReadAll(r)
 	if err != nil {
-		return nil, err
+		return ocispec.Descriptor{}, err
 	}
-	var idx ocispec.Index
+	desc := ocispec.Descriptor{
-	if err := json.Unmarshal(b, &idx); err != nil {
+		MediaType: ocispec.MediaTypeImageIndex,
-		return nil, err
+		Digest:    digest.FromBytes(b),
+		Size:      size,
 	}
-	var imgrecs []images.Image
+	if int64(len(b)) != size {
-	for _, m := range idx.Manifests {
+		return ocispec.Descriptor{}, errors.Errorf("size mismatch %d v %d", len(b), size)
-		ref, err := normalizeImageRef(imageName, m)
-		if err != nil {
-			return nil, err
-		}
-		imgrecs = append(imgrecs, images.Image{
-			Name:   ref,
-			Target: m,
-		})
-	}
-	return imgrecs, nil
 	}
 
-func normalizeImageRef(imageName string, manifest ocispec.Descriptor) (string, error) {
+	if err := content.WriteBlob(ctx, store, "index-"+desc.Digest.String(), bytes.NewReader(b), desc); err != nil {
-	digest := manifest.Digest
+		return ocispec.Descriptor{}, err
-	if digest == "" {
-		return "", errors.Errorf("manifest with empty digest: %v", manifest)
 	}
-	ociRef := manifest.Annotations[ocispec.AnnotationRefName]
+
-	if ociRef == "" {
+	return desc, err
-		return imageName + "@" + digest.String(), nil
-	}
-	return imageName + ":" + ociRef, nil
 }
 
 func onUntarBlob(ctx context.Context, r io.Reader, store content.Ingester, name string, size int64) error {
@@ -140,65 +115,22 @@ func onUntarBlob(ctx context.Context, r io.Reader, store content.Ingester, name
 		return errors.Errorf("unsupported algorithm: %s", algo)
 	}
 	dgst := digest.NewDigestFromHex(algo.String(), split[2])
-	return content.WriteBlob(ctx, store, "unknown-"+dgst.String(), r, ocispec.Descriptor{Size: size, Digest: dgst})
+	return content.WriteBlob(ctx, store, "blob-"+dgst.String(), r, ocispec.Descriptor{Size: size, Digest: dgst})
 }
 
-// GetChildrenDescriptors returns children blob descriptors for the following supported types:
+// RefTranslator creates a reference using an OCI ref annotation,
-// - images.MediaTypeDockerSchema2Manifest, ocispec.MediaTypeImageManifest
+// which is mentioned in the spec as only a tag compontent,
-// - images.MediaTypeDockerSchema2ManifestList, ocispec.MediaTypeImageIndex
+// concatenated with an image name
-func GetChildrenDescriptors(r io.Reader, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
+func RefTranslator(prefix string) func(string) string {
-	switch desc.MediaType {
+	return func(ref string) string {
-	case images.MediaTypeDockerSchema2Manifest, ocispec.MediaTypeImageManifest:
+		return prefix + ":" + ref
-		var manifest ocispec.Manifest
-		if err := json.NewDecoder(r).Decode(&manifest); err != nil {
-			return nil, err
 	}
-		return append([]ocispec.Descriptor{manifest.Config}, manifest.Layers...), nil
-	case images.MediaTypeDockerSchema2ManifestList, ocispec.MediaTypeImageIndex:
-		var index ocispec.Index
-		if err := json.NewDecoder(r).Decode(&index); err != nil {
-			return nil, err
-		}
-		return index.Manifests, nil
-	}
-	return nil, nil
 }
 
-func setGCRefContentLabels(ctx context.Context, store content.Store, desc ocispec.Descriptor) error {
+// DigestTranslator creates a digest reference by adding the
-	info, err := store.Info(ctx, desc.Digest)
+// digest to an image name
-	if err != nil {
+func DigestTranslator(prefix string) func(digest.Digest) string {
-		if errdefs.IsNotFound(err) {
+	return func(dgst digest.Digest) string {
-			// when the archive is created from multi-arch image,
+		return prefix + "@" + dgst.String()
-			// it may contain only blobs for a certain platform.
-			// So ErrNotFound (on manifest list) is expected here.
-			return nil
-		}
-		return err
-	}
-	ra, err := store.ReaderAt(ctx, desc)
-	if err != nil {
-		return err
-	}
-	defer ra.Close()
-	r := content.NewReader(ra)
-	children, err := GetChildrenDescriptors(r, desc)
-	if err != nil {
-		return err
-	}
-	if info.Labels == nil {
-		info.Labels = map[string]string{}
-	}
-	for i, child := range children {
-		// Note: child blob is not guaranteed to be written to the content store. (multi-arch)
-		info.Labels[fmt.Sprintf("containerd.io/gc.ref.content.%d", i)] = child.Digest.String()
-	}
-	if _, err := store.Update(ctx, info, "labels"); err != nil {
-		return err
-	}
-	for _, child := range children {
-		if err := setGCRefContentLabels(ctx, store, child); err != nil {
-			return err
 	}
 }
-	return nil
-}

images/oci/importer_test.go

@@ -1,53 +0,0 @@
-/*
-   Copyright The containerd Authors.
-
-   Licensed under the Apache License, Version 2.0 (the "License");
-   you may not use this file except in compliance with the License.
-   You may obtain a copy of the License at
-
-       http://www.apache.org/licenses/LICENSE-2.0
-
-   Unless required by applicable law or agreed to in writing, software
-   distributed under the License is distributed on an "AS IS" BASIS,
-   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-   See the License for the specific language governing permissions and
-   limitations under the License.
-*/
-
-package oci
-
-import (
-	"testing"
-
-	"github.com/opencontainers/go-digest"
-	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
-	"gotest.tools/assert"
-)
-
-func TestNormalizeImageRef(t *testing.T) {
-	imageBaseName := "foo/bar"
-	for _, test := range []struct {
-		input  ocispec.Descriptor
-		expect string
-	}{
-		{
-			input: ocispec.Descriptor{
-				Digest: digest.Digest("sha256:e22e93af8657d43d7f204b93d69604aeacf273f71d2586288cde312808c0ec77"),
-			},
-			expect: "foo/bar@sha256:e22e93af8657d43d7f204b93d69604aeacf273f71d2586288cde312808c0ec77",
-		},
-		{
-			input: ocispec.Descriptor{
-				Digest: digest.Digest("sha256:e22e93af8657d43d7f204b93d69604aeacf273f71d2586288cde312808c0ec77"),
-				Annotations: map[string]string{
-					ocispec.AnnotationRefName: "latest",
-				},
-			},
-			expect: "foo/bar:latest", // no @digest for simplicity
-		},
-	} {
-		normalized, err := normalizeImageRef(imageBaseName, test.input)
-		assert.NilError(t, err)
-		assert.Equal(t, test.expect, normalized)
-	}
-}

import.go

@@ -18,36 +18,76 @@ package containerd
 
 import (
 	"context"
+	"encoding/json"
 	"io"
 
+	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
+	"github.com/containerd/containerd/images/oci"
+	digest "github.com/opencontainers/go-digest"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 type importOpts struct {
+	indexName string
+	imageRefT func(string) string
+	dgstRefT  func(digest.Digest) string
+	importer  images.Importer
 }
 
 // ImportOpt allows the caller to specify import specific options
-type ImportOpt func(c *importOpts) error
+type ImportOpt func(*importOpts) error
 
-func resolveImportOpt(opts ...ImportOpt) (importOpts, error) {
+// WithImageRefTranslator is used to translate the index reference
-	var iopts importOpts
+// to an image reference for the image store.
-	for _, o := range opts {
+func WithImageRefTranslator(f func(string) string) ImportOpt {
-		if err := o(&iopts); err != nil {
+	return func(c *importOpts) error {
-			return iopts, err
+		c.imageRefT = f
+		return nil
 	}
 }
-	return iopts, nil
+
+// WithDigestRef is used to create digest images for each
+// manifest in the index.
+func WithDigestRef(f func(digest.Digest) string) ImportOpt {
+	return func(c *importOpts) error {
+		c.dgstRefT = f
+		return nil
+	}
+}
+
+// WithIndexName creates a tag pointing to the imported index
+func WithIndexName(name string) ImportOpt {
+	return func(c *importOpts) error {
+		c.indexName = name
+		return nil
+	}
+}
+
+// WithImporter sets the importer to use for converting
+// the read stream into an OCI Index.
+func WithImporter(importer images.Importer) ImportOpt {
+	return func(c *importOpts) error {
+		c.importer = importer
+		return nil
+	}
 }
 
 // Import imports an image from a Tar stream using reader.
 // Caller needs to specify importer. Future version may use oci.v1 as the default.
 // Note that unreferrenced blobs may be imported to the content store as well.
-func (c *Client) Import(ctx context.Context, importer images.Importer, reader io.Reader, opts ...ImportOpt) ([]Image, error) {
+func (c *Client) Import(ctx context.Context, reader io.Reader, opts ...ImportOpt) ([]images.Image, error) {
-	_, err := resolveImportOpt(opts...) // unused now
+	var iopts importOpts
-	if err != nil {
+	for _, o := range opts {
+		if err := o(&iopts); err != nil {
 			return nil, err
 		}
+	}
+
+	if iopts.importer == nil {
+		iopts.importer = &oci.V1Importer{}
+	}
 
 	ctx, done, err := c.WithLease(ctx)
 	if err != nil {
@@ -55,31 +95,86 @@ func (c *Client) Import(ctx context.Context, importer images.Importer, reader io
 	}
 	defer done(ctx)
 
-	imgrecs, err := importer.Import(ctx, c.ContentStore(), reader)
+	index, err := iopts.importer.Import(ctx, c.ContentStore(), reader)
 	if err != nil {
-		// is.Update() is not called on error
 		return nil, err
 	}
 
-	is := c.ImageService()
+	var (
-	var images []Image
+		imgs []images.Image
-	for _, imgrec := range imgrecs {
+		cs   = c.ContentStore()
-		if updated, err := is.Update(ctx, imgrec, "target"); err != nil {
+		is   = c.ImageService()
+	)
+
+	if iopts.indexName != "" {
+		imgs = append(imgs, images.Image{
+			Name:   iopts.indexName,
+			Target: index,
+		})
+	}
+
+	var handler images.HandlerFunc
+	handler = func(ctx context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
+		// Only save images at top level
+		if desc.Digest != index.Digest {
+			return images.Children(ctx, cs, desc)
+		}
+
+		p, err := content.ReadBlob(ctx, cs, desc)
+		if err != nil {
+			return nil, err
+		}
+
+		var idx ocispec.Index
+		if err := json.Unmarshal(p, &idx); err != nil {
+			return nil, err
+		}
+
+		for _, m := range idx.Manifests {
+			if ref := m.Annotations[ocispec.AnnotationRefName]; ref != "" {
+				if iopts.imageRefT != nil {
+					ref = iopts.imageRefT(ref)
+				}
+				if ref != "" {
+					imgs = append(imgs, images.Image{
+						Name:   ref,
+						Target: m,
+					})
+				}
+			}
+			if iopts.dgstRefT != nil {
+				ref := iopts.dgstRefT(m.Digest)
+				if ref != "" {
+					imgs = append(imgs, images.Image{
+						Name:   ref,
+						Target: m,
+					})
+				}
+			}
+		}
+
+		return idx.Manifests, nil
+	}
+
+	handler = images.SetChildrenLabels(cs, handler)
+	if err := images.Walk(ctx, handler, index); err != nil {
+		return nil, err
+	}
+
+	for i := range imgs {
+		img, err := is.Update(ctx, imgs[i], "target")
+		if err != nil {
 			if !errdefs.IsNotFound(err) {
 				return nil, err
 			}
 
-			created, err := is.Create(ctx, imgrec)
+			img, err = is.Create(ctx, imgs[i])
 			if err != nil {
 				return nil, err
 			}
-
+		}
-			imgrec = created
+		imgs[i] = img
-		} else {
-			imgrec = updated
 	}
 
-		images = append(images, NewImage(c, imgrec))
+	return imgs, nil
-	}
-	return images, nil
 }

import_test.go

@@ -49,13 +49,17 @@ func TestOCIExportAndImport(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	imgrecs, err := client.Import(ctx, &oci.V1Importer{ImageName: "foo/bar:"}, exported)
+	opts := []ImportOpt{
+		WithImporter(&oci.V1Importer{}),
+		WithImageRefTranslator(oci.RefTranslator("foo/bar")),
+	}
+	imgrecs, err := client.Import(ctx, exported, opts...)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	for _, imgrec := range imgrecs {
-		err = client.ImageService().Delete(ctx, imgrec.Name())
+		err = client.ImageService().Delete(ctx, imgrec.Name)
 		if err != nil {
 			t.Fatal(err)
 		}