1. docker
  2. Commit e0702e9f373f5086f53de2485262dde9fc1e7ac2
Browse code

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>

Aaron Lehmann authored on 2016/11/30 10:06:48
Showing 2 changed files
distribution/push_v2.go
History View file @ e0702e9
... ... 
@@ -523,6 +523,7 @@ func (pd *v2PushDescriptor) layerAlreadyExists(
523 523 
 		layerDigests = append(layerDigests, meta.Digest)
524 524 
 	}
525 525
526 
+attempts:
526 527 
 	for _, dgst := range layerDigests {
527 528 
 		meta := digestToMetadata[dgst]
528 529 
 		logrus.Debugf("Checking for presence of layer %s (%s) in %s", diffID, dgst, pd.repoInfo.FullName())
... ... 
@@ -541,15 +542,14 @@ func (pd *v2PushDescriptor) layerAlreadyExists(
541 541 
 			}
542 542 
 			desc.MediaType = schema2.MediaTypeLayer
543 543 
 			exists = true
544 
-			break
544 
+			break attempts
545 545 
 		case distribution.ErrBlobUnknown:
546 546 
 			if meta.SourceRepository == pd.repoInfo.FullName() {
547 547 
 				// remove the mapping to the target repository
548 548 
 				pd.v2MetadataService.Remove(*meta)
549 549 
 			}
550 550 
 		default:
551 
-			progress.Update(progressOutput, pd.ID(), "Image push failed")
552 
-			return desc, false, retryOnError(err)
551 
+			logrus.WithError(err).Debugf("Failed to check for presence of layer %s (%s) in %s", diffID, dgst, pd.repoInfo.FullName())
553 552 
 		}
554 553 
 	}
555 554
distribution/push_v2_test.go
History View file @ e0702e9
... ... 
@@ -180,7 +180,7 @@ func TestLayerAlreadyExists(t *testing.T) {
180 180 
 			maxExistenceChecks: 1,
181 181 
 			metadata:           []metadata.V2Metadata{{Digest: digest.Digest("apple"), SourceRepository: "docker.io/library/busybox"}},
182 182 
 			remoteErrors:       map[digest.Digest]error{digest.Digest("apple"): distribution.ErrAccessDenied},
183 
-			expectedError:      distribution.ErrAccessDenied,
183 
+			expectedError:      nil,
184 184 
 			expectedRequests:   []string{"apple"},
185 185 
 		},
186 186 
 		{
... ... 
@@ -310,7 +310,7 @@ func TestLayerAlreadyExists(t *testing.T) {
310 310 
 			expectedRemovals:   []metadata.V2Metadata{taggedMetadata("key3", "apple", "docker.io/library/busybox")},
311 311 
 		},
312 312 
 		{
313 
-			name:       "stop on first error",
313 
+			name:       "don't stop on first error",
314 314 
 			targetRepo: "user/app",
315 315 
 			hmacKey:    "key",
316 316 
 			metadata: []metadata.V2Metadata{
... ... 
@@ -321,9 +321,12 @@ func TestLayerAlreadyExists(t *testing.T) {
321 321 
 			maxExistenceChecks: 3,
322 322 
 			remoteErrors:       map[digest.Digest]error{"orange": distribution.ErrAccessDenied},
323 323 
 			remoteBlobs:        map[digest.Digest]distribution.Descriptor{digest.Digest("apple"): {}},
324 
-			expectedError:      distribution.ErrAccessDenied,
325 
-			expectedRequests:   []string{"plum", "orange"},
326 
-			expectedRemovals:   []metadata.V2Metadata{taggedMetadata("key", "plum", "docker.io/user/app")},
324 
+			expectedError:      nil,
325 
+			expectedRequests:   []string{"plum", "orange", "banana"},
326 
+			expectedRemovals: []metadata.V2Metadata{
327 
+				taggedMetadata("key", "plum", "docker.io/user/app"),
328 
+				taggedMetadata("key", "banana", "docker.io/user/app"),
329 
+			},
327 330 
 		},
328 331 
 		{
329 332 
 			name:       "remove outdated metadata",