From 8b7e56d242a90e8cc125ae1fc94a7eaf7767fb92 Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Wed, 24 Feb 2016 11:10:37 -0800 Subject: [PATCH] Allow cross-group subresource registrations in the APIInstaller. This allows subresources which belong to different API groups than their parents to be registered in the APIInstaller and REST handlers installed for them. The specific changes that makes this possible are: 1. Allow subresource overrides to be specified while registering an API group. 2. Use those overrides in the APIInstaller while validating the resource/subresource group version to allow subresources which belong to a different group to be registered if they have an override specified. 3. Use the RESTMapper supplied in the override to map the REST paths to the correct subresource storage object, i.e. correct group version kinds. --- pkg/apiserver/api_installer.go | 158 ++++++++++++----------- pkg/apiserver/apiserver.go | 6 + pkg/apiserver/apiserver_test.go | 137 +++++++++++++++++++- pkg/genericapiserver/genericapiserver.go | 7 + 4 files changed, 234 insertions(+), 74 deletions(-) diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go index bc497ca2ca6..0b9adcd54df 100644 --- a/pkg/apiserver/api_installer.go +++ b/pkg/apiserver/api_installer.go @@ -110,6 +110,63 @@ func (a *APIInstaller) NewWebService() *restful.WebService { return ws } +// getResourceKind returns the external group version kind registered for the given storage +// object. If the storage object is a subresource and has an override supplied for it, it returns +// the group version kind supplied in the override. +func (a *APIInstaller) getResourceKind(path string, storage rest.Storage) (unversioned.GroupVersionKind, error) { + if fqKindToRegister, ok := a.group.SubresourceGroupVersionKind[path]; ok { + return fqKindToRegister, nil + } + + object := storage.New() + fqKinds, err := a.group.Typer.ObjectKinds(object) + if err != nil { + return unversioned.GroupVersionKind{}, err + } + + // a given go type can have multiple potential fully qualified kinds. Find the one that corresponds with the group + // we're trying to register here + fqKindToRegister := unversioned.GroupVersionKind{} + for _, fqKind := range fqKinds { + if fqKind.Group == a.group.GroupVersion.Group { + fqKindToRegister = a.group.GroupVersion.WithKind(fqKind.Kind) + break + } + + // TODO This keeps it doing what it was doing before, but it doesn't feel right. + if fqKind.Group == extensions.GroupName && fqKind.Kind == "ThirdPartyResourceData" { + fqKindToRegister = a.group.GroupVersion.WithKind(fqKind.Kind) + } + } + if fqKindToRegister.IsEmpty() { + return unversioned.GroupVersionKind{}, fmt.Errorf("unable to locate fully qualified kind for %v: found %v when registering for %v", reflect.TypeOf(object), fqKinds, a.group.GroupVersion) + } + return fqKindToRegister, nil +} + +// restMapping returns rest mapper for the resource. +// Example REST paths that this mapper maps. +// 1. Resource only, no subresource: +// Resource Type: batch/v1.Job (input args: resource = "jobs") +// REST path: /apis/batch/v1/namespaces/{namespace}/job/{name} +// 2. Subresource and its parent belong to different API groups and/or versions: +// Resource Type: extensions/v1beta1.ReplicaSet (input args: resource = "replicasets") +// Subresource Type: autoscaling/v1.Scale +// REST path: /apis/extensions/v1beta1/namespaces/{namespace}/replicaset/{name}/scale +func (a *APIInstaller) restMapping(resource string) (*meta.RESTMapping, error) { + // subresources must have parent resources, and follow the namespacing rules of their parent. + // So get the storage of the resource (which is the parent resource in case of subresources) + storage, ok := a.group.Storage[resource] + if !ok { + return nil, fmt.Errorf("unable to locate the storage object for resource: %s", resource) + } + fqKindToRegister, err := a.getResourceKind(resource, storage) + if err != nil { + return nil, fmt.Errorf("unable to locate fully qualified kind for mapper resource %s: %v", resource, err) + } + return a.group.Mapper.RESTMapping(fqKindToRegister.GroupKind(), fqKindToRegister.Version) +} + func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storage, ws *restful.WebService, proxyHandler http.Handler) (*unversioned.APIResource, error) { admit := a.group.Admit context := a.group.Context @@ -119,88 +176,28 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag optionsExternalVersion = *a.group.OptionsExternalVersion } - var resource, subresource string - switch parts := strings.Split(path, "/"); len(parts) { - case 2: - resource, subresource = parts[0], parts[1] - case 1: - resource = parts[0] - default: - // TODO: support deeper paths - return nil, fmt.Errorf("api_installer allows only one or two segment paths (resource or resource/subresource)") - } - hasSubresource := len(subresource) > 0 - - object := storage.New() - fqKinds, err := a.group.Typer.ObjectKinds(object) + resource, subresource, err := splitSubresource(path) if err != nil { return nil, err } - // a given go type can have multiple potential fully qualified kinds. Find the one that corresponds with the group - // we're trying to register here - fqKindToRegister := unversioned.GroupVersionKind{} - for _, fqKind := range fqKinds { - if fqKind.Group == a.group.GroupVersion.Group { - fqKindToRegister = fqKind - break - } - // TODO This keeps it doing what it was doing before, but it doesn't feel right. - if fqKind.Group == extensions.GroupName && fqKind.Kind == "ThirdPartyResourceData" { - fqKindToRegister = fqKind - fqKindToRegister.Group = a.group.GroupVersion.Group - fqKindToRegister.Version = a.group.GroupVersion.Version - } + mapping, err := a.restMapping(resource) + if err != nil { + return nil, err } - kind := fqKindToRegister.Kind - - if fqKindToRegister.IsEmpty() { - return nil, fmt.Errorf("unable to locate fully qualified kind for %v: found %v when registering for %v", reflect.TypeOf(object), fqKinds, a.group.GroupVersion) + fqKindToRegister, err := a.getResourceKind(path, storage) + if err != nil { + return nil, err } - versionedPtr, err := a.group.Creater.New(a.group.GroupVersion.WithKind(kind)) + versionedPtr, err := a.group.Creater.New(fqKindToRegister) if err != nil { return nil, err } versionedObject := indirectArbitraryPointer(versionedPtr) - - mapping, err := a.group.Mapper.RESTMapping(fqKindToRegister.GroupKind(), a.group.GroupVersion.Version) - if err != nil { - return nil, err - } - - // subresources must have parent resources, and follow the namespacing rules of their parent - if hasSubresource { - parentStorage, ok := a.group.Storage[resource] - if !ok { - return nil, fmt.Errorf("subresources can only be declared when the parent is also registered: %s needs %s", path, resource) - } - parentObject := parentStorage.New() - - parentFQKinds, err := a.group.Typer.ObjectKinds(parentObject) - if err != nil { - return nil, err - } - // a given go type can have multiple potential fully qualified kinds. Find the one that corresponds with the group - // we're trying to register here - parentFQKindToRegister := unversioned.GroupVersionKind{} - for _, fqKind := range parentFQKinds { - if fqKind.Group == a.group.GroupVersion.Group { - parentFQKindToRegister = fqKind - break - } - } - if parentFQKindToRegister.IsEmpty() { - return nil, fmt.Errorf("unable to locate fully qualified kind for %v: found %v when registering for %v", reflect.TypeOf(object), fqKinds, a.group.GroupVersion) - } - - parentMapping, err := a.group.Mapper.RESTMapping(parentFQKindToRegister.GroupKind(), a.group.GroupVersion.Version) - if err != nil { - return nil, err - } - mapping.Scope = parentMapping.Scope - } + kind := fqKindToRegister.Kind + hasSubresource := len(subresource) > 0 // what verbs are supported by the storage, used to know what verbs we support per path creater, isCreater := storage.(rest.Creater) @@ -329,7 +326,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag if ok { resourceKind = kindProvider.Kind() } else { - resourceKind = fqKindToRegister.Kind + resourceKind = kind } var apiResource unversioned.APIResource @@ -451,9 +448,10 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag Creater: a.group.Creater, Convertor: a.group.Convertor, + // TODO: This seems wrong for cross-group subresources. It makes an assumption that a subresource and its parent are in the same group version. Revisit this. Resource: a.group.GroupVersion.WithResource(resource), Subresource: subresource, - Kind: a.group.GroupVersion.WithKind(kind), + Kind: fqKindToRegister, } for _, action := range actions { reqScope.Namer = action.Namer @@ -948,3 +946,19 @@ var _ rest.StorageMetadata = defaultStorageMetadata{} func (defaultStorageMetadata) ProducesMIMETypes(verb string) []string { return nil } + +// splitSubresource checks if the given storage path is the path of a subresource and returns +// the resource and subresource components. +func splitSubresource(path string) (string, string, error) { + var resource, subresource string + switch parts := strings.Split(path, "/"); len(parts) { + case 2: + resource, subresource = parts[0], parts[1] + case 1: + resource = parts[0] + default: + // TODO: support deeper paths + return "", "", fmt.Errorf("api_installer allows only one or two segment paths (resource or resource/subresource)") + } + return resource, subresource, nil +} diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 235b8dc9167..b1e25e72301 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -96,6 +96,12 @@ type APIGroupVersion struct { Context api.RequestContextMapper MinRequestTimeout time.Duration + + // SubresourceGroupVersionKind contains the GroupVersionKind overrides for each subresource that is + // accessible from this API group version. The GroupVersionKind is that of the external version of + // the subresource. The key of this map should be the path of the subresource. The keys here should + // match the keys in the Storage map above for subresources. + SubresourceGroupVersionKind map[string]unversioned.GroupVersionKind } type ProxyDialerFunc func(network, addr string) (net.Conn, error) diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index f997cd4c5e0..1c7daaf12db 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -59,9 +59,12 @@ func convert(obj runtime.Object) (runtime.Object, error) { // This creates fake API versions, similar to api/latest.go. var testAPIGroup = "test.group" +var testAPIGroup2 = "test.group2" var testInternalGroupVersion = unversioned.GroupVersion{Group: testAPIGroup, Version: runtime.APIVersionInternal} var testGroupVersion = unversioned.GroupVersion{Group: testAPIGroup, Version: "version"} var newGroupVersion = unversioned.GroupVersion{Group: testAPIGroup, Version: "version2"} +var testGroup2Version = unversioned.GroupVersion{Group: testAPIGroup2, Version: "version"} +var testInternalGroup2Version = unversioned.GroupVersion{Group: testAPIGroup2, Version: runtime.APIVersionInternal} var prefix = "apis" var grouplessGroupVersion = unversioned.GroupVersion{Group: "", Version: "v1"} @@ -99,6 +102,11 @@ func interfacesFor(version unversioned.GroupVersion) (*meta.VersionInterfaces, e ObjectConvertor: api.Scheme, MetadataAccessor: accessor, }, nil + case testGroup2Version: + return &meta.VersionInterfaces{ + ObjectConvertor: api.Scheme, + MetadataAccessor: accessor, + }, nil default: return nil, fmt.Errorf("unsupported storage version: %s (valid: %v)", version, groupVersions) } @@ -139,11 +147,18 @@ func addTestTypes() { } api.Scheme.AddKnownTypes(testGroupVersion, &apiservertesting.Simple{}, &apiservertesting.SimpleList{}, &ListOptions{}, - &api.DeleteOptions{}, &apiservertesting.SimpleGetOptions{}, &apiservertesting.SimpleRoot{}) + &api.DeleteOptions{}, &apiservertesting.SimpleGetOptions{}, &apiservertesting.SimpleRoot{}, + &SimpleXGSubresource{}) api.Scheme.AddKnownTypes(testGroupVersion, &api.Pod{}) api.Scheme.AddKnownTypes(testInternalGroupVersion, &apiservertesting.Simple{}, &apiservertesting.SimpleList{}, &api.ListOptions{}, - &apiservertesting.SimpleGetOptions{}, &apiservertesting.SimpleRoot{}) + &apiservertesting.SimpleGetOptions{}, &apiservertesting.SimpleRoot{}, + &SimpleXGSubresource{}) + // Register SimpleXGSubresource in both testGroupVersion and testGroup2Version, and also their + // their corresponding internal versions, to verify that the desired group version object is + // served in the tests. + api.Scheme.AddKnownTypes(testGroup2Version, &SimpleXGSubresource{}) + api.Scheme.AddKnownTypes(testInternalGroup2Version, &SimpleXGSubresource{}) } func addNewTestTypes() { @@ -3157,6 +3172,124 @@ func TestUpdateChecksAPIVersion(t *testing.T) { } } +// SimpleXGSubresource is a cross group subresource, i.e. the subresource does not belong to the +// same group as its parent resource. +type SimpleXGSubresource struct { + unversioned.TypeMeta `json:",inline"` + api.ObjectMeta `json:"metadata"` + SubresourceInfo string `json:"subresourceInfo,omitempty"` + Labels map[string]string `json:"labels,omitempty"` +} + +func (obj *SimpleXGSubresource) GetObjectKind() unversioned.ObjectKind { return &obj.TypeMeta } + +type SimpleXGSubresourceRESTStorage struct { + item SimpleXGSubresource +} + +func (storage *SimpleXGSubresourceRESTStorage) New() runtime.Object { + return &SimpleXGSubresource{} +} + +func (storage *SimpleXGSubresourceRESTStorage) Get(ctx api.Context, id string) (runtime.Object, error) { + copied, err := api.Scheme.Copy(&storage.item) + if err != nil { + panic(err) + } + return copied, nil +} + +func TestXGSubresource(t *testing.T) { + container := restful.NewContainer() + container.Router(restful.CurlyRouter{}) + mux := container.ServeMux + + itemID := "theID" + subresourceStorage := &SimpleXGSubresourceRESTStorage{ + item: SimpleXGSubresource{ + SubresourceInfo: "foo", + }, + } + storage := map[string]rest.Storage{ + "simple": &SimpleRESTStorage{}, + "simple/subsimple": subresourceStorage, + } + + group := APIGroupVersion{ + Storage: storage, + + RequestInfoResolver: newTestRequestInfoResolver(), + + Creater: api.Scheme, + Convertor: api.Scheme, + Typer: api.Scheme, + Linker: selfLinker, + Mapper: namespaceMapper, + + ParameterCodec: api.ParameterCodec, + + Admit: admissionControl, + Context: requestContextMapper, + + Root: "/" + prefix, + GroupVersion: testGroupVersion, + OptionsExternalVersion: &testGroupVersion, + Serializer: api.Codecs, + + SubresourceGroupVersionKind: map[string]unversioned.GroupVersionKind{ + "simple/subsimple": testGroup2Version.WithKind("SimpleXGSubresource"), + }, + } + + if err := (&group).InstallREST(container); err != nil { + panic(fmt.Sprintf("unable to install container %s: %v", group.GroupVersion, err)) + } + + ws := new(restful.WebService) + InstallSupport(mux, ws) + container.Add(ws) + + handler := defaultAPIServer{mux, container} + server := httptest.NewServer(handler) + // TODO: Uncomment when fix #19254 + // defer server.Close() + + resp, err := http.Get(server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple/" + itemID + "/subsimple") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.StatusCode != http.StatusOK { + t.Fatalf("unexpected response: %#v", resp) + } + var itemOut SimpleXGSubresource + body, err := extractBody(resp, &itemOut) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + // Test if the returned object has the expected group, version and kind + // We are directly unmarshaling JSON here because TypeMeta cannot be decoded through the + // installed decoders. TypeMeta cannot be decoded because it is added to the ignored + // conversion type list in API scheme and hence cannot be converted from input type object + // to output type object. So it's values don't appear in the decoded output object. + decoder := json.NewDecoder(strings.NewReader(body)) + var itemFromBody SimpleXGSubresource + err = decoder.Decode(&itemFromBody) + if err != nil { + t.Errorf("unexpected JSON decoding error: %v", err) + } + if want := fmt.Sprintf("%s/%s", testGroup2Version.Group, testGroup2Version.Version); itemFromBody.APIVersion != want { + t.Errorf("unexpected APIVersion got: %+v want: %+v", itemFromBody.APIVersion, want) + } + if itemFromBody.Kind != "SimpleXGSubresource" { + t.Errorf("unexpected Kind got: %+v want: SimpleXGSubresource", itemFromBody.Kind) + } + + if itemOut.Name != subresourceStorage.item.Name { + t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, subresourceStorage.item, string(body)) + } +} + func readBodyOrDie(r io.Reader) []byte { body, err := ioutil.ReadAll(r) if err != nil { diff --git a/pkg/genericapiserver/genericapiserver.go b/pkg/genericapiserver/genericapiserver.go index c2a328661df..c1e20fda2d8 100644 --- a/pkg/genericapiserver/genericapiserver.go +++ b/pkg/genericapiserver/genericapiserver.go @@ -192,6 +192,12 @@ type APIGroupInfo struct { NegotiatedSerializer runtime.NegotiatedSerializer // ParameterCodec performs conversions for query parameters passed to API calls ParameterCodec runtime.ParameterCodec + + // SubresourceGroupVersionKind contains the GroupVersionKind overrides for each subresource that is + // accessible from this API group version. The GroupVersionKind is that of the external version of + // the subresource. The key of this map should be the path of the subresource. The keys here should + // match the keys in the Storage map above for subresources. + SubresourceGroupVersionKind map[string]unversioned.GroupVersionKind } // Config is a structure used to configure a GenericAPIServer. @@ -838,6 +844,7 @@ func (s *GenericAPIServer) getAPIGroupVersion(apiGroupInfo *APIGroupInfo, groupV version.Creater = apiGroupInfo.Scheme version.Convertor = apiGroupInfo.Scheme version.Typer = apiGroupInfo.Scheme + version.SubresourceGroupVersionKind = apiGroupInfo.SubresourceGroupVersionKind return version, err }