Merge pull request #43767 from deads2k/server-13-namer

Automatic merge from submit-queue

remove go-restful from namer for rest handling

Our RESTHandler code is currently tightly coupled to go-restful, but there's no reason for this coupling.  It makes integrations that want API handling (decode, sanity check, admission, verb handling), but don't need the REST installer flow impractical.  I know of two layers now: metrics and TPR.

This starts the process of unwinding by switching the `ScopeNamer` (used for request identification and selflinks) to use the standard http library along with the `RequestInfo` we place in the context for authorization and any other interested layer.

@kubernetes/sig-api-machinery-misc @smarterclayton @ncdc @sttts

staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go

@@ -26,6 +26,7 @@ import (
 	"math/rand"
 	"net/http"
 	"net/http/httptest"
+	"net/http/httputil"
 	"net/url"
 	"reflect"
 	"strconv"
@@ -331,7 +332,17 @@ func handleInternal(storage map[string]rest.Storage, admissionControl admission.
 		}
 	}
 
-	return &defaultAPIServer{mux, container}
+	handler := genericapifilters.WithRequestInfo(mux, testRequestInfoResolver(), requestContextMapper)
+	handler = request.WithRequestContext(handler, requestContextMapper)
+
+	return &defaultAPIServer{handler, container}
+}
+
+func testRequestInfoResolver() *request.RequestInfoFactory {
+	return &request.RequestInfoFactory{
+		APIPrefixes:          sets.NewString("api", "apis"),
+		GrouplessAPIPrefixes: sets.NewString("api"),
+	}
 }
 
 func TestSimpleSetupRight(t *testing.T) {
@@ -746,7 +757,7 @@ func TestNotFound(t *testing.T) {
 		"groupless root DELETE with extra segment":    {"DELETE", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/simpleroots/bar/baz", http.StatusNotFound},
 		"groupless root PUT without extra segment":    {"PUT", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/simpleroots", http.StatusMethodNotAllowed},
 		"groupless root PUT with extra segment":       {"PUT", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/simpleroots/bar/baz", http.StatusNotFound},
-		"groupless root watch missing storage":        {"GET", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/watch/", http.StatusNotFound},
+		"groupless root watch missing storage":        {"GET", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/watch/", http.StatusInternalServerError},
 
 		"groupless namespaced PATCH method":                 {"PATCH", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/ns/simples", http.StatusMethodNotAllowed},
 		"groupless namespaced GET long prefix":              {"GET", "/" + grouplessPrefix + "/", http.StatusNotFound},
@@ -757,7 +768,7 @@ func TestNotFound(t *testing.T) {
 		"groupless namespaced DELETE with extra segment":    {"DELETE", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/ns/simples/bar/baz", http.StatusNotFound},
 		"groupless namespaced PUT without extra segment":    {"PUT", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/ns/simples", http.StatusMethodNotAllowed},
 		"groupless namespaced PUT with extra segment":       {"PUT", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/ns/simples/bar/baz", http.StatusNotFound},
-		"groupless namespaced watch missing storage":        {"GET", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/watch/", http.StatusNotFound},
+		"groupless namespaced watch missing storage":        {"GET", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/watch/", http.StatusInternalServerError},
 		"groupless namespaced watch with bad method":        {"POST", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/watch/namespaces/ns/simples/bar", http.StatusMethodNotAllowed},
 		"groupless namespaced watch param with bad method":  {"POST", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/ns/simples/bar?watch=true", http.StatusMethodNotAllowed},
 
@@ -777,7 +788,7 @@ func TestNotFound(t *testing.T) {
 		"root DELETE with extra segment":    {"DELETE", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/simpleroots/bar/baz", http.StatusNotFound},
 		"root PUT without extra segment":    {"PUT", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/simpleroots", http.StatusMethodNotAllowed},
 		"root PUT with extra segment":       {"PUT", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/simpleroots/bar/baz", http.StatusNotFound},
-		"root watch missing storage":        {"GET", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/", http.StatusNotFound},
+		"root watch missing storage":        {"GET", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/", http.StatusInternalServerError},
 		// TODO: JTL: "root watch with bad method":        {"POST", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/simpleroot/bar", http.StatusMethodNotAllowed},
 
 		"namespaced PATCH method":                 {"PATCH", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/ns/simples", http.StatusMethodNotAllowed},
@@ -789,7 +800,7 @@ func TestNotFound(t *testing.T) {
 		"namespaced DELETE with extra segment":    {"DELETE", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/ns/simples/bar/baz", http.StatusNotFound},
 		"namespaced PUT without extra segment":    {"PUT", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/ns/simples", http.StatusMethodNotAllowed},
 		"namespaced PUT with extra segment":       {"PUT", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/ns/simples/bar/baz", http.StatusNotFound},
-		"namespaced watch missing storage":        {"GET", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/", http.StatusNotFound},
+		"namespaced watch missing storage":        {"GET", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/", http.StatusInternalServerError},
 		"namespaced watch with bad method":        {"POST", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/namespaces/ns/simples/bar", http.StatusMethodNotAllowed},
 		"namespaced watch param with bad method":  {"POST", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/ns/simples/bar?watch=true", http.StatusMethodNotAllowed},
 	}
@@ -1081,7 +1092,7 @@ func TestList(t *testing.T) {
 		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)
+			t.Errorf("%d: %q unexpected resource namespace: %s", i, testCase.url, simpleStorage.actualNamespace)
 		}
 		if simpleStorage.requestedLabelSelector == nil || simpleStorage.requestedLabelSelector.String() != testCase.label {
 			t.Errorf("%d: unexpected label selector: %v", i, simpleStorage.requestedLabelSelector)
@@ -1169,6 +1180,7 @@ func TestNonEmptyList(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
+	t.Log(body)
 
 	if len(listOut.Items) != 1 {
 		t.Errorf("Unexpected response: %#v", listOut)
@@ -2220,10 +2232,12 @@ func TestPatch(t *testing.T) {
 	client := http.Client{}
 	request, err := http.NewRequest("PATCH", server.URL+"/"+prefix+"/"+testGroupVersion.Group+"/"+testGroupVersion.Version+"/namespaces/default/simple/"+ID, bytes.NewReader([]byte(`{"labels":{"foo":"bar"}}`)))
 	request.Header.Set("Content-Type", "application/merge-patch+json; charset=UTF-8")
-	_, err = client.Do(request)
+	response, err := client.Do(request)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
+	dump, _ := httputil.DumpResponse(response, true)
+	t.Log(string(dump))
 
 	if simpleStorage.updated == nil || simpleStorage.updated.Labels["foo"] != "bar" {
 		t.Errorf("Unexpected update value %#v, expected %#v.", simpleStorage.updated, item)
@@ -2292,10 +2306,12 @@ func TestUpdate(t *testing.T) {
 
 	client := http.Client{}
 	request, err := http.NewRequest("PUT", server.URL+"/"+prefix+"/"+testGroupVersion.Group+"/"+testGroupVersion.Version+"/namespaces/default/simple/"+ID, bytes.NewReader(body))
-	_, err = client.Do(request)
+	response, err := client.Do(request)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
+	dump, _ := httputil.DumpResponse(response, true)
+	t.Log(string(dump))
 
 	if simpleStorage.updated == nil || simpleStorage.updated.Name != item.Name {
 		t.Errorf("Unexpected update value %#v, expected %#v.", simpleStorage.updated, item)
@@ -2333,6 +2349,9 @@ func TestUpdateInvokesAdmissionControl(t *testing.T) {
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
+	dump, _ := httputil.DumpResponse(response, true)
+	t.Log(string(dump))
+
 	if response.StatusCode != http.StatusForbidden {
 		t.Errorf("Unexpected response %#v", response)
 	}
@@ -2343,7 +2362,7 @@ func TestUpdateRequiresMatchingName(t *testing.T) {
 	simpleStorage := SimpleRESTStorage{}
 	ID := "id"
 	storage["simple"] = &simpleStorage
-	handler := handleDeny(storage)
+	handler := handle(storage)
 	server := httptest.NewServer(handler)
 	defer server.Close()
 
@@ -2363,6 +2382,8 @@ func TestUpdateRequiresMatchingName(t *testing.T) {
 		t.Errorf("unexpected error: %v", err)
 	}
 	if response.StatusCode != http.StatusBadRequest {
+		dump, _ := httputil.DumpResponse(response, true)
+		t.Log(string(dump))
 		t.Errorf("Unexpected response %#v", response)
 	}
 }
@@ -2394,13 +2415,16 @@ func TestUpdateAllowsMissingNamespace(t *testing.T) {
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
+	dump, _ := httputil.DumpResponse(response, true)
+	t.Log(string(dump))
+
 	if response.StatusCode != http.StatusOK {
 		t.Errorf("Unexpected response %#v", response)
 	}
 }
 
-// when the object name and namespace can't be retrieved, skip name checking
+// when the object name and namespace can't be retrieved, don't update.  It isn't safe.
-func TestUpdateAllowsMismatchedNamespaceOnError(t *testing.T) {
+func TestUpdateDisallowsMismatchedNamespaceOnError(t *testing.T) {
 	storage := map[string]rest.Storage{}
 	simpleStorage := SimpleRESTStorage{}
 	ID := "id"
@@ -2428,13 +2452,15 @@ func TestUpdateAllowsMismatchedNamespaceOnError(t *testing.T) {
 
 	client := http.Client{}
 	request, err := http.NewRequest("PUT", server.URL+"/"+prefix+"/"+testGroupVersion.Group+"/"+testGroupVersion.Version+"/namespaces/default/simple/"+ID, bytes.NewReader(body))
-	_, err = client.Do(request)
+	response, err := client.Do(request)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
+	dump, _ := httputil.DumpResponse(response, true)
+	t.Log(string(dump))
 
-	if simpleStorage.updated == nil || simpleStorage.updated.Name != item.Name {
+	if simpleStorage.updated != nil {
-		t.Errorf("Unexpected update value %#v, expected %#v.", simpleStorage.updated, item)
+		t.Errorf("Unexpected update value %#v.", simpleStorage.updated)
 	}
 	if selfLinker.called {
 		t.Errorf("self link ignored")
@@ -2605,14 +2631,17 @@ func TestUpdateREST(t *testing.T) {
 	}
 
 	testREST := func(t *testing.T, container *restful.Container, barCode int) {
+		handler := genericapifilters.WithRequestInfo(container, newTestRequestInfoResolver(), requestContextMapper)
+		handler = request.WithRequestContext(handler, requestContextMapper)
+
 		w := httptest.NewRecorder()
-		container.ServeHTTP(w, &http.Request{Method: "GET", URL: &url.URL{Path: "/" + prefix + "/" + newGroupVersion.Group + "/" + newGroupVersion.Version + "/namespaces/test/foo/test"}})
+		handler.ServeHTTP(w, &http.Request{Method: "GET", URL: &url.URL{Path: "/" + prefix + "/" + newGroupVersion.Group + "/" + newGroupVersion.Version + "/namespaces/test/foo/test"}})
 		if w.Code != http.StatusOK {
 			t.Fatalf("expected OK: %#v", w)
 		}
 
 		w = httptest.NewRecorder()
-		container.ServeHTTP(w, &http.Request{Method: "GET", URL: &url.URL{Path: "/" + prefix + "/" + newGroupVersion.Group + "/" + newGroupVersion.Version + "/namespaces/test/bar/test"}})
+		handler.ServeHTTP(w, &http.Request{Method: "GET", URL: &url.URL{Path: "/" + prefix + "/" + newGroupVersion.Group + "/" + newGroupVersion.Version + "/namespaces/test/bar/test"}})
 		if w.Code != barCode {
 			t.Errorf("expected response code %d for GET to bar but received %d", barCode, w.Code)
 		}
@@ -2716,16 +2745,19 @@ func TestParentResourceIsRequired(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	handler := genericapifilters.WithRequestInfo(container, newTestRequestInfoResolver(), requestContextMapper)
+	handler = request.WithRequestContext(handler, requestContextMapper)
+
 	// resource is NOT registered in the root scope
 	w := httptest.NewRecorder()
-	container.ServeHTTP(w, &http.Request{Method: "GET", URL: &url.URL{Path: "/" + prefix + "/simple/test/sub"}})
+	handler.ServeHTTP(w, &http.Request{Method: "GET", URL: &url.URL{Path: "/" + prefix + "/simple/test/sub"}})
 	if w.Code != http.StatusNotFound {
 		t.Errorf("expected not found: %#v", w)
 	}
 
 	// resource is registered in the namespace scope
 	w = httptest.NewRecorder()
-	container.ServeHTTP(w, &http.Request{Method: "GET", URL: &url.URL{Path: "/" + prefix + "/" + newGroupVersion.Group + "/" + newGroupVersion.Version + "/namespaces/test/simple/test/sub"}})
+	handler.ServeHTTP(w, &http.Request{Method: "GET", URL: &url.URL{Path: "/" + prefix + "/" + newGroupVersion.Group + "/" + newGroupVersion.Version + "/namespaces/test/simple/test/sub"}})
 	if w.Code != http.StatusOK {
 		t.Fatalf("expected OK: %#v", w)
 	}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go

@@ -0,0 +1,147 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package handlers
+
+import (
+	"fmt"
+	"net/http"
+	"net/url"
+
+	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/endpoints/request"
+)
+
+// ContextFunc returns a Context given a request - a context must be returned
+type ContextFunc func(req *http.Request) request.Context
+
+// 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 *http.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 *http.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 an encoded URI for a given runtime object that represents the canonical path
+	// and query.
+	GenerateLink(req *http.Request, obj runtime.Object) (uri string, err error)
+	// GenerateLink creates an encoded URI for a list that represents the canonical path and query.
+	GenerateListLink(req *http.Request) (uri string, err error)
+}
+
+type ContextBasedNaming struct {
+	GetContext    ContextFunc
+	SelfLinker    runtime.SelfLinker
+	ClusterScoped bool
+
+	SelfLinkPathPrefix string
+	SelfLinkPathSuffix string
+}
+
+// ContextBasedNaming implements ScopeNamer
+var _ ScopeNamer = ContextBasedNaming{}
+
+func (n ContextBasedNaming) SetSelfLink(obj runtime.Object, url string) error {
+	return n.SelfLinker.SetSelfLink(obj, url)
+}
+
+func (n ContextBasedNaming) Namespace(req *http.Request) (namespace string, err error) {
+	requestInfo, ok := request.RequestInfoFrom(n.GetContext(req))
+	if !ok {
+		return "", fmt.Errorf("missing requestInfo")
+	}
+	return requestInfo.Namespace, nil
+}
+
+func (n ContextBasedNaming) Name(req *http.Request) (namespace, name string, err error) {
+	requestInfo, ok := request.RequestInfoFrom(n.GetContext(req))
+	if !ok {
+		return "", "", fmt.Errorf("missing requestInfo")
+	}
+	ns, err := n.Namespace(req)
+	if err != nil {
+		return "", "", err
+	}
+
+	if len(requestInfo.Name) == 0 {
+		return "", "", errEmptyName
+	}
+	return ns, requestInfo.Name, nil
+}
+
+func (n ContextBasedNaming) GenerateLink(req *http.Request, obj runtime.Object) (uri string, err error) {
+	namespace, name, err := n.ObjectName(obj)
+	if err != nil {
+		return "", err
+	}
+	requestInfo, ok := request.RequestInfoFrom(n.GetContext(req))
+	if !ok {
+		return "", fmt.Errorf("missing requestInfo")
+	}
+
+	if len(namespace) == 0 && len(name) == 0 {
+		if len(requestInfo.Name) == 0 {
+			return "", errEmptyName
+		}
+
+		namespace = requestInfo.Namespace
+		name = requestInfo.Name
+	}
+
+	if n.ClusterScoped {
+		return n.SelfLinkPathPrefix + url.QueryEscape(name) + n.SelfLinkPathSuffix, nil
+	}
+
+	return n.SelfLinkPathPrefix +
+			url.QueryEscape(namespace) +
+			"/" + url.QueryEscape(requestInfo.Resource) + "/" +
+			url.QueryEscape(name) +
+			n.SelfLinkPathSuffix,
+		nil
+}
+
+func (n ContextBasedNaming) GenerateListLink(req *http.Request) (uri string, err error) {
+	if len(req.URL.RawPath) > 0 {
+		return req.URL.RawPath, nil
+	}
+	return req.URL.EscapedPath(), nil
+}
+
+func (n ContextBasedNaming) 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
+	}
+	namespace, err = n.SelfLinker.Namespace(obj)
+	if err != nil {
+		return "", "", err
+	}
+	return namespace, name, err
+}
+
+// errEmptyName is returned when API requests do not fill the name section of the path.
+var errEmptyName = errors.NewBadRequest("name must be provided")

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go

@@ -50,30 +50,6 @@ import (
 	utiltrace "k8s.io/apiserver/pkg/util/trace"
 )
 
-// ContextFunc returns a Context given a request - a context must be returned
-type ContextFunc func(req *restful.Request) request.Context
-
-// 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 an encoded URI for a given runtime object that represents the canonical path
-	// and query.
-	GenerateLink(req *restful.Request, obj runtime.Object) (uri string, err error)
-	// GenerateLink creates an encoded URI for a list that represents the canonical path and query.
-	GenerateListLink(req *restful.Request) (uri string, err error)
-}
-
 // RequestScope encapsulates common fields across all RESTful handler methods.
 type RequestScope struct {
 	Namer ScopeNamer
@@ -112,12 +88,12 @@ const MaxRetryWhenPatchConflicts = 5
 func getResourceHandler(scope RequestScope, getter getterFunc) restful.RouteFunction {
 	return func(req *restful.Request, res *restful.Response) {
 		w := res.ResponseWriter
-		namespace, name, err := scope.Namer.Name(req)
+		namespace, name, err := scope.Namer.Name(req.Request)
 		if err != nil {
 			scope.err(err, res.ResponseWriter, req.Request)
 			return
 		}
-		ctx := scope.ContextFunc(req)
+		ctx := scope.ContextFunc(req.Request)
 		ctx = request.WithNamespace(ctx, namespace)
 
 		result, err := getter(ctx, name, req)
@@ -196,12 +172,12 @@ func getRequestOptions(req *restful.Request, scope RequestScope, into runtime.Ob
 func ConnectResource(connecter rest.Connecter, scope RequestScope, admit admission.Interface, restPath string) restful.RouteFunction {
 	return func(req *restful.Request, res *restful.Response) {
 		w := res.ResponseWriter
-		namespace, name, err := scope.Namer.Name(req)
+		namespace, name, err := scope.Namer.Name(req.Request)
 		if err != nil {
 			scope.err(err, res.ResponseWriter, req.Request)
 			return
 		}
-		ctx := scope.ContextFunc(req)
+		ctx := scope.ContextFunc(req.Request)
 		ctx = request.WithNamespace(ctx, namespace)
 		opts, subpath, subpathKey := connecter.NewConnectOptions()
 		if err := getRequestOptions(req, scope, opts, subpath, subpathKey); err != nil {
@@ -254,7 +230,7 @@ func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch
 
 		w := res.ResponseWriter
 
-		namespace, err := scope.Namer.Namespace(req)
+		namespace, err := scope.Namer.Namespace(req.Request)
 		if err != nil {
 			scope.err(err, res.ResponseWriter, req.Request)
 			return
@@ -263,12 +239,12 @@ func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch
 		// Watches for single objects are routed to this function.
 		// Treat a /name parameter the same as a field selector entry.
 		hasName := true
-		_, name, err := scope.Namer.Name(req)
+		_, name, err := scope.Namer.Name(req.Request)
 		if err != nil {
 			hasName = false
 		}
 
-		ctx := scope.ContextFunc(req)
+		ctx := scope.ContextFunc(req.Request)
 		ctx = request.WithNamespace(ctx, namespace)
 
 		opts := metainternalversion.ListOptions{}
@@ -371,16 +347,16 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object
 			err             error
 		)
 		if includeName {
-			namespace, name, err = scope.Namer.Name(req)
+			namespace, name, err = scope.Namer.Name(req.Request)
 		} else {
-			namespace, err = scope.Namer.Namespace(req)
+			namespace, err = scope.Namer.Namespace(req.Request)
 		}
 		if err != nil {
 			scope.err(err, res.ResponseWriter, req.Request)
 			return
 		}
 
-		ctx := scope.ContextFunc(req)
+		ctx := scope.ContextFunc(req.Request)
 		ctx = request.WithNamespace(ctx, namespace)
 
 		gv := scope.Kind.GroupVersion()
@@ -476,13 +452,13 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface
 		// api_installer)
 		timeout := parseTimeout(req.Request.URL.Query().Get("timeout"))
 
-		namespace, name, err := scope.Namer.Name(req)
+		namespace, name, err := scope.Namer.Name(req.Request)
 		if err != nil {
 			scope.err(err, res.ResponseWriter, req.Request)
 			return
 		}
 
-		ctx := scope.ContextFunc(req)
+		ctx := scope.ContextFunc(req.Request)
 		ctx = request.WithNamespace(ctx, namespace)
 
 		versionedObj, err := converter.ConvertToVersion(r.New(), scope.Kind.GroupVersion())
@@ -790,12 +766,12 @@ func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectType
 		// 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 := scope.Namer.Name(req)
+		namespace, name, err := scope.Namer.Name(req.Request)
 		if err != nil {
 			scope.err(err, res.ResponseWriter, req.Request)
 			return
 		}
-		ctx := scope.ContextFunc(req)
+		ctx := scope.ContextFunc(req.Request)
 		ctx = request.WithNamespace(ctx, namespace)
 
 		body, err := readBody(req.Request)
@@ -877,12 +853,12 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco
 		// 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 := scope.Namer.Name(req)
+		namespace, name, err := scope.Namer.Name(req.Request)
 		if err != nil {
 			scope.err(err, res.ResponseWriter, req.Request)
 			return
 		}
-		ctx := scope.ContextFunc(req)
+		ctx := scope.ContextFunc(req.Request)
 		ctx = request.WithNamespace(ctx, namespace)
 
 		options := &metav1.DeleteOptions{}
@@ -985,13 +961,13 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco
 		// 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 := scope.Namer.Namespace(req)
+		namespace, err := scope.Namer.Namespace(req.Request)
 		if err != nil {
 			scope.err(err, res.ResponseWriter, req.Request)
 			return
 		}
 
-		ctx := scope.ContextFunc(req)
+		ctx := scope.ContextFunc(req.Request)
 		ctx = request.WithNamespace(ctx, namespace)
 
 		if admit != nil && admit.Handles(admission.Delete) {
@@ -1139,7 +1115,7 @@ func transformDecodeError(typer runtime.ObjectTyper, baseErr error, into runtime
 // plus the path and query generated by the provided linkFunc
 func setSelfLink(obj runtime.Object, req *restful.Request, namer ScopeNamer) error {
 	// TODO: SelfLink generation should return a full URL?
-	uri, err := namer.GenerateLink(req, obj)
+	uri, err := namer.GenerateLink(req.Request, obj)
 	if err != nil {
 		return nil
 	}
@@ -1163,21 +1139,22 @@ func hasUID(obj runtime.Object) (bool, error) {
 
 // checkName checks the provided name against the request
 func checkName(obj runtime.Object, name, namespace string, namer ScopeNamer) error {
-	if objNamespace, objName, err := namer.ObjectName(obj); err == nil {
+	objNamespace, objName, err := namer.ObjectName(obj)
-		if err != nil {
+	if err != nil {
-			return err
+		return errors.NewBadRequest(fmt.Sprintf(
-		}
+			"the name of the object (%s based on URL) was undeterminable: %v", name, err))
-		if objName != name {
+	}
+	if objName != name {
+		return errors.NewBadRequest(fmt.Sprintf(
+			"the name of the object (%s) does not match the name on the URL (%s)", objName, name))
+	}
+	if len(namespace) > 0 {
+		if len(objNamespace) > 0 && objNamespace != namespace {
 			return errors.NewBadRequest(fmt.Sprintf(
-				"the name of the object (%s) does not match the name on the URL (%s)", objName, name))
+				"the namespace of the object (%s) does not match the namespace on the request (%s)", objNamespace, namespace))
-		}
-		if len(namespace) > 0 {
-			if len(objNamespace) > 0 && objNamespace != namespace {
-				return errors.NewBadRequest(fmt.Sprintf(
-					"the namespace of the object (%s) does not match the namespace on the request (%s)", objNamespace, namespace))
-			}
 		}
 	}
+
 	return nil
 }
 
@@ -1188,7 +1165,7 @@ func setListSelfLink(obj runtime.Object, req *restful.Request, namer ScopeNamer)
 		return 0, nil
 	}
 
-	uri, err := namer.GenerateListLink(req)
+	uri, err := namer.GenerateListLink(req.Request)
 	if err != nil {
 		return 0, err
 	}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go

@@ -19,11 +19,11 @@ package handlers
 import (
 	"errors"
 	"fmt"
+	"net/http"
 	"reflect"
 	"testing"
 	"time"
 
-	"github.com/emicklei/go-restful"
 	"github.com/evanphx/json-patch"
 
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
@@ -128,13 +128,13 @@ type testNamer struct {
 	name      string
 }
 
-func (p *testNamer) Namespace(req *restful.Request) (namespace string, err error) {
+func (p *testNamer) Namespace(req *http.Request) (namespace string, err error) {
 	return p.namespace, nil
 }
 
 // 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.
-func (p *testNamer) Name(req *restful.Request) (namespace, name string, err error) {
+func (p *testNamer) Name(req *http.Request) (namespace, name string, err error) {
 	return p.namespace, p.name, nil
 }
 
@@ -151,12 +151,12 @@ func (p *testNamer) SetSelfLink(obj runtime.Object, url string) error {
 }
 
 // GenerateLink creates a path and query for a given runtime object that represents the canonical path.
-func (p *testNamer) GenerateLink(req *restful.Request, obj runtime.Object) (uri string, err error) {
+func (p *testNamer) GenerateLink(req *http.Request, obj runtime.Object) (uri string, err error) {
 	return "", errors.New("not implemented")
 }
 
 // GenerateLink creates a path and query for a list that represents the canonical path.
-func (p *testNamer) GenerateListLink(req *restful.Request) (uri string, err error) {
+func (p *testNamer) GenerateListLink(req *http.Request) (uri string, err error) {
 	return "", errors.New("not implemented")
 }
 

staging/src/k8s.io/apiserver/pkg/endpoints/installer.go

@@ -17,10 +17,8 @@ limitations under the License.
 package endpoints
 
 import (
-	"bytes"
 	"fmt"
 	"net/http"
-	"net/url"
 	gpath "path"
 	"reflect"
 	"sort"
@@ -28,7 +26,6 @@ import (
 	"time"
 	"unicode"
 
-	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/conversion"
@@ -79,9 +76,6 @@ var toDiscoveryKubeVerb = map[string]string{
 	"WATCHLIST":        "watch",
 }
 
-// errEmptyName is returned when API requests do not fill the name section of the path.
-var errEmptyName = errors.NewBadRequest("name must be provided")
-
 // Installs handlers for API resources.
 func (a *APIInstaller) Install(ws *restful.WebService) (apiResources []metav1.APIResource, errors []error) {
 	errors = make([]error, 0)
@@ -191,6 +185,9 @@ func (a *APIInstaller) restMapping(resource string) (*meta.RESTMapping, error) {
 func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storage, ws *restful.WebService, proxyHandler http.Handler) (*metav1.APIResource, error) {
 	admit := a.group.Admit
 	context := a.group.Context
+	if context == nil {
+		return nil, fmt.Errorf("%v missing Context", a.group.GroupVersion)
+	}
 
 	optionsExternalVersion := a.group.GroupVersion
 	if a.group.OptionsExternalVersion != nil {
@@ -342,14 +339,11 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	}
 
 	var ctxFn handlers.ContextFunc
-	ctxFn = func(req *restful.Request) request.Context {
+	ctxFn = func(req *http.Request) request.Context {
-		if context == nil {
+		if ctx, ok := context.Get(req); ok {
-			return request.WithUserAgent(request.NewContext(), req.HeaderParameter("User-Agent"))
+			return request.WithUserAgent(ctx, req.Header.Get("User-Agent"))
 		}
-		if ctx, ok := context.Get(req.Request); ok {
+		return request.WithUserAgent(request.NewContext(), req.Header.Get("User-Agent"))
-			return request.WithUserAgent(ctx, req.HeaderParameter("User-Agent"))
-		}
-		return request.WithUserAgent(request.NewContext(), req.HeaderParameter("User-Agent"))
 	}
 
 	allowWatchList := isWatcher && isLister // watching on lists is allowed only for kinds that support both watch and list.
@@ -394,7 +388,13 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		apiResource.Name = path
 		apiResource.Namespaced = false
 		apiResource.Kind = resourceKind
-		namer := rootScopeNaming{scope, a.group.Linker, gpath.Join(a.prefix, resourcePath, "/"), suffix}
+		namer := handlers.ContextBasedNaming{
+			GetContext:         ctxFn,
+			SelfLinker:         a.group.Linker,
+			ClusterScoped:      true,
+			SelfLinkPathPrefix: gpath.Join(a.prefix, resourcePath, "/"),
+			SelfLinkPathSuffix: suffix,
+		}
 
 		// Handler for standard REST verbs (GET, PUT, POST and DELETE).
 		// Add actions at the resource path: /api/apiVersion/resource
@@ -430,9 +430,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 
 		resourcePath := namespacedPath
 		resourceParams := namespaceParams
-		itemPathPrefix := gpath.Join(a.prefix, scope.ParamName()) + "/"
 		itemPath := namespacedPath + "/{name}"
-		itemPathMiddle := "/" + resource + "/"
 		nameParams := append(namespaceParams, nameParam)
 		proxyParams := append(nameParams, pathParam)
 		itemPathSuffix := ""
@@ -445,17 +443,13 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		apiResource.Name = path
 		apiResource.Namespaced = true
 		apiResource.Kind = resourceKind
-
+		namer := handlers.ContextBasedNaming{
-		itemPathFn := func(name, namespace string) bytes.Buffer {
+			GetContext:         ctxFn,
-			var buf bytes.Buffer
+			SelfLinker:         a.group.Linker,
-			buf.WriteString(itemPathPrefix)
+			ClusterScoped:      false,
-			buf.WriteString(url.QueryEscape(namespace))
+			SelfLinkPathPrefix: gpath.Join(a.prefix, scope.ParamName()) + "/",
-			buf.WriteString(itemPathMiddle)
+			SelfLinkPathSuffix: itemPathSuffix,
-			buf.WriteString(url.QueryEscape(name))
-			buf.WriteString(itemPathSuffix)
-			return buf
 		}
-		namer := scopeNaming{scope, a.group.Linker, itemPathFn, false}
 
 		actions = appendIf(actions, action{"LIST", resourcePath, resourceParams, namer, false}, isLister)
 		actions = appendIf(actions, action{"POST", resourcePath, resourceParams, namer, false}, isCreater)
@@ -484,7 +478,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		// For ex: LIST all pods in all namespaces by sending a LIST request at /api/apiVersion/pods.
 		// TODO: more strongly type whether a resource allows these actions on "all namespaces" (bulk delete)
 		if !hasSubresource {
-			namer = scopeNaming{scope, a.group.Linker, itemPathFn, true}
 			actions = appendIf(actions, action{"LIST", resource, params, namer, true}, isLister)
 			actions = appendIf(actions, action{"WATCHLIST", "watch/" + resource, params, namer, true}, allowWatchList)
 		}
@@ -811,149 +804,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	return &apiResource, 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
-	pathPrefix string
-	pathSuffix string
-}
-
-// rootScopeNaming implements ScopeNamer
-var _ handlers.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) (uri 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
-		}
-	}
-	return n.pathPrefix + url.QueryEscape(name) + n.pathSuffix, nil
-}
-
-// GenerateListLink returns the appropriate path and query to locate a list by its canonical path.
-func (n rootScopeNaming) GenerateListLink(req *restful.Request) (uri string, err error) {
-	if len(req.Request.URL.RawPath) > 0 {
-		return req.Request.URL.RawPath, nil
-	}
-	return req.Request.URL.EscapedPath(), 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
-	itemPathFn    func(name, namespace string) bytes.Buffer
-	allNamespaces bool
-}
-
-// scopeNaming implements ScopeNamer
-var _ handlers.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.ArgumentName())
-	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) (uri 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
-		}
-	}
-	if len(name) == 0 {
-		return "", errEmptyName
-	}
-	result := n.itemPathFn(name, namespace)
-	return result.String(), nil
-}
-
-// GenerateListLink returns the appropriate path and query to locate a list by its canonical path.
-func (n scopeNaming) GenerateListLink(req *restful.Request) (uri string, err error) {
-	if len(req.Request.URL.RawPath) > 0 {
-		return req.Request.URL.RawPath, nil
-	}
-	return req.Request.URL.EscapedPath(), 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
-}
-
 // This magic incantation returns *ptrToObject for an arbitrary pointer
 func indirectArbitraryPointer(ptrToObject interface{}) interface{} {
 	return reflect.Indirect(reflect.ValueOf(ptrToObject)).Interface()

staging/src/k8s.io/apiserver/pkg/endpoints/installer_test.go

@@ -17,46 +17,9 @@ limitations under the License.
 package endpoints
 
 import (
-	"bytes"
 	"testing"
-
-	"k8s.io/apimachinery/pkg/api/meta"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/pkg/api"
-
-	"github.com/emicklei/go-restful"
 )
 
-func TestScopeNamingGenerateLink(t *testing.T) {
-	selfLinker := &setTestSelfLinker{
-		t:           t,
-		expectedSet: "/api/v1/namespaces/other/services/foo",
-		name:        "foo",
-		namespace:   "other",
-	}
-	s := scopeNaming{
-		meta.RESTScopeNamespace,
-		selfLinker,
-		func(name, namespace string) bytes.Buffer {
-			return *bytes.NewBufferString("/api/v1/namespaces/" + namespace + "/services/" + name)
-		},
-		true,
-	}
-	service := &api.Service{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "foo",
-			Namespace: "other",
-		},
-		TypeMeta: metav1.TypeMeta{
-			Kind: "Service",
-		},
-	}
-	_, err := s.GenerateLink(&restful.Request{}, service)
-	if err != nil {
-		t.Errorf("Unexpected error %v", err)
-	}
-}
-
 func TestIsVowel(t *testing.T) {
 	tests := []struct {
 		name string

vendor/BUILD

@@ -9950,7 +9950,6 @@ go_test(
         "//vendor:k8s.io/apiserver/pkg/endpoints/request",
         "//vendor:k8s.io/apiserver/pkg/endpoints/testing",
         "//vendor:k8s.io/apiserver/pkg/registry/rest",
-        "//vendor:k8s.io/client-go/pkg/api",
     ],
 )
 
@@ -10041,7 +10040,6 @@ go_test(
     library = ":k8s.io/apiserver/pkg/endpoints/handlers",
     tags = ["automanaged"],
     deps = [
-        "//vendor:github.com/emicklei/go-restful",
         "//vendor:github.com/evanphx/json-patch",
         "//vendor:k8s.io/apimachinery/pkg/api/equality",
         "//vendor:k8s.io/apimachinery/pkg/api/errors",
@@ -10064,6 +10062,7 @@ go_library(
     srcs = [
         "k8s.io/apiserver/pkg/endpoints/handlers/discovery.go",
         "k8s.io/apiserver/pkg/endpoints/handlers/doc.go",
+        "k8s.io/apiserver/pkg/endpoints/handlers/namer.go",
         "k8s.io/apiserver/pkg/endpoints/handlers/patch.go",
         "k8s.io/apiserver/pkg/endpoints/handlers/proxy.go",
         "k8s.io/apiserver/pkg/endpoints/handlers/rest.go",