diff --git a/app_test.go b/app_test.go index 43b001ec..e0fa727f 100644 --- a/app_test.go +++ b/app_test.go @@ -87,21 +87,21 @@ func TestAppDispatcher(t *testing.T) { endpoint: routeNameLayer, vars: []string{ "name", "foo/bar", - "tarsum", "thetarsum", + "tarsum", "tarsum.v1+bogus:abcdef0123456789", }, }, { endpoint: routeNameLayerUpload, vars: []string{ "name", "foo/bar", - "tarsum", "thetarsum", + "tarsum", "tarsum.v1+bogus:abcdef0123456789", }, }, { endpoint: routeNameLayerUploadResume, vars: []string{ "name", "foo/bar", - "tarsum", "thetarsum", + "tarsum", "tarsum.v1+bogus:abcdef0123456789", "uuid", "theuuid", }, }, diff --git a/routes.go b/routes.go index d4942696..8da7c3e2 100644 --- a/routes.go +++ b/routes.go @@ -1,12 +1,11 @@ package registry import ( + "github.com/docker/docker-registry/common" "github.com/gorilla/mux" ) const ( - routeNameRoot = "root" - routeNameName = "name" routeNameImageManifest = "image-manifest" routeNameTags = "tags" routeNameLayer = "layer" @@ -25,47 +24,36 @@ var allEndpoints = []string{ // 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 := mux.NewRouter() - - rootRouter := router. - PathPrefix("/v2"). - Name(routeNameRoot). - Subrouter() - - // All routes are subordinate to named routes - namedRouter := rootRouter. - PathPrefix("/{name:[A-Za-z0-9-_]+/[A-Za-z0-9-_]+}"). // TODO(stevvooe): Verify this format with core - Name(routeNameName). - Subrouter(). + router := mux.NewRouter(). StrictSlash(true) // GET /v2//image/ Image Manifest Fetch the image manifest identified by name and tag. // PUT /v2//image/ Image Manifest Upload the image manifest identified by name and tag. // DELETE /v2//image/ Image Manifest Delete the image identified by name and tag. - namedRouter. - Path("/image/{tag:[A-Za-z0-9-_]+}"). + router. + Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/image/{tag:" + common.TagNameRegexp.String() + "}"). Name(routeNameImageManifest) - // GET /v2//tags Tags Fetch the tags under the repository identified by name. - namedRouter. - Path("/tags"). + // 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) // GET /v2//layer/ Layer Fetch the layer identified by tarsum. - namedRouter. - Path("/layer/{tarsum}"). + router. + Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/layer/{tarsum:" + common.TarsumRegexp.String() + "}"). Name(routeNameLayer) // POST /v2//layer//upload/ Layer Upload Initiate an upload of the layer identified by tarsum. Requires length and a checksum parameter. - namedRouter. - Path("/layer/{tarsum}/upload/"). + router. + Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/layer/{tarsum:" + common.TarsumRegexp.String() + "}/upload/"). Name(routeNameLayerUpload) // GET /v2//layer//upload/ Layer Upload Get the status of the upload identified by tarsum and uuid. // PUT /v2//layer//upload/ Layer Upload Upload all or a chunk of the upload identified by tarsum and uuid. // DELETE /v2//layer//upload/ Layer Upload Cancel the upload identified by layer and uuid - namedRouter. - Path("/layer/{tarsum}/upload/{uuid}"). + router. + Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/layer/{tarsum:" + common.TarsumRegexp.String() + "}/upload/{uuid}"). Name(routeNameLayerUploadResume) return router diff --git a/routes_test.go b/routes_test.go index e3ef371a..8907684a 100644 --- a/routes_test.go +++ b/routes_test.go @@ -10,9 +10,10 @@ import ( "github.com/gorilla/mux" ) -type routeInfo struct { +type routeTestCase struct { RequestURI string Vars map[string]string + RouteName string } // TestRouter registers a test handler with all the routes and ensures that @@ -26,14 +27,15 @@ func TestRouter(t *testing.T) { router := v2APIRouter() testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - routeInfo := routeInfo{ + testCase := routeTestCase{ RequestURI: r.RequestURI, Vars: mux.Vars(r), + RouteName: mux.CurrentRoute(r).GetName(), } enc := json.NewEncoder(w) - if err := enc.Encode(routeInfo); err != nil { + if err := enc.Encode(testCase); err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } @@ -42,64 +44,81 @@ func TestRouter(t *testing.T) { // Startup test server server := httptest.NewServer(router) - for _, testcase := range []struct { - routeName string - expectedRouteInfo routeInfo - }{ + for _, testcase := range []routeTestCase{ { - routeName: routeNameImageManifest, - expectedRouteInfo: routeInfo{ - RequestURI: "/v2/foo/bar/image/tag", - Vars: map[string]string{ - "name": "foo/bar", - "tag": "tag", - }, + RouteName: routeNameImageManifest, + RequestURI: "/v2/foo/bar/image/tag", + Vars: map[string]string{ + "name": "foo/bar", + "tag": "tag", }, }, { - routeName: routeNameTags, - expectedRouteInfo: routeInfo{ - RequestURI: "/v2/foo/bar/tags", - Vars: map[string]string{ - "name": "foo/bar", - }, + RouteName: routeNameTags, + RequestURI: "/v2/foo/bar/tags/list", + Vars: map[string]string{ + "name": "foo/bar", }, }, { - routeName: routeNameLayer, - expectedRouteInfo: routeInfo{ - RequestURI: "/v2/foo/bar/layer/tarsum", - Vars: map[string]string{ - "name": "foo/bar", - "tarsum": "tarsum", - }, + RouteName: routeNameLayer, + RequestURI: "/v2/foo/bar/layer/tarsum.dev+foo:abcdef0919234", + Vars: map[string]string{ + "name": "foo/bar", + "tarsum": "tarsum.dev+foo:abcdef0919234", }, }, { - routeName: routeNameLayerUpload, - expectedRouteInfo: routeInfo{ - RequestURI: "/v2/foo/bar/layer/tarsum/upload/", - Vars: map[string]string{ - "name": "foo/bar", - "tarsum": "tarsum", - }, + RouteName: routeNameLayerUpload, + RequestURI: "/v2/foo/bar/layer/tarsum.dev+foo:abcdef0919234/upload/", + Vars: map[string]string{ + "name": "foo/bar", + "tarsum": "tarsum.dev+foo:abcdef0919234", }, }, { - routeName: routeNameLayerUploadResume, - expectedRouteInfo: routeInfo{ - RequestURI: "/v2/foo/bar/layer/tarsum/upload/uuid", - Vars: map[string]string{ - "name": "foo/bar", - "tarsum": "tarsum", - "uuid": "uuid", - }, + RouteName: routeNameLayerUploadResume, + RequestURI: "/v2/foo/bar/layer/tarsum.dev+foo:abcdef0919234/upload/uuid", + Vars: map[string]string{ + "name": "foo/bar", + "tarsum": "tarsum.dev+foo:abcdef0919234", + "uuid": "uuid", + }, + }, + { + RouteName: routeNameLayerUploadResume, + RequestURI: "/v2/foo/bar/layer/tarsum.dev+foo:abcdef0919234/upload/D95306FA-FAD3-4E36-8D41-CF1C93EF8286", + Vars: map[string]string{ + "name": "foo/bar", + "tarsum": "tarsum.dev+foo:abcdef0919234", + "uuid": "D95306FA-FAD3-4E36-8D41-CF1C93EF8286", + }, + }, + + { + // Check ambiguity: ensure we can distinguish between tags for + // "foo/bar/image/image" and image for "foo/bar/image" with tag + // "tags" + RouteName: routeNameImageManifest, + RequestURI: "/v2/foo/bar/image/image/tags", + Vars: map[string]string{ + "name": "foo/bar/image", + "tag": "tags", + }, + }, + { + // This case presents an ambiguity between foo/bar with tag="tags" + // and list tags for "foo/bar/image" + RouteName: routeNameTags, + RequestURI: "/v2/foo/bar/image/tags/list", + Vars: map[string]string{ + "name": "foo/bar/image", }, }, } { // Register the endpoint - router.GetRoute(testcase.routeName).Handler(testHandler) - u := server.URL + testcase.expectedRouteInfo.RequestURI + router.GetRoute(testcase.RouteName).Handler(testHandler) + u := server.URL + testcase.RequestURI resp, err := http.Get(u) @@ -107,15 +126,23 @@ func TestRouter(t *testing.T) { t.Fatalf("error issuing get request: %v", err) } + if resp.StatusCode != http.StatusOK { + t.Fatalf("unexpected status for %s: %v %v", u, resp.Status, resp.StatusCode) + } + dec := json.NewDecoder(resp.Body) - var actualRouteInfo routeInfo + var actualRouteInfo routeTestCase if err := dec.Decode(&actualRouteInfo); err != nil { t.Fatalf("error reading json response: %v", err) } - if !reflect.DeepEqual(actualRouteInfo, testcase.expectedRouteInfo) { - t.Fatalf("actual does not equal expected: %v != %v", actualRouteInfo, testcase.expectedRouteInfo) + if actualRouteInfo.RouteName != testcase.RouteName { + t.Fatalf("incorrect route %q matched, expected %q", actualRouteInfo.RouteName, testcase.RouteName) + } + + if !reflect.DeepEqual(actualRouteInfo, testcase) { + t.Fatalf("actual does not equal expected: %#v != %#v", actualRouteInfo, testcase) } }