Unify docker and oci importer

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

cmd/ctr/commands/images/import.go

@@ -20,13 +20,11 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/cmd/ctr/commands"
-	"github.com/containerd/containerd/images/docker"
+	"github.com/containerd/containerd/images/archive"
-	oci "github.com/containerd/containerd/images/oci"
 	"github.com/containerd/containerd/log"
 	"github.com/urfave/cli"
 )
@@ -42,30 +40,25 @@ Implemented formats:
 - docker.v1.2
 
 
-For OCI v1, you may need to specify --base-name because an OCI archive
+For OCI v1, you may need to specify --base-name because an OCI archive may
-contains only partial image references (tags without the base image name).
+contain only partial image references (tags without the base image name).
-If no base image name is provided, a name will be generated as "import-%{date}".
+If no base image name is provided, a name will be generated as "import-%{yyyy-MM-dd}".
 
 e.g.
-  $ ctr images import --format oci.v1 --oci-name foo/bar foobar.tar
+  $ ctr images import --base-name foo/bar foobar.tar
 
 If foobar.tar contains an OCI ref named "latest" and anonymous ref "sha256:deadbeef", the command will create
 "foo/bar:latest" and "foo/bar@sha256:deadbeef" images in the containerd store.
 `,
 	Flags: append([]cli.Flag{
 		cli.StringFlag{
-			Name:  "format",
+			Name:  "base-name",
 			Value: "",
-			Usage: "image format, by default supports OCI v1, Docker v1.1, Docker v1.2",
+			Usage: "base image name for added images, when provided only images with this name prefix are imported",
-		},
-		cli.StringFlag{
-			Name:  "base-name,oci-name",
-			Value: "",
-			Usage: "base image name for added images, when provided images without this name prefix are filtered out",
 		},
 		cli.BoolFlag{
 			Name:  "digests",
-			Usage: "whether to create digest images",
+			Usage: "whether to create digest images (default: false)",
 		},
 		cli.StringFlag{
 			Name:  "index-name",
@@ -82,25 +75,14 @@ If foobar.tar contains an OCI ref named "latest" and anonymous ref "sha256:deadb
 		prefix := context.String("base-name")
 		if prefix == "" {
 			prefix = fmt.Sprintf("import-%s", time.Now().Format("2006-01-02"))
-			opts = append(opts, containerd.WithImageRefTranslator(docker.RefTranslator(prefix, false)))
+			opts = append(opts, containerd.WithImageRefTranslator(archive.AddRefPrefix(prefix)))
 		} else {
 			// When provided, filter out references which do not match
-			opts = append(opts, containerd.WithImageRefTranslator(docker.RefTranslator(prefix, true)))
+			opts = append(opts, containerd.WithImageRefTranslator(archive.FilterRefPrefix(prefix)))
-		}
-
-		switch format := context.String("format"); format {
-		case "", "docker", "docker.v1.1", "docker.v1.2":
-			opts = append(opts, containerd.WithImporter(&docker.V1Importer{
-				SkipOCI: strings.HasPrefix(format, "docker"),
-			}))
-		case "oci", "oci.v1":
-			opts = append(opts, containerd.WithImporter(&oci.V1Importer{}))
-		default:
-			return fmt.Errorf("unknown format %s", format)
 		}
 
 		if context.Bool("digests") {
-			opts = append(opts, containerd.WithDigestRef(oci.DigestTranslator(prefix)))
+			opts = append(opts, containerd.WithDigestRef(archive.DigestTranslator(prefix)))
 		}
 
 		if idxName := context.String("index-name"); idxName != "" {

content/helpers.go

@@ -70,7 +70,7 @@ func WriteBlob(ctx context.Context, cs Ingester, ref string, r io.Reader, desc o
 	cw, err := OpenWriter(ctx, cs, WithRef(ref), WithDescriptor(desc))
 	if err != nil {
 		if !errdefs.IsAlreadyExists(err) {
-			return err
+			return errors.Wrap(err, "failed to open writer")
 		}
 
 		return nil // all ready present
@@ -127,7 +127,7 @@ func OpenWriter(ctx context.Context, cs Ingester, opts ...WriterOpt) (Writer, er
 func Copy(ctx context.Context, cw Writer, r io.Reader, size int64, expected digest.Digest, opts ...Opt) error {
 	ws, err := cw.Status()
 	if err != nil {
-		return err
+		return errors.Wrap(err, "failed to get status")
 	}
 
 	if ws.Offset > 0 {
@@ -138,7 +138,7 @@ func Copy(ctx context.Context, cw Writer, r io.Reader, size int64, expected dige
 	}
 
 	if _, err := copyWithBuffer(cw, r); err != nil {
-		return err
+		return errors.Wrap(err, "failed to copy")
 	}
 
 	if err := cw.Commit(ctx, size, expected, opts...); err != nil {

images/archive/importer.go

@@ -14,9 +14,8 @@
    limitations under the License.
 */
 
-// Package docker provides a Docker compatible importer capable of
+// Package archive provides a Docker and OCI compatible importer
-// importing both Docker and OCI formats.
+package archive
-package docker
 
 import (
 	"archive/tar"
@@ -37,19 +36,15 @@ import (
 	"github.com/pkg/errors"
 )
 
-// V1Importer implements Docker v1.1, v1.2 and OCI v1.
+// ImportIndex imports an index from a tar achive image bundle
-type V1Importer struct {
+// - implements Docker v1.1, v1.2 and OCI v1.
-	// SkipOCI prevent interpretting OCI files
+// - prefers OCI v1 when provided
-	SkipOCI bool
+// - creates OCI index for Docker formats
-
+// - normalizes Docker references and adds as OCI ref name
-	// TODO: Add option to compress layers on ingest
+//      e.g. alpine:latest -> docker.io/library/alpine:latest
-
+// - existing OCI reference names are untouched
-}
+// - TODO: support option to compress layers on ingest
-
+func ImportIndex(ctx context.Context, store content.Store, reader io.Reader) (ocispec.Descriptor, error) {
-var _ images.Importer = &V1Importer{}
-
-// Import implements Importer.
-func (oi *V1Importer) Import(ctx context.Context, store content.Store, reader io.Reader) (ocispec.Descriptor, error) {
 	var (
 		tr = tar.NewReader(reader)
 
@@ -82,7 +77,7 @@ func (oi *V1Importer) Import(ctx context.Context, store content.Store, reader io
 		}
 
 		hdrName := path.Clean(hdr.Name)
-		if hdrName == ocispec.ImageLayoutFile && !oi.SkipOCI {
+		if hdrName == ocispec.ImageLayoutFile {
 			if err = onUntarJSON(tr, &ociLayout); err != nil {
 				return ocispec.Descriptor{}, errors.Wrapf(err, "untar oci layout %q", hdr.Name)
 			}
@@ -103,6 +98,9 @@ func (oi *V1Importer) Import(ctx context.Context, store content.Store, reader io
 		}
 	}
 
+	// If OCI layout was given, interpret the tar as an OCI layout.
+	// When not provided, the layout of the tar will be interpretted
+	// as Docker v1.1 or v1.2.
 	if ociLayout.Version != "" {
 		if ociLayout.Version != ocispec.ImageLayoutVersion {
 			return ocispec.Descriptor{}, errors.Errorf("unsupported OCI version %s", ociLayout.Version)
@@ -156,7 +154,9 @@ func (oi *V1Importer) Import(ctx context.Context, store content.Store, reader io
 			return ocispec.Descriptor{}, errors.Wrap(err, "unable to resolve platform")
 		}
 		if len(platforms) > 0 {
-			// Only one platform can be resolved from non-index manifest
+			// Only one platform can be resolved from non-index manifest,
+			// The platform can only come from the config included above,
+			// if the config has no platform it can be safely ommitted.
 			desc.Platform = &platforms[0]
 		}
 
@@ -165,18 +165,18 @@ func (oi *V1Importer) Import(ctx context.Context, store content.Store, reader io
 		} else {
 			// Add descriptor per tag
 			for _, ref := range mfst.RepoTags {
-				msftdesc := desc
+				mfstdesc := desc
 
 				normalized, err := normalizeReference(ref)
 				if err != nil {
 					return ocispec.Descriptor{}, err
 				}
 
-				msftdesc.Annotations = map[string]string{
+				mfstdesc.Annotations = map[string]string{
 					ocispec.AnnotationRefName: normalized,
 				}
 
-				idx.Manifests = append(idx.Manifests, msftdesc)
+				idx.Manifests = append(idx.Manifests, mfstdesc)
 			}
 		}
 	}

images/archive/reference.go

@@ -14,18 +14,31 @@
    limitations under the License.
 */
 
-package docker
+package archive
 
 import (
 	"strings"
 
 	"github.com/containerd/cri/pkg/util"
+	digest "github.com/opencontainers/go-digest"
 	"github.com/pkg/errors"
 )
 
-// RefTranslator creates a reference which only has a tag or verifies
+// FilterRefPrefix restricts references to having the given image
+// prefix. Tag-only references will have the prefix prepended.
+func FilterRefPrefix(image string) func(string) string {
+	return refTranslator(image, true)
+}
+
+// AddRefPrefix prepends the given image prefix to tag-only references,
+// while leaving returning full references unmodified.
+func AddRefPrefix(image string) func(string) string {
+	return refTranslator(image, false)
+}
+
+// refTranslator creates a reference which only has a tag or verifies
 // a full reference.
-func RefTranslator(image string, checkPrefix bool) func(string) string {
+func refTranslator(image string, checkPrefix bool) func(string) string {
 	return func(ref string) string {
 		// Check if ref is full reference
 		if strings.ContainsAny(ref, "/:@") {
@@ -63,3 +76,11 @@ func normalizeReference(ref string) (string, error) {
 
 	return normalized.String(), nil
 }
+
+// DigestTranslator creates a digest reference by adding the
+// digest to an image name
+func DigestTranslator(prefix string) func(digest.Digest) string {
+	return func(dgst digest.Digest) string {
+		return prefix + "@" + dgst.String()
+	}
+}

images/oci/importer.go

@@ -1,127 +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 provides the importer and the exporter for OCI Image Spec.
-package oci
-
-import (
-	"archive/tar"
-	"bytes"
-	"context"
-	"io"
-	"io/ioutil"
-	"path"
-	"strings"
-
-	"github.com/containerd/containerd/content"
-	"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{}
-
-var _ images.Importer = &V1Importer{}
-
-// Import implements Importer.
-func (oi *V1Importer) Import(ctx context.Context, store content.Store, reader io.Reader) (ocispec.Descriptor, error) {
-	var (
-		desc ocispec.Descriptor
-		tr   = tar.NewReader(reader)
-	)
-	for {
-		hdr, err := tr.Next()
-		if err == io.EOF {
-			break
-		}
-		if err != nil {
-			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 desc.Digest != "" {
-				return ocispec.Descriptor{}, errors.New("duplicated index.json")
-			}
-			desc, err = onUntarIndexJSON(ctx, tr, store, hdr.Size)
-			if err != nil {
-				return ocispec.Descriptor{}, err
-			}
-		} else if strings.HasPrefix(hdrName, "blobs/") {
-			if err := onUntarBlob(ctx, tr, store, hdrName, hdr.Size); err != nil {
-				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 == "" {
-		return ocispec.Descriptor{}, errors.New("no index.json found")
-	}
-
-	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 ocispec.Descriptor{}, err
-	}
-	desc := ocispec.Descriptor{
-		MediaType: ocispec.MediaTypeImageIndex,
-		Digest:    digest.FromBytes(b),
-		Size:      size,
-	}
-	if int64(len(b)) != size {
-		return ocispec.Descriptor{}, errors.Errorf("size mismatch %d v %d", len(b), size)
-	}
-
-	if err := content.WriteBlob(ctx, store, "index-"+desc.Digest.String(), bytes.NewReader(b), desc); err != nil {
-		return ocispec.Descriptor{}, err
-	}
-
-	return desc, err
-}
-
-func onUntarBlob(ctx context.Context, r io.Reader, store content.Ingester, name string, size int64) error {
-	// name is like "blobs/sha256/deadbeef"
-	split := strings.Split(name, "/")
-	if len(split) != 3 {
-		return errors.Errorf("unexpected name: %q", name)
-	}
-	algo := digest.Algorithm(split[1])
-	if !algo.Available() {
-		return errors.Errorf("unsupported algorithm: %s", algo)
-	}
-	dgst := digest.NewDigestFromHex(algo.String(), split[2])
-	return content.WriteBlob(ctx, store, "blob-"+dgst.String(), r, ocispec.Descriptor{Size: size, Digest: dgst})
-}
-
-// DigestTranslator creates a digest reference by adding the
-// digest to an image name
-func DigestTranslator(prefix string) func(digest.Digest) string {
-	return func(dgst digest.Digest) string {
-		return prefix + "@" + dgst.String()
-	}
-}

import.go

@@ -24,7 +24,7 @@ import (
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
-	"github.com/containerd/containerd/images/oci"
+	"github.com/containerd/containerd/images/archive"
 	digest "github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
@@ -33,7 +33,6 @@ 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
@@ -65,15 +64,6 @@ func WithIndexName(name string) ImportOpt {
 	}
 }
 
-// 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.
@@ -85,17 +75,13 @@ func (c *Client) Import(ctx context.Context, reader io.Reader, opts ...ImportOpt
 		}
 	}
 
-	if iopts.importer == nil {
-		iopts.importer = &oci.V1Importer{}
-	}
-
 	ctx, done, err := c.WithLease(ctx)
 	if err != nil {
 		return nil, err
 	}
 	defer done(ctx)
 
-	index, err := iopts.importer.Import(ctx, c.ContentStore(), reader)
+	index, err := archive.ImportIndex(ctx, c.ContentStore(), reader)
 	if err != nil {
 		return nil, err
 	}

import_test.go

@@ -20,7 +20,7 @@ import (
 	"runtime"
 	"testing"
 
-	"github.com/containerd/containerd/images/docker"
+	"github.com/containerd/containerd/images/archive"
 	"github.com/containerd/containerd/images/oci"
 )
 
@@ -51,12 +51,11 @@ func TestOCIExportAndImport(t *testing.T) {
 	}
 
 	opts := []ImportOpt{
-		WithImporter(&oci.V1Importer{}),
+		WithImageRefTranslator(archive.AddRefPrefix("foo/bar")),
-		WithImageRefTranslator(docker.RefTranslator("foo/bar", false)),
 	}
 	imgrecs, err := client.Import(ctx, exported, opts...)
 	if err != nil {
-		t.Fatal(err)
+		t.Fatalf("Import failed: %+v", err)
 	}
 
 	for _, imgrec := range imgrecs {