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: