From 4afafe953825d7b1b530907995989b3e9013f741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 5 May 2017 23:30:04 +0200 Subject: [PATCH 1/8] Log output of docker/distribution registries instead of sending it to /dev/null This is useful for diagnosing failures which are logged locally but not reported to the clients. --- integration/registry.go | 1 + 1 file changed, 1 insertion(+) diff --git a/integration/registry.go b/integration/registry.go index 0136e336..fa749bd7 100644 --- a/integration/registry.go +++ b/integration/registry.go @@ -109,6 +109,7 @@ http: } cmd := exec.Command(binary, confPath) + consumeAndLogOutputs(c, fmt.Sprintf("registry-%s", url), cmd) if err := cmd.Start(); err != nil { os.RemoveAll(tmp) if os.IsNotExist(err) { From 6f23c88e84ebcb9c8bf7ab2491ac9f39849d961c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 4 May 2017 21:30:05 +0200 Subject: [PATCH 2/8] Make schema1 dir: comparisons nondestructive Use (diff -x manifest.json) instead of removing the manifest.json files. Also rename the helper from destructiveCheckDirImageAreEqual to assertDirImagesAreEqual. --- integration/copy_test.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/integration/copy_test.go b/integration/copy_test.go index 1b28759b..d9182091 100644 --- a/integration/copy_test.go +++ b/integration/copy_test.go @@ -105,7 +105,7 @@ func (s *CopySuite) TestCopySimpleAtomicRegistry(c *check.C) { assertSkopeoSucceeds(c, "", "--tls-verify=false", "--debug", "copy", "dir:"+dir1, "atomic:localhost:5000/myns/unsigned:unsigned") // The result of pushing and pulling is an unmodified image. assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5000/myns/unsigned:unsigned", "dir:"+dir2) - destructiveCheckDirImagesAreEqual(c, dir1, dir2) + assertDirImagesAreEqual(c, dir1, dir2) } // The most basic (skopeo copy) use: @@ -139,11 +139,10 @@ func (s *CopySuite) TestCopySimple(c *check.C) { c.Assert(err, check.IsNil) } -// Check whether dir: images in dir1 and dir2 are equal. -// WARNING: This modifies the contents of dir1 and dir2! -func destructiveCheckDirImagesAreEqual(c *check.C, dir1, dir2 string) { +// Check whether dir: images in dir1 and dir2 are equal, ignoring schema1 signatures. +func assertDirImagesAreEqual(c *check.C, dir1, dir2 string) { // The manifests may have different JWS signatures; so, compare the manifests by digests, which - // strips the signatures, and remove them, comparing the rest file by file. + // strips the signatures. digests := []digest.Digest{} for _, dir := range []string{dir1, dir2} { manifestPath := filepath.Join(dir, "manifest.json") @@ -152,12 +151,10 @@ func destructiveCheckDirImagesAreEqual(c *check.C, dir1, dir2 string) { digest, err := manifest.Digest(m) c.Assert(err, check.IsNil) digests = append(digests, digest) - err = os.Remove(manifestPath) - c.Assert(err, check.IsNil) - c.Logf("Manifest file %s (digest %s) removed", manifestPath, digest) } c.Assert(digests[0], check.Equals, digests[1]) - out := combinedOutputOfCommand(c, "diff", "-urN", dir1, dir2) + // Then compare the rest file by file. + out := combinedOutputOfCommand(c, "diff", "-urN", "-x", "manifest.json", dir1, dir2) c.Assert(out, check.Equals, "") } @@ -176,7 +173,7 @@ func (s *CopySuite) TestCopyStreaming(c *check.C) { // Compare (copies of) the original and the copy: assertSkopeoSucceeds(c, "", "copy", "docker://estesp/busybox:amd64", "dir:"+dir1) assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5000/myns/unsigned:streaming", "dir:"+dir2) - destructiveCheckDirImagesAreEqual(c, dir1, dir2) + assertDirImagesAreEqual(c, dir1, dir2) // FIXME: Also check pushing to docker:// } @@ -501,7 +498,7 @@ func (s *CopySuite) TestCopyAtomicExtension(c *check.C) { assertSkopeoSucceeds(c, "", "--tls-verify=false", "--policy", policy, "--registries.d", registriesDir, "copy", "docker://localhost:5000/myns/extension:atomic", dirDest+"/dirAD") // Both access methods result in the same data. - destructiveCheckDirImagesAreEqual(c, filepath.Join(topDir, "dirAA"), filepath.Join(topDir, "dirAD")) + assertDirImagesAreEqual(c, filepath.Join(topDir, "dirAA"), filepath.Join(topDir, "dirAD")) // Get another image (different so that they don't share signatures, and sign it using docker://) assertSkopeoSucceeds(c, "", "--tls-verify=false", "--registries.d", registriesDir, @@ -514,7 +511,7 @@ func (s *CopySuite) TestCopyAtomicExtension(c *check.C) { assertSkopeoSucceeds(c, "", "--debug", "--tls-verify=false", "--policy", policy, "--registries.d", registriesDir, "copy", "docker://localhost:5000/myns/extension:extension", dirDest+"/dirDD") // Both access methods result in the same data. - destructiveCheckDirImagesAreEqual(c, filepath.Join(topDir, "dirDA"), filepath.Join(topDir, "dirDD")) + assertDirImagesAreEqual(c, filepath.Join(topDir, "dirDA"), filepath.Join(topDir, "dirDD")) } func (s *SkopeoSuite) TestCopySrcWithAuth(c *check.C) { From 22965c443f76dc7263714b6a56429f27ae39b459 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 4 May 2017 22:06:59 +0200 Subject: [PATCH 3/8] Allow updated names when comparing schema1 images The new version of containers/image will update the name and tag fields when pushing to schema1; so accept that before we update, so that tests keep working. For now, just ignore the name/tag fields, so that both the current and updated versions of containers/image are acceptable; we will tighten that after the update. --- integration/copy_test.go | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/integration/copy_test.go b/integration/copy_test.go index d9182091..a55551d0 100644 --- a/integration/copy_test.go +++ b/integration/copy_test.go @@ -1,6 +1,7 @@ package main import ( + "encoding/json" "fmt" "io/ioutil" "net/http" @@ -103,9 +104,9 @@ func (s *CopySuite) TestCopySimpleAtomicRegistry(c *check.C) { assertSkopeoSucceeds(c, "", "copy", "docker://estesp/busybox:amd64", "dir:"+dir1) // "push": dir: → atomic: assertSkopeoSucceeds(c, "", "--tls-verify=false", "--debug", "copy", "dir:"+dir1, "atomic:localhost:5000/myns/unsigned:unsigned") - // The result of pushing and pulling is an unmodified image. + // The result of pushing and pulling is an equivalent image, except for schema1 embedded names. assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5000/myns/unsigned:unsigned", "dir:"+dir2) - assertDirImagesAreEqual(c, dir1, dir2) + assertSchema1DirImagesAreEqualExceptNames(c, dir1, dir2) } // The most basic (skopeo copy) use: @@ -158,6 +159,29 @@ func assertDirImagesAreEqual(c *check.C, dir1, dir2 string) { c.Assert(out, check.Equals, "") } +// Check whether schema1 dir: images in dir1 and dir2 are equal, ignoring schema1 signatures and the embedded path/tag values. +func assertSchema1DirImagesAreEqualExceptNames(c *check.C, dir1, dir2 string) { + // The manifests may have different JWS signatures and names; so, unmarshal and delete these elements. + manifests := []map[string]interface{}{} + for _, dir := range []string{dir1, dir2} { + manifestPath := filepath.Join(dir, "manifest.json") + m, err := ioutil.ReadFile(manifestPath) + c.Assert(err, check.IsNil) + data := map[string]interface{}{} + err = json.Unmarshal(m, &data) + c.Assert(err, check.IsNil) + c.Assert(data["schemaVersion"], check.Equals, float64(1)) + for _, key := range []string{"signatures", "name", "tag"} { + delete(data, key) + } + manifests = append(manifests, data) + } + c.Assert(manifests[0], check.DeepEquals, manifests[1]) + // Then compare the rest file by file. + out := combinedOutputOfCommand(c, "diff", "-urN", "-x", "manifest.json", dir1, dir2) + c.Assert(out, check.Equals, "") +} + // Streaming (skopeo copy) func (s *CopySuite) TestCopyStreaming(c *check.C) { dir1, err := ioutil.TempDir("", "streaming-1") @@ -173,7 +197,7 @@ func (s *CopySuite) TestCopyStreaming(c *check.C) { // Compare (copies of) the original and the copy: assertSkopeoSucceeds(c, "", "copy", "docker://estesp/busybox:amd64", "dir:"+dir1) assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5000/myns/unsigned:streaming", "dir:"+dir2) - assertDirImagesAreEqual(c, dir1, dir2) + assertSchema1DirImagesAreEqualExceptNames(c, dir1, dir2) // FIXME: Also check pushing to docker:// } From 9bc847e656b4f1339ad5d71b8f1bd9733c89de49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 4 May 2017 23:38:19 +0200 Subject: [PATCH 4/8] Use a schema2 server in TestCopySignatures TestCopySignatures, among other things, tests handling of a correctly signed image to a different name without breaking the signature, which will be impossible with schema1 after we start updating the names embedded in the schema1 manifest. So, use the schema2 server binary, and docker://busybox image versions which use schema2. --- integration/copy_test.go | 34 ++++++++++++++++---------------- integration/fixtures/policy.json | 14 ++++++------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/integration/copy_test.go b/integration/copy_test.go index a55551d0..f7b6d0eb 100644 --- a/integration/copy_test.go +++ b/integration/copy_test.go @@ -274,34 +274,34 @@ func (s *CopySuite) TestCopySignatures(c *check.C) { // type: signedBy // Sign the images - assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "--sign-by", "personal@example.com", "docker://busybox:1.23", "atomic:localhost:5000/myns/personal:personal") - assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "--sign-by", "official@example.com", "docker://busybox:1.23.2", "atomic:localhost:5000/myns/official:official") + assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "--sign-by", "personal@example.com", "docker://busybox:1.26", "atomic:localhost:5006/myns/personal:personal") + assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "--sign-by", "official@example.com", "docker://busybox:1.26.1", "atomic:localhost:5006/myns/official:official") // Verify that we can pull them - assertSkopeoSucceeds(c, "", "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5000/myns/personal:personal", dirDest) - assertSkopeoSucceeds(c, "", "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5000/myns/official:official", dirDest) + assertSkopeoSucceeds(c, "", "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5006/myns/personal:personal", dirDest) + assertSkopeoSucceeds(c, "", "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5006/myns/official:official", dirDest) // Verify that mis-signed images are rejected - assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5000/myns/personal:personal", "atomic:localhost:5000/myns/official:attack") - assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5000/myns/official:official", "atomic:localhost:5000/myns/personal:attack") + assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5006/myns/personal:personal", "atomic:localhost:5006/myns/official:attack") + assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5006/myns/official:official", "atomic:localhost:5006/myns/personal:attack") assertSkopeoFails(c, ".*Source image rejected: Invalid GPG signature.*", - "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5000/myns/personal:attack", dirDest) + "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5006/myns/personal:attack", dirDest) assertSkopeoFails(c, ".*Source image rejected: Invalid GPG signature.*", - "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5000/myns/official:attack", dirDest) + "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5006/myns/official:attack", dirDest) // Verify that signed identity is verified. - assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5000/myns/official:official", "atomic:localhost:5000/myns/naming:test1") - assertSkopeoFails(c, ".*Source image rejected: Signature for identity localhost:5000/myns/official:official is not accepted.*", - "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5000/myns/naming:test1", dirDest) + assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5006/myns/official:official", "atomic:localhost:5006/myns/naming:test1") + assertSkopeoFails(c, ".*Source image rejected: Signature for identity localhost:5006/myns/official:official is not accepted.*", + "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5006/myns/naming:test1", dirDest) // signedIdentity works - assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5000/myns/official:official", "atomic:localhost:5000/myns/naming:naming") - assertSkopeoSucceeds(c, "", "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5000/myns/naming:naming", dirDest) + assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5006/myns/official:official", "atomic:localhost:5006/myns/naming:naming") + assertSkopeoSucceeds(c, "", "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5006/myns/naming:naming", dirDest) // Verify that cosigning requirements are enforced - assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5000/myns/official:official", "atomic:localhost:5000/myns/cosigned:cosigned") + assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5006/myns/official:official", "atomic:localhost:5006/myns/cosigned:cosigned") assertSkopeoFails(c, ".*Source image rejected: Invalid GPG signature.*", - "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5000/myns/cosigned:cosigned", dirDest) + "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5006/myns/cosigned:cosigned", dirDest) - assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "--sign-by", "personal@example.com", "atomic:localhost:5000/myns/official:official", "atomic:localhost:5000/myns/cosigned:cosigned") - assertSkopeoSucceeds(c, "", "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5000/myns/cosigned:cosigned", dirDest) + assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "--sign-by", "personal@example.com", "atomic:localhost:5006/myns/official:official", "atomic:localhost:5006/myns/cosigned:cosigned") + assertSkopeoSucceeds(c, "", "--tls-verify=false", "--policy", policy, "copy", "atomic:localhost:5006/myns/cosigned:cosigned", dirDest) } // --policy copy for dir: sources diff --git a/integration/fixtures/policy.json b/integration/fixtures/policy.json index eb68d7f2..2ba1c2a2 100644 --- a/integration/fixtures/policy.json +++ b/integration/fixtures/policy.json @@ -45,46 +45,46 @@ ] }, "atomic": { - "localhost:5000/myns/personal": [ + "localhost:5006/myns/personal": [ { "type": "signedBy", "keyType": "GPGKeys", "keyPath": "@keydir@/personal-pubkey.gpg" } ], - "localhost:5000/myns/official": [ + "localhost:5006/myns/official": [ { "type": "signedBy", "keyType": "GPGKeys", "keyPath": "@keydir@/official-pubkey.gpg" } ], - "localhost:5000/myns/naming:test1": [ + "localhost:5006/myns/naming:test1": [ { "type": "signedBy", "keyType": "GPGKeys", "keyPath": "@keydir@/official-pubkey.gpg" } ], - "localhost:5000/myns/naming:naming": [ + "localhost:5006/myns/naming:naming": [ { "type": "signedBy", "keyType": "GPGKeys", "keyPath": "@keydir@/official-pubkey.gpg", "signedIdentity": { "type": "exactRepository", - "dockerRepository": "localhost:5000/myns/official" + "dockerRepository": "localhost:5006/myns/official" } } ], - "localhost:5000/myns/cosigned:cosigned": [ + "localhost:5006/myns/cosigned:cosigned": [ { "type": "signedBy", "keyType": "GPGKeys", "keyPath": "@keydir@/official-pubkey.gpg", "signedIdentity": { "type": "exactRepository", - "dockerRepository": "localhost:5000/myns/official" + "dockerRepository": "localhost:5006/myns/official" } }, { From 03233a5ca7cb257150d8f0cca5494739eeb5a621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 4 May 2017 17:04:31 +0200 Subject: [PATCH 5/8] Vendor after merging mtrmac/image:docker-push-to-tag --- .../github.com/containers/image/copy/copy.go | 22 ++++++++++++++ .../containers/image/image/docker_schema1.go | 29 +++++++++++++++++++ .../containers/image/image/docker_schema2.go | 9 ++++++ .../containers/image/image/manifest.go | 5 ++++ .../github.com/containers/image/image/oci.go | 9 ++++++ .../containers/image/types/types.go | 9 ++++-- 6 files changed, 81 insertions(+), 2 deletions(-) diff --git a/vendor/github.com/containers/image/copy/copy.go b/vendor/github.com/containers/image/copy/copy.go index 1fb753ed..fd131765 100644 --- a/vendor/github.com/containers/image/copy/copy.go +++ b/vendor/github.com/containers/image/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/vendor/github.com/containers/image/image/docker_schema1.go b/vendor/github.com/containers/image/image/docker_schema1.go index 6d09a868..4152b3cd 100644 --- a/vendor/github.com/containers/image/image/docker_schema1.go +++ b/vendor/github.com/containers/image/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/vendor/github.com/containers/image/image/docker_schema2.go b/vendor/github.com/containers/image/image/docker_schema2.go index ea529e24..a2a36ea2 100644 --- a/vendor/github.com/containers/image/image/docker_schema2.go +++ b/vendor/github.com/containers/image/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/vendor/github.com/containers/image/image/manifest.go b/vendor/github.com/containers/image/image/manifest.go index 4715a3bc..75c9e711 100644 --- a/vendor/github.com/containers/image/image/manifest.go +++ b/vendor/github.com/containers/image/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/vendor/github.com/containers/image/image/oci.go b/vendor/github.com/containers/image/image/oci.go index 74084204..2575d1e0 100644 --- a/vendor/github.com/containers/image/image/oci.go +++ b/vendor/github.com/containers/image/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/vendor/github.com/containers/image/types/types.go b/vendor/github.com/containers/image/types/types.go index 62f5732b..63337c81 100644 --- a/vendor/github.com/containers/image/types/types.go +++ b/vendor/github.com/containers/image/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 } From cf7e58a297c14fd9414aa9125c7175f6acaaf0b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 4 May 2017 22:06:59 +0200 Subject: [PATCH 6/8] Test that names are updated as expected when pushing to schema1 Before the update, we have loosened the equality check to ignore the name/tag; now that we are generating them correctly, test for the expected values. --- integration/copy_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/integration/copy_test.go b/integration/copy_test.go index f7b6d0eb..f0838ff3 100644 --- a/integration/copy_test.go +++ b/integration/copy_test.go @@ -106,7 +106,7 @@ func (s *CopySuite) TestCopySimpleAtomicRegistry(c *check.C) { assertSkopeoSucceeds(c, "", "--tls-verify=false", "--debug", "copy", "dir:"+dir1, "atomic:localhost:5000/myns/unsigned:unsigned") // The result of pushing and pulling is an equivalent image, except for schema1 embedded names. assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5000/myns/unsigned:unsigned", "dir:"+dir2) - assertSchema1DirImagesAreEqualExceptNames(c, dir1, dir2) + assertSchema1DirImagesAreEqualExceptNames(c, dir1, "estesp/busybox:amd64", dir2, "myns/unsigned:unsigned") } // The most basic (skopeo copy) use: @@ -159,11 +159,11 @@ func assertDirImagesAreEqual(c *check.C, dir1, dir2 string) { c.Assert(out, check.Equals, "") } -// Check whether schema1 dir: images in dir1 and dir2 are equal, ignoring schema1 signatures and the embedded path/tag values. -func assertSchema1DirImagesAreEqualExceptNames(c *check.C, dir1, dir2 string) { +// Check whether schema1 dir: images in dir1 and dir2 are equal, ignoring schema1 signatures and the embedded path/tag values, which should have the expected values. +func assertSchema1DirImagesAreEqualExceptNames(c *check.C, dir1, ref1, dir2, ref2 string) { // The manifests may have different JWS signatures and names; so, unmarshal and delete these elements. manifests := []map[string]interface{}{} - for _, dir := range []string{dir1, dir2} { + for dir, ref := range map[string]string{dir1: ref1, dir2: ref2} { manifestPath := filepath.Join(dir, "manifest.json") m, err := ioutil.ReadFile(manifestPath) c.Assert(err, check.IsNil) @@ -171,6 +171,10 @@ func assertSchema1DirImagesAreEqualExceptNames(c *check.C, dir1, dir2 string) { err = json.Unmarshal(m, &data) c.Assert(err, check.IsNil) c.Assert(data["schemaVersion"], check.Equals, float64(1)) + colon := strings.LastIndex(ref, ":") + c.Assert(colon, check.Not(check.Equals), -1) + c.Assert(data["name"], check.Equals, ref[:colon]) + c.Assert(data["tag"], check.Equals, ref[colon+1:]) for _, key := range []string{"signatures", "name", "tag"} { delete(data, key) } @@ -197,7 +201,7 @@ func (s *CopySuite) TestCopyStreaming(c *check.C) { // Compare (copies of) the original and the copy: assertSkopeoSucceeds(c, "", "copy", "docker://estesp/busybox:amd64", "dir:"+dir1) assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "atomic:localhost:5000/myns/unsigned:streaming", "dir:"+dir2) - assertSchema1DirImagesAreEqualExceptNames(c, dir1, dir2) + assertSchema1DirImagesAreEqualExceptNames(c, dir1, "estesp/busybox:amd64", dir2, "myns/unsigned:streaming") // FIXME: Also check pushing to docker:// } From c4ec970bb2c81d753880fa4a8212beb598f48a67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 4 May 2017 20:28:29 +0200 Subject: [PATCH 7/8] Uncomment TestCopyCompression test cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have been able to push to Docker tags for a long time, and recently implemented s2→s1 autodetection. --- integration/copy_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/integration/copy_test.go b/integration/copy_test.go index f0838ff3..13c3e5a4 100644 --- a/integration/copy_test.go +++ b/integration/copy_test.go @@ -367,10 +367,10 @@ func (s *CopySuite) TestCopyCompression(c *check.C) { defer os.RemoveAll(topDir) for i, t := range []struct{ fixture, remote string }{ - //{"uncompressed-image-s1", "docker://" + v2DockerRegistryURL + "/compression/compression:s1"}, // FIXME: depends on push to tag working - //{"uncompressed-image-s2", "docker://" + v2DockerRegistryURL + "/compression/compression:s2"}, // FIXME: depends on push to tag working + {"uncompressed-image-s1", "docker://" + v2DockerRegistryURL + "/compression/compression:s1"}, + {"uncompressed-image-s2", "docker://" + v2DockerRegistryURL + "/compression/compression:s2"}, {"uncompressed-image-s1", "atomic:localhost:5000/myns/compression:s1"}, - //{"uncompressed-image-s2", "atomic:localhost:5000/myns/compression:s2"}, // FIXME: The unresolved "MANIFEST_UNKNOWN"/"unexpected end of JSON input" failure + {"uncompressed-image-s2", "atomic:localhost:5000/myns/compression:s2"}, } { dir := filepath.Join(topDir, fmt.Sprintf("case%d", i)) err := os.MkdirAll(dir, 0755) From 8ec2a142c93fb750f3c972a2a4c8ec09b1af2c02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 5 May 2017 23:30:23 +0200 Subject: [PATCH 8/8] =?UTF-8?q?Enable=20tests=20for=20schema2=E2=86=92sche?= =?UTF-8?q?ma1=20conversion=20with=20docker/distribution=20registries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that we can update the embedded name:tag, the test no longer fails on a schema1→schema1 copy with the old schema1 server which verifies the name:tag value. --- integration/copy_test.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/integration/copy_test.go b/integration/copy_test.go index 13c3e5a4..43a4d4aa 100644 --- a/integration/copy_test.go +++ b/integration/copy_test.go @@ -572,13 +572,7 @@ func (s *CopySuite) TestCopySchemaConversion(c *check.C) { // Test conversion / schema autodetection both for the OpenShift embedded registry… s.testCopySchemaConversionRegistries(c, "docker://localhost:5005/myns/schema1", "docker://localhost:5006/myns/schema2") // … and for various docker/distribution registry versions. - if false { - // FIXME: This does not currently work, because the schema1-only docker/distribution registry we have (unlike newer versions) - // enforces that a schema1 manifest contains a matching tag field. Our _s2→s1 conversion_ code does set the tag correctly, - // but a mere copy of schema1→schema1 changing the tag does not update the manifest. - // So, enabling this test results in a successful schema2→schema1 conversion, followed by a schema1→schema1 copy failure. - s.testCopySchemaConversionRegistries(c, "docker://"+v2s1DockerRegistryURL+"/schema1", "docker://"+v2DockerRegistryURL+"/schema2") - } + s.testCopySchemaConversionRegistries(c, "docker://"+v2s1DockerRegistryURL+"/schema1", "docker://"+v2DockerRegistryURL+"/schema2") } func (s *CopySuite) testCopySchemaConversionRegistries(c *check.C, schema1Registry, schema2Registry string) {