From dd2d0c4792cf0bce15c82f04c85ca05473d4f391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=85=95=E9=99=B6?= Date: Wed, 7 Mar 2018 19:30:17 +0800 Subject: [PATCH] fix(distribution): digest cache should not be moved if it was an auth error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: 慕陶 Upstream-commit: 8b387b165ab2eaab3f9fdac25caa186d05d236a0 Component: engine --- components/engine/distribution/push_v2.go | 20 ++- .../engine/distribution/push_v2_test.go | 157 ++++++++++++++++++ 2 files changed, 176 insertions(+), 1 deletion(-) diff --git a/components/engine/distribution/push_v2.go b/components/engine/distribution/push_v2.go index 7b7155169d..f7b9a6d657 100644 --- a/components/engine/distribution/push_v2.go +++ b/components/engine/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) } diff --git a/components/engine/distribution/push_v2_test.go b/components/engine/distribution/push_v2_test.go index ac68470b64..c3616b936d 100644 --- a/components/engine/distribution/push_v2_test.go +++ b/components/engine/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: ®istry.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),