Merge pull request #9600 from liggitt/remove_namespace_param

pkg/apiserver/api_installer.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"net"
 	"net/http"
-	"net/url"
 	gpath "path"
 	"reflect"
 	"sort"
@@ -282,7 +281,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		actions = appendIf(actions, action{"CONNECT", itemPath + "/{path:*}", nameParams, namer}, isConnecter && connectSubpath)
 
 	} else {
-		// v1beta3 format with namespace in path
+		// 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")
@@ -325,42 +324,9 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 			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)
-
 		} 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}
-
-			basePath := resource
-			resourcePath := basePath
-			resourceParams := namespaceParams
-			itemPath := resourcePath + "/{name}"
-			nameParams := append(namespaceParams, nameParam)
-			proxyParams := append(nameParams, pathParam)
-			if hasSubresource {
-				itemPath = itemPath + "/" + subresource
-				resourcePath = itemPath
-				resourceParams = nameParams
-			}
-			namer := legacyScopeNaming{scope, a.group.Linker, gpath.Join(a.prefix, itemPath)}
-
-			actions = appendIf(actions, action{"LIST", resourcePath, resourceParams, namer}, isLister)
-			actions = appendIf(actions, action{"POST", resourcePath, resourceParams, namer}, isCreater)
-			actions = appendIf(actions, action{"WATCHLIST", "watch/" + resourcePath, resourceParams, namer}, allowWatchList)
-
-			actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, isGetter)
-			if getSubpath {
-				actions = appendIf(actions, action{"GET", itemPath + "/{path:*}", proxyParams, namer}, isGetter)
-			}
-			actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, isUpdater)
-			actions = appendIf(actions, action{"PATCH", itemPath, nameParams, namer}, isPatcher)
-			actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, isDeleter)
-			actions = appendIf(actions, action{"WATCH", "watch/" + itemPath, nameParams, namer}, isWatcher)
-			actions = appendIf(actions, action{"PROXY", "proxy/" + itemPath + "/{path:*}", proxyParams, namer}, isRedirector)
-			actions = appendIf(actions, action{"PROXY", "proxy/" + itemPath, nameParams, namer}, isRedirector)
-			actions = appendIf(actions, action{"CONNECT", itemPath, nameParams, namer}, isConnecter)
-			actions = appendIf(actions, action{"CONNECT", itemPath + "/{path:*}", nameParams, namer}, isConnecter && connectSubpath)
+			// Namespace as param is no longer supported
+			return fmt.Errorf("namespace as a parameter is no longer supported")
 		}
 	}
 
@@ -711,6 +677,9 @@ func (n scopeNaming) GenerateLink(req *restful.Request, obj runtime.Object) (pat
 			return "", "", err
 		}
 	}
+	if len(name) == 0 {
+		return "", "", errEmptyName
+	}
 	path = strings.Replace(n.itemPath, "{name}", name, 1)
 	path = strings.Replace(path, "{"+n.scope.ParamName()+"}", namespace, 1)
 	return path, "", nil
@@ -737,88 +706,6 @@ func (n scopeNaming) ObjectName(obj runtime.Object) (namespace, name string, 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) {
-	if n.scope.Name() == meta.RESTScopeNameRoot {
-		return api.NamespaceNone, nil
-	}
-	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()

pkg/apiserver/handlers.go

@@ -267,8 +267,6 @@ type APIRequestInfoResolver struct {
 // /namespaces/{namespace}/{resource}/{resourceName}
 // /{resource}
 // /{resource}/{resourceName}
-// /{resource}/{resourceName}?namespace={namespace}
-// /{resource}?namespace={namespace}
 //
 // Special verbs:
 // /proxy/{resource}/{resourceName}
@@ -341,18 +339,7 @@ func (r *APIRequestInfoResolver) GetAPIRequestInfo(req *http.Request) (APIReques
 			}
 		}
 	} else {
-		// URL forms: /{resource}/*
-		// URL forms: POST /{resource} is a legacy API convention to create in "default" namespace
-		// URL forms: /{resource}/{resourceName} use the "default" namespace if omitted from query param
-		// URL forms: /{resource} assume cross-namespace operation if omitted from query param
-		requestInfo.Namespace = req.URL.Query().Get("namespace")
-		if len(requestInfo.Namespace) == 0 {
-			if len(currentParts) > 1 || req.Method == "POST" {
-				requestInfo.Namespace = api.NamespaceDefault
-			} else {
-				requestInfo.Namespace = api.NamespaceAll
-			}
-		}
+		requestInfo.Namespace = api.NamespaceNone
 	}
 
 	// parsing successful, so we now know the proper value for .Parts

