Allow v1 protocol fallback when pulling all tags from a repository unknown to v2 registry

This is a followup to #18839. That PR relaxed the fallback logic so that
if a manifest doesn't exist on v2, or the user is unauthorized to access
it, we try again with the v1 protocol. A similar special case is needed
for "pull all tags" (docker pull -a). If the v2 registry doesn't
recognize the repository, or doesn't allow the user to access it, we
should fall back to v1 and try to pull all tags from the v1 registry.
Conversely, if the v2 registry does allow us to list the tags, there
should be no fallback, even if there are errors pulling those tags.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

Author: aaronlehmann

distribution/pull_v2.go

@@ -47,6 +47,9 @@ func (p *v2Puller) Pull(ctx context.Context, ref reference.Named) (err error) {
 	}
 
 	if err = p.pullV2Repository(ctx, ref); err != nil {
+		if _, ok := err.(fallbackError); ok {
+			return err
+		}
 		if registry.ContinueOnError(err) {
 			logrus.Debugf("Error trying v2 registry: %v", err)
 			return fallbackError{err: err, confirmedV2: p.confirmedV2}
@@ -56,9 +59,13 @@ func (p *v2Puller) Pull(ctx context.Context, ref reference.Named) (err error) {
 }
 
 func (p *v2Puller) pullV2Repository(ctx context.Context, ref reference.Named) (err error) {
-	var refs []reference.Named
+	var layersDownloaded bool
 	if !reference.IsNameOnly(ref) {
-		refs = []reference.Named{ref}
+		var err error
+		layersDownloaded, err = p.pullV2Tag(ctx, ref)
+		if err != nil {
+			return err
+		}
 	} else {
 		manSvc, err := p.repo.Manifests(ctx)
 		if err != nil {
@@ -67,11 +74,14 @@ func (p *v2Puller) pullV2Repository(ctx context.Context, ref reference.Named) (e
 
 		tags, err := manSvc.Tags()
 		if err != nil {
-			return err
+			// If this repository doesn't exist on V2, we should
+			// permit a fallback to V1.
+			return allowV1Fallback(err)
 		}
 
-		// If this call succeeded, we can be confident that the
-		// registry on the other side speaks the v2 protocol.
+		// 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
 
 		// This probably becomes a lot nicer after the manifest
@@ -81,19 +91,20 @@ func (p *v2Puller) pullV2Repository(ctx context.Context, ref reference.Named) (e
 			if err != nil {
 				return err
 			}
-			refs = append(refs, tagRef)
-		}
-	}
-
-	var layersDownloaded bool
-	for _, pullRef := range refs {
-		// pulledNew is true if either new layers were downloaded OR if existing images were newly tagged
-		// TODO(tiborvass): should we change the name of `layersDownload`? What about message in WriteStatus?
-		pulledNew, err := p.pullV2Tag(ctx, pullRef)
-		if err != nil {
-			return err
+			pulledNew, err := p.pullV2Tag(ctx, tagRef)
+			if err != nil {
+				// Since this is the pull-all-tags case, don't
+				// allow an error pulling a particular tag to
+				// make the whole pull fall back to v1.
+				if fallbackErr, ok := err.(fallbackError); ok {
+					return fallbackErr.err
+				}
+				return err
+			}
+			// pulledNew is true if either new layers were downloaded OR if existing images were newly tagged
+			// TODO(tiborvass): should we change the name of `layersDownload`? What about message in WriteStatus?
+			layersDownloaded = layersDownloaded || pulledNew
 		}
-		layersDownloaded = layersDownloaded || pulledNew
 	}
 
 	writeStatus(ref.String(), p.config.ProgressOutput, layersDownloaded)
@@ -214,20 +225,7 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named) (tagUpdat
 		// fallback to the v1 protocol, because dual-version setups may
 		// not host all manifests with the v2 protocol. We may also get
 		// a "not authorized" error if the manifest doesn't exist.
-		switch v := err.(type) {
-		case errcode.Errors:
-			if len(v) != 0 {
-				if v0, ok := v[0].(errcode.Error); ok && registry.ShouldV2Fallback(v0) {
-					p.confirmedV2 = false
-				}
-			}
-		case errcode.Error:
-			if registry.ShouldV2Fallback(v) {
-				p.confirmedV2 = false
-			}
-		}
-
-		return false, err
+		return false, allowV1Fallback(err)
 	}
 	if unverifiedManifest == nil {
 		return false, fmt.Errorf("image manifest does not exist for tag or digest %q", tagOrDigest)
@@ -334,6 +332,27 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named) (tagUpdat
 	return true, 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 && registry.ShouldV2Fallback(v0) {
+				return fallbackError{err: err, confirmedV2: false}
+			}
+		}
+	case errcode.Error:
+		if registry.ShouldV2Fallback(v) {
+			return fallbackError{err: err, confirmedV2: false}
+		}
+	}
+
+	return err
+}
+
 func verifyManifest(signedManifest *schema1.SignedManifest, ref reference.Named) (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

integration-cli/docker_cli_pull_test.go

@@ -58,7 +58,15 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) {
 		// the v2 protocol - but we should end up falling back to v1,
 		// which does return a 404.
 		c.Assert(out, checker.Contains, fmt.Sprintf("Error: image %s not found", e.Repo), check.Commentf("expected image not found error messages"))
+
+		// pull -a on a nonexistent registry should fall back as well
+		if !strings.ContainsRune(e.Alias, ':') {
+			out, err := s.CmdWithError("pull", "-a", e.Alias)
+			c.Assert(err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", out))
+			c.Assert(out, checker.Contains, fmt.Sprintf("Error: image %s not found", e.Repo), check.Commentf("expected image not found error messages"))
+		}
 	}
+
 }
 
 // TestPullFromCentralRegistryImplicitRefParts pulls an image from the central registry and verifies

registry/registry.go

@@ -191,7 +191,7 @@ func addRequiredHeadersToRedirectedRequests(req *http.Request, via []*http.Reque
 // 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:
+	case errcode.ErrorCodeUnauthorized, v2.ErrorCodeManifestUnknown, v2.ErrorCodeNameUnknown:
 		return true
 	}
 	return false