Browse code

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>

Aaron Lehmann authored on 2015/12/22 08:42:04
Showing 4 changed files
... ...
@@ -13,6 +13,7 @@ import (
13 13
 	"github.com/docker/distribution"
14 14
 	"github.com/docker/distribution/digest"
15 15
 	"github.com/docker/distribution/manifest/schema1"
16
+	"github.com/docker/distribution/registry/api/errcode"
16 17
 	"github.com/docker/docker/distribution/metadata"
17 18
 	"github.com/docker/docker/distribution/xfer"
18 19
 	"github.com/docker/docker/image"
... ...
@@ -209,6 +210,23 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named) (tagUpdat
209 209
 
210 210
 	unverifiedManifest, err := manSvc.GetByTag(tagOrDigest)
211 211
 	if err != nil {
212
+		// If this manifest did not exist, we should allow a possible
213
+		// fallback to the v1 protocol, because dual-version setups may
214
+		// not host all manifests with the v2 protocol. We may also get
215
+		// a "not authorized" error if the manifest doesn't exist.
216
+		switch v := err.(type) {
217
+		case errcode.Errors:
218
+			if len(v) != 0 {
219
+				if v0, ok := v[0].(errcode.Error); ok && registry.ShouldV2Fallback(v0) {
220
+					p.confirmedV2 = false
221
+				}
222
+			}
223
+		case errcode.Error:
224
+			if registry.ShouldV2Fallback(v) {
225
+				p.confirmedV2 = false
226
+			}
227
+		}
228
+
212 229
 		return false, err
213 230
 	}
214 231
 	if unverifiedManifest == nil {
... ...
@@ -228,3 +228,15 @@ func (s *DockerRegistrySuite) TestPullIDStability(c *check.C) {
228 228
 		c.Fatalf("expected %s; got %s", derivedImage, out)
229 229
 	}
230 230
 }
231
+
232
+// TestPullFallbackOn404 tries to pull a nonexistent manifest and confirms that
233
+// the pull falls back to the v1 protocol.
234
+//
235
+// Ref: docker/docker#18832
236
+func (s *DockerRegistrySuite) TestPullFallbackOn404(c *check.C) {
237
+	repoName := fmt.Sprintf("%v/does/not/exist", privateRegistryURL)
238
+
239
+	out, _, _ := dockerCmdWithError("pull", repoName)
240
+
241
+	c.Assert(out, checker.Contains, "v1 ping attempt")
242
+}
... ...
@@ -1,6 +1,7 @@
1 1
 package main
2 2
 
3 3
 import (
4
+	"fmt"
4 5
 	"regexp"
5 6
 	"strings"
6 7
 	"time"
... ...
@@ -53,8 +54,10 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) {
53 53
 	} {
54 54
 		out, err := s.CmdWithError("pull", e.Alias)
55 55
 		c.Assert(err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", out))
56
-		// Hub returns 401 rather than 404 for nonexistent library/ repos.
57
-		c.Assert(out, checker.Contains, "unauthorized: access to the requested resource is not authorized", check.Commentf("expected unauthorized error message"))
56
+		// Hub returns 401 rather than 404 for nonexistent repos over
57
+		// the v2 protocol - but we should end up falling back to v1,
58
+		// which does return a 404.
59
+		c.Assert(out, checker.Contains, fmt.Sprintf("Error: image %s not found", e.Repo), check.Commentf("expected image not found error messages"))
58 60
 	}
59 61
 }
60 62
 
... ...
@@ -188,8 +188,8 @@ func addRequiredHeadersToRedirectedRequests(req *http.Request, via []*http.Reque
188 188
 	return nil
189 189
 }
190 190
 
191
-func shouldV2Fallback(err errcode.Error) bool {
192
-	logrus.Debugf("v2 error: %T %v", err, err)
191
+// ShouldV2Fallback returns true if this error is a reason to fall back to v1.
192
+func ShouldV2Fallback(err errcode.Error) bool {
193 193
 	switch err.Code {
194 194
 	case errcode.ErrorCodeUnauthorized, v2.ErrorCodeManifestUnknown:
195 195
 		return true
... ...
@@ -220,7 +220,7 @@ func ContinueOnError(err error) bool {
220 220
 	case ErrNoSupport:
221 221
 		return ContinueOnError(v.Err)
222 222
 	case errcode.Error:
223
-		return shouldV2Fallback(v)
223
+		return ShouldV2Fallback(v)
224 224
 	case *client.UnexpectedHTTPResponseError:
225 225
 		return true
226 226
 	case error: