From 9175082a1c26c68b8cdf38f33bd387b46ec66a65 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 12 Feb 2015 14:21:47 -0500 Subject: [PATCH] Split naming behavior out into objects that are derived from the request Fix bug with cross namespace queries not being possible. Ensure selflink is set on lists correctly. --- cmd/integration/integration.go | 11 +- hack/test-cmd.sh | 5 + pkg/apiserver/api_installer.go | 393 ++++++++++++++++++++++---------- pkg/apiserver/apiserver_test.go | 254 ++++++++++++++++++--- pkg/apiserver/resthandler.go | 159 +++++++------ 5 files changed, 590 insertions(+), 232 deletions(-) diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go index e1def7c8547..4c1a7782913 100644 --- a/cmd/integration/integration.go +++ b/cmd/integration/integration.go @@ -23,6 +23,7 @@ import ( "net" "net/http" "net/http/httptest" + "net/url" "os" "reflect" "runtime" @@ -335,7 +336,9 @@ func runSelfLinkTest(c *client.Client) { if err != nil { glog.Fatalf("Failed creating selflinktest service: %v", err) } - err = c.Get().AbsPath(svc.SelfLink).Do().Into(&svc) + // TODO: this is not namespace aware + link, _ := url.Parse(svc.SelfLink) + err = c.Get().AbsPath(link.Path).Do().Into(&svc) if err != nil { glog.Fatalf("Failed listing service with supplied self link '%v': %v", svc.SelfLink, err) } @@ -346,7 +349,8 @@ func runSelfLinkTest(c *client.Client) { glog.Fatalf("Failed listing services: %v", err) } - err = c.Get().AbsPath(svcList.SelfLink).Do().Into(&svcList) + link, _ = url.Parse(svcList.SelfLink) + err = c.Get().AbsPath(link.Path).Do().Into(&svcList) if err != nil { glog.Fatalf("Failed listing services with supplied self link '%v': %v", svcList.SelfLink, err) } @@ -358,7 +362,8 @@ func runSelfLinkTest(c *client.Client) { continue } found = true - err = c.Get().AbsPath(item.SelfLink).Do().Into(&svc) + link, _ = url.Parse(item.SelfLink) + err = c.Get().AbsPath(link.Path).Do().Into(&svc) if err != nil { glog.Fatalf("Failed listing service with supplied self link '%v': %v", item.SelfLink, err) } diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index 2fd73970294..d4c95f97c2d 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -146,6 +146,11 @@ for version in "${kube_api_versions[@]}"; do kubectl describe pod redis-master "${kube_flags[@]}" | grep -q 'Name:.*redis-master' kubectl delete -f examples/guestbook/redis-master.json "${kube_flags[@]}" + # make calls in another namespace + kubectl create --namespace=other -f examples/guestbook/redis-master.json "${kube_flags[@]}" + kubectl get pod --namespace=other redis-master + kubectl delete pod --namespace=other redis-master + kube::log::status "Testing kubectl(${version}:services)" kubectl get services "${kube_flags[@]}" kubectl create -f examples/guestbook/frontend-service.json "${kube_flags[@]}" diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go index 03a771e136b..95dd50d5a57 100644 --- a/pkg/apiserver/api_installer.go +++ b/pkg/apiserver/api_installer.go @@ -42,6 +42,7 @@ 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. + Namer ScopeNamer } // errEmptyName is returned when API requests do not fill the name section of the path. @@ -87,7 +88,6 @@ func (a *APIInstaller) newWebService() *restful.WebService { func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage, ws *restful.WebService, watchHandler http.Handler, redirectHandler http.Handler, proxyHandler http.Handler) error { codec := a.group.codec admit := a.group.admit - linker := a.group.linker context := a.group.context resource := path @@ -149,13 +149,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage } var ctxFn ContextFunc - var namespaceFn ResourceNamespaceFunc - var nameFn ResourceNameFunc - var generateLinkFn linkFunc - var objNameFn ObjectNameFunc - linkFn := func(req *restful.Request, obj runtime.Object) error { - return setSelfLink(obj, req.Request, a.group.linker, generateLinkFn) - } ctxFn = func(req *restful.Request) api.Context { if ctx, ok := context.Get(req.Request); ok { return ctx @@ -168,146 +161,76 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage 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 { - objNameFn = func(obj runtime.Object) (namespace, name string, err error) { - name, err = linker.Name(obj) - if len(name) == 0 { - err = errEmptyName - } - return - } - - // 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) - namespaceFn = func(req *restful.Request) (namespace string, err error) { - return - } - nameFn = func(req *restful.Request) (namespace, name string, err error) { - name = req.PathParameter("name") - if len(name) == 0 { - err = errEmptyName - } - return - } - generateLinkFn = func(namespace, name string) (path string, query string) { - path = strings.Replace(itemPath, "{name}", name, 1) - path = gpath.Join(a.prefix, path) - return - } + 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", path, params, namer}, storageVerbs["RESTLister"]) + actions = appendIf(actions, action{"POST", path, params, namer}, storageVerbs["RESTCreater"]) + actions = appendIf(actions, action{"WATCHLIST", "/watch/" + path, params, namer}, allowWatchList) + + actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, storageVerbs["RESTGetter"]) + actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, storageVerbs["RESTUpdater"]) + actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, storageVerbs["RESTDeleter"]) + actions = appendIf(actions, action{"WATCH", "/watch/" + itemPath, nameParams, namer}, storageVerbs["ResourceWatcher"]) + actions = appendIf(actions, action{"REDIRECT", "/redirect/" + itemPath, nameParams, namer}, storageVerbs["Redirector"]) + actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath + "/{path:*}", nameParams, namer}, storageVerbs["Redirector"]) + actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath, nameParams, namer}, storageVerbs["Redirector"]) - 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"]) - actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath, nameParams}, storageVerbs["Redirector"]) } else { - objNameFn = func(obj runtime.Object) (namespace, name string, err error) { - if name, err = linker.Name(obj); err != nil { - return - } - namespace, err = linker.Namespace(obj) - return - } - // 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} - namespaceFn = func(req *restful.Request) (namespace string, err error) { - namespace = req.PathParameter(scope.ParamName()) - if len(namespace) == 0 { - namespace = api.NamespaceDefault - } - return - } - - 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) - nameFn = func(req *restful.Request) (namespace, name string, err error) { - namespace, _ = namespaceFn(req) - name = req.PathParameter("name") - if len(name) == 0 { - err = errEmptyName - } - return - } - generateLinkFn = func(namespace, name string) (path string, query string) { - path = strings.Replace(itemPath, "{name}", name, 1) - path = strings.Replace(path, "{"+scope.ParamName()+"}", namespace, 1) - path = gpath.Join(a.prefix, path) - return - } + namer := scopeNaming{scope, a.group.linker, gpath.Join(a.prefix, itemPath), false} - 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"]) - actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath, nameParams}, storageVerbs["Redirector"]) + actions = appendIf(actions, action{"LIST", namespacedPath, namespaceParams, namer}, storageVerbs["RESTLister"]) + actions = appendIf(actions, action{"POST", namespacedPath, namespaceParams, namer}, storageVerbs["RESTCreater"]) + actions = appendIf(actions, action{"WATCHLIST", "/watch/" + namespacedPath, namespaceParams, namer}, allowWatchList) + + actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, storageVerbs["RESTGetter"]) + actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, storageVerbs["RESTUpdater"]) + actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, storageVerbs["RESTDeleter"]) + actions = appendIf(actions, action{"WATCH", "/watch/" + itemPath, nameParams, namer}, storageVerbs["ResourceWatcher"]) + actions = appendIf(actions, action{"REDIRECT", "/redirect/" + itemPath, nameParams, namer}, storageVerbs["Redirector"]) + actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath + "/{path:*}", nameParams, namer}, storageVerbs["Redirector"]) + actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath, nameParams, namer}, storageVerbs["Redirector"]) // list across namespace. - actions = appendIf(actions, action{"LIST", path, params}, storageVerbs["RESTLister"]) - actions = appendIf(actions, action{"WATCHLIST", "/watch/" + path, params}, allowWatchList) + namer = scopeNaming{scope, a.group.linker, gpath.Join(a.prefix, itemPath), true} + actions = appendIf(actions, action{"LIST", path, params, namer}, storageVerbs["RESTLister"]) + actions = appendIf(actions, action{"WATCHLIST", "/watch/" + path, params, namer}, 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} - namespaceFn = func(req *restful.Request) (namespace string, err error) { - namespace = req.QueryParameter(scope.ParamName()) - if len(namespace) == 0 { - namespace = api.NamespaceDefault - } - return - } - - 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) - nameFn = func(req *restful.Request) (namespace, name string, err error) { - namespace, _ = namespaceFn(req) - name = req.PathParameter("name") - if len(name) == 0 { - err = errEmptyName - } - return - } - generateLinkFn = func(namespace, name string) (path string, query string) { - path = strings.Replace(itemPath, "{name}", name, -1) - path = gpath.Join(a.prefix, path) - if len(namespace) > 0 { - values := make(url.Values) - values.Set(scope.ParamName(), namespace) - query = values.Encode() - } - return - } + namer := legacyScopeNaming{scope, a.group.linker, gpath.Join(a.prefix, itemPath)} - 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"]) - actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath, nameParams}, storageVerbs["Redirector"]) + actions = appendIf(actions, action{"LIST", path, namespaceParams, namer}, storageVerbs["RESTLister"]) + actions = appendIf(actions, action{"POST", path, namespaceParams, namer}, storageVerbs["RESTCreater"]) + actions = appendIf(actions, action{"WATCHLIST", "/watch/" + path, namespaceParams, namer}, allowWatchList) + + actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, storageVerbs["RESTGetter"]) + actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, storageVerbs["RESTUpdater"]) + actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, storageVerbs["RESTDeleter"]) + actions = appendIf(actions, action{"WATCH", "/watch/" + itemPath, nameParams, namer}, storageVerbs["ResourceWatcher"]) + actions = appendIf(actions, action{"REDIRECT", "/redirect/" + itemPath, nameParams, namer}, storageVerbs["Redirector"]) + actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath + "/{path:*}", nameParams, namer}, storageVerbs["Redirector"]) + actions = appendIf(actions, action{"PROXY", "/proxy/" + itemPath, nameParams, namer}, storageVerbs["Redirector"]) } } @@ -332,7 +255,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, nameFn, linkFn, codec)). + route := ws.GET(action.Path).To(GetResource(getter, ctxFn, action.Namer, codec)). Filter(m). Doc("read the specified " + kind). Operation("read" + kind). @@ -340,7 +263,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, namespaceFn, linkFn, codec)). + route := ws.GET(action.Path).To(ListResource(lister, ctxFn, action.Namer, codec)). Filter(m). Doc("list objects of kind " + kind). Operation("list" + kind). @@ -348,7 +271,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, nameFn, objNameFn, linkFn, codec, resource, admit)). + route := ws.PUT(action.Path).To(UpdateResource(updater, ctxFn, action.Namer, codec, resource, admit)). Filter(m). Doc("update the specified " + kind). Operation("update" + kind). @@ -356,7 +279,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, namespaceFn, linkFn, codec, resource, admit)). + route := ws.POST(action.Path).To(CreateResource(creater, ctxFn, action.Namer, codec, resource, admit)). Filter(m). Doc("create a " + kind). Operation("create" + kind). @@ -364,7 +287,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, nameFn, linkFn, codec, resource, kind, admit)). + route := ws.DELETE(action.Path).To(DeleteResource(deleter, ctxFn, action.Namer, codec, resource, kind, admit)). Filter(m). Doc("delete a " + kind). Operation("delete" + kind) @@ -409,6 +332,224 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage return nil } +// rootScopeNaming reads only names from a request and ignores namespaces. It implements ScopeNamer +// for root scoped resources. +type rootScopeNaming struct { + scope meta.RESTScope + runtime.SelfLinker + itemPath string +} + +// rootScopeNaming implements ScopeNamer +var _ ScopeNamer = rootScopeNaming{} + +// Namespace returns an empty string because root scoped objects have no namespace. +func (n rootScopeNaming) Namespace(req *restful.Request) (namespace string, err error) { + return "", nil +} + +// Name returns the name from the path and an empty string for namespace, or an error if the +// name is empty. +func (n rootScopeNaming) Name(req *restful.Request) (namespace, name string, err error) { + name = req.PathParameter("name") + if len(name) == 0 { + return "", "", errEmptyName + } + return "", name, nil +} + +// GenerateLink returns the appropriate path and query to locate an object by its canonical path. +func (n rootScopeNaming) GenerateLink(req *restful.Request, obj runtime.Object) (path, query string, err error) { + _, name, err := n.ObjectName(obj) + if err != nil { + return "", "", err + } + if len(name) == 0 { + _, name, err = n.Name(req) + if err != nil { + return "", "", err + } + } + path = strings.Replace(n.itemPath, "{name}", name, 1) + return path, "", nil +} + +// GenerateListLink returns the appropriate path and query to locate a list by its canonical path. +func (n rootScopeNaming) GenerateListLink(req *restful.Request) (path, query string, err error) { + path = req.Request.URL.Path + return path, "", nil +} + +// ObjectName returns the name set on the object, or an error if the +// name cannot be returned. Namespace is empty +// TODO: distinguish between objects with name/namespace and without via a specific error. +func (n rootScopeNaming) ObjectName(obj runtime.Object) (namespace, name string, err error) { + name, err = n.SelfLinker.Name(obj) + if err != nil { + return "", "", err + } + if len(name) == 0 { + return "", "", errEmptyName + } + return "", name, nil +} + +// scopeNaming returns naming information from a request. It implements ScopeNamer for +// namespace scoped resources. +type scopeNaming struct { + scope meta.RESTScope + runtime.SelfLinker + itemPath string + allNamespaces bool +} + +// scopeNaming implements ScopeNamer +var _ ScopeNamer = scopeNaming{} + +// Namespace returns the namespace from the path or the default. +func (n scopeNaming) Namespace(req *restful.Request) (namespace string, err error) { + if n.allNamespaces { + return "", nil + } + namespace = req.PathParameter(n.scope.ParamName()) + if len(namespace) == 0 { + // a URL was constructed without the namespace, or this method was invoked + // on an object without a namespace path parameter. + return "", fmt.Errorf("no namespace parameter found on request") + } + return namespace, nil +} + +// Name returns the name from the path, the namespace (or default), or an error if the +// name is empty. +func (n scopeNaming) Name(req *restful.Request) (namespace, name string, err error) { + namespace, _ = n.Namespace(req) + name = req.PathParameter("name") + if len(name) == 0 { + return "", "", errEmptyName + } + return +} + +// GenerateLink returns the appropriate path and query to locate an object by its canonical path. +func (n scopeNaming) GenerateLink(req *restful.Request, obj runtime.Object) (path, query string, err error) { + namespace, name, err := n.ObjectName(obj) + if err != nil { + return "", "", err + } + if len(namespace) == 0 && len(name) == 0 { + namespace, name, err = n.Name(req) + if err != nil { + return "", "", err + } + } + path = strings.Replace(n.itemPath, "{name}", name, 1) + if !n.allNamespaces { + path = strings.Replace(path, "{"+n.scope.ParamName()+"}", namespace, 1) + } + return path, "", nil +} + +// GenerateListLink returns the appropriate path and query to locate a list by its canonical path. +func (n scopeNaming) GenerateListLink(req *restful.Request) (path, query string, err error) { + path = req.Request.URL.Path + return path, "", nil +} + +// ObjectName returns the name and namespace set on the object, or an error if the +// name cannot be returned. +// TODO: distinguish between objects with name/namespace and without via a specific error. +func (n scopeNaming) ObjectName(obj runtime.Object) (namespace, name string, err error) { + name, err = n.SelfLinker.Name(obj) + if err != nil { + return "", "", err + } + namespace, err = n.SelfLinker.Namespace(obj) + if err != nil { + return "", "", err + } + return namespace, name, err +} + +// legacyScopeNaming modifies a scopeNaming to read namespace from the query. It implements +// ScopeNamer for older query based namespace parameters. +type legacyScopeNaming struct { + scope meta.RESTScope + runtime.SelfLinker + itemPath string +} + +// legacyScopeNaming implements ScopeNamer +var _ ScopeNamer = legacyScopeNaming{} + +// Namespace returns the namespace from the query or the default. +func (n legacyScopeNaming) Namespace(req *restful.Request) (namespace string, err error) { + values, ok := req.Request.URL.Query()[n.scope.ParamName()] + if !ok || len(values) == 0 { + // legacy behavior + if req.Request.Method == "POST" || len(req.PathParameter("name")) > 0 { + return api.NamespaceDefault, nil + } + return api.NamespaceAll, nil + } + return values[0], nil +} + +// Name returns the name from the path, the namespace (or default), or an error if the +// name is empty. +func (n legacyScopeNaming) Name(req *restful.Request) (namespace, name string, err error) { + namespace, _ = n.Namespace(req) + name = req.PathParameter("name") + if len(name) == 0 { + return "", "", errEmptyName + } + return namespace, name, nil +} + +// GenerateLink returns the appropriate path and query to locate an object by its canonical path. +func (n legacyScopeNaming) GenerateLink(req *restful.Request, obj runtime.Object) (path, query string, err error) { + namespace, name, err := n.ObjectName(obj) + if err != nil { + return "", "", err + } + if len(name) == 0 { + return "", "", errEmptyName + } + path = strings.Replace(n.itemPath, "{name}", name, -1) + values := make(url.Values) + values.Set(n.scope.ParamName(), namespace) + query = values.Encode() + return path, query, nil +} + +// GenerateListLink returns the appropriate path and query to locate a list by its canonical path. +func (n legacyScopeNaming) GenerateListLink(req *restful.Request) (path, query string, err error) { + namespace, err := n.Namespace(req) + if err != nil { + return "", "", err + } + path = req.Request.URL.Path + values := make(url.Values) + values.Set(n.scope.ParamName(), namespace) + query = values.Encode() + return path, query, nil +} + +// ObjectName returns the name and namespace set on the object, or an error if the +// name cannot be returned. +// TODO: distinguish between objects with name/namespace and without via a specific error. +func (n legacyScopeNaming) ObjectName(obj runtime.Object) (namespace, name string, err error) { + name, err = n.SelfLinker.Name(obj) + if err != nil { + return "", "", err + } + namespace, err = n.SelfLinker.Namespace(obj) + if err != nil { + return "", "", err + } + return namespace, name, err +} + // This magic incantation returns *ptrToObject for an arbitrary pointer func indirectArbitraryPointer(ptrToObject interface{}) interface{} { return reflect.Indirect(reflect.ValueOf(ptrToObject)).Interface() diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index f4260c22514..b94c3753334 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -154,6 +154,9 @@ type SimpleRESTStorage struct { updated *Simple created *Simple + actualNamespace string + namespacePresent bool + // These are set when Watch is called fakeWatch *watch.FakeWatcher requestedLabelSelector labels.Selector @@ -172,6 +175,7 @@ type SimpleRESTStorage struct { } func (storage *SimpleRESTStorage) List(ctx api.Context, label, field labels.Selector) (runtime.Object, error) { + storage.checkContext(ctx) result := &SimpleList{ Items: storage.list, } @@ -179,10 +183,16 @@ func (storage *SimpleRESTStorage) List(ctx api.Context, label, field labels.Sele } func (storage *SimpleRESTStorage) Get(ctx api.Context, id string) (runtime.Object, error) { + storage.checkContext(ctx) return api.Scheme.CopyOrDie(&storage.item), storage.errors["get"] } +func (storage *SimpleRESTStorage) checkContext(ctx api.Context) { + storage.actualNamespace, storage.namespacePresent = api.NamespaceFrom(ctx) +} + func (storage *SimpleRESTStorage) Delete(ctx api.Context, id string) (runtime.Object, error) { + storage.checkContext(ctx) storage.deleted = id if err := storage.errors["delete"]; err != nil { return nil, err @@ -204,6 +214,7 @@ func (storage *SimpleRESTStorage) NewList() runtime.Object { } func (storage *SimpleRESTStorage) Create(ctx api.Context, obj runtime.Object) (runtime.Object, error) { + storage.checkContext(ctx) storage.created = obj.(*Simple) if err := storage.errors["create"]; err != nil { return nil, err @@ -216,6 +227,7 @@ func (storage *SimpleRESTStorage) Create(ctx api.Context, obj runtime.Object) (r } func (storage *SimpleRESTStorage) Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool, error) { + storage.checkContext(ctx) storage.updated = obj.(*Simple) if err := storage.errors["update"]; err != nil { return nil, false, err @@ -229,6 +241,7 @@ func (storage *SimpleRESTStorage) Update(ctx api.Context, obj runtime.Object) (r // Implement ResourceWatcher. func (storage *SimpleRESTStorage) Watch(ctx api.Context, label, field labels.Selector, resourceVersion string) (watch.Interface, error) { + storage.checkContext(ctx) storage.requestedLabelSelector = label storage.requestedFieldSelector = field storage.requestedResourceVersion = resourceVersion @@ -242,6 +255,7 @@ func (storage *SimpleRESTStorage) Watch(ctx api.Context, label, field labels.Sel // Implement Redirector. func (storage *SimpleRESTStorage) ResourceLocation(ctx api.Context, id string) (string, error) { + storage.checkContext(ctx) // validate that the namespace context on the request matches the expected input storage.requestedResourceNamespace = api.NamespaceValue(ctx) if storage.expectedResourceNamespace != storage.requestedResourceNamespace { @@ -388,29 +402,57 @@ func TestVersion(t *testing.T) { } } -func TestSimpleList(t *testing.T) { - storage := map[string]RESTStorage{} - simpleStorage := SimpleRESTStorage{} - storage["simple"] = &simpleStorage - selfLinker := &setTestSelfLinker{ - t: t, - namespace: "other", - expectedSet: "/prefix/version/simple?namespace=other", +func TestList(t *testing.T) { + testCases := []struct { + url string + namespace string + selfLink string + legacy bool + }{ + {"/prefix/version/simple", "", "/prefix/version/simple?namespace=", true}, + {"/prefix/version/simple?namespace=other", "other", "/prefix/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}, + // list items across all namespaces + {"/prefix/version/simple", "", "/prefix/version/simple", false}, } - handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) - server := httptest.NewServer(handler) - defer server.Close() + for i, testCase := range testCases { + storage := map[string]RESTStorage{} + simpleStorage := SimpleRESTStorage{expectedResourceNamespace: testCase.namespace} + storage["simple"] = &simpleStorage + selfLinker := &setTestSelfLinker{ + t: t, + namespace: testCase.namespace, + expectedSet: testCase.selfLink, + } + var handler http.Handler + if testCase.legacy { + handler = Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + } else { + handler = Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, namespaceMapper) + } + server := httptest.NewServer(handler) + defer server.Close() - resp, err := http.Get(server.URL + "/prefix/version/simple") - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - if resp.StatusCode != http.StatusOK { - t.Errorf("Unexpected status: %d, Expected: %d, %#v", resp.StatusCode, http.StatusOK, resp) - } - if !selfLinker.called { - t.Errorf("Never set self link") + resp, err := http.Get(server.URL + testCase.url) + if err != nil { + t.Errorf("%d: unexpected error: %v", i, err) + continue + } + if resp.StatusCode != http.StatusOK { + t.Errorf("%d: unexpected status: %d, Expected: %d, %#v", i, resp.StatusCode, http.StatusOK, resp) + } + // TODO: future, restore get links + if !selfLinker.called { + t.Errorf("%d: never set self link", i) + } + if !simpleStorage.namespacePresent { + t.Errorf("%d: namespace not set", i) + } else if simpleStorage.actualNamespace != testCase.namespace { + t.Errorf("%d: unexpected resource namespace: %s", i, simpleStorage.actualNamespace) + } } } @@ -426,7 +468,7 @@ func TestErrorList(t *testing.T) { resp, err := http.Get(server.URL + "/prefix/version/simple") if err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v", err) } if resp.StatusCode != http.StatusInternalServerError { @@ -439,7 +481,54 @@ func TestNonEmptyList(t *testing.T) { simpleStorage := SimpleRESTStorage{ list: []Simple{ { - TypeMeta: api.TypeMeta{Kind: "Simple"}, + ObjectMeta: api.ObjectMeta{Name: "something", Namespace: "other"}, + Other: "foo", + }, + }, + } + storage["simple"] = &simpleStorage + handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + server := httptest.NewServer(handler) + defer server.Close() + + resp, err := http.Get(server.URL + "/prefix/version/simple") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if resp.StatusCode != http.StatusOK { + t.Errorf("Unexpected status: %d, Expected: %d, %#v", resp.StatusCode, http.StatusOK, resp) + body, _ := ioutil.ReadAll(resp.Body) + t.Logf("Data: %s", string(body)) + } + + var listOut SimpleList + body, err := extractBody(resp, &listOut) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(listOut.Items) != 1 { + t.Errorf("Unexpected response: %#v", listOut) + return + } + 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=" { + t.Errorf("unexpected list self link: %#v", listOut) + } + expectedSelfLink := "/prefix/version/simple/something?namespace=other" + if listOut.Items[0].ObjectMeta.SelfLink != expectedSelfLink { + t.Errorf("Unexpected data: %#v, %s", listOut.Items[0].ObjectMeta.SelfLink, expectedSelfLink) + } +} + +func TestSelfLinkSkipsEmptyName(t *testing.T) { + storage := map[string]RESTStorage{} + simpleStorage := SimpleRESTStorage{ + list: []Simple{ + { ObjectMeta: api.ObjectMeta{Namespace: "other"}, Other: "foo", }, @@ -452,7 +541,7 @@ func TestNonEmptyList(t *testing.T) { resp, err := http.Get(server.URL + "/prefix/version/simple") if err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v", err) } if resp.StatusCode != http.StatusOK { @@ -460,11 +549,10 @@ func TestNonEmptyList(t *testing.T) { body, _ := ioutil.ReadAll(resp.Body) t.Logf("Data: %s", string(body)) } - var listOut SimpleList body, err := extractBody(resp, &listOut) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v", err) } if len(listOut.Items) != 1 { @@ -474,7 +562,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)) } - expectedSelfLink := "/prefix/version/simple?namespace=other" + if listOut.SelfLink != "/prefix/version/simple?namespace=" { + t.Errorf("unexpected list self link: %#v", listOut) + } + expectedSelfLink := "" if listOut.Items[0].ObjectMeta.SelfLink != expectedSelfLink { t.Errorf("Unexpected data: %#v, %s", listOut.Items[0].ObjectMeta.SelfLink, expectedSelfLink) } @@ -817,6 +908,48 @@ func TestUpdateAllowsMissingNamespace(t *testing.T) { } } +// when the object name and namespace can't be retrieved, skip name checking +func TestUpdateAllowsMismatchedNamespaceOnError(t *testing.T) { + storage := map[string]RESTStorage{} + simpleStorage := SimpleRESTStorage{} + ID := "id" + storage["simple"] = &simpleStorage + selfLinker := &setTestSelfLinker{ + t: t, + err: fmt.Errorf("test error"), + } + handler := Handle(storage, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + server := httptest.NewServer(handler) + defer server.Close() + + item := &Simple{ + ObjectMeta: api.ObjectMeta{ + Name: ID, + Namespace: "other", // does not match request + }, + Other: "bar", + } + body, err := codec.Encode(item) + if err != nil { + // The following cases will fail, so die now + t.Fatalf("unexpected error: %v", err) + } + + client := http.Client{} + request, err := http.NewRequest("PUT", server.URL+"/prefix/version/simple/"+ID, bytes.NewReader(body)) + _, err = client.Do(request) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if simpleStorage.updated == nil || simpleStorage.updated.Name != item.Name { + t.Errorf("Unexpected update value %#v, expected %#v.", simpleStorage.updated, item) + } + if selfLinker.called { + t.Errorf("self link ignored") + } +} + func TestUpdatePreventsMismatchedNamespace(t *testing.T) { storage := map[string]RESTStorage{} simpleStorage := SimpleRESTStorage{} @@ -931,20 +1064,79 @@ type setTestSelfLinker struct { name string namespace string called bool + err error } -func (s *setTestSelfLinker) Namespace(runtime.Object) (string, error) { return s.namespace, nil } -func (s *setTestSelfLinker) Name(runtime.Object) (string, error) { return s.name, nil } -func (*setTestSelfLinker) SelfLink(runtime.Object) (string, error) { return "", nil } +func (s *setTestSelfLinker) Namespace(runtime.Object) (string, error) { return s.namespace, s.err } +func (s *setTestSelfLinker) Name(runtime.Object) (string, error) { return s.name, s.err } +func (s *setTestSelfLinker) SelfLink(runtime.Object) (string, error) { return "", s.err } func (s *setTestSelfLinker) SetSelfLink(obj runtime.Object, selfLink string) error { if e, a := s.expectedSet, selfLink; e != a { s.t.Errorf("expected '%v', got '%v'", e, a) } s.called = true - return nil + return s.err } func TestCreate(t *testing.T) { + storage := SimpleRESTStorage{ + injectedFunction: func(obj runtime.Object) (runtime.Object, error) { + time.Sleep(5 * time.Millisecond) + return obj, nil + }, + } + selfLinker := &setTestSelfLinker{ + t: t, + name: "bar", + namespace: "default", + expectedSet: "/prefix/version/foo/bar?namespace=default", + } + handler := Handle(map[string]RESTStorage{ + "foo": &storage, + }, codec, "/prefix", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + server := httptest.NewServer(handler) + defer server.Close() + client := http.Client{} + + simple := &Simple{ + Other: "bar", + } + data, _ := codec.Encode(simple) + request, err := http.NewRequest("POST", server.URL+"/prefix/version/foo", bytes.NewBuffer(data)) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + wg := sync.WaitGroup{} + wg.Add(1) + var response *http.Response + go func() { + response, err = client.Do(request) + wg.Done() + }() + wg.Wait() + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + var itemOut Simple + body, err := extractBody(response, &itemOut) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if !reflect.DeepEqual(&itemOut, simple) { + t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, simple, string(body)) + } + if response.StatusCode != http.StatusCreated { + t.Errorf("Unexpected status: %d, Expected: %d, %#v", response.StatusCode, http.StatusOK, response) + } + if !selfLinker.called { + t.Errorf("Never set self link") + } +} + +func TestCreateInNamespace(t *testing.T) { storage := SimpleRESTStorage{ injectedFunction: func(obj runtime.Object) (runtime.Object, error) { time.Sleep(5 * time.Millisecond) diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index 7e052811b51..496e6a253ee 100644 --- a/pkg/apiserver/resthandler.go +++ b/pkg/apiserver/resthandler.go @@ -18,6 +18,7 @@ package apiserver import ( "net/http" + gpath "path" "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/admission" @@ -27,66 +28,70 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/emicklei/go-restful" + "github.com/golang/glog" ) // ContextFunc returns a Context given a request - a context must be returned type ContextFunc func(req *restful.Request) api.Context -// ResourceNameFunc returns a name (and optional namespace) given a request - if no name is present -// an error must be returned. -type ResourceNameFunc func(req *restful.Request) (namespace, name string, err error) - -// ObjectNameFunc returns the name (and optional namespace) of an object -type ObjectNameFunc func(obj runtime.Object) (namespace, name string, err error) - -// ResourceNamespaceFunc returns the namespace associated with the given request - if no namespace -// is present an error must be returned. -type ResourceNamespaceFunc func(req *restful.Request) (namespace string, err error) - -// LinkResourceFunc updates the provided object with a SelfLink that is appropriate for the current -// request. -type LinkResourceFunc func(req *restful.Request, obj runtime.Object) error +// ScopeNamer handles accessing names from requests and objects +type ScopeNamer interface { + // Namespace returns the appropriate namespace value from the request (may be empty) or an + // error. + Namespace(req *restful.Request) (namespace string, err error) + // Name returns the name from the request, and an optional namespace value if this is a namespace + // scoped call. An error is returned if the name is not available. + Name(req *restful.Request) (namespace, name string, err error) + // ObjectName returns the namespace and name from an object if they exist, or an error if the object + // does not support names. + ObjectName(obj runtime.Object) (namespace, name string, err error) + // SetSelfLink sets the provided URL onto the object. The method should return nil if the object + // does not support selfLinks. + SetSelfLink(obj runtime.Object, url string) error + // GenerateLink creates a path and query for a given runtime object that represents the canonical path. + GenerateLink(req *restful.Request, obj runtime.Object) (path, query string, err error) + // GenerateLink creates a path and query for a list that represents the canonical path. + GenerateListLink(req *restful.Request) (path, query string, err error) +} // GetResource returns a function that handles retrieving a single resource from a RESTStorage object. -func GetResource(r RESTGetter, ctxFn ContextFunc, nameFn ResourceNameFunc, linkFn LinkResourceFunc, codec runtime.Codec) restful.RouteFunction { +func GetResource(r RESTGetter, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { w := res.ResponseWriter - namespace, name, err := nameFn(req) + namespace, name, err := namer.Name(req) if err != nil { notFound(w, req.Request) return } ctx := ctxFn(req) - if len(namespace) > 0 { - ctx = api.WithNamespace(ctx, namespace) - } - item, err := r.Get(ctx, name) + ctx = api.WithNamespace(ctx, namespace) + + result, err := r.Get(ctx, name) if err != nil { errorJSON(err, codec, w) return } - if err := linkFn(req, item); err != nil { + if err := setSelfLink(result, req, namer); err != nil { errorJSON(err, codec, w) return } - writeJSON(http.StatusOK, codec, item, w) + writeJSON(http.StatusOK, codec, result, w) } } // ListResource returns a function that handles retrieving a list of resources from a RESTStorage object. -func ListResource(r RESTLister, ctxFn ContextFunc, namespaceFn ResourceNamespaceFunc, linkFn LinkResourceFunc, codec runtime.Codec) restful.RouteFunction { +func ListResource(r RESTLister, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { w := res.ResponseWriter - namespace, err := namespaceFn(req) + namespace, err := namer.Namespace(req) if err != nil { notFound(w, req.Request) return } ctx := ctxFn(req) - if len(namespace) > 0 { - ctx = api.WithNamespace(ctx, namespace) - } + ctx = api.WithNamespace(ctx, namespace) + label, err := labels.ParseSelector(req.Request.URL.Query().Get("labels")) if err != nil { errorJSON(err, codec, w) @@ -98,36 +103,34 @@ func ListResource(r RESTLister, ctxFn ContextFunc, namespaceFn ResourceNamespace return } - item, err := r.List(ctx, label, field) + result, err := r.List(ctx, label, field) if err != nil { errorJSON(err, codec, w) return } - if err := linkFn(req, item); err != nil { + if err := setListSelfLink(result, req, namer); err != nil { errorJSON(err, codec, w) return } - writeJSON(http.StatusOK, codec, item, w) + writeJSON(http.StatusOK, codec, result, w) } } // CreateResource returns a function that will handle a resource creation. -func CreateResource(r RESTCreater, ctxFn ContextFunc, namespaceFn ResourceNamespaceFunc, linkFn LinkResourceFunc, codec runtime.Codec, resource string, admit admission.Interface) restful.RouteFunction { +func CreateResource(r RESTCreater, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, resource string, admit admission.Interface) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { w := res.ResponseWriter // TODO: we either want to remove timeout or document it (if we document, move timeout out of this function and declare it in api_installer) timeout := parseTimeout(req.Request.URL.Query().Get("timeout")) - namespace, err := namespaceFn(req) + namespace, err := namer.Namespace(req) if err != nil { notFound(w, req.Request) return } ctx := ctxFn(req) - if len(namespace) > 0 { - ctx = api.WithNamespace(ctx, namespace) - } + ctx = api.WithNamespace(ctx, namespace) body, err := readBody(req.Request) if err != nil { @@ -159,7 +162,7 @@ func CreateResource(r RESTCreater, ctxFn ContextFunc, namespaceFn ResourceNamesp return } - if err := linkFn(req, result); err != nil { + if err := setSelfLink(result, req, namer); err != nil { errorJSON(err, codec, w) return } @@ -169,22 +172,20 @@ func CreateResource(r RESTCreater, ctxFn ContextFunc, namespaceFn ResourceNamesp } // UpdateResource returns a function that will handle a resource update -func UpdateResource(r RESTUpdater, ctxFn ContextFunc, nameFn ResourceNameFunc, objNameFunc ObjectNameFunc, linkFn LinkResourceFunc, codec runtime.Codec, resource string, admit admission.Interface) restful.RouteFunction { +func UpdateResource(r RESTUpdater, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, resource string, admit admission.Interface) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { w := res.ResponseWriter // TODO: we either want to remove timeout or document it (if we document, move timeout out of this function and declare it in api_installer) timeout := parseTimeout(req.Request.URL.Query().Get("timeout")) - namespace, name, err := nameFn(req) + namespace, name, err := namer.Name(req) if err != nil { notFound(w, req.Request) return } ctx := ctxFn(req) - if len(namespace) > 0 { - ctx = api.WithNamespace(ctx, namespace) - } + ctx = api.WithNamespace(ctx, namespace) body, err := readBody(req.Request) if err != nil { @@ -198,20 +199,18 @@ func UpdateResource(r RESTUpdater, ctxFn ContextFunc, nameFn ResourceNameFunc, o return } - objNamespace, objName, err := objNameFunc(obj) - if err != nil { - errorJSON(err, codec, w) - return - } - if objName != name { - errorJSON(errors.NewBadRequest("the name of the object does not match the name on the URL"), codec, w) - return - } - if len(namespace) > 0 { - if len(objNamespace) > 0 && objNamespace != namespace { - errorJSON(errors.NewBadRequest("the namespace of the object does not match the namespace on the request"), codec, w) + // check the provided name against the request + if objNamespace, objName, err := namer.ObjectName(obj); err == nil { + if objName != name { + errorJSON(errors.NewBadRequest("the name of the object does not match the name on the URL"), codec, w) return } + if len(namespace) > 0 { + if len(objNamespace) > 0 && objNamespace != namespace { + errorJSON(errors.NewBadRequest("the namespace of the object does not match the namespace on the request"), codec, w) + return + } + } } err = admit.Admit(admission.NewAttributesRecord(obj, namespace, resource, "UPDATE")) @@ -231,7 +230,7 @@ func UpdateResource(r RESTUpdater, ctxFn ContextFunc, nameFn ResourceNameFunc, o return } - if err := linkFn(req, result); err != nil { + if err := setSelfLink(result, req, namer); err != nil { errorJSON(err, codec, w) return } @@ -245,14 +244,14 @@ func UpdateResource(r RESTUpdater, ctxFn ContextFunc, nameFn ResourceNameFunc, o } // DeleteResource returns a function that will handle a resource deletion -func DeleteResource(r RESTDeleter, ctxFn ContextFunc, nameFn ResourceNameFunc, linkFn LinkResourceFunc, codec runtime.Codec, resource, kind string, admit admission.Interface) restful.RouteFunction { +func DeleteResource(r RESTDeleter, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, resource, kind string, admit admission.Interface) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { w := res.ResponseWriter // TODO: we either want to remove timeout or document it (if we document, move timeout out of this function and declare it in api_installer) timeout := parseTimeout(req.Request.URL.Query().Get("timeout")) - namespace, name, err := nameFn(req) + namespace, name, err := namer.Name(req) if err != nil { notFound(w, req.Request) return @@ -290,7 +289,7 @@ func DeleteResource(r RESTDeleter, ctxFn ContextFunc, nameFn ResourceNameFunc, l } else { // when a non-status response is returned, set the self link if _, ok := result.(*api.Status); !ok { - if err := linkFn(req, result); err != nil { + if err := setSelfLink(result, req, namer); err != nil { errorJSON(err, codec, w) return } @@ -329,42 +328,58 @@ func finishRequest(timeout time.Duration, fn resultFunc) (result runtime.Object, } } -type linkFunc func(namespace, name string) (path string, query string) - // 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 *http.Request, linker runtime.SelfLinker, fn linkFunc) error { - namespace, err := linker.Namespace(obj) +func setSelfLink(obj runtime.Object, req *restful.Request, namer ScopeNamer) error { + // TODO: SelfLink generation should return a full URL? + path, query, err := namer.GenerateLink(req, obj) + if err == errEmptyName { + return nil + } if err != nil { return err } - name, err := linker.Name(obj) - if err != nil { - return err - } - path, query := fn(namespace, name) - newURL := *req.URL - newURL.Path = path + newURL := *req.Request.URL + // use only canonical paths + newURL.Path = gpath.Clean(path) newURL.RawQuery = query newURL.Fragment = "" - if err := linker.SetSelfLink(obj, newURL.String()); err != nil { - return err - } + return namer.SetSelfLink(obj, newURL.String()) +} + +// setListSelfLink sets the self link of a list to the base URL, then sets the self links +// on all child objects returned. +func setListSelfLink(obj runtime.Object, req *restful.Request, namer ScopeNamer) error { if !runtime.IsListType(obj) { return nil } + // TODO: List SelfLink generation should return a full URL? + path, query, err := namer.GenerateListLink(req) + if err != nil { + return err + } + newURL := *req.Request.URL + newURL.Path = path + newURL.RawQuery = query + // use the path that got us here + newURL.Fragment = "" + if err := namer.SetSelfLink(obj, newURL.String()); err != nil { + glog.V(4).Infof("Unable to set self link on object: %v", err) + } + // Set self-link of objects in the list. items, err := runtime.ExtractList(obj) if err != nil { return err } for i := range items { - if err := setSelfLink(items[i], req, linker, fn); err != nil { + if err := setSelfLink(items[i], req, namer); err != nil { return err } } return runtime.SetList(obj, items) + }