Merge pull request #18839 from aaronlehmann/v1-fallback-404

When a manifest is not found, allow fallback to v1

distribution/pull_v2.go

@@ -13,6 +13,7 @@ import (
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest/schema1"
+	"github.com/docker/distribution/registry/api/errcode"
 	"github.com/docker/docker/distribution/metadata"
 	"github.com/docker/docker/distribution/xfer"
 	"github.com/docker/docker/image"
@@ -209,6 +210,23 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named) (tagUpdat
 
 	unverifiedManifest, err := manSvc.GetByTag(tagOrDigest)
 	if err != nil {
+		// If this manifest did not exist, we should allow a possible
+		// 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
 	}
 	if unverifiedManifest == nil {

integration-cli/docker_cli_pull_local_test.go

@@ -228,3 +228,15 @@ func (s *DockerRegistrySuite) TestPullIDStability(c *check.C) {
 		c.Fatalf("expected %s; got %s", derivedImage, out)
 	}
 }
+
+// TestPullFallbackOn404 tries to pull a nonexistent manifest and confirms that
+// the pull falls back to the v1 protocol.
+//
+// Ref: docker/docker#18832
+func (s *DockerRegistrySuite) TestPullFallbackOn404(c *check.C) {
+	repoName := fmt.Sprintf("%v/does/not/exist", privateRegistryURL)
+
+	out, _, _ := dockerCmdWithError("pull", repoName)
+
+	c.Assert(out, checker.Contains, "v1 ping attempt")
+}

integration-cli/docker_cli_pull_test.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"fmt"
 	"regexp"
 	"strings"
 	"time"
@@ -53,8 +54,10 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) {
 	} {
 		out, err := s.CmdWithError("pull", e.Alias)
 		c.Assert(err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", out))
-		// Hub returns 401 rather than 404 for nonexistent library/ repos.
-		c.Assert(out, checker.Contains, "unauthorized: access to the requested resource is not authorized", check.Commentf("expected unauthorized error message"))
+		// Hub returns 401 rather than 404 for nonexistent repos over
+		// 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"))
 	}
 }
 

registry/registry.go

@@ -188,8 +188,8 @@ func addRequiredHeadersToRedirectedRequests(req *http.Request, via []*http.Reque
 	return nil
 }
 
-func shouldV2Fallback(err errcode.Error) bool {
-	logrus.Debugf("v2 error: %T %v", err, 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:
 		return true
@@ -220,7 +220,7 @@ func ContinueOnError(err error) bool {
 	case ErrNoSupport:
 		return ContinueOnError(v.Err)
 	case errcode.Error:
-		return shouldV2Fallback(v)
+		return ShouldV2Fallback(v)
 	case *client.UnexpectedHTTPResponseError:
 		return true
 	case error: