diff --git a/copy/copy.go b/copy/copy.go index 1fb753edd5..fd13176571 100644 --- a/copy/copy.go +++ b/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() diff --git a/copy/manifest_test.go b/copy/manifest_test.go index dd8ac427c4..eda492dce9 100644 --- a/copy/manifest_test.go +++ b/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") } diff --git a/image/docker_schema1.go b/image/docker_schema1.go index 6d09a868fe..4152b3cdf7 100644 --- a/image/docker_schema1.go +++ b/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 diff --git a/image/docker_schema2.go b/image/docker_schema2.go index ea529e24b4..a2a36ea2cd 100644 --- a/image/docker_schema2.go +++ b/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 diff --git a/image/docker_schema2_test.go b/image/docker_schema2_test.go index 9d1ab7ee33..29164abe59 100644 --- a/image/docker_schema2_test.go +++ b/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{ diff --git a/image/manifest.go b/image/manifest.go index 4715a3bc0c..75c9e71164 100644 --- a/image/manifest.go +++ b/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 diff --git a/image/oci.go b/image/oci.go index 7408420456..2575d1e0f5 100644 --- a/image/oci.go +++ b/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 diff --git a/image/oci_test.go b/image/oci_test.go index c51b3ae774..17012c713b 100644 --- a/image/oci_test.go +++ b/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{ diff --git a/types/types.go b/types/types.go index 62f5732b54..63337c8105 100644 --- a/types/types.go +++ b/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 }