diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go index 1a463dd34bd..4b874ac57e3 100644 --- a/pkg/apiserver/api_installer.go +++ b/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 } diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 1608c117c12..1c0ccdc2cb0 100644 --- a/pkg/apiserver/apiserver.go +++ b/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) { - requestInfo, err := requestResolver.GetAPIRequestInfo(request.Request) +func serviceErrorHandler(requestResolver *RequestInfoResolver, apiVersions []string, serviceErr restful.ServiceError, request *restful.Request, response *restful.Response) { + requestInfo, err := requestResolver.GetRequestInfo(request.Request) codec := latest.GroupOrDie("").Codec if err == nil && requestInfo.APIVersion != "" { // check if the api version is valid. diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 86d454c4297..eabe8ebad30 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/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 { - return &APIRequestInfoResolver{sets.NewString("api", "apis"), sets.NewString("api")} +func newTestRequestInfoResolver() *RequestInfoResolver { + 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", - APIRequestInfoResolver: newTestAPIRequestInfoResolver(), + Root: "/api", + 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, - Root: "/api", - APIRequestInfoResolver: newTestAPIRequestInfoResolver(), - Creater: api.Scheme, - Convertor: api.Scheme, - Typer: api.Scheme, - Linker: selfLinker, + Storage: storage, + Root: "/api", + RequestInfoResolver: newTestRequestInfoResolver(), + Creater: api.Scheme, + Convertor: api.Scheme, + Typer: api.Scheme, + 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", - APIRequestInfoResolver: newTestAPIRequestInfoResolver(), - Creater: api.Scheme, - Convertor: api.Scheme, - Typer: api.Scheme, - Linker: selfLinker, + Root: "/api", + RequestInfoResolver: newTestRequestInfoResolver(), + Creater: api.Scheme, + Convertor: api.Scheme, + Typer: api.Scheme, + Linker: selfLinker, Admit: admissionControl, Context: requestContextMapper, @@ -2186,12 +2186,12 @@ func TestParentResourceIsRequired(t *testing.T) { "simple": &SimpleRESTStorage{}, "simple/sub": storage, }, - Root: "/api", - APIRequestInfoResolver: newTestAPIRequestInfoResolver(), - Creater: api.Scheme, - Convertor: api.Scheme, - Typer: api.Scheme, - Linker: selfLinker, + Root: "/api", + RequestInfoResolver: newTestRequestInfoResolver(), + Creater: api.Scheme, + Convertor: api.Scheme, + Typer: api.Scheme, + Linker: selfLinker, Admit: admissionControl, Context: requestContextMapper, diff --git a/pkg/apiserver/errors.go b/pkg/apiserver/errors.go index a7c1fbdf328..f9c47250b1b 100644 --- a/pkg/apiserver/errors.go +++ b/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 diff --git a/pkg/apiserver/handlers.go b/pkg/apiserver/handlers.go index 307ee582acb..61e0da5dab1 100644 --- a/pkg/apiserver/handlers.go +++ b/pkg/apiserver/handlers.go @@ -343,13 +343,13 @@ type RequestAttributeGetter interface { } type requestAttributeGetter struct { - requestContextMapper api.RequestContextMapper - apiRequestInfoResolver *APIRequestInfoResolver + requestContextMapper api.RequestContextMapper + requestInfoResolver *RequestInfoResolver } // NewAttributeGetter returns an object which implements the RequestAttributeGetter interface. -func NewRequestAttributeGetter(requestContextMapper api.RequestContextMapper, apiRequestInfoResolver *APIRequestInfoResolver) RequestAttributeGetter { - return &requestAttributeGetter{requestContextMapper, apiRequestInfoResolver} +func NewRequestAttributeGetter(requestContextMapper api.RequestContextMapper, requestInfoResolver *RequestInfoResolver) RequestAttributeGetter { + 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 -type APIRequestInfo struct { - // Verb is the kube verb associated with the request, not the http verb. This includes things like list and watch. - Verb string +// RequestInfo holds information parsed from the http.Request +type RequestInfo struct { + // IsResourceRequest indicates whether or not the request is for an API resource or subresource + 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 -// 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 +// TODO write an integration test against the swagger doc to test the RequestInfo and match up behavior to responses +// 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{ - Raw: splitPath(req.URL.Path), - Verb: strings.ToLower(req.Method), +// +// NonResource paths +// /apis/{api-group}/{version} +// /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 { diff --git a/pkg/apiserver/handlers_test.go b/pkg/apiserver/handlers_test.go index 029684e7dc7..fcfed364fe7 100644 --- a/pkg/apiserver/handlers_test.go +++ b/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) - if err == nil { - t.Errorf("Expected error for key: %s", k) + apiRequestInfo, err := requestInfoResolver.GetRequestInfo(req) + if err != nil { + 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) } } } diff --git a/pkg/apiserver/proxy.go b/pkg/apiserver/proxy.go index 3398ab71ff3..d693f78a7ea 100644 --- a/pkg/apiserver/proxy.go +++ b/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 - storage map[string]rest.Storage - codec runtime.Codec - context api.RequestContextMapper - apiRequestInfoResolver *APIRequestInfoResolver + prefix string + storage map[string]rest.Storage + codec runtime.Codec + context api.RequestContextMapper + 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) - if err != nil { + requestInfo, err := r.requestInfoResolver.GetRequestInfo(req) + if err != nil || !requestInfo.IsResourceRequest { notFound(w, req) httpCode = http.StatusNotFound return diff --git a/pkg/master/master.go b/pkg/master/master.go index 4b06c21b98b..20bdff5daa3 100644 --- a/pkg/master/master.go +++ b/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 { - return &apiserver.APIRequestInfoResolver{ +func (m *Master) newRequestInfoResolver() *apiserver.RequestInfoResolver { + 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, - APIRequestInfoResolver: m.newAPIRequestInfoResolver(), + Root: m.apiPrefix, + 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, - Version: apiutil.GetGroupVersion(group, version), - APIRequestInfoResolver: m.newAPIRequestInfoResolver(), + Root: apiRoot, + Version: apiutil.GetGroupVersion(group, version), + 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, - APIRequestInfoResolver: m.newAPIRequestInfoResolver(), + Root: m.apiGroupPrefix, + RequestInfoResolver: m.newRequestInfoResolver(), Creater: api.Scheme, Convertor: api.Scheme,