Browse code

fix(distribution): digest cache should not be moved if it was an auth error

local digest cache will be removed when error occured on push image
but it should not be removed if it is an auth error while on auth was
provided

https://github.com/moby/moby/issues/36309
Signed-off-by: 慕陶 <jihui.xjh@alibaba-inc.com>

慕陶 authored on 2018/03/07 20:30:17
Showing 2 changed files
... ...
@@ -15,6 +15,7 @@ import (
15 15
 	"github.com/docker/distribution/manifest/schema1"
16 16
 	"github.com/docker/distribution/manifest/schema2"
17 17
 	"github.com/docker/distribution/reference"
18
+	"github.com/docker/distribution/registry/api/errcode"
18 19
 	"github.com/docker/distribution/registry/client"
19 20
 	apitypes "github.com/docker/docker/api/types"
20 21
 	"github.com/docker/docker/distribution/metadata"
... ...
@@ -55,12 +56,14 @@ type pushState struct {
55 55
 	// confirmedV2 is set to true if we confirm we're talking to a v2
56 56
 	// registry. This is used to limit fallbacks to the v1 protocol.
57 57
 	confirmedV2 bool
58
+	hasAuthInfo bool
58 59
 }
59 60
 
60 61
 func (p *v2Pusher) Push(ctx context.Context) (err error) {
61 62
 	p.pushState.remoteLayers = make(map[layer.DiffID]distribution.Descriptor)
62 63
 
63 64
 	p.repo, p.pushState.confirmedV2, err = NewV2Repository(ctx, p.repoInfo, p.endpoint, p.config.MetaHeaders, p.config.AuthConfig, "push", "pull")
65
+	p.pushState.hasAuthInfo = p.config.AuthConfig.RegistryToken != "" || (p.config.AuthConfig.Username != "" && p.config.AuthConfig.Password != "")
64 66
 	if err != nil {
65 67
 		logrus.Debugf("Error getting v2 registry: %v", err)
66 68
 		return err
... ...
@@ -308,6 +311,7 @@ func (pd *v2PushDescriptor) Upload(ctx context.Context, progressOutput progress.
308 308
 
309 309
 	// Attempt to find another repository in the same registry to mount the layer from to avoid an unnecessary upload
310 310
 	candidates := getRepositoryMountCandidates(pd.repoInfo, pd.hmacKey, maxMountAttempts, v2Metadata)
311
+	isUnauthorizedError := false
311 312
 	for _, mountCandidate := range candidates {
312 313
 		logrus.Debugf("attempting to mount layer %s (%s) from %s", diffID, mountCandidate.Digest, mountCandidate.SourceRepository)
313 314
 		createOpts := []distribution.BlobCreateOption{}
... ...
@@ -360,11 +364,26 @@ func (pd *v2PushDescriptor) Upload(ctx context.Context, progressOutput progress.
360 360
 				return distribution.Descriptor{}, xfer.DoNotRetry{Err: err}
361 361
 			}
362 362
 			return err.Descriptor, nil
363
+		case errcode.Errors:
364
+			for _, e := range err {
365
+				switch e := e.(type) {
366
+				case errcode.Error:
367
+					if e.Code == errcode.ErrorCodeUnauthorized {
368
+						// when unauthorized error that indicate user don't has right to push layer to register
369
+						logrus.Debugln("failed to push layer to registry because unauthorized error")
370
+						isUnauthorizedError = true
371
+					}
372
+				default:
373
+				}
374
+			}
363 375
 		default:
364 376
 			logrus.Infof("failed to mount layer %s (%s) from %s: %v", diffID, mountCandidate.Digest, mountCandidate.SourceRepository, err)
365 377
 		}
366 378
 
379
+		// when error is unauthorizedError and user don't hasAuthInfo that's the case user don't has right to push layer to register
380
+		// and he hasn't login either, in this case candidate cache should be removed
367 381
 		if len(mountCandidate.SourceRepository) > 0 &&
382
+			!(isUnauthorizedError && !pd.pushState.hasAuthInfo) &&
368 383
 			(metadata.CheckV2MetadataHMAC(&mountCandidate, pd.hmacKey) ||
369 384
 				len(mountCandidate.HMAC) == 0) {
370 385
 			cause := "blob mount failure"
... ...
@@ -398,7 +417,6 @@ func (pd *v2PushDescriptor) Upload(ctx context.Context, progressOutput progress.
398 398
 		}
399 399
 	}
400 400
 	defer layerUpload.Close()
401
-
402 401
 	// upload the blob
403 402
 	return pd.uploadUsingSession(ctx, progressOutput, diffID, layerUpload)
404 403
 }
... ...
@@ -2,6 +2,7 @@ package distribution // import "github.com/docker/docker/distribution"
2 2
 
3 3
 import (
4 4
 	"net/http"
5
+	"net/url"
5 6
 	"reflect"
6 7
 	"testing"
7 8
 
... ...
@@ -9,9 +10,13 @@ import (
9 9
 	"github.com/docker/distribution/context"
10 10
 	"github.com/docker/distribution/manifest/schema2"
11 11
 	"github.com/docker/distribution/reference"
12
+	"github.com/docker/distribution/registry/api/errcode"
13
+	"github.com/docker/docker/api/types"
12 14
 	"github.com/docker/docker/distribution/metadata"
13 15
 	"github.com/docker/docker/layer"
14 16
 	"github.com/docker/docker/pkg/progress"
17
+	refstore "github.com/docker/docker/reference"
18
+	"github.com/docker/docker/registry"
15 19
 	"github.com/opencontainers/go-digest"
16 20
 )
17 21
 
... ...
@@ -461,6 +466,158 @@ func TestLayerAlreadyExists(t *testing.T) {
461 461
 	}
462 462
 }
463 463
 
464
+type mockReferenceStore struct {
465
+}
466
+
467
+func (s *mockReferenceStore) References(id digest.Digest) []reference.Named {
468
+	return []reference.Named{}
469
+}
470
+func (s *mockReferenceStore) ReferencesByName(ref reference.Named) []refstore.Association {
471
+	return []refstore.Association{}
472
+}
473
+func (s *mockReferenceStore) AddTag(ref reference.Named, id digest.Digest, force bool) error {
474
+	return nil
475
+}
476
+func (s *mockReferenceStore) AddDigest(ref reference.Canonical, id digest.Digest, force bool) error {
477
+	return nil
478
+}
479
+func (s *mockReferenceStore) Delete(ref reference.Named) (bool, error) {
480
+	return true, nil
481
+}
482
+func (s *mockReferenceStore) Get(ref reference.Named) (digest.Digest, error) {
483
+	return "", nil
484
+}
485
+
486
+func TestWhenEmptyAuthConfig(t *testing.T) {
487
+	for _, authInfo := range []struct {
488
+		username      string
489
+		password      string
490
+		registryToken string
491
+		expected      bool
492
+	}{
493
+		{
494
+			username:      "",
495
+			password:      "",
496
+			registryToken: "",
497
+			expected:      false,
498
+		},
499
+		{
500
+			username:      "username",
501
+			password:      "password",
502
+			registryToken: "",
503
+			expected:      true,
504
+		},
505
+		{
506
+			username:      "",
507
+			password:      "",
508
+			registryToken: "token",
509
+			expected:      true,
510
+		},
511
+	} {
512
+		imagePushConfig := &ImagePushConfig{}
513
+		imagePushConfig.AuthConfig = &types.AuthConfig{
514
+			Username:      authInfo.username,
515
+			Password:      authInfo.password,
516
+			RegistryToken: authInfo.registryToken,
517
+		}
518
+		imagePushConfig.ReferenceStore = &mockReferenceStore{}
519
+		repoInfo, _ := reference.ParseNormalizedNamed("xujihui1985/test.img")
520
+		pusher := &v2Pusher{
521
+			config: imagePushConfig,
522
+			repoInfo: &registry.RepositoryInfo{
523
+				Name: repoInfo,
524
+			},
525
+			endpoint: registry.APIEndpoint{
526
+				URL: &url.URL{
527
+					Scheme: "https",
528
+					Host:   "index.docker.io",
529
+				},
530
+				Version:      registry.APIVersion1,
531
+				TrimHostname: true,
532
+			},
533
+		}
534
+		pusher.Push(context.Background())
535
+		if pusher.pushState.hasAuthInfo != authInfo.expected {
536
+			t.Errorf("hasAuthInfo does not match expected: %t != %t", authInfo.expected, pusher.pushState.hasAuthInfo)
537
+		}
538
+	}
539
+}
540
+
541
+type mockBlobStoreWithCreate struct {
542
+	mockBlobStore
543
+	repo *mockRepoWithBlob
544
+}
545
+
546
+func (blob *mockBlobStoreWithCreate) Create(ctx context.Context, options ...distribution.BlobCreateOption) (distribution.BlobWriter, error) {
547
+	return nil, errcode.Errors(append([]error{errcode.ErrorCodeUnauthorized.WithMessage("unauthorized")}))
548
+}
549
+
550
+type mockRepoWithBlob struct {
551
+	mockRepo
552
+}
553
+
554
+func (m *mockRepoWithBlob) Blobs(ctx context.Context) distribution.BlobStore {
555
+	blob := &mockBlobStoreWithCreate{}
556
+	blob.mockBlobStore.repo = &m.mockRepo
557
+	blob.repo = m
558
+	return blob
559
+}
560
+
561
+type mockMetadataService struct {
562
+	mockV2MetadataService
563
+}
564
+
565
+func (m *mockMetadataService) GetMetadata(diffID layer.DiffID) ([]metadata.V2Metadata, error) {
566
+	return []metadata.V2Metadata{
567
+		taggedMetadata("abcd", "sha256:ff3a5c916c92643ff77519ffa742d3ec61b7f591b6b7504599d95a4a41134e28", "docker.io/user/app1"),
568
+		taggedMetadata("abcd", "sha256:ff3a5c916c92643ff77519ffa742d3ec61b7f591b6b7504599d95a4a41134e22", "docker.io/user/app/base"),
569
+		taggedMetadata("hash", "sha256:ff3a5c916c92643ff77519ffa742d3ec61b7f591b6b7504599d95a4a41134e23", "docker.io/user/app"),
570
+		taggedMetadata("abcd", "sha256:ff3a5c916c92643ff77519ffa742d3ec61b7f591b6b7504599d95a4a41134e24", "127.0.0.1/user/app"),
571
+		taggedMetadata("hash", "sha256:ff3a5c916c92643ff77519ffa742d3ec61b7f591b6b7504599d95a4a41134e25", "docker.io/user/foo"),
572
+		taggedMetadata("hash", "sha256:ff3a5c916c92643ff77519ffa742d3ec61b7f591b6b7504599d95a4a41134e26", "docker.io/app/bar"),
573
+	}, nil
574
+}
575
+
576
+var removeMetadata bool
577
+
578
+func (m *mockMetadataService) Remove(metadata metadata.V2Metadata) error {
579
+	removeMetadata = true
580
+	return nil
581
+}
582
+
583
+func TestPushRegistryWhenAuthInfoEmpty(t *testing.T) {
584
+	repoInfo, _ := reference.ParseNormalizedNamed("user/app")
585
+	ms := &mockMetadataService{}
586
+	remoteErrors := map[digest.Digest]error{digest.Digest("sha256:apple"): distribution.ErrAccessDenied}
587
+	remoteBlobs := map[digest.Digest]distribution.Descriptor{digest.Digest("sha256:apple"): {Digest: digest.Digest("shar256:apple")}}
588
+	repo := &mockRepoWithBlob{
589
+		mockRepo: mockRepo{
590
+			t:        t,
591
+			errors:   remoteErrors,
592
+			blobs:    remoteBlobs,
593
+			requests: []string{},
594
+		},
595
+	}
596
+	pd := &v2PushDescriptor{
597
+		hmacKey:  []byte("abcd"),
598
+		repoInfo: repoInfo,
599
+		layer: &storeLayer{
600
+			Layer: layer.EmptyLayer,
601
+		},
602
+		repo:              repo,
603
+		v2MetadataService: ms,
604
+		pushState: &pushState{
605
+			remoteLayers: make(map[layer.DiffID]distribution.Descriptor),
606
+			hasAuthInfo:  false,
607
+		},
608
+		checkedDigests: make(map[digest.Digest]struct{}),
609
+	}
610
+	pd.Upload(context.Background(), &progressSink{t})
611
+	if removeMetadata {
612
+		t.Fatalf("expect remove not be called but called")
613
+	}
614
+}
615
+
464 616
 func taggedMetadata(key string, dgst string, sourceRepo string) metadata.V2Metadata {
465 617
 	meta := metadata.V2Metadata{
466 618
 		Digest:           digest.Digest(dgst),