From c78d173cf71dce2ed38199c8ead7bcdc573d7512 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 11 Dec 2014 21:03:45 -0800 Subject: [PATCH 01/12] Move routes to urls package To lock down V2 api routing, we are moving it to a separate package, with an exported router creation function and route names. Updates will follow to prepare the package for export. --- routes.go => api/urls/routes.go | 0 routes_test.go => api/urls/routes_test.go | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename routes.go => api/urls/routes.go (100%) rename routes_test.go => api/urls/routes_test.go (100%) diff --git a/routes.go b/api/urls/routes.go similarity index 100% rename from routes.go rename to api/urls/routes.go diff --git a/routes_test.go b/api/urls/routes_test.go similarity index 100% rename from routes_test.go rename to api/urls/routes_test.go From da19114d1a54d65b04e422353155640162d9b728 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 11 Dec 2014 21:08:23 -0800 Subject: [PATCH 02/12] Prepare urls package for exports The route definition files have been prepared for export with documentation. Consts have been updated and tests are now passing for the urls package. --- api/urls/routes.go | 45 ++++++++++++++++++++++------------------- api/urls/routes_test.go | 28 ++++++++++++------------- 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/api/urls/routes.go b/api/urls/routes.go index b291ee4b..79138a4a 100644 --- a/api/urls/routes.go +++ b/api/urls/routes.go @@ -1,66 +1,69 @@ -package registry +package urls import ( "github.com/docker/docker-registry/common" "github.com/gorilla/mux" ) +// The following are definitions of the name under which all V2 routes are +// registered. These symbols can be used to look up a route based on the name. const ( - routeNameBase = "base" - routeNameImageManifest = "image-manifest" - routeNameTags = "tags" - routeNameBlob = "blob" - routeNameBlobUpload = "blob-upload" - routeNameBlobUploadResume = "blob-upload-resume" + RouteNameBase = "base" + RouteNameManifest = "manifest" + RouteNameTags = "tags" + RouteNameBlob = "blob" + RouteNameBlobUpload = "blob-upload" + RouteNameBlobUploadChunk = "blob-upload-chunk" ) var allEndpoints = []string{ - routeNameImageManifest, - routeNameTags, - routeNameBlob, - routeNameBlobUpload, - routeNameBlobUploadResume, + RouteNameManifest, + RouteNameTags, + RouteNameBlob, + RouteNameBlobUpload, + RouteNameBlobUploadChunk, } -// v2APIRouter builds a gorilla router with named routes for the various API -// methods. We may export this for use by the client. -func v2APIRouter() *mux.Router { +// Router builds a gorilla router with named routes for the various API +// methods. This can be used directly by both server implementations and +// clients. +func Router() *mux.Router { router := mux.NewRouter(). StrictSlash(true) // GET /v2/ Check Check that the registry implements API version 2(.1) router. Path("/v2/"). - Name(routeNameBase) + Name(RouteNameBase) // GET /v2//manifest/ Image Manifest Fetch the image manifest identified by name and tag. // PUT /v2//manifest/ Image Manifest Upload the image manifest identified by name and tag. // DELETE /v2//manifest/ Image Manifest Delete the image identified by name and tag. router. Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/manifests/{tag:" + common.TagNameRegexp.String() + "}"). - Name(routeNameImageManifest) + Name(RouteNameManifest) // GET /v2//tags/list Tags Fetch the tags under the repository identified by name. router. Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/tags/list"). - Name(routeNameTags) + Name(RouteNameTags) // GET /v2//blob/ Layer Fetch the blob identified by digest. router. Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/blobs/{digest:[a-zA-Z0-9-_+.]+:[a-zA-Z0-9-_+.=]+}"). - Name(routeNameBlob) + Name(RouteNameBlob) // POST /v2//blob/upload/ Layer Upload Initiate an upload of the layer identified by tarsum. router. Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/blobs/uploads/"). - Name(routeNameBlobUpload) + Name(RouteNameBlobUpload) // GET /v2//blob/upload/ Layer Upload Get the status of the upload identified by tarsum and uuid. // PUT /v2//blob/upload/ Layer Upload Upload all or a chunk of the upload identified by tarsum and uuid. // DELETE /v2//blob/upload/ Layer Upload Cancel the upload identified by layer and uuid router. Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/blobs/uploads/{uuid}"). - Name(routeNameBlobUploadResume) + Name(RouteNameBlobUploadChunk) return router } diff --git a/api/urls/routes_test.go b/api/urls/routes_test.go index 6d684a61..f2e95270 100644 --- a/api/urls/routes_test.go +++ b/api/urls/routes_test.go @@ -1,4 +1,4 @@ -package registry +package urls import ( "encoding/json" @@ -25,7 +25,7 @@ type routeTestCase struct { // This may go away as the application structure comes together. func TestRouter(t *testing.T) { - router := v2APIRouter() + router := Router() testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { testCase := routeTestCase{ @@ -47,12 +47,12 @@ func TestRouter(t *testing.T) { for _, testcase := range []routeTestCase{ { - RouteName: routeNameBase, + RouteName: RouteNameBase, RequestURI: "/v2/", Vars: map[string]string{}, }, { - RouteName: routeNameImageManifest, + RouteName: RouteNameManifest, RequestURI: "/v2/foo/bar/manifests/tag", Vars: map[string]string{ "name": "foo/bar", @@ -60,14 +60,14 @@ func TestRouter(t *testing.T) { }, }, { - RouteName: routeNameTags, + RouteName: RouteNameTags, RequestURI: "/v2/foo/bar/tags/list", Vars: map[string]string{ "name": "foo/bar", }, }, { - RouteName: routeNameBlob, + RouteName: RouteNameBlob, RequestURI: "/v2/foo/bar/blobs/tarsum.dev+foo:abcdef0919234", Vars: map[string]string{ "name": "foo/bar", @@ -75,7 +75,7 @@ func TestRouter(t *testing.T) { }, }, { - RouteName: routeNameBlob, + RouteName: RouteNameBlob, RequestURI: "/v2/foo/bar/blobs/sha256:abcdef0919234", Vars: map[string]string{ "name": "foo/bar", @@ -83,14 +83,14 @@ func TestRouter(t *testing.T) { }, }, { - RouteName: routeNameBlobUpload, + RouteName: RouteNameBlobUpload, RequestURI: "/v2/foo/bar/blobs/uploads/", Vars: map[string]string{ "name": "foo/bar", }, }, { - RouteName: routeNameBlobUploadResume, + RouteName: RouteNameBlobUploadChunk, RequestURI: "/v2/foo/bar/blobs/uploads/uuid", Vars: map[string]string{ "name": "foo/bar", @@ -98,7 +98,7 @@ func TestRouter(t *testing.T) { }, }, { - RouteName: routeNameBlobUploadResume, + RouteName: RouteNameBlobUploadChunk, RequestURI: "/v2/foo/bar/blobs/uploads/D95306FA-FAD3-4E36-8D41-CF1C93EF8286", Vars: map[string]string{ "name": "foo/bar", @@ -106,7 +106,7 @@ func TestRouter(t *testing.T) { }, }, { - RouteName: routeNameBlobUploadResume, + RouteName: RouteNameBlobUploadChunk, RequestURI: "/v2/foo/bar/blobs/uploads/RDk1MzA2RkEtRkFEMy00RTM2LThENDEtQ0YxQzkzRUY4Mjg2IA==", Vars: map[string]string{ "name": "foo/bar", @@ -117,7 +117,7 @@ func TestRouter(t *testing.T) { // Check ambiguity: ensure we can distinguish between tags for // "foo/bar/image/image" and image for "foo/bar/image" with tag // "tags" - RouteName: routeNameImageManifest, + RouteName: RouteNameManifest, RequestURI: "/v2/foo/bar/manifests/manifests/tags", Vars: map[string]string{ "name": "foo/bar/manifests", @@ -127,14 +127,14 @@ func TestRouter(t *testing.T) { { // This case presents an ambiguity between foo/bar with tag="tags" // and list tags for "foo/bar/manifest" - RouteName: routeNameTags, + RouteName: RouteNameTags, RequestURI: "/v2/foo/bar/manifests/tags/list", Vars: map[string]string{ "name": "foo/bar/manifests", }, }, { - RouteName: routeNameBlobUploadResume, + RouteName: RouteNameBlobUploadChunk, RequestURI: "/v2/foo/../../blob/uploads/D95306FA-FAD3-4E36-8D41-CF1C93EF8286", StatusCode: http.StatusNotFound, }, From 5b13e955119926630d1fe57df3ef9f7444df5e7a Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 11 Dec 2014 21:10:43 -0800 Subject: [PATCH 03/12] Porting registry to use urls package This simply moves the registry app to be using the urls package and its exported route names. This supports locking down exported route definitions for use in client packages. --- app.go | 15 ++++++++------- app_test.go | 15 ++++++++------- urls.go | 15 ++++++++------- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/app.go b/app.go index 76605f1b..b34a77de 100644 --- a/app.go +++ b/app.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" + "github.com/docker/docker-registry/api/urls" "github.com/docker/docker-registry/storagedriver" "github.com/docker/docker-registry/storagedriver/factory" @@ -35,18 +36,18 @@ type App struct { func NewApp(configuration configuration.Configuration) *App { app := &App{ Config: configuration, - router: v2APIRouter(), + router: urls.Router(), } // Register the handler dispatchers. - app.register(routeNameBase, func(ctx *Context, r *http.Request) http.Handler { + app.register(urls.RouteNameBase, func(ctx *Context, r *http.Request) http.Handler { return http.HandlerFunc(apiBase) }) - app.register(routeNameImageManifest, imageManifestDispatcher) - app.register(routeNameTags, tagsDispatcher) - app.register(routeNameBlob, layerDispatcher) - app.register(routeNameBlobUpload, layerUploadDispatcher) - app.register(routeNameBlobUploadResume, layerUploadDispatcher) + app.register(urls.RouteNameManifest, imageManifestDispatcher) + app.register(urls.RouteNameTags, tagsDispatcher) + app.register(urls.RouteNameBlob, layerDispatcher) + app.register(urls.RouteNameBlobUpload, layerUploadDispatcher) + app.register(urls.RouteNameBlobUploadChunk, layerUploadDispatcher) driver, err := factory.Create(configuration.Storage.Type(), configuration.Storage.Parameters()) diff --git a/app_test.go b/app_test.go index bb78044a..eb8ec597 100644 --- a/app_test.go +++ b/app_test.go @@ -6,6 +6,7 @@ import ( "net/url" "testing" + "github.com/docker/docker-registry/api/urls" "github.com/docker/docker-registry/configuration" ) @@ -16,10 +17,10 @@ import ( func TestAppDispatcher(t *testing.T) { app := &App{ Config: configuration.Configuration{}, - router: v2APIRouter(), + router: urls.Router(), } server := httptest.NewServer(app) - router := v2APIRouter() + router := urls.Router() serverURL, err := url.Parse(server.URL) if err != nil { @@ -71,33 +72,33 @@ func TestAppDispatcher(t *testing.T) { vars []string }{ { - endpoint: routeNameImageManifest, + endpoint: urls.RouteNameManifest, vars: []string{ "name", "foo/bar", "tag", "sometag", }, }, { - endpoint: routeNameTags, + endpoint: urls.RouteNameTags, vars: []string{ "name", "foo/bar", }, }, { - endpoint: routeNameBlob, + endpoint: urls.RouteNameBlob, vars: []string{ "name", "foo/bar", "digest", "tarsum.v1+bogus:abcdef0123456789", }, }, { - endpoint: routeNameBlobUpload, + endpoint: urls.RouteNameBlobUpload, vars: []string{ "name", "foo/bar", }, }, { - endpoint: routeNameBlobUploadResume, + endpoint: urls.RouteNameBlobUploadChunk, vars: []string{ "name", "foo/bar", "uuid", "theuuid", diff --git a/urls.go b/urls.go index 92233da4..4f294c95 100644 --- a/urls.go +++ b/urls.go @@ -4,6 +4,7 @@ import ( "net/http" "net/url" + "github.com/docker/docker-registry/api/urls" "github.com/docker/docker-registry/digest" "github.com/docker/docker-registry/storage" "github.com/gorilla/mux" @@ -17,7 +18,7 @@ type urlBuilder struct { func newURLBuilder(root *url.URL) *urlBuilder { return &urlBuilder{ url: root, - router: v2APIRouter(), + router: urls.Router(), } } @@ -40,7 +41,7 @@ func newURLBuilderFromString(root string) (*urlBuilder, error) { } func (ub *urlBuilder) buildBaseURL() (string, error) { - route := clonedRoute(ub.router, routeNameBase) + route := clonedRoute(ub.router, urls.RouteNameBase) baseURL, err := route. Schemes(ub.url.Scheme). @@ -54,7 +55,7 @@ func (ub *urlBuilder) buildBaseURL() (string, error) { } func (ub *urlBuilder) buildTagsURL(name string) (string, error) { - route := clonedRoute(ub.router, routeNameTags) + route := clonedRoute(ub.router, urls.RouteNameTags) tagsURL, err := route. Schemes(ub.url.Scheme). @@ -72,7 +73,7 @@ func (ub *urlBuilder) forManifest(m *storage.Manifest) (string, error) { } func (ub *urlBuilder) buildManifestURL(name, tag string) (string, error) { - route := clonedRoute(ub.router, routeNameImageManifest) + route := clonedRoute(ub.router, urls.RouteNameManifest) manifestURL, err := route. Schemes(ub.url.Scheme). @@ -90,7 +91,7 @@ func (ub *urlBuilder) forLayer(l storage.Layer) (string, error) { } func (ub *urlBuilder) buildLayerURL(name string, dgst digest.Digest) (string, error) { - route := clonedRoute(ub.router, routeNameBlob) + route := clonedRoute(ub.router, urls.RouteNameBlob) layerURL, err := route. Schemes(ub.url.Scheme). @@ -104,7 +105,7 @@ func (ub *urlBuilder) buildLayerURL(name string, dgst digest.Digest) (string, er } func (ub *urlBuilder) buildLayerUploadURL(name string) (string, error) { - route := clonedRoute(ub.router, routeNameBlobUpload) + route := clonedRoute(ub.router, urls.RouteNameBlobUpload) uploadURL, err := route. Schemes(ub.url.Scheme). @@ -122,7 +123,7 @@ func (ub *urlBuilder) forLayerUpload(layerUpload storage.LayerUpload) (string, e } func (ub *urlBuilder) buildLayerUploadResumeURL(name, uuid string, values ...url.Values) (string, error) { - route := clonedRoute(ub.router, routeNameBlobUploadResume) + route := clonedRoute(ub.router, urls.RouteNameBlobUploadChunk) uploadURL, err := route. Schemes(ub.url.Scheme). From e5b6da80d092a1d20ad0ec4345a638522353a209 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 11 Dec 2014 21:15:02 -0800 Subject: [PATCH 04/12] Move urls.go into urls package --- urls.go => api/urls/urls.go | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename urls.go => api/urls/urls.go (100%) diff --git a/urls.go b/api/urls/urls.go similarity index 100% rename from urls.go rename to api/urls/urls.go From 9b872ca150e89eac269508e47d844b946b1a9e9d Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 11 Dec 2014 21:57:14 -0800 Subject: [PATCH 05/12] Prepare urls.URLBuilder package for export The URLBuilder is now exported with documentation for its behavior. Its a light wrapper around gorilla mux that avoids one having to remember exact arguments take by each route. --- api/urls/urls.go | 145 +++++++++++++++++++++++------------------------ 1 file changed, 70 insertions(+), 75 deletions(-) diff --git a/api/urls/urls.go b/api/urls/urls.go index 4f294c95..7bfd6335 100644 --- a/api/urls/urls.go +++ b/api/urls/urls.go @@ -1,52 +1,61 @@ -package registry +package urls import ( "net/http" "net/url" - "github.com/docker/docker-registry/api/urls" "github.com/docker/docker-registry/digest" - "github.com/docker/docker-registry/storage" "github.com/gorilla/mux" ) -type urlBuilder struct { - url *url.URL // url root (ie http://localhost/) +// URLBuilder creates registry API urls from a single base endpoint. It can be +// used to create urls for use in a registry client or server. +// +// All urls will be created from the given base, including the api version. +// For example, if a root of "/foo/" is provided, urls generated will be fall +// under "/foo/v2/...". Most application will only provide a schema, host and +// port, such as "https://localhost:5000/". +type URLBuilder struct { + root *url.URL // url root (ie http://localhost/) router *mux.Router } -func newURLBuilder(root *url.URL) *urlBuilder { - return &urlBuilder{ - url: root, - router: urls.Router(), +// NewURLBuilder creates a URLBuilder with provided root url object. +func NewURLBuilder(root *url.URL) *URLBuilder { + return &URLBuilder{ + root: root, + router: Router(), } } -func newURLBuilderFromRequest(r *http.Request) *urlBuilder { - u := &url.URL{ - Scheme: r.URL.Scheme, - Host: r.Host, - } - - return newURLBuilder(u) -} - -func newURLBuilderFromString(root string) (*urlBuilder, error) { +// NewURLBuilderFromString workes identically to NewURLBuilder except it takes +// a string argument for the root, returning an error if it is not a valid +// url. +func NewURLBuilderFromString(root string) (*URLBuilder, error) { u, err := url.Parse(root) if err != nil { return nil, err } - return newURLBuilder(u), nil + return NewURLBuilder(u), nil } -func (ub *urlBuilder) buildBaseURL() (string, error) { - route := clonedRoute(ub.router, urls.RouteNameBase) +// NewURLBuilderFromRequest uses information from an *http.Request to +// construct the root url. +func NewURLBuilderFromRequest(r *http.Request) *URLBuilder { + u := &url.URL{ + Scheme: r.URL.Scheme, + Host: r.Host, + } - baseURL, err := route. - Schemes(ub.url.Scheme). - Host(ub.url.Host). - URL() + return NewURLBuilder(u) +} + +// BuildBaseURL constructs a base url for the API, typically just "/v2/". +func (ub *URLBuilder) BuildBaseURL() (string, error) { + route := ub.cloneRoute(RouteNameBase) + + baseURL, err := route.URL() if err != nil { return "", err } @@ -54,13 +63,11 @@ func (ub *urlBuilder) buildBaseURL() (string, error) { return baseURL.String(), nil } -func (ub *urlBuilder) buildTagsURL(name string) (string, error) { - route := clonedRoute(ub.router, urls.RouteNameTags) +// BuildTagsURL constructs a url to list the tags in the named repository. +func (ub *URLBuilder) BuildTagsURL(name string) (string, error) { + route := ub.cloneRoute(RouteNameTags) - tagsURL, err := route. - Schemes(ub.url.Scheme). - Host(ub.url.Host). - URL("name", name) + tagsURL, err := route.URL("name", name) if err != nil { return "", err } @@ -68,17 +75,11 @@ func (ub *urlBuilder) buildTagsURL(name string) (string, error) { return tagsURL.String(), nil } -func (ub *urlBuilder) forManifest(m *storage.Manifest) (string, error) { - return ub.buildManifestURL(m.Name, m.Tag) -} +// BuildManifestURL constructs a url for the manifest identified by name and tag. +func (ub *URLBuilder) BuildManifestURL(name, tag string) (string, error) { + route := ub.cloneRoute(RouteNameManifest) -func (ub *urlBuilder) buildManifestURL(name, tag string) (string, error) { - route := clonedRoute(ub.router, urls.RouteNameManifest) - - manifestURL, err := route. - Schemes(ub.url.Scheme). - Host(ub.url.Host). - URL("name", name, "tag", tag) + manifestURL, err := route.URL("name", name, "tag", tag) if err != nil { return "", err } @@ -86,17 +87,11 @@ func (ub *urlBuilder) buildManifestURL(name, tag string) (string, error) { return manifestURL.String(), nil } -func (ub *urlBuilder) forLayer(l storage.Layer) (string, error) { - return ub.buildLayerURL(l.Name(), l.Digest()) -} +// BuildLayerURL constructs the url for the blob identified by name and dgst. +func (ub *URLBuilder) BuildBlobURL(name string, dgst digest.Digest) (string, error) { + route := ub.cloneRoute(RouteNameBlob) -func (ub *urlBuilder) buildLayerURL(name string, dgst digest.Digest) (string, error) { - route := clonedRoute(ub.router, urls.RouteNameBlob) - - layerURL, err := route. - Schemes(ub.url.Scheme). - Host(ub.url.Host). - URL("name", name, "digest", dgst.String()) + layerURL, err := route.URL("name", name, "digest", dgst.String()) if err != nil { return "", err } @@ -104,13 +99,12 @@ func (ub *urlBuilder) buildLayerURL(name string, dgst digest.Digest) (string, er return layerURL.String(), nil } -func (ub *urlBuilder) buildLayerUploadURL(name string) (string, error) { - route := clonedRoute(ub.router, urls.RouteNameBlobUpload) +// BuildBlobURL constructs a url to begin a blob upload in the repository +// identified by name. +func (ub *URLBuilder) BuildBlobUploadURL(name string) (string, error) { + route := ub.cloneRoute(RouteNameBlobUpload) - uploadURL, err := route. - Schemes(ub.url.Scheme). - Host(ub.url.Host). - URL("name", name) + uploadURL, err := route.URL("name", name) if err != nil { return "", err } @@ -118,17 +112,14 @@ func (ub *urlBuilder) buildLayerUploadURL(name string) (string, error) { return uploadURL.String(), nil } -func (ub *urlBuilder) forLayerUpload(layerUpload storage.LayerUpload) (string, error) { - return ub.buildLayerUploadResumeURL(layerUpload.Name(), layerUpload.UUID()) -} +// BuildBlobUploadChunkURL constructs a url for the upload identified by uuid, +// 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) { + route := ub.cloneRoute(RouteNameBlobUploadChunk) -func (ub *urlBuilder) buildLayerUploadResumeURL(name, uuid string, values ...url.Values) (string, error) { - route := clonedRoute(ub.router, urls.RouteNameBlobUploadChunk) - - uploadURL, err := route. - Schemes(ub.url.Scheme). - Host(ub.url.Host). - URL("name", name, "uuid", uuid) + uploadURL, err := route.URL("name", name, "uuid", uuid) if err != nil { return "", err } @@ -136,6 +127,17 @@ func (ub *urlBuilder) buildLayerUploadResumeURL(name, uuid string, values ...url return appendValuesURL(uploadURL, values...).String(), nil } +// clondedRoute returns a clone of the named route from the router. Routes +// must be cloned to avoid modifying them during url generation. +func (ub *URLBuilder) cloneRoute(name string) *mux.Route { + route := new(mux.Route) + *route = *ub.router.GetRoute(name) // clone the route + + return route. + Schemes(ub.root.Scheme). + Host(ub.root.Host) +} + // appendValuesURL appends the parameters to the url. func appendValuesURL(u *url.URL, values ...url.Values) *url.URL { merged := u.Query() @@ -161,10 +163,3 @@ func appendValues(u string, values ...url.Values) string { return appendValuesURL(up, values...).String() } - -// clondedRoute returns a clone of the named route from the router. -func clonedRoute(router *mux.Router, name string) *mux.Route { - route := new(mux.Route) - *route = *router.GetRoute(name) // clone the route - return route -} From 83f882b427e4e29f1dabededf1c309493788c84f Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 11 Dec 2014 21:59:59 -0800 Subject: [PATCH 06/12] Migrate webapp to use exported urls.URLBuilder package --- api_test.go | 36 ++++++++++++++++++++++-------------- app.go | 2 +- context.go | 3 ++- layerupload.go | 4 ++-- 4 files changed, 27 insertions(+), 18 deletions(-) diff --git a/api_test.go b/api_test.go index d6cf34dd..e71edc17 100644 --- a/api_test.go +++ b/api_test.go @@ -14,6 +14,7 @@ import ( "testing" "github.com/docker/docker-registry/api/errors" + "github.com/docker/docker-registry/api/urls" "github.com/docker/docker-registry/common/testutil" "github.com/docker/docker-registry/configuration" "github.com/docker/docker-registry/digest" @@ -34,13 +35,13 @@ func TestCheckAPI(t *testing.T) { app := NewApp(config) server := httptest.NewServer(handlers.CombinedLoggingHandler(os.Stderr, app)) - builder, err := newURLBuilderFromString(server.URL) + builder, err := urls.NewURLBuilderFromString(server.URL) if err != nil { t.Fatalf("error creating url builder: %v", err) } - baseURL, err := builder.buildBaseURL() + baseURL, err := builder.BuildBaseURL() if err != nil { t.Fatalf("unexpected error building base url: %v", err) } @@ -81,7 +82,7 @@ func TestLayerAPI(t *testing.T) { app := NewApp(config) server := httptest.NewServer(handlers.CombinedLoggingHandler(os.Stderr, app)) - builder, err := newURLBuilderFromString(server.URL) + builder, err := urls.NewURLBuilderFromString(server.URL) if err != nil { t.Fatalf("error creating url builder: %v", err) @@ -98,7 +99,7 @@ func TestLayerAPI(t *testing.T) { // ----------------------------------- // Test fetch for non-existent content - layerURL, err := builder.buildLayerURL(imageName, layerDigest) + layerURL, err := builder.BuildBlobURL(imageName, layerDigest) if err != nil { t.Fatalf("error building url: %v", err) } @@ -121,7 +122,7 @@ func TestLayerAPI(t *testing.T) { // ------------------------------------------ // Upload a layer - layerUploadURL, err := builder.buildLayerUploadURL(imageName) + layerUploadURL, err := builder.BuildBlobUploadURL(imageName) if err != nil { t.Fatalf("error building upload url: %v", err) } @@ -196,7 +197,7 @@ func TestManifestAPI(t *testing.T) { app := NewApp(config) server := httptest.NewServer(handlers.CombinedLoggingHandler(os.Stderr, app)) - builder, err := newURLBuilderFromString(server.URL) + builder, err := urls.NewURLBuilderFromString(server.URL) if err != nil { t.Fatalf("unexpected error creating url builder: %v", err) } @@ -204,7 +205,7 @@ func TestManifestAPI(t *testing.T) { imageName := "foo/bar" tag := "thetag" - manifestURL, err := builder.buildManifestURL(imageName, tag) + manifestURL, err := builder.BuildManifestURL(imageName, tag) if err != nil { t.Fatalf("unexpected error getting manifest url: %v", err) } @@ -240,7 +241,7 @@ func TestManifestAPI(t *testing.T) { t.Fatalf("expected manifest unknown error: got %v", respErrs) } - tagsURL, err := builder.buildTagsURL(imageName) + tagsURL, err := builder.BuildTagsURL(imageName) if err != nil { t.Fatalf("unexpected error building tags url: %v", err) } @@ -427,8 +428,8 @@ func putManifest(t *testing.T, msg, url string, v interface{}) *http.Response { return resp } -func startPushLayer(t *testing.T, ub *urlBuilder, name string) string { - layerUploadURL, err := ub.buildLayerUploadURL(name) +func startPushLayer(t *testing.T, ub *urls.URLBuilder, name string) string { + layerUploadURL, err := ub.BuildBlobUploadURL(name) if err != nil { t.Fatalf("unexpected error building layer upload url: %v", err) } @@ -449,14 +450,21 @@ func startPushLayer(t *testing.T, ub *urlBuilder, name string) string { } // pushLayer pushes the layer content returning the url on success. -func pushLayer(t *testing.T, ub *urlBuilder, name string, dgst digest.Digest, uploadURLBase string, rs io.ReadSeeker) string { +func pushLayer(t *testing.T, ub *urls.URLBuilder, name string, dgst digest.Digest, uploadURLBase string, rs io.ReadSeeker) string { rsLength, _ := rs.Seek(0, os.SEEK_END) rs.Seek(0, os.SEEK_SET) - uploadURL := appendValues(uploadURLBase, url.Values{ + u, err := url.Parse(uploadURLBase) + if err != nil { + t.Fatalf("unexpected error parsing pushLayer url: %v", err) + } + + u.RawQuery = url.Values{ "digest": []string{dgst.String()}, "size": []string{fmt.Sprint(rsLength)}, - }) + }.Encode() + + uploadURL := u.String() // Just do a monolithic upload req, err := http.NewRequest("PUT", uploadURL, rs) @@ -472,7 +480,7 @@ func pushLayer(t *testing.T, ub *urlBuilder, name string, dgst digest.Digest, up checkResponse(t, "putting monolithic chunk", resp, http.StatusCreated) - expectedLayerURL, err := ub.buildLayerURL(name, dgst) + expectedLayerURL, err := ub.BuildBlobURL(name, dgst) if err != nil { t.Fatalf("error building expected layer url: %v", err) } diff --git a/app.go b/app.go index b34a77de..e1b299a7 100644 --- a/app.go +++ b/app.go @@ -115,7 +115,7 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { context := &Context{ App: app, Name: vars["name"], - urlBuilder: newURLBuilderFromRequest(r), + urlBuilder: urls.NewURLBuilderFromRequest(r), } // Store vars for underlying handlers. diff --git a/context.go b/context.go index a1e47abe..23d669b8 100644 --- a/context.go +++ b/context.go @@ -3,6 +3,7 @@ package registry import ( "github.com/Sirupsen/logrus" "github.com/docker/docker-registry/api/errors" + "github.com/docker/docker-registry/api/urls" ) // Context should contain the request specific context for use in across @@ -28,5 +29,5 @@ type Context struct { // log provides a context specific logger. log *logrus.Entry - urlBuilder *urlBuilder + urlBuilder *urls.URLBuilder } diff --git a/layerupload.go b/layerupload.go index af8bd457..898b279b 100644 --- a/layerupload.go +++ b/layerupload.go @@ -151,7 +151,7 @@ func (luh *layerUploadHandler) CancelLayerUpload(w http.ResponseWriter, r *http. // chunk responses. This sets the correct headers but the response status is // left to the caller. func (luh *layerUploadHandler) layerUploadResponse(w http.ResponseWriter, r *http.Request) error { - uploadURL, err := luh.urlBuilder.forLayerUpload(luh.Upload) + uploadURL, err := luh.urlBuilder.BuildBlobUploadChunkURL(luh.Upload.Name(), luh.Upload.UUID()) if err != nil { logrus.Infof("error building upload url: %s", err) return err @@ -200,7 +200,7 @@ func (luh *layerUploadHandler) completeUpload(w http.ResponseWriter, r *http.Req return } - layerURL, err := luh.urlBuilder.forLayer(layer) + layerURL, err := luh.urlBuilder.BuildBlobURL(layer.Name(), layer.Digest()) if err != nil { luh.Errors.Push(errors.ErrorCodeUnknown, err) w.WriteHeader(http.StatusInternalServerError) From e14e5d14b104cf0cd309c2ee516d3a049f4d31ab Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 11 Dec 2014 22:02:17 -0800 Subject: [PATCH 07/12] Correct documentation errors in urls package --- api/urls/urls.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/urls/urls.go b/api/urls/urls.go index 7bfd6335..86306c67 100644 --- a/api/urls/urls.go +++ b/api/urls/urls.go @@ -87,7 +87,7 @@ func (ub *URLBuilder) BuildManifestURL(name, tag string) (string, error) { return manifestURL.String(), nil } -// BuildLayerURL constructs the url for the blob identified by name and dgst. +// BuildBlobURL constructs the url for the blob identified by name and dgst. func (ub *URLBuilder) BuildBlobURL(name string, dgst digest.Digest) (string, error) { route := ub.cloneRoute(RouteNameBlob) @@ -99,8 +99,8 @@ func (ub *URLBuilder) BuildBlobURL(name string, dgst digest.Digest) (string, err return layerURL.String(), nil } -// BuildBlobURL constructs a url to begin a blob upload in the repository -// identified by name. +// BuildBlobUploadURL constructs a url to begin a blob upload in the +// repository identified by name. func (ub *URLBuilder) BuildBlobUploadURL(name string) (string, error) { route := ub.cloneRoute(RouteNameBlobUpload) From 92dca269f0ccf4fe744ccc7a298d0093321fe113 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 11 Dec 2014 22:04:26 -0800 Subject: [PATCH 08/12] Change errors export from Descriptors to ErrorDescriptors --- api/errors/descriptors.go | 12 ++++++------ api/errors/errors_test.go | 2 +- cmd/registry-api-doctable-gen/main.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/api/errors/descriptors.go b/api/errors/descriptors.go index 1d71162f..d2f0f7da 100644 --- a/api/errors/descriptors.go +++ b/api/errors/descriptors.go @@ -26,9 +26,9 @@ type ErrorDescriptor struct { HTTPStatusCodes []int } -// Descriptors provides a list of HTTP API Error codes that may be encountered -// when interacting with the registry API. -var Descriptors = []ErrorDescriptor{ +// ErrorDescriptors provides a list of HTTP API Error codes that may be +// encountered when interacting with the registry API. +var ErrorDescriptors = []ErrorDescriptor{ { Code: ErrorCodeUnknown, Value: "UNKNOWN", @@ -131,10 +131,10 @@ var errorCodeToDescriptors map[ErrorCode]ErrorDescriptor var idToDescriptors map[string]ErrorDescriptor func init() { - errorCodeToDescriptors = make(map[ErrorCode]ErrorDescriptor, len(Descriptors)) - idToDescriptors = make(map[string]ErrorDescriptor, len(Descriptors)) + errorCodeToDescriptors = make(map[ErrorCode]ErrorDescriptor, len(ErrorDescriptors)) + idToDescriptors = make(map[string]ErrorDescriptor, len(ErrorDescriptors)) - for _, descriptor := range Descriptors { + for _, descriptor := range ErrorDescriptors { errorCodeToDescriptors[descriptor.Code] = descriptor idToDescriptors[descriptor.Value] = descriptor } diff --git a/api/errors/errors_test.go b/api/errors/errors_test.go index 7a68fe90..2f5c69e0 100644 --- a/api/errors/errors_test.go +++ b/api/errors/errors_test.go @@ -11,7 +11,7 @@ import ( // TestErrorCodes ensures that error code format, mappings and // marshaling/unmarshaling. round trips are stable. func TestErrorCodes(t *testing.T) { - for _, desc := range Descriptors { + for _, desc := range ErrorDescriptors { if desc.Code.String() != desc.Value { t.Fatalf("error code string incorrect: %q != %q", desc.Code.String(), desc.Value) } diff --git a/cmd/registry-api-doctable-gen/main.go b/cmd/registry-api-doctable-gen/main.go index f76c249e..869f1a37 100644 --- a/cmd/registry-api-doctable-gen/main.go +++ b/cmd/registry-api-doctable-gen/main.go @@ -61,7 +61,7 @@ func dumpErrors(wr io.Writer) { fmt.Fprintln(writer, "\n"+divider) - for _, descriptor := range errors.Descriptors { + for _, descriptor := range errors.ErrorDescriptors { fmt.Fprint(writer, "|") v := reflect.ValueOf(descriptor) From 5abfc91021077f522f2aa9f3b5f9b2ea3d903980 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 11 Dec 2014 22:10:18 -0800 Subject: [PATCH 09/12] Merge errors and urls package into unified v2 package To provide a single package with v2 API definitions, the locked down portions of the API have been merged into a single package. References to exported items will appear as v2.XXX, indicating their origin. The definitions in this package will soon be locked down for change, acceppting only additions that will not change protocol behavior. --- api/{errors => v2}/descriptors.go | 2 +- api/v2/doc.go | 9 +++++++++ api/{errors => v2}/errors.go | 10 +--------- api/{errors => v2}/errors_test.go | 2 +- api/{urls => v2}/routes.go | 2 +- api/{urls => v2}/routes_test.go | 2 +- api/{urls => v2}/urls.go | 2 +- 7 files changed, 15 insertions(+), 14 deletions(-) rename api/{errors => v2}/descriptors.go (99%) create mode 100644 api/v2/doc.go rename api/{errors => v2}/errors.go (91%) rename api/{errors => v2}/errors_test.go (99%) rename api/{urls => v2}/routes.go (99%) rename api/{urls => v2}/routes_test.go (99%) rename api/{urls => v2}/urls.go (99%) diff --git a/api/errors/descriptors.go b/api/v2/descriptors.go similarity index 99% rename from api/errors/descriptors.go rename to api/v2/descriptors.go index d2f0f7da..77528d72 100644 --- a/api/errors/descriptors.go +++ b/api/v2/descriptors.go @@ -1,4 +1,4 @@ -package errors +package v2 import "net/http" diff --git a/api/v2/doc.go b/api/v2/doc.go new file mode 100644 index 00000000..cde01195 --- /dev/null +++ b/api/v2/doc.go @@ -0,0 +1,9 @@ +// Package v2 describes routes, urls and the error codes used in the Docker +// Registry JSON HTTP API V2. In addition to declarations, descriptors are +// provided for routes and error codes that can be used for implementation and +// automatically generating documentation. +// +// Definitions here are considered to be locked down for the V2 registry api. +// Any changes must be considered carefully and should not proceed without a +// change proposal in docker core. +package v2 diff --git a/api/errors/errors.go b/api/v2/errors.go similarity index 91% rename from api/errors/errors.go rename to api/v2/errors.go index b6e64e2a..8c85d3a9 100644 --- a/api/errors/errors.go +++ b/api/v2/errors.go @@ -1,12 +1,4 @@ -// Package errors describes the error codes that may be returned via the -// Docker Registry JSON HTTP API V2. In addition to declaractions, -// descriptions about the error codes and the conditions causing them are -// avialable in detail. -// -// Error definitions here are considered to be locked down for the V2 registry -// api. Any changes must be considered carefully and should not proceed -// without a change proposal in docker core. -package errors +package v2 import ( "fmt" diff --git a/api/errors/errors_test.go b/api/v2/errors_test.go similarity index 99% rename from api/errors/errors_test.go rename to api/v2/errors_test.go index 2f5c69e0..d2fc091a 100644 --- a/api/errors/errors_test.go +++ b/api/v2/errors_test.go @@ -1,4 +1,4 @@ -package errors +package v2 import ( "encoding/json" diff --git a/api/urls/routes.go b/api/v2/routes.go similarity index 99% rename from api/urls/routes.go rename to api/v2/routes.go index 79138a4a..7ebe61d6 100644 --- a/api/urls/routes.go +++ b/api/v2/routes.go @@ -1,4 +1,4 @@ -package urls +package v2 import ( "github.com/docker/docker-registry/common" diff --git a/api/urls/routes_test.go b/api/v2/routes_test.go similarity index 99% rename from api/urls/routes_test.go rename to api/v2/routes_test.go index f2e95270..9969ebcc 100644 --- a/api/urls/routes_test.go +++ b/api/v2/routes_test.go @@ -1,4 +1,4 @@ -package urls +package v2 import ( "encoding/json" diff --git a/api/urls/urls.go b/api/v2/urls.go similarity index 99% rename from api/urls/urls.go rename to api/v2/urls.go index 86306c67..15d65484 100644 --- a/api/urls/urls.go +++ b/api/v2/urls.go @@ -1,4 +1,4 @@ -package urls +package v2 import ( "net/http" From d08f0edcf18c0a854c8bda6ac8e1ac41598b3639 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 11 Dec 2014 22:24:25 -0800 Subject: [PATCH 10/12] Migrate references to consolidated v2 package Routes and errors are now all referenced from a single v2 package. This packages exports are acceptable for use in the server side as well as integration into docker core. --- api/v2/descriptors.go | 3 +++ api_test.go | 25 ++++++++++++------------- app.go | 18 +++++++++--------- app_test.go | 16 ++++++++-------- client/client.go | 26 +++++++++++++------------- client/objectstore.go | 5 ++--- client/push.go | 7 +++---- cmd/registry-api-doctable-gen/main.go | 6 +++--- context.go | 7 +++---- images.go | 16 ++++++++-------- layer.go | 8 ++++---- layerupload.go | 24 ++++++++++++------------ tags.go | 4 ++-- 13 files changed, 82 insertions(+), 83 deletions(-) diff --git a/api/v2/descriptors.go b/api/v2/descriptors.go index 77528d72..68d18241 100644 --- a/api/v2/descriptors.go +++ b/api/v2/descriptors.go @@ -2,6 +2,9 @@ package v2 import "net/http" +// TODO(stevvooe): Add route descriptors for each named route, along with +// accepted methods, parameters, returned status codes and error codes. + // ErrorDescriptor provides relevant information about a given error code. type ErrorDescriptor struct { // Code is the error code that this descriptor describes. diff --git a/api_test.go b/api_test.go index e71edc17..a650a102 100644 --- a/api_test.go +++ b/api_test.go @@ -13,8 +13,7 @@ import ( "os" "testing" - "github.com/docker/docker-registry/api/errors" - "github.com/docker/docker-registry/api/urls" + "github.com/docker/docker-registry/api/v2" "github.com/docker/docker-registry/common/testutil" "github.com/docker/docker-registry/configuration" "github.com/docker/docker-registry/digest" @@ -35,7 +34,7 @@ func TestCheckAPI(t *testing.T) { app := NewApp(config) server := httptest.NewServer(handlers.CombinedLoggingHandler(os.Stderr, app)) - builder, err := urls.NewURLBuilderFromString(server.URL) + builder, err := v2.NewURLBuilderFromString(server.URL) if err != nil { t.Fatalf("error creating url builder: %v", err) @@ -82,7 +81,7 @@ func TestLayerAPI(t *testing.T) { app := NewApp(config) server := httptest.NewServer(handlers.CombinedLoggingHandler(os.Stderr, app)) - builder, err := urls.NewURLBuilderFromString(server.URL) + builder, err := v2.NewURLBuilderFromString(server.URL) if err != nil { t.Fatalf("error creating url builder: %v", err) @@ -197,7 +196,7 @@ func TestManifestAPI(t *testing.T) { app := NewApp(config) server := httptest.NewServer(handlers.CombinedLoggingHandler(os.Stderr, app)) - builder, err := urls.NewURLBuilderFromString(server.URL) + builder, err := v2.NewURLBuilderFromString(server.URL) if err != nil { t.Fatalf("unexpected error creating url builder: %v", err) } @@ -228,7 +227,7 @@ func TestManifestAPI(t *testing.T) { // } dec := json.NewDecoder(resp.Body) - var respErrs errors.Errors + var respErrs v2.Errors if err := dec.Decode(&respErrs); err != nil { t.Fatalf("unexpected error decoding error response: %v", err) } @@ -237,7 +236,7 @@ func TestManifestAPI(t *testing.T) { t.Fatalf("expected errors in response") } - if respErrs.Errors[0].Code != errors.ErrorCodeManifestUnknown { + if respErrs.Errors[0].Code != v2.ErrorCodeManifestUnknown { t.Fatalf("expected manifest unknown error: got %v", respErrs) } @@ -263,7 +262,7 @@ func TestManifestAPI(t *testing.T) { t.Fatalf("expected errors in response") } - if respErrs.Errors[0].Code != errors.ErrorCodeNameUnknown { + if respErrs.Errors[0].Code != v2.ErrorCodeNameUnknown { t.Fatalf("expected respository unknown error: got %v", respErrs) } @@ -297,11 +296,11 @@ func TestManifestAPI(t *testing.T) { for _, err := range respErrs.Errors { switch err.Code { - case errors.ErrorCodeManifestUnverified: + case v2.ErrorCodeManifestUnverified: unverified++ - case errors.ErrorCodeBlobUnknown: + case v2.ErrorCodeBlobUnknown: missingLayers++ - case errors.ErrorCodeDigestInvalid: + case v2.ErrorCodeDigestInvalid: // TODO(stevvooe): This error isn't quite descriptive enough -- // the layer with an invalid digest isn't identified. invalidDigests++ @@ -428,7 +427,7 @@ func putManifest(t *testing.T, msg, url string, v interface{}) *http.Response { return resp } -func startPushLayer(t *testing.T, ub *urls.URLBuilder, name string) string { +func startPushLayer(t *testing.T, ub *v2.URLBuilder, name string) string { layerUploadURL, err := ub.BuildBlobUploadURL(name) if err != nil { t.Fatalf("unexpected error building layer upload url: %v", err) @@ -450,7 +449,7 @@ func startPushLayer(t *testing.T, ub *urls.URLBuilder, name string) string { } // pushLayer pushes the layer content returning the url on success. -func pushLayer(t *testing.T, ub *urls.URLBuilder, name string, dgst digest.Digest, uploadURLBase string, rs io.ReadSeeker) string { +func pushLayer(t *testing.T, ub *v2.URLBuilder, name string, dgst digest.Digest, uploadURLBase string, rs io.ReadSeeker) string { rsLength, _ := rs.Seek(0, os.SEEK_END) rs.Seek(0, os.SEEK_SET) diff --git a/app.go b/app.go index e1b299a7..5a770c6c 100644 --- a/app.go +++ b/app.go @@ -4,7 +4,7 @@ import ( "fmt" "net/http" - "github.com/docker/docker-registry/api/urls" + "github.com/docker/docker-registry/api/v2" "github.com/docker/docker-registry/storagedriver" "github.com/docker/docker-registry/storagedriver/factory" @@ -36,18 +36,18 @@ type App struct { func NewApp(configuration configuration.Configuration) *App { app := &App{ Config: configuration, - router: urls.Router(), + router: v2.Router(), } // Register the handler dispatchers. - app.register(urls.RouteNameBase, func(ctx *Context, r *http.Request) http.Handler { + app.register(v2.RouteNameBase, func(ctx *Context, r *http.Request) http.Handler { return http.HandlerFunc(apiBase) }) - app.register(urls.RouteNameManifest, imageManifestDispatcher) - app.register(urls.RouteNameTags, tagsDispatcher) - app.register(urls.RouteNameBlob, layerDispatcher) - app.register(urls.RouteNameBlobUpload, layerUploadDispatcher) - app.register(urls.RouteNameBlobUploadChunk, layerUploadDispatcher) + app.register(v2.RouteNameManifest, imageManifestDispatcher) + app.register(v2.RouteNameTags, tagsDispatcher) + app.register(v2.RouteNameBlob, layerDispatcher) + app.register(v2.RouteNameBlobUpload, layerUploadDispatcher) + app.register(v2.RouteNameBlobUploadChunk, layerUploadDispatcher) driver, err := factory.Create(configuration.Storage.Type(), configuration.Storage.Parameters()) @@ -115,7 +115,7 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { context := &Context{ App: app, Name: vars["name"], - urlBuilder: urls.NewURLBuilderFromRequest(r), + urlBuilder: v2.NewURLBuilderFromRequest(r), } // Store vars for underlying handlers. diff --git a/app_test.go b/app_test.go index eb8ec597..f256c968 100644 --- a/app_test.go +++ b/app_test.go @@ -6,7 +6,7 @@ import ( "net/url" "testing" - "github.com/docker/docker-registry/api/urls" + "github.com/docker/docker-registry/api/v2" "github.com/docker/docker-registry/configuration" ) @@ -17,10 +17,10 @@ import ( func TestAppDispatcher(t *testing.T) { app := &App{ Config: configuration.Configuration{}, - router: urls.Router(), + router: v2.Router(), } server := httptest.NewServer(app) - router := urls.Router() + router := v2.Router() serverURL, err := url.Parse(server.URL) if err != nil { @@ -72,33 +72,33 @@ func TestAppDispatcher(t *testing.T) { vars []string }{ { - endpoint: urls.RouteNameManifest, + endpoint: v2.RouteNameManifest, vars: []string{ "name", "foo/bar", "tag", "sometag", }, }, { - endpoint: urls.RouteNameTags, + endpoint: v2.RouteNameTags, vars: []string{ "name", "foo/bar", }, }, { - endpoint: urls.RouteNameBlob, + endpoint: v2.RouteNameBlob, vars: []string{ "name", "foo/bar", "digest", "tarsum.v1+bogus:abcdef0123456789", }, }, { - endpoint: urls.RouteNameBlobUpload, + endpoint: v2.RouteNameBlobUpload, vars: []string{ "name", "foo/bar", }, }, { - endpoint: urls.RouteNameBlobUploadChunk, + endpoint: v2.RouteNameBlobUploadChunk, vars: []string{ "name", "foo/bar", "uuid", "theuuid", diff --git a/client/client.go b/client/client.go index 8f31cb4e..e25fbff4 100644 --- a/client/client.go +++ b/client/client.go @@ -10,7 +10,7 @@ import ( "regexp" "strconv" - "github.com/docker/docker-registry/api/errors" + "github.com/docker/docker-registry/api/v2" "github.com/docker/docker-registry/digest" "github.com/docker/docker-registry/storage" ) @@ -96,7 +96,7 @@ func (r *clientImpl) GetImageManifest(name, tag string) (*storage.SignedManifest case response.StatusCode == http.StatusNotFound: return nil, &ImageManifestNotFoundError{Name: name, Tag: tag} case response.StatusCode >= 400 && response.StatusCode < 500: - var errs errors.Errors + var errs v2.Errors decoder := json.NewDecoder(response.Body) err = decoder.Decode(&errs) @@ -136,7 +136,7 @@ func (r *clientImpl) PutImageManifest(name, tag string, manifest *storage.Signed case response.StatusCode == http.StatusOK: return nil case response.StatusCode >= 400 && response.StatusCode < 500: - var errors errors.Errors + var errors v2.Errors decoder := json.NewDecoder(response.Body) err = decoder.Decode(&errors) if err != nil { @@ -169,7 +169,7 @@ func (r *clientImpl) DeleteImage(name, tag string) error { case response.StatusCode == http.StatusNotFound: return &ImageManifestNotFoundError{Name: name, Tag: tag} case response.StatusCode >= 400 && response.StatusCode < 500: - var errs errors.Errors + var errs v2.Errors decoder := json.NewDecoder(response.Body) err = decoder.Decode(&errs) if err != nil { @@ -197,7 +197,7 @@ func (r *clientImpl) ListImageTags(name string) ([]string, error) { case response.StatusCode == http.StatusNotFound: return nil, &RepositoryNotFoundError{Name: name} case response.StatusCode >= 400 && response.StatusCode < 500: - var errs errors.Errors + var errs v2.Errors decoder := json.NewDecoder(response.Body) err = decoder.Decode(&errs) if err != nil { @@ -240,7 +240,7 @@ func (r *clientImpl) BlobLength(name string, dgst digest.Digest) (int, error) { case response.StatusCode == http.StatusNotFound: return -1, nil case response.StatusCode >= 400 && response.StatusCode < 500: - var errs errors.Errors + var errs v2.Errors decoder := json.NewDecoder(response.Body) err = decoder.Decode(&errs) if err != nil { @@ -279,7 +279,7 @@ func (r *clientImpl) GetBlob(name string, dgst digest.Digest, byteOffset int) (i response.Body.Close() return nil, 0, &BlobNotFoundError{Name: name, Digest: dgst} case response.StatusCode >= 400 && response.StatusCode < 500: - var errs errors.Errors + var errs v2.Errors decoder := json.NewDecoder(response.Body) err = decoder.Decode(&errs) if err != nil { @@ -312,7 +312,7 @@ func (r *clientImpl) InitiateBlobUpload(name string) (string, error) { // case response.StatusCode == http.StatusNotFound: // return case response.StatusCode >= 400 && response.StatusCode < 500: - var errs errors.Errors + var errs v2.Errors decoder := json.NewDecoder(response.Body) err = decoder.Decode(&errs) if err != nil { @@ -338,7 +338,7 @@ func (r *clientImpl) GetBlobUploadStatus(location string) (int, int, error) { case response.StatusCode == http.StatusNotFound: return 0, 0, &BlobUploadNotFoundError{Location: location} case response.StatusCode >= 400 && response.StatusCode < 500: - var errs errors.Errors + var errs v2.Errors decoder := json.NewDecoder(response.Body) err = decoder.Decode(&errs) if err != nil { @@ -379,7 +379,7 @@ func (r *clientImpl) UploadBlob(location string, blob io.ReadCloser, length int, case response.StatusCode == http.StatusNotFound: return &BlobUploadNotFoundError{Location: location} case response.StatusCode >= 400 && response.StatusCode < 500: - var errs errors.Errors + var errs v2.Errors decoder := json.NewDecoder(response.Body) err = decoder.Decode(&errs) if err != nil { @@ -430,7 +430,7 @@ func (r *clientImpl) UploadBlobChunk(location string, blobChunk io.ReadCloser, l case response.StatusCode == http.StatusNotFound: return &BlobUploadNotFoundError{Location: location} case response.StatusCode >= 400 && response.StatusCode < 500: - var errs errors.Errors + var errs v2.Errors decoder := json.NewDecoder(response.Body) err = decoder.Decode(&errs) if err != nil { @@ -472,7 +472,7 @@ func (r *clientImpl) FinishChunkedBlobUpload(location string, length int, dgst d case response.StatusCode == http.StatusNotFound: return &BlobUploadNotFoundError{Location: location} case response.StatusCode >= 400 && response.StatusCode < 500: - var errs errors.Errors + var errs v2.Errors decoder := json.NewDecoder(response.Body) err = decoder.Decode(&errs) if err != nil { @@ -504,7 +504,7 @@ func (r *clientImpl) CancelBlobUpload(location string) error { case response.StatusCode == http.StatusNotFound: return &BlobUploadNotFoundError{Location: location} case response.StatusCode >= 400 && response.StatusCode < 500: - var errs errors.Errors + var errs v2.Errors decoder := json.NewDecoder(response.Body) err = decoder.Decode(&errs) if err != nil { diff --git a/client/objectstore.go b/client/objectstore.go index 06fba3d8..55ab20a5 100644 --- a/client/objectstore.go +++ b/client/objectstore.go @@ -2,7 +2,6 @@ package client import ( "bytes" - "errors" "fmt" "io" "sync" @@ -14,11 +13,11 @@ import ( var ( // ErrLayerAlreadyExists is returned when attempting to create a layer with // a tarsum that is already in use. - ErrLayerAlreadyExists = errors.New("Layer already exists") + ErrLayerAlreadyExists = fmt.Errorf("Layer already exists") // ErrLayerLocked is returned when attempting to write to a layer which is // currently being written to. - ErrLayerLocked = errors.New("Layer locked") + ErrLayerLocked = fmt.Errorf("Layer locked") ) // ObjectStore is an interface which is designed to approximate the docker diff --git a/client/push.go b/client/push.go index 61853b53..aac3fc40 100644 --- a/client/push.go +++ b/client/push.go @@ -1,11 +1,10 @@ package client import ( - "errors" - - "github.com/docker/docker-registry/storage" + "fmt" log "github.com/Sirupsen/logrus" + "github.com/docker/docker-registry/storage" ) // simultaneousLayerPushWindow is the size of the parallel layer push window. @@ -100,7 +99,7 @@ func pushLayer(c Client, objectStore ObjectStore, name string, fsLayer storage.F "currentSize": layerReader.CurrentSize(), "size": layerReader.Size(), }).Warn("Local layer incomplete") - return errors.New("Local layer incomplete") + return fmt.Errorf("Local layer incomplete") } length, err := c.BlobLength(name, fsLayer.BlobSum) diff --git a/cmd/registry-api-doctable-gen/main.go b/cmd/registry-api-doctable-gen/main.go index 869f1a37..a9e71fff 100644 --- a/cmd/registry-api-doctable-gen/main.go +++ b/cmd/registry-api-doctable-gen/main.go @@ -17,7 +17,7 @@ import ( "strings" "text/tabwriter" - "github.com/docker/docker-registry/api/errors" + "github.com/docker/docker-registry/api/v2" ) func main() { @@ -40,7 +40,7 @@ func dumpErrors(wr io.Writer) { defer writer.Flush() fmt.Fprint(writer, "|") - dtype := reflect.TypeOf(errors.ErrorDescriptor{}) + dtype := reflect.TypeOf(v2.ErrorDescriptor{}) var fieldsPrinted int for i := 0; i < dtype.NumField(); i++ { field := dtype.Field(i) @@ -61,7 +61,7 @@ func dumpErrors(wr io.Writer) { fmt.Fprintln(writer, "\n"+divider) - for _, descriptor := range errors.ErrorDescriptors { + for _, descriptor := range v2.ErrorDescriptors { fmt.Fprint(writer, "|") v := reflect.ValueOf(descriptor) diff --git a/context.go b/context.go index 23d669b8..0c5ba587 100644 --- a/context.go +++ b/context.go @@ -2,8 +2,7 @@ package registry import ( "github.com/Sirupsen/logrus" - "github.com/docker/docker-registry/api/errors" - "github.com/docker/docker-registry/api/urls" + "github.com/docker/docker-registry/api/v2" ) // Context should contain the request specific context for use in across @@ -20,7 +19,7 @@ type Context struct { // Errors is a collection of errors encountered during the request to be // returned to the client API. If errors are added to the collection, the // handler *must not* start the response via http.ResponseWriter. - Errors errors.Errors + Errors v2.Errors // vars contains the extracted gorilla/mux variables that can be used for // assignment. @@ -29,5 +28,5 @@ type Context struct { // log provides a context specific logger. log *logrus.Entry - urlBuilder *urls.URLBuilder + urlBuilder *v2.URLBuilder } diff --git a/images.go b/images.go index 74ae067e..5a373f1f 100644 --- a/images.go +++ b/images.go @@ -5,7 +5,7 @@ import ( "fmt" "net/http" - "github.com/docker/docker-registry/api/errors" + "github.com/docker/docker-registry/api/v2" "github.com/docker/docker-registry/digest" "github.com/docker/docker-registry/storage" "github.com/gorilla/handlers" @@ -41,7 +41,7 @@ func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http manifest, err := manifests.Get(imh.Name, imh.Tag) if err != nil { - imh.Errors.Push(errors.ErrorCodeManifestUnknown, err) + imh.Errors.Push(v2.ErrorCodeManifestUnknown, err) w.WriteHeader(http.StatusNotFound) return } @@ -58,7 +58,7 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http var manifest storage.SignedManifest if err := dec.Decode(&manifest); err != nil { - imh.Errors.Push(errors.ErrorCodeManifestInvalid, err) + imh.Errors.Push(v2.ErrorCodeManifestInvalid, err) w.WriteHeader(http.StatusBadRequest) return } @@ -71,14 +71,14 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http for _, verificationError := range err { switch verificationError := verificationError.(type) { case storage.ErrUnknownLayer: - imh.Errors.Push(errors.ErrorCodeBlobUnknown, verificationError.FSLayer) + imh.Errors.Push(v2.ErrorCodeBlobUnknown, verificationError.FSLayer) case storage.ErrManifestUnverified: - imh.Errors.Push(errors.ErrorCodeManifestUnverified) + imh.Errors.Push(v2.ErrorCodeManifestUnverified) default: if verificationError == digest.ErrDigestInvalidFormat { // TODO(stevvooe): We need to really need to move all // errors to types. Its much more straightforward. - imh.Errors.Push(errors.ErrorCodeDigestInvalid) + imh.Errors.Push(v2.ErrorCodeDigestInvalid) } else { imh.Errors.PushErr(verificationError) } @@ -99,10 +99,10 @@ func (imh *imageManifestHandler) DeleteImageManifest(w http.ResponseWriter, r *h if err := manifests.Delete(imh.Name, imh.Tag); err != nil { switch err := err.(type) { case storage.ErrUnknownManifest: - imh.Errors.Push(errors.ErrorCodeManifestUnknown, err) + imh.Errors.Push(v2.ErrorCodeManifestUnknown, err) w.WriteHeader(http.StatusNotFound) default: - imh.Errors.Push(errors.ErrorCodeUnknown, err) + imh.Errors.Push(v2.ErrorCodeUnknown, err) w.WriteHeader(http.StatusBadRequest) } return diff --git a/layer.go b/layer.go index 4da7723a..094a54cf 100644 --- a/layer.go +++ b/layer.go @@ -3,7 +3,7 @@ package registry import ( "net/http" - "github.com/docker/docker-registry/api/errors" + "github.com/docker/docker-registry/api/v2" "github.com/docker/docker-registry/digest" "github.com/docker/docker-registry/storage" "github.com/gorilla/handlers" @@ -15,7 +15,7 @@ func layerDispatcher(ctx *Context, r *http.Request) http.Handler { if err != nil { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx.Errors.Push(errors.ErrorCodeDigestInvalid, err) + ctx.Errors.Push(v2.ErrorCodeDigestInvalid, err) }) } @@ -50,9 +50,9 @@ func (lh *layerHandler) GetLayer(w http.ResponseWriter, r *http.Request) { switch err := err.(type) { case storage.ErrUnknownLayer: w.WriteHeader(http.StatusNotFound) - lh.Errors.Push(errors.ErrorCodeBlobUnknown, err.FSLayer) + lh.Errors.Push(v2.ErrorCodeBlobUnknown, err.FSLayer) default: - lh.Errors.Push(errors.ErrorCodeUnknown, err) + lh.Errors.Push(v2.ErrorCodeUnknown, err) } return } diff --git a/layerupload.go b/layerupload.go index 898b279b..b65c8ef2 100644 --- a/layerupload.go +++ b/layerupload.go @@ -7,7 +7,7 @@ import ( "strconv" "github.com/Sirupsen/logrus" - "github.com/docker/docker-registry/api/errors" + "github.com/docker/docker-registry/api/v2" "github.com/docker/docker-registry/digest" "github.com/docker/docker-registry/storage" "github.com/gorilla/handlers" @@ -39,7 +39,7 @@ func layerUploadDispatcher(ctx *Context, r *http.Request) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { logrus.Infof("error resolving upload: %v", err) w.WriteHeader(http.StatusInternalServerError) - luh.Errors.Push(errors.ErrorCodeUnknown, err) + luh.Errors.Push(v2.ErrorCodeUnknown, err) }) } @@ -67,7 +67,7 @@ func (luh *layerUploadHandler) StartLayerUpload(w http.ResponseWriter, r *http.R upload, err := layers.Upload(luh.Name) if err != nil { w.WriteHeader(http.StatusInternalServerError) // Error conditions here? - luh.Errors.Push(errors.ErrorCodeUnknown, err) + luh.Errors.Push(v2.ErrorCodeUnknown, err) return } @@ -76,7 +76,7 @@ func (luh *layerUploadHandler) StartLayerUpload(w http.ResponseWriter, r *http.R if err := luh.layerUploadResponse(w, r); err != nil { w.WriteHeader(http.StatusInternalServerError) // Error conditions here? - luh.Errors.Push(errors.ErrorCodeUnknown, err) + luh.Errors.Push(v2.ErrorCodeUnknown, err) return } w.WriteHeader(http.StatusAccepted) @@ -86,12 +86,12 @@ func (luh *layerUploadHandler) StartLayerUpload(w http.ResponseWriter, r *http.R func (luh *layerUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Request) { if luh.Upload == nil { w.WriteHeader(http.StatusNotFound) - luh.Errors.Push(errors.ErrorCodeBlobUploadUnknown) + luh.Errors.Push(v2.ErrorCodeBlobUploadUnknown) } if err := luh.layerUploadResponse(w, r); err != nil { w.WriteHeader(http.StatusInternalServerError) // Error conditions here? - luh.Errors.Push(errors.ErrorCodeUnknown, err) + luh.Errors.Push(v2.ErrorCodeUnknown, err) return } @@ -103,7 +103,7 @@ func (luh *layerUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Re func (luh *layerUploadHandler) PutLayerChunk(w http.ResponseWriter, r *http.Request) { if luh.Upload == nil { w.WriteHeader(http.StatusNotFound) - luh.Errors.Push(errors.ErrorCodeBlobUploadUnknown) + luh.Errors.Push(v2.ErrorCodeBlobUploadUnknown) } var finished bool @@ -120,14 +120,14 @@ func (luh *layerUploadHandler) PutLayerChunk(w http.ResponseWriter, r *http.Requ if err := luh.maybeCompleteUpload(w, r); err != nil { if err != errNotReadyToComplete { w.WriteHeader(http.StatusInternalServerError) - luh.Errors.Push(errors.ErrorCodeUnknown, err) + luh.Errors.Push(v2.ErrorCodeUnknown, err) return } } if err := luh.layerUploadResponse(w, r); err != nil { w.WriteHeader(http.StatusInternalServerError) // Error conditions here? - luh.Errors.Push(errors.ErrorCodeUnknown, err) + luh.Errors.Push(v2.ErrorCodeUnknown, err) return } @@ -142,7 +142,7 @@ func (luh *layerUploadHandler) PutLayerChunk(w http.ResponseWriter, r *http.Requ func (luh *layerUploadHandler) CancelLayerUpload(w http.ResponseWriter, r *http.Request) { if luh.Upload == nil { w.WriteHeader(http.StatusNotFound) - luh.Errors.Push(errors.ErrorCodeBlobUploadUnknown) + luh.Errors.Push(v2.ErrorCodeBlobUploadUnknown) } } @@ -195,14 +195,14 @@ func (luh *layerUploadHandler) maybeCompleteUpload(w http.ResponseWriter, r *htt func (luh *layerUploadHandler) completeUpload(w http.ResponseWriter, r *http.Request, size int64, dgst digest.Digest) { layer, err := luh.Upload.Finish(size, dgst) if err != nil { - luh.Errors.Push(errors.ErrorCodeUnknown, err) + luh.Errors.Push(v2.ErrorCodeUnknown, err) w.WriteHeader(http.StatusInternalServerError) return } layerURL, err := luh.urlBuilder.BuildBlobURL(layer.Name(), layer.Digest()) if err != nil { - luh.Errors.Push(errors.ErrorCodeUnknown, err) + luh.Errors.Push(v2.ErrorCodeUnknown, err) w.WriteHeader(http.StatusInternalServerError) return } diff --git a/tags.go b/tags.go index 12a5062f..04d994b9 100644 --- a/tags.go +++ b/tags.go @@ -4,7 +4,7 @@ import ( "encoding/json" "net/http" - "github.com/docker/docker-registry/api/errors" + "github.com/docker/docker-registry/api/v2" "github.com/docker/docker-registry/storage" "github.com/gorilla/handlers" ) @@ -40,7 +40,7 @@ func (th *tagsHandler) GetTags(w http.ResponseWriter, r *http.Request) { switch err := err.(type) { case storage.ErrUnknownRepository: w.WriteHeader(404) - th.Errors.Push(errors.ErrorCodeNameUnknown, map[string]string{"name": th.Name}) + th.Errors.Push(v2.ErrorCodeNameUnknown, map[string]string{"name": th.Name}) default: th.Errors.PushErr(err) } From bb300231d04ec749818a5640b14d03790390c1ac Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Fri, 12 Dec 2014 13:55:14 -0800 Subject: [PATCH 11/12] Allow variadic url values for BuildBlobUploadURL URL values may be required to complete an upload in a single POST request, which may include digest and size. This is not implemented in the server side, yet, but is part of the HTTP API specification. --- api/v2/urls.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/v2/urls.go b/api/v2/urls.go index 15d65484..72f44299 100644 --- a/api/v2/urls.go +++ b/api/v2/urls.go @@ -101,7 +101,7 @@ 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) (string, error) { +func (ub *URLBuilder) BuildBlobUploadURL(name string, values ...url.Values) (string, error) { route := ub.cloneRoute(RouteNameBlobUpload) uploadURL, err := route.URL("name", name) @@ -109,7 +109,7 @@ func (ub *URLBuilder) BuildBlobUploadURL(name string) (string, error) { return "", err } - return uploadURL.String(), nil + return appendValuesURL(uploadURL, values...).String(), nil } // BuildBlobUploadChunkURL constructs a url for the upload identified by uuid, From 39169384810393a00c40ed4dfaf4fa4fe4ce3d08 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Fri, 12 Dec 2014 15:48:41 -0800 Subject: [PATCH 12/12] Add tests for URLBuilder --- api/v2/urls_test.go | 100 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 api/v2/urls_test.go diff --git a/api/v2/urls_test.go b/api/v2/urls_test.go new file mode 100644 index 00000000..a9590dba --- /dev/null +++ b/api/v2/urls_test.go @@ -0,0 +1,100 @@ +package v2 + +import ( + "net/url" + "testing" +) + +type urlBuilderTestCase struct { + description string + expected string + build func() (string, error) +} + +// TestURLBuilder tests the various url building functions, ensuring they are +// returning the expected values. +func TestURLBuilder(t *testing.T) { + + root := "http://localhost:5000/" + urlBuilder, err := NewURLBuilderFromString(root) + if err != nil { + t.Fatalf("unexpected error creating urlbuilder: %v", err) + } + + for _, testcase := range []struct { + description string + expected string + build func() (string, error) + }{ + { + description: "test base url", + expected: "http://localhost:5000/v2/", + build: urlBuilder.BuildBaseURL, + }, + { + description: "test tags url", + expected: "http://localhost:5000/v2/foo/bar/tags/list", + build: func() (string, error) { + return urlBuilder.BuildTagsURL("foo/bar") + }, + }, + { + description: "test manifest url", + expected: "http://localhost:5000/v2/foo/bar/manifests/tag", + build: func() (string, error) { + return urlBuilder.BuildManifestURL("foo/bar", "tag") + }, + }, + { + description: "build blob url", + expected: "http://localhost:5000/v2/foo/bar/blobs/tarsum.v1+sha256:abcdef0123456789", + build: func() (string, error) { + return urlBuilder.BuildBlobURL("foo/bar", "tarsum.v1+sha256:abcdef0123456789") + }, + }, + { + description: "build blob upload url", + expected: "http://localhost:5000/v2/foo/bar/blobs/uploads/", + build: func() (string, error) { + return urlBuilder.BuildBlobUploadURL("foo/bar") + }, + }, + { + description: "build blob upload url with digest and size", + expected: "http://localhost:5000/v2/foo/bar/blobs/uploads/?digest=tarsum.v1%2Bsha256%3Aabcdef0123456789&size=10000", + build: func() (string, error) { + return urlBuilder.BuildBlobUploadURL("foo/bar", url.Values{ + "size": []string{"10000"}, + "digest": []string{"tarsum.v1+sha256:abcdef0123456789"}, + }) + }, + }, + { + description: "build blob upload chunk url", + expected: "http://localhost:5000/v2/foo/bar/blobs/uploads/uuid-part", + build: func() (string, error) { + return urlBuilder.BuildBlobUploadChunkURL("foo/bar", "uuid-part") + }, + }, + { + description: "build blob upload chunk url with digest and size", + expected: "http://localhost:5000/v2/foo/bar/blobs/uploads/uuid-part?digest=tarsum.v1%2Bsha256%3Aabcdef0123456789&size=10000", + build: func() (string, error) { + return urlBuilder.BuildBlobUploadChunkURL("foo/bar", "uuid-part", url.Values{ + "size": []string{"10000"}, + "digest": []string{"tarsum.v1+sha256:abcdef0123456789"}, + }) + }, + }, + } { + u, err := testcase.build() + if err != nil { + t.Fatalf("%s: error building url: %v", testcase.description, err) + } + + if u != testcase.expected { + t.Fatalf("%s: %q != %q", testcase.description, u, testcase.expected) + } + } + +}