diff --git a/registry/client/repository.go b/registry/client/repository.go index 0d405077..1ebd0b18 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -301,18 +301,20 @@ func (t *tags) Get(ctx context.Context, tag string) (distribution.Descriptor, er return distribution.Descriptor{}, err } - req, err := http.NewRequest("HEAD", u, nil) - if err != nil { - return distribution.Descriptor{}, err + newRequest := func(method string) (*http.Response, error) { + req, err := http.NewRequest(method, u, nil) + if err != nil { + return nil, err + } + + for _, t := range distribution.ManifestMediaTypes() { + req.Header.Add("Accept", t) + } + resp, err := t.client.Do(req) + return resp, err } - for _, t := range distribution.ManifestMediaTypes() { - req.Header.Add("Accept", t) - } - - var attempts int - resp, err := t.client.Do(req) -check: + resp, err := newRequest("HEAD") if err != nil { return distribution.Descriptor{}, err } @@ -321,23 +323,20 @@ check: switch { case resp.StatusCode >= 200 && resp.StatusCode < 400: return descriptorFromResponse(resp) - case resp.StatusCode == http.StatusMethodNotAllowed: - req, err = http.NewRequest("GET", u, nil) + default: + // if the response is an error - there will be no body to decode. + // Issue a GET request: + // - for data from a server that does not handle HEAD + // - to get error details in case of a failure + resp, err = newRequest("GET") if err != nil { return distribution.Descriptor{}, err } + defer resp.Body.Close() - for _, t := range distribution.ManifestMediaTypes() { - req.Header.Add("Accept", t) + if resp.StatusCode >= 200 && resp.StatusCode < 400 { + return descriptorFromResponse(resp) } - - resp, err = t.client.Do(req) - attempts++ - if attempts > 1 { - return distribution.Descriptor{}, err - } - goto check - default: return distribution.Descriptor{}, HandleErrorResponse(resp) } } diff --git a/registry/client/repository_test.go b/registry/client/repository_test.go index d945596b..a232e03e 100644 --- a/registry/client/repository_test.go +++ b/registry/client/repository_test.go @@ -21,6 +21,7 @@ import ( "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/api/errcode" + "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/testutil" "github.com/docker/distribution/uuid" "github.com/docker/libtrust" @@ -950,6 +951,49 @@ func TestManifestTags(t *testing.T) { // TODO(dmcgowan): Check for error cases } +func TestObtainsErrorForMissingTag(t *testing.T) { + repo, _ := reference.ParseNamed("test.example.com/repo") + + var m testutil.RequestResponseMap + var errors errcode.Errors + errors = append(errors, v2.ErrorCodeManifestUnknown.WithDetail("unknown manifest")) + errBytes, err := json.Marshal(errors) + if err != nil { + t.Fatal(err) + } + m = append(m, testutil.RequestResponseMapping{ + Request: testutil.Request{ + Method: "GET", + Route: "/v2/" + repo.Name() + "/manifests/1.0.0", + }, + Response: testutil.Response{ + StatusCode: http.StatusNotFound, + Body: errBytes, + Headers: http.Header(map[string][]string{ + "Content-Type": {"application/json; charset=utf-8"}, + }), + }, + }) + e, c := testServer(m) + defer c() + + ctx := context.Background() + r, err := NewRepository(ctx, repo, e, nil) + if err != nil { + t.Fatal(err) + } + + tagService := r.Tags(ctx) + + _, err = tagService.Get(ctx, "1.0.0") + if err == nil { + t.Fatalf("Expected an error") + } + if !strings.Contains(err.Error(), "manifest unknown") { + t.Fatalf("Expected unknown manifest error message") + } +} + func TestManifestTagsPaginated(t *testing.T) { s := httptest.NewServer(http.NotFoundHandler()) defer s.Close()