From 18609e17062ad4a56c3533f80014299815bfee76 Mon Sep 17 00:00:00 2001 From: nikhiljindal Date: Fri, 30 Jan 2015 16:08:59 -0800 Subject: [PATCH] Moving /watch, /proxy and /redirect registration to go-restful --- pkg/apiserver/api_installer.go | 320 ++++++++++++++++++++++++++++++++ pkg/apiserver/apiserver.go | 262 +------------------------- pkg/apiserver/apiserver_test.go | 58 +++--- pkg/apiserver/proxy_test.go | 30 ++- pkg/apiserver/redirect_test.go | 2 +- pkg/master/master.go | 8 +- test/integration/auth_test.go | 1 - 7 files changed, 383 insertions(+), 298 deletions(-) create mode 100644 pkg/apiserver/api_installer.go diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go new file mode 100644 index 0000000000..4c9bb1ac23 --- /dev/null +++ b/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) + } +} diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 85b8c33eb0..7d1f572a8c 100644 --- a/pkg/apiserver/apiserver.go +++ b/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) } diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 806852886a..a04a57a81e 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/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 } } diff --git a/pkg/apiserver/proxy_test.go b/pkg/apiserver/proxy_test.go index 0113f86e8c..22fc494fcd 100644 --- a/pkg/apiserver/proxy_test.go +++ b/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) } } } diff --git a/pkg/apiserver/redirect_test.go b/pkg/apiserver/redirect_test.go index 2ec95f2a58..a0a1c4cf2f 100644 --- a/pkg/apiserver/redirect_test.go +++ b/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() diff --git a/pkg/master/master.go b/pkg/master/master.go index b84afd77aa..fabc8dee29 100644 --- a/pkg/master/master.go +++ b/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"} diff --git a/test/integration/auth_test.go b/test/integration/auth_test.go index 838d24dd61..1043de8e53 100644 --- a/test/integration/auth_test.go +++ b/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.