Better error message for BuildManifestURL if not tagged or digested

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 <christy@linux.vnet.ibm.com>
This commit is contained in:
Christy Perez 2017-03-01 12:24:08 -06:00
parent 50133d6372
commit 0810eba2ad
2 changed files with 48 additions and 9 deletions

View file

@ -1,6 +1,7 @@
package v2 package v2
import ( import (
"fmt"
"net" "net"
"net/http" "net/http"
"net/url" "net/url"
@ -175,6 +176,8 @@ func (ub *URLBuilder) BuildManifestURL(ref reference.Named) (string, error) {
tagOrDigest = v.Tag() tagOrDigest = v.Tag()
case reference.Digested: case reference.Digested:
tagOrDigest = v.Digest().String() tagOrDigest = v.Digest().String()
default:
return "", fmt.Errorf("reference must have a tag or digest")
} }
manifestURL, err := route.URL("name", ref.Name(), "reference", tagOrDigest) manifestURL, err := route.URL("name", ref.Name(), "reference", tagOrDigest)

View file

@ -1,8 +1,10 @@
package v2 package v2
import ( import (
"fmt"
"net/http" "net/http"
"net/url" "net/url"
"reflect"
"testing" "testing"
"github.com/docker/distribution/reference" "github.com/docker/distribution/reference"
@ -11,6 +13,7 @@ import (
type urlBuilderTestCase struct { type urlBuilderTestCase struct {
description string description string
expectedPath string expectedPath string
expectedErr error
build func() (string, error) build func() (string, error)
} }
@ -20,26 +23,38 @@ func makeURLBuilderTestCases(urlBuilder *URLBuilder) []urlBuilderTestCase {
{ {
description: "test base url", description: "test base url",
expectedPath: "/v2/", expectedPath: "/v2/",
expectedErr: nil,
build: urlBuilder.BuildBaseURL, build: urlBuilder.BuildBaseURL,
}, },
{ {
description: "test tags url", description: "test tags url",
expectedPath: "/v2/foo/bar/tags/list", expectedPath: "/v2/foo/bar/tags/list",
expectedErr: nil,
build: func() (string, error) { build: func() (string, error) {
return urlBuilder.BuildTagsURL(fooBarRef) return urlBuilder.BuildTagsURL(fooBarRef)
}, },
}, },
{ {
description: "test manifest url", description: "test manifest url tagged ref",
expectedPath: "/v2/foo/bar/manifests/tag", expectedPath: "/v2/foo/bar/manifests/tag",
expectedErr: nil,
build: func() (string, error) { build: func() (string, error) {
ref, _ := reference.WithTag(fooBarRef, "tag") ref, _ := reference.WithTag(fooBarRef, "tag")
return urlBuilder.BuildManifestURL(ref) 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", description: "build blob url",
expectedPath: "/v2/foo/bar/blobs/sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5", expectedPath: "/v2/foo/bar/blobs/sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5",
expectedErr: nil,
build: func() (string, error) { build: func() (string, error) {
ref, _ := reference.WithDigest(fooBarRef, "sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5") ref, _ := reference.WithDigest(fooBarRef, "sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5")
return urlBuilder.BuildBlobURL(ref) return urlBuilder.BuildBlobURL(ref)
@ -48,6 +63,7 @@ func makeURLBuilderTestCases(urlBuilder *URLBuilder) []urlBuilderTestCase {
{ {
description: "build blob upload url", description: "build blob upload url",
expectedPath: "/v2/foo/bar/blobs/uploads/", expectedPath: "/v2/foo/bar/blobs/uploads/",
expectedErr: nil,
build: func() (string, error) { build: func() (string, error) {
return urlBuilder.BuildBlobUploadURL(fooBarRef) return urlBuilder.BuildBlobUploadURL(fooBarRef)
}, },
@ -55,6 +71,7 @@ func makeURLBuilderTestCases(urlBuilder *URLBuilder) []urlBuilderTestCase {
{ {
description: "build blob upload url with digest and size", description: "build blob upload url with digest and size",
expectedPath: "/v2/foo/bar/blobs/uploads/?digest=sha256%3A3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5&size=10000", expectedPath: "/v2/foo/bar/blobs/uploads/?digest=sha256%3A3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5&size=10000",
expectedErr: nil,
build: func() (string, error) { build: func() (string, error) {
return urlBuilder.BuildBlobUploadURL(fooBarRef, url.Values{ return urlBuilder.BuildBlobUploadURL(fooBarRef, url.Values{
"size": []string{"10000"}, "size": []string{"10000"},
@ -65,6 +82,7 @@ func makeURLBuilderTestCases(urlBuilder *URLBuilder) []urlBuilderTestCase {
{ {
description: "build blob upload chunk url", description: "build blob upload chunk url",
expectedPath: "/v2/foo/bar/blobs/uploads/uuid-part", expectedPath: "/v2/foo/bar/blobs/uploads/uuid-part",
expectedErr: nil,
build: func() (string, error) { build: func() (string, error) {
return urlBuilder.BuildBlobUploadChunkURL(fooBarRef, "uuid-part") return urlBuilder.BuildBlobUploadChunkURL(fooBarRef, "uuid-part")
}, },
@ -72,6 +90,7 @@ func makeURLBuilderTestCases(urlBuilder *URLBuilder) []urlBuilderTestCase {
{ {
description: "build blob upload chunk url with digest and size", description: "build blob upload chunk url with digest and size",
expectedPath: "/v2/foo/bar/blobs/uploads/uuid-part?digest=sha256%3A3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5&size=10000", expectedPath: "/v2/foo/bar/blobs/uploads/uuid-part?digest=sha256%3A3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5&size=10000",
expectedErr: nil,
build: func() (string, error) { build: func() (string, error) {
return urlBuilder.BuildBlobUploadChunkURL(fooBarRef, "uuid-part", url.Values{ return urlBuilder.BuildBlobUploadChunkURL(fooBarRef, "uuid-part", url.Values{
"size": []string{"10000"}, "size": []string{"10000"},
@ -101,9 +120,14 @@ func TestURLBuilder(t *testing.T) {
for _, testCase := range makeURLBuilderTestCases(urlBuilder) { for _, testCase := range makeURLBuilderTestCases(urlBuilder) {
url, err := testCase.build() url, err := testCase.build()
if err != nil { expectedErr := testCase.expectedErr
t.Fatalf("%s: error building url: %v", testCase.description, err) 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 expectedURL := testCase.expectedPath
if !relative { if !relative {
expectedURL = root + expectedURL expectedURL = root + expectedURL
@ -136,8 +160,12 @@ func TestURLBuilderWithPrefix(t *testing.T) {
for _, testCase := range makeURLBuilderTestCases(urlBuilder) { for _, testCase := range makeURLBuilderTestCases(urlBuilder) {
url, err := testCase.build() url, err := testCase.build()
if err != nil { expectedErr := testCase.expectedErr
t.Fatalf("%s: error building url: %v", testCase.description, err) 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 expectedURL := testCase.expectedPath
@ -392,8 +420,12 @@ func TestBuilderFromRequest(t *testing.T) {
for _, testCase := range makeURLBuilderTestCases(builder) { for _, testCase := range makeURLBuilderTestCases(builder) {
buildURL, err := testCase.build() buildURL, err := testCase.build()
if err != nil { expectedErr := testCase.expectedErr
t.Fatalf("[relative=%t, request=%q, case=%q]: error building url: %v", relative, tr.name, testCase.description, err) 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 var expectedURL string
@ -475,8 +507,12 @@ func TestBuilderFromRequestWithPrefix(t *testing.T) {
for _, testCase := range makeURLBuilderTestCases(builder) { for _, testCase := range makeURLBuilderTestCases(builder) {
buildURL, err := testCase.build() buildURL, err := testCase.build()
if err != nil { expectedErr := testCase.expectedErr
t.Fatalf("%s: error building url: %v", testCase.description, err) 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 var expectedURL string