Merge pull request #400 from nalind/manifest-lists

Improve support for manifest lists and image indexes

copy/manifest.go

@@ -119,3 +119,36 @@ func isMultiImage(ctx context.Context, img types.UnparsedImage) (bool, error) {
 	}
 	return manifest.MIMETypeIsMultiImage(mt), nil
 }
+
+// determineListConversion takes the current MIME type of a list of manifests,
+// the list of MIME types supported for a given destination, and a possible
+// forced value, and returns the MIME type to which we should convert the list
+// of manifests, whether we are converting to it or using it unmodified.
+func (c *copier) determineListConversion(currentListMIMEType string, destSupportedMIMETypes []string, forcedListMIMEType string) (string, error) {
+	// If we're forcing it, we prefer the forced value over everything else.
+	if forcedListMIMEType != "" {
+		return forcedListMIMEType, nil
+	}
+	// If there's no list of supported types, then anything we support is expected to be supported.
+	if len(destSupportedMIMETypes) == 0 {
+		destSupportedMIMETypes = manifest.SupportedListMIMETypes
+	}
+	var selectedType string
+	for i := range destSupportedMIMETypes {
+		// The second priority is the first member of the list of acceptable types that is a list,
+		// but keep going in case current type occurs later in the list.
+		if selectedType == "" && manifest.MIMETypeIsMultiImage(destSupportedMIMETypes[i]) {
+			selectedType = destSupportedMIMETypes[i]
+		}
+		// The first priority is the current type, if it's in the list, since that lets us avoid a
+		// conversion that isn't strictly necessary.
+		if destSupportedMIMETypes[i] == currentListMIMEType {
+			selectedType = destSupportedMIMETypes[i]
+		}
+	}
+	if selectedType == "" {
+		return "", errors.Errorf("destination does not support any supported manifest list types (%v)", manifest.SupportedListMIMETypes)
+	}
+	// Done.
+	return selectedType, nil
+}

copy/manifest_test.go

@@ -203,6 +203,8 @@ func TestIsMultiImage(t *testing.T) {
 	}{
 		{manifest.DockerV2ListMediaType, true},
 		{manifest.DockerV2Schema2MediaType, false},
+		{v1.MediaTypeImageManifest, false},
+		{v1.MediaTypeImageIndex, true},
 	} {
 		src := fakeImageSource(c.mt)
 		res, err := isMultiImage(context.Background(), src)

directory/directory_dest.go

@@ -199,16 +199,23 @@ func (d *dirImageDestination) TryReusingBlob(ctx context.Context, info types.Blo
 }
 
 // PutManifest writes manifest to the destination.
+// If instanceDigest is not nil, it contains a digest of the specific manifest instance to write the manifest for (when
+// the primary manifest is a manifest list); this should always be nil if the primary manifest is not a manifest list.
+// It is expected but not enforced that the instanceDigest, when specified, matches the digest of `manifest` as generated
+// by `manifest.Digest()`.
 // FIXME? This should also receive a MIME type if known, to differentiate between schema versions.
 // If the destination is in principle available, refuses this manifest type (e.g. it does not recognize the schema),
 // but may accept a different manifest type, the returned error must be an ManifestTypeRejectedError.
-func (d *dirImageDestination) PutManifest(ctx context.Context, manifest []byte) error {
-	return ioutil.WriteFile(d.ref.manifestPath(), manifest, 0644)
+func (d *dirImageDestination) PutManifest(ctx context.Context, manifest []byte, instanceDigest *digest.Digest) error {
+	return ioutil.WriteFile(d.ref.manifestPath(instanceDigest), manifest, 0644)
 }
 
-func (d *dirImageDestination) PutSignatures(ctx context.Context, signatures [][]byte) error {
+// PutSignatures writes a set of signatures to the destination.
+// If instanceDigest is not nil, it contains a digest of the specific manifest instance to write or overwrite the signatures for
+// (when the primary manifest is a manifest list); this should always be nil if the primary manifest is not a manifest list.
+func (d *dirImageDestination) PutSignatures(ctx context.Context, signatures [][]byte, instanceDigest *digest.Digest) error {
 	for i, sig := range signatures {
-		if err := ioutil.WriteFile(d.ref.signaturePath(i), sig, 0644); err != nil {
+		if err := ioutil.WriteFile(d.ref.signaturePath(i, instanceDigest), sig, 0644); err != nil {
 			return err
 		}
 	}
@@ -219,7 +226,7 @@ func (d *dirImageDestination) PutSignatures(ctx context.Context, signatures [][]
 // WARNING: This does not have any transactional semantics:
 // - Uploaded data MAY be visible to others before Commit() is called
 // - Uploaded data MAY be removed or MAY remain around if Close() is called without Commit() (i.e. rollback is allowed but not guaranteed)
-func (d *dirImageDestination) Commit(ctx context.Context) error {
+func (d *dirImageDestination) Commit(context.Context, types.UnparsedImage) error {
 	return nil
 }
 

directory/directory_src.go

@@ -9,7 +9,6 @@ import (
 	"github.com/containers/image/v4/manifest"
 	"github.com/containers/image/v4/types"
 	"github.com/opencontainers/go-digest"
-	"github.com/pkg/errors"
 )
 
 type dirImageSource struct {
@@ -38,10 +37,7 @@ func (s *dirImageSource) Close() error {
 // If instanceDigest is not nil, it contains a digest of the specific manifest instance to retrieve (when the primary manifest is a manifest list);
 // this never happens if the primary manifest is not a manifest list (e.g. if the source never returns manifest lists).
 func (s *dirImageSource) GetManifest(ctx context.Context, instanceDigest *digest.Digest) ([]byte, string, error) {
-	if instanceDigest != nil {
-		return nil, "", errors.Errorf(`Getting target manifest not supported by "dir:"`)
-	}
-	m, err := ioutil.ReadFile(s.ref.manifestPath())
+	m, err := ioutil.ReadFile(s.ref.manifestPath(instanceDigest))
 	if err != nil {
 		return nil, "", err
 	}
@@ -73,12 +69,9 @@ func (s *dirImageSource) GetBlob(ctx context.Context, info types.BlobInfo, cache
 // (when the primary manifest is a manifest list); this never happens if the primary manifest is not a manifest list
 // (e.g. if the source never returns manifest lists).
 func (s *dirImageSource) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) {
-	if instanceDigest != nil {
-		return nil, errors.Errorf(`Manifests lists are not supported by "dir:"`)
-	}
 	signatures := [][]byte{}
 	for i := 0; ; i++ {
-		signature, err := ioutil.ReadFile(s.ref.signaturePath(i))
+		signature, err := ioutil.ReadFile(s.ref.signaturePath(i, instanceDigest))
 		if err != nil {
 			if os.IsNotExist(err) {
 				break
@@ -90,7 +83,14 @@ func (s *dirImageSource) GetSignatures(ctx context.Context, instanceDigest *dige
 	return signatures, nil
 }
 
-// LayerInfosForCopy() returns updated layer info that should be used when copying, in preference to values in the manifest, if specified.
-func (s *dirImageSource) LayerInfosForCopy(ctx context.Context) ([]types.BlobInfo, error) {
+// LayerInfosForCopy returns either nil (meaning the values in the manifest are fine), or updated values for the layer
+// blobsums that are listed in the image's manifest.  If values are returned, they should be used when using GetBlob()
+// to read the image's layers.
+// If instanceDigest is not nil, it contains a digest of the specific manifest instance to retrieve BlobInfos for
+// (when the primary manifest is a manifest list); this never happens if the primary manifest is not a manifest list
+// (e.g. if the source never returns manifest lists).
+// The Digest field is guaranteed to be provided; Size may be -1.
+// WARNING: The list may contain duplicates, and they are semantically relevant.
+func (s *dirImageSource) LayerInfosForCopy(context.Context, *digest.Digest) ([]types.BlobInfo, error) {
 	return nil, nil
 }

directory/directory_test.go

@@ -32,27 +32,31 @@ func TestGetPutManifest(t *testing.T) {
 	defer os.RemoveAll(tmpDir)
 
 	man := []byte("test-manifest")
+	list := []byte("test-manifest-list")
+	md, err := manifest.Digest(man)
+	require.NoError(t, err)
 	dest, err := ref.NewImageDestination(context.Background(), nil)
 	require.NoError(t, err)
 	defer dest.Close()
-	err = dest.PutManifest(context.Background(), man)
+	err = dest.PutManifest(context.Background(), man, &md)
 	assert.NoError(t, err)
-	err = dest.Commit(context.Background())
+	err = dest.PutManifest(context.Background(), list, nil)
+	assert.NoError(t, err)
+	err = dest.Commit(context.Background(), nil)
 	assert.NoError(t, err)
 
 	src, err := ref.NewImageSource(context.Background(), nil)
 	require.NoError(t, err)
 	defer src.Close()
 	m, mt, err := src.GetManifest(context.Background(), nil)
 	assert.NoError(t, err)
-	assert.Equal(t, man, m)
+	assert.Equal(t, list, m)
 	assert.Equal(t, "", mt)
 
-	// Non-default instances are not supported
-	md, err := manifest.Digest(man)
-	require.NoError(t, err)
-	_, _, err = src.GetManifest(context.Background(), &md)
-	assert.Error(t, err)
+	m, mt, err = src.GetManifest(context.Background(), &md)
+	assert.NoError(t, err)
+	assert.Equal(t, man, m)
+	assert.Equal(t, "", mt)
 }
 
 func TestGetPutBlob(t *testing.T) {
@@ -67,7 +71,7 @@ func TestGetPutBlob(t *testing.T) {
 	assert.Equal(t, types.PreserveOriginal, dest.DesiredLayerCompression())
 	info, err := dest.PutBlob(context.Background(), bytes.NewReader(blob), types.BlobInfo{Digest: digest.Digest("sha256:digest-test"), Size: int64(9)}, cache, false)
 	assert.NoError(t, err)
-	err = dest.Commit(context.Background())
+	err = dest.Commit(context.Background(), nil)
 	assert.NoError(t, err)
 	assert.Equal(t, int64(9), info.Size)
 	assert.Equal(t, digest.FromBytes(blob), info.Digest)
@@ -126,7 +130,7 @@ func TestPutBlobDigestFailure(t *testing.T) {
 	_, err = dest.PutBlob(context.Background(), reader, types.BlobInfo{Digest: blobDigest, Size: -1}, cache, false)
 	assert.Error(t, err)
 	assert.Contains(t, digestErrorString, err.Error())
-	err = dest.Commit(context.Background())
+	err = dest.Commit(context.Background(), nil)
 	assert.NoError(t, err)
 
 	_, err = os.Lstat(blobPath)
@@ -148,26 +152,41 @@ func TestGetPutSignatures(t *testing.T) {
 	}
 	err = dest.SupportsSignatures(context.Background())
 	assert.NoError(t, err)
-	err = dest.PutManifest(context.Background(), man)
+	err = dest.PutManifest(context.Background(), man, nil)
+	require.NoError(t, err)
+
+	err = dest.PutSignatures(context.Background(), signatures, nil)
+	listSignatures := [][]byte{
+		[]byte("sig3"),
+		[]byte("sig4"),
+	}
+	md, err := manifest.Digest(man)
+	require.NoError(t, err)
+
+	err = dest.SupportsSignatures(context.Background())
+	assert.NoError(t, err)
+	err = dest.PutManifest(context.Background(), man, nil)
+	require.NoError(t, err)
+	err = dest.PutManifest(context.Background(), man, &md)
 	require.NoError(t, err)
 
-	err = dest.PutSignatures(context.Background(), signatures)
+	err = dest.PutSignatures(context.Background(), listSignatures, nil)
+	assert.NoError(t, err)
+	err = dest.PutSignatures(context.Background(), signatures, &md)
 	assert.NoError(t, err)
-	err = dest.Commit(context.Background())
+	err = dest.Commit(context.Background(), nil)
 	assert.NoError(t, err)
 
 	src, err := ref.NewImageSource(context.Background(), nil)
 	require.NoError(t, err)
 	defer src.Close()
 	sigs, err := src.GetSignatures(context.Background(), nil)
 	assert.NoError(t, err)
-	assert.Equal(t, signatures, sigs)
+	assert.Equal(t, listSignatures, sigs)
 
-	// Non-default instances are not supported
-	md, err := manifest.Digest(man)
-	require.NoError(t, err)
-	_, err = src.GetSignatures(context.Background(), &md)
-	assert.Error(t, err)
+	sigs, err = src.GetSignatures(context.Background(), &md)
+	assert.NoError(t, err)
+	assert.Equal(t, signatures, sigs)
 }
 
 func TestSourceReference(t *testing.T) {

directory/directory_transport.go

@@ -166,18 +166,24 @@ func (ref dirReference) DeleteImage(ctx context.Context, sys *types.SystemContex
 }
 
 // manifestPath returns a path for the manifest within a directory using our conventions.
-func (ref dirReference) manifestPath() string {
+func (ref dirReference) manifestPath(instanceDigest *digest.Digest) string {
+	if instanceDigest != nil {
+		return filepath.Join(ref.path, instanceDigest.Encoded()+".manifest.json")
+	}
 	return filepath.Join(ref.path, "manifest.json")
 }
 
 // layerPath returns a path for a layer tarball within a directory using our conventions.
 func (ref dirReference) layerPath(digest digest.Digest) string {
 	// FIXME: Should we keep the digest identification?
-	return filepath.Join(ref.path, digest.Hex())
+	return filepath.Join(ref.path, digest.Encoded())
 }
 
 // signaturePath returns a path for a signature within a directory using our conventions.
-func (ref dirReference) signaturePath(index int) string {
+func (ref dirReference) signaturePath(index int, instanceDigest *digest.Digest) string {
+	if instanceDigest != nil {
+		return filepath.Join(ref.path, fmt.Sprintf(instanceDigest.Encoded()+".signature-%d", index+1))
+	}
 	return filepath.Join(ref.path, fmt.Sprintf("signature-%d", index+1))
 }
 

directory/directory_transport_test.go

@@ -9,6 +9,7 @@ import (
 
 	_ "github.com/containers/image/v4/internal/testing/explicitfilepath-tmpdir"
 	"github.com/containers/image/v4/types"
+	digest "github.com/opencontainers/go-digest"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -157,9 +158,9 @@ func TestReferenceNewImage(t *testing.T) {
 	defer dest.Close()
 	mFixture, err := ioutil.ReadFile("../manifest/fixtures/v2s1.manifest.json")
 	require.NoError(t, err)
-	err = dest.PutManifest(context.Background(), mFixture)
+	err = dest.PutManifest(context.Background(), mFixture, nil)
 	assert.NoError(t, err)
-	err = dest.Commit(context.Background())
+	err = dest.Commit(context.Background(), nil)
 	assert.NoError(t, err)
 
 	img, err := ref.NewImage(context.Background(), nil)
@@ -174,9 +175,9 @@ func TestReferenceNewImageNoValidManifest(t *testing.T) {
 	dest, err := ref.NewImageDestination(context.Background(), nil)
 	require.NoError(t, err)
 	defer dest.Close()
-	err = dest.PutManifest(context.Background(), []byte(`{"schemaVersion":1}`))
+	err = dest.PutManifest(context.Background(), []byte(`{"schemaVersion":1}`), nil)
 	assert.NoError(t, err)
-	err = dest.Commit(context.Background())
+	err = dest.Commit(context.Background(), nil)
 	assert.NoError(t, err)
 
 	_, err = ref.NewImage(context.Background(), nil)
@@ -207,11 +208,14 @@ func TestReferenceDeleteImage(t *testing.T) {
 }
 
 func TestReferenceManifestPath(t *testing.T) {
+	dhex := digest.Digest("sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef")
+
 	ref, tmpDir := refToTempDir(t)
 	defer os.RemoveAll(tmpDir)
 	dirRef, ok := ref.(dirReference)
 	require.True(t, ok)
-	assert.Equal(t, tmpDir+"/manifest.json", dirRef.manifestPath())
+	assert.Equal(t, tmpDir+"/manifest.json", dirRef.manifestPath(nil))
+	assert.Equal(t, tmpDir+"/"+dhex.Encoded()+".manifest.json", dirRef.manifestPath(&dhex))
 }
 
 func TestReferenceLayerPath(t *testing.T) {
@@ -225,12 +229,16 @@ func TestReferenceLayerPath(t *testing.T) {
 }
 
 func TestReferenceSignaturePath(t *testing.T) {
+	dhex := digest.Digest("sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef")
+
 	ref, tmpDir := refToTempDir(t)
 	defer os.RemoveAll(tmpDir)
 	dirRef, ok := ref.(dirReference)
 	require.True(t, ok)
-	assert.Equal(t, tmpDir+"/signature-1", dirRef.signaturePath(0))
-	assert.Equal(t, tmpDir+"/signature-10", dirRef.signaturePath(9))
+	assert.Equal(t, tmpDir+"/signature-1", dirRef.signaturePath(0, nil))
+	assert.Equal(t, tmpDir+"/signature-10", dirRef.signaturePath(9, nil))
+	assert.Equal(t, tmpDir+"/"+dhex.Encoded()+".signature-1", dirRef.signaturePath(0, &dhex))
+	assert.Equal(t, tmpDir+"/"+dhex.Encoded()+".signature-10", dirRef.signaturePath(9, &dhex))
 }
 
 func TestReferenceVersionPath(t *testing.T) {

docker/archive/dest.go

@@ -67,6 +67,6 @@ func (d *archiveImageDestination) Close() error {
 // WARNING: This does not have any transactional semantics:
 // - Uploaded data MAY be visible to others before Commit() is called
 // - Uploaded data MAY be removed or MAY remain around if Close() is called without Commit() (i.e. rollback is allowed but not guaranteed)
-func (d *archiveImageDestination) Commit(ctx context.Context) error {
+func (d *archiveImageDestination) Commit(ctx context.Context, unparsedToplevel types.UnparsedImage) error {
 	return d.Destination.Commit(ctx)
 }

docker/archive/src.go

@@ -33,8 +33,3 @@ func newImageSource(ctx context.Context, ref archiveReference) (types.ImageSourc
 func (s *archiveImageSource) Reference() types.ImageReference {
 	return s.ref
 }
-
-// LayerInfosForCopy() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
-func (s *archiveImageSource) LayerInfosForCopy(ctx context.Context) ([]types.BlobInfo, error) {
-	return nil, nil
-}

docker/daemon/daemon_dest.go

@@ -124,7 +124,7 @@ func (d *daemonImageDestination) Reference() types.ImageReference {
 // WARNING: This does not have any transactional semantics:
 // - Uploaded data MAY be visible to others before Commit() is called
 // - Uploaded data MAY be removed or MAY remain around if Close() is called without Commit() (i.e. rollback is allowed but not guaranteed)
-func (d *daemonImageDestination) Commit(ctx context.Context) error {
+func (d *daemonImageDestination) Commit(ctx context.Context, unparsedToplevel types.UnparsedImage) error {
 	logrus.Debugf("docker-daemon: Closing tar stream")
 	if err := d.Destination.Commit(ctx); err != nil {
 		return err

docker/daemon/daemon_src.go

@@ -55,8 +55,3 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref daemonRef
 func (s *daemonImageSource) Reference() types.ImageReference {
 	return s.ref
 }
-
-// LayerInfosForCopy() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
-func (s *daemonImageSource) LayerInfosForCopy(ctx context.Context) ([]types.BlobInfo, error) {
-	return nil, nil
-}