From 4b16a870969ba8809275ac279d9785ae82a817dd Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 4 Mar 2015 15:57:05 -0500 Subject: [PATCH] Simplify api_installer and setup methods --- pkg/apiserver/api_installer.go | 118 ++++++------- pkg/apiserver/apiserver.go | 75 +++----- pkg/apiserver/apiserver_test.go | 293 +++++++++++++++++++++----------- pkg/apiserver/proxy_test.go | 18 +- pkg/apiserver/redirect_test.go | 12 +- pkg/apiserver/resthandler.go | 29 ++-- pkg/apiserver/watch_test.go | 16 +- pkg/master/master.go | 47 +++-- pkg/runtime/interfaces.go | 5 + 9 files changed, 349 insertions(+), 264 deletions(-) diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go index 72d25c2b8e2..c2e15b95043 100644 --- a/pkg/apiserver/api_installer.go +++ b/pkg/apiserver/api_installer.go @@ -26,16 +26,15 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta" - "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/emicklei/go-restful" ) type APIInstaller struct { - group *APIGroupVersion - prefix string // Path prefix where API resources are to be registered. - version string // The API version being installed. + group *APIGroupVersion + info *APIRequestInfoResolver + prefix string // Path prefix where API resources are to be registered. } // Struct capturing information about an action ("GET", "POST", "WATCH", PROXY", etc). @@ -58,15 +57,15 @@ func (a *APIInstaller) Install() (ws *restful.WebService, errors []error) { // Initialize the custom handlers. watchHandler := (&WatchHandler{ - storage: a.group.storage, - codec: a.group.codec, - linker: a.group.linker, - info: a.group.info, + storage: a.group.Storage, + codec: a.group.Codec, + linker: a.group.Linker, + info: a.info, }) - redirectHandler := (&RedirectHandler{a.group.storage, a.group.codec, a.group.context, a.group.info}) - proxyHandler := (&ProxyHandler{a.prefix + "/proxy/", a.group.storage, a.group.codec, a.group.context, a.group.info}) + redirectHandler := (&RedirectHandler{a.group.Storage, a.group.Codec, a.group.Context, a.info}) + proxyHandler := (&ProxyHandler{a.prefix + "/proxy/", a.group.Storage, a.group.Codec, a.group.Context, a.info}) - for path, storage := range a.group.storage { + for path, storage := range a.group.Storage { if err := a.registerResourceHandlers(path, storage, ws, watchHandler, redirectHandler, proxyHandler); err != nil { errors = append(errors, err) } @@ -77,18 +76,17 @@ func (a *APIInstaller) Install() (ws *restful.WebService, errors []error) { func (a *APIInstaller) newWebService() *restful.WebService { ws := new(restful.WebService) ws.Path(a.prefix) - ws.Doc("API at " + a.prefix + " version " + a.version) + ws.Doc("API at " + a.prefix + " version " + a.group.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) + ws.ApiVersion(a.group.Version) return ws } func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage, ws *restful.WebService, watchHandler, redirectHandler, proxyHandler http.Handler) error { - codec := a.group.codec - admit := a.group.admit - context := a.group.context + admit := a.group.Admit + context := a.group.Context var resource, subresource string switch parts := strings.Split(path, "/"); len(parts) { @@ -97,19 +95,16 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage case 1: resource = parts[0] default: + // TODO: support deeper paths return fmt.Errorf("api_installer allows only one or two segment paths (resource or resource/subresource)") } object := storage.New() - // TODO: add scheme to APIInstaller rather than using api.Scheme - _, kind, err := api.Scheme.ObjectVersionAndKind(object) + _, kind, err := a.group.Typer.ObjectVersionAndKind(object) if err != nil { return err } - versionedPtr, err := api.Scheme.New(a.version, kind) - if conversion.IsNotRegisteredError(err) { - return nil - } + versionedPtr, err := a.group.Creater.New(a.group.Version, kind) if err != nil { return err } @@ -118,15 +113,15 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage 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) + _, listKind, err := a.group.Typer.ObjectVersionAndKind(list) + versionedListPtr, err := a.group.Creater.New(a.group.Version, listKind) if err != nil { return err } versionedList = indirectArbitraryPointer(versionedListPtr) } - mapping, err := a.group.mapper.RESTMapping(kind, a.version) + mapping, err := a.group.Mapper.RESTMapping(kind, a.group.Version) if err != nil { return err } @@ -156,25 +151,27 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage // Get the list of actions for the given scope. if scope.Name() != meta.RESTScopeNameNamespace { - itemPath := resource + "/{name}" + resourcePath := resource + itemPath := resourcePath + "/{name}" if len(subresource) > 0 { itemPath = itemPath + "/" + subresource + resourcePath = itemPath } nameParams := append(params, nameParam) - namer := rootScopeNaming{scope, a.group.linker, gpath.Join(a.prefix, itemPath)} + namer := rootScopeNaming{scope, a.group.Linker, gpath.Join(a.prefix, itemPath)} // Handler for standard REST verbs (GET, PUT, POST and DELETE). - actions = appendIf(actions, action{"LIST", resource, params, namer}, isLister) - actions = appendIf(actions, action{"POST", resource, params, namer}, isCreater) - actions = appendIf(actions, action{"WATCHLIST", "/watch/" + resource, params, namer}, allowWatchList) + actions = appendIf(actions, action{"LIST", resourcePath, params, namer}, isLister) + actions = appendIf(actions, action{"POST", resourcePath, params, namer}, isCreater) + actions = appendIf(actions, action{"WATCHLIST", "watch/" + resourcePath, params, namer}, allowWatchList) actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, isGetter) actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, isUpdater) actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, isDeleter) - actions = appendIf(actions, action{"WATCH", "/watch/" + itemPath, nameParams, namer}, isWatcher) - actions = appendIf(actions, action{"REDIRECT", "/redirect/" + itemPath, nameParams, namer}, isRedirector) - actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath + "/{path:*}", nameParams, namer}, isRedirector) - actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath, nameParams, namer}, isRedirector) + actions = appendIf(actions, action{"WATCH", "watch/" + itemPath, nameParams, namer}, isWatcher) + actions = appendIf(actions, action{"REDIRECT", "redirect/" + itemPath, nameParams, namer}, isRedirector) + actions = appendIf(actions, action{"PROXY", "proxy/" + itemPath + "/{path:*}", nameParams, namer}, isRedirector) + actions = appendIf(actions, action{"PROXY", "proxy/" + itemPath, nameParams, namer}, isRedirector) } else { // v1beta3 format with namespace in path @@ -184,29 +181,31 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage namespacedPath := scope.ParamName() + "/{" + scope.ParamName() + "}/" + resource namespaceParams := []*restful.Parameter{namespaceParam} + resourcePath := namespacedPath itemPath := namespacedPath + "/{name}" if len(subresource) > 0 { itemPath = itemPath + "/" + subresource + resourcePath = itemPath } nameParams := append(namespaceParams, nameParam) - namer := scopeNaming{scope, a.group.linker, gpath.Join(a.prefix, itemPath), false} + namer := scopeNaming{scope, a.group.Linker, gpath.Join(a.prefix, itemPath), false} - actions = appendIf(actions, action{"LIST", namespacedPath, namespaceParams, namer}, isLister) - actions = appendIf(actions, action{"POST", namespacedPath, namespaceParams, namer}, isCreater) - actions = appendIf(actions, action{"WATCHLIST", "/watch/" + namespacedPath, namespaceParams, namer}, allowWatchList) + actions = appendIf(actions, action{"LIST", resourcePath, namespaceParams, namer}, isLister) + actions = appendIf(actions, action{"POST", resourcePath, namespaceParams, namer}, isCreater) + actions = appendIf(actions, action{"WATCHLIST", "watch/" + resourcePath, namespaceParams, namer}, allowWatchList) actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, isGetter) actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, isUpdater) actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, isDeleter) - actions = appendIf(actions, action{"WATCH", "/watch/" + itemPath, nameParams, namer}, isWatcher) - actions = appendIf(actions, action{"REDIRECT", "/redirect/" + itemPath, nameParams, namer}, isRedirector) - actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath + "/{path:*}", nameParams, namer}, isRedirector) - actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath, nameParams, namer}, isRedirector) + actions = appendIf(actions, action{"WATCH", "watch/" + itemPath, nameParams, namer}, isWatcher) + actions = appendIf(actions, action{"REDIRECT", "redirect/" + itemPath, nameParams, namer}, isRedirector) + actions = appendIf(actions, action{"PROXY", "proxy/" + itemPath + "/{path:*}", nameParams, namer}, isRedirector) + actions = appendIf(actions, action{"PROXY", "proxy/" + itemPath, nameParams, namer}, isRedirector) // list across namespace. - namer = scopeNaming{scope, a.group.linker, gpath.Join(a.prefix, itemPath), true} + namer = scopeNaming{scope, a.group.Linker, gpath.Join(a.prefix, itemPath), true} actions = appendIf(actions, action{"LIST", resource, params, namer}, isLister) - actions = appendIf(actions, action{"WATCHLIST", "/watch/" + resource, params, namer}, allowWatchList) + actions = appendIf(actions, action{"WATCHLIST", "watch/" + resource, params, namer}, allowWatchList) } else { // Handler for standard REST verbs (GET, PUT, POST and DELETE). @@ -214,24 +213,27 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage namespaceParam := ws.QueryParameter(scope.ParamName(), scope.ParamDescription()).DataType("string") namespaceParams := []*restful.Parameter{namespaceParam} - itemPath := resource + "/{name}" + basePath := resource + resourcePath := basePath + itemPath := resourcePath + "/{name}" if len(subresource) > 0 { itemPath = itemPath + "/" + subresource + resourcePath = itemPath } nameParams := append(namespaceParams, nameParam) - namer := legacyScopeNaming{scope, a.group.linker, gpath.Join(a.prefix, itemPath)} + namer := legacyScopeNaming{scope, a.group.Linker, gpath.Join(a.prefix, itemPath)} - actions = appendIf(actions, action{"LIST", resource, namespaceParams, namer}, isLister) - actions = appendIf(actions, action{"POST", resource, namespaceParams, namer}, isCreater) - actions = appendIf(actions, action{"WATCHLIST", "/watch/" + resource, namespaceParams, namer}, allowWatchList) + actions = appendIf(actions, action{"LIST", resourcePath, namespaceParams, namer}, isLister) + actions = appendIf(actions, action{"POST", resourcePath, namespaceParams, namer}, isCreater) + actions = appendIf(actions, action{"WATCHLIST", "watch/" + resourcePath, namespaceParams, namer}, allowWatchList) actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, isGetter) actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, isUpdater) actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, isDeleter) - actions = appendIf(actions, action{"WATCH", "/watch/" + itemPath, nameParams, namer}, isWatcher) - actions = appendIf(actions, action{"REDIRECT", "/redirect/" + itemPath, nameParams, namer}, isRedirector) - actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath + "/{path:*}", nameParams, namer}, isRedirector) - actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath, nameParams, namer}, isRedirector) + actions = appendIf(actions, action{"WATCH", "watch/" + itemPath, nameParams, namer}, isWatcher) + actions = appendIf(actions, action{"REDIRECT", "redirect/" + itemPath, nameParams, namer}, isRedirector) + actions = appendIf(actions, action{"PROXY", "proxy/" + itemPath + "/{path:*}", nameParams, namer}, isRedirector) + actions = appendIf(actions, action{"PROXY", "proxy/" + itemPath, nameParams, namer}, isRedirector) } } @@ -256,7 +258,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage m := monitorFilter(action.Verb, resource) switch action.Verb { case "GET": // Get a resource. - route := ws.GET(action.Path).To(GetResource(getter, ctxFn, action.Namer, codec)). + route := ws.GET(action.Path).To(GetResource(getter, ctxFn, action.Namer, mapping.Codec)). Filter(m). Doc("read the specified " + kind). Operation("read" + kind). @@ -264,7 +266,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage addParams(route, action.Params) ws.Route(route) case "LIST": // List all resources of a kind. - route := ws.GET(action.Path).To(ListResource(lister, ctxFn, action.Namer, codec, a.group.info)). + route := ws.GET(action.Path).To(ListResource(lister, ctxFn, action.Namer, mapping.Codec, a.group.Version, resource)). Filter(m). Doc("list objects of kind " + kind). Operation("list" + kind). @@ -272,7 +274,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage addParams(route, action.Params) ws.Route(route) case "PUT": // Update a resource. - route := ws.PUT(action.Path).To(UpdateResource(updater, ctxFn, action.Namer, codec, resource, admit)). + route := ws.PUT(action.Path).To(UpdateResource(updater, ctxFn, action.Namer, mapping.Codec, a.group.Typer, resource, admit)). Filter(m). Doc("replace the specified " + kind). Operation("replace" + kind). @@ -280,7 +282,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage addParams(route, action.Params) ws.Route(route) case "POST": // Create a resource. - route := ws.POST(action.Path).To(CreateResource(creater, ctxFn, action.Namer, codec, resource, admit)). + route := ws.POST(action.Path).To(CreateResource(creater, ctxFn, action.Namer, mapping.Codec, a.group.Typer, resource, admit)). Filter(m). Doc("create a " + kind). Operation("create" + kind). @@ -288,7 +290,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage addParams(route, action.Params) ws.Route(route) case "DELETE": // Delete a resource. - route := ws.DELETE(action.Path).To(DeleteResource(deleter, ctxFn, action.Namer, codec, resource, kind, admit)). + route := ws.DELETE(action.Path).To(DeleteResource(deleter, ctxFn, action.Namer, mapping.Codec, resource, kind, admit)). Filter(m). Doc("delete a " + kind). Operation("delete" + kind) diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 5dc15667360..ee3f78a5bcd 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -29,7 +29,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/admission" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta" "github.com/GoogleCloudPlatform/kubernetes/pkg/healthz" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" @@ -91,72 +90,38 @@ type Mux interface { HandleFunc(pattern string, handler func(http.ResponseWriter, *http.Request)) } -// defaultAPIServer exposes nested objects for testability. -type defaultAPIServer struct { - http.Handler - group *APIGroupVersion -} - -// Handle returns a Handler function that exposes the provided storage interfaces -// as RESTful resources at prefix, serialized by codec, and also includes the support -// http resources. -// Note: This method is used only in tests. -func Handle(storage map[string]RESTStorage, codec runtime.Codec, root string, version string, linker runtime.SelfLinker, admissionControl admission.Interface, contextMapper api.RequestContextMapper, mapper meta.RESTMapper) http.Handler { - prefix := path.Join(root, version) - group := NewAPIGroupVersion(storage, codec, root, prefix, linker, admissionControl, contextMapper, mapper) - container := restful.NewContainer() - container.Router(restful.CurlyRouter{}) - mux := container.ServeMux - group.InstallREST(container, root, version) - ws := new(restful.WebService) - InstallSupport(mux, ws) - container.Add(ws) - return &defaultAPIServer{mux, group} -} - // APIGroupVersion is a helper for exposing RESTStorage objects as http.Handlers via go-restful // It handles URLs of the form: // /${storage_key}[/${object_name}] // Where 'storage_key' points to a RESTStorage object stored in storage. type APIGroupVersion struct { - storage map[string]RESTStorage - codec runtime.Codec - prefix string - linker runtime.SelfLinker - admit admission.Interface - context api.RequestContextMapper - mapper meta.RESTMapper - // TODO: put me into a cleaner interface - info *APIRequestInfoResolver -} + Storage map[string]RESTStorage -// NewAPIGroupVersion returns an object that will serve a set of REST resources and their -// associated operations. The provided codec controls serialization and deserialization. -// This is a helper method for registering multiple sets of REST handlers under different -// prefixes onto a server. -// TODO: add multitype codec serialization -func NewAPIGroupVersion(storage map[string]RESTStorage, codec runtime.Codec, root, prefix string, linker runtime.SelfLinker, admissionControl admission.Interface, contextMapper api.RequestContextMapper, mapper meta.RESTMapper) *APIGroupVersion { - return &APIGroupVersion{ - storage: storage, - codec: codec, - prefix: prefix, - linker: linker, - admit: admissionControl, - context: contextMapper, - mapper: mapper, - info: &APIRequestInfoResolver{util.NewStringSet(strings.TrimPrefix(root, "/")), latest.RESTMapper}, - } + Root string + Version string + + Mapper meta.RESTMapper + + Codec runtime.Codec + Typer runtime.ObjectTyper + Creater runtime.ObjectCreater + Linker runtime.SelfLinker + + Admit admission.Interface + Context api.RequestContextMapper } // 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, root string, version string) error { - prefix := path.Join(root, version) +func (g *APIGroupVersion) InstallREST(container *restful.Container) error { + info := &APIRequestInfoResolver{util.NewStringSet(strings.TrimPrefix(g.Root, "/")), g.Mapper} + + prefix := path.Join(g.Root, g.Version) installer := &APIInstaller{ - group: g, - prefix: prefix, - version: version, + group: g, + info: info, + prefix: prefix, } ws, registrationErrors := installer.Install() container.Add(ws) diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index b94c3753334..07d50e3c8b3 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -41,6 +41,8 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/admission/admit" "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/admission/deny" + + "github.com/emicklei/go-restful" ) func convert(obj runtime.Object) (runtime.Object, error) { @@ -115,6 +117,59 @@ func init() { requestContextMapper = api.NewRequestContextMapper() } +// defaultAPIServer exposes nested objects for testability. +type defaultAPIServer struct { + http.Handler + group *APIGroupVersion +} + +// uses the default settings +func handle(storage map[string]RESTStorage) http.Handler { + return handleInternal(storage, admissionControl, mapper, selfLinker) +} + +// tests with a deny admission controller +func handleDeny(storage map[string]RESTStorage) http.Handler { + return handleInternal(storage, deny.NewAlwaysDeny(), mapper, selfLinker) +} + +// tests using the new namespace scope mechanism +func handleNamespaced(storage map[string]RESTStorage) http.Handler { + return handleInternal(storage, admissionControl, namespaceMapper, selfLinker) +} + +// tests using a custom self linker +func handleLinker(storage map[string]RESTStorage, selfLinker runtime.SelfLinker) http.Handler { + return handleInternal(storage, admissionControl, mapper, selfLinker) +} + +func handleInternal(storage map[string]RESTStorage, admissionControl admission.Interface, mapper meta.RESTMapper, selfLinker runtime.SelfLinker) http.Handler { + group := &APIGroupVersion{ + Storage: storage, + + Mapper: mapper, + + Root: "/api", + Version: testVersion, + + Creater: api.Scheme, + Typer: api.Scheme, + Codec: codec, + Linker: selfLinker, + + Admit: admissionControl, + Context: requestContextMapper, + } + container := restful.NewContainer() + container.Router(restful.CurlyRouter{}) + mux := container.ServeMux + group.InstallREST(container) + ws := new(restful.WebService) + InstallSupport(mux, ws) + container.Add(ws) + return &defaultAPIServer{mux, group} +} + type Simple struct { api.TypeMeta `json:",inline"` api.ObjectMeta `json:"metadata"` @@ -285,21 +340,21 @@ func TestNotFound(t *testing.T) { Status int } cases := map[string]T{ - "PATCH method": {"PATCH", "/prefix/version/foo", http.StatusMethodNotAllowed}, - "GET long prefix": {"GET", "/prefix/", http.StatusNotFound}, - "GET missing storage": {"GET", "/prefix/version/blah", http.StatusNotFound}, - "GET with extra segment": {"GET", "/prefix/version/foo/bar/baz", http.StatusNotFound}, - "POST with extra segment": {"POST", "/prefix/version/foo/bar", http.StatusMethodNotAllowed}, - "DELETE without extra segment": {"DELETE", "/prefix/version/foo", http.StatusMethodNotAllowed}, - "DELETE with extra segment": {"DELETE", "/prefix/version/foo/bar/baz", http.StatusNotFound}, - "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.StatusMethodNotAllowed}, + "PATCH method": {"PATCH", "/api/version/foo", http.StatusMethodNotAllowed}, + "GET long prefix": {"GET", "/api/", http.StatusNotFound}, + "GET missing storage": {"GET", "/api/version/blah", http.StatusNotFound}, + "GET with extra segment": {"GET", "/api/version/foo/bar/baz", http.StatusNotFound}, + "POST with extra segment": {"POST", "/api/version/foo/bar", http.StatusMethodNotAllowed}, + "DELETE without extra segment": {"DELETE", "/api/version/foo", http.StatusMethodNotAllowed}, + "DELETE with extra segment": {"DELETE", "/api/version/foo/bar/baz", http.StatusNotFound}, + "PUT without extra segment": {"PUT", "/api/version/foo", http.StatusMethodNotAllowed}, + "PUT with extra segment": {"PUT", "/api/version/foo/bar/baz", http.StatusNotFound}, + "watch missing storage": {"GET", "/api/version/watch/", http.StatusNotFound}, + "watch with bad method": {"POST", "/api/version/watch/foo/bar", http.StatusMethodNotAllowed}, } - handler := Handle(map[string]RESTStorage{ + handler := handle(map[string]RESTStorage{ "foo": &SimpleRESTStorage{}, - }, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + }) server := httptest.NewServer(handler) defer server.Close() client := http.Client{} @@ -339,19 +394,19 @@ 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", 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}, + "GET object": {"GET", "/api/version/foo/bar", http.StatusNotFound}, + "GET list": {"GET", "/api/version/foo", http.StatusNotFound}, + "POST list": {"POST", "/api/version/foo", http.StatusNotFound}, + "PUT object": {"PUT", "/api/version/foo/bar", http.StatusNotFound}, + "DELETE object": {"DELETE", "/api/version/foo/bar", http.StatusNotFound}, + "watch list": {"GET", "/api/version/watch/foo", http.StatusNotFound}, + "watch object": {"GET", "/api/version/watch/foo/bar", http.StatusNotFound}, + "proxy object": {"GET", "/api/version/proxy/foo/bar", http.StatusNotFound}, + "redirect object": {"GET", "/api/version/redirect/foo/bar", http.StatusNotFound}, } - handler := Handle(map[string]RESTStorage{ + handler := handle(map[string]RESTStorage{ "foo": UnimplementedRESTStorage{}, - }, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + }) server := httptest.NewServer(handler) defer server.Close() client := http.Client{} @@ -376,7 +431,7 @@ func TestUnimplementedRESTStorage(t *testing.T) { } func TestVersion(t *testing.T) { - handler := Handle(map[string]RESTStorage{}, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(map[string]RESTStorage{}) server := httptest.NewServer(handler) defer server.Close() client := http.Client{} @@ -409,14 +464,14 @@ func TestList(t *testing.T) { selfLink string legacy bool }{ - {"/prefix/version/simple", "", "/prefix/version/simple?namespace=", true}, - {"/prefix/version/simple?namespace=other", "other", "/prefix/version/simple?namespace=other", true}, + {"/api/version/simple", "", "/api/version/simple?namespace=", true}, + {"/api/version/simple?namespace=other", "other", "/api/version/simple?namespace=other", true}, // list items across all namespaces - {"/prefix/version/simple?namespace=", "", "/prefix/version/simple?namespace=", true}, - {"/prefix/version/namespaces/default/simple", "default", "/prefix/version/namespaces/default/simple", false}, - {"/prefix/version/namespaces/other/simple", "other", "/prefix/version/namespaces/other/simple", false}, + {"/api/version/simple?namespace=", "", "/api/version/simple?namespace=", true}, + {"/api/version/namespaces/default/simple", "default", "/api/version/namespaces/default/simple", false}, + {"/api/version/namespaces/other/simple", "other", "/api/version/namespaces/other/simple", false}, // list items across all namespaces - {"/prefix/version/simple", "", "/prefix/version/simple", false}, + {"/api/version/simple", "", "/api/version/simple", false}, } for i, testCase := range testCases { storage := map[string]RESTStorage{} @@ -429,9 +484,9 @@ func TestList(t *testing.T) { } var handler http.Handler if testCase.legacy { - handler = Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler = handleLinker(storage, selfLinker) } else { - handler = Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, namespaceMapper) + handler = handleInternal(storage, admissionControl, namespaceMapper, selfLinker) } server := httptest.NewServer(handler) defer server.Close() @@ -462,11 +517,11 @@ func TestErrorList(t *testing.T) { errors: map[string]error{"list": fmt.Errorf("test Error")}, } storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(storage) server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/prefix/version/simple") + resp, err := http.Get(server.URL + "/api/version/simple") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -487,11 +542,11 @@ func TestNonEmptyList(t *testing.T) { }, } storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(storage) server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/prefix/version/simple") + resp, err := http.Get(server.URL + "/api/version/simple") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -515,10 +570,10 @@ func TestNonEmptyList(t *testing.T) { if listOut.Items[0].Other != simpleStorage.list[0].Other { t.Errorf("Unexpected data: %#v, %s", listOut.Items[0], string(body)) } - if listOut.SelfLink != "/prefix/version/simple?namespace=" { + if listOut.SelfLink != "/api/version/simple?namespace=" { t.Errorf("unexpected list self link: %#v", listOut) } - expectedSelfLink := "/prefix/version/simple/something?namespace=other" + expectedSelfLink := "/api/version/simple/something?namespace=other" if listOut.Items[0].ObjectMeta.SelfLink != expectedSelfLink { t.Errorf("Unexpected data: %#v, %s", listOut.Items[0].ObjectMeta.SelfLink, expectedSelfLink) } @@ -535,11 +590,11 @@ func TestSelfLinkSkipsEmptyName(t *testing.T) { }, } storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(storage) server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/prefix/version/simple") + resp, err := http.Get(server.URL + "/api/version/simple") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -562,7 +617,7 @@ func TestSelfLinkSkipsEmptyName(t *testing.T) { if listOut.Items[0].Other != simpleStorage.list[0].Other { t.Errorf("Unexpected data: %#v, %s", listOut.Items[0], string(body)) } - if listOut.SelfLink != "/prefix/version/simple?namespace=" { + if listOut.SelfLink != "/api/version/simple?namespace=" { t.Errorf("unexpected list self link: %#v", listOut) } expectedSelfLink := "" @@ -580,16 +635,16 @@ func TestGet(t *testing.T) { } selfLinker := &setTestSelfLinker{ t: t, - expectedSet: "/prefix/version/simple/id?namespace=default", + expectedSet: "/api/version/simple/id?namespace=default", name: "id", namespace: "default", } storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handleLinker(storage, selfLinker) server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/prefix/version/simple/id") + resp, err := http.Get(server.URL + "/api/version/simple/id") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -619,16 +674,16 @@ func TestGetAlternateSelfLink(t *testing.T) { } selfLinker := &setTestSelfLinker{ t: t, - expectedSet: "/prefix/version/simple/id?namespace=test", + expectedSet: "/api/version/simple/id?namespace=test", name: "id", namespace: "test", } storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, legacyNamespaceMapper) + handler := handleLinker(storage, selfLinker) server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/prefix/version/simple/id?namespace=test") + resp, err := http.Get(server.URL + "/api/version/simple/id?namespace=test") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -657,16 +712,16 @@ func TestGetNamespaceSelfLink(t *testing.T) { } selfLinker := &setTestSelfLinker{ t: t, - expectedSet: "/prefix/version/namespaces/foo/simple/id", + expectedSet: "/api/version/namespaces/foo/simple/id", name: "id", namespace: "foo", } storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, namespaceMapper) + handler := handleInternal(storage, admissionControl, namespaceMapper, selfLinker) server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/prefix/version/namespaces/foo/simple/id") + resp, err := http.Get(server.URL + "/api/version/namespaces/foo/simple/id") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -691,11 +746,11 @@ func TestGetMissing(t *testing.T) { errors: map[string]error{"get": apierrs.NewNotFound("simple", "id")}, } storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(storage) server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/prefix/version/simple/id") + resp, err := http.Get(server.URL + "/api/version/simple/id") if err != nil { t.Errorf("unexpected error: %v", err) } @@ -710,12 +765,12 @@ func TestDelete(t *testing.T) { simpleStorage := SimpleRESTStorage{} ID := "id" storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(storage) server := httptest.NewServer(handler) defer server.Close() client := http.Client{} - request, err := http.NewRequest("DELETE", server.URL+"/prefix/version/simple/"+ID, nil) + request, err := http.NewRequest("DELETE", server.URL+"/api/version/simple/"+ID, nil) res, err := client.Do(request) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -733,12 +788,12 @@ func TestDeleteInvokesAdmissionControl(t *testing.T) { simpleStorage := SimpleRESTStorage{} ID := "id" storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, deny.NewAlwaysDeny(), requestContextMapper, mapper) + handler := handleDeny(storage) server := httptest.NewServer(handler) defer server.Close() client := http.Client{} - request, err := http.NewRequest("DELETE", server.URL+"/prefix/version/simple/"+ID, nil) + request, err := http.NewRequest("DELETE", server.URL+"/api/version/simple/"+ID, nil) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -755,12 +810,12 @@ func TestDeleteMissing(t *testing.T) { errors: map[string]error{"delete": apierrs.NewNotFound("simple", ID)}, } storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(storage) server := httptest.NewServer(handler) defer server.Close() client := http.Client{} - request, err := http.NewRequest("DELETE", server.URL+"/prefix/version/simple/"+ID, nil) + request, err := http.NewRequest("DELETE", server.URL+"/api/version/simple/"+ID, nil) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -778,11 +833,11 @@ func TestUpdate(t *testing.T) { storage["simple"] = &simpleStorage selfLinker := &setTestSelfLinker{ t: t, - expectedSet: "/prefix/version/simple/" + ID + "?namespace=default", + expectedSet: "/api/version/simple/" + ID + "?namespace=default", name: ID, namespace: api.NamespaceDefault, } - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handleLinker(storage, selfLinker) server := httptest.NewServer(handler) defer server.Close() @@ -800,7 +855,7 @@ func TestUpdate(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/prefix/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) _, err = client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -819,7 +874,7 @@ func TestUpdateInvokesAdmissionControl(t *testing.T) { simpleStorage := SimpleRESTStorage{} ID := "id" storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, deny.NewAlwaysDeny(), requestContextMapper, mapper) + handler := handleDeny(storage) server := httptest.NewServer(handler) defer server.Close() @@ -837,7 +892,7 @@ func TestUpdateInvokesAdmissionControl(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/prefix/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -852,7 +907,7 @@ func TestUpdateRequiresMatchingName(t *testing.T) { simpleStorage := SimpleRESTStorage{} ID := "id" storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, deny.NewAlwaysDeny(), requestContextMapper, mapper) + handler := handleDeny(storage) server := httptest.NewServer(handler) defer server.Close() @@ -866,7 +921,7 @@ func TestUpdateRequiresMatchingName(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/prefix/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -881,7 +936,7 @@ func TestUpdateAllowsMissingNamespace(t *testing.T) { simpleStorage := SimpleRESTStorage{} ID := "id" storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(storage) server := httptest.NewServer(handler) defer server.Close() @@ -898,7 +953,7 @@ func TestUpdateAllowsMissingNamespace(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/prefix/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -918,7 +973,7 @@ func TestUpdateAllowsMismatchedNamespaceOnError(t *testing.T) { t: t, err: fmt.Errorf("test error"), } - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handleLinker(storage, selfLinker) server := httptest.NewServer(handler) defer server.Close() @@ -936,7 +991,7 @@ func TestUpdateAllowsMismatchedNamespaceOnError(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/prefix/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) _, err = client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -955,7 +1010,7 @@ func TestUpdatePreventsMismatchedNamespace(t *testing.T) { simpleStorage := SimpleRESTStorage{} ID := "id" storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(storage) server := httptest.NewServer(handler) defer server.Close() @@ -973,7 +1028,7 @@ func TestUpdatePreventsMismatchedNamespace(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/prefix/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -990,7 +1045,7 @@ func TestUpdateMissing(t *testing.T) { errors: map[string]error{"update": apierrs.NewNotFound("simple", ID)}, } storage["simple"] = &simpleStorage - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(storage) server := httptest.NewServer(handler) defer server.Close() @@ -1007,7 +1062,7 @@ func TestUpdateMissing(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/prefix/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -1018,20 +1073,20 @@ func TestUpdateMissing(t *testing.T) { } func TestCreateNotFound(t *testing.T) { - handler := Handle(map[string]RESTStorage{ + handler := handle(map[string]RESTStorage{ "simple": &SimpleRESTStorage{ // storage.Create can fail with not found error in theory. // See https://github.com/GoogleCloudPlatform/kubernetes/pull/486#discussion_r15037092. errors: map[string]error{"create": apierrs.NewNotFound("simple", "id")}, }, - }, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + }) server := httptest.NewServer(handler) defer server.Close() client := http.Client{} simple := &Simple{Other: "foo"} data, _ := codec.Encode(simple) - request, err := http.NewRequest("POST", server.URL+"/prefix/version/simple", bytes.NewBuffer(data)) + request, err := http.NewRequest("POST", server.URL+"/api/version/simple", bytes.NewBuffer(data)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1046,6 +1101,54 @@ func TestCreateNotFound(t *testing.T) { } } +func TestCreateChecksDecode(t *testing.T) { + handler := handle(map[string]RESTStorage{"simple": &SimpleRESTStorage{}}) + server := httptest.NewServer(handler) + defer server.Close() + client := http.Client{} + + simple := &api.Pod{} + data, _ := codec.Encode(simple) + request, err := http.NewRequest("POST", server.URL+"/api/version/simple", bytes.NewBuffer(data)) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + response, err := client.Do(request) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if response.StatusCode != http.StatusBadRequest { + t.Errorf("Unexpected response %#v", response) + } + if b, _ := ioutil.ReadAll(response.Body); !strings.Contains(string(b), "must be of type Simple") { + t.Errorf("unexpected response: %s", string(b)) + } +} + +func TestUpdateChecksDecode(t *testing.T) { + handler := handle(map[string]RESTStorage{"simple": &SimpleRESTStorage{}}) + server := httptest.NewServer(handler) + defer server.Close() + client := http.Client{} + + simple := &api.Pod{} + data, _ := codec.Encode(simple) + request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/bar", bytes.NewBuffer(data)) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + response, err := client.Do(request) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if response.StatusCode != http.StatusBadRequest { + t.Errorf("Unexpected response %#v", response) + } + if b, _ := ioutil.ReadAll(response.Body); !strings.Contains(string(b), "must be of type Simple") { + t.Errorf("unexpected response: %s", string(b)) + } +} + func TestParseTimeout(t *testing.T) { if d := parseTimeout(""); d != 30*time.Second { t.Errorf("blank timeout produces %v", d) @@ -1089,11 +1192,9 @@ func TestCreate(t *testing.T) { t: t, name: "bar", namespace: "default", - expectedSet: "/prefix/version/foo/bar?namespace=default", + expectedSet: "/api/version/foo/bar?namespace=default", } - handler := Handle(map[string]RESTStorage{ - "foo": &storage, - }, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handleLinker(map[string]RESTStorage{"foo": &storage}, selfLinker) server := httptest.NewServer(handler) defer server.Close() client := http.Client{} @@ -1102,7 +1203,7 @@ func TestCreate(t *testing.T) { Other: "bar", } data, _ := codec.Encode(simple) - request, err := http.NewRequest("POST", server.URL+"/prefix/version/foo", bytes.NewBuffer(data)) + request, err := http.NewRequest("POST", server.URL+"/api/version/foo", bytes.NewBuffer(data)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1147,11 +1248,9 @@ func TestCreateInNamespace(t *testing.T) { t: t, name: "bar", namespace: "other", - expectedSet: "/prefix/version/foo/bar?namespace=other", + expectedSet: "/api/version/foo/bar?namespace=other", } - handler := Handle(map[string]RESTStorage{ - "foo": &storage, - }, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handleLinker(map[string]RESTStorage{"foo": &storage}, selfLinker) server := httptest.NewServer(handler) defer server.Close() client := http.Client{} @@ -1160,7 +1259,7 @@ func TestCreateInNamespace(t *testing.T) { Other: "bar", } data, _ := codec.Encode(simple) - request, err := http.NewRequest("POST", server.URL+"/prefix/version/foo?namespace=other", bytes.NewBuffer(data)) + request, err := http.NewRequest("POST", server.URL+"/api/version/foo?namespace=other", bytes.NewBuffer(data)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1205,11 +1304,9 @@ func TestCreateInvokesAdmissionControl(t *testing.T) { t: t, name: "bar", namespace: "other", - expectedSet: "/prefix/version/foo/bar?namespace=other", + expectedSet: "/api/version/foo/bar?namespace=other", } - handler := Handle(map[string]RESTStorage{ - "foo": &storage, - }, codec, "/prefix", testVersion, selfLinker, deny.NewAlwaysDeny(), requestContextMapper, mapper) + handler := handleInternal(map[string]RESTStorage{"foo": &storage}, deny.NewAlwaysDeny(), mapper, selfLinker) server := httptest.NewServer(handler) defer server.Close() client := http.Client{} @@ -1218,7 +1315,7 @@ func TestCreateInvokesAdmissionControl(t *testing.T) { Other: "bar", } data, _ := codec.Encode(simple) - request, err := http.NewRequest("POST", server.URL+"/prefix/version/foo?namespace=other", bytes.NewBuffer(data)) + request, err := http.NewRequest("POST", server.URL+"/api/version/foo?namespace=other", bytes.NewBuffer(data)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1269,11 +1366,11 @@ func TestDelayReturnsError(t *testing.T) { return nil, apierrs.NewAlreadyExists("foo", "bar") }, } - handler := Handle(map[string]RESTStorage{"foo": &storage}, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(map[string]RESTStorage{"foo": &storage}) server := httptest.NewServer(handler) defer server.Close() - status := expectApiStatus(t, "DELETE", fmt.Sprintf("%s/prefix/version/foo/bar", server.URL), nil, http.StatusConflict) + status := expectApiStatus(t, "DELETE", fmt.Sprintf("%s/api/version/foo/bar", server.URL), nil, http.StatusConflict) if status.Status != api.StatusFailure || status.Message == "" || status.Details == nil || status.Reason != api.StatusReasonAlreadyExists { t.Errorf("Unexpected status %#v", status) } @@ -1333,15 +1430,15 @@ func TestCreateTimeout(t *testing.T) { return obj, nil }, } - handler := Handle(map[string]RESTStorage{ + handler := handle(map[string]RESTStorage{ "foo": &storage, - }, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + }) server := httptest.NewServer(handler) defer server.Close() simple := &Simple{Other: "foo"} data, _ := codec.Encode(simple) - itemOut := expectApiStatus(t, "POST", server.URL+"/prefix/version/foo?timeout=4ms", data, apierrs.StatusServerTimeout) + itemOut := expectApiStatus(t, "POST", server.URL+"/api/version/foo?timeout=4ms", data, apierrs.StatusServerTimeout) if itemOut.Status != api.StatusFailure || itemOut.Reason != api.StatusReasonTimeout { t.Errorf("Unexpected status %#v", itemOut) } @@ -1367,7 +1464,7 @@ func TestCORSAllowedOrigins(t *testing.T) { } handler := CORS( - Handle(map[string]RESTStorage{}, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper), + handle(map[string]RESTStorage{}), allowedOriginRegexps, nil, nil, "true", ) server := httptest.NewServer(handler) diff --git a/pkg/apiserver/proxy_test.go b/pkg/apiserver/proxy_test.go index 90b56ec9b33..797728ffc14 100644 --- a/pkg/apiserver/proxy_test.go +++ b/pkg/apiserver/proxy_test.go @@ -280,14 +280,10 @@ func TestProxy(t *testing.T) { expectedResourceNamespace: item.reqNamespace, } - namespaceHandler := Handle(map[string]RESTStorage{ - "foo": simpleStorage, - }, codec, "/prefix", "version", selfLinker, admissionControl, requestContextMapper, namespaceMapper) + namespaceHandler := handleNamespaced(map[string]RESTStorage{"foo": simpleStorage}) namespaceServer := httptest.NewServer(namespaceHandler) defer namespaceServer.Close() - legacyNamespaceHandler := Handle(map[string]RESTStorage{ - "foo": simpleStorage, - }, codec, "/prefix", "version", selfLinker, admissionControl, requestContextMapper, legacyNamespaceMapper) + legacyNamespaceHandler := handle(map[string]RESTStorage{"foo": simpleStorage}) legacyNamespaceServer := httptest.NewServer(legacyNamespaceHandler) defer legacyNamespaceServer.Close() @@ -296,8 +292,8 @@ func TestProxy(t *testing.T) { server *httptest.Server proxyTestPattern string }{ - {namespaceServer, "/prefix/version/proxy/namespaces/" + item.reqNamespace + "/foo/id" + item.path}, - {legacyNamespaceServer, "/prefix/version/proxy/foo/id" + item.path + "?namespace=" + item.reqNamespace}, + {namespaceServer, "/api/version/proxy/namespaces/" + item.reqNamespace + "/foo/id" + item.path}, + {legacyNamespaceServer, "/api/version/proxy/foo/id" + item.path + "?namespace=" + item.reqNamespace}, } for _, serverPattern := range serverPatterns { @@ -344,14 +340,12 @@ func TestProxyUpgrade(t *testing.T) { expectedResourceNamespace: "myns", } - namespaceHandler := Handle(map[string]RESTStorage{ - "foo": simpleStorage, - }, codec, "/prefix", "version", selfLinker, admissionControl, requestContextMapper, namespaceMapper) + namespaceHandler := handleNamespaced(map[string]RESTStorage{"foo": simpleStorage}) server := httptest.NewServer(namespaceHandler) defer server.Close() - ws, err := websocket.Dial("ws://"+server.Listener.Addr().String()+"/prefix/version/proxy/namespaces/myns/foo/123", "", "http://127.0.0.1/") + ws, err := websocket.Dial("ws://"+server.Listener.Addr().String()+"/api/version/proxy/namespaces/myns/foo/123", "", "http://127.0.0.1/") if err != nil { t.Fatalf("websocket dial err: %s", err) } diff --git a/pkg/apiserver/redirect_test.go b/pkg/apiserver/redirect_test.go index cc3edaa2876..c74c99994c8 100644 --- a/pkg/apiserver/redirect_test.go +++ b/pkg/apiserver/redirect_test.go @@ -29,9 +29,7 @@ func TestRedirect(t *testing.T) { errors: map[string]error{}, expectedResourceNamespace: "default", } - handler := Handle(map[string]RESTStorage{ - "foo": simpleStorage, - }, codec, "/prefix", "version", selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(map[string]RESTStorage{"foo": simpleStorage}) server := httptest.NewServer(handler) defer server.Close() @@ -54,7 +52,7 @@ func TestRedirect(t *testing.T) { for _, item := range table { simpleStorage.errors["resourceLocation"] = item.err simpleStorage.resourceLocation = item.id - resp, err := client.Get(server.URL + "/prefix/version/redirect/foo/" + item.id) + resp, err := client.Get(server.URL + "/api/version/redirect/foo/" + item.id) if resp == nil { t.Fatalf("Unexpected nil resp") } @@ -82,9 +80,7 @@ func TestRedirectWithNamespaces(t *testing.T) { errors: map[string]error{}, expectedResourceNamespace: "other", } - handler := Handle(map[string]RESTStorage{ - "foo": simpleStorage, - }, codec, "/prefix", "version", selfLinker, admissionControl, requestContextMapper, namespaceMapper) + handler := handleNamespaced(map[string]RESTStorage{"foo": simpleStorage}) server := httptest.NewServer(handler) defer server.Close() @@ -107,7 +103,7 @@ func TestRedirectWithNamespaces(t *testing.T) { for _, item := range table { simpleStorage.errors["resourceLocation"] = item.err simpleStorage.resourceLocation = item.id - resp, err := client.Get(server.URL + "/prefix/version/redirect/namespaces/other/foo/" + item.id) + resp, err := client.Get(server.URL + "/api/version/redirect/namespaces/other/foo/" + item.id) if resp == nil { t.Fatalf("Unexpected nil resp") } diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index dd295d03859..d5230c6ee58 100644 --- a/pkg/apiserver/resthandler.go +++ b/pkg/apiserver/resthandler.go @@ -17,6 +17,7 @@ limitations under the License. package apiserver import ( + "fmt" "net/http" "net/url" gpath "path" @@ -97,7 +98,7 @@ func parseSelectorQueryParams(query url.Values, version, apiResource string) (la } // ListResource returns a function that handles retrieving a list of resources from a RESTStorage object. -func ListResource(r RESTLister, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, requestInfoResolver *APIRequestInfoResolver) restful.RouteFunction { +func ListResource(r RESTLister, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, version, apiResource string) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { w := res.ResponseWriter @@ -109,13 +110,7 @@ func ListResource(r RESTLister, ctxFn ContextFunc, namer ScopeNamer, codec runti ctx := ctxFn(req) ctx = api.WithNamespace(ctx, namespace) - requestInfo, err := requestInfoResolver.GetAPIRequestInfo(req.Request) - if err != nil { - errorJSON(err, codec, w) - return - } - - label, field, err := parseSelectorQueryParams(req.Request.URL.Query(), requestInfo.APIVersion, requestInfo.Resource) + label, field, err := parseSelectorQueryParams(req.Request.URL.Query(), version, apiResource) if err != nil { errorJSON(err, codec, w) return @@ -135,7 +130,7 @@ func ListResource(r RESTLister, ctxFn ContextFunc, namer ScopeNamer, codec runti } // CreateResource returns a function that will handle a resource creation. -func CreateResource(r RESTCreater, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, resource string, admit admission.Interface) restful.RouteFunction { +func CreateResource(r RESTCreater, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, typer runtime.ObjectTyper, resource string, admit admission.Interface) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { w := res.ResponseWriter @@ -158,6 +153,7 @@ func CreateResource(r RESTCreater, ctxFn ContextFunc, namer ScopeNamer, codec ru obj := r.New() if err := codec.DecodeInto(body, obj); err != nil { + err = transformDecodeError(typer, err, obj, body) errorJSON(err, codec, w) return } @@ -190,7 +186,7 @@ func CreateResource(r RESTCreater, ctxFn ContextFunc, namer ScopeNamer, codec ru } // UpdateResource returns a function that will handle a resource update -func UpdateResource(r RESTUpdater, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, resource string, admit admission.Interface) restful.RouteFunction { +func UpdateResource(r RESTUpdater, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, typer runtime.ObjectTyper, resource string, admit admission.Interface) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { w := res.ResponseWriter @@ -213,6 +209,7 @@ func UpdateResource(r RESTUpdater, ctxFn ContextFunc, namer ScopeNamer, codec ru obj := r.New() if err := codec.DecodeInto(body, obj); err != nil { + err = transformDecodeError(typer, err, obj, body) errorJSON(err, codec, w) return } @@ -346,6 +343,18 @@ func finishRequest(timeout time.Duration, fn resultFunc) (result runtime.Object, } } +// transformDecodeError adds additional information when a decode fails. +func transformDecodeError(typer runtime.ObjectTyper, baseErr error, into runtime.Object, body []byte) error { + _, kind, err := typer.ObjectVersionAndKind(into) + if err != nil { + return err + } + if version, dataKind, err := typer.DataVersionAndKind(body); err == nil && len(dataKind) > 0 { + return errors.NewBadRequest(fmt.Sprintf("%s in version %s cannot be handled as a %s: %v", dataKind, version, kind, baseErr)) + } + return errors.NewBadRequest(fmt.Sprintf("the object provided is unrecognized (must be of type %s): %v", kind, baseErr)) +} + // setSelfLink sets the self link of an object (or the child items in a list) to the base URL of the request // plus the path and query generated by the provided linkFunc func setSelfLink(obj runtime.Object, req *restful.Request, namer ScopeNamer) error { diff --git a/pkg/apiserver/watch_test.go b/pkg/apiserver/watch_test.go index d7b44be6307..95e4ef1c363 100644 --- a/pkg/apiserver/watch_test.go +++ b/pkg/apiserver/watch_test.go @@ -49,9 +49,7 @@ var watchTestTable = []struct { func TestWatchWebsocket(t *testing.T) { simpleStorage := &SimpleRESTStorage{} _ = ResourceWatcher(simpleStorage) // Give compile error if this doesn't work. - handler := Handle(map[string]RESTStorage{ - "foo": simpleStorage, - }, codec, "/api", "version", selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(map[string]RESTStorage{"foo": simpleStorage}) server := httptest.NewServer(handler) defer server.Close() @@ -103,9 +101,7 @@ func TestWatchWebsocket(t *testing.T) { func TestWatchHTTP(t *testing.T) { simpleStorage := &SimpleRESTStorage{} - handler := Handle(map[string]RESTStorage{ - "foo": simpleStorage, - }, codec, "/api", "version", selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(map[string]RESTStorage{"foo": simpleStorage}) server := httptest.NewServer(handler) defer server.Close() client := http.Client{} @@ -170,9 +166,7 @@ func TestWatchParamParsing(t *testing.T) { return label, value, nil }) simpleStorage := &SimpleRESTStorage{} - handler := Handle(map[string]RESTStorage{ - "foo": simpleStorage, - }, codec, "/api", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(map[string]RESTStorage{"foo": simpleStorage}) server := httptest.NewServer(handler) defer server.Close() @@ -242,9 +236,7 @@ func TestWatchParamParsing(t *testing.T) { func TestWatchProtocolSelection(t *testing.T) { simpleStorage := &SimpleRESTStorage{} - handler := Handle(map[string]RESTStorage{ - "foo": simpleStorage, - }, codec, "/api", "version", selfLinker, admissionControl, requestContextMapper, mapper) + handler := handle(map[string]RESTStorage{"foo": simpleStorage}) server := httptest.NewServer(handler) defer server.Close() defer server.CloseClientConnections() diff --git a/pkg/master/master.go b/pkg/master/master.go index 6dc999ccb6d..4d35bc34635 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -30,7 +30,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/admission" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta2" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3" @@ -55,7 +54,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/resourcequotausage" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/secret" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/service" - "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" "github.com/GoogleCloudPlatform/kubernetes/pkg/ui" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" @@ -425,14 +423,14 @@ func (m *Master) init(c *Config) { } apiVersions := []string{"v1beta1", "v1beta2"} - if err := apiserver.NewAPIGroupVersion(m.api_v1beta1()).InstallREST(m.handlerContainer, c.APIPrefix, "v1beta1"); err != nil { + if err := m.api_v1beta1().InstallREST(m.handlerContainer); err != nil { glog.Fatalf("Unable to setup API v1beta1: %v", err) } - if err := apiserver.NewAPIGroupVersion(m.api_v1beta2()).InstallREST(m.handlerContainer, c.APIPrefix, "v1beta2"); err != nil { + if err := m.api_v1beta2().InstallREST(m.handlerContainer); err != nil { glog.Fatalf("Unable to setup API v1beta2: %v", err) } if c.EnableV1Beta3 { - if err := apiserver.NewAPIGroupVersion(m.api_v1beta3()).InstallREST(m.handlerContainer, c.APIPrefix, "v1beta3"); err != nil { + if err := m.api_v1beta3().InstallREST(m.handlerContainer); err != nil { glog.Fatalf("Unable to setup API v1beta3: %v", err) } apiVersions = []string{"v1beta1", "v1beta2", "v1beta3"} @@ -569,26 +567,49 @@ func (m *Master) getServersToValidate(c *Config) map[string]apiserver.Server { return serversToValidate } +func (m *Master) defaultAPIGroupVersion() *apiserver.APIGroupVersion { + return &apiserver.APIGroupVersion{ + Root: m.apiPrefix, + + Mapper: latest.RESTMapper, + + Creater: api.Scheme, + Typer: api.Scheme, + Linker: latest.SelfLinker, + + Admit: m.admissionControl, + Context: m.requestContextMapper, + } +} + // api_v1beta1 returns the resources and codec for API version v1beta1. -func (m *Master) api_v1beta1() (map[string]apiserver.RESTStorage, runtime.Codec, string, string, runtime.SelfLinker, admission.Interface, api.RequestContextMapper, meta.RESTMapper) { +func (m *Master) api_v1beta1() *apiserver.APIGroupVersion { storage := make(map[string]apiserver.RESTStorage) for k, v := range m.storage { storage[k] = v } - return storage, v1beta1.Codec, "api", "/api/v1beta1", latest.SelfLinker, m.admissionControl, m.requestContextMapper, latest.RESTMapper + version := m.defaultAPIGroupVersion() + version.Storage = storage + version.Version = "v1beta1" + version.Codec = v1beta1.Codec + return version } // api_v1beta2 returns the resources and codec for API version v1beta2. -func (m *Master) api_v1beta2() (map[string]apiserver.RESTStorage, runtime.Codec, string, string, runtime.SelfLinker, admission.Interface, api.RequestContextMapper, meta.RESTMapper) { +func (m *Master) api_v1beta2() *apiserver.APIGroupVersion { storage := make(map[string]apiserver.RESTStorage) for k, v := range m.storage { storage[k] = v } - return storage, v1beta2.Codec, "api", "/api/v1beta2", latest.SelfLinker, m.admissionControl, m.requestContextMapper, latest.RESTMapper + version := m.defaultAPIGroupVersion() + version.Storage = storage + version.Version = "v1beta2" + version.Codec = v1beta2.Codec + return version } // api_v1beta3 returns the resources and codec for API version v1beta3. -func (m *Master) api_v1beta3() (map[string]apiserver.RESTStorage, runtime.Codec, string, string, runtime.SelfLinker, admission.Interface, api.RequestContextMapper, meta.RESTMapper) { +func (m *Master) api_v1beta3() *apiserver.APIGroupVersion { storage := make(map[string]apiserver.RESTStorage) for k, v := range m.storage { if k == "minions" { @@ -596,5 +617,9 @@ func (m *Master) api_v1beta3() (map[string]apiserver.RESTStorage, runtime.Codec, } storage[strings.ToLower(k)] = v } - return storage, v1beta3.Codec, "api", "/api/v1beta3", latest.SelfLinker, m.admissionControl, m.requestContextMapper, latest.RESTMapper + version := m.defaultAPIGroupVersion() + version.Storage = storage + version.Version = "v1beta3" + version.Codec = v1beta3.Codec + return version } diff --git a/pkg/runtime/interfaces.go b/pkg/runtime/interfaces.go index 4bfbf087e8c..11735027b9e 100644 --- a/pkg/runtime/interfaces.go +++ b/pkg/runtime/interfaces.go @@ -45,6 +45,11 @@ type ObjectTyper interface { ObjectVersionAndKind(Object) (version, kind string, err error) } +// ObjectCreater contains methods for instantiating an object by kind and version. +type ObjectCreater interface { + New(version, kind string) (out Object, err error) +} + // ResourceVersioner provides methods for setting and retrieving // the resource version from an API object. type ResourceVersioner interface {