From df870cf36a742ad232894112e5670bd15f56c435 Mon Sep 17 00:00:00 2001 From: deads2k Date: Fri, 25 Sep 2015 14:57:10 -0400 Subject: [PATCH] remove Kind from APIRequestInfo --- pkg/apiserver/apiserver.go | 43 ++++----------------------------- pkg/apiserver/apiserver_test.go | 41 +++++++++++++++++++------------ pkg/apiserver/handlers.go | 13 ++-------- pkg/apiserver/handlers_test.go | 43 +++++++++++++++------------------ pkg/master/master.go | 29 ++++++++++++---------- 5 files changed, 67 insertions(+), 102 deletions(-) diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 50ab1d9ae2b..ec19f11e705 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -42,7 +42,6 @@ import ( "k8s.io/kubernetes/pkg/util" "k8s.io/kubernetes/pkg/util/errors" "k8s.io/kubernetes/pkg/util/flushwriter" - "k8s.io/kubernetes/pkg/util/sets" "k8s.io/kubernetes/pkg/version" "github.com/emicklei/go-restful" @@ -78,13 +77,13 @@ type Mux interface { type APIGroupVersion struct { Storage map[string]rest.Storage - // Root is the APIPrefix under which this is being served. It can also be the APIPrefix/APIGroup that is being served - // Since the APIGroup may not contain a '/', you can get the APIGroup by parsing from the last '/' - // TODO Currently, an APIPrefix with a '/' is not supported in conjunction with an empty APIGroup. This struct should - // be refactored to keep separate information separate to avoid this sort of problem in the future. Root string Version string + // APIRequestInfoResolver 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 + // ServerVersion controls the Kubernetes APIVersion used for common objects in the apiserver // schema like api.Status, api.DeleteOptions, and api.ListOptions. Other implementors may // define a version "v1beta1" but want to use the Kubernetes "v1" internal objects. If @@ -116,38 +115,6 @@ const ( MaxTimeoutSecs = 600 ) -func (g *APIGroupVersion) GetAPIPrefix() string { - slashlessRoot := strings.Trim(g.Root, "/") - if lastSlashIndex := strings.LastIndex(slashlessRoot, "/"); lastSlashIndex != -1 { - return slashlessRoot[:lastSlashIndex] - } - - return slashlessRoot -} - -func (g *APIGroupVersion) GetAPIGroup() string { - slashlessRoot := strings.Trim(g.Root, "/") - if lastSlashIndex := strings.LastIndex(slashlessRoot, "/"); lastSlashIndex != -1 { - return slashlessRoot[lastSlashIndex:] - } - - return "" -} - -func (g *APIGroupVersion) GetAPIVersion() string { - return g.Version -} - -func (g *APIGroupVersion) GetAPIRequestInfoResolver() *APIRequestInfoResolver { - apiPrefix := g.GetAPIPrefix() - info := &APIRequestInfoResolver{sets.NewString(apiPrefix), sets.String{}, g.Mapper} - if len(g.GetAPIGroup()) == 0 { - info.GrouplessAPIPrefixes.Insert(apiPrefix) - } - - return info -} - // InstallREST registers the REST handlers (storage, watch, proxy and redirect) into a restful Container. // It is expected that the provided path root prefix will serve all operations. Root MUST NOT end // in a slash. @@ -190,7 +157,7 @@ func (g *APIGroupVersion) newInstaller() *APIInstaller { prefix := path.Join(g.Root, g.Version) installer := &APIInstaller{ group: g, - info: g.GetAPIRequestInfoResolver(), + info: g.APIRequestInfoResolver, prefix: prefix, minRequestTimeout: g.MinRequestTimeout, proxyDialerFn: g.ProxyDialerFn, diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 82a2c292175..e2cae1467cd 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -42,6 +42,7 @@ import ( "k8s.io/kubernetes/pkg/labels" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util" + "k8s.io/kubernetes/pkg/util/sets" "k8s.io/kubernetes/pkg/version" "k8s.io/kubernetes/pkg/watch" "k8s.io/kubernetes/plugin/pkg/admission/admit" @@ -190,11 +191,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 handleInternal(legacy bool, storage map[string]rest.Storage, admissionControl admission.Interface, selfLinker runtime.SelfLinker) http.Handler { group := &APIGroupVersion{ Storage: storage, Root: "/api", + APIRequestInfoResolver: newTestAPIRequestInfoResolver(), Creater: api.Scheme, Convertor: api.Scheme, @@ -2014,12 +2020,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", - Creater: api.Scheme, - Convertor: api.Scheme, - Typer: api.Scheme, - Linker: selfLinker, + Storage: storage, + Root: "/api", + APIRequestInfoResolver: newTestAPIRequestInfoResolver(), + Creater: api.Scheme, + Convertor: api.Scheme, + Typer: api.Scheme, + Linker: selfLinker, Admit: admissionControl, Context: requestContextMapper, @@ -2096,11 +2103,12 @@ func TestParentResourceIsRequired(t *testing.T) { Storage: map[string]rest.Storage{ "simple/sub": storage, }, - Root: "/api", - Creater: api.Scheme, - Convertor: api.Scheme, - Typer: api.Scheme, - Linker: selfLinker, + Root: "/api", + APIRequestInfoResolver: newTestAPIRequestInfoResolver(), + Creater: api.Scheme, + Convertor: api.Scheme, + Typer: api.Scheme, + Linker: selfLinker, Admit: admissionControl, Context: requestContextMapper, @@ -2124,11 +2132,12 @@ func TestParentResourceIsRequired(t *testing.T) { "simple": &SimpleRESTStorage{}, "simple/sub": storage, }, - Root: "/api", - Creater: api.Scheme, - Convertor: api.Scheme, - Typer: api.Scheme, - Linker: selfLinker, + Root: "/api", + APIRequestInfoResolver: newTestAPIRequestInfoResolver(), + Creater: api.Scheme, + Convertor: api.Scheme, + Typer: api.Scheme, + Linker: selfLinker, Admit: admissionControl, Context: requestContextMapper, diff --git a/pkg/apiserver/handlers.go b/pkg/apiserver/handlers.go index 296ea9aa32b..8661b735101 100644 --- a/pkg/apiserver/handlers.go +++ b/pkg/apiserver/handlers.go @@ -31,7 +31,6 @@ import ( "github.com/golang/glog" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/errors" - "k8s.io/kubernetes/pkg/api/meta" "k8s.io/kubernetes/pkg/auth/authorizer" "k8s.io/kubernetes/pkg/httplog" "k8s.io/kubernetes/pkg/util/sets" @@ -348,8 +347,8 @@ type requestAttributeGetter struct { } // NewAttributeGetter returns an object which implements the RequestAttributeGetter interface. -func NewRequestAttributeGetter(requestContextMapper api.RequestContextMapper, restMapper meta.RESTMapper, apiRoots []string, grouplessAPIRoots []string) RequestAttributeGetter { - return &requestAttributeGetter{requestContextMapper, &APIRequestInfoResolver{sets.NewString(apiRoots...), sets.NewString(grouplessAPIRoots...), restMapper}} +func NewRequestAttributeGetter(requestContextMapper api.RequestContextMapper, apiRequestInfoResolver *APIRequestInfoResolver) RequestAttributeGetter { + return &requestAttributeGetter{requestContextMapper, apiRequestInfoResolver} } func (r *requestAttributeGetter) GetAttribs(req *http.Request) authorizer.Attributes { @@ -407,8 +406,6 @@ type APIRequestInfo struct { // For instance, /pods has the resource "pods" and the kind "Pod", while /pods/foo/status has the resource "pods", the sub resource "status", and the kind "Pod" // (because status operates on pods). The binding resource for a pod though may be /pods/foo/binding, which has resource "pods", subresource "binding", and kind "Binding". Subresource string - // Kind is the type of object being manipulated. For example: Pod - Kind string // Name is empty for some verbs, but if the request directly indicates a name (not in body content) then this field is filled in. Name string // Parts are the path parts for the request, always starting with /{resource}/{name} @@ -421,7 +418,6 @@ type APIRequestInfo struct { type APIRequestInfoResolver struct { APIPrefixes sets.String GrouplessAPIPrefixes sets.String - RestMapper meta.RESTMapper } // TODO write an integration test against the swagger doc to test the APIRequestInfo and match up behavior to responses @@ -534,10 +530,5 @@ func (r *APIRequestInfoResolver) GetAPIRequestInfo(req *http.Request) (APIReques requestInfo.Verb = "list" } - // if we have a resource, we have a good shot at being able to determine kind - if len(requestInfo.Resource) > 0 { - _, requestInfo.Kind, _ = r.RestMapper.VersionAndKindForResource(requestInfo.Resource) - } - return requestInfo, nil } diff --git a/pkg/apiserver/handlers_test.go b/pkg/apiserver/handlers_test.go index a0927757dd8..cf9e79f425f 100644 --- a/pkg/apiserver/handlers_test.go +++ b/pkg/apiserver/handlers_test.go @@ -30,7 +30,6 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/errors" "k8s.io/kubernetes/pkg/api/testapi" - "k8s.io/kubernetes/pkg/util/sets" ) type fakeRL bool @@ -205,44 +204,43 @@ func TestGetAPIRequestInfo(t *testing.T) { expectedNamespace string expectedResource string expectedSubresource string - expectedKind string expectedName string expectedParts []string }{ // resource paths - {"GET", "/api/v1/namespaces", "list", "api", "", "v1", "", "namespaces", "", "Namespace", "", []string{"namespaces"}}, - {"GET", "/api/v1/namespaces/other", "get", "api", "", "v1", "other", "namespaces", "", "Namespace", "other", []string{"namespaces", "other"}}, + {"GET", "/api/v1/namespaces", "list", "api", "", "v1", "", "namespaces", "", "", []string{"namespaces"}}, + {"GET", "/api/v1/namespaces/other", "get", "api", "", "v1", "other", "namespaces", "", "other", []string{"namespaces", "other"}}, - {"GET", "/api/v1/namespaces/other/pods", "list", "api", "", "v1", "other", "pods", "", "Pod", "", []string{"pods"}}, - {"GET", "/api/v1/namespaces/other/pods/foo", "get", "api", "", "v1", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/api/v1/pods", "list", "api", "", "v1", api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}}, - {"GET", "/api/v1/namespaces/other/pods/foo", "get", "api", "", "v1", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/api/v1/namespaces/other/pods", "list", "api", "", "v1", "other", "pods", "", "Pod", "", []string{"pods"}}, + {"GET", "/api/v1/namespaces/other/pods", "list", "api", "", "v1", "other", "pods", "", "", []string{"pods"}}, + {"GET", "/api/v1/namespaces/other/pods/foo", "get", "api", "", "v1", "other", "pods", "", "foo", []string{"pods", "foo"}}, + {"GET", "/api/v1/pods", "list", "api", "", "v1", api.NamespaceAll, "pods", "", "", []string{"pods"}}, + {"GET", "/api/v1/namespaces/other/pods/foo", "get", "api", "", "v1", "other", "pods", "", "foo", []string{"pods", "foo"}}, + {"GET", "/api/v1/namespaces/other/pods", "list", "api", "", "v1", "other", "pods", "", "", []string{"pods"}}, // special verbs - {"GET", "/api/v1/proxy/namespaces/other/pods/foo", "proxy", "api", "", "v1", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/api/v1/redirect/namespaces/other/pods/foo", "redirect", "api", "", "v1", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/api/v1/watch/pods", "watch", "api", "", "v1", api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}}, - {"GET", "/api/v1/watch/namespaces/other/pods", "watch", "api", "", "v1", "other", "pods", "", "Pod", "", []string{"pods"}}, + {"GET", "/api/v1/proxy/namespaces/other/pods/foo", "proxy", "api", "", "v1", "other", "pods", "", "foo", []string{"pods", "foo"}}, + {"GET", "/api/v1/redirect/namespaces/other/pods/foo", "redirect", "api", "", "v1", "other", "pods", "", "foo", []string{"pods", "foo"}}, + {"GET", "/api/v1/watch/pods", "watch", "api", "", "v1", api.NamespaceAll, "pods", "", "", []string{"pods"}}, + {"GET", "/api/v1/watch/namespaces/other/pods", "watch", "api", "", "v1", "other", "pods", "", "", []string{"pods"}}, // subresource identification - {"GET", "/api/v1/namespaces/other/pods/foo/status", "get", "api", "", "v1", "other", "pods", "status", "Pod", "foo", []string{"pods", "foo", "status"}}, - {"PUT", "/api/v1/namespaces/other/finalize", "update", "api", "", "v1", "other", "finalize", "", "", "", []string{"finalize"}}, + {"GET", "/api/v1/namespaces/other/pods/foo/status", "get", "api", "", "v1", "other", "pods", "status", "foo", []string{"pods", "foo", "status"}}, + {"PUT", "/api/v1/namespaces/other/finalize", "update", "api", "", "v1", "other", "finalize", "", "", []string{"finalize"}}, // verb identification - {"PATCH", "/api/v1/namespaces/other/pods/foo", "patch", "api", "", "v1", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, - {"DELETE", "/api/v1/namespaces/other/pods/foo", "delete", "api", "", "v1", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, - {"POST", "/api/v1/namespaces/other/pods", "create", "api", "", "v1", "other", "pods", "", "Pod", "", []string{"pods"}}, + {"PATCH", "/api/v1/namespaces/other/pods/foo", "patch", "api", "", "v1", "other", "pods", "", "foo", []string{"pods", "foo"}}, + {"DELETE", "/api/v1/namespaces/other/pods/foo", "delete", "api", "", "v1", "other", "pods", "", "foo", []string{"pods", "foo"}}, + {"POST", "/api/v1/namespaces/other/pods", "create", "api", "", "v1", "other", "pods", "", "", []string{"pods"}}, // api group identification - {"POST", "/apis/experimental/v1/namespaces/other/pods", "create", "api", "experimental", "v1", "other", "pods", "", "Pod", "", []string{"pods"}}, + {"POST", "/apis/experimental/v1/namespaces/other/pods", "create", "api", "experimental", "v1", "other", "pods", "", "", []string{"pods"}}, // api version identification - {"POST", "/apis/experimental/v1beta3/namespaces/other/pods", "create", "api", "experimental", "v1beta3", "other", "pods", "", "Pod", "", []string{"pods"}}, + {"POST", "/apis/experimental/v1beta3/namespaces/other/pods", "create", "api", "experimental", "v1beta3", "other", "pods", "", "", []string{"pods"}}, } - apiRequestInfoResolver := &APIRequestInfoResolver{sets.NewString("api", "apis"), sets.NewString("api"), testapi.Default.RESTMapper()} + apiRequestInfoResolver := newTestAPIRequestInfoResolver() for _, successCase := range successCases { req, _ := http.NewRequest(successCase.method, successCase.url, nil) @@ -260,9 +258,6 @@ func TestGetAPIRequestInfo(t *testing.T) { if successCase.expectedNamespace != apiRequestInfo.Namespace { t.Errorf("Unexpected namespace for url: %s, expected: %s, actual: %s", successCase.url, successCase.expectedNamespace, apiRequestInfo.Namespace) } - if successCase.expectedKind != apiRequestInfo.Kind { - t.Errorf("Unexpected kind for url: %s, expected: %s, actual: %s", successCase.url, successCase.expectedKind, apiRequestInfo.Kind) - } if successCase.expectedResource != apiRequestInfo.Resource { t.Errorf("Unexpected resource for url: %s, expected: %s, actual: %s", successCase.url, successCase.expectedResource, apiRequestInfo.Resource) } diff --git a/pkg/master/master.go b/pkg/master/master.go index 01e9088bf61..697a2da8eda 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -572,9 +572,7 @@ func (m *Master) init(c *Config) { apiserver.InstallSupport(m.muxHelper, m.rootWebService, c.EnableProfiling, healthzChecks...) apiserver.AddApiWebService(m.handlerContainer, c.APIPrefix, apiVersions) - defaultVersion := m.defaultAPIGroupVersion() - requestInfoResolver := defaultVersion.GetAPIRequestInfoResolver() - apiserver.InstallServiceErrorHandler(m.handlerContainer, requestInfoResolver, apiVersions) + apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newAPIRequestInfoResolver(), apiVersions) // allGroups records all supported groups at /apis allGroups := []api.APIGroup{} @@ -608,8 +606,7 @@ func (m *Master) init(c *Config) { } apiserver.AddGroupWebService(m.handlerContainer, c.APIGroupPrefix+"/"+latest.GroupOrDie("experimental").Group+"/", group) allGroups = append(allGroups, group) - expRequestInfoResolver := expVersion.GetAPIRequestInfoResolver() - apiserver.InstallServiceErrorHandler(m.handlerContainer, expRequestInfoResolver, []string{expVersion.Version}) + apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newAPIRequestInfoResolver(), []string{expVersion.Version}) } // This should be done after all groups are registered @@ -652,10 +649,7 @@ func (m *Master) init(c *Config) { m.InsecureHandler = handler - attributeGetter := apiserver.NewRequestAttributeGetter(m.requestContextMapper, latest.GroupOrDie("").RESTMapper, - []string{strings.Trim(c.APIPrefix, "/"), strings.Trim(thirdpartyprefix, "/")}, // all possible API prefixes - []string{strings.Trim(c.APIPrefix, "/")}, // APIPrefixes that won't have groups (legacy) - ) + attributeGetter := apiserver.NewRequestAttributeGetter(m.requestContextMapper, m.newAPIRequestInfoResolver()) handler = apiserver.WithAuthorizationCheck(handler, attributeGetter, m.authorizer) // Install Authenticator @@ -781,9 +775,17 @@ func (m *Master) getServersToValidate(c *Config) map[string]apiserver.Server { return serversToValidate } +func (m *Master) newAPIRequestInfoResolver() *apiserver.APIRequestInfoResolver { + return &apiserver.APIRequestInfoResolver{ + 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) + } +} + func (m *Master) defaultAPIGroupVersion() *apiserver.APIGroupVersion { return &apiserver.APIGroupVersion{ Root: m.apiPrefix, + APIRequestInfoResolver: m.newAPIRequestInfoResolver(), Mapper: latest.GroupOrDie("").RESTMapper, @@ -921,8 +923,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)) - thirdPartyRequestInfoResolver := &apiserver.APIRequestInfoResolver{APIPrefixes: sets.NewString(strings.Trim(thirdpartyprefix, "/")), RestMapper: thirdparty.Mapper} - apiserver.InstallServiceErrorHandler(m.handlerContainer, thirdPartyRequestInfoResolver, []string{thirdparty.Version}) + apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newAPIRequestInfoResolver(), []string{thirdparty.Version}) return nil } @@ -936,7 +937,9 @@ func (m *Master) thirdpartyapi(group, kind, version string) *apiserver.APIGroupV } return &apiserver.APIGroupVersion{ - Root: apiRoot, + Root: apiRoot, + Version: version, + APIRequestInfoResolver: m.newAPIRequestInfoResolver(), Creater: thirdpartyresourcedata.NewObjectCreator(version, api.Scheme), Convertor: api.Scheme, @@ -946,7 +949,6 @@ func (m *Master) thirdpartyapi(group, kind, version string) *apiserver.APIGroupV Codec: thirdpartyresourcedata.NewCodec(latest.GroupOrDie("experimental").Codec, kind), Linker: latest.GroupOrDie("experimental").SelfLinker, Storage: storage, - Version: version, ServerVersion: latest.GroupOrDie("").GroupVersion, Context: m.requestContextMapper, @@ -993,6 +995,7 @@ func (m *Master) experimental(c *Config) *apiserver.APIGroupVersion { return &apiserver.APIGroupVersion{ Root: m.apiGroupPrefix, + APIRequestInfoResolver: m.newAPIRequestInfoResolver(), Creater: api.Scheme, Convertor: api.Scheme,