Merge pull request #15958 from deads2k/add-is-api-url

add nonResourceURL detection

pkg/apiserver/api_installer.go

@@ -39,7 +39,7 @@ import (
 
 type APIInstaller struct {
 	group             *APIGroupVersion
-	info              *APIRequestInfoResolver
+	info              *RequestInfoResolver
 	prefix            string // Path prefix where API resources are to be registered.
 	minRequestTimeout time.Duration
 }

pkg/apiserver/apiserver.go

@@ -83,9 +83,9 @@ type APIGroupVersion struct {
 	// TODO: caesarxuchao: Version actually contains "group/version", refactor it to avoid confusion.
 	Version string
 
-	// APIRequestInfoResolver is used to parse URLs for the legacy proxy handler.  Don't use this for anything else
+	// RequestInfoResolver is used to parse URLs for the legacy proxy handler.  Don't use this for anything else
 	// TODO: refactor proxy handler to use sub resources
-	APIRequestInfoResolver *APIRequestInfoResolver
+	RequestInfoResolver *RequestInfoResolver
 
 	// ServerVersion controls the Kubernetes APIVersion used for common objects in the apiserver
 	// schema like api.Status, api.DeleteOptions, and api.ListOptions. Other implementors may
@@ -161,7 +161,7 @@ func (g *APIGroupVersion) newInstaller() *APIInstaller {
 	prefix := path.Join(g.Root, g.Version)
 	installer := &APIInstaller{
 		group:             g,
-		info:              g.APIRequestInfoResolver,
+		info:              g.RequestInfoResolver,
 		prefix:            prefix,
 		minRequestTimeout: g.MinRequestTimeout,
 	}
@@ -217,14 +217,14 @@ func logStackOnRecover(panicReason interface{}, httpWriter http.ResponseWriter)
 	errorJSON(apierrors.NewGenericServerResponse(http.StatusInternalServerError, "", "", "", "", 0, false), latest.GroupOrDie("").Codec, httpWriter)
 }
 
-func InstallServiceErrorHandler(container *restful.Container, requestResolver *APIRequestInfoResolver, apiVersions []string) {
+func InstallServiceErrorHandler(container *restful.Container, requestResolver *RequestInfoResolver, apiVersions []string) {
 	container.ServiceErrorHandler(func(serviceErr restful.ServiceError, request *restful.Request, response *restful.Response) {
 		serviceErrorHandler(requestResolver, apiVersions, serviceErr, request, response)
 	})
 }
 
-func serviceErrorHandler(requestResolver *APIRequestInfoResolver, apiVersions []string, serviceErr restful.ServiceError, request *restful.Request, response *restful.Response) {
+func serviceErrorHandler(requestResolver *RequestInfoResolver, apiVersions []string, serviceErr restful.ServiceError, request *restful.Request, response *restful.Response) {
-	requestInfo, err := requestResolver.GetAPIRequestInfo(request.Request)
+	requestInfo, err := requestResolver.GetRequestInfo(request.Request)
 	codec := latest.GroupOrDie("").Codec
 	if err == nil && requestInfo.APIVersion != "" {
 		// check if the api version is valid.

pkg/apiserver/apiserver_test.go

@@ -198,16 +198,16 @@ func handleLinker(storage map[string]rest.Storage, selfLinker runtime.SelfLinker
 	return handleInternal(true, storage, admissionControl, selfLinker)
 }
 
-func newTestAPIRequestInfoResolver() *APIRequestInfoResolver {
+func newTestRequestInfoResolver() *RequestInfoResolver {
-	return &APIRequestInfoResolver{sets.NewString("api", "apis"), sets.NewString("api")}
+	return &RequestInfoResolver{sets.NewString("api", "apis"), sets.NewString("api")}
 }
 
 func handleInternal(legacy bool, storage map[string]rest.Storage, admissionControl admission.Interface, selfLinker runtime.SelfLinker) http.Handler {
 	group := &APIGroupVersion{
 		Storage: storage,
 
-		Root: "/api",
+		Root:                "/api",
-		APIRequestInfoResolver: newTestAPIRequestInfoResolver(),
+		RequestInfoResolver: newTestRequestInfoResolver(),
 
 		Creater:   api.Scheme,
 		Convertor: api.Scheme,
@@ -2074,13 +2074,13 @@ func TestCreateChecksDecode(t *testing.T) {
 func TestUpdateREST(t *testing.T) {
 	makeGroup := func(storage map[string]rest.Storage) *APIGroupVersion {
 		return &APIGroupVersion{
-			Storage: storage,
+			Storage:             storage,
-			Root:    "/api",
+			Root:                "/api",
-			APIRequestInfoResolver: newTestAPIRequestInfoResolver(),
+			RequestInfoResolver: newTestRequestInfoResolver(),
-			Creater:                api.Scheme,
+			Creater:             api.Scheme,
-			Convertor:              api.Scheme,
+			Convertor:           api.Scheme,
-			Typer:                  api.Scheme,
+			Typer:               api.Scheme,
-			Linker:                 selfLinker,
+			Linker:              selfLinker,
 
 			Admit:   admissionControl,
 			Context: requestContextMapper,
@@ -2157,12 +2157,12 @@ func TestParentResourceIsRequired(t *testing.T) {
 		Storage: map[string]rest.Storage{
 			"simple/sub": storage,
 		},
-		Root: "/api",
+		Root:                "/api",
-		APIRequestInfoResolver: newTestAPIRequestInfoResolver(),
+		RequestInfoResolver: newTestRequestInfoResolver(),
-		Creater:                api.Scheme,
+		Creater:             api.Scheme,
-		Convertor:              api.Scheme,
+		Convertor:           api.Scheme,
-		Typer:                  api.Scheme,
+		Typer:               api.Scheme,
-		Linker:                 selfLinker,
+		Linker:              selfLinker,
 
 		Admit:   admissionControl,
 		Context: requestContextMapper,
@@ -2186,12 +2186,12 @@ func TestParentResourceIsRequired(t *testing.T) {
 			"simple":     &SimpleRESTStorage{},
 			"simple/sub": storage,
 		},
-		Root: "/api",
+		Root:                "/api",
-		APIRequestInfoResolver: newTestAPIRequestInfoResolver(),
+		RequestInfoResolver: newTestRequestInfoResolver(),
-		Creater:                api.Scheme,
+		Creater:             api.Scheme,
-		Convertor:              api.Scheme,
+		Convertor:           api.Scheme,
-		Typer:                  api.Scheme,
+		Typer:               api.Scheme,
-		Linker:                 selfLinker,
+		Linker:              selfLinker,
 
 		Admit:   admissionControl,
 		Context: requestContextMapper,

pkg/apiserver/errors.go

@@ -87,7 +87,7 @@ func forbidden(w http.ResponseWriter, req *http.Request) {
 	fmt.Fprintf(w, "Forbidden: %#v", req.RequestURI)
 }
 
-// errAPIPrefixNotFound indicates that a APIRequestInfo resolution failed because the request isn't under
+// errAPIPrefixNotFound indicates that a RequestInfo resolution failed because the request isn't under
 // any known API prefixes
 type errAPIPrefixNotFound struct {
 	SpecifiedPrefix string

pkg/apiserver/handlers.go

@@ -343,13 +343,13 @@ type RequestAttributeGetter interface {
 }
 
 type requestAttributeGetter struct {
-	requestContextMapper   api.RequestContextMapper
+	requestContextMapper api.RequestContextMapper
-	apiRequestInfoResolver *APIRequestInfoResolver
+	requestInfoResolver  *RequestInfoResolver
 }
 
 // NewAttributeGetter returns an object which implements the RequestAttributeGetter interface.
-func NewRequestAttributeGetter(requestContextMapper api.RequestContextMapper, apiRequestInfoResolver *APIRequestInfoResolver) RequestAttributeGetter {
+func NewRequestAttributeGetter(requestContextMapper api.RequestContextMapper, requestInfoResolver *RequestInfoResolver) RequestAttributeGetter {
-	return &requestAttributeGetter{requestContextMapper, apiRequestInfoResolver}
+	return &requestAttributeGetter{requestContextMapper, requestInfoResolver}
 }
 
 func (r *requestAttributeGetter) GetAttribs(req *http.Request) authorizer.Attributes {
@@ -363,7 +363,7 @@ func (r *requestAttributeGetter) GetAttribs(req *http.Request) authorizer.Attrib
 		}
 	}
 
-	apiRequestInfo, _ := r.apiRequestInfoResolver.GetAPIRequestInfo(req)
+	apiRequestInfo, _ := r.requestInfoResolver.GetRequestInfo(req)
 
 	attribs.APIGroup = apiRequestInfo.APIGroup
 	attribs.Verb = apiRequestInfo.Verb
@@ -392,10 +392,16 @@ func WithAuthorizationCheck(handler http.Handler, getAttribs RequestAttributeGet
 	})
 }
 
-// APIRequestInfo holds information parsed from the http.Request
+// RequestInfo holds information parsed from the http.Request
-type APIRequestInfo struct {
+type RequestInfo struct {
-	// Verb is the kube verb associated with the request, not the http verb.  This includes things like list and watch.
+	// IsResourceRequest indicates whether or not the request is for an API resource or subresource
-	Verb       string
+	IsResourceRequest bool
+	// Path is the URL path of the request
+	Path string
+	// Verb is the kube verb associated with the request for API requests, not the http verb.  This includes things like list and watch.
+	// for non-resource requests, this is the lowercase http verb
+	Verb string
+
 	APIPrefix  string
 	APIGroup   string
 	APIVersion string
@@ -410,20 +416,18 @@ type APIRequestInfo struct {
 	Name string
 	// Parts are the path parts for the request, always starting with /{resource}/{name}
 	Parts []string
-	// Raw is the unparsed form of everything other than parts.
-	// Raw + Parts = complete URL path
-	Raw []string
 }
 
-type APIRequestInfoResolver struct {
+type RequestInfoResolver struct {
 	APIPrefixes          sets.String
 	GrouplessAPIPrefixes sets.String
 }
 
-// TODO write an integration test against the swagger doc to test the APIRequestInfo and match up behavior to responses
+// TODO write an integration test against the swagger doc to test the RequestInfo and match up behavior to responses
-// GetAPIRequestInfo returns the information from the http request.  If error is not nil, APIRequestInfo holds the information as best it is known before the failure
+// GetRequestInfo returns the information from the http request.  If error is not nil, RequestInfo holds the information as best it is known before the failure
+// It handles both resource and non-resource requests and fills in all the pertinent information for each.
 // Valid Inputs:
-// Storage paths
+// Resource paths
 // /apis/{api-group}/{version}/namespaces
 // /api/{version}/namespaces
 // /api/{version}/namespaces/{namespace}
@@ -439,19 +443,32 @@ type APIRequestInfoResolver struct {
 // /api/{version}/redirect/{resource}/{resourceName}
 // /api/{version}/watch/{resource}
 // /api/{version}/watch/namespaces/{namespace}/{resource}
-func (r *APIRequestInfoResolver) GetAPIRequestInfo(req *http.Request) (APIRequestInfo, error) {
+//
-	requestInfo := APIRequestInfo{
+// NonResource paths
-		Raw:  splitPath(req.URL.Path),
+// /apis/{api-group}/{version}
-		Verb: strings.ToLower(req.Method),
+// /apis/{api-group}
+// /apis
+// /api/{version}
+// /api
+// /healthz
+// /
+func (r *RequestInfoResolver) GetRequestInfo(req *http.Request) (RequestInfo, error) {
+	// start with a non-resource request until proven otherwise
+	requestInfo := RequestInfo{
+		IsResourceRequest: false,
+		Path:              req.URL.Path,
+		Verb:              strings.ToLower(req.Method),
 	}
 
-	currentParts := requestInfo.Raw
+	currentParts := splitPath(req.URL.Path)
 	if len(currentParts) < 3 {
-		return requestInfo, fmt.Errorf("a resource request must have a url with at least three parts, not %v", req.URL)
+		// return a non-resource request
+		return requestInfo, nil
 	}
 
 	if !r.APIPrefixes.Has(currentParts[0]) {
-		return requestInfo, &errAPIPrefixNotFound{currentParts[0]}
+		// return a non-resource request
+		return requestInfo, nil
 	}
 	requestInfo.APIPrefix = currentParts[0]
 	currentParts = currentParts[1:]
@@ -459,13 +476,15 @@ func (r *APIRequestInfoResolver) GetAPIRequestInfo(req *http.Request) (APIReques
 	if !r.GrouplessAPIPrefixes.Has(requestInfo.APIPrefix) {
 		// one part (APIPrefix) has already been consumed, so this is actually "do we have four parts?"
 		if len(currentParts) < 3 {
-			return requestInfo, fmt.Errorf("a resource request with an API group must have a url with at least four parts, not %v", req.URL)
+			// return a non-resource request
+			return requestInfo, nil
 		}
 
 		requestInfo.APIGroup = currentParts[0]
 		currentParts = currentParts[1:]
 	}
 
+	requestInfo.IsResourceRequest = true
 	requestInfo.APIVersion = currentParts[0]
 	currentParts = currentParts[1:]
 
@@ -512,8 +531,6 @@ func (r *APIRequestInfoResolver) GetAPIRequestInfo(req *http.Request) (APIReques
 
 	// parsing successful, so we now know the proper value for .Parts
 	requestInfo.Parts = currentParts
-	// Raw should have everything not in Parts
-	requestInfo.Raw = requestInfo.Raw[:len(requestInfo.Raw)-len(currentParts)]
 
 	// parts look like: resource/resourceName/subresource/other/stuff/we/don't/interpret
 	switch {

pkg/apiserver/handlers_test.go

@@ -248,15 +248,18 @@ func TestGetAPIRequestInfo(t *testing.T) {
 		{"POST", "/apis/extensions/v1beta3/namespaces/other/pods", "create", "api", "extensions", "v1beta3", "other", "pods", "", "", []string{"pods"}},
 	}
 
-	apiRequestInfoResolver := newTestAPIRequestInfoResolver()
+	requestInfoResolver := newTestRequestInfoResolver()
 
 	for _, successCase := range successCases {
 		req, _ := http.NewRequest(successCase.method, successCase.url, nil)
 
-		apiRequestInfo, err := apiRequestInfoResolver.GetAPIRequestInfo(req)
+		apiRequestInfo, err := requestInfoResolver.GetRequestInfo(req)
 		if err != nil {
 			t.Errorf("Unexpected error for url: %s %v", successCase.url, err)
 		}
+		if !apiRequestInfo.IsResourceRequest {
+			t.Errorf("Expected resource request")
+		}
 		if successCase.expectedVerb != apiRequestInfo.Verb {
 			t.Errorf("Unexpected verb for url: %s, expected: %s, actual: %s", successCase.url, successCase.expectedVerb, apiRequestInfo.Verb)
 		}
@@ -293,9 +296,48 @@ func TestGetAPIRequestInfo(t *testing.T) {
 		if err != nil {
 			t.Errorf("Unexpected error %v", err)
 		}
-		_, err = apiRequestInfoResolver.GetAPIRequestInfo(req)
+		apiRequestInfo, err := requestInfoResolver.GetRequestInfo(req)
-		if err == nil {
+		if err != nil {
-			t.Errorf("Expected error for key: %s", k)
+			t.Errorf("%s: Unexpected error %v", k, err)
+		}
+		if apiRequestInfo.IsResourceRequest {
+			t.Errorf("%s: expected non-resource request", k)
+		}
+	}
+}
+
+func TestGetNonAPIRequestInfo(t *testing.T) {
+	tests := map[string]struct {
+		url      string
+		expected bool
+	}{
+		"simple groupless":  {"/api/version/resource", true},
+		"simple group":      {"/apis/group/version/resource/name/subresource", true},
+		"more steps":        {"/api/version/resource/name/subresource", true},
+		"group list":        {"/apis/extensions/v1beta1/job", true},
+		"group get":         {"/apis/extensions/v1beta1/job/foo", true},
+		"group subresource": {"/apis/extensions/v1beta1/job/foo/scale", true},
+
+		"bad root":                     {"/not-api/version/resource", false},
+		"group without enough steps":   {"/apis/extensions/v1beta1", false},
+		"group without enough steps 2": {"/apis/extensions/v1beta1/", false},
+		"not enough steps":             {"/api/version", false},
+		"one step":                     {"/api", false},
+		"zero step":                    {"/", false},
+		"empty":                        {"", false},
+	}
+
+	requestInfoResolver := newTestRequestInfoResolver()
+
+	for testName, tc := range tests {
+		req, _ := http.NewRequest("GET", tc.url, nil)
+
+		apiRequestInfo, err := requestInfoResolver.GetRequestInfo(req)
+		if err != nil {
+			t.Errorf("%s: Unexpected error %v", testName, err)
+		}
+		if e, a := tc.expected, apiRequestInfo.IsResourceRequest; e != a {
+			t.Errorf("%s: expected %v, actual %v", testName, e, a)
 		}
 	}
 }

pkg/apiserver/proxy.go

@@ -42,11 +42,11 @@ import (
 // ProxyHandler provides a http.Handler which will proxy traffic to locations
 // specified by items implementing Redirector.
 type ProxyHandler struct {
-	prefix                 string
+	prefix              string
-	storage                map[string]rest.Storage
+	storage             map[string]rest.Storage
-	codec                  runtime.Codec
+	codec               runtime.Codec
-	context                api.RequestContextMapper
+	context             api.RequestContextMapper
-	apiRequestInfoResolver *APIRequestInfoResolver
+	requestInfoResolver *RequestInfoResolver
 }
 
 func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
@@ -58,8 +58,8 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	reqStart := time.Now()
 	defer metrics.Monitor(&verb, &apiResource, util.GetClient(req), &httpCode, reqStart)
 
-	requestInfo, err := r.apiRequestInfoResolver.GetAPIRequestInfo(req)
+	requestInfo, err := r.requestInfoResolver.GetRequestInfo(req)
-	if err != nil {
+	if err != nil || !requestInfo.IsResourceRequest {
 		notFound(w, req)
 		httpCode = http.StatusNotFound
 		return

pkg/master/master.go

@@ -641,7 +641,7 @@ func (m *Master) init(c *Config) {
 
 	apiserver.InstallSupport(m.muxHelper, m.rootWebService, c.EnableProfiling, healthzChecks...)
 	apiserver.AddApiWebService(m.handlerContainer, c.APIPrefix, apiVersions)
-	apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newAPIRequestInfoResolver(), apiVersions)
+	apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newRequestInfoResolver(), apiVersions)
 
 	// allGroups records all supported groups at /apis
 	allGroups := []unversioned.APIGroup{}
@@ -676,7 +676,7 @@ func (m *Master) init(c *Config) {
 		}
 		apiserver.AddGroupWebService(m.handlerContainer, c.APIGroupPrefix+"/"+latest.GroupOrDie("extensions").Group+"/", group)
 		allGroups = append(allGroups, group)
-		apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newAPIRequestInfoResolver(), []string{expVersion.Version})
+		apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newRequestInfoResolver(), []string{expVersion.Version})
 	}
 
 	// This should be done after all groups are registered
@@ -719,7 +719,7 @@ func (m *Master) init(c *Config) {
 
 	m.InsecureHandler = handler
 
-	attributeGetter := apiserver.NewRequestAttributeGetter(m.requestContextMapper, m.newAPIRequestInfoResolver())
+	attributeGetter := apiserver.NewRequestAttributeGetter(m.requestContextMapper, m.newRequestInfoResolver())
 	handler = apiserver.WithAuthorizationCheck(handler, attributeGetter, m.authorizer)
 
 	// Install Authenticator
@@ -849,8 +849,8 @@ func (m *Master) getServersToValidate(c *Config) map[string]apiserver.Server {
 	return serversToValidate
 }
 
-func (m *Master) newAPIRequestInfoResolver() *apiserver.APIRequestInfoResolver {
+func (m *Master) newRequestInfoResolver() *apiserver.RequestInfoResolver {
-	return &apiserver.APIRequestInfoResolver{
+	return &apiserver.RequestInfoResolver{
 		sets.NewString(strings.Trim(m.apiPrefix, "/"), strings.Trim(thirdpartyprefix, "/")), // all possible API prefixes
 		sets.NewString(strings.Trim(m.apiPrefix, "/")),                                      // APIPrefixes that won't have groups (legacy)
 	}
@@ -858,8 +858,8 @@ func (m *Master) newAPIRequestInfoResolver() *apiserver.APIRequestInfoResolver {
 
 func (m *Master) defaultAPIGroupVersion() *apiserver.APIGroupVersion {
 	return &apiserver.APIGroupVersion{
-		Root: m.apiPrefix,
+		Root:                m.apiPrefix,
-		APIRequestInfoResolver: m.newAPIRequestInfoResolver(),
+		RequestInfoResolver: m.newRequestInfoResolver(),
 
 		Mapper: latest.GroupOrDie("").RESTMapper,
 
@@ -996,7 +996,7 @@ func (m *Master) InstallThirdPartyResource(rsrc *expapi.ThirdPartyResource) erro
 	}
 	apiserver.AddGroupWebService(m.handlerContainer, path, apiGroup)
 	m.addThirdPartyResourceStorage(path, thirdparty.Storage[strings.ToLower(kind)+"s"].(*thirdpartyresourcedataetcd.REST))
-	apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newAPIRequestInfoResolver(), []string{thirdparty.Version})
+	apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newRequestInfoResolver(), []string{thirdparty.Version})
 	return nil
 }
 
@@ -1010,9 +1010,9 @@ func (m *Master) thirdpartyapi(group, kind, version string) *apiserver.APIGroupV
 	}
 
 	return &apiserver.APIGroupVersion{
-		Root:                   apiRoot,
+		Root:                apiRoot,
-		Version:                apiutil.GetGroupVersion(group, version),
+		Version:             apiutil.GetGroupVersion(group, version),
-		APIRequestInfoResolver: m.newAPIRequestInfoResolver(),
+		RequestInfoResolver: m.newRequestInfoResolver(),
 
 		Creater:   thirdpartyresourcedata.NewObjectCreator(group, version, api.Scheme),
 		Convertor: api.Scheme,
@@ -1097,8 +1097,8 @@ func (m *Master) experimental(c *Config) *apiserver.APIGroupVersion {
 	extensionsGroup := latest.GroupOrDie("extensions")
 
 	return &apiserver.APIGroupVersion{
-		Root: m.apiGroupPrefix,
+		Root:                m.apiGroupPrefix,
-		APIRequestInfoResolver: m.newAPIRequestInfoResolver(),
+		RequestInfoResolver: m.newRequestInfoResolver(),
 
 		Creater:   api.Scheme,
 		Convertor: api.Scheme,