From 9d6acbee92016c47796ee8751dce9c59056f850d Mon Sep 17 00:00:00 2001
From: Aaron Lehmann <aaron.lehmann@docker.com>
Date: Mon, 21 Dec 2015 15:42:04 -0800
Subject: [PATCH] When a manifest is not found, allow fallback to v1

PR #18590 caused compatibility issues with registries such as gcr.io
which support both the v1 and v2 protocols, but do not provide the same
set of images over both protocols. After #18590, pulls from these
registries would never use the v1 protocol, because of the
Docker-Distribution-Api-Version header indicating that v2 was supported.

Fix the problem by making an exception for the case where a manifest is
not found. This should allow fallback to v1 in case that image is
exposed over the v1 protocol but not the v2 protocol.

This avoids the overly aggressive fallback behavior before #18590 which
would allow protocol fallback after almost any error, but restores
interoperability with mixed v1/v2 registry setups.

Fixes #18832

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
---
 distribution/pull_v2.go                       | 18 ++++++++++++++++++
 integration-cli/docker_cli_pull_local_test.go | 12 ++++++++++++
 integration-cli/docker_cli_pull_test.go       |  7 +++++--
 registry/registry.go                          |  6 +++---
 4 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/distribution/pull_v2.go b/distribution/pull_v2.go
index 3422b2ef38..45493a736d 100644
--- a/distribution/pull_v2.go
+++ b/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 {
diff --git a/integration-cli/docker_cli_pull_local_test.go b/integration-cli/docker_cli_pull_local_test.go
index 64d3214b63..8e7da2fa6e 100644
--- a/integration-cli/docker_cli_pull_local_test.go
+++ b/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")
+}
diff --git a/integration-cli/docker_cli_pull_test.go b/integration-cli/docker_cli_pull_test.go
index eb74c6fbe1..b1ea5a8aa2 100644
--- a/integration-cli/docker_cli_pull_test.go
+++ b/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"))
 	}
 }
 
diff --git a/registry/registry.go b/registry/registry.go
index fc2959a5d5..ba1626c137 100644
--- a/registry/registry.go
+++ b/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: