Upstream-commit: ece4cd4c4d9e0a8353a5b04a89ecbb6d477341b9

Component: engine

Author: cpuguy83

components/engine/api/server/router/distribution/backend.go

@@ -11,5 +11,5 @@ import (
 // Backend is all the methods that need to be implemented
 // to provide image specific functionality.
 type Backend interface {
-	GetRepository(context.Context, reference.Named, *types.AuthConfig) (distribution.Repository, bool, error)
+	GetRepository(context.Context, reference.Named, *types.AuthConfig) (distribution.Repository, error)
 }

components/engine/api/server/router/distribution/distribution_routes.go

@@ -57,7 +57,7 @@ func (s *distributionRouter) getDistributionInfo(ctx context.Context, w http.Res
 		return errdefs.InvalidParameter(errors.Errorf("unknown image reference format: %s", image))
 	}
 
-	distrepo, _, err := s.backend.GetRepository(ctx, namedRef, config)
+	distrepo, err := s.backend.GetRepository(ctx, namedRef, config)
 	if err != nil {
 		return err
 	}

components/engine/daemon/cluster/executor/backend.go

@@ -72,6 +72,6 @@ type VolumeBackend interface {
 // ImageBackend is used by an executor to perform image operations
 type ImageBackend interface {
 	PullImage(ctx context.Context, image, tag string, platform *specs.Platform, metaHeaders map[string][]string, authConfig *types.AuthConfig, outStream io.Writer) error
-	GetRepository(context.Context, reference.Named, *types.AuthConfig) (distribution.Repository, bool, error)
+	GetRepository(context.Context, reference.Named, *types.AuthConfig) (distribution.Repository, error)
 	LookupImage(name string) (*types.ImageInspect, error)
 }

components/engine/daemon/cluster/services.go

@@ -633,7 +633,7 @@ func (c *Cluster) imageWithDigestString(ctx context.Context, image string, authC
 			return "", errors.Errorf("image reference not tagged: %s", image)
 		}
 
-		repo, _, err := c.config.ImageBackend.GetRepository(ctx, taggedRef, authConfig)
+		repo, err := c.config.ImageBackend.GetRepository(ctx, taggedRef, authConfig)
 		if err != nil {
 			return "", err
 		}

components/engine/daemon/images/image_pull.go

@@ -15,7 +15,6 @@ import (
 	progressutils "github.com/docker/docker/distribution/utils"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/pkg/progress"
-	"github.com/docker/docker/registry"
 	digest "github.com/opencontainers/go-digest"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
@@ -110,41 +109,36 @@ func (i *ImageService) pullImageWithReference(ctx context.Context, ref reference
 }
 
 // GetRepository returns a repository from the registry.
-func (i *ImageService) GetRepository(ctx context.Context, ref reference.Named, authConfig *types.AuthConfig) (dist.Repository, bool, error) {
+func (i *ImageService) GetRepository(ctx context.Context, ref reference.Named, authConfig *types.AuthConfig) (dist.Repository, error) {
 	// get repository info
 	repoInfo, err := i.registryService.ResolveRepository(ref)
 	if err != nil {
-		return nil, false, errdefs.InvalidParameter(err)
+		return nil, errdefs.InvalidParameter(err)
 	}
 	// makes sure name is not empty or `scratch`
 	if err := distribution.ValidateRepoName(repoInfo.Name); err != nil {
-		return nil, false, errdefs.InvalidParameter(err)
+		return nil, errdefs.InvalidParameter(err)
 	}
 
 	// get endpoints
 	endpoints, err := i.registryService.LookupPullEndpoints(reference.Domain(repoInfo.Name))
 	if err != nil {
-		return nil, false, err
+		return nil, err
 	}
 
 	// retrieve repository
 	var (
-		confirmedV2 bool
-		repository  dist.Repository
-		lastError   error
+		repository dist.Repository
+		lastError  error
 	)
 
 	for _, endpoint := range endpoints {
-		if endpoint.Version == registry.APIVersion1 {
-			continue
-		}
-
-		repository, confirmedV2, lastError = distribution.NewV2Repository(ctx, repoInfo, endpoint, nil, authConfig, "pull")
-		if lastError == nil && confirmedV2 {
+		repository, lastError = distribution.NewV2Repository(ctx, repoInfo, endpoint, nil, authConfig, "pull")
+		if lastError == nil {
 			break
 		}
 	}
-	return repository, confirmedV2, lastError
+	return repository, lastError
 }
 
 func tempLease(ctx context.Context, mgr leases.Manager) (context.Context, func(context.Context) error, error) {

components/engine/distribution/errors.go

@@ -34,10 +34,6 @@ func (e ErrNoSupport) Error() string {
 type fallbackError struct {
 	// err is the error being wrapped.
 	err error
-	// confirmedV2 is set to true if it was confirmed that the registry
-	// supports the v2 protocol. This is used to limit fallbacks to the v1
-	// protocol.
-	confirmedV2 bool
 	// transportOK is set to true if we managed to speak HTTP with the
 	// registry. This confirms that we're using appropriate TLS settings
 	// (or lack of TLS).
@@ -53,15 +49,6 @@ func (f fallbackError) Cause() error {
 	return f.err
 }
 
-// shouldV2Fallback returns true if this error is a reason to fall back to v1.
-func shouldV2Fallback(err errcode.Error) bool {
-	switch err.Code {
-	case errcode.ErrorCodeUnauthorized, v2.ErrorCodeManifestUnknown, v2.ErrorCodeNameUnknown:
-		return true
-	}
-	return false
-}
-
 type notFoundError struct {
 	cause errcode.Error
 	ref   reference.Named
@@ -141,7 +128,7 @@ func continueOnError(err error, mirrorEndpoint bool) bool {
 	case ErrNoSupport:
 		return continueOnError(v.Err, mirrorEndpoint)
 	case errcode.Error:
-		return mirrorEndpoint || shouldV2Fallback(v)
+		return mirrorEndpoint
 	case *client.UnexpectedHTTPResponseError:
 		return true
 	case ImageConfigPullError:

components/engine/distribution/errors_test.go

@@ -7,26 +7,26 @@ import (
 	"testing"
 
 	"github.com/docker/distribution/registry/api/errcode"
-	v2 "github.com/docker/distribution/registry/api/v2"
 	"github.com/docker/distribution/registry/client"
 )
 
+var errUnexpected = errors.New("some totally unexpected error")
+
 var alwaysContinue = []error{
 	&client.UnexpectedHTTPResponseError{},
-
-	// Some errcode.Errors that don't disprove the existence of a V1 image
-	errcode.Error{Code: errcode.ErrorCodeUnauthorized},
-	errcode.Error{Code: v2.ErrorCodeManifestUnknown},
-	errcode.Error{Code: v2.ErrorCodeNameUnknown},
-
-	errors.New("some totally unexpected error"),
+	errcode.Errors{},
+	errUnexpected,
+	// nested
+	errcode.Errors{errUnexpected},
+	ErrNoSupport{Err: errUnexpected},
 }
 
 var continueFromMirrorEndpoint = []error{
 	ImageConfigPullError{},
-
-	// Some other errcode.Error that doesn't indicate we should search for a V1 image.
-	errcode.Error{Code: errcode.ErrorCodeTooManyRequests},
+	errcode.Error{},
+	// nested
+	errcode.Errors{errcode.Error{}},
+	ErrNoSupport{Err: errcode.Error{}},
 }
 
 var neverContinue = []error{
@@ -67,19 +67,3 @@ func TestContinueOnError_NeverContinue(t *testing.T) {
 		}
 	}
 }
-
-func TestContinueOnError_UnnestsErrors(t *testing.T) {
-	// ContinueOnError should evaluate nested errcode.Errors.
-
-	// Assumes that v2.ErrorCodeNameUnknown is a continueable error code.
-	err := errcode.Errors{errcode.Error{Code: v2.ErrorCodeNameUnknown}}
-	if !continueOnError(err, false) {
-		t.Fatal("ContinueOnError should unnest, base return value on errcode.Errors")
-	}
-
-	// Assumes that errcode.ErrorCodeTooManyRequests is not a V1-fallback indication
-	err = errcode.Errors{errcode.Error{Code: errcode.ErrorCodeTooManyRequests}}
-	if continueOnError(err, false) {
-		t.Fatal("ContinueOnError should unnest, base return value on errcode.Errors")
-	}
-}

components/engine/distribution/pull.go

@@ -24,11 +24,7 @@ type Puller interface {
 	Pull(ctx context.Context, ref reference.Named, platform *specs.Platform) error
 }
 
-// newPuller returns a Puller interface that will pull from either a v1 or v2
-// registry. The endpoint argument contains a Version field that determines
-// whether a v1 or v2 puller will be created. The other parameters are passed
-// through to the underlying puller implementation for use during the actual
-// pull operation.
+// newPuller returns a Puller interface that will pull from a v2 registry.
 func newPuller(endpoint registry.APIEndpoint, repoInfo *registry.RepositoryInfo, imagePullConfig *ImagePullConfig, local ContentStore) (Puller, error) {
 	switch endpoint.Version {
 	case registry.APIVersion2:
@@ -78,26 +74,12 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo
 		// error is the ones from v2 endpoints not v1.
 		discardNoSupportErrors bool
 
-		// confirmedV2 is set to true if a pull attempt managed to
-		// confirm that it was talking to a v2 registry. This will
-		// prevent fallback to the v1 protocol.
-		confirmedV2 bool
-
 		// confirmedTLSRegistries is a map indicating which registries
 		// are known to be using TLS. There should never be a plaintext
 		// retry for any of these.
 		confirmedTLSRegistries = make(map[string]struct{})
 	)
 	for _, endpoint := range endpoints {
-		if imagePullConfig.RequireSchema2 && endpoint.Version == registry.APIVersion1 {
-			continue
-		}
-
-		if confirmedV2 && endpoint.Version == registry.APIVersion1 {
-			logrus.Debugf("Skipping v1 endpoint %s because v2 registry was detected", endpoint.URL)
-			continue
-		}
-
 		if endpoint.URL.Scheme != "https" {
 			if _, confirmedTLS := confirmedTLSRegistries[endpoint.URL.Host]; confirmedTLS {
 				logrus.Debugf("Skipping non-TLS endpoint %s for host/port that appears to use TLS", endpoint.URL)
@@ -122,7 +104,6 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo
 			default:
 				if fallbackErr, ok := err.(fallbackError); ok {
 					fallback = true
-					confirmedV2 = confirmedV2 || fallbackErr.confirmedV2
 					if fallbackErr.transportOK && endpoint.URL.Scheme == "https" {
 						confirmedTLSRegistries[endpoint.URL.Host] = struct{}{}
 					}

components/engine/distribution/pull_v2.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"net/url"
 	"os"
 	"runtime"
 	"strings"
@@ -19,8 +18,6 @@ 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/auth"
 	"github.com/docker/distribution/registry/client/transport"
 	"github.com/docker/docker/distribution/metadata"
 	"github.com/docker/docker/distribution/xfer"
@@ -61,15 +58,12 @@ type v2Puller struct {
 	config            *ImagePullConfig
 	repoInfo          *registry.RepositoryInfo
 	repo              distribution.Repository
-	// 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
-	manifestStore *manifestStore
+	manifestStore     *manifestStore
 }
 
 func (p *v2Puller) Pull(ctx context.Context, ref reference.Named, platform *specs.Platform) (err error) {
 	// TODO(tiborvass): was ReceiveTimeout
-	p.repo, p.confirmedV2, err = NewV2Repository(ctx, p.repoInfo, p.endpoint, p.config.MetaHeaders, p.config.AuthConfig, "pull")
+	p.repo, err = NewV2Repository(ctx, p.repoInfo, p.endpoint, p.config.MetaHeaders, p.config.AuthConfig, "pull")
 	if err != nil {
 		logrus.Warnf("Error getting v2 registry: %v", err)
 		return err
@@ -87,7 +81,6 @@ func (p *v2Puller) Pull(ctx context.Context, ref reference.Named, platform *spec
 		if continueOnError(err, p.endpoint.Mirror) {
 			return fallbackError{
 				err:         err,
-				confirmedV2: p.confirmedV2,
 				transportOK: true,
 			}
 		}
@@ -105,16 +98,9 @@ func (p *v2Puller) pullV2Repository(ctx context.Context, ref reference.Named, pl
 	} else {
 		tags, err := p.repo.Tags(ctx).All(ctx)
 		if err != nil {
-			// If this repository doesn't exist on V2, we should
-			// permit a fallback to V1.
-			return allowV1Fallback(err)
+			return err
 		}
 
-		// The v2 registry knows about this repository, so we will not
-		// allow fallback to the v1 protocol even if we encounter an
-		// error later on.
-		p.confirmedV2 = true
-
 		for _, tag := range tags {
 			tagRef, err := reference.WithTag(ref, tag)
 			if err != nil {
@@ -353,7 +339,7 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named, platform
 		tagService := p.repo.Tags(ctx)
 		desc, err := tagService.Get(ctx, tagged.Tag())
 		if err != nil {
-			return false, allowV1Fallback(err)
+			return false, err
 		}
 
 		dgst = desc.Digest
@@ -427,10 +413,6 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named, platform
 		}
 	}
 
-	// If manSvc.Get succeeded, we can be confident that the registry on
-	// the other side speaks the v2 protocol.
-	p.confirmedV2 = true
-
 	logrus.Debugf("Pulling ref from V2 registry: %s", reference.FamiliarString(ref))
 	progress.Message(p.config.ProgressOutput, tagOrDigest, "Pulling from "+reference.FamiliarName(p.repo.Named()))
 
@@ -945,39 +927,6 @@ func schema2ManifestDigest(ref reference.Named, mfst distribution.Manifest) (dig
 	return digest.FromBytes(canonical), nil
 }
 
-// allowV1Fallback checks if the error is a possible reason to fallback to v1
-// (even if confirmedV2 has been set already), and if so, wraps the error in
-// a fallbackError with confirmedV2 set to false. Otherwise, it returns the
-// error unmodified.
-func allowV1Fallback(err error) error {
-	switch v := err.(type) {
-	case errcode.Errors:
-		if len(v) != 0 {
-			if v0, ok := v[0].(errcode.Error); ok && shouldV2Fallback(v0) {
-				return fallbackError{
-					err:         err,
-					confirmedV2: false,
-					transportOK: true,
-				}
-			}
-		}
-	case errcode.Error:
-		if shouldV2Fallback(v) {
-			return fallbackError{
-				err:         err,
-				confirmedV2: false,
-				transportOK: true,
-			}
-		}
-	case *url.Error:
-		if v.Err == auth.ErrNoBasicAuthCredentials {
-			return fallbackError{err: err, confirmedV2: false}
-		}
-	}
-
-	return err
-}
-
 func verifySchema1Manifest(signedManifest *schema1.SignedManifest, ref reference.Reference) (m *schema1.Manifest, err error) {
 	// If pull by digest, then verify the manifest digest. NOTE: It is
 	// important to do this first, before any other content validation. If the

components/engine/distribution/push.go

@@ -73,26 +73,13 @@ func Push(ctx context.Context, ref reference.Named, imagePushConfig *ImagePushCo
 	var (
 		lastErr error
 
-		// confirmedV2 is set to true if a push attempt managed to
-		// confirm that it was talking to a v2 registry. This will
-		// prevent fallback to the v1 protocol.
-		confirmedV2 bool
-
 		// confirmedTLSRegistries is a map indicating which registries
 		// are known to be using TLS. There should never be a plaintext
 		// retry for any of these.
 		confirmedTLSRegistries = make(map[string]struct{})
 	)
 
 	for _, endpoint := range endpoints {
-		if imagePushConfig.RequireSchema2 && endpoint.Version == registry.APIVersion1 {
-			continue
-		}
-		if confirmedV2 && endpoint.Version == registry.APIVersion1 {
-			logrus.Debugf("Skipping v1 endpoint %s because v2 registry was detected", endpoint.URL)
-			continue
-		}
-
 		if endpoint.URL.Scheme != "https" {
 			if _, confirmedTLS := confirmedTLSRegistries[endpoint.URL.Host]; confirmedTLS {
 				logrus.Debugf("Skipping non-TLS endpoint %s for host/port that appears to use TLS", endpoint.URL)
@@ -114,7 +101,6 @@ func Push(ctx context.Context, ref reference.Named, imagePushConfig *ImagePushCo
 			case <-ctx.Done():
 			default:
 				if fallbackErr, ok := err.(fallbackError); ok {
-					confirmedV2 = confirmedV2 || fallbackErr.confirmedV2
 					if fallbackErr.transportOK && endpoint.URL.Scheme == "https" {
 						confirmedTLSRegistries[endpoint.URL.Host] = struct{}{}
 					}