Merge pull request #36509 from xujihui1985/master

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

distribution/push_v2.go

@@ -15,6 +15,7 @@ import (
 	"github.com/docker/distribution/manifest/schema1"
 	"github.com/docker/distribution/manifest/schema2"
 	"github.com/docker/distribution/reference"
+	"github.com/docker/distribution/registry/api/errcode"
 	"github.com/docker/distribution/registry/client"
 	apitypes "github.com/docker/docker/api/types"
 	"github.com/docker/docker/distribution/metadata"
@@ -55,12 +56,14 @@ type pushState struct {
 	// confirmedV2 is set to true if we confirm we're talking to a v2
 	// registry. This is used to limit fallbacks to the v1 protocol.
 	confirmedV2 bool
+	hasAuthInfo bool
 }
 
 func (p *v2Pusher) Push(ctx context.Context) (err error) {
 	p.pushState.remoteLayers = make(map[layer.DiffID]distribution.Descriptor)
 
 	p.repo, p.pushState.confirmedV2, err = NewV2Repository(ctx, p.repoInfo, p.endpoint, p.config.MetaHeaders, p.config.AuthConfig, "push", "pull")
+	p.pushState.hasAuthInfo = p.config.AuthConfig.RegistryToken != "" || (p.config.AuthConfig.Username != "" && p.config.AuthConfig.Password != "")
 	if err != nil {
 		logrus.Debugf("Error getting v2 registry: %v", err)
 		return err
@@ -308,6 +311,7 @@ func (pd *v2PushDescriptor) Upload(ctx context.Context, progressOutput progress.
 
 	// Attempt to find another repository in the same registry to mount the layer from to avoid an unnecessary upload
 	candidates := getRepositoryMountCandidates(pd.repoInfo, pd.hmacKey, maxMountAttempts, v2Metadata)
+	isUnauthorizedError := false
 	for _, mountCandidate := range candidates {
 		logrus.Debugf("attempting to mount layer %s (%s) from %s", diffID, mountCandidate.Digest, mountCandidate.SourceRepository)
 		createOpts := []distribution.BlobCreateOption{}
@@ -360,11 +364,26 @@ func (pd *v2PushDescriptor) Upload(ctx context.Context, progressOutput progress.
 				return distribution.Descriptor{}, xfer.DoNotRetry{Err: err}
 			}
 			return err.Descriptor, nil
+		case errcode.Errors:
+			for _, e := range err {
+				switch e := e.(type) {
+				case errcode.Error:
+					if e.Code == errcode.ErrorCodeUnauthorized {
+						// when unauthorized error that indicate user don't has right to push layer to register
+						logrus.Debugln("failed to push layer to registry because unauthorized error")
+						isUnauthorizedError = true
+					}
+				default:
+				}
+			}
 		default:
 			logrus.Infof("failed to mount layer %s (%s) from %s: %v", diffID, mountCandidate.Digest, mountCandidate.SourceRepository, err)
 		}
 
+		// when error is unauthorizedError and user don't hasAuthInfo that's the case user don't has right to push layer to register
+		// and he hasn't login either, in this case candidate cache should be removed
 		if len(mountCandidate.SourceRepository) > 0 &&
+			!(isUnauthorizedError && !pd.pushState.hasAuthInfo) &&
 			(metadata.CheckV2MetadataHMAC(&mountCandidate, pd.hmacKey) ||
 				len(mountCandidate.HMAC) == 0) {
 			cause := "blob mount failure"
@@ -398,7 +417,6 @@ func (pd *v2PushDescriptor) Upload(ctx context.Context, progressOutput progress.
 		}
 	}
 	defer layerUpload.Close()
-
 	// upload the blob
 	return pd.uploadUsingSession(ctx, progressOutput, diffID, layerUpload)
 }

distribution/push_v2_test.go

@@ -2,6 +2,7 @@ package distribution // import "github.com/docker/docker/distribution"
 
 import (
 	"net/http"
+	"net/url"
 	"reflect"
 	"testing"
 
@@ -9,9 +10,13 @@ import (
 	"github.com/docker/distribution/context"
 	"github.com/docker/distribution/manifest/schema2"
 	"github.com/docker/distribution/reference"
+	"github.com/docker/distribution/registry/api/errcode"
+	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/distribution/metadata"
 	"github.com/docker/docker/layer"
 	"github.com/docker/docker/pkg/progress"
+	refstore "github.com/docker/docker/reference"
+	"github.com/docker/docker/registry"
 	"github.com/opencontainers/go-digest"
 )
 
@@ -461,6 +466,158 @@ func TestLayerAlreadyExists(t *testing.T) {
 	}
 }
 
+type mockReferenceStore struct {
+}
+
+func (s *mockReferenceStore) References(id digest.Digest) []reference.Named {
+	return []reference.Named{}
+}
+func (s *mockReferenceStore) ReferencesByName(ref reference.Named) []refstore.Association {
+	return []refstore.Association{}
+}
+func (s *mockReferenceStore) AddTag(ref reference.Named, id digest.Digest, force bool) error {
+	return nil
+}
+func (s *mockReferenceStore) AddDigest(ref reference.Canonical, id digest.Digest, force bool) error {
+	return nil
+}
+func (s *mockReferenceStore) Delete(ref reference.Named) (bool, error) {
+	return true, nil
+}
+func (s *mockReferenceStore) Get(ref reference.Named) (digest.Digest, error) {
+	return "", nil
+}
+
+func TestWhenEmptyAuthConfig(t *testing.T) {
+	for _, authInfo := range []struct {
+		username      string
+		password      string
+		registryToken string
+		expected      bool
+	}{
+		{
+			username:      "",
+			password:      "",
+			registryToken: "",
+			expected:      false,
+		},
+		{
+			username:      "username",
+			password:      "password",
+			registryToken: "",
+			expected:      true,
+		},
+		{
+			username:      "",
+			password:      "",
+			registryToken: "token",
+			expected:      true,
+		},
+	} {
+		imagePushConfig := &ImagePushConfig{}
+		imagePushConfig.AuthConfig = &types.AuthConfig{
+			Username:      authInfo.username,
+			Password:      authInfo.password,
+			RegistryToken: authInfo.registryToken,
+		}
+		imagePushConfig.ReferenceStore = &mockReferenceStore{}
+		repoInfo, _ := reference.ParseNormalizedNamed("xujihui1985/test.img")
+		pusher := &v2Pusher{
+			config: imagePushConfig,
+			repoInfo: &registry.RepositoryInfo{
+				Name: repoInfo,
+			},
+			endpoint: registry.APIEndpoint{
+				URL: &url.URL{
+					Scheme: "https",
+					Host:   "index.docker.io",
+				},
+				Version:      registry.APIVersion1,
+				TrimHostname: true,
+			},
+		}
+		pusher.Push(context.Background())
+		if pusher.pushState.hasAuthInfo != authInfo.expected {
+			t.Errorf("hasAuthInfo does not match expected: %t != %t", authInfo.expected, pusher.pushState.hasAuthInfo)
+		}
+	}
+}
+
+type mockBlobStoreWithCreate struct {
+	mockBlobStore
+	repo *mockRepoWithBlob
+}
+
+func (blob *mockBlobStoreWithCreate) Create(ctx context.Context, options ...distribution.BlobCreateOption) (distribution.BlobWriter, error) {
+	return nil, errcode.Errors(append([]error{errcode.ErrorCodeUnauthorized.WithMessage("unauthorized")}))
+}
+
+type mockRepoWithBlob struct {
+	mockRepo
+}
+
+func (m *mockRepoWithBlob) Blobs(ctx context.Context) distribution.BlobStore {
+	blob := &mockBlobStoreWithCreate{}
+	blob.mockBlobStore.repo = &m.mockRepo
+	blob.repo = m
+	return blob
+}
+
+type mockMetadataService struct {
+	mockV2MetadataService
+}
+
+func (m *mockMetadataService) GetMetadata(diffID layer.DiffID) ([]metadata.V2Metadata, error) {
+	return []metadata.V2Metadata{
+		taggedMetadata("abcd", "sha256:ff3a5c916c92643ff77519ffa742d3ec61b7f591b6b7504599d95a4a41134e28", "docker.io/user/app1"),
+		taggedMetadata("abcd", "sha256:ff3a5c916c92643ff77519ffa742d3ec61b7f591b6b7504599d95a4a41134e22", "docker.io/user/app/base"),
+		taggedMetadata("hash", "sha256:ff3a5c916c92643ff77519ffa742d3ec61b7f591b6b7504599d95a4a41134e23", "docker.io/user/app"),
+		taggedMetadata("abcd", "sha256:ff3a5c916c92643ff77519ffa742d3ec61b7f591b6b7504599d95a4a41134e24", "127.0.0.1/user/app"),
+		taggedMetadata("hash", "sha256:ff3a5c916c92643ff77519ffa742d3ec61b7f591b6b7504599d95a4a41134e25", "docker.io/user/foo"),
+		taggedMetadata("hash", "sha256:ff3a5c916c92643ff77519ffa742d3ec61b7f591b6b7504599d95a4a41134e26", "docker.io/app/bar"),
+	}, nil
+}
+
+var removeMetadata bool
+
+func (m *mockMetadataService) Remove(metadata metadata.V2Metadata) error {
+	removeMetadata = true
+	return nil
+}
+
+func TestPushRegistryWhenAuthInfoEmpty(t *testing.T) {
+	repoInfo, _ := reference.ParseNormalizedNamed("user/app")
+	ms := &mockMetadataService{}
+	remoteErrors := map[digest.Digest]error{digest.Digest("sha256:apple"): distribution.ErrAccessDenied}
+	remoteBlobs := map[digest.Digest]distribution.Descriptor{digest.Digest("sha256:apple"): {Digest: digest.Digest("shar256:apple")}}
+	repo := &mockRepoWithBlob{
+		mockRepo: mockRepo{
+			t:        t,
+			errors:   remoteErrors,
+			blobs:    remoteBlobs,
+			requests: []string{},
+		},
+	}
+	pd := &v2PushDescriptor{
+		hmacKey:  []byte("abcd"),
+		repoInfo: repoInfo,
+		layer: &storeLayer{
+			Layer: layer.EmptyLayer,
+		},
+		repo:              repo,
+		v2MetadataService: ms,
+		pushState: &pushState{
+			remoteLayers: make(map[layer.DiffID]distribution.Descriptor),
+			hasAuthInfo:  false,
+		},
+		checkedDigests: make(map[digest.Digest]struct{}),
+	}
+	pd.Upload(context.Background(), &progressSink{t})
+	if removeMetadata {
+		t.Fatalf("expect remove not be called but called")
+	}
+}
+
 func taggedMetadata(key string, dgst string, sourceRepo string) metadata.V2Metadata {
 	meta := metadata.V2Metadata{
 		Digest:           digest.Digest(dgst),