Pārlūkot izejas kodu

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 <aaron.lehmann@docker.com>
(cherry picked from commit e0702e9f373f5086f53de2485262dde9fc1e7ac2)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>
Aaron Lehmann 8 gadi atpakaļ
vecāks
revīzija
1638b8f5c5
2 mainītis faili ar 11 papildinājumiem un 8 dzēšanām
  1. 3 3
      distribution/push_v2.go
  2. 8 5
      distribution/push_v2_test.go

+ 3 - 3
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())
 		}
 	}
 

+ 8 - 5
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",