From ee12ecb1b69894cc109f836a1d347dab43f1207f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 19 Sep 2016 22:11:39 +0200 Subject: [PATCH 1/2] Add types.Image.EmbeddedDockerReferenceConflicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows us to detect whether we need to edit the reference before trying to (i.e. before uploading any of the layers). The method interface may be a bit surprising, it is designed to hide the complexity and schema1 specifics in manifestSchema1; see the comment in that implementation for more details. Signed-off-by: Miloslav Trmač --- copy/manifest_test.go | 4 ++++ image/docker_schema1.go | 21 +++++++++++++++++++++ image/docker_schema2.go | 8 ++++++++ image/docker_schema2_test.go | 14 ++++++++++++++ image/manifest.go | 5 +++++ image/oci.go | 8 ++++++++ image/oci_test.go | 14 ++++++++++++++ types/types.go | 4 ++++ 8 files changed, 78 insertions(+) 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..2f46232ef3 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 { diff --git a/image/docker_schema2.go b/image/docker_schema2.go index ea529e24b4..6390b3ef8f 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 { diff --git a/image/docker_schema2_test.go b/image/docker_schema2_test.go index 9d1ab7ee33..aa4c8b50eb 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) 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..dd86a13a42 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 { diff --git a/image/oci_test.go b/image/oci_test.go index c51b3ae774..c823abe61e 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) diff --git a/types/types.go b/types/types.go index 62f5732b54..0247fb2d86 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. From 0cd00ea08c02d75455460078b05988263b9f59c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 19 Sep 2016 22:32:48 +0200 Subject: [PATCH 2/2] Update embedded Docker reference, if any, in copy.Image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add EmbbeddedDockerReference to types.ManifestUpdateOptions, and use it in copy.Image if necessary. Pushing signed schema1 images to a different location is impossible (skopeo users can use --remove-signatures), but signing an unsigned schema1 image while pushing it does work. Signed-off-by: Miloslav Trmač --- copy/copy.go | 22 ++++++++++++++++++++++ image/docker_schema1.go | 8 ++++++++ image/docker_schema2.go | 1 + image/docker_schema2_test.go | 13 +++++++++++++ image/oci.go | 1 + image/oci_test.go | 13 +++++++++++++ types/types.go | 5 +++-- 7 files changed, 61 insertions(+), 2 deletions(-) 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/image/docker_schema1.go b/image/docker_schema1.go index 2f46232ef3..4152b3cdf7 100644 --- a/image/docker_schema1.go +++ b/image/docker_schema1.go @@ -194,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 6390b3ef8f..a2a36ea2cd 100644 --- a/image/docker_schema2.go +++ b/image/docker_schema2.go @@ -188,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 aa4c8b50eb..29164abe59 100644 --- a/image/docker_schema2_test.go +++ b/image/docker_schema2_test.go @@ -421,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/oci.go b/image/oci.go index dd86a13a42..2575d1e0f5 100644 --- a/image/oci.go +++ b/image/oci.go @@ -154,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 c823abe61e..17012c713b 100644 --- a/image/oci_test.go +++ b/image/oci_test.go @@ -302,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 0247fb2d86..63337c8105 100644 --- a/types/types.go +++ b/types/types.go @@ -249,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 }