Merge pull request kubernetes#29724 from brendandburns/thirdparty3

Automatic merge from submit-queue Fix third party APIResource reporting @polvi @caesarxuchao @deads2k This "fixes" some additional bugs in third party `APIResourceList` reporting. This code needs a bunch of cleanup, and more tests, but sending it out for a quick smell check review in case I'm doing something stupid. Fixes the bug referenced here: kubernetes#28414 (comment) and in kubernetes#23831 Fixes kubernetes#25570
lavalamp · Aug 14, 2016 · 3a8b21b · 3a8b21b
2 parents 817256a + b3658c7
commit 3a8b21b
Show file tree

Hide file tree

Showing 5 changed files with 208 additions and 52 deletions.
diff --git 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[@]}"
 
   #################
   # Run cmd w img #

diff --git 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,14 +124,29 @@ 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.
 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,15 +363,15 @@ 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
 		// with empty APIVersion, we use StripVersionNegotiatedSerializer to
 		// 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()})
 	}
 }