From e0702e9f373f5086f53de2485262dde9fc1e7ac2 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Tue, 29 Nov 2016 17:06:48 -0800 Subject: [PATCH] distribution: Fix panic on push When building a manifest during a push operation, all layers must have an associated descriptor. If a layer is missing a descriptor, that leads to a panic. A break inside a switch in layerAlreadyExists meant to break from the loop surrounding the switch, but instead breaks from the switch. This causes the loop to continue, and can overwrite the descriptor with an empty one, leading to the panic. Also, fix layerAlreadyExists not to abort the push when a speculative stat on a candidate layer digest fails with an error. This could happen in situations like a potential cross-repository mount where the user does not have permission to access the source repository. Signed-off-by: Aaron Lehmann --- distribution/push_v2.go | 6 +++--- distribution/push_v2_test.go | 13 ++++++++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/distribution/push_v2.go b/distribution/push_v2.go index f2a1f02714..d72882b0d9 100644 --- a/distribution/push_v2.go +++ b/distribution/push_v2.go @@ -523,6 +523,7 @@ func (pd *v2PushDescriptor) layerAlreadyExists( layerDigests = append(layerDigests, meta.Digest) } +attempts: for _, dgst := range layerDigests { meta := digestToMetadata[dgst] logrus.Debugf("Checking for presence of layer %s (%s) in %s", diffID, dgst, pd.repoInfo.FullName()) @@ -541,15 +542,14 @@ func (pd *v2PushDescriptor) layerAlreadyExists( } desc.MediaType = schema2.MediaTypeLayer exists = true - break + break attempts case distribution.ErrBlobUnknown: if meta.SourceRepository == pd.repoInfo.FullName() { // remove the mapping to the target repository pd.v2MetadataService.Remove(*meta) } default: - progress.Update(progressOutput, pd.ID(), "Image push failed") - return desc, false, retryOnError(err) + logrus.WithError(err).Debugf("Failed to check for presence of layer %s (%s) in %s", diffID, dgst, pd.repoInfo.FullName()) } } diff --git a/distribution/push_v2_test.go b/distribution/push_v2_test.go index b76e1a3733..c56f50bdae 100644 --- a/distribution/push_v2_test.go +++ b/distribution/push_v2_test.go @@ -180,7 +180,7 @@ func TestLayerAlreadyExists(t *testing.T) { maxExistenceChecks: 1, metadata: []metadata.V2Metadata{{Digest: digest.Digest("apple"), SourceRepository: "docker.io/library/busybox"}}, remoteErrors: map[digest.Digest]error{digest.Digest("apple"): distribution.ErrAccessDenied}, - expectedError: distribution.ErrAccessDenied, + expectedError: nil, expectedRequests: []string{"apple"}, }, { @@ -310,7 +310,7 @@ func TestLayerAlreadyExists(t *testing.T) { expectedRemovals: []metadata.V2Metadata{taggedMetadata("key3", "apple", "docker.io/library/busybox")}, }, { - name: "stop on first error", + name: "don't stop on first error", targetRepo: "user/app", hmacKey: "key", metadata: []metadata.V2Metadata{ @@ -321,9 +321,12 @@ func TestLayerAlreadyExists(t *testing.T) { maxExistenceChecks: 3, remoteErrors: map[digest.Digest]error{"orange": distribution.ErrAccessDenied}, remoteBlobs: map[digest.Digest]distribution.Descriptor{digest.Digest("apple"): {}}, - expectedError: distribution.ErrAccessDenied, - expectedRequests: []string{"plum", "orange"}, - expectedRemovals: []metadata.V2Metadata{taggedMetadata("key", "plum", "docker.io/user/app")}, + expectedError: nil, + expectedRequests: []string{"plum", "orange", "banana"}, + expectedRemovals: []metadata.V2Metadata{ + taggedMetadata("key", "plum", "docker.io/user/app"), + taggedMetadata("key", "banana", "docker.io/user/app"), + }, }, { name: "remove outdated metadata",