Merge pull request #3999 from nikhiljindal/go-restful

Moving /watch, /proxy and /redirect registrations to go-restful

pkg/apiserver/api_installer.go

@@ -0,0 +1,320 @@
+/*
+Copyright 2015 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package apiserver
+
+import (
+	"net/http"
+	"reflect"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
+
+	"github.com/emicklei/go-restful"
+)
+
+type APIInstaller struct {
+	prefix      string // Path prefix where API resources are to be registered.
+	version     string // The API version being installed.
+	restHandler *RESTHandler
+	mapper      meta.RESTMapper
+}
+
+// Struct capturing information about an action ("GET", "POST", "WATCH", PROXY", etc).
+type action struct {
+	Verb   string               // Verb identifying the action ("GET", "POST", "WATCH", PROXY", etc).
+	Path   string               // The path of the action
+	Params []*restful.Parameter // List of parameters associated with the action.
+}
+
+// Installs handlers for API resources.
+func (a *APIInstaller) Install() (ws *restful.WebService, errors []error) {
+	errors = make([]error, 0)
+
+	// Create the WebService.
+	ws = a.newWebService()
+
+	// Initialize the custom handlers.
+	watchHandler := (&WatchHandler{
+		storage:         a.restHandler.storage,
+		codec:           a.restHandler.codec,
+		canonicalPrefix: a.restHandler.canonicalPrefix,
+		selfLinker:      a.restHandler.selfLinker,
+	})
+	redirectHandler := (&RedirectHandler{a.restHandler.storage, a.restHandler.codec})
+	proxyHandler := (&ProxyHandler{a.prefix + "/proxy/", a.restHandler.storage, a.restHandler.codec})
+
+	for path, storage := range a.restHandler.storage {
+		if err := a.registerResourceHandlers(path, storage, ws, watchHandler, redirectHandler, proxyHandler); err != nil {
+			errors = append(errors, err)
+		}
+	}
+	return ws, errors
+}
+
+func (a *APIInstaller) newWebService() *restful.WebService {
+	ws := new(restful.WebService)
+	ws.Path(a.prefix)
+	ws.Doc("API at " + a.prefix + " version " + a.version)
+	// TODO: change to restful.MIME_JSON when we set content type in client
+	ws.Consumes("*/*")
+	ws.Produces(restful.MIME_JSON)
+	ws.ApiVersion(a.version)
+	return ws
+}
+
+func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage, ws *restful.WebService, watchHandler http.Handler, redirectHandler http.Handler, proxyHandler http.Handler) error {
+
+	// Handler for standard REST verbs (GET, PUT, POST and DELETE).
+	restVerbHandler := restfulStripPrefix(a.prefix, a.restHandler)
+	object := storage.New()
+	// TODO: add scheme to APIInstaller rather than using api.Scheme
+	_, kind, err := api.Scheme.ObjectVersionAndKind(object)
+	if err != nil {
+		return err
+	}
+	versionedPtr, err := api.Scheme.New(a.version, kind)
+	if err != nil {
+		return err
+	}
+	versionedObject := indirectArbitraryPointer(versionedPtr)
+
+	var versionedList interface{}
+	if lister, ok := storage.(RESTLister); ok {
+		list := lister.NewList()
+		_, listKind, err := api.Scheme.ObjectVersionAndKind(list)
+		versionedListPtr, err := api.Scheme.New(a.version, listKind)
+		if err != nil {
+			return err
+		}
+		versionedList = indirectArbitraryPointer(versionedListPtr)
+	}
+
+	mapping, err := a.mapper.RESTMapping(kind, a.version)
+	if err != nil {
+		return err
+	}
+
+	// what verbs are supported by the storage, used to know what verbs we support per path
+	storageVerbs := map[string]bool{}
+	if _, ok := storage.(RESTCreater); ok {
+		// Handler for standard REST verbs (GET, PUT, POST and DELETE).
+		storageVerbs["RESTCreater"] = true
+	}
+	if _, ok := storage.(RESTLister); ok {
+		// Handler for standard REST verbs (GET, PUT, POST and DELETE).
+		storageVerbs["RESTLister"] = true
+	}
+	if _, ok := storage.(RESTGetter); ok {
+		storageVerbs["RESTGetter"] = true
+	}
+	if _, ok := storage.(RESTDeleter); ok {
+		storageVerbs["RESTDeleter"] = true
+	}
+	if _, ok := storage.(RESTUpdater); ok {
+		storageVerbs["RESTUpdater"] = true
+	}
+	if _, ok := storage.(ResourceWatcher); ok {
+		storageVerbs["ResourceWatcher"] = true
+	}
+	if _, ok := storage.(Redirector); ok {
+		storageVerbs["Redirector"] = true
+	}
+
+	allowWatchList := storageVerbs["ResourceWatcher"] && storageVerbs["RESTLister"] // watching on lists is allowed only for kinds that support both watch and list.
+	scope := mapping.Scope
+	nameParam := ws.PathParameter("name", "name of the "+kind).DataType("string")
+	params := []*restful.Parameter{}
+	actions := []action{}
+	// Get the list of actions for the given scope.
+	if scope.Name() != meta.RESTScopeNameNamespace {
+		// Handler for standard REST verbs (GET, PUT, POST and DELETE).
+		actions = appendIf(actions, action{"LIST", path, params}, storageVerbs["RESTLister"])
+		actions = appendIf(actions, action{"POST", path, params}, storageVerbs["RESTCreater"])
+		actions = appendIf(actions, action{"WATCHLIST", "/watch/" + path, params}, allowWatchList)
+
+		itemPath := path + "/{name}"
+		nameParams := append(params, nameParam)
+		actions = appendIf(actions, action{"GET", itemPath, nameParams}, storageVerbs["RESTGetter"])
+		actions = appendIf(actions, action{"PUT", itemPath, nameParams}, storageVerbs["RESTUpdater"])
+		actions = appendIf(actions, action{"DELETE", itemPath, nameParams}, storageVerbs["RESTDeleter"])
+		actions = appendIf(actions, action{"WATCH", "/watch/" + itemPath, nameParams}, storageVerbs["ResourceWatcher"])
+		actions = appendIf(actions, action{"REDIRECT", "/redirect/" + itemPath, nameParams}, storageVerbs["Redirector"])
+		actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath + "/{path:*}", nameParams}, storageVerbs["Redirector"])
+	} else {
+		// v1beta3 format with namespace in path
+		if scope.ParamPath() {
+			// Handler for standard REST verbs (GET, PUT, POST and DELETE).
+			namespaceParam := ws.PathParameter(scope.ParamName(), scope.ParamDescription()).DataType("string")
+			namespacedPath := scope.ParamName() + "/{" + scope.ParamName() + "}/" + path
+			namespaceParams := []*restful.Parameter{namespaceParam}
+			actions = appendIf(actions, action{"LIST", namespacedPath, namespaceParams}, storageVerbs["RESTLister"])
+			actions = appendIf(actions, action{"POST", namespacedPath, namespaceParams}, storageVerbs["RESTCreater"])
+			actions = appendIf(actions, action{"WATCHLIST", "/watch/" + namespacedPath, namespaceParams}, allowWatchList)
+
+			itemPath := namespacedPath + "/{name}"
+			nameParams := append(namespaceParams, nameParam)
+			actions = appendIf(actions, action{"GET", itemPath, nameParams}, storageVerbs["RESTGetter"])
+			actions = appendIf(actions, action{"PUT", itemPath, nameParams}, storageVerbs["RESTUpdater"])
+			actions = appendIf(actions, action{"DELETE", itemPath, nameParams}, storageVerbs["RESTDeleter"])
+			actions = appendIf(actions, action{"WATCH", "/watch/" + itemPath, nameParams}, storageVerbs["ResourceWatcher"])
+			actions = appendIf(actions, action{"REDIRECT", "/redirect/" + itemPath, nameParams}, storageVerbs["Redirector"])
+			actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath + "/{path:*}", nameParams}, storageVerbs["Redirector"])
+
+			// list across namespace.
+			actions = appendIf(actions, action{"LIST", path, params}, storageVerbs["RESTLister"])
+			actions = appendIf(actions, action{"WATCHLIST", "/watch/" + path, params}, allowWatchList)
+		} else {
+			// Handler for standard REST verbs (GET, PUT, POST and DELETE).
+			// v1beta1/v1beta2 format where namespace was a query parameter
+			namespaceParam := ws.QueryParameter(scope.ParamName(), scope.ParamDescription()).DataType("string")
+			namespaceParams := []*restful.Parameter{namespaceParam}
+			actions = appendIf(actions, action{"LIST", path, namespaceParams}, storageVerbs["RESTLister"])
+			actions = appendIf(actions, action{"POST", path, namespaceParams}, storageVerbs["RESTCreater"])
+			actions = appendIf(actions, action{"WATCHLIST", "/watch/" + path, namespaceParams}, allowWatchList)
+
+			itemPath := path + "/{name}"
+			nameParams := append(namespaceParams, nameParam)
+			actions = appendIf(actions, action{"GET", itemPath, nameParams}, storageVerbs["RESTGetter"])
+			actions = appendIf(actions, action{"PUT", itemPath, nameParams}, storageVerbs["RESTUpdater"])
+			actions = appendIf(actions, action{"DELETE", itemPath, nameParams}, storageVerbs["RESTDeleter"])
+			actions = appendIf(actions, action{"WATCH", "/watch/" + itemPath, nameParams}, storageVerbs["ResourceWatcher"])
+			actions = appendIf(actions, action{"REDIRECT", "/redirect/" + itemPath, nameParams}, storageVerbs["Redirector"])
+			actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath + "/{path:*}", nameParams}, storageVerbs["Redirector"])
+		}
+	}
+
+	// Create Routes for the actions.
+	// TODO: Add status documentation using Returns()
+	// Errors (see api/errors/errors.go as well as go-restful router):
+	// http.StatusNotFound, http.StatusMethodNotAllowed,
+	// http.StatusUnsupportedMediaType, http.StatusNotAcceptable,
+	// http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden,
+	// http.StatusRequestTimeout, http.StatusConflict, http.StatusPreconditionFailed,
+	// 422 (StatusUnprocessableEntity), http.StatusInternalServerError,
+	// http.StatusServiceUnavailable
+	// and api error codes
+	// Note that if we specify a versioned Status object here, we may need to
+	// create one for the tests, also
+	// Success:
+	// http.StatusOK, http.StatusCreated, http.StatusAccepted, http.StatusNoContent
+	//
+	// test/integration/auth_test.go is currently the most comprehensive status code test
+
+	for _, action := range actions {
+		switch action.Verb {
+		case "GET": // Get a resource.
+			route := ws.GET(action.Path).To(restVerbHandler).
+				Doc("read the specified " + kind).
+				Operation("read" + kind).
+				Writes(versionedObject)
+			addParams(route, action.Params)
+			ws.Route(route)
+		case "LIST": // List all resources of a kind.
+			route := ws.GET(action.Path).To(restVerbHandler).
+				Doc("list objects of kind " + kind).
+				Operation("list" + kind).
+				Writes(versionedList)
+			addParams(route, action.Params)
+			ws.Route(route)
+		case "PUT": // Update a resource.
+			route := ws.PUT(action.Path).To(restVerbHandler).
+				Doc("update the specified " + kind).
+				Operation("update" + kind).
+				Reads(versionedObject)
+			addParams(route, action.Params)
+			ws.Route(route)
+		case "POST": // Create a resource.
+			route := ws.POST(action.Path).To(restVerbHandler).
+				Doc("create a " + kind).
+				Operation("create" + kind).
+				Reads(versionedObject)
+			addParams(route, action.Params)
+			ws.Route(route)
+		case "DELETE": // Delete a resource.
+			route := ws.DELETE(action.Path).To(restVerbHandler).
+				Doc("delete a " + kind).
+				Operation("delete" + kind)
+			addParams(route, action.Params)
+			ws.Route(route)
+		case "WATCH": // Watch a resource.
+			route := ws.GET(action.Path).To(restfulStripPrefix(a.prefix+"/watch", watchHandler)).
+				Doc("watch a particular " + kind).
+				Operation("watch" + kind).
+				Writes(versionedObject)
+			addParams(route, action.Params)
+			ws.Route(route)
+		case "WATCHLIST": // Watch all resources of a kind.
+			route := ws.GET(action.Path).To(restfulStripPrefix(a.prefix+"/watch", watchHandler)).
+				Doc("watch a list of " + kind).
+				Operation("watch" + kind + "list").
+				Writes(versionedList)
+			addParams(route, action.Params)
+			ws.Route(route)
+		case "REDIRECT": // Get the redirect URL for a resource.
+			route := ws.GET(action.Path).To(restfulStripPrefix(a.prefix+"/redirect", redirectHandler)).
+				Doc("redirect GET request to " + kind).
+				Operation("redirect" + kind).
+				Produces("*/*").
+				Consumes("*/*")
+			addParams(route, action.Params)
+			ws.Route(route)
+		case "PROXY": // Proxy requests to a resource.
+			// Accept all methods as per https://github.com/GoogleCloudPlatform/kubernetes/issues/3996
+			addProxyRoute(ws, "GET", a.prefix, action.Path, proxyHandler, kind, action.Params)
+			addProxyRoute(ws, "PUT", a.prefix, action.Path, proxyHandler, kind, action.Params)
+			addProxyRoute(ws, "POST", a.prefix, action.Path, proxyHandler, kind, action.Params)
+			addProxyRoute(ws, "DELETE", a.prefix, action.Path, proxyHandler, kind, action.Params)
+		}
+		// Note: update GetAttribs() when adding a custom handler.
+	}
+	return nil
+}
+
+// This magic incantation returns *ptrToObject for an arbitrary pointer
+func indirectArbitraryPointer(ptrToObject interface{}) interface{} {
+	return reflect.Indirect(reflect.ValueOf(ptrToObject)).Interface()
+}
+
+func appendIf(actions []action, a action, shouldAppend bool) []action {
+	if shouldAppend {
+		actions = append(actions, a)
+	}
+	return actions
+}
+
+// Returns a restful RouteFunction that calls the given handler after stripping prefix from the request path.
+func restfulStripPrefix(prefix string, handler http.Handler) restful.RouteFunction {
+	return func(restReq *restful.Request, restResp *restful.Response) {
+		http.StripPrefix(prefix, handler).ServeHTTP(restResp.ResponseWriter, restReq.Request)
+	}
+}
+
+func addProxyRoute(ws *restful.WebService, method string, prefix string, path string, proxyHandler http.Handler, kind string, params []*restful.Parameter) {
+	proxyRoute := ws.Method(method).Path(path).To(restfulStripPrefix(prefix+"/proxy", proxyHandler)).
+		Doc("proxy " + method + " requests to " + kind).
+		Operation("proxy" + method + kind).
+		Produces("*/*").
+		Consumes("*/*")
+	addParams(proxyRoute, params)
+	ws.Route(proxyRoute)
+}
+
+func addParams(route *restful.RouteBuilder, params []*restful.Parameter) {
+	for _, param := range params {
+		route.Param(param)
+	}
+}

pkg/apiserver/apiserver.go

@@ -23,7 +23,6 @@ import (
 	"io/ioutil"
 	"net/http"
 	"path"
-	"reflect"
 	"strings"
 	"time"
 
@@ -60,8 +59,9 @@ func Handle(storage map[string]RESTStorage, codec runtime.Codec, root string, ve
 	prefix := root + "/" + version
 	group := NewAPIGroupVersion(storage, codec, prefix, selfLinker, admissionControl, mapper)
 	container := restful.NewContainer()
+	container.Router(restful.CurlyRouter{})
 	mux := container.ServeMux
-	group.InstallREST(container, mux, root, version)
+	group.InstallREST(container, root, version)
 	ws := new(restful.WebService)
 	InstallSupport(mux, ws)
 	container.Add(ws)
@@ -99,267 +99,13 @@ func NewAPIGroupVersion(storage map[string]RESTStorage, codec runtime.Codec, can
 	}
 }
 
-// This magic incantation returns *ptrToObject for an arbitrary pointer
-func indirectArbitraryPointer(ptrToObject interface{}) interface{} {
-	return reflect.Indirect(reflect.ValueOf(ptrToObject)).Interface()
-}
-
-func registerResourceHandlers(ws *restful.WebService, version string, path string, storage RESTStorage, h restful.RouteFunction, mapper meta.RESTMapper) error {
-	object := storage.New()
-	_, kind, err := api.Scheme.ObjectVersionAndKind(object)
-	if err != nil {
-		return err
-	}
-	versionedPtr, err := api.Scheme.New(version, kind)
-	if err != nil {
-		return err
-	}
-	versionedObject := indirectArbitraryPointer(versionedPtr)
-
-	var versionedList interface{}
-	if lister, ok := storage.(RESTLister); ok {
-		list := lister.NewList()
-		_, listKind, err := api.Scheme.ObjectVersionAndKind(list)
-		versionedListPtr, err := api.Scheme.New(version, listKind)
-		if err != nil {
-			return err
-		}
-		versionedList = indirectArbitraryPointer(versionedListPtr)
-	}
-
-	mapping, err := mapper.RESTMapping(kind, version)
-	if err != nil {
-		return err
-	}
-
-	// names are always in path
-	nameParam := ws.PathParameter("name", "name of the "+kind).DataType("string")
-
-	// what verbs are supported by the storage, used to know what verbs we support per path
-	storageVerbs := map[string]bool{}
-	if _, ok := storage.(RESTCreater); ok {
-		storageVerbs["RESTCreater"] = true
-	}
-	if _, ok := storage.(RESTLister); ok {
-		storageVerbs["RESTLister"] = true
-	}
-	if _, ok := storage.(RESTGetter); ok {
-		storageVerbs["RESTGetter"] = true
-	}
-	if _, ok := storage.(RESTDeleter); ok {
-		storageVerbs["RESTDeleter"] = true
-	}
-	if _, ok := storage.(RESTUpdater); ok {
-		storageVerbs["RESTUpdater"] = true
-	}
-
-	// path to verbs takes a path, and set of http verbs we should claim to support on this path
-	// given current url formats, this is scope specific
-	// for example, in v1beta3 url styles we would support the following:
-	// /ns/{namespace}/{resource} = []{POST, GET}  // list resources in this namespace scope
-	// /ns/{namespace}/{resource}/{name} = []{GET, PUT, DELETE} // handle an individual resource
-	// /{resource} = []{GET} // list resources across all namespaces
-	pathToVerbs := map[string][]string{}
-	// similar to the above, it maps the ordered set of parameters that need to be documented
-	pathToParam := map[string][]*restful.Parameter{}
-	// pathToStorageVerb lets us distinguish between RESTLister and RESTGetter on verb=GET
-	pathToStorageVerb := map[string]string{}
-	scope := mapping.Scope
-	if scope.Name() != meta.RESTScopeNameNamespace {
-		// non-namespaced resources support a smaller set of resource paths
-		resourcesPath := path
-		resourcesPathVerbs := []string{}
-		resourcesPathVerbs = appendStringIf(resourcesPathVerbs, "POST", storageVerbs["RESTCreater"])
-		resourcesPathVerbs = appendStringIf(resourcesPathVerbs, "GET", storageVerbs["RESTLister"])
-		pathToVerbs[resourcesPath] = resourcesPathVerbs
-		pathToParam[resourcesPath] = []*restful.Parameter{}
-		pathToStorageVerb[resourcesPath] = "RESTLister"
-
-		itemPath := resourcesPath + "/{name}"
-		itemPathVerbs := []string{}
-		itemPathVerbs = appendStringIf(itemPathVerbs, "GET", storageVerbs["RESTGetter"])
-		itemPathVerbs = appendStringIf(itemPathVerbs, "PUT", storageVerbs["RESTUpdater"])
-		itemPathVerbs = appendStringIf(itemPathVerbs, "DELETE", storageVerbs["RESTDeleter"])
-		pathToVerbs[itemPath] = itemPathVerbs
-		pathToParam[itemPath] = []*restful.Parameter{nameParam}
-		pathToStorageVerb[itemPath] = "RESTGetter"
-
-	} else {
-		// v1beta3 format with namespace in path
-		if scope.ParamPath() {
-			namespaceParam := ws.PathParameter(scope.ParamName(), scope.ParamDescription()).DataType("string")
-
-			resourceByNamespace := scope.ParamName() + "/{" + scope.ParamName() + "}/" + path
-			resourceByNamespaceVerbs := []string{}
-			resourceByNamespaceVerbs = appendStringIf(resourceByNamespaceVerbs, "POST", storageVerbs["RESTCreater"])
-			resourceByNamespaceVerbs = appendStringIf(resourceByNamespaceVerbs, "GET", storageVerbs["RESTLister"])
-			pathToVerbs[resourceByNamespace] = resourceByNamespaceVerbs
-			pathToParam[resourceByNamespace] = []*restful.Parameter{namespaceParam}
-			pathToStorageVerb[resourceByNamespace] = "RESTLister"
-
-			itemPath := resourceByNamespace + "/{name}"
-			itemPathVerbs := []string{}
-			itemPathVerbs = appendStringIf(itemPathVerbs, "GET", storageVerbs["RESTGetter"])
-			itemPathVerbs = appendStringIf(itemPathVerbs, "PUT", storageVerbs["RESTUpdater"])
-			itemPathVerbs = appendStringIf(itemPathVerbs, "DELETE", storageVerbs["RESTDeleter"])
-			pathToVerbs[itemPath] = itemPathVerbs
-			pathToParam[itemPath] = []*restful.Parameter{namespaceParam, nameParam}
-			pathToStorageVerb[itemPath] = "RESTGetter"
-
-			listAcrossNamespace := path
-			listAcrossNamespaceVerbs := []string{}
-			listAcrossNamespaceVerbs = appendStringIf(listAcrossNamespaceVerbs, "GET", storageVerbs["RESTLister"])
-			pathToVerbs[listAcrossNamespace] = listAcrossNamespaceVerbs
-			pathToParam[listAcrossNamespace] = []*restful.Parameter{}
-			pathToStorageVerb[listAcrossNamespace] = "RESTLister"
-
-		} else {
-			// v1beta1/v1beta2 format where namespace was a query parameter
-			namespaceParam := ws.QueryParameter(scope.ParamName(), scope.ParamDescription()).DataType("string")
-
-			resourcesPath := path
-			resourcesPathVerbs := []string{}
-			resourcesPathVerbs = appendStringIf(resourcesPathVerbs, "POST", storageVerbs["RESTCreater"])
-			resourcesPathVerbs = appendStringIf(resourcesPathVerbs, "GET", storageVerbs["RESTLister"])
-			pathToVerbs[resourcesPath] = resourcesPathVerbs
-			pathToParam[resourcesPath] = []*restful.Parameter{namespaceParam}
-			pathToStorageVerb[resourcesPath] = "RESTLister"
-
-			itemPath := resourcesPath + "/{name}"
-			itemPathVerbs := []string{}
-			itemPathVerbs = appendStringIf(itemPathVerbs, "GET", storageVerbs["RESTGetter"])
-			itemPathVerbs = appendStringIf(itemPathVerbs, "PUT", storageVerbs["RESTUpdater"])
-			itemPathVerbs = appendStringIf(itemPathVerbs, "DELETE", storageVerbs["RESTDeleter"])
-			pathToVerbs[itemPath] = itemPathVerbs
-			pathToParam[itemPath] = []*restful.Parameter{namespaceParam, nameParam}
-			pathToStorageVerb[itemPath] = "RESTGetter"
-		}
-	}
-
-	// See github.com/emicklei/go-restful/blob/master/jsr311.go for routing logic
-	// and status-code behavior
-	for path, verbs := range pathToVerbs {
-
-		params := pathToParam[path]
-		for _, verb := range verbs {
-			var route *restful.RouteBuilder
-			switch verb {
-			case "POST":
-				route = ws.POST(path).To(h).
-					Doc("create a " + kind).
-					Operation("create" + kind).Reads(versionedObject)
-			case "PUT":
-				route = ws.PUT(path).To(h).
-					Doc("update the specified " + kind).
-					Operation("update" + kind).Reads(versionedObject)
-			case "DELETE":
-				route = ws.DELETE(path).To(h).
-					Doc("delete the specified " + kind).
-					Operation("delete" + kind)
-			case "GET":
-				doc := "read the specified " + kind
-				op := "read" + kind
-				if pathToStorageVerb[path] == "RESTLister" {
-					doc = "list objects of kind " + kind
-					op = "list" + kind
-				}
-				route = ws.GET(path).To(h).
-					Doc(doc).
-					Operation(op)
-				if pathToStorageVerb[path] == "RESTLister" {
-					route = route.Returns(http.StatusOK, "OK", versionedList)
-				} else {
-					route = route.Writes(versionedObject)
-				}
-			}
-			for paramIndex := range params {
-				route.Param(params[paramIndex])
-			}
-			ws.Route(route)
-		}
-	}
-	return nil
-}
-
-// appendStringIf appends the value to the slice if shouldAdd is true
-func appendStringIf(slice []string, value string, shouldAdd bool) []string {
-	if shouldAdd {
-		return append(slice, value)
-	}
-	return slice
-}
-
 // InstallREST registers the REST handlers (storage, watch, proxy and redirect) into a restful Container.
 // It is expected that the provided path root prefix will serve all operations. Root MUST NOT end
 // in a slash. A restful WebService is created for the group and version.
-func (g *APIGroupVersion) InstallREST(container *restful.Container, mux Mux, root string, version string) error {
+func (g *APIGroupVersion) InstallREST(container *restful.Container, root string, version string) error {
 	prefix := path.Join(root, version)
-	restHandler := &g.handler
-	strippedHandler := http.StripPrefix(prefix, restHandler)
-	watchHandler := &WatchHandler{
-		storage:         g.handler.storage,
-		codec:           g.handler.codec,
-		canonicalPrefix: g.handler.canonicalPrefix,
-		selfLinker:      g.handler.selfLinker,
-	}
-	proxyHandler := &ProxyHandler{prefix + "/proxy/", g.handler.storage, g.handler.codec}
-	redirectHandler := &RedirectHandler{g.handler.storage, g.handler.codec}
-
-	// Create a new WebService for this APIGroupVersion at the specified path prefix
-	// TODO: Pass in more descriptive documentation
-	ws := new(restful.WebService)
-	ws.Path(prefix)
-	ws.Doc("API at " + root + ", version " + version)
-	// TODO: change to restful.MIME_JSON when we convert YAML->JSON and set content type in client
-	ws.Consumes("*/*")
-	ws.Produces(restful.MIME_JSON)
-	// TODO: require json on input
-	//ws.Consumes(restful.MIME_JSON)
-	ws.ApiVersion(version)
-
-	// TODO: add scheme to APIGroupVersion rather than using api.Scheme
-
-	// TODO: #2057: Return API resources on "/".
-
-	// TODO: Add status documentation using Returns()
-	// Errors (see api/errors/errors.go as well as go-restful router):
-	// http.StatusNotFound, http.StatusMethodNotAllowed,
-	// http.StatusUnsupportedMediaType, http.StatusNotAcceptable,
-	// http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden,
-	// http.StatusRequestTimeout, http.StatusConflict, http.StatusPreconditionFailed,
-	// 422 (StatusUnprocessableEntity), http.StatusInternalServerError,
-	// http.StatusServiceUnavailable
-	// and api error codes
-	// Note that if we specify a versioned Status object here, we may need to
-	// create one for the tests, also
-	// Success:
-	// http.StatusOK, http.StatusCreated, http.StatusAccepted, http.StatusNoContent
-	//
-	// test/integration/auth_test.go is currently the most comprehensive status code test
-
-	// TODO: eliminate all the restful wrappers
-	// TODO: create a separate handler per verb
-	h := func(req *restful.Request, resp *restful.Response) {
-		strippedHandler.ServeHTTP(resp.ResponseWriter, req.Request)
-	}
-
-	registrationErrors := make([]error, 0)
-
-	for path, storage := range g.handler.storage {
-		if err := registerResourceHandlers(ws, version, path, storage, h, g.mapper); err != nil {
-			registrationErrors = append(registrationErrors, err)
-		}
-	}
-
-	// TODO: port the rest of these. Sadly, if we don't, we'll have inconsistent
-	// API behavior, as well as lack of documentation
-	// Note: update GetAttribs() when adding a handler.
-	mux.Handle(prefix+"/watch/", http.StripPrefix(prefix+"/watch/", watchHandler))
-	mux.Handle(prefix+"/proxy/", http.StripPrefix(prefix+"/proxy/", proxyHandler))
-	mux.Handle(prefix+"/redirect/", http.StripPrefix(prefix+"/redirect/", redirectHandler))
-
+	ws, registrationErrors := (&APIInstaller{prefix, version, &g.handler, g.mapper}).Install()
 	container.Add(ws)
-
 	return errors.NewAggregate(registrationErrors)
 }
 

pkg/apiserver/apiserver_test.go

@@ -55,7 +55,7 @@ var codec = runtime.CodecFor(api.Scheme, testVersion)
 var accessor = meta.NewAccessor()
 var versioner runtime.ResourceVersioner = accessor
 var selfLinker runtime.SelfLinker = accessor
-var mapper meta.RESTMapper
+var mapper, namespaceMapper, legacyNamespaceMapper meta.RESTMapper // The mappers with namespace and with legacy namespace scopes.
 var admissionControl admission.Interface
 
 func interfacesFor(version string) (*meta.VersionInterfaces, error) {
@@ -71,6 +71,19 @@ func interfacesFor(version string) (*meta.VersionInterfaces, error) {
 	}
 }
 
+func newMapper() *meta.DefaultRESTMapper {
+	return meta.NewDefaultRESTMapper(
+		versions,
+		func(version string) (*meta.VersionInterfaces, bool) {
+			interfaces, err := interfacesFor(version)
+			if err != nil {
+				return nil, false
+			}
+			return interfaces, true
+		},
+	)
+}
+
 func init() {
 	// Certain API objects are returned regardless of the contents of storage:
 	// api.Status is returned in errors
@@ -83,26 +96,20 @@ func init() {
 	api.Scheme.AddKnownTypes(testVersion, &Simple{}, &SimpleList{},
 		&api.Status{})
 
-	defMapper := meta.NewDefaultRESTMapper(
-		versions,
-		func(version string) (*meta.VersionInterfaces, bool) {
-			interfaces, err := interfacesFor(version)
-			if err != nil {
-				return nil, false
-			}
-			return interfaces, true
-		},
-	)
+	nsMapper := newMapper()
+	legacyNsMapper := newMapper()
 	// enumerate all supported versions, get the kinds, and register with the mapper how to address our resources
 	for _, version := range versions {
 		for kind := range api.Scheme.KnownTypes(version) {
 			mixedCase := true
-			scope := meta.RESTScopeNamespaceLegacy
-			defMapper.Add(scope, kind, version, mixedCase)
+			legacyNsMapper.Add(meta.RESTScopeNamespaceLegacy, kind, version, mixedCase)
+			nsMapper.Add(meta.RESTScopeNamespace, kind, version, mixedCase)
 		}
 	}
 
-	mapper = defMapper
+	mapper = legacyNsMapper
+	legacyNamespaceMapper = legacyNsMapper
+	namespaceMapper = nsMapper
 	admissionControl = admit.NewAlwaysAdmit()
 }
 
@@ -274,7 +281,7 @@ func TestNotFound(t *testing.T) {
 		"PUT without extra segment":    {"PUT", "/prefix/version/foo", http.StatusMethodNotAllowed},
 		"PUT with extra segment":       {"PUT", "/prefix/version/foo/bar/baz", http.StatusNotFound},
 		"watch missing storage":        {"GET", "/prefix/version/watch/", http.StatusNotFound},
-		"watch with bad method":        {"POST", "/prefix/version/watch/foo/bar", http.StatusNotFound},
+		"watch with bad method":        {"POST", "/prefix/version/watch/foo/bar", http.StatusMethodNotAllowed},
 	}
 	handler := Handle(map[string]RESTStorage{
 		"foo": &SimpleRESTStorage{},
@@ -318,15 +325,15 @@ func TestUnimplementedRESTStorage(t *testing.T) {
 		ErrCode int
 	}
 	cases := map[string]T{
-		"GET object":    {"GET", "/prefix/version/foo/bar", http.StatusNotFound},
-		"GET list":      {"GET", "/prefix/version/foo", http.StatusNotFound},
-		"POST list":     {"POST", "/prefix/version/foo", http.StatusNotFound},
-		"PUT object":    {"PUT", "/prefix/version/foo/bar", http.StatusNotFound},
-		"DELETE object": {"DELETE", "/prefix/version/foo/bar", http.StatusNotFound},
-		//"watch list":      {"GET", "/prefix/version/watch/foo"},
-		//"watch object":    {"GET", "/prefix/version/watch/foo/bar"},
-		"proxy object":    {"GET", "/prefix/version/proxy/foo/bar", http.StatusMethodNotAllowed},
-		"redirect object": {"GET", "/prefix/version/redirect/foo/bar", http.StatusMethodNotAllowed},
+		"GET object":      {"GET", "/prefix/version/foo/bar", http.StatusNotFound},
+		"GET list":        {"GET", "/prefix/version/foo", http.StatusNotFound},
+		"POST list":       {"POST", "/prefix/version/foo", http.StatusNotFound},
+		"PUT object":      {"PUT", "/prefix/version/foo/bar", http.StatusNotFound},
+		"DELETE object":   {"DELETE", "/prefix/version/foo/bar", http.StatusNotFound},
+		"watch list":      {"GET", "/prefix/version/watch/foo", http.StatusNotFound},
+		"watch object":    {"GET", "/prefix/version/watch/foo/bar", http.StatusNotFound},
+		"proxy object":    {"GET", "/prefix/version/proxy/foo/bar", http.StatusNotFound},
+		"redirect object": {"GET", "/prefix/version/redirect/foo/bar", http.StatusNotFound},
 	}
 	handler := Handle(map[string]RESTStorage{
 		"foo": UnimplementedRESTStorage{},
@@ -347,9 +354,8 @@ func TestUnimplementedRESTStorage(t *testing.T) {
 		}
 		defer response.Body.Close()
 		data, _ := ioutil.ReadAll(response.Body)
-		t.Logf("resp: %s", string(data))
 		if response.StatusCode != v.ErrCode {
-			t.Errorf("%s: expected %d for %s, Got %s", k, http.StatusNotFound, v.Method, string(data))
+			t.Errorf("%s: expected %d for %s, Got %s", k, v.ErrCode, v.Method, string(data))
 			continue
 		}
 	}

pkg/apiserver/proxy_test.go

@@ -236,18 +236,30 @@ func TestProxy(t *testing.T) {
 			resourceLocation:          proxyServer.URL,
 			expectedResourceNamespace: item.reqNamespace,
 		}
-		handler := Handle(map[string]RESTStorage{
+
+		namespaceHandler := Handle(map[string]RESTStorage{
 			"foo": simpleStorage,
-		}, codec, "/prefix", "version", selfLinker, admissionControl, mapper)
-		server := httptest.NewServer(handler)
-		defer server.Close()
+		}, codec, "/prefix", "version", selfLinker, admissionControl, namespaceMapper)
+		namespaceServer := httptest.NewServer(namespaceHandler)
+		defer namespaceServer.Close()
+		legacyNamespaceHandler := Handle(map[string]RESTStorage{
+			"foo": simpleStorage,
+		}, codec, "/prefix", "version", selfLinker, admissionControl, legacyNamespaceMapper)
+		legacyNamespaceServer := httptest.NewServer(legacyNamespaceHandler)
+		defer legacyNamespaceServer.Close()
 
 		// test each supported URL pattern for finding the redirection resource in the proxy in a particular namespace
-		proxyTestPatterns := []string{
-			"/prefix/version/proxy/foo/id" + item.path + "?namespace=" + item.reqNamespace,
-			"/prefix/version/proxy/ns/" + item.reqNamespace + "/foo/id" + item.path,
+		serverPatterns := []struct {
+			server           *httptest.Server
+			proxyTestPattern string
+		}{
+			{namespaceServer, "/prefix/version/proxy/ns/" + item.reqNamespace + "/foo/id" + item.path},
+			{legacyNamespaceServer, "/prefix/version/proxy/foo/id" + item.path + "?namespace=" + item.reqNamespace},
 		}
-		for _, proxyTestPattern := range proxyTestPatterns {
+
+		for _, serverPattern := range serverPatterns {
+			server := serverPattern.server
+			proxyTestPattern := serverPattern.proxyTestPattern
 			req, err := http.NewRequest(
 				item.method,
 				server.URL+proxyTestPattern,
@@ -268,7 +280,7 @@ func TestProxy(t *testing.T) {
 			}
 			resp.Body.Close()
 			if e, a := item.respBody, string(gotResp); e != a {
-				t.Errorf("%v - expected %v, got %v", item.method, e, a)
+				t.Errorf("%v - expected %v, got %v. url: %#v", item.method, e, a, req.URL)
 			}
 		}
 	}

pkg/apiserver/redirect_test.go

@@ -84,7 +84,7 @@ func TestRedirectWithNamespaces(t *testing.T) {
 	}
 	handler := Handle(map[string]RESTStorage{
 		"foo": simpleStorage,
-	}, codec, "/prefix", "version", selfLinker, admissionControl, mapper)
+	}, codec, "/prefix", "version", selfLinker, admissionControl, namespaceMapper)
 	server := httptest.NewServer(handler)
 	defer server.Close()
 

pkg/master/master.go

@@ -317,6 +317,8 @@ func New(c *Config) *Master {
 		m.mux = mux
 		m.handlerContainer = NewHandlerContainer(mux)
 	}
+	// Use CurlyRouter to be able to use regular expressions in paths. Regular expressions are required in paths for example for proxy (where the path is proxy/{kind}/{name}/{*})
+	m.handlerContainer.Router(restful.CurlyRouter{})
 	m.muxHelper = &apiserver.MuxHelper{m.mux, []string{}}
 
 	m.masterServices = util.NewRunner(m.serviceWriterLoop, m.roServiceWriterLoop)
@@ -401,14 +403,14 @@ func (m *Master) init(c *Config) {
 	}
 
 	apiVersions := []string{"v1beta1", "v1beta2"}
-	if err := apiserver.NewAPIGroupVersion(m.api_v1beta1()).InstallREST(m.handlerContainer, m.muxHelper, c.APIPrefix, "v1beta1"); err != nil {
+	if err := apiserver.NewAPIGroupVersion(m.api_v1beta1()).InstallREST(m.handlerContainer, c.APIPrefix, "v1beta1"); err != nil {
 		glog.Fatalf("Unable to setup API v1beta1: %v", err)
 	}
-	if err := apiserver.NewAPIGroupVersion(m.api_v1beta2()).InstallREST(m.handlerContainer, m.muxHelper, c.APIPrefix, "v1beta2"); err != nil {
+	if err := apiserver.NewAPIGroupVersion(m.api_v1beta2()).InstallREST(m.handlerContainer, c.APIPrefix, "v1beta2"); err != nil {
 		glog.Fatalf("Unable to setup API v1beta2: %v", err)
 	}
 	if c.EnableV1Beta3 {
-		if err := apiserver.NewAPIGroupVersion(m.api_v1beta3()).InstallREST(m.handlerContainer, m.muxHelper, c.APIPrefix, "v1beta3"); err != nil {
+		if err := apiserver.NewAPIGroupVersion(m.api_v1beta3()).InstallREST(m.handlerContainer, c.APIPrefix, "v1beta3"); err != nil {
 			glog.Fatalf("Unable to setup API v1beta3: %v", err)
 		}
 		apiVersions = []string{"v1beta1", "v1beta2", "v1beta3"}

test/integration/auth_test.go

@@ -259,7 +259,6 @@ func getTestRequests() []struct {
 		{"DELETE", "/api/v1beta1/foo" + timeoutFlag, "", code404},
 
 		// Special verbs on nodes
-		// TODO: Will become 405 once these are converted to go-restful
 		{"GET", "/api/v1beta1/proxy/minions/a", "", code404},
 		{"GET", "/api/v1beta1/redirect/minions/a", "", code404},
 		// TODO: test .../watch/..., which doesn't end before the test timeout.