From e9692b8037d032d3dfd6cd5c2f9737aa22884e57 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Tue, 15 Dec 2015 14:35:23 -0800 Subject: [PATCH 1/2] Use reference package internally Most places in the registry were using string types to refer to repository names. This changes them to use reference.Named, so the type system can enforce validation of the naming rules. Signed-off-by: Aaron Lehmann --- docs/api/v2/urls.go | 21 +++--- docs/api/v2/urls_test.go | 17 +++-- docs/client/repository.go | 24 +++--- docs/client/repository_test.go | 101 ++++++++++++-------------- docs/handlers/api_test.go | 91 +++++++++++++---------- docs/handlers/app.go | 15 +++- docs/handlers/app_test.go | 2 +- docs/handlers/blobupload.go | 4 +- docs/handlers/images.go | 13 +++- docs/handlers/tags.go | 4 +- docs/proxy/proxyblobstore.go | 5 +- docs/proxy/proxyblobstore_test.go | 10 ++- docs/proxy/proxymanifeststore.go | 5 +- docs/proxy/proxymanifeststore_test.go | 10 ++- docs/proxy/proxyregistry.go | 9 ++- docs/proxy/scheduler/scheduler.go | 10 ++- docs/storage/blob_test.go | 16 ++-- docs/storage/blobwriter.go | 2 +- docs/storage/blobwriter_resumable.go | 4 +- docs/storage/linkedblobstore.go | 18 ++--- docs/storage/manifeststore.go | 2 +- docs/storage/manifeststore_test.go | 10 ++- docs/storage/registry.go | 15 +--- docs/storage/signaturestore.go | 2 +- docs/storage/tagstore.go | 14 ++-- docs/storage/tagstore_test.go | 4 +- 26 files changed, 235 insertions(+), 193 deletions(-) diff --git a/docs/api/v2/urls.go b/docs/api/v2/urls.go index 6ba39cc9b..5b63ccaa5 100644 --- a/docs/api/v2/urls.go +++ b/docs/api/v2/urls.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/docker/distribution/digest" + "github.com/docker/distribution/reference" "github.com/gorilla/mux" ) @@ -113,10 +114,10 @@ func (ub *URLBuilder) BuildCatalogURL(values ...url.Values) (string, error) { } // BuildTagsURL constructs a url to list the tags in the named repository. -func (ub *URLBuilder) BuildTagsURL(name string) (string, error) { +func (ub *URLBuilder) BuildTagsURL(name reference.Named) (string, error) { route := ub.cloneRoute(RouteNameTags) - tagsURL, err := route.URL("name", name) + tagsURL, err := route.URL("name", name.Name()) if err != nil { return "", err } @@ -126,10 +127,10 @@ func (ub *URLBuilder) BuildTagsURL(name string) (string, error) { // BuildManifestURL constructs a url for the manifest identified by name and // reference. The argument reference may be either a tag or digest. -func (ub *URLBuilder) BuildManifestURL(name, reference string) (string, error) { +func (ub *URLBuilder) BuildManifestURL(name reference.Named, reference string) (string, error) { route := ub.cloneRoute(RouteNameManifest) - manifestURL, err := route.URL("name", name, "reference", reference) + manifestURL, err := route.URL("name", name.Name(), "reference", reference) if err != nil { return "", err } @@ -138,10 +139,10 @@ func (ub *URLBuilder) BuildManifestURL(name, reference string) (string, error) { } // BuildBlobURL constructs the url for the blob identified by name and dgst. -func (ub *URLBuilder) BuildBlobURL(name string, dgst digest.Digest) (string, error) { +func (ub *URLBuilder) BuildBlobURL(name reference.Named, dgst digest.Digest) (string, error) { route := ub.cloneRoute(RouteNameBlob) - layerURL, err := route.URL("name", name, "digest", dgst.String()) + layerURL, err := route.URL("name", name.Name(), "digest", dgst.String()) if err != nil { return "", err } @@ -151,10 +152,10 @@ func (ub *URLBuilder) BuildBlobURL(name string, dgst digest.Digest) (string, err // BuildBlobUploadURL constructs a url to begin a blob upload in the // repository identified by name. -func (ub *URLBuilder) BuildBlobUploadURL(name string, values ...url.Values) (string, error) { +func (ub *URLBuilder) BuildBlobUploadURL(name reference.Named, values ...url.Values) (string, error) { route := ub.cloneRoute(RouteNameBlobUpload) - uploadURL, err := route.URL("name", name) + uploadURL, err := route.URL("name", name.Name()) if err != nil { return "", err } @@ -166,10 +167,10 @@ func (ub *URLBuilder) BuildBlobUploadURL(name string, values ...url.Values) (str // including any url values. This should generally not be used by clients, as // this url is provided by server implementations during the blob upload // process. -func (ub *URLBuilder) BuildBlobUploadChunkURL(name, uuid string, values ...url.Values) (string, error) { +func (ub *URLBuilder) BuildBlobUploadChunkURL(name reference.Named, uuid string, values ...url.Values) (string, error) { route := ub.cloneRoute(RouteNameBlobUploadChunk) - uploadURL, err := route.URL("name", name, "uuid", uuid) + uploadURL, err := route.URL("name", name.Name(), "uuid", uuid) if err != nil { return "", err } diff --git a/docs/api/v2/urls_test.go b/docs/api/v2/urls_test.go index 0ad33add8..7dab00fcb 100644 --- a/docs/api/v2/urls_test.go +++ b/docs/api/v2/urls_test.go @@ -4,6 +4,8 @@ import ( "net/http" "net/url" "testing" + + "github.com/docker/distribution/reference" ) type urlBuilderTestCase struct { @@ -13,6 +15,7 @@ type urlBuilderTestCase struct { } func makeURLBuilderTestCases(urlBuilder *URLBuilder) []urlBuilderTestCase { + fooBarRef, _ := reference.ParseNamed("foo/bar") return []urlBuilderTestCase{ { description: "test base url", @@ -23,35 +26,35 @@ func makeURLBuilderTestCases(urlBuilder *URLBuilder) []urlBuilderTestCase { description: "test tags url", expectedPath: "/v2/foo/bar/tags/list", build: func() (string, error) { - return urlBuilder.BuildTagsURL("foo/bar") + return urlBuilder.BuildTagsURL(fooBarRef) }, }, { description: "test manifest url", expectedPath: "/v2/foo/bar/manifests/tag", build: func() (string, error) { - return urlBuilder.BuildManifestURL("foo/bar", "tag") + return urlBuilder.BuildManifestURL(fooBarRef, "tag") }, }, { description: "build blob url", expectedPath: "/v2/foo/bar/blobs/sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5", build: func() (string, error) { - return urlBuilder.BuildBlobURL("foo/bar", "sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5") + return urlBuilder.BuildBlobURL(fooBarRef, "sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5") }, }, { description: "build blob upload url", expectedPath: "/v2/foo/bar/blobs/uploads/", build: func() (string, error) { - return urlBuilder.BuildBlobUploadURL("foo/bar") + return urlBuilder.BuildBlobUploadURL(fooBarRef) }, }, { description: "build blob upload url with digest and size", expectedPath: "/v2/foo/bar/blobs/uploads/?digest=sha256%3A3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5&size=10000", build: func() (string, error) { - return urlBuilder.BuildBlobUploadURL("foo/bar", url.Values{ + return urlBuilder.BuildBlobUploadURL(fooBarRef, url.Values{ "size": []string{"10000"}, "digest": []string{"sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5"}, }) @@ -61,14 +64,14 @@ func makeURLBuilderTestCases(urlBuilder *URLBuilder) []urlBuilderTestCase { description: "build blob upload chunk url", expectedPath: "/v2/foo/bar/blobs/uploads/uuid-part", build: func() (string, error) { - return urlBuilder.BuildBlobUploadChunkURL("foo/bar", "uuid-part") + 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", build: func() (string, error) { - return urlBuilder.BuildBlobUploadChunkURL("foo/bar", "uuid-part", url.Values{ + return urlBuilder.BuildBlobUploadChunkURL(fooBarRef, "uuid-part", url.Values{ "size": []string{"10000"}, "digest": []string{"sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5"}, }) diff --git a/docs/client/repository.go b/docs/client/repository.go index d65212110..43826907e 100644 --- a/docs/client/repository.go +++ b/docs/client/repository.go @@ -98,11 +98,7 @@ func (r *registry) Repositories(ctx context.Context, entries []string, last stri } // NewRepository creates a new Repository for the given repository name and base URL. -func NewRepository(ctx context.Context, name, baseURL string, transport http.RoundTripper) (distribution.Repository, error) { - if _, err := reference.ParseNamed(name); err != nil { - return nil, err - } - +func NewRepository(ctx context.Context, name reference.Named, baseURL string, transport http.RoundTripper) (distribution.Repository, error) { ub, err := v2.NewURLBuilderFromString(baseURL) if err != nil { return nil, err @@ -125,21 +121,21 @@ type repository struct { client *http.Client ub *v2.URLBuilder context context.Context - name string + name reference.Named } -func (r *repository) Name() string { +func (r *repository) Name() reference.Named { return r.name } func (r *repository) Blobs(ctx context.Context) distribution.BlobStore { statter := &blobStatter{ - name: r.Name(), + name: r.name, ub: r.ub, client: r.client, } return &blobs{ - name: r.Name(), + name: r.name, ub: r.ub, client: r.client, statter: cache.NewCachedBlobStatter(memory.NewInMemoryBlobDescriptorCacheProvider(), statter), @@ -149,7 +145,7 @@ func (r *repository) Blobs(ctx context.Context) distribution.BlobStore { func (r *repository) Manifests(ctx context.Context, options ...distribution.ManifestServiceOption) (distribution.ManifestService, error) { // todo(richardscothern): options should be sent over the wire return &manifests{ - name: r.Name(), + name: r.name, ub: r.ub, client: r.client, etags: make(map[string]string), @@ -170,7 +166,7 @@ type tags struct { client *http.Client ub *v2.URLBuilder context context.Context - name string + name reference.Named } // All returns all tags @@ -293,7 +289,7 @@ func (t *tags) Untag(ctx context.Context, tag string) error { } type manifests struct { - name string + name reference.Named ub *v2.URLBuilder client *http.Client etags map[string]string @@ -493,7 +489,7 @@ func (ms *manifests) Delete(ctx context.Context, dgst digest.Digest) error { }*/ type blobs struct { - name string + name reference.Named ub *v2.URLBuilder client *http.Client @@ -666,7 +662,7 @@ func (bs *blobs) Delete(ctx context.Context, dgst digest.Digest) error { } type blobStatter struct { - name string + name reference.Named ub *v2.URLBuilder client *http.Client } diff --git a/docs/client/repository_test.go b/docs/client/repository_test.go index 69987c878..b7b782c70 100644 --- a/docs/client/repository_test.go +++ b/docs/client/repository_test.go @@ -98,11 +98,11 @@ func addTestCatalog(route string, content []byte, link string, m *testutil.Reque func TestBlobDelete(t *testing.T) { dgst, _ := newRandomBlob(1024) var m testutil.RequestResponseMap - repo := "test.example.com/repo1" + repo, _ := reference.ParseNamed("test.example.com/repo1") m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "DELETE", - Route: "/v2/" + repo + "/blobs/" + dgst.String(), + Route: "/v2/" + repo.Name() + "/blobs/" + dgst.String(), }, Response: testutil.Response{ StatusCode: http.StatusAccepted, @@ -137,7 +137,8 @@ func TestBlobFetch(t *testing.T) { defer c() ctx := context.Background() - r, err := NewRepository(ctx, "test.example.com/repo1", e, nil) + repo, _ := reference.ParseNamed("test.example.com/repo1") + r, err := NewRepository(ctx, repo, e, nil) if err != nil { t.Fatal(err) } @@ -157,12 +158,12 @@ func TestBlobFetch(t *testing.T) { func TestBlobExistsNoContentLength(t *testing.T) { var m testutil.RequestResponseMap - repo := "biff" + repo, _ := reference.ParseNamed("biff") dgst, content := newRandomBlob(1024) m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "GET", - Route: "/v2/" + repo + "/blobs/" + dgst.String(), + Route: "/v2/" + repo.Name() + "/blobs/" + dgst.String(), }, Response: testutil.Response{ StatusCode: http.StatusOK, @@ -177,7 +178,7 @@ func TestBlobExistsNoContentLength(t *testing.T) { m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "HEAD", - Route: "/v2/" + repo + "/blobs/" + dgst.String(), + Route: "/v2/" + repo.Name() + "/blobs/" + dgst.String(), }, Response: testutil.Response{ StatusCode: http.StatusOK, @@ -216,7 +217,8 @@ func TestBlobExists(t *testing.T) { defer c() ctx := context.Background() - r, err := NewRepository(ctx, "test.example.com/repo1", e, nil) + repo, _ := reference.ParseNamed("test.example.com/repo1") + r, err := NewRepository(ctx, repo, e, nil) if err != nil { t.Fatal(err) } @@ -247,18 +249,18 @@ func TestBlobUploadChunked(t *testing.T) { b1[512:513], b1[513:1024], } - repo := "test.example.com/uploadrepo" + repo, _ := reference.ParseNamed("test.example.com/uploadrepo") uuids := []string{uuid.Generate().String()} m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "POST", - Route: "/v2/" + repo + "/blobs/uploads/", + Route: "/v2/" + repo.Name() + "/blobs/uploads/", }, Response: testutil.Response{ StatusCode: http.StatusAccepted, Headers: http.Header(map[string][]string{ "Content-Length": {"0"}, - "Location": {"/v2/" + repo + "/blobs/uploads/" + uuids[0]}, + "Location": {"/v2/" + repo.Name() + "/blobs/uploads/" + uuids[0]}, "Docker-Upload-UUID": {uuids[0]}, "Range": {"0-0"}, }), @@ -271,14 +273,14 @@ func TestBlobUploadChunked(t *testing.T) { m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "PATCH", - Route: "/v2/" + repo + "/blobs/uploads/" + uuids[i], + Route: "/v2/" + repo.Name() + "/blobs/uploads/" + uuids[i], Body: chunk, }, Response: testutil.Response{ StatusCode: http.StatusAccepted, Headers: http.Header(map[string][]string{ "Content-Length": {"0"}, - "Location": {"/v2/" + repo + "/blobs/uploads/" + uuids[i+1]}, + "Location": {"/v2/" + repo.Name() + "/blobs/uploads/" + uuids[i+1]}, "Docker-Upload-UUID": {uuids[i+1]}, "Range": {fmt.Sprintf("%d-%d", offset, newOffset-1)}, }), @@ -289,7 +291,7 @@ func TestBlobUploadChunked(t *testing.T) { m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "PUT", - Route: "/v2/" + repo + "/blobs/uploads/" + uuids[len(uuids)-1], + Route: "/v2/" + repo.Name() + "/blobs/uploads/" + uuids[len(uuids)-1], QueryParams: map[string][]string{ "digest": {dgst.String()}, }, @@ -306,7 +308,7 @@ func TestBlobUploadChunked(t *testing.T) { m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "HEAD", - Route: "/v2/" + repo + "/blobs/" + dgst.String(), + Route: "/v2/" + repo.Name() + "/blobs/" + dgst.String(), }, Response: testutil.Response{ StatusCode: http.StatusOK, @@ -362,18 +364,18 @@ func TestBlobUploadChunked(t *testing.T) { func TestBlobUploadMonolithic(t *testing.T) { dgst, b1 := newRandomBlob(1024) var m testutil.RequestResponseMap - repo := "test.example.com/uploadrepo" + repo, _ := reference.ParseNamed("test.example.com/uploadrepo") uploadID := uuid.Generate().String() m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "POST", - Route: "/v2/" + repo + "/blobs/uploads/", + Route: "/v2/" + repo.Name() + "/blobs/uploads/", }, Response: testutil.Response{ StatusCode: http.StatusAccepted, Headers: http.Header(map[string][]string{ "Content-Length": {"0"}, - "Location": {"/v2/" + repo + "/blobs/uploads/" + uploadID}, + "Location": {"/v2/" + repo.Name() + "/blobs/uploads/" + uploadID}, "Docker-Upload-UUID": {uploadID}, "Range": {"0-0"}, }), @@ -382,13 +384,13 @@ func TestBlobUploadMonolithic(t *testing.T) { m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "PATCH", - Route: "/v2/" + repo + "/blobs/uploads/" + uploadID, + Route: "/v2/" + repo.Name() + "/blobs/uploads/" + uploadID, Body: b1, }, Response: testutil.Response{ StatusCode: http.StatusAccepted, Headers: http.Header(map[string][]string{ - "Location": {"/v2/" + repo + "/blobs/uploads/" + uploadID}, + "Location": {"/v2/" + repo.Name() + "/blobs/uploads/" + uploadID}, "Docker-Upload-UUID": {uploadID}, "Content-Length": {"0"}, "Docker-Content-Digest": {dgst.String()}, @@ -399,7 +401,7 @@ func TestBlobUploadMonolithic(t *testing.T) { m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "PUT", - Route: "/v2/" + repo + "/blobs/uploads/" + uploadID, + Route: "/v2/" + repo.Name() + "/blobs/uploads/" + uploadID, QueryParams: map[string][]string{ "digest": {dgst.String()}, }, @@ -416,7 +418,7 @@ func TestBlobUploadMonolithic(t *testing.T) { m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "HEAD", - Route: "/v2/" + repo + "/blobs/" + dgst.String(), + Route: "/v2/" + repo.Name() + "/blobs/" + dgst.String(), }, Response: testutil.Response{ StatusCode: http.StatusOK, @@ -470,29 +472,22 @@ func TestBlobUploadMonolithic(t *testing.T) { func TestBlobMount(t *testing.T) { dgst, content := newRandomBlob(1024) var m testutil.RequestResponseMap - repo := "test.example.com/uploadrepo" - sourceRepo := "test.example.com/sourcerepo" + repo, _ := reference.ParseNamed("test.example.com/uploadrepo") - namedRef, err := reference.ParseNamed(sourceRepo) - if err != nil { - t.Fatal(err) - } - canonicalRef, err := reference.WithDigest(namedRef, dgst) - if err != nil { - t.Fatal(err) - } + sourceRepo, _ := reference.ParseNamed("test.example.com/sourcerepo") + canonicalRef, _ := reference.WithDigest(sourceRepo, dgst) m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "POST", - Route: "/v2/" + repo + "/blobs/uploads/", - QueryParams: map[string][]string{"from": {sourceRepo}, "mount": {dgst.String()}}, + Route: "/v2/" + repo.Name() + "/blobs/uploads/", + QueryParams: map[string][]string{"from": {sourceRepo.Name()}, "mount": {dgst.String()}}, }, Response: testutil.Response{ StatusCode: http.StatusCreated, Headers: http.Header(map[string][]string{ "Content-Length": {"0"}, - "Location": {"/v2/" + repo + "/blobs/" + dgst.String()}, + "Location": {"/v2/" + repo.Name() + "/blobs/" + dgst.String()}, "Docker-Content-Digest": {dgst.String()}, }), }, @@ -500,7 +495,7 @@ func TestBlobMount(t *testing.T) { m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "HEAD", - Route: "/v2/" + repo + "/blobs/" + dgst.String(), + Route: "/v2/" + repo.Name() + "/blobs/" + dgst.String(), }, Response: testutil.Response{ StatusCode: http.StatusOK, @@ -531,7 +526,7 @@ func TestBlobMount(t *testing.T) { if ebm.From.Digest() != dgst { t.Fatalf("Unexpected digest: %s, expected %s", ebm.From.Digest(), dgst) } - if ebm.From.Name() != sourceRepo { + if ebm.From.Name() != sourceRepo.Name() { t.Fatalf("Unexpected from: %s, expected %s", ebm.From.Name(), sourceRepo) } } else { @@ -539,7 +534,7 @@ func TestBlobMount(t *testing.T) { } } -func newRandomSchemaV1Manifest(name, tag string, blobCount int) (*schema1.SignedManifest, digest.Digest, []byte) { +func newRandomSchemaV1Manifest(name reference.Named, tag string, blobCount int) (*schema1.SignedManifest, digest.Digest, []byte) { blobs := make([]schema1.FSLayer, blobCount) history := make([]schema1.History, blobCount) @@ -551,7 +546,7 @@ func newRandomSchemaV1Manifest(name, tag string, blobCount int) (*schema1.Signed } m := schema1.Manifest{ - Name: name, + Name: name.String(), Tag: tag, Architecture: "x86", FSLayers: blobs, @@ -574,11 +569,11 @@ func newRandomSchemaV1Manifest(name, tag string, blobCount int) (*schema1.Signed return sm, digest.FromBytes(sm.Canonical), sm.Canonical } -func addTestManifestWithEtag(repo, reference string, content []byte, m *testutil.RequestResponseMap, dgst string) { +func addTestManifestWithEtag(repo reference.Named, reference string, content []byte, m *testutil.RequestResponseMap, dgst string) { actualDigest := digest.FromBytes(content) getReqWithEtag := testutil.Request{ Method: "GET", - Route: "/v2/" + repo + "/manifests/" + reference, + Route: "/v2/" + repo.Name() + "/manifests/" + reference, Headers: http.Header(map[string][]string{ "If-None-Match": {fmt.Sprintf(`"%s"`, dgst)}, }), @@ -610,11 +605,11 @@ func addTestManifestWithEtag(repo, reference string, content []byte, m *testutil *m = append(*m, testutil.RequestResponseMapping{Request: getReqWithEtag, Response: getRespWithEtag}) } -func addTestManifest(repo, reference string, mediatype string, content []byte, m *testutil.RequestResponseMap) { +func addTestManifest(repo reference.Named, reference string, mediatype string, content []byte, m *testutil.RequestResponseMap) { *m = append(*m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "GET", - Route: "/v2/" + repo + "/manifests/" + reference, + Route: "/v2/" + repo.Name() + "/manifests/" + reference, }, Response: testutil.Response{ StatusCode: http.StatusOK, @@ -629,7 +624,7 @@ func addTestManifest(repo, reference string, mediatype string, content []byte, m *m = append(*m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "HEAD", - Route: "/v2/" + repo + "/manifests/" + reference, + Route: "/v2/" + repo.Name() + "/manifests/" + reference, }, Response: testutil.Response{ StatusCode: http.StatusOK, @@ -671,7 +666,7 @@ func checkEqualManifest(m1, m2 *schema1.SignedManifest) error { func TestV1ManifestFetch(t *testing.T) { ctx := context.Background() - repo := "test.example.com/repo" + repo, _ := reference.ParseNamed("test.example.com/repo") m1, dgst, _ := newRandomSchemaV1Manifest(repo, "latest", 6) var m testutil.RequestResponseMap _, pl, err := m1.Payload() @@ -743,7 +738,7 @@ func TestV1ManifestFetch(t *testing.T) { } func TestManifestFetchWithEtag(t *testing.T) { - repo := "test.example.com/repo/by/tag" + repo, _ := reference.ParseNamed("test.example.com/repo/by/tag") _, d1, p1 := newRandomSchemaV1Manifest(repo, "latest", 6) var m testutil.RequestResponseMap addTestManifestWithEtag(repo, "latest", p1, &m, d1.String()) @@ -773,14 +768,14 @@ func TestManifestFetchWithEtag(t *testing.T) { } func TestManifestDelete(t *testing.T) { - repo := "test.example.com/repo/delete" + repo, _ := reference.ParseNamed("test.example.com/repo/delete") _, dgst1, _ := newRandomSchemaV1Manifest(repo, "latest", 6) _, dgst2, _ := newRandomSchemaV1Manifest(repo, "latest", 6) var m testutil.RequestResponseMap m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "DELETE", - Route: "/v2/" + repo + "/manifests/" + dgst1.String(), + Route: "/v2/" + repo.Name() + "/manifests/" + dgst1.String(), }, Response: testutil.Response{ StatusCode: http.StatusAccepted, @@ -813,7 +808,7 @@ func TestManifestDelete(t *testing.T) { } func TestManifestPut(t *testing.T) { - repo := "test.example.com/repo/delete" + repo, _ := reference.ParseNamed("test.example.com/repo/delete") m1, dgst, _ := newRandomSchemaV1Manifest(repo, "other", 6) _, payload, err := m1.Payload() @@ -824,7 +819,7 @@ func TestManifestPut(t *testing.T) { m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "PUT", - Route: "/v2/" + repo + "/manifests/other", + Route: "/v2/" + repo.Name() + "/manifests/other", Body: payload, }, Response: testutil.Response{ @@ -857,7 +852,7 @@ func TestManifestPut(t *testing.T) { } func TestManifestTags(t *testing.T) { - repo := "test.example.com/repo/tags/list" + repo, _ := reference.ParseNamed("test.example.com/repo/tags/list") tagsList := []byte(strings.TrimSpace(` { "name": "test.example.com/repo/tags/list", @@ -873,7 +868,7 @@ func TestManifestTags(t *testing.T) { m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "GET", - Route: "/v2/" + repo + "/tags/list", + Route: "/v2/" + repo.Name() + "/tags/list", }, Response: testutil.Response{ StatusCode: http.StatusOK, @@ -919,14 +914,14 @@ func TestManifestTags(t *testing.T) { } func TestManifestUnauthorized(t *testing.T) { - repo := "test.example.com/repo" + repo, _ := reference.ParseNamed("test.example.com/repo") _, dgst, _ := newRandomSchemaV1Manifest(repo, "latest", 6) var m testutil.RequestResponseMap m = append(m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "GET", - Route: "/v2/" + repo + "/manifests/" + dgst.String(), + Route: "/v2/" + repo.Name() + "/manifests/" + dgst.String(), }, Response: testutil.Response{ StatusCode: http.StatusUnauthorized, diff --git a/docs/handlers/api_test.go b/docs/handlers/api_test.go index 206a461e8..b59db6cc0 100644 --- a/docs/handlers/api_test.go +++ b/docs/handlers/api_test.go @@ -26,6 +26,7 @@ import ( "github.com/docker/distribution/manifest/manifestlist" "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/manifest/schema2" + "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" _ "github.com/docker/distribution/registry/storage/driver/inmemory" @@ -251,7 +252,7 @@ func TestURLPrefix(t *testing.T) { } type blobArgs struct { - imageName string + imageName reference.Named layerFile io.ReadSeeker layerDigest digest.Digest } @@ -263,10 +264,10 @@ func makeBlobArgs(t *testing.T) blobArgs { } args := blobArgs{ - imageName: "foo/bar", layerFile: layerFile, layerDigest: layerDigest, } + args.imageName, _ = reference.ParseNamed("foo/bar") return args } @@ -609,7 +610,7 @@ func testBlobDelete(t *testing.T, env *testEnv, args blobArgs) { func TestDeleteDisabled(t *testing.T) { env := newTestEnv(t, false) - imageName := "foo/bar" + imageName, _ := reference.ParseNamed("foo/bar") // "build" our layer file layerFile, layerDigest, err := testutil.CreateRandomTarFile() if err != nil { @@ -634,7 +635,7 @@ func TestDeleteDisabled(t *testing.T) { func TestDeleteReadOnly(t *testing.T) { env := newTestEnv(t, true) - imageName := "foo/bar" + imageName, _ := reference.ParseNamed("foo/bar") // "build" our layer file layerFile, layerDigest, err := testutil.CreateRandomTarFile() if err != nil { @@ -662,7 +663,7 @@ func TestStartPushReadOnly(t *testing.T) { env := newTestEnv(t, true) env.app.readOnly = true - imageName := "foo/bar" + imageName, _ := reference.ParseNamed("foo/bar") layerUploadURL, err := env.builder.BuildBlobUploadURL(imageName) if err != nil { @@ -693,42 +694,49 @@ func httpDelete(url string) (*http.Response, error) { } type manifestArgs struct { - imageName string + imageName reference.Named mediaType string manifest distribution.Manifest dgst digest.Digest } func TestManifestAPI(t *testing.T) { + schema1Repo, _ := reference.ParseNamed("foo/schema1") + schema2Repo, _ := reference.ParseNamed("foo/schema2") + deleteEnabled := false env := newTestEnv(t, deleteEnabled) - testManifestAPISchema1(t, env, "foo/schema1") - schema2Args := testManifestAPISchema2(t, env, "foo/schema2") + testManifestAPISchema1(t, env, schema1Repo) + schema2Args := testManifestAPISchema2(t, env, schema2Repo) testManifestAPIManifestList(t, env, schema2Args) deleteEnabled = true env = newTestEnv(t, deleteEnabled) - testManifestAPISchema1(t, env, "foo/schema1") - schema2Args = testManifestAPISchema2(t, env, "foo/schema2") + testManifestAPISchema1(t, env, schema1Repo) + schema2Args = testManifestAPISchema2(t, env, schema2Repo) testManifestAPIManifestList(t, env, schema2Args) } func TestManifestDelete(t *testing.T) { + schema1Repo, _ := reference.ParseNamed("foo/schema1") + schema2Repo, _ := reference.ParseNamed("foo/schema2") + deleteEnabled := true env := newTestEnv(t, deleteEnabled) - schema1Args := testManifestAPISchema1(t, env, "foo/schema1") + schema1Args := testManifestAPISchema1(t, env, schema1Repo) testManifestDelete(t, env, schema1Args) - schema2Args := testManifestAPISchema2(t, env, "foo/schema2") + schema2Args := testManifestAPISchema2(t, env, schema2Repo) testManifestDelete(t, env, schema2Args) } func TestManifestDeleteDisabled(t *testing.T) { + schema1Repo, _ := reference.ParseNamed("foo/schema1") deleteEnabled := false env := newTestEnv(t, deleteEnabled) - testManifestDeleteDisabled(t, env, "foo/schema1") + testManifestDeleteDisabled(t, env, schema1Repo) } -func testManifestDeleteDisabled(t *testing.T, env *testEnv, imageName string) { +func testManifestDeleteDisabled(t *testing.T, env *testEnv, imageName reference.Named) { manifestURL, err := env.builder.BuildManifestURL(imageName, digest.DigestSha256EmptyTar) if err != nil { t.Fatalf("unexpected error getting manifest url: %v", err) @@ -743,7 +751,7 @@ func testManifestDeleteDisabled(t *testing.T, env *testEnv, imageName string) { checkResponse(t, "status of disabled delete of manifest", resp, http.StatusMethodNotAllowed) } -func testManifestAPISchema1(t *testing.T, env *testEnv, imageName string) manifestArgs { +func testManifestAPISchema1(t *testing.T, env *testEnv, imageName reference.Named) manifestArgs { tag := "thetag" args := manifestArgs{imageName: imageName} @@ -784,7 +792,7 @@ func testManifestAPISchema1(t *testing.T, env *testEnv, imageName string) manife Versioned: manifest.Versioned{ SchemaVersion: 1, }, - Name: imageName, + Name: imageName.Name(), Tag: tag, FSLayers: []schema1.FSLayer{ { @@ -1032,8 +1040,8 @@ func testManifestAPISchema1(t *testing.T, env *testEnv, imageName string) manife t.Fatalf("unexpected error decoding error response: %v", err) } - if tagsResponse.Name != imageName { - t.Fatalf("tags name should match image name: %v != %v", tagsResponse.Name, imageName) + if tagsResponse.Name != imageName.Name() { + t.Fatalf("tags name should match image name: %v != %v", tagsResponse.Name, imageName.Name()) } if len(tagsResponse.Tags) != 1 { @@ -1060,7 +1068,7 @@ func testManifestAPISchema1(t *testing.T, env *testEnv, imageName string) manife return args } -func testManifestAPISchema2(t *testing.T, env *testEnv, imageName string) manifestArgs { +func testManifestAPISchema2(t *testing.T, env *testEnv, imageName reference.Named) manifestArgs { tag := "schema2tag" args := manifestArgs{ imageName: imageName, @@ -1340,7 +1348,7 @@ func testManifestAPISchema2(t *testing.T, env *testEnv, imageName string) manife t.Fatalf("unexpected error decoding error response: %v", err) } - if tagsResponse.Name != imageName { + if tagsResponse.Name != imageName.Name() { t.Fatalf("tags name should match image name: %v != %v", tagsResponse.Name, imageName) } @@ -1379,7 +1387,7 @@ func testManifestAPISchema2(t *testing.T, env *testEnv, imageName string) manife if fetchedSchema1Manifest.Architecture != "amd64" { t.Fatal("wrong architecture") } - if fetchedSchema1Manifest.Name != imageName { + if fetchedSchema1Manifest.Name != imageName.Name() { t.Fatal("wrong image name") } if fetchedSchema1Manifest.Tag != tag { @@ -1602,7 +1610,7 @@ func testManifestAPIManifestList(t *testing.T, env *testEnv, args manifestArgs) if fetchedSchema1Manifest.Architecture != "amd64" { t.Fatal("wrong architecture") } - if fetchedSchema1Manifest.Name != imageName { + if fetchedSchema1Manifest.Name != imageName.Name() { t.Fatal("wrong image name") } if fetchedSchema1Manifest.Tag != tag { @@ -1715,7 +1723,7 @@ func testManifestDelete(t *testing.T, env *testEnv, args manifestArgs) { t.Fatalf("unexpected error decoding error response: %v", err) } - if tagsResponse.Name != imageName { + if tagsResponse.Name != imageName.Name() { t.Fatalf("tags name should match image name: %v != %v", tagsResponse.Name, imageName) } @@ -1749,7 +1757,7 @@ func testManifestDelete(t *testing.T, env *testEnv, args manifestArgs) { t.Fatalf("unexpected error decoding error response: %v", err) } - if tagsResponse.Name != imageName { + if tagsResponse.Name != imageName.Name() { t.Fatalf("tags name should match image name: %v != %v", tagsResponse.Name, imageName) } @@ -1863,7 +1871,7 @@ func putManifest(t *testing.T, msg, url, contentType string, v interface{}) *htt return resp } -func startPushLayer(t *testing.T, ub *v2.URLBuilder, name string) (location string, uuid string) { +func startPushLayer(t *testing.T, ub *v2.URLBuilder, name reference.Named) (location string, uuid string) { layerUploadURL, err := ub.BuildBlobUploadURL(name) if err != nil { t.Fatalf("unexpected error building layer upload url: %v", err) @@ -1875,7 +1883,7 @@ func startPushLayer(t *testing.T, ub *v2.URLBuilder, name string) (location stri } defer resp.Body.Close() - checkResponse(t, fmt.Sprintf("pushing starting layer push %v", name), resp, http.StatusAccepted) + checkResponse(t, fmt.Sprintf("pushing starting layer push %v", name.String()), resp, http.StatusAccepted) u, err := url.Parse(resp.Header.Get("Location")) if err != nil { @@ -1894,7 +1902,7 @@ func startPushLayer(t *testing.T, ub *v2.URLBuilder, name string) (location stri // doPushLayer pushes the layer content returning the url on success returning // the response. If you're only expecting a successful response, use pushLayer. -func doPushLayer(t *testing.T, ub *v2.URLBuilder, name string, dgst digest.Digest, uploadURLBase string, body io.Reader) (*http.Response, error) { +func doPushLayer(t *testing.T, ub *v2.URLBuilder, name reference.Named, dgst digest.Digest, uploadURLBase string, body io.Reader) (*http.Response, error) { u, err := url.Parse(uploadURLBase) if err != nil { t.Fatalf("unexpected error parsing pushLayer url: %v", err) @@ -1918,7 +1926,7 @@ func doPushLayer(t *testing.T, ub *v2.URLBuilder, name string, dgst digest.Diges } // pushLayer pushes the layer content returning the url on success. -func pushLayer(t *testing.T, ub *v2.URLBuilder, name string, dgst digest.Digest, uploadURLBase string, body io.Reader) string { +func pushLayer(t *testing.T, ub *v2.URLBuilder, name reference.Named, dgst digest.Digest, uploadURLBase string, body io.Reader) string { digester := digest.Canonical.New() resp, err := doPushLayer(t, ub, name, dgst, uploadURLBase, io.TeeReader(body, digester.Hash())) @@ -1949,7 +1957,7 @@ func pushLayer(t *testing.T, ub *v2.URLBuilder, name string, dgst digest.Digest, return resp.Header.Get("Location") } -func finishUpload(t *testing.T, ub *v2.URLBuilder, name string, uploadURLBase string, dgst digest.Digest) string { +func finishUpload(t *testing.T, ub *v2.URLBuilder, name reference.Named, uploadURLBase string, dgst digest.Digest) string { resp, err := doPushLayer(t, ub, name, dgst, uploadURLBase, nil) if err != nil { t.Fatalf("unexpected error doing push layer request: %v", err) @@ -1997,7 +2005,7 @@ func doPushChunk(t *testing.T, uploadURLBase string, body io.Reader) (*http.Resp return resp, digester.Digest(), err } -func pushChunk(t *testing.T, ub *v2.URLBuilder, name string, uploadURLBase string, body io.Reader, length int64) (string, digest.Digest) { +func pushChunk(t *testing.T, ub *v2.URLBuilder, name reference.Named, uploadURLBase string, body io.Reader, length int64) (string, digest.Digest) { resp, dgst, err := doPushChunk(t, uploadURLBase, body) if err != nil { t.Fatalf("unexpected error doing push layer request: %v", err) @@ -2133,6 +2141,11 @@ func checkErr(t *testing.T, err error, msg string) { } func createRepository(env *testEnv, t *testing.T, imageName string, tag string) digest.Digest { + imageNameRef, err := reference.ParseNamed(imageName) + if err != nil { + t.Fatalf("unable to parse reference: %v", err) + } + unsignedManifest := &schema1.Manifest{ Versioned: manifest.Versioned{ SchemaVersion: 1, @@ -2164,8 +2177,8 @@ func createRepository(env *testEnv, t *testing.T, imageName string, tag string) expectedLayers[dgst] = rs unsignedManifest.FSLayers[i].BlobSum = dgst - uploadURLBase, _ := startPushLayer(t, env.builder, imageName) - pushLayer(t, env.builder, imageName, dgst, uploadURLBase, rs) + uploadURLBase, _ := startPushLayer(t, env.builder, imageNameRef) + pushLayer(t, env.builder, imageNameRef, dgst, uploadURLBase, rs) } signedManifest, err := schema1.Sign(unsignedManifest, env.pk) @@ -2176,10 +2189,10 @@ func createRepository(env *testEnv, t *testing.T, imageName string, tag string) dgst := digest.FromBytes(signedManifest.Canonical) // Create this repository by tag to ensure the tag mapping is made in the registry - manifestDigestURL, err := env.builder.BuildManifestURL(imageName, tag) + manifestDigestURL, err := env.builder.BuildManifestURL(imageNameRef, tag) checkErr(t, err, "building manifest url") - location, err := env.builder.BuildManifestURL(imageName, dgst.String()) + location, err := env.builder.BuildManifestURL(imageNameRef, dgst.String()) checkErr(t, err, "building location URL") resp := putManifest(t, "putting signed manifest", manifestDigestURL, "", signedManifest) @@ -2197,7 +2210,7 @@ func TestRegistryAsCacheMutationAPIs(t *testing.T) { deleteEnabled := true env := newTestEnvMirror(t, deleteEnabled) - imageName := "foo/bar" + imageName, _ := reference.ParseNamed("foo/bar") tag := "latest" manifestURL, err := env.builder.BuildManifestURL(imageName, tag) if err != nil { @@ -2209,7 +2222,7 @@ func TestRegistryAsCacheMutationAPIs(t *testing.T) { Versioned: manifest.Versioned{ SchemaVersion: 1, }, - Name: imageName, + Name: imageName.Name(), Tag: tag, FSLayers: []schema1.FSLayer{}, History: []schema1.History{}, @@ -2284,12 +2297,12 @@ func TestProxyManifestGetByTag(t *testing.T) { } truthConfig.HTTP.Headers = headerConfig - imageName := "foo/bar" + imageName, _ := reference.ParseNamed("foo/bar") tag := "latest" truthEnv := newTestEnvWithConfig(t, &truthConfig) // create a repository in the truth registry - dgst := createRepository(truthEnv, t, imageName, tag) + dgst := createRepository(truthEnv, t, imageName.Name(), tag) proxyConfig := configuration.Configuration{ Storage: configuration.Storage{ @@ -2322,7 +2335,7 @@ func TestProxyManifestGetByTag(t *testing.T) { }) // Create another manifest in the remote with the same image/tag pair - newDigest := createRepository(truthEnv, t, imageName, tag) + newDigest := createRepository(truthEnv, t, imageName.Name(), tag) if dgst == newDigest { t.Fatalf("non-random test data") } diff --git a/docs/handlers/app.go b/docs/handlers/app.go index 232254932..70b7417f5 100644 --- a/docs/handlers/app.go +++ b/docs/handlers/app.go @@ -18,6 +18,7 @@ import ( "github.com/docker/distribution/health" "github.com/docker/distribution/health/checks" "github.com/docker/distribution/notifications" + "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/registry/auth" @@ -590,7 +591,19 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { context.Context = ctxu.WithLogger(context.Context, ctxu.GetLogger(context.Context, "auth.user.name")) if app.nameRequired(r) { - repository, err := app.registry.Repository(context, getName(context)) + nameRef, err := reference.ParseNamed(getName(context)) + if err != nil { + ctxu.GetLogger(context).Errorf("error parsing reference from context: %v", err) + context.Errors = append(context.Errors, distribution.ErrRepositoryNameInvalid{ + Name: getName(context), + Reason: err, + }) + if err := errcode.ServeJSON(w, context.Errors); err != nil { + ctxu.GetLogger(context).Errorf("error serving error json: %v (from %v)", err, context.Errors) + } + return + } + repository, err := app.registry.Repository(context, nameRef) if err != nil { ctxu.GetLogger(context).Errorf("error resolving repository: %v", err) diff --git a/docs/handlers/app_test.go b/docs/handlers/app_test.go index de27f443b..907ae53a2 100644 --- a/docs/handlers/app_test.go +++ b/docs/handlers/app_test.go @@ -48,7 +48,7 @@ func TestAppDispatcher(t *testing.T) { varCheckingDispatcher := func(expectedVars map[string]string) dispatchFunc { return func(ctx *Context, r *http.Request) http.Handler { // Always checks the same name context - if ctx.Repository.Name() != getName(ctx) { + if ctx.Repository.Name().Name() != getName(ctx) { t.Fatalf("unexpected name: %q != %q", ctx.Repository.Name(), "foo/bar") } diff --git a/docs/handlers/blobupload.go b/docs/handlers/blobupload.go index 1e3bff955..56403dd9f 100644 --- a/docs/handlers/blobupload.go +++ b/docs/handlers/blobupload.go @@ -46,7 +46,7 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { } buh.State = state - if state.Name != ctx.Repository.Name() { + if state.Name != ctx.Repository.Name().Name() { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("mismatched repository name in upload state: %q != %q", state.Name, buh.Repository.Name()) buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) @@ -312,7 +312,7 @@ func (buh *blobUploadHandler) blobUploadResponse(w http.ResponseWriter, r *http. } // TODO(stevvooe): Need a better way to manage the upload state automatically. - buh.State.Name = buh.Repository.Name() + buh.State.Name = buh.Repository.Name().Name() buh.State.UUID = buh.Upload.ID() buh.State.Offset = offset buh.State.StartedAt = buh.Upload.StartedAt() diff --git a/docs/handlers/images.go b/docs/handlers/images.go index 51156d3b4..9b4e53998 100644 --- a/docs/handlers/images.go +++ b/docs/handlers/images.go @@ -11,6 +11,7 @@ import ( "github.com/docker/distribution/manifest/manifestlist" "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/manifest/schema2" + "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "github.com/gorilla/handlers" @@ -173,7 +174,17 @@ func (imh *imageManifestHandler) convertSchema2Manifest(schema2Manifest *schema2 return nil, err } - builder := schema1.NewConfigManifestBuilder(imh.Repository.Blobs(imh), imh.Context.App.trustKey, imh.Repository.Name(), imh.Tag, configJSON) + ref := imh.Repository.Name() + + if imh.Tag != "" { + ref, err = reference.WithTag(imh.Repository.Name(), imh.Tag) + if err != nil { + imh.Errors = append(imh.Errors, v2.ErrorCodeTagInvalid.WithDetail(err)) + return nil, err + } + } + + builder := schema1.NewConfigManifestBuilder(imh.Repository.Blobs(imh), imh.Context.App.trustKey, ref, configJSON) for _, d := range schema2Manifest.References() { if err := builder.AppendReference(d); err != nil { imh.Errors = append(imh.Errors, v2.ErrorCodeManifestInvalid.WithDetail(err)) diff --git a/docs/handlers/tags.go b/docs/handlers/tags.go index d9f0106c9..72c21bbe8 100644 --- a/docs/handlers/tags.go +++ b/docs/handlers/tags.go @@ -40,7 +40,7 @@ func (th *tagsHandler) GetTags(w http.ResponseWriter, r *http.Request) { if err != nil { switch err := err.(type) { case distribution.ErrRepositoryUnknown: - th.Errors = append(th.Errors, v2.ErrorCodeNameUnknown.WithDetail(map[string]string{"name": th.Repository.Name()})) + th.Errors = append(th.Errors, v2.ErrorCodeNameUnknown.WithDetail(map[string]string{"name": th.Repository.Name().Name()})) default: th.Errors = append(th.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) } @@ -51,7 +51,7 @@ func (th *tagsHandler) GetTags(w http.ResponseWriter, r *http.Request) { enc := json.NewEncoder(w) if err := enc.Encode(tagsAPIResponse{ - Name: th.Repository.Name(), + Name: th.Repository.Name().Name(), Tags: tags, }); err != nil { th.Errors = append(th.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) diff --git a/docs/proxy/proxyblobstore.go b/docs/proxy/proxyblobstore.go index 41b76e8ee..278e5864d 100644 --- a/docs/proxy/proxyblobstore.go +++ b/docs/proxy/proxyblobstore.go @@ -10,6 +10,7 @@ import ( "github.com/docker/distribution" "github.com/docker/distribution/context" "github.com/docker/distribution/digest" + "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/proxy/scheduler" ) @@ -133,7 +134,7 @@ func (pbs *proxyBlobStore) ServeBlob(ctx context.Context, w http.ResponseWriter, if err := pbs.storeLocal(ctx, dgst); err != nil { context.GetLogger(ctx).Errorf("Error committing to storage: %s", err.Error()) } - pbs.scheduler.AddBlob(dgst.String(), repositoryTTL) + pbs.scheduler.AddBlob(dgst, repositoryTTL) }(dgst) _, err = pbs.copyContent(ctx, dgst, w) @@ -169,7 +170,7 @@ func (pbs *proxyBlobStore) Resume(ctx context.Context, id string) (distribution. return nil, distribution.ErrUnsupported } -func (pbs *proxyBlobStore) Mount(ctx context.Context, sourceRepo string, dgst digest.Digest) (distribution.Descriptor, error) { +func (pbs *proxyBlobStore) Mount(ctx context.Context, sourceRepo reference.Named, dgst digest.Digest) (distribution.Descriptor, error) { return distribution.Descriptor{}, distribution.ErrUnsupported } diff --git a/docs/proxy/proxyblobstore_test.go b/docs/proxy/proxyblobstore_test.go index 7702771cd..978f878ef 100644 --- a/docs/proxy/proxyblobstore_test.go +++ b/docs/proxy/proxyblobstore_test.go @@ -12,6 +12,7 @@ import ( "github.com/docker/distribution" "github.com/docker/distribution/context" "github.com/docker/distribution/digest" + "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/proxy/scheduler" "github.com/docker/distribution/registry/storage" "github.com/docker/distribution/registry/storage/cache/memory" @@ -114,6 +115,11 @@ func (te *testEnv) RemoteStats() *map[string]int { // Populate remote store and record the digests func makeTestEnv(t *testing.T, name string) *testEnv { + nameRef, err := reference.ParseNamed(name) + if err != nil { + t.Fatalf("unable to parse reference: %s", err) + } + ctx := context.Background() truthDir, err := ioutil.TempDir("", "truth") @@ -131,7 +137,7 @@ func makeTestEnv(t *testing.T, name string) *testEnv { if err != nil { t.Fatalf("error creating registry: %v", err) } - localRepo, err := localRegistry.Repository(ctx, name) + localRepo, err := localRegistry.Repository(ctx, nameRef) if err != nil { t.Fatalf("unexpected error getting repo: %v", err) } @@ -140,7 +146,7 @@ func makeTestEnv(t *testing.T, name string) *testEnv { if err != nil { t.Fatalf("error creating registry: %v", err) } - truthRepo, err := truthRegistry.Repository(ctx, name) + truthRepo, err := truthRegistry.Repository(ctx, nameRef) if err != nil { t.Fatalf("unexpected error getting repo: %v", err) } diff --git a/docs/proxy/proxymanifeststore.go b/docs/proxy/proxymanifeststore.go index 13cb5f6b9..e0a5ac280 100644 --- a/docs/proxy/proxymanifeststore.go +++ b/docs/proxy/proxymanifeststore.go @@ -6,6 +6,7 @@ import ( "github.com/docker/distribution" "github.com/docker/distribution/context" "github.com/docker/distribution/digest" + "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/proxy/scheduler" ) @@ -16,7 +17,7 @@ type proxyManifestStore struct { ctx context.Context localManifests distribution.ManifestService remoteManifests distribution.ManifestService - repositoryName string + repositoryName reference.Named scheduler *scheduler.TTLExpirationScheduler } @@ -65,7 +66,7 @@ func (pms proxyManifestStore) Get(ctx context.Context, dgst digest.Digest, optio pms.scheduler.AddManifest(pms.repositoryName, repositoryTTL) // Ensure the manifest blob is cleaned up - pms.scheduler.AddBlob(dgst.String(), repositoryTTL) + pms.scheduler.AddBlob(dgst, repositoryTTL) } return manifest, err diff --git a/docs/proxy/proxymanifeststore_test.go b/docs/proxy/proxymanifeststore_test.go index aeecae10a..5e717bf05 100644 --- a/docs/proxy/proxymanifeststore_test.go +++ b/docs/proxy/proxymanifeststore_test.go @@ -9,6 +9,7 @@ import ( "github.com/docker/distribution/digest" "github.com/docker/distribution/manifest" "github.com/docker/distribution/manifest/schema1" + "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/proxy/scheduler" "github.com/docker/distribution/registry/storage" "github.com/docker/distribution/registry/storage/cache/memory" @@ -64,12 +65,17 @@ func (sm statsManifest) Put(ctx context.Context, manifest distribution.Manifest, */ func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestEnv { + nameRef, err := reference.ParseNamed(name) + if err != nil { + t.Fatalf("unable to parse reference: %s", err) + } + ctx := context.Background() truthRegistry, err := storage.NewRegistry(ctx, inmemory.New(), storage.BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider())) if err != nil { t.Fatalf("error creating registry: %v", err) } - truthRepo, err := truthRegistry.Repository(ctx, name) + truthRepo, err := truthRegistry.Repository(ctx, nameRef) if err != nil { t.Fatalf("unexpected error getting repo: %v", err) } @@ -91,7 +97,7 @@ func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestE if err != nil { t.Fatalf("error creating registry: %v", err) } - localRepo, err := localRegistry.Repository(ctx, name) + localRepo, err := localRegistry.Repository(ctx, nameRef) if err != nil { t.Fatalf("unexpected error getting repo: %v", err) } diff --git a/docs/proxy/proxyregistry.go b/docs/proxy/proxyregistry.go index 8e1be5f27..1b3fcf326 100644 --- a/docs/proxy/proxyregistry.go +++ b/docs/proxy/proxyregistry.go @@ -7,6 +7,7 @@ import ( "github.com/docker/distribution" "github.com/docker/distribution/configuration" "github.com/docker/distribution/context" + "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/client" "github.com/docker/distribution/registry/client/auth" "github.com/docker/distribution/registry/client/transport" @@ -71,9 +72,9 @@ func (pr *proxyingRegistry) Repositories(ctx context.Context, repos []string, la return pr.embedded.Repositories(ctx, repos, last) } -func (pr *proxyingRegistry) Repository(ctx context.Context, name string) (distribution.Repository, error) { +func (pr *proxyingRegistry) Repository(ctx context.Context, name reference.Named) (distribution.Repository, error) { tr := transport.NewTransport(http.DefaultTransport, - auth.NewAuthorizer(pr.challengeManager, auth.NewTokenHandler(http.DefaultTransport, pr.credentialStore, name, "pull"))) + auth.NewAuthorizer(pr.challengeManager, auth.NewTokenHandler(http.DefaultTransport, pr.credentialStore, name.Name(), "pull"))) localRepo, err := pr.embedded.Repository(ctx, name) if err != nil { @@ -121,7 +122,7 @@ func (pr *proxyingRegistry) Repository(ctx context.Context, name string) (distri type proxiedRepository struct { blobStore distribution.BlobStore manifests distribution.ManifestService - name string + name reference.Named tags distribution.TagService } @@ -133,7 +134,7 @@ func (pr *proxiedRepository) Blobs(ctx context.Context) distribution.BlobStore { return pr.blobStore } -func (pr *proxiedRepository) Name() string { +func (pr *proxiedRepository) Name() reference.Named { return pr.name } diff --git a/docs/proxy/scheduler/scheduler.go b/docs/proxy/scheduler/scheduler.go index e91920a1d..f53349073 100644 --- a/docs/proxy/scheduler/scheduler.go +++ b/docs/proxy/scheduler/scheduler.go @@ -7,6 +7,8 @@ import ( "time" "github.com/docker/distribution/context" + "github.com/docker/distribution/digest" + "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/storage/driver" ) @@ -80,19 +82,19 @@ func (ttles *TTLExpirationScheduler) OnManifestExpire(f expiryFunc) { } // AddBlob schedules a blob cleanup after ttl expires -func (ttles *TTLExpirationScheduler) AddBlob(dgst string, ttl time.Duration) error { +func (ttles *TTLExpirationScheduler) AddBlob(dgst digest.Digest, ttl time.Duration) error { ttles.Lock() defer ttles.Unlock() if ttles.stopped { return fmt.Errorf("scheduler not started") } - ttles.add(dgst, ttl, entryTypeBlob) + ttles.add(dgst.String(), ttl, entryTypeBlob) return nil } // AddManifest schedules a manifest cleanup after ttl expires -func (ttles *TTLExpirationScheduler) AddManifest(repoName string, ttl time.Duration) error { +func (ttles *TTLExpirationScheduler) AddManifest(repoName reference.Named, ttl time.Duration) error { ttles.Lock() defer ttles.Unlock() @@ -100,7 +102,7 @@ func (ttles *TTLExpirationScheduler) AddManifest(repoName string, ttl time.Durat return fmt.Errorf("scheduler not started") } - ttles.add(repoName, ttl, entryTypeManifest) + ttles.add(repoName.Name(), ttl, entryTypeManifest) return nil } diff --git a/docs/storage/blob_test.go b/docs/storage/blob_test.go index e1eacc003..246648b0c 100644 --- a/docs/storage/blob_test.go +++ b/docs/storage/blob_test.go @@ -27,7 +27,7 @@ func TestSimpleBlobUpload(t *testing.T) { } ctx := context.Background() - imageName := "foo/bar" + imageName, _ := reference.ParseNamed("foo/bar") driver := inmemory.New() registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect) if err != nil { @@ -209,7 +209,7 @@ func TestSimpleBlobUpload(t *testing.T) { // other tests. func TestSimpleBlobRead(t *testing.T) { ctx := context.Background() - imageName := "foo/bar" + imageName, _ := reference.ParseNamed("foo/bar") driver := inmemory.New() registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect) if err != nil { @@ -320,8 +320,8 @@ func TestBlobMount(t *testing.T) { } ctx := context.Background() - imageName := "foo/bar" - sourceImageName := "foo/source" + imageName, _ := reference.ParseNamed("foo/bar") + sourceImageName, _ := reference.ParseNamed("foo/source") driver := inmemory.New() registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect) if err != nil { @@ -378,11 +378,7 @@ func TestBlobMount(t *testing.T) { t.Fatalf("unexpected non-error stating unmounted blob: %v", desc) } - namedRef, err := reference.ParseNamed(sourceRepository.Name()) - if err != nil { - t.Fatal(err) - } - canonicalRef, err := reference.WithDigest(namedRef, desc.Digest) + canonicalRef, err := reference.WithDigest(sourceRepository.Name(), desc.Digest) if err != nil { t.Fatal(err) } @@ -476,7 +472,7 @@ func TestBlobMount(t *testing.T) { // TestLayerUploadZeroLength uploads zero-length func TestLayerUploadZeroLength(t *testing.T) { ctx := context.Background() - imageName := "foo/bar" + imageName, _ := reference.ParseNamed("foo/bar") driver := inmemory.New() registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect) if err != nil { diff --git a/docs/storage/blobwriter.go b/docs/storage/blobwriter.go index 379031760..e485cc6d0 100644 --- a/docs/storage/blobwriter.go +++ b/docs/storage/blobwriter.go @@ -326,7 +326,7 @@ func (bw *blobWriter) moveBlob(ctx context.Context, desc distribution.Descriptor // resources are already not present, no error will be returned. func (bw *blobWriter) removeResources(ctx context.Context) error { dataPath, err := pathFor(uploadDataPathSpec{ - name: bw.blobStore.repository.Name(), + name: bw.blobStore.repository.Name().Name(), id: bw.id, }) diff --git a/docs/storage/blobwriter_resumable.go b/docs/storage/blobwriter_resumable.go index d33f544da..fc62bcc45 100644 --- a/docs/storage/blobwriter_resumable.go +++ b/docs/storage/blobwriter_resumable.go @@ -113,7 +113,7 @@ type hashStateEntry struct { // getStoredHashStates returns a slice of hashStateEntries for this upload. func (bw *blobWriter) getStoredHashStates(ctx context.Context) ([]hashStateEntry, error) { uploadHashStatePathPrefix, err := pathFor(uploadHashStatePathSpec{ - name: bw.blobStore.repository.Name(), + name: bw.blobStore.repository.Name().String(), id: bw.id, alg: bw.digester.Digest().Algorithm(), list: true, @@ -159,7 +159,7 @@ func (bw *blobWriter) storeHashState(ctx context.Context) error { } uploadHashStatePath, err := pathFor(uploadHashStatePathSpec{ - name: bw.blobStore.repository.Name(), + name: bw.blobStore.repository.Name().String(), id: bw.id, alg: bw.digester.Digest().Algorithm(), offset: int64(h.Len()), diff --git a/docs/storage/linkedblobstore.go b/docs/storage/linkedblobstore.go index a1f8724d7..0c0c622c8 100644 --- a/docs/storage/linkedblobstore.go +++ b/docs/storage/linkedblobstore.go @@ -142,7 +142,7 @@ func (lbs *linkedBlobStore) Create(ctx context.Context, options ...distribution. } if opts.Mount.ShouldMount { - desc, err := lbs.mount(ctx, opts.Mount.From.Name(), opts.Mount.From.Digest()) + desc, err := lbs.mount(ctx, opts.Mount.From, opts.Mount.From.Digest()) if err == nil { // Mount successful, no need to initiate an upload session return nil, distribution.ErrBlobMounted{From: opts.Mount.From, Descriptor: desc} @@ -153,7 +153,7 @@ func (lbs *linkedBlobStore) Create(ctx context.Context, options ...distribution. startedAt := time.Now().UTC() path, err := pathFor(uploadDataPathSpec{ - name: lbs.repository.Name(), + name: lbs.repository.Name().Name(), id: uuid, }) @@ -162,7 +162,7 @@ func (lbs *linkedBlobStore) Create(ctx context.Context, options ...distribution. } startedAtPath, err := pathFor(uploadStartedAtPathSpec{ - name: lbs.repository.Name(), + name: lbs.repository.Name().Name(), id: uuid, }) @@ -182,7 +182,7 @@ func (lbs *linkedBlobStore) Resume(ctx context.Context, id string) (distribution context.GetLogger(ctx).Debug("(*linkedBlobStore).Resume") startedAtPath, err := pathFor(uploadStartedAtPathSpec{ - name: lbs.repository.Name(), + name: lbs.repository.Name().Name(), id: id, }) @@ -206,7 +206,7 @@ func (lbs *linkedBlobStore) Resume(ctx context.Context, id string) (distribution } path, err := pathFor(uploadDataPathSpec{ - name: lbs.repository.Name(), + name: lbs.repository.Name().Name(), id: id, }) @@ -236,7 +236,7 @@ func (lbs *linkedBlobStore) Delete(ctx context.Context, dgst digest.Digest) erro return nil } -func (lbs *linkedBlobStore) mount(ctx context.Context, sourceRepo string, dgst digest.Digest) (distribution.Descriptor, error) { +func (lbs *linkedBlobStore) mount(ctx context.Context, sourceRepo reference.Named, dgst digest.Digest) (distribution.Descriptor, error) { repo, err := lbs.registry.Repository(ctx, sourceRepo) if err != nil { return distribution.Descriptor{}, err @@ -298,7 +298,7 @@ func (lbs *linkedBlobStore) linkBlob(ctx context.Context, canonical distribution } seenDigests[dgst] = struct{}{} - blobLinkPath, err := linkPathFn(lbs.repository.Name(), dgst) + blobLinkPath, err := linkPathFn(lbs.repository.Name().Name(), dgst) if err != nil { return err } @@ -368,7 +368,7 @@ func (lbs *linkedBlobStatter) Stat(ctx context.Context, dgst digest.Digest) (dis func (lbs *linkedBlobStatter) Clear(ctx context.Context, dgst digest.Digest) (err error) { // clear any possible existence of a link described in linkPathFns for _, linkPathFn := range lbs.linkPathFns { - blobLinkPath, err := linkPathFn(lbs.repository.Name(), dgst) + blobLinkPath, err := linkPathFn(lbs.repository.Name().Name(), dgst) if err != nil { return err } @@ -391,7 +391,7 @@ func (lbs *linkedBlobStatter) Clear(ctx context.Context, dgst digest.Digest) (er // linkPathFuncs to let us try a few different paths before returning not // found. func (lbs *linkedBlobStatter) resolveWithLinkFunc(ctx context.Context, dgst digest.Digest, linkPathFn linkPathFunc) (digest.Digest, error) { - blobLinkPath, err := linkPathFn(lbs.repository.Name(), dgst) + blobLinkPath, err := linkPathFn(lbs.repository.Name().Name(), dgst) if err != nil { return "", err } diff --git a/docs/storage/manifeststore.go b/docs/storage/manifeststore.go index 31daa83ca..33c0c3514 100644 --- a/docs/storage/manifeststore.go +++ b/docs/storage/manifeststore.go @@ -77,7 +77,7 @@ func (ms *manifestStore) Get(ctx context.Context, dgst digest.Digest, options .. if err != nil { if err == distribution.ErrBlobUnknown { return nil, distribution.ErrManifestUnknownRevision{ - Name: ms.repository.Name(), + Name: ms.repository.Name().Name(), Revision: dgst, } } diff --git a/docs/storage/manifeststore_test.go b/docs/storage/manifeststore_test.go index a41feb045..7885c4662 100644 --- a/docs/storage/manifeststore_test.go +++ b/docs/storage/manifeststore_test.go @@ -11,6 +11,7 @@ import ( "github.com/docker/distribution/digest" "github.com/docker/distribution/manifest" "github.com/docker/distribution/manifest/schema1" + "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/storage/cache/memory" "github.com/docker/distribution/registry/storage/driver" "github.com/docker/distribution/registry/storage/driver/inmemory" @@ -23,11 +24,11 @@ type manifestStoreTestEnv struct { driver driver.StorageDriver registry distribution.Namespace repository distribution.Repository - name string + name reference.Named tag string } -func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestEnv { +func newManifestStoreTestEnv(t *testing.T, name reference.Named, tag string) *manifestStoreTestEnv { ctx := context.Background() driver := inmemory.New() registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider( @@ -52,7 +53,8 @@ func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestE } func TestManifestStorage(t *testing.T) { - env := newManifestStoreTestEnv(t, "foo/bar", "thetag") + repoName, _ := reference.ParseNamed("foo/bar") + env := newManifestStoreTestEnv(t, repoName, "thetag") ctx := context.Background() ms, err := env.repository.Manifests(ctx) if err != nil { @@ -63,7 +65,7 @@ func TestManifestStorage(t *testing.T) { Versioned: manifest.Versioned{ SchemaVersion: 1, }, - Name: env.name, + Name: env.name.Name(), Tag: env.tag, } diff --git a/docs/storage/registry.go b/docs/storage/registry.go index 869895dd9..be570cbcb 100644 --- a/docs/storage/registry.go +++ b/docs/storage/registry.go @@ -107,18 +107,11 @@ func (reg *registry) Scope() distribution.Scope { // Repository returns an instance of the repository tied to the registry. // Instances should not be shared between goroutines but are cheap to // allocate. In general, they should be request scoped. -func (reg *registry) Repository(ctx context.Context, canonicalName string) (distribution.Repository, error) { - if _, err := reference.ParseNamed(canonicalName); err != nil { - return nil, distribution.ErrRepositoryNameInvalid{ - Name: canonicalName, - Reason: err, - } - } - +func (reg *registry) Repository(ctx context.Context, canonicalName reference.Named) (distribution.Repository, error) { var descriptorCache distribution.BlobDescriptorService if reg.blobDescriptorCacheProvider != nil { var err error - descriptorCache, err = reg.blobDescriptorCacheProvider.RepositoryScoped(canonicalName) + descriptorCache, err = reg.blobDescriptorCacheProvider.RepositoryScoped(canonicalName.Name()) if err != nil { return nil, err } @@ -136,12 +129,12 @@ func (reg *registry) Repository(ctx context.Context, canonicalName string) (dist type repository struct { *registry ctx context.Context - name string + name reference.Named descriptorCache distribution.BlobDescriptorService } // Name returns the name of the repository. -func (repo *repository) Name() string { +func (repo *repository) Name() reference.Named { return repo.name } diff --git a/docs/storage/signaturestore.go b/docs/storage/signaturestore.go index ede4e0e2a..205d6009e 100644 --- a/docs/storage/signaturestore.go +++ b/docs/storage/signaturestore.go @@ -16,7 +16,7 @@ type signatureStore struct { func (s *signatureStore) Get(dgst digest.Digest) ([][]byte, error) { signaturesPath, err := pathFor(manifestSignaturesPathSpec{ - name: s.repository.Name(), + name: s.repository.Name().Name(), revision: dgst, }) diff --git a/docs/storage/tagstore.go b/docs/storage/tagstore.go index df6e8dfa6..8381d244d 100644 --- a/docs/storage/tagstore.go +++ b/docs/storage/tagstore.go @@ -26,7 +26,7 @@ func (ts *tagStore) All(ctx context.Context) ([]string, error) { var tags []string pathSpec, err := pathFor(manifestTagPathSpec{ - name: ts.repository.Name(), + name: ts.repository.Name().Name(), }) if err != nil { return tags, err @@ -36,7 +36,7 @@ func (ts *tagStore) All(ctx context.Context) ([]string, error) { if err != nil { switch err := err.(type) { case storagedriver.PathNotFoundError: - return tags, distribution.ErrRepositoryUnknown{Name: ts.repository.Name()} + return tags, distribution.ErrRepositoryUnknown{Name: ts.repository.Name().Name()} default: return tags, err } @@ -53,7 +53,7 @@ func (ts *tagStore) All(ctx context.Context) ([]string, error) { // exists returns true if the specified manifest tag exists in the repository. func (ts *tagStore) exists(ctx context.Context, tag string) (bool, error) { tagPath, err := pathFor(manifestTagCurrentPathSpec{ - name: ts.repository.Name(), + name: ts.repository.Name().Name(), tag: tag, }) @@ -73,7 +73,7 @@ func (ts *tagStore) exists(ctx context.Context, tag string) (bool, error) { // the current tag. The digest must point to a manifest. func (ts *tagStore) Tag(ctx context.Context, tag string, desc distribution.Descriptor) error { currentPath, err := pathFor(manifestTagCurrentPathSpec{ - name: ts.repository.Name(), + name: ts.repository.Name().Name(), tag: tag, }) @@ -95,7 +95,7 @@ func (ts *tagStore) Tag(ctx context.Context, tag string, desc distribution.Descr // resolve the current revision for name and tag. func (ts *tagStore) Get(ctx context.Context, tag string) (distribution.Descriptor, error) { currentPath, err := pathFor(manifestTagCurrentPathSpec{ - name: ts.repository.Name(), + name: ts.repository.Name().Name(), tag: tag, }) @@ -119,7 +119,7 @@ func (ts *tagStore) Get(ctx context.Context, tag string) (distribution.Descripto // Untag removes the tag association func (ts *tagStore) Untag(ctx context.Context, tag string) error { tagPath, err := pathFor(manifestTagPathSpec{ - name: ts.repository.Name(), + name: ts.repository.Name().Name(), tag: tag, }) @@ -172,7 +172,7 @@ func (ts *tagStore) Lookup(ctx context.Context, desc distribution.Descriptor) ([ var tags []string for _, tag := range allTags { tagLinkPathSpec := manifestTagCurrentPathSpec{ - name: ts.repository.Name(), + name: ts.repository.Name().Name(), tag: tag, } diff --git a/docs/storage/tagstore_test.go b/docs/storage/tagstore_test.go index c257adeaf..52873a696 100644 --- a/docs/storage/tagstore_test.go +++ b/docs/storage/tagstore_test.go @@ -5,6 +5,7 @@ import ( "github.com/docker/distribution" "github.com/docker/distribution/context" + "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/storage/driver/inmemory" ) @@ -21,7 +22,8 @@ func testTagStore(t *testing.T) *tagsTestEnv { t.Fatal(err) } - repo, err := reg.Repository(ctx, "a/b") + repoRef, _ := reference.ParseNamed("a/b") + repo, err := reg.Repository(ctx, repoRef) if err != nil { t.Fatal(err) } From 6149a8c6343f01352876e2c91cc0281547abc823 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Tue, 15 Dec 2015 16:43:13 -0800 Subject: [PATCH 2/2] Change URLBuilder methods to use references for tags and digests Signed-off-by: Aaron Lehmann --- docs/api/v2/urls.go | 17 +++++--- docs/api/v2/urls_test.go | 6 ++- docs/client/repository.go | 75 +++++++++++++++++++++++++++--------- docs/handlers/api_test.go | 77 ++++++++++++++++++++++++------------- docs/handlers/blobupload.go | 6 ++- docs/handlers/images.go | 8 +++- 6 files changed, 134 insertions(+), 55 deletions(-) diff --git a/docs/api/v2/urls.go b/docs/api/v2/urls.go index 5b63ccaa5..408c7b74b 100644 --- a/docs/api/v2/urls.go +++ b/docs/api/v2/urls.go @@ -5,7 +5,6 @@ import ( "net/url" "strings" - "github.com/docker/distribution/digest" "github.com/docker/distribution/reference" "github.com/gorilla/mux" ) @@ -127,10 +126,18 @@ func (ub *URLBuilder) BuildTagsURL(name reference.Named) (string, error) { // BuildManifestURL constructs a url for the manifest identified by name and // reference. The argument reference may be either a tag or digest. -func (ub *URLBuilder) BuildManifestURL(name reference.Named, reference string) (string, error) { +func (ub *URLBuilder) BuildManifestURL(ref reference.Named) (string, error) { route := ub.cloneRoute(RouteNameManifest) - manifestURL, err := route.URL("name", name.Name(), "reference", reference) + tagOrDigest := "" + switch v := ref.(type) { + case reference.Tagged: + tagOrDigest = v.Tag() + case reference.Digested: + tagOrDigest = v.Digest().String() + } + + manifestURL, err := route.URL("name", ref.Name(), "reference", tagOrDigest) if err != nil { return "", err } @@ -139,10 +146,10 @@ func (ub *URLBuilder) BuildManifestURL(name reference.Named, reference string) ( } // BuildBlobURL constructs the url for the blob identified by name and dgst. -func (ub *URLBuilder) BuildBlobURL(name reference.Named, dgst digest.Digest) (string, error) { +func (ub *URLBuilder) BuildBlobURL(ref reference.Canonical) (string, error) { route := ub.cloneRoute(RouteNameBlob) - layerURL, err := route.URL("name", name.Name(), "digest", dgst.String()) + layerURL, err := route.URL("name", ref.Name(), "digest", ref.Digest().String()) if err != nil { return "", err } diff --git a/docs/api/v2/urls_test.go b/docs/api/v2/urls_test.go index 7dab00fcb..1af1f2618 100644 --- a/docs/api/v2/urls_test.go +++ b/docs/api/v2/urls_test.go @@ -33,14 +33,16 @@ func makeURLBuilderTestCases(urlBuilder *URLBuilder) []urlBuilderTestCase { description: "test manifest url", expectedPath: "/v2/foo/bar/manifests/tag", build: func() (string, error) { - return urlBuilder.BuildManifestURL(fooBarRef, "tag") + ref, _ := reference.WithTag(fooBarRef, "tag") + return urlBuilder.BuildManifestURL(ref) }, }, { description: "build blob url", expectedPath: "/v2/foo/bar/blobs/sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5", build: func() (string, error) { - return urlBuilder.BuildBlobURL(fooBarRef, "sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5") + ref, _ := reference.WithDigest(fooBarRef, "sha256:3b3692957d439ac1928219a83fac91e7bf96c153725526874673ae1f2023f8d5") + return urlBuilder.BuildBlobURL(ref) }, }, { diff --git a/docs/client/repository.go b/docs/client/repository.go index 43826907e..1f777adda 100644 --- a/docs/client/repository.go +++ b/docs/client/repository.go @@ -249,7 +249,11 @@ func descriptorFromResponse(response *http.Response) (distribution.Descriptor, e // to construct a descriptor for the tag. If the registry doesn't support HEADing // a manifest, fallback to GET. func (t *tags) Get(ctx context.Context, tag string) (distribution.Descriptor, error) { - u, err := t.ub.BuildManifestURL(t.name, tag) + ref, err := reference.WithTag(t.name, tag) + if err != nil { + return distribution.Descriptor{}, err + } + u, err := t.ub.BuildManifestURL(ref) if err != nil { return distribution.Descriptor{}, err } @@ -296,7 +300,11 @@ type manifests struct { } func (ms *manifests) Exists(ctx context.Context, dgst digest.Digest) (bool, error) { - u, err := ms.ub.BuildManifestURL(ms.name, dgst.String()) + ref, err := reference.WithDigest(ms.name, dgst) + if err != nil { + return false, err + } + u, err := ms.ub.BuildManifestURL(ref) if err != nil { return false, err } @@ -333,11 +341,19 @@ func (o etagOption) Apply(ms distribution.ManifestService) error { } func (ms *manifests) Get(ctx context.Context, dgst digest.Digest, options ...distribution.ManifestServiceOption) (distribution.Manifest, error) { + var ( + digestOrTag string + ref reference.Named + err error + ) - var tag string for _, option := range options { if opt, ok := option.(withTagOption); ok { - tag = opt.tag + digestOrTag = opt.tag + ref, err = reference.WithTag(ms.name, opt.tag) + if err != nil { + return nil, err + } } else { err := option.Apply(ms) if err != nil { @@ -346,14 +362,15 @@ func (ms *manifests) Get(ctx context.Context, dgst digest.Digest, options ...dis } } - var ref string - if tag != "" { - ref = tag - } else { - ref = dgst.String() + if digestOrTag == "" { + digestOrTag = dgst.String() + ref, err = reference.WithDigest(ms.name, dgst) + if err != nil { + return nil, err + } } - u, err := ms.ub.BuildManifestURL(ms.name, ref) + u, err := ms.ub.BuildManifestURL(ref) if err != nil { return nil, err } @@ -367,8 +384,8 @@ func (ms *manifests) Get(ctx context.Context, dgst digest.Digest, options ...dis req.Header.Add("Accept", t) } - if _, ok := ms.etags[ref]; ok { - req.Header.Set("If-None-Match", ms.etags[ref]) + if _, ok := ms.etags[digestOrTag]; ok { + req.Header.Set("If-None-Match", ms.etags[digestOrTag]) } resp, err := ms.client.Do(req) @@ -412,11 +429,15 @@ func (o withTagOption) Apply(m distribution.ManifestService) error { // Put puts a manifest. A tag can be specified using an options parameter which uses some shared state to hold the // tag name in order to build the correct upload URL. This state is written and read under a lock. func (ms *manifests) Put(ctx context.Context, m distribution.Manifest, options ...distribution.ManifestServiceOption) (digest.Digest, error) { - var tag string + ref := ms.name for _, option := range options { if opt, ok := option.(withTagOption); ok { - tag = opt.tag + var err error + ref, err = reference.WithTag(ref, opt.tag) + if err != nil { + return "", err + } } else { err := option.Apply(ms) if err != nil { @@ -425,7 +446,7 @@ func (ms *manifests) Put(ctx context.Context, m distribution.Manifest, options . } } - manifestURL, err := ms.ub.BuildManifestURL(ms.name, tag) + manifestURL, err := ms.ub.BuildManifestURL(ref) if err != nil { return "", err } @@ -462,7 +483,11 @@ func (ms *manifests) Put(ctx context.Context, m distribution.Manifest, options . } func (ms *manifests) Delete(ctx context.Context, dgst digest.Digest) error { - u, err := ms.ub.BuildManifestURL(ms.name, dgst.String()) + ref, err := reference.WithDigest(ms.name, dgst) + if err != nil { + return err + } + u, err := ms.ub.BuildManifestURL(ref) if err != nil { return err } @@ -527,7 +552,11 @@ func (bs *blobs) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) { } func (bs *blobs) Open(ctx context.Context, dgst digest.Digest) (distribution.ReadSeekCloser, error) { - blobURL, err := bs.ub.BuildBlobURL(bs.name, dgst) + ref, err := reference.WithDigest(bs.name, dgst) + if err != nil { + return nil, err + } + blobURL, err := bs.ub.BuildBlobURL(ref) if err != nil { return nil, err } @@ -668,7 +697,11 @@ type blobStatter struct { } func (bs *blobStatter) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { - u, err := bs.ub.BuildBlobURL(bs.name, dgst) + ref, err := reference.WithDigest(bs.name, dgst) + if err != nil { + return distribution.Descriptor{}, err + } + u, err := bs.ub.BuildBlobURL(ref) if err != nil { return distribution.Descriptor{}, err } @@ -716,7 +749,11 @@ func buildCatalogValues(maxEntries int, last string) url.Values { } func (bs *blobStatter) Clear(ctx context.Context, dgst digest.Digest) error { - blobURL, err := bs.ub.BuildBlobURL(bs.name, dgst) + ref, err := reference.WithDigest(bs.name, dgst) + if err != nil { + return err + } + blobURL, err := bs.ub.BuildBlobURL(ref) if err != nil { return err } diff --git a/docs/handlers/api_test.go b/docs/handlers/api_test.go index b59db6cc0..5fffaa5a1 100644 --- a/docs/handlers/api_test.go +++ b/docs/handlers/api_test.go @@ -301,7 +301,8 @@ func TestBlobDeleteDisabled(t *testing.T) { imageName := args.imageName layerDigest := args.layerDigest - layerURL, err := env.builder.BuildBlobURL(imageName, layerDigest) + ref, _ := reference.WithDigest(imageName, layerDigest) + layerURL, err := env.builder.BuildBlobURL(ref) if err != nil { t.Fatalf("error building url: %v", err) } @@ -324,7 +325,8 @@ func testBlobAPI(t *testing.T, env *testEnv, args blobArgs) *testEnv { // ----------------------------------- // Test fetch for non-existent content - layerURL, err := env.builder.BuildBlobURL(imageName, layerDigest) + ref, _ := reference.WithDigest(imageName, layerDigest) + layerURL, err := env.builder.BuildBlobURL(ref) if err != nil { t.Fatalf("error building url: %v", err) } @@ -534,7 +536,8 @@ func testBlobDelete(t *testing.T, env *testEnv, args blobArgs) { layerFile := args.layerFile layerDigest := args.layerDigest - layerURL, err := env.builder.BuildBlobURL(imageName, layerDigest) + ref, _ := reference.WithDigest(imageName, layerDigest) + layerURL, err := env.builder.BuildBlobURL(ref) if err != nil { t.Fatalf(err.Error()) } @@ -617,7 +620,8 @@ func TestDeleteDisabled(t *testing.T) { t.Fatalf("error creating random layer file: %v", err) } - layerURL, err := env.builder.BuildBlobURL(imageName, layerDigest) + ref, _ := reference.WithDigest(imageName, layerDigest) + layerURL, err := env.builder.BuildBlobURL(ref) if err != nil { t.Fatalf("Error building blob URL") } @@ -642,7 +646,8 @@ func TestDeleteReadOnly(t *testing.T) { t.Fatalf("error creating random layer file: %v", err) } - layerURL, err := env.builder.BuildBlobURL(imageName, layerDigest) + ref, _ := reference.WithDigest(imageName, layerDigest) + layerURL, err := env.builder.BuildBlobURL(ref) if err != nil { t.Fatalf("Error building blob URL") } @@ -737,7 +742,8 @@ func TestManifestDeleteDisabled(t *testing.T) { } func testManifestDeleteDisabled(t *testing.T, env *testEnv, imageName reference.Named) { - manifestURL, err := env.builder.BuildManifestURL(imageName, digest.DigestSha256EmptyTar) + ref, _ := reference.WithDigest(imageName, digest.DigestSha256EmptyTar) + manifestURL, err := env.builder.BuildManifestURL(ref) if err != nil { t.Fatalf("unexpected error getting manifest url: %v", err) } @@ -755,7 +761,8 @@ func testManifestAPISchema1(t *testing.T, env *testEnv, imageName reference.Name tag := "thetag" args := manifestArgs{imageName: imageName} - manifestURL, err := env.builder.BuildManifestURL(imageName, tag) + tagRef, _ := reference.WithTag(imageName, tag) + manifestURL, err := env.builder.BuildManifestURL(tagRef) if err != nil { t.Fatalf("unexpected error getting manifest url: %v", err) } @@ -879,7 +886,8 @@ func testManifestAPISchema1(t *testing.T, env *testEnv, imageName reference.Name args.manifest = signedManifest args.dgst = dgst - manifestDigestURL, err := env.builder.BuildManifestURL(imageName, dgst.String()) + digestRef, _ := reference.WithDigest(imageName, dgst) + manifestDigestURL, err := env.builder.BuildManifestURL(digestRef) checkErr(t, err, "building manifest url") resp = putManifest(t, "putting signed manifest no error", manifestURL, "", signedManifest) @@ -1075,7 +1083,8 @@ func testManifestAPISchema2(t *testing.T, env *testEnv, imageName reference.Name mediaType: schema2.MediaTypeManifest, } - manifestURL, err := env.builder.BuildManifestURL(imageName, tag) + tagRef, _ := reference.WithTag(imageName, tag) + manifestURL, err := env.builder.BuildManifestURL(tagRef) if err != nil { t.Fatalf("unexpected error getting manifest url: %v", err) } @@ -1219,7 +1228,8 @@ func testManifestAPISchema2(t *testing.T, env *testEnv, imageName reference.Name args.dgst = dgst args.manifest = deserializedManifest - manifestDigestURL, err := env.builder.BuildManifestURL(imageName, dgst.String()) + digestRef, _ := reference.WithDigest(imageName, dgst) + manifestDigestURL, err := env.builder.BuildManifestURL(digestRef) checkErr(t, err, "building manifest url") resp = putManifest(t, "putting manifest no error", manifestURL, schema2.MediaTypeManifest, manifest) @@ -1415,7 +1425,8 @@ func testManifestAPIManifestList(t *testing.T, env *testEnv, args manifestArgs) imageName := args.imageName tag := "manifestlisttag" - manifestURL, err := env.builder.BuildManifestURL(imageName, tag) + tagRef, _ := reference.WithTag(imageName, tag) + manifestURL, err := env.builder.BuildManifestURL(tagRef) if err != nil { t.Fatalf("unexpected error getting manifest url: %v", err) } @@ -1468,7 +1479,8 @@ func testManifestAPIManifestList(t *testing.T, env *testEnv, args manifestArgs) } dgst := digest.FromBytes(canonical) - manifestDigestURL, err := env.builder.BuildManifestURL(imageName, dgst.String()) + digestRef, _ := reference.WithDigest(imageName, dgst) + manifestDigestURL, err := env.builder.BuildManifestURL(digestRef) checkErr(t, err, "building manifest url") resp = putManifest(t, "putting manifest list no error", manifestURL, manifestlist.MediaTypeManifestList, deserializedManifestList) @@ -1637,8 +1649,9 @@ func testManifestDelete(t *testing.T, env *testEnv, args manifestArgs) { imageName := args.imageName dgst := args.dgst manifest := args.manifest - manifestDigestURL, err := env.builder.BuildManifestURL(imageName, dgst.String()) + ref, _ := reference.WithDigest(imageName, dgst) + manifestDigestURL, err := env.builder.BuildManifestURL(ref) // --------------- // Delete by digest resp, err := httpDelete(manifestDigestURL) @@ -1686,8 +1699,9 @@ func testManifestDelete(t *testing.T, env *testEnv, args manifestArgs) { // --------------- // Attempt to delete an unknown manifest - unknownDigest := "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" - unknownManifestDigestURL, err := env.builder.BuildManifestURL(imageName, unknownDigest) + unknownDigest := digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + unknownRef, _ := reference.WithDigest(imageName, unknownDigest) + unknownManifestDigestURL, err := env.builder.BuildManifestURL(unknownRef) checkErr(t, err, "building unknown manifest url") resp, err = httpDelete(unknownManifestDigestURL) @@ -1695,11 +1709,12 @@ func testManifestDelete(t *testing.T, env *testEnv, args manifestArgs) { checkResponse(t, "fetching deleted manifest", resp, http.StatusNotFound) // -------------------- - // Uupload manifest by tag + // Upload manifest by tag tag := "atag" - manifestTagURL, err := env.builder.BuildManifestURL(imageName, tag) - resp = putManifest(t, "putting signed manifest by tag", manifestTagURL, args.mediaType, manifest) - checkResponse(t, "putting signed manifest by tag", resp, http.StatusCreated) + tagRef, _ := reference.WithTag(imageName, tag) + manifestTagURL, err := env.builder.BuildManifestURL(tagRef) + resp = putManifest(t, "putting manifest by tag", manifestTagURL, args.mediaType, manifest) + checkResponse(t, "putting manifest by tag", resp, http.StatusCreated) checkHeaders(t, resp, http.Header{ "Location": []string{manifestDigestURL}, "Docker-Content-Digest": []string{dgst.String()}, @@ -1943,7 +1958,8 @@ func pushLayer(t *testing.T, ub *v2.URLBuilder, name reference.Named, dgst diges sha256Dgst := digester.Digest() - expectedLayerURL, err := ub.BuildBlobURL(name, sha256Dgst) + ref, _ := reference.WithDigest(name, sha256Dgst) + expectedLayerURL, err := ub.BuildBlobURL(ref) if err != nil { t.Fatalf("error building expected layer url: %v", err) } @@ -1966,7 +1982,8 @@ func finishUpload(t *testing.T, ub *v2.URLBuilder, name reference.Named, uploadU checkResponse(t, "putting monolithic chunk", resp, http.StatusCreated) - expectedLayerURL, err := ub.BuildBlobURL(name, dgst) + ref, _ := reference.WithDigest(name, dgst) + expectedLayerURL, err := ub.BuildBlobURL(ref) if err != nil { t.Fatalf("error building expected layer url: %v", err) } @@ -2189,10 +2206,12 @@ func createRepository(env *testEnv, t *testing.T, imageName string, tag string) dgst := digest.FromBytes(signedManifest.Canonical) // Create this repository by tag to ensure the tag mapping is made in the registry - manifestDigestURL, err := env.builder.BuildManifestURL(imageNameRef, tag) + tagRef, _ := reference.WithTag(imageNameRef, tag) + manifestDigestURL, err := env.builder.BuildManifestURL(tagRef) checkErr(t, err, "building manifest url") - location, err := env.builder.BuildManifestURL(imageNameRef, dgst.String()) + digestRef, _ := reference.WithDigest(imageNameRef, dgst) + location, err := env.builder.BuildManifestURL(digestRef) checkErr(t, err, "building location URL") resp := putManifest(t, "putting signed manifest", manifestDigestURL, "", signedManifest) @@ -2212,7 +2231,8 @@ func TestRegistryAsCacheMutationAPIs(t *testing.T) { imageName, _ := reference.ParseNamed("foo/bar") tag := "latest" - manifestURL, err := env.builder.BuildManifestURL(imageName, tag) + tagRef, _ := reference.WithTag(imageName, tag) + manifestURL, err := env.builder.BuildManifestURL(tagRef) if err != nil { t.Fatalf("unexpected error building base url: %v", err) } @@ -2255,7 +2275,8 @@ func TestRegistryAsCacheMutationAPIs(t *testing.T) { checkResponse(t, fmt.Sprintf("starting layer push to cache %v", imageName), resp, errcode.ErrorCodeUnsupported.Descriptor().HTTPStatusCode) // Blob Delete - blobURL, err := env.builder.BuildBlobURL(imageName, digest.DigestSha256EmptyTar) + ref, _ := reference.WithDigest(imageName, digest.DigestSha256EmptyTar) + blobURL, err := env.builder.BuildBlobURL(ref) resp, err = httpDelete(blobURL) checkResponse(t, "deleting blob from cache", resp, errcode.ErrorCodeUnsupported.Descriptor().HTTPStatusCode) @@ -2316,14 +2337,16 @@ func TestProxyManifestGetByTag(t *testing.T) { proxyEnv := newTestEnvWithConfig(t, &proxyConfig) - manifestDigestURL, err := proxyEnv.builder.BuildManifestURL(imageName, dgst.String()) + digestRef, _ := reference.WithDigest(imageName, dgst) + manifestDigestURL, err := proxyEnv.builder.BuildManifestURL(digestRef) checkErr(t, err, "building manifest url") resp, err := http.Get(manifestDigestURL) checkErr(t, err, "fetching manifest from proxy by digest") defer resp.Body.Close() - manifestTagURL, err := proxyEnv.builder.BuildManifestURL(imageName, tag) + tagRef, _ := reference.WithTag(imageName, tag) + manifestTagURL, err := proxyEnv.builder.BuildManifestURL(tagRef) checkErr(t, err, "building manifest url") resp, err = http.Get(manifestTagURL) diff --git a/docs/handlers/blobupload.go b/docs/handlers/blobupload.go index 56403dd9f..a42e57f63 100644 --- a/docs/handlers/blobupload.go +++ b/docs/handlers/blobupload.go @@ -372,7 +372,11 @@ func (buh *blobUploadHandler) createBlobMountOption(fromRepo, mountDigest string // created blob. A 201 Created is written as well as the canonical URL and // blob digest. func (buh *blobUploadHandler) writeBlobCreatedHeaders(w http.ResponseWriter, desc distribution.Descriptor) error { - blobURL, err := buh.urlBuilder.BuildBlobURL(buh.Repository.Name(), desc.Digest) + ref, err := reference.WithDigest(buh.Repository.Name(), desc.Digest) + if err != nil { + return err + } + blobURL, err := buh.urlBuilder.BuildBlobURL(ref) if err != nil { return err } diff --git a/docs/handlers/images.go b/docs/handlers/images.go index 9b4e53998..808ead54a 100644 --- a/docs/handlers/images.go +++ b/docs/handlers/images.go @@ -289,7 +289,13 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http } // Construct a canonical url for the uploaded manifest. - location, err := imh.urlBuilder.BuildManifestURL(imh.Repository.Name(), imh.Digest.String()) + ref, err := reference.WithDigest(imh.Repository.Name(), imh.Digest) + if err != nil { + imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) + return + } + + location, err := imh.urlBuilder.BuildManifestURL(ref) if err != nil { // NOTE(stevvooe): Given the behavior above, this absurdly unlikely to // happen. We'll log the error here but proceed as if it worked. Worst