Port client to use URLBuilder to create urls

This change ports the client use the URLBuilder to create urls. Without this,
it produces broken urls for certain use cases. The client has also been updated
to no longer use the size argument to complete blob uploads. Much of this work
has been done after testing with the staging registry instance.

client/client.go

@@ -71,19 +71,33 @@ type Client interface {
 // New returns a new Client which operates against a registry with the
 // given base endpoint
 // This endpoint should not include /v2/ or any part of the url after this.
-func New(endpoint string) Client {
+func New(endpoint string) (Client, error) {
-	return &clientImpl{endpoint}
+	ub, err := v2.NewURLBuilderFromString(endpoint)
+	if err != nil {
+		return nil, err
+	}
+
+	return &clientImpl{
+		endpoint: endpoint,
+		ub:       ub,
+	}, nil
 }
 
 // clientImpl is the default implementation of the Client interface
 type clientImpl struct {
-	Endpoint string
+	endpoint string
+	ub       *v2.URLBuilder
 }
 
 // TODO(bbland): use consistent route generation between server and client
 
 func (r *clientImpl) GetImageManifest(name, tag string) (*storage.SignedManifest, error) {
-	response, err := http.Get(r.imageManifestURL(name, tag))
+	manifestURL, err := r.ub.BuildManifestURL(name, tag)
+	if err != nil {
+		return nil, err
+	}
+
+	response, err := http.Get(manifestURL)
 	if err != nil {
 		return nil, err
 	}
@@ -119,8 +133,12 @@ func (r *clientImpl) GetImageManifest(name, tag string) (*storage.SignedManifest
 }
 
 func (r *clientImpl) PutImageManifest(name, tag string, manifest *storage.SignedManifest) error {
-	putRequest, err := http.NewRequest("PUT",
+	manifestURL, err := r.ub.BuildManifestURL(name, tag)
-		r.imageManifestURL(name, tag), bytes.NewReader(manifest.Raw))
+	if err != nil {
+		return err
+	}
+
+	putRequest, err := http.NewRequest("PUT", manifestURL, bytes.NewReader(manifest.Raw))
 	if err != nil {
 		return err
 	}
@@ -150,8 +168,12 @@ func (r *clientImpl) PutImageManifest(name, tag string, manifest *storage.Signed
 }
 
 func (r *clientImpl) DeleteImage(name, tag string) error {
-	deleteRequest, err := http.NewRequest("DELETE",
+	manifestURL, err := r.ub.BuildManifestURL(name, tag)
-		r.imageManifestURL(name, tag), nil)
+	if err != nil {
+		return err
+	}
+
+	deleteRequest, err := http.NewRequest("DELETE", manifestURL, nil)
 	if err != nil {
 		return err
 	}
@@ -184,7 +206,12 @@ func (r *clientImpl) DeleteImage(name, tag string) error {
 }
 
 func (r *clientImpl) ListImageTags(name string) ([]string, error) {
-	response, err := http.Get(fmt.Sprintf("%s/v2/%s/tags/list", r.Endpoint, name))
+	tagsURL, err := r.ub.BuildTagsURL(name)
+	if err != nil {
+		return nil, err
+	}
+
+	response, err := http.Get(tagsURL)
 	if err != nil {
 		return nil, err
 	}
@@ -222,7 +249,12 @@ func (r *clientImpl) ListImageTags(name string) ([]string, error) {
 }
 
 func (r *clientImpl) BlobLength(name string, dgst digest.Digest) (int, error) {
-	response, err := http.Head(fmt.Sprintf("%s/v2/%s/blobs/%s", r.Endpoint, name, dgst))
+	blobURL, err := r.ub.BuildBlobURL(name, dgst)
+	if err != nil {
+		return -1, err
+	}
+
+	response, err := http.Head(blobURL)
 	if err != nil {
 		return -1, err
 	}
@@ -254,8 +286,12 @@ func (r *clientImpl) BlobLength(name string, dgst digest.Digest) (int, error) {
 }
 
 func (r *clientImpl) GetBlob(name string, dgst digest.Digest, byteOffset int) (io.ReadCloser, int, error) {
-	getRequest, err := http.NewRequest("GET",
+	blobURL, err := r.ub.BuildBlobURL(name, dgst)
-		fmt.Sprintf("%s/v2/%s/blobs/%s", r.Endpoint, name, dgst), nil)
+	if err != nil {
+		return nil, 0, err
+	}
+
+	getRequest, err := http.NewRequest("GET", blobURL, nil)
 	if err != nil {
 		return nil, 0, err
 	}
@@ -293,8 +329,12 @@ func (r *clientImpl) GetBlob(name string, dgst digest.Digest, byteOffset int) (i
 }
 
 func (r *clientImpl) InitiateBlobUpload(name string) (string, error) {
-	postRequest, err := http.NewRequest("POST",
+	uploadURL, err := r.ub.BuildBlobUploadURL(name)
-		fmt.Sprintf("%s/v2/%s/blobs/uploads/", r.Endpoint, name), nil)
+	if err != nil {
+		return "", err
+	}
+
+	postRequest, err := http.NewRequest("POST", uploadURL, nil)
 	if err != nil {
 		return "", err
 	}
@@ -359,7 +399,6 @@ func (r *clientImpl) UploadBlob(location string, blob io.ReadCloser, length int,
 	}
 
 	queryValues := url.Values{}
-	queryValues.Set("size", fmt.Sprint(length))
 	queryValues.Set("digest", dgst.String())
 	putRequest.URL.RawQuery = queryValues.Encode()
 
@@ -394,8 +433,7 @@ func (r *clientImpl) UploadBlob(location string, blob io.ReadCloser, length int,
 func (r *clientImpl) UploadBlobChunk(location string, blobChunk io.ReadCloser, length, startByte int) error {
 	defer blobChunk.Close()
 
-	putRequest, err := http.NewRequest("PUT",
+	putRequest, err := http.NewRequest("PUT", location, blobChunk)
-		fmt.Sprintf("%s%s", r.Endpoint, location), blobChunk)
 	if err != nil {
 		return err
 	}
@@ -443,14 +481,12 @@ func (r *clientImpl) UploadBlobChunk(location string, blobChunk io.ReadCloser, l
 }
 
 func (r *clientImpl) FinishChunkedBlobUpload(location string, length int, dgst digest.Digest) error {
-	putRequest, err := http.NewRequest("PUT",
+	putRequest, err := http.NewRequest("PUT", location, nil)
-		fmt.Sprintf("%s%s", r.Endpoint, location), nil)
 	if err != nil {
 		return err
 	}
 
 	queryValues := new(url.Values)
-	queryValues.Set("size", fmt.Sprint(length))
 	queryValues.Set("digest", dgst.String())
 	putRequest.URL.RawQuery = queryValues.Encode()
 
@@ -485,8 +521,7 @@ func (r *clientImpl) FinishChunkedBlobUpload(location string, length int, dgst d
 }
 
 func (r *clientImpl) CancelBlobUpload(location string) error {
-	deleteRequest, err := http.NewRequest("DELETE",
+	deleteRequest, err := http.NewRequest("DELETE", location, nil)
-		fmt.Sprintf("%s%s", r.Endpoint, location), nil)
 	if err != nil {
 		return err
 	}
@@ -516,12 +551,6 @@ func (r *clientImpl) CancelBlobUpload(location string) error {
 	}
 }
 
-// imageManifestURL is a helper method for returning the full url to an image
-// manifest
-func (r *clientImpl) imageManifestURL(name, tag string) string {
-	return fmt.Sprintf("%s/v2/%s/manifests/%s", r.Endpoint, name, tag)
-}
-
 // parseRangeHeader parses out the offset and length from a returned Range
 // header
 func parseRangeHeader(byteRangeHeader string) (int, int, error) {

client/client_test.go

@@ -24,11 +24,11 @@ func TestPush(t *testing.T) {
 	tag := "sometag"
 	testBlobs := []testBlob{
 		{
-			digest:   "12345",
+			digest:   "tarsum.v2+sha256:12345",
 			contents: []byte("some contents"),
 		},
 		{
-			digest:   "98765",
+			digest:   "tarsum.v2+sha256:98765",
 			contents: []byte("some other contents"),
 		},
 	}
@@ -80,7 +80,6 @@ func TestPush(t *testing.T) {
 				Method: "PUT",
 				Route:  uploadLocations[i],
 				QueryParams: map[string][]string{
-					"length": {fmt.Sprint(len(blob.contents))},
 					"digest": {blob.digest.String()},
 				},
 				Body: blob.contents,
@@ -114,7 +113,10 @@ func TestPush(t *testing.T) {
 	})
 
 	server = httptest.NewServer(hack)
-	client := New(server.URL)
+	client, err := New(server.URL)
+	if err != nil {
+		t.Fatalf("error creating client: %v", err)
+	}
 	objectStore := &memoryObjectStore{
 		mutex:           new(sync.Mutex),
 		manifestStorage: make(map[string]*storage.SignedManifest),
@@ -150,11 +152,11 @@ func TestPull(t *testing.T) {
 	tag := "sometag"
 	testBlobs := []testBlob{
 		{
-			digest:   "12345",
+			digest:   "tarsum.v2+sha256:12345",
 			contents: []byte("some contents"),
 		},
 		{
-			digest:   "98765",
+			digest:   "tarsum.v2+sha256:98765",
 			contents: []byte("some other contents"),
 		},
 	}
@@ -205,7 +207,10 @@ func TestPull(t *testing.T) {
 		},
 	}))
 	server := httptest.NewServer(handler)
-	client := New(server.URL)
+	client, err := New(server.URL)
+	if err != nil {
+		t.Fatalf("error creating client: %v", err)
+	}
 	objectStore := &memoryObjectStore{
 		mutex:           new(sync.Mutex),
 		manifestStorage: make(map[string]*storage.SignedManifest),
@@ -259,11 +264,11 @@ func TestPullResume(t *testing.T) {
 	tag := "sometag"
 	testBlobs := []testBlob{
 		{
-			digest:   "12345",
+			digest:   "tarsum.v2+sha256:12345",
 			contents: []byte("some contents"),
 		},
 		{
-			digest:   "98765",
+			digest:   "tarsum.v2+sha256:98765",
 			contents: []byte("some other contents"),
 		},
 	}
@@ -329,7 +334,10 @@ func TestPullResume(t *testing.T) {
 
 	handler := testutil.NewHandler(layerRequestResponseMappings)
 	server := httptest.NewServer(handler)
-	client := New(server.URL)
+	client, err := New(server.URL)
+	if err != nil {
+		t.Fatalf("error creating client: %v", err)
+	}
 	objectStore := &memoryObjectStore{
 		mutex:           new(sync.Mutex),
 		manifestStorage: make(map[string]*storage.SignedManifest),