Fix third party APIResource reporting

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 #

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()})
 	}
 }
 

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]
+	entry := m.thirdPartyResources[path]
-	if found {
+	if entry == nil {
-		if err := m.removeAllThirdPartyResources(storage.storage); err != nil {
+		return false, nil
-			return err
+	}
-		}
+	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 <api-group>/<resource-plural-name>, 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 <path>/<resource-plural-name>
 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}
+	entry, found := m.thirdPartyResources[path]
-	m.AddAPIGroupForDiscovery(apiGroup)
+	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)},
 	}
 }
 

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)
+		hasRsrc, err := master.HasThirdPartyResource(api)
-		if !assert.NoError(err) {
+		if err != nil {
-			t.Logf("Failed to install API: %v", err)
+			t.Errorf("Unexpected error: %v", err)
-			t.FailNow()
+		}
+		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) {

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 <api-group-path>/<resource-plural-name>
 	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 <api-group-path>/<resource-plural-name>
 	ListThirdPartyResources() []string
 }