pkg/apiserver/handlers_test.go

@@ -163,16 +163,12 @@ func TestGetAPIRequestInfo(t *testing.T) {
 		{"GET", "/namespaces/other/pods", "list", "", "other", "pods", "", "Pod", "", []string{"pods"}},
 		{"GET", "/namespaces/other/pods/foo", "get", "", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}},
 		{"GET", "/pods", "list", "", api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}},
-		{"POST", "/pods", "create", "", api.NamespaceDefault, "pods", "", "Pod", "", []string{"pods"}},
-		{"GET", "/pods/foo", "get", "", api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}},
-		{"GET", "/pods/foo?namespace=other", "get", "", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}},
-		{"GET", "/pods?namespace=other", "list", "", "other", "pods", "", "Pod", "", []string{"pods"}},
+		{"GET", "/namespaces/other/pods/foo", "get", "", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}},
+		{"GET", "/namespaces/other/pods", "list", "", "other", "pods", "", "Pod", "", []string{"pods"}},
 
 		// special verbs
 		{"GET", "/proxy/namespaces/other/pods/foo", "proxy", "", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}},
-		{"GET", "/proxy/pods/foo", "proxy", "", api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}},
 		{"GET", "/redirect/namespaces/other/pods/foo", "redirect", "", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}},
-		{"GET", "/redirect/pods/foo", "redirect", "", api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}},
 		{"GET", "/watch/pods", "watch", "", api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}},
 		{"GET", "/watch/namespaces/other/pods", "watch", "", "other", "pods", "", "Pod", "", []string{"pods"}},
 
@@ -180,9 +176,9 @@ func TestGetAPIRequestInfo(t *testing.T) {
 		{"GET", getPath("pods", "other", ""), "list", testapi.Version(), "other", "pods", "", "Pod", "", []string{"pods"}},
 		{"GET", getPath("pods", "other", "foo"), "get", testapi.Version(), "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}},
 		{"GET", getPath("pods", "", ""), "list", testapi.Version(), api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}},
-		{"POST", getPath("pods", "", ""), "create", testapi.Version(), api.NamespaceDefault, "pods", "", "Pod", "", []string{"pods"}},
-		{"GET", getPath("pods", "", "foo"), "get", testapi.Version(), api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}},
-		{"GET", pathWithPrefix("proxy", "pods", "", "foo"), "proxy", testapi.Version(), api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}},
+		{"POST", getPath("pods", "", ""), "create", testapi.Version(), api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}},
+		{"GET", getPath("pods", "", "foo"), "get", testapi.Version(), api.NamespaceAll, "pods", "", "Pod", "foo", []string{"pods", "foo"}},
+		{"GET", pathWithPrefix("proxy", "pods", "", "foo"), "proxy", testapi.Version(), api.NamespaceAll, "pods", "", "Pod", "foo", []string{"pods", "foo"}},
 		{"GET", pathWithPrefix("watch", "pods", "", ""), "watch", testapi.Version(), api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}},
 		{"GET", pathWithPrefix("redirect", "pods", "", ""), "redirect", testapi.Version(), api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}},
 		{"GET", pathWithPrefix("watch", "pods", "other", ""), "watch", testapi.Version(), "other", "pods", "", "Pod", "", []string{"pods"}},

pkg/apiserver/proxy_test.go

@@ -86,9 +86,6 @@ func TestProxy(t *testing.T) {
 		namespaceHandler := handleNamespaced(map[string]rest.Storage{"foo": simpleStorage})
 		namespaceServer := httptest.NewServer(namespaceHandler)
 		defer namespaceServer.Close()
-		legacyNamespaceHandler := handle(map[string]rest.Storage{"foo": simpleStorage})
-		legacyNamespaceServer := httptest.NewServer(legacyNamespaceHandler)
-		defer legacyNamespaceServer.Close()
 
 		// test each supported URL pattern for finding the redirection resource in the proxy in a particular namespace
 		serverPatterns := []struct {
@@ -96,7 +93,6 @@ func TestProxy(t *testing.T) {
 			proxyTestPattern string
 		}{
 			{namespaceServer, "/api/version2/proxy/namespaces/" + item.reqNamespace + "/foo/id" + item.path},
-			{legacyNamespaceServer, "/api/version/proxy/foo/id" + item.path + "?namespace=" + item.reqNamespace},
 		}
 
 		for _, serverPattern := range serverPatterns {

pkg/apiserver/watch_test.go

@@ -166,14 +166,20 @@ func TestWatchHTTP(t *testing.T) {
 
 func TestWatchParamParsing(t *testing.T) {
 	simpleStorage := &SimpleRESTStorage{}
-	handler := handle(map[string]rest.Storage{"simples": simpleStorage})
+	handler := handle(map[string]rest.Storage{
+		"simples":     simpleStorage,
+		"simpleroots": simpleStorage,
+	})
 	server := httptest.NewServer(handler)
 	defer server.Close()
 
 	dest, _ := url.Parse(server.URL)
-	dest.Path = "/api/" + testVersion + "/watch/simples"
+
+	rootPath := "/api/" + testVersion + "/watch/simples"
+	namespacedPath := "/api/" + testVersion + "/watch/namespaces/other/simpleroots"
 
 	table := []struct {
+		path            string
 		rawQuery        string
 		resourceVersion string
 		labelSelector   string
@@ -181,30 +187,63 @@ func TestWatchParamParsing(t *testing.T) {
 		namespace       string
 	}{
 		{
+			path:            rootPath,
 			rawQuery:        "resourceVersion=1234",
 			resourceVersion: "1234",
 			labelSelector:   "",
 			fieldSelector:   "",
 			namespace:       api.NamespaceAll,
 		}, {
-			rawQuery:        "namespace=default&resourceVersion=314159&fields=Host%3D&labels=name%3Dfoo",
+			path:            rootPath,
+			rawQuery:        "resourceVersion=314159&fields=Host%3D&labels=name%3Dfoo",
 			resourceVersion: "314159",
 			labelSelector:   "name=foo",
 			fieldSelector:   "Host=",
-			namespace:       api.NamespaceDefault,
+			namespace:       api.NamespaceAll,
 		}, {
-			rawQuery:        "namespace=watchother&fields=id%3dfoo&resourceVersion=1492",
+			path:            rootPath,
+			rawQuery:        "fields=id%3dfoo&resourceVersion=1492",
 			resourceVersion: "1492",
 			labelSelector:   "",
 			fieldSelector:   "id=foo",
-			namespace:       "watchother",
+			namespace:       api.NamespaceAll,
 		}, {
+			path:            rootPath,
 			rawQuery:        "",
 			resourceVersion: "",
 			labelSelector:   "",
 			fieldSelector:   "",
 			namespace:       api.NamespaceAll,
 		},
+		{
+			path:            namespacedPath,
+			rawQuery:        "resourceVersion=1234",
+			resourceVersion: "1234",
+			labelSelector:   "",
+			fieldSelector:   "",
+			namespace:       "other",
+		}, {
+			path:            namespacedPath,
+			rawQuery:        "resourceVersion=314159&fields=Host%3D&labels=name%3Dfoo",
+			resourceVersion: "314159",
+			labelSelector:   "name=foo",
+			fieldSelector:   "Host=",
+			namespace:       "other",
+		}, {
+			path:            namespacedPath,
+			rawQuery:        "fields=id%3dfoo&resourceVersion=1492",
+			resourceVersion: "1492",
+			labelSelector:   "",
+			fieldSelector:   "id=foo",
+			namespace:       "other",
+		}, {
+			path:            namespacedPath,
+			rawQuery:        "",
+			resourceVersion: "",
+			labelSelector:   "",
+			fieldSelector:   "",
+			namespace:       "other",
+		},
 	}
 
 	for _, item := range table {
@@ -212,6 +251,7 @@ func TestWatchParamParsing(t *testing.T) {
 		simpleStorage.requestedFieldSelector = fields.Everything()
 		simpleStorage.requestedResourceVersion = "5" // Prove this is set in all cases
 		simpleStorage.requestedResourceNamespace = ""
+		dest.Path = item.path
 		dest.RawQuery = item.rawQuery
 		resp, err := http.Get(dest.String())
 		if err != nil {