Skip to content

Commit

Permalink
Better error message for BuildManifestURL if not tagged or digested
Browse files Browse the repository at this point in the history 
Since there's no default case, if there's not a tag or digest you get
back a confusing error from the router about it not matching the
expected pattern.

Also redoing the tests for URLs a bit so that they can handle checking
for failures.

Signed-off-by: Christy Perez <[email protected]>
  • Loading branch information
@clnperez
clnperez committed Mar 2, 2017
1 parent 50133d6 commit 0810eba
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 9 deletions.
3 changes: 3 additions & 0 deletions registry/api/v2/urls.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package v2

import (
"fmt"
"net"
"net/http"
"net/url"
Expand Down Expand Up @@ -175,6 +176,8 @@ func (ub *URLBuilder) BuildManifestURL(ref reference.Named) (string, error) {
tagOrDigest = v.Tag()
case reference.Digested:
tagOrDigest = v.Digest().String()
default:
return "", fmt.Errorf("reference must have a tag or digest")
}

manifestURL, err := route.URL("name", ref.Name(), "reference", tagOrDigest)
Expand Down
54 changes: 45 additions & 9 deletions registry/api/v2/urls_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package v2

import (
"fmt"
"net/http"
"net/url"
"reflect"
"testing"

"github.com/docker/distribution/reference"
Expand All @@ -11,6 +13,7 @@ import (
type urlBuilderTestCase struct {
description string
expectedPath string
expectedErr error
build func() (string, error)
}

Expand All @@ -20,26 +23,38 @@ func makeURLBuilderTestCases(urlBuilder *URLBuilder) []urlBuilderTestCase {
{
description: "test base url",
expectedPath: "/v2/",
expectedErr: nil,
build: urlBuilder.BuildBaseURL,
},
{
description: "test tags url",
expectedPath: "/v2/foo/bar/tags/list",
expectedErr: nil,
build: func() (string, error) {
return urlBuilder.BuildTagsURL(fooBarRef)
},
},
{
description: "test manifest url",
description: "test manifest url tagged ref",
expectedPath: "/v2/foo/bar/manifests/tag",
expectedErr: nil,
build: func() (string, error) {
ref, _ := reference.WithTag(fooBarRef, "tag")
return urlBuilder.BuildManifestURL(ref)
},
},
{
description: "test manifest url bare ref",
expectedPath: "",
expectedErr: fmt.Errorf("reference must have a tag or digest"),
build: func() (string, error) {
return urlBuilder.BuildManifestURL(fooBarRef)
},
},
{
description: "build blob url",
expectedPath: "/v2/foo/bar/blobs/sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5",
expectedErr: nil,
build: func() (string, error) {
ref, _ := reference.WithDigest(fooBarRef, "sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5")
return urlBuilder.BuildBlobURL(ref)
Expand All @@ -48,13 +63,15 @@ func makeURLBuilderTestCases(urlBuilder *URLBuilder) []urlBuilderTestCase {
{
description: "build blob upload url",
expectedPath: "/v2/foo/bar/blobs/uploads/",
expectedErr: nil,
build: func() (string, error) {
return urlBuilder.BuildBlobUploadURL(fooBarRef)
},
},
{
description: "build blob upload url with digest and size",
expectedPath: "/v2/foo/bar/blobs/uploads/?digest=sha256%3A3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5&size=10000",
expectedErr: nil,
build: func() (string, error) {
return urlBuilder.BuildBlobUploadURL(fooBarRef, url.Values{
"size": []string{"10000"},
Expand All @@ -65,13 +82,15 @@ func makeURLBuilderTestCases(urlBuilder *URLBuilder) []urlBuilderTestCase {
{
description: "build blob upload chunk url",
expectedPath: "/v2/foo/bar/blobs/uploads/uuid-part",
expectedErr: nil,
build: func() (string, error) {
return urlBuilder.BuildBlobUploadChunkURL(fooBarRef, "uuid-part")
},
},
{
description: "build blob upload chunk url with digest and size",
expectedPath: "/v2/foo/bar/blobs/uploads/uuid-part?digest=sha256%3A3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5&size=10000",
expectedErr: nil,
build: func() (string, error) {
return urlBuilder.BuildBlobUploadChunkURL(fooBarRef, "uuid-part", url.Values{
"size": []string{"10000"},
Expand Down Expand Up @@ -101,9 +120,14 @@ func TestURLBuilder(t *testing.T) {

for _, testCase := range makeURLBuilderTestCases(urlBuilder) {
url, err := testCase.build()
if err != nil {
t.Fatalf("%s: error building url: %v", testCase.description, err)
expectedErr := testCase.expectedErr
if !reflect.DeepEqual(expectedErr, err) {
t.Fatalf("%s: Expecting %v but got error %v", testCase.description, expectedErr, err)
}
if expectedErr != nil {
continue
}

expectedURL := testCase.expectedPath
if !relative {
expectedURL = root + expectedURL
Expand Down Expand Up @@ -136,8 +160,12 @@ func TestURLBuilderWithPrefix(t *testing.T) {

for _, testCase := range makeURLBuilderTestCases(urlBuilder) {
url, err := testCase.build()
if err != nil {
t.Fatalf("%s: error building url: %v", testCase.description, err)
expectedErr := testCase.expectedErr
if !reflect.DeepEqual(expectedErr, err) {
t.Fatalf("%s: Expecting %v but got error %v", testCase.description, expectedErr, err)
}
if expectedErr != nil {
continue
}

expectedURL := testCase.expectedPath
Expand Down Expand Up @@ -392,8 +420,12 @@ func TestBuilderFromRequest(t *testing.T) {

for _, testCase := range makeURLBuilderTestCases(builder) {
buildURL, err := testCase.build()
if err != nil {
t.Fatalf("[relative=%t, request=%q, case=%q]: error building url: %v", relative, tr.name, testCase.description, err)
expectedErr := testCase.expectedErr
if !reflect.DeepEqual(expectedErr, err) {
t.Fatalf("%s: Expecting %v but got error %v", testCase.description, expectedErr, err)
}
if expectedErr != nil {
continue
}

var expectedURL string
Expand Down Expand Up @@ -475,8 +507,12 @@ func TestBuilderFromRequestWithPrefix(t *testing.T) {

for _, testCase := range makeURLBuilderTestCases(builder) {
buildURL, err := testCase.build()
if err != nil {
t.Fatalf("%s: error building url: %v", testCase.description, err)
expectedErr := testCase.expectedErr
if !reflect.DeepEqual(expectedErr, err) {
t.Fatalf("%s: Expecting %v but got error %v", testCase.description, expectedErr, err)
}
if expectedErr != nil {
continue
}

var expectedURL string
Expand Down

0 comments on commit 0810eba

Please sign in to comment.