Disambiguate routing for multi-level repository names
To be able to support multi-level repository names, the API has been adjusted to disabiguate routes tagged image manifest routes and tag list routes. With this effort, the regular expressions have been defined in a single place to reduce repitition and ensure that validation is consistent across the registry. The router was also refactored to remove the use of subrouters, simplifying the route definition code. This also reduces the number of regular expression match checks during the routing process.master
							parent
							
								
									375f3cc136
								
							
						
					
					
						commit
						145c89bb94
					
				|  | @ -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", | ||||
| 			}, | ||||
| 		}, | ||||
|  |  | |||
							
								
								
									
										38
									
								
								routes.go
								
								
								
								
							
							
						
						
									
										38
									
								
								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/<name>/image/<tag>	Image Manifest	Fetch the image manifest identified by name and tag.
 | ||||
| 	// PUT      /v2/<name>/image/<tag>	Image Manifest	Upload the image manifest identified by name and tag.
 | ||||
| 	// DELETE   /v2/<name>/image/<tag>	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/<name>/tags	Tags	Fetch the tags under the repository identified by name.
 | ||||
| 	namedRouter. | ||||
| 		Path("/tags"). | ||||
| 	// GET	/v2/<name>/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/<name>/layer/<tarsum>	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/<name>/layer/<tarsum>/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/<name>/layer/<tarsum>/upload/<uuid>	Layer Upload	Get the status of the upload identified by tarsum and uuid.
 | ||||
| 	// PUT	/v2/<name>/layer/<tarsum>/upload/<uuid>	Layer Upload	Upload all or a chunk of the upload identified by tarsum and uuid.
 | ||||
| 	// DELETE	/v2/<name>/layer/<tarsum>/upload/<uuid>	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 | ||||
|  |  | |||
							
								
								
									
										121
									
								
								routes_test.go
								
								
								
								
							
							
						
						
									
										121
									
								
								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) | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue