Update name/tag embedded into schema1 manifests

copy/copy.go

@@ -183,6 +183,10 @@ func Image(policyContext *signature.PolicyContext, destRef, srcRef types.ImageRe
 	manifestUpdates := types.ManifestUpdateOptions{}
 	manifestUpdates.InformationOnly.Destination = dest
 
+	if err := updateEmbeddedDockerReference(&manifestUpdates, dest, src, canModifyManifest); err != nil {
+		return err
+	}
+
 	// We compute preferredManifestMIMEType only to show it in error messages.
 	// Without having to add this context in an error message, we would be happy enough to know only that no conversion is needed.
 	preferredManifestMIMEType, otherManifestMIMETypeCandidates, err := determineManifestConversion(&manifestUpdates, src, destSupportedManifestMIMETypes, canModifyManifest)
@@ -273,6 +277,24 @@ func Image(policyContext *signature.PolicyContext, destRef, srcRef types.ImageRe
 	return nil
 }
 
+// updateEmbeddedDockerReference handles the Docker reference embedded in Docker schema1 manifests.
+func updateEmbeddedDockerReference(manifestUpdates *types.ManifestUpdateOptions, dest types.ImageDestination, src types.Image, canModifyManifest bool) error {
+	destRef := dest.Reference().DockerReference()
+	if destRef == nil {
+		return nil // Destination does not care about Docker references
+	}
+	if !src.EmbeddedDockerReferenceConflicts(destRef) {
+		return nil // No reference embedded in the manifest, or it matches destRef already.
+	}
+
+	if !canModifyManifest {
+		return errors.Errorf("Copying a schema1 image with an embedded Docker reference to %s (Docker reference %s) would invalidate existing signatures. Explicitly enable signature removal to proceed anyway",
+			transports.ImageName(dest.Reference()), destRef.String())
+	}
+	manifestUpdates.EmbeddedDockerReference = destRef
+	return nil
+}
+
 // copyLayers copies layers from src/rawSource to dest, using and updating ic.manifestUpdates if necessary and ic.canModifyManifest.
 func (ic *imageCopier) copyLayers() error {
 	srcInfos := ic.src.LayerInfos()

copy/manifest_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/containers/image/docker/reference"
 	"github.com/containers/image/manifest"
 	"github.com/containers/image/types"
 	"github.com/opencontainers/image-spec/specs-go/v1"
@@ -57,6 +58,9 @@ func (f fakeImageSource) OCIConfig() (*v1.Image, error) {
 func (f fakeImageSource) LayerInfos() []types.BlobInfo {
 	panic("Unexpected call to a mock function")
 }
+func (f fakeImageSource) EmbeddedDockerReferenceConflicts(ref reference.Named) bool {
+	panic("Unexpected call to a mock function")
+}
 func (f fakeImageSource) Inspect() (*types.ImageInspectInfo, error) {
 	panic("Unexpected call to a mock function")
 }

image/docker_schema1.go

@@ -135,6 +135,27 @@ func (m *manifestSchema1) LayerInfos() []types.BlobInfo {
 	return layers
 }
 
+// EmbeddedDockerReferenceConflicts whether a Docker reference embedded in the manifest, if any, conflicts with destination ref.
+// It returns false if the manifest does not embed a Docker reference.
+// (This embedding unfortunately happens for Docker schema1, please do not add support for this in any new formats.)
+func (m *manifestSchema1) EmbeddedDockerReferenceConflicts(ref reference.Named) bool {
+	// This is a bit convoluted: We can’t just have a "get embedded docker reference" method
+	// and have the “does it conflict” logic in the generic copy code, because the manifest does not actually
+	// embed a full docker/distribution reference, but only the repo name and tag (without the host name).
+	// So we would have to provide a “return repo without host name, and tag” getter for the generic code,
+	// which would be very awkward.  Instead, we do the matching here in schema1-specific code, and all the
+	// generic copy code needs to know about is reference.Named and that a manifest may need updating
+	// for some destinations.
+	name := reference.Path(ref)
+	var tag string
+	if tagged, isTagged := ref.(reference.NamedTagged); isTagged {
+		tag = tagged.Tag()
+	} else {
+		tag = ""
+	}
+	return m.Name != name || m.Tag != tag
+}
+
 func (m *manifestSchema1) imageInspectInfo() (*types.ImageInspectInfo, error) {
 	v1 := &v1Image{}
 	if err := json.Unmarshal([]byte(m.History[0].V1Compatibility), v1); err != nil {
@@ -173,6 +194,14 @@ func (m *manifestSchema1) UpdatedImage(options types.ManifestUpdateOptions) (typ
 			copy.FSLayers[(len(options.LayerInfos)-1)-i].BlobSum = info.Digest
 		}
 	}
+	if options.EmbeddedDockerReference != nil {
+		copy.Name = reference.Path(options.EmbeddedDockerReference)
+		if tagged, isTagged := options.EmbeddedDockerReference.(reference.NamedTagged); isTagged {
+			copy.Tag = tagged.Tag()
+		} else {
+			copy.Tag = ""
+		}
+	}
 
 	switch options.ManifestMIMEType {
 	case "": // No conversion, OK

image/docker_schema2.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 
 	"github.com/Sirupsen/logrus"
+	"github.com/containers/image/docker/reference"
 	"github.com/containers/image/manifest"
 	"github.com/containers/image/types"
 	"github.com/opencontainers/go-digest"
@@ -140,6 +141,13 @@ func (m *manifestSchema2) LayerInfos() []types.BlobInfo {
 	return blobs
 }
 
+// EmbeddedDockerReferenceConflicts whether a Docker reference embedded in the manifest, if any, conflicts with destination ref.
+// It returns false if the manifest does not embed a Docker reference.
+// (This embedding unfortunately happens for Docker schema1, please do not add support for this in any new formats.)
+func (m *manifestSchema2) EmbeddedDockerReferenceConflicts(ref reference.Named) bool {
+	return false
+}
+
 func (m *manifestSchema2) imageInspectInfo() (*types.ImageInspectInfo, error) {
 	config, err := m.ConfigBlob()
 	if err != nil {
@@ -180,6 +188,7 @@ func (m *manifestSchema2) UpdatedImage(options types.ManifestUpdateOptions) (typ
 			copy.LayersDescriptors[i].URLs = info.URLs
 		}
 	}
+	// Ignore options.EmbeddedDockerReference: it may be set when converting from schema1 to schema2, but we really don't care.
 
 	switch options.ManifestMIMEType {
 	case "": // No conversion, OK

image/docker_schema2_test.go

@@ -242,6 +242,20 @@ func TestManifestSchema2LayerInfo(t *testing.T) {
 	}
 }
 
+func TestManifestSchema2EmbeddedDockerReferenceConflicts(t *testing.T) {
+	for _, m := range []genericManifest{
+		manifestSchema2FromFixture(t, unusedImageSource{}, "schema2.json"),
+		manifestSchema2FromComponentsLikeFixture(nil),
+	} {
+		for _, name := range []string{"busybox", "example.com:5555/ns/repo:tag"} {
+			ref, err := reference.ParseNormalizedNamed(name)
+			require.NoError(t, err)
+			conflicts := m.EmbeddedDockerReferenceConflicts(ref)
+			assert.False(t, conflicts)
+		}
+	}
+}
+
 func TestManifestSchema2ImageInspectInfo(t *testing.T) {
 	configJSON, err := ioutil.ReadFile("fixtures/schema2-config.json")
 	require.NoError(t, err)
@@ -407,6 +421,19 @@ func TestManifestSchema2UpdatedImage(t *testing.T) {
 	})
 	assert.Error(t, err)
 
+	// EmbeddedDockerReference:
+	// … is ignored
+	embeddedRef, err := reference.ParseNormalizedNamed("busybox")
+	require.NoError(t, err)
+	res, err = original.UpdatedImage(types.ManifestUpdateOptions{
+		EmbeddedDockerReference: embeddedRef,
+	})
+	require.NoError(t, err)
+	nonEmbeddedRef, err := reference.ParseNormalizedNamed("notbusybox:notlatest")
+	require.NoError(t, err)
+	conflicts := res.EmbeddedDockerReferenceConflicts(nonEmbeddedRef)
+	assert.False(t, conflicts)
+
 	// ManifestMIMEType:
 	// Only smoke-test the valid conversions, detailed tests are below. (This also verifies that “original” is not affected.)
 	for _, mime := range []string{

image/manifest.go

@@ -3,6 +3,7 @@ package image
 import (
 	"time"
 
+	"github.com/containers/image/docker/reference"
 	"github.com/containers/image/manifest"
 	"github.com/containers/image/pkg/strslice"
 	"github.com/containers/image/types"
@@ -72,6 +73,10 @@ type genericManifest interface {
 	// The Digest field is guaranteed to be provided; Size may be -1.
 	// WARNING: The list may contain duplicates, and they are semantically relevant.
 	LayerInfos() []types.BlobInfo
+	// EmbeddedDockerReferenceConflicts whether a Docker reference embedded in the manifest, if any, conflicts with destination ref.
+	// It returns false if the manifest does not embed a Docker reference.
+	// (This embedding unfortunately happens for Docker schema1, please do not add support for this in any new formats.)
+	EmbeddedDockerReferenceConflicts(ref reference.Named) bool
 	imageInspectInfo() (*types.ImageInspectInfo, error) // To be called by inspectManifest
 	// UpdatedImageNeedsLayerDiffIDs returns true iff UpdatedImage(options) needs InformationOnly.LayerDiffIDs.
 	// This is a horribly specific interface, but computing InformationOnly.LayerDiffIDs can be very expensive to compute

image/oci.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"io/ioutil"
 
+	"github.com/containers/image/docker/reference"
 	"github.com/containers/image/manifest"
 	"github.com/containers/image/types"
 	"github.com/opencontainers/go-digest"
@@ -107,6 +108,13 @@ func (m *manifestOCI1) LayerInfos() []types.BlobInfo {
 	return blobs
 }
 
+// EmbeddedDockerReferenceConflicts whether a Docker reference embedded in the manifest, if any, conflicts with destination ref.
+// It returns false if the manifest does not embed a Docker reference.
+// (This embedding unfortunately happens for Docker schema1, please do not add support for this in any new formats.)
+func (m *manifestOCI1) EmbeddedDockerReferenceConflicts(ref reference.Named) bool {
+	return false
+}
+
 func (m *manifestOCI1) imageInspectInfo() (*types.ImageInspectInfo, error) {
 	config, err := m.ConfigBlob()
 	if err != nil {
@@ -146,6 +154,7 @@ func (m *manifestOCI1) UpdatedImage(options types.ManifestUpdateOptions) (types.
 			copy.LayersDescriptors[i].Size = info.Size
 		}
 	}
+	// Ignore options.EmbeddedDockerReference: it may be set when converting from schema1, but we really don't care.
 
 	switch options.ManifestMIMEType {
 	case "": // No conversion, OK

image/oci_test.go

@@ -207,6 +207,20 @@ func TestManifestOCI1LayerInfo(t *testing.T) {
 	}
 }
 
+func TestManifestOCI1EmbeddedDockerReferenceConflicts(t *testing.T) {
+	for _, m := range []genericManifest{
+		manifestOCI1FromFixture(t, unusedImageSource{}, "oci1.json"),
+		manifestOCI1FromComponentsLikeFixture(nil),
+	} {
+		for _, name := range []string{"busybox", "example.com:5555/ns/repo:tag"} {
+			ref, err := reference.ParseNormalizedNamed(name)
+			require.NoError(t, err)
+			conflicts := m.EmbeddedDockerReferenceConflicts(ref)
+			assert.False(t, conflicts)
+		}
+	}
+}
+
 func TestManifestOCI1ImageInspectInfo(t *testing.T) {
 	configJSON, err := ioutil.ReadFile("fixtures/oci1-config.json")
 	require.NoError(t, err)
@@ -288,6 +302,19 @@ func TestManifestOCI1UpdatedImage(t *testing.T) {
 	})
 	assert.Error(t, err)
 
+	// EmbeddedDockerReference:
+	// … is ignored
+	embeddedRef, err := reference.ParseNormalizedNamed("busybox")
+	require.NoError(t, err)
+	res, err = original.UpdatedImage(types.ManifestUpdateOptions{
+		EmbeddedDockerReference: embeddedRef,
+	})
+	require.NoError(t, err)
+	nonEmbeddedRef, err := reference.ParseNormalizedNamed("notbusybox:notlatest")
+	require.NoError(t, err)
+	conflicts := res.EmbeddedDockerReferenceConflicts(nonEmbeddedRef)
+	assert.False(t, conflicts)
+
 	// ManifestMIMEType:
 	// Only smoke-test the valid conversions, detailed tests are below. (This also verifies that “original” is not affected.)
 	for _, mime := range []string{

types/types.go

@@ -226,6 +226,10 @@ type Image interface {
 	// The Digest field is guaranteed to be provided; Size may be -1.
 	// WARNING: The list may contain duplicates, and they are semantically relevant.
 	LayerInfos() []BlobInfo
+	// EmbeddedDockerReferenceConflicts whether a Docker reference embedded in the manifest, if any, conflicts with destination ref.
+	// It returns false if the manifest does not embed a Docker reference.
+	// (This embedding unfortunately happens for Docker schema1, please do not add support for this in any new formats.)
+	EmbeddedDockerReferenceConflicts(ref reference.Named) bool
 	// Inspect returns various information for (skopeo inspect) parsed from the manifest and configuration.
 	Inspect() (*ImageInspectInfo, error)
 	// UpdatedImageNeedsLayerDiffIDs returns true iff UpdatedImage(options) needs InformationOnly.LayerDiffIDs.
@@ -245,8 +249,9 @@ type Image interface {
 
 // ManifestUpdateOptions is a way to pass named optional arguments to Image.UpdatedManifest
 type ManifestUpdateOptions struct {
-	LayerInfos       []BlobInfo // Complete BlobInfos (size+digest+urls) which should replace the originals, in order (the root layer first, and then successive layered layers)
-	ManifestMIMEType string
+	LayerInfos              []BlobInfo // Complete BlobInfos (size+digest+urls) which should replace the originals, in order (the root layer first, and then successive layered layers)
+	EmbeddedDockerReference reference.Named
+	ManifestMIMEType        string
 	// The values below are NOT requests to modify the image; they provide optional context which may or may not be used.
 	InformationOnly ManifestUpdateInformation
 }