From 497d545093bce4f01455bf8d2e1658435dbb040b Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Sat, 12 Mar 2016 18:01:01 +0100 Subject: [PATCH 1/2] distribution: errors: do not retry if no credentials provided Fix and add test for case c) in #21054 Signed-off-by: Antonio Murdaca --- distribution/errors.go | 4 ++++ integration-cli/docker_cli_push_test.go | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/distribution/errors.go b/distribution/errors.go index 1cb34fdd51..1a2923a8cf 100644 --- a/distribution/errors.go +++ b/distribution/errors.go @@ -8,6 +8,7 @@ import ( "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/registry/client" + "github.com/docker/distribution/registry/client/auth" "github.com/docker/docker/distribution/xfer" ) @@ -90,6 +91,9 @@ func retryOnError(err error) error { return xfer.DoNotRetry{Err: err} } case *url.Error: + if v.Err == auth.ErrNoBasicAuthCredentials { + return xfer.DoNotRetry{Err: v.Err} + } return retryOnError(v.Err) case *client.UnexpectedHTTPResponseError: return xfer.DoNotRetry{Err: err} diff --git a/integration-cli/docker_cli_push_test.go b/integration-cli/docker_cli_push_test.go index ee91abfb2f..8c05342d27 100644 --- a/integration-cli/docker_cli_push_test.go +++ b/integration-cli/docker_cli_push_test.go @@ -526,3 +526,12 @@ func (s *DockerTrustSuite) TestTrustedPushWithReleasesDelegation(c *check.C) { c.Assert(err, check.IsNil, check.Commentf("Unable to read targets/releases metadata")) c.Assert(string(contents), checker.Contains, `"latest"`, check.Commentf(string(contents))) } + +func (s *DockerRegistryAuthSuite) TestPushNoCredentialsNoRetry(c *check.C) { + repoName := fmt.Sprintf("%s/busybox", privateRegistryURL) + dockerCmd(c, "tag", "busybox", repoName) + out, _, err := dockerCmdWithError("push", repoName) + c.Assert(err, check.NotNil, check.Commentf(out)) + c.Assert(out, check.Not(checker.Contains), "Retrying") + c.Assert(out, checker.Contains, "no basic auth credentials") +} From 6d23c3c57a733436d902b7883f2f0b00846df9e2 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Sat, 12 Mar 2016 20:34:07 +0100 Subject: [PATCH 2/2] integration-cli: add tests for case a) and d) in #21054 - add test for pull from private registry with no credentials - add test for push to docker hub with no credentials Signed-off-by: Antonio Murdaca Signed-off-by: Antonio Murdaca Signed-off-by: Antonio Murdaca --- integration-cli/docker_cli_pull_test.go | 9 +++++++++ integration-cli/docker_cli_push_test.go | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/integration-cli/docker_cli_pull_test.go b/integration-cli/docker_cli_pull_test.go index 43369c8ed6..8722405a43 100644 --- a/integration-cli/docker_cli_pull_test.go +++ b/integration-cli/docker_cli_pull_test.go @@ -254,3 +254,12 @@ func (s *DockerHubPullSuite) TestPullClientDisconnect(c *check.C) { _, err = s.CmdWithError("inspect", repoName) c.Assert(err, checker.NotNil, check.Commentf("image was pulled after client disconnected")) } + +func (s *DockerRegistryAuthSuite) TestPullNoCredentialsNotFound(c *check.C) { + // we don't care about the actual image, we just want to see image not found + // because that means v2 call returned 401 and we fell back to v1 which usually + // gives a 404 (in this case the test registry doesn't handle v1 at all) + out, _, err := dockerCmdWithError("pull", privateRegistryURL+"/busybox") + c.Assert(err, check.NotNil, check.Commentf(out)) + c.Assert(out, checker.Contains, "Error: image busybox not found") +} diff --git a/integration-cli/docker_cli_push_test.go b/integration-cli/docker_cli_push_test.go index 8c05342d27..a7e10badb9 100644 --- a/integration-cli/docker_cli_push_test.go +++ b/integration-cli/docker_cli_push_test.go @@ -535,3 +535,13 @@ func (s *DockerRegistryAuthSuite) TestPushNoCredentialsNoRetry(c *check.C) { c.Assert(out, check.Not(checker.Contains), "Retrying") c.Assert(out, checker.Contains, "no basic auth credentials") } + +// This may be flaky but it's needed not to regress on unauthorized push, see #21054 +func (s *DockerSuite) TestPushToCentralRegistryUnauthorized(c *check.C) { + testRequires(c, Network) + repoName := "test/busybox" + dockerCmd(c, "tag", "busybox", repoName) + out, _, err := dockerCmdWithError("push", repoName) + c.Assert(err, check.NotNil, check.Commentf(out)) + c.Assert(out, checker.Contains, "unauthorized: access to the requested resource is not authorized") +}