From b3658c7b16cd5b55d29769a79f64e941937d7fbc Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Wed, 27 Jul 2016 23:18:04 -0700 Subject: [PATCH] Fix third party APIResource reporting --- hack/make-rules/test-cmd.sh | 54 ++++++++++- pkg/apiserver/apiserver.go | 39 ++++++-- pkg/master/master.go | 135 ++++++++++++++++++++-------- pkg/master/master_test.go | 20 +++-- pkg/master/thirdparty_controller.go | 20 ++++- 5 files changed, 212 insertions(+), 56 deletions(-) diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh index aa095f3fd4d..18965e869e6 100755 --- a/hack/make-rules/test-cmd.sh +++ b/hack/make-rules/test-cmd.sh @@ -1036,14 +1036,39 @@ __EOF__ # Post-Condition: assertion object exist kube::test::get_object_assert thirdpartyresources "{{range.items}}{{$id_field}}:{{end}}" 'foo.company.com:' + kubectl "${kube_flags[@]}" create -f - "${kube_flags[@]}" << __EOF__ +{ + "kind": "ThirdPartyResource", + "apiVersion": "extensions/v1beta1", + "metadata": { + "name": "bar.company.com" + }, + "versions": [ + { + "name": "v1" + } + ] +} +__EOF__ + + # Post-Condition: assertion object exist + kube::test::get_object_assert thirdpartyresources "{{range.items}}{{$id_field}}:{{end}}" 'bar.company.com:foo.company.com:' + kube::util::wait_for_url "http://127.0.0.1:${API_PORT}/apis/company.com/v1" "third party api" - # Test that we can list this new third party resource + kube::util::wait_for_url "http://127.0.0.1:${API_PORT}/apis/company.com/v1/foos" "third party api Foo" + + kube::util::wait_for_url "http://127.0.0.1:${API_PORT}/apis/company.com/v1/bars" "third party api Bar" + + # Test that we can list this new third party resource (foos) kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" '' + # Test that we can list this new third party resource (bars) + kube::test::get_object_assert bars "{{range.items}}{{$id_field}}:{{end}}" '' + # Test that we can create a new resource of type Foo - kubectl "${kube_flags[@]}" create -f - "${kube_flags[@]}" << __EOF__ - { + kubectl "${kube_flags[@]}" create -f - "${kube_flags[@]}" << __EOF__ +{ "kind": "Foo", "apiVersion": "company.com/v1", "metadata": { @@ -1063,8 +1088,31 @@ __EOF__ # Make sure it's gone kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" '' + # Test that we can create a new resource of type Bar + kubectl "${kube_flags[@]}" create -f - "${kube_flags[@]}" << __EOF__ +{ + "kind": "Bar", + "apiVersion": "company.com/v1", + "metadata": { + "name": "test" + }, + "some-field": "field1", + "other-field": "field2" +} +__EOF__ + + # Test that we can list this new third party resource + kube::test::get_object_assert bars "{{range.items}}{{$id_field}}:{{end}}" 'test:' + + # Delete the resource + kubectl "${kube_flags[@]}" delete bars test + + # Make sure it's gone + kube::test::get_object_assert bars "{{range.items}}{{$id_field}}:{{end}}" '' + # teardown kubectl delete thirdpartyresources foo.company.com "${kube_flags[@]}" + kubectl delete thirdpartyresources bar.company.com "${kube_flags[@]}" ##################################### # Recursive Resources via directory # diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 1876016522a..533cce0c4a8 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -58,6 +58,10 @@ type Mux interface { HandleFunc(pattern string, handler func(http.ResponseWriter, *http.Request)) } +type APIResourceLister interface { + ListAPIResources() []unversioned.APIResource +} + // APIGroupVersion is a helper for exposing rest.Storage objects as http.Handlers via go-restful // It handles URLs of the form: // /${storage_key}[/${object_name}] @@ -104,6 +108,10 @@ type APIGroupVersion struct { // 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 + + // ResourceLister is an interface that knows how to list resources + // for this API Group. + ResourceLister APIResourceLister } type ProxyDialerFunc func(network, addr string) (net.Conn, error) @@ -116,6 +124,17 @@ const ( MaxTimeoutSecs = 600 ) +// staticLister implements the APIResourceLister interface +type staticLister struct { + list []unversioned.APIResource +} + +func (s staticLister) ListAPIResources() []unversioned.APIResource { + return s.list +} + +var _ APIResourceLister = &staticLister{} + // 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. @@ -123,7 +142,11 @@ func (g *APIGroupVersion) InstallREST(container *restful.Container) error { installer := g.newInstaller() ws := installer.NewWebService() apiResources, registrationErrors := installer.Install(ws) - AddSupportedResourcesWebService(g.Serializer, ws, g.GroupVersion, apiResources) + lister := g.ResourceLister + if lister == nil { + lister = staticLister{apiResources} + } + AddSupportedResourcesWebService(g.Serializer, ws, g.GroupVersion, lister) container.Add(ws) return utilerrors.NewAggregate(registrationErrors) } @@ -147,7 +170,11 @@ func (g *APIGroupVersion) UpdateREST(container *restful.Container) error { return apierrors.NewInternalError(fmt.Errorf("unable to find an existing webservice for prefix %s", installer.prefix)) } apiResources, registrationErrors := installer.Install(ws) - AddSupportedResourcesWebService(g.Serializer, ws, g.GroupVersion, apiResources) + lister := g.ResourceLister + if lister == nil { + lister = staticLister{apiResources} + } + AddSupportedResourcesWebService(g.Serializer, ws, g.GroupVersion, lister) return utilerrors.NewAggregate(registrationErrors) } @@ -336,7 +363,7 @@ func AddGroupWebService(s runtime.NegotiatedSerializer, container *restful.Conta // Adds a service to return the supported resources, E.g., a such web service // will be registered at /apis/extensions/v1. -func AddSupportedResourcesWebService(s runtime.NegotiatedSerializer, ws *restful.WebService, groupVersion unversioned.GroupVersion, apiResources []unversioned.APIResource) { +func AddSupportedResourcesWebService(s runtime.NegotiatedSerializer, ws *restful.WebService, groupVersion unversioned.GroupVersion, lister APIResourceLister) { ss := s if keepUnversioned(groupVersion.Group) { // Because in release 1.1, /apis/extensions/v1beta1 returns response @@ -344,7 +371,7 @@ func AddSupportedResourcesWebService(s runtime.NegotiatedSerializer, ws *restful // keep the response backwards compatible. ss = StripVersionNegotiatedSerializer{s} } - resourceHandler := SupportedResourcesHandler(ss, groupVersion, apiResources) + resourceHandler := SupportedResourcesHandler(ss, groupVersion, lister) ws.Route(ws.GET("/").To(resourceHandler). Doc("get available resources"). Operation("getAPIResources"). @@ -381,9 +408,9 @@ func GroupHandler(s runtime.NegotiatedSerializer, group unversioned.APIGroup) re } // SupportedResourcesHandler returns a handler which will list the provided resources as available. -func SupportedResourcesHandler(s runtime.NegotiatedSerializer, groupVersion unversioned.GroupVersion, apiResources []unversioned.APIResource) restful.RouteFunction { +func SupportedResourcesHandler(s runtime.NegotiatedSerializer, groupVersion unversioned.GroupVersion, lister APIResourceLister) restful.RouteFunction { return func(req *restful.Request, resp *restful.Response) { - writeNegotiated(s, unversioned.GroupVersion{}, resp.ResponseWriter, req.Request, http.StatusOK, &unversioned.APIResourceList{GroupVersion: groupVersion.String(), APIResources: apiResources}) + writeNegotiated(s, unversioned.GroupVersion{}, resp.ResponseWriter, req.Request, http.StatusOK, &unversioned.APIResourceList{GroupVersion: groupVersion.String(), APIResources: lister.ListAPIResources()}) } } diff --git a/pkg/master/master.go b/pkg/master/master.go index fc870b9992e..c02754d1c6b 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -143,7 +143,7 @@ type Master struct { // storage for third party objects thirdPartyStorage storage.Interface // map from api path to a tuple of (storage for the objects, APIGroup) - thirdPartyResources map[string]thirdPartyEntry + thirdPartyResources map[string]*thirdPartyEntry // protects the map thirdPartyResourcesLock sync.RWMutex // Useful for reliable testing. Shouldn't be used otherwise. @@ -156,7 +156,8 @@ type Master struct { // thirdPartyEntry combines objects storage and API group into one struct // for easy lookup. type thirdPartyEntry struct { - storage *thirdpartyresourcedataetcd.REST + // Map from plural resource name to entry + storage map[string]*thirdpartyresourcedataetcd.REST group unversioned.APIGroup } @@ -279,7 +280,7 @@ func (m *Master) InstallAPIs(c *Config) { if err != nil { glog.Fatalf("Error getting third party storage: %v", err) } - m.thirdPartyResources = map[string]thirdPartyEntry{} + m.thirdPartyResources = map[string]*thirdPartyEntry{} } restOptionsGetter := func(resource unversioned.GroupResource) generic.RESTOptions { @@ -510,37 +511,60 @@ func (m *Master) getServersToValidate(c *Config) map[string]apiserver.Server { // HasThirdPartyResource returns true if a particular third party resource currently installed. func (m *Master) HasThirdPartyResource(rsrc *extensions.ThirdPartyResource) (bool, error) { - _, group, err := thirdpartyresourcedata.ExtractApiGroupAndKind(rsrc) + kind, group, err := thirdpartyresourcedata.ExtractApiGroupAndKind(rsrc) if err != nil { return false, err } path := makeThirdPartyPath(group) - services := m.HandlerContainer.RegisteredWebServices() - for ix := range services { - if services[ix].RootPath() == path { - return true, nil - } - } - return false, nil -} - -func (m *Master) removeThirdPartyStorage(path string) error { m.thirdPartyResourcesLock.Lock() defer m.thirdPartyResourcesLock.Unlock() - storage, found := m.thirdPartyResources[path] - if found { - if err := m.removeAllThirdPartyResources(storage.storage); err != nil { - return err - } + entry := m.thirdPartyResources[path] + if entry == nil { + return false, nil + } + plural, _ := meta.KindToResource(unversioned.GroupVersionKind{ + Group: group, + Version: rsrc.Versions[0].Name, + Kind: kind, + }) + _, found := entry.storage[plural.Resource] + return found, nil +} + +func (m *Master) removeThirdPartyStorage(path, resource string) error { + m.thirdPartyResourcesLock.Lock() + defer m.thirdPartyResourcesLock.Unlock() + entry, found := m.thirdPartyResources[path] + if !found { + return nil + } + storage, found := entry.storage[resource] + if !found { + return nil + } + if err := m.removeAllThirdPartyResources(storage); err != nil { + return err + } + delete(entry.storage, resource) + if len(entry.storage) == 0 { delete(m.thirdPartyResources, path) m.RemoveAPIGroupForDiscovery(getThirdPartyGroupName(path)) + } else { + m.thirdPartyResources[path] = entry } return nil } // RemoveThirdPartyResource removes all resources matching `path`. Also deletes any stored data func (m *Master) RemoveThirdPartyResource(path string) error { - if err := m.removeThirdPartyStorage(path); err != nil { + ix := strings.LastIndex(path, "/") + if ix == -1 { + return fmt.Errorf("expected /, saw: %s", path) + } + resource := path[ix+1:] + path = path[0:ix] + + if err := m.removeThirdPartyStorage(path, resource); err != nil { return err } @@ -574,28 +598,58 @@ func (m *Master) removeAllThirdPartyResources(registry *thirdpartyresourcedataet } // ListThirdPartyResources lists all currently installed third party resources +// The format is / func (m *Master) ListThirdPartyResources() []string { m.thirdPartyResourcesLock.RLock() defer m.thirdPartyResourcesLock.RUnlock() result := []string{} for key := range m.thirdPartyResources { - result = append(result, key) + for rsrc := range m.thirdPartyResources[key].storage { + result = append(result, key+"/"+rsrc) + } } return result } -func (m *Master) hasThirdPartyResourceStorage(path string) bool { +func (m *Master) getExistingThirdPartyResources(path string) []unversioned.APIResource { + result := []unversioned.APIResource{} + m.thirdPartyResourcesLock.Lock() + defer m.thirdPartyResourcesLock.Unlock() + entry := m.thirdPartyResources[path] + if entry != nil { + for key, obj := range entry.storage { + result = append(result, unversioned.APIResource{ + Name: key, + Namespaced: true, + Kind: obj.Kind(), + }) + } + } + return result +} + +func (m *Master) hasThirdPartyGroupStorage(path string) bool { m.thirdPartyResourcesLock.Lock() defer m.thirdPartyResourcesLock.Unlock() _, found := m.thirdPartyResources[path] return found } -func (m *Master) addThirdPartyResourceStorage(path string, storage *thirdpartyresourcedataetcd.REST, apiGroup unversioned.APIGroup) { +func (m *Master) addThirdPartyResourceStorage(path, resource string, storage *thirdpartyresourcedataetcd.REST, apiGroup unversioned.APIGroup) { m.thirdPartyResourcesLock.Lock() defer m.thirdPartyResourcesLock.Unlock() - m.thirdPartyResources[path] = thirdPartyEntry{storage, apiGroup} - m.AddAPIGroupForDiscovery(apiGroup) + entry, found := m.thirdPartyResources[path] + if entry == nil { + entry = &thirdPartyEntry{ + group: apiGroup, + storage: map[string]*thirdpartyresourcedataetcd.REST{}, + } + m.thirdPartyResources[path] = entry + } + entry.storage[resource] = storage + if !found { + m.AddAPIGroupForDiscovery(apiGroup) + } } // InstallThirdPartyResource installs a third party resource specified by 'rsrc'. When a resource is @@ -617,17 +671,6 @@ func (m *Master) InstallThirdPartyResource(rsrc *extensions.ThirdPartyResource) }) path := makeThirdPartyPath(group) - thirdparty := m.thirdpartyapi(group, kind, rsrc.Versions[0].Name, plural.Resource) - - // If storage exists, this group has already been added, just update - // the group with the new API - if m.hasThirdPartyResourceStorage(path) { - return thirdparty.UpdateREST(m.HandlerContainer) - } - - if err := thirdparty.InstallREST(m.HandlerContainer); err != nil { - glog.Errorf("Unable to setup thirdparty api: %v", err) - } groupVersion := unversioned.GroupVersionForDiscovery{ GroupVersion: group + "/" + rsrc.Versions[0].Name, Version: rsrc.Versions[0].Name, @@ -637,9 +680,22 @@ func (m *Master) InstallThirdPartyResource(rsrc *extensions.ThirdPartyResource) Versions: []unversioned.GroupVersionForDiscovery{groupVersion}, PreferredVersion: groupVersion, } + + thirdparty := m.thirdpartyapi(group, kind, rsrc.Versions[0].Name, plural.Resource) + + // If storage exists, this group has already been added, just update + // the group with the new API + if m.hasThirdPartyGroupStorage(path) { + m.addThirdPartyResourceStorage(path, plural.Resource, thirdparty.Storage[plural.Resource].(*thirdpartyresourcedataetcd.REST), apiGroup) + return thirdparty.UpdateREST(m.HandlerContainer) + } + + if err := thirdparty.InstallREST(m.HandlerContainer); err != nil { + glog.Errorf("Unable to setup thirdparty api: %v", err) + } apiserver.AddGroupWebService(api.Codecs, m.HandlerContainer, path, apiGroup) - m.addThirdPartyResourceStorage(path, thirdparty.Storage[plural.Resource].(*thirdpartyresourcedataetcd.REST), apiGroup) + m.addThirdPartyResourceStorage(path, plural.Resource, thirdparty.Storage[plural.Resource].(*thirdpartyresourcedataetcd.REST), apiGroup) apiserver.InstallServiceErrorHandler(api.Codecs, m.HandlerContainer, m.NewRequestInfoResolver(), []string{thirdparty.GroupVersion.String()}) return nil } @@ -655,8 +711,6 @@ func (m *Master) thirdpartyapi(group, kind, version, pluralResource string) *api kind, ) - apiRoot := makeThirdPartyPath("") - storage := map[string]rest.Storage{ pluralResource: resourceStorage, } @@ -665,6 +719,7 @@ func (m *Master) thirdpartyapi(group, kind, version, pluralResource string) *api internalVersion := unversioned.GroupVersion{Group: group, Version: runtime.APIVersionInternal} externalVersion := unversioned.GroupVersion{Group: group, Version: version} + apiRoot := makeThirdPartyPath("") return &apiserver.APIGroupVersion{ Root: apiRoot, GroupVersion: externalVersion, @@ -686,6 +741,8 @@ func (m *Master) thirdpartyapi(group, kind, version, pluralResource string) *api Context: m.RequestContextMapper, MinRequestTimeout: m.MinRequestTimeout, + + ResourceLister: dynamicLister{m, makeThirdPartyPath(group)}, } } diff --git a/pkg/master/master_test.go b/pkg/master/master_test.go index 334a40b9c38..6e8621f3d56 100644 --- a/pkg/master/master_test.go +++ b/pkg/master/master_test.go @@ -508,7 +508,7 @@ func TestDiscoveryAtAPIS(t *testing.T) { } thirdPartyGV := unversioned.GroupVersionForDiscovery{GroupVersion: "company.com/v1", Version: "v1"} - master.addThirdPartyResourceStorage("/apis/company.com/v1", nil, + master.addThirdPartyResourceStorage("/apis/company.com/v1", "foos", nil, unversioned.APIGroup{ Name: "company.com", Versions: []unversioned.GroupVersionForDiscovery{thirdPartyGV}, @@ -575,10 +575,18 @@ func initThirdPartyMultiple(t *testing.T, versions, names []string) (*Master, *e }, }, } - err := master.InstallThirdPartyResource(api) - if !assert.NoError(err) { - t.Logf("Failed to install API: %v", err) - t.FailNow() + hasRsrc, err := master.HasThirdPartyResource(api) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if !hasRsrc { + err := master.InstallThirdPartyResource(api) + if !assert.NoError(err) { + t.Errorf("Failed to install API: %v", err) + t.FailNow() + } + } else { + t.Errorf("Expected %s: %v not to be present!", names[ix], api) } } @@ -1078,7 +1086,7 @@ func testInstallThirdPartyResourceRemove(t *testing.T, version string) { } path := makeThirdPartyPath("company.com") - master.RemoveThirdPartyResource(path) + master.RemoveThirdPartyResource(path + "/foos") resp, err = http.Get(server.URL + "/apis/company.com/" + version + "/namespaces/default/foos/test") if !assert.NoError(err) { diff --git a/pkg/master/thirdparty_controller.go b/pkg/master/thirdparty_controller.go index ed49257350a..defd84ab3c6 100644 --- a/pkg/master/thirdparty_controller.go +++ b/pkg/master/thirdparty_controller.go @@ -21,7 +21,9 @@ import ( "strings" "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/unversioned" expapi "k8s.io/kubernetes/pkg/apis/extensions" + "k8s.io/kubernetes/pkg/apiserver" thirdpartyresourceetcd "k8s.io/kubernetes/pkg/registry/thirdpartyresource/etcd" "k8s.io/kubernetes/pkg/registry/thirdpartyresourcedata" "k8s.io/kubernetes/pkg/runtime" @@ -30,6 +32,19 @@ import ( const thirdpartyprefix = "/apis" +// dynamicLister is used to list resources for dynamic third party +// apis. It implements the apiserver.APIResourceLister interface +type dynamicLister struct { + m *Master + path string +} + +func (d dynamicLister) ListAPIResources() []unversioned.APIResource { + return d.m.getExistingThirdPartyResources(d.path) +} + +var _ apiserver.APIResourceLister = &dynamicLister{} + func makeThirdPartyPath(group string) string { if len(group) == 0 { return thirdpartyprefix @@ -44,13 +59,14 @@ func getThirdPartyGroupName(path string) string { // resourceInterface is the interface for the parts of the master that know how to add/remove // third party resources. Extracted into an interface for injection for testing. type resourceInterface interface { - // Remove a third party resource based on the RESTful path for that resource + // Remove a third party resource based on the RESTful path for that resource, the path is / RemoveThirdPartyResource(path string) error // Install a third party resource described by 'rsrc' InstallThirdPartyResource(rsrc *expapi.ThirdPartyResource) error // Is a particular third party resource currently installed? HasThirdPartyResource(rsrc *expapi.ThirdPartyResource) (bool, error) - // List all currently installed third party resources + // List all currently installed third party resources, the returned + // names are of the form / ListThirdPartyResources() []string }