Merge pull request #25374 from brendandburns/plural

Automatic merge from submit-queue

Fix a bug with pluralization of third party resources

Fixes https://github.com/kubernetes/kubernetes/issues/25129

@kubernetes/sig-api-machinery 

[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]()

pkg/master/master.go

@@ -28,6 +28,7 @@ import (
 	"time"
 
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/meta"
 	"k8s.io/kubernetes/pkg/api/rest"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	apiv1 "k8s.io/kubernetes/pkg/api/v1"
@@ -749,7 +750,13 @@ func (m *Master) InstallThirdPartyResource(rsrc *extensions.ThirdPartyResource)
 	if err != nil {
 		return err
 	}
-	thirdparty := m.thirdpartyapi(group, kind, rsrc.Versions[0].Name)
+	plural, _ := meta.KindToResource(unversioned.GroupVersionKind{
+		Group:   group,
+		Version: rsrc.Versions[0].Name,
+		Kind:    kind,
+	})
+
+	thirdparty := m.thirdpartyapi(group, kind, rsrc.Versions[0].Name, plural.Resource)
 	if err := thirdparty.InstallREST(m.HandlerContainer); err != nil {
 		glog.Fatalf("Unable to setup thirdparty api: %v", err)
 	}
@@ -764,12 +771,13 @@ func (m *Master) InstallThirdPartyResource(rsrc *extensions.ThirdPartyResource)
 		PreferredVersion: groupVersion,
 	}
 	apiserver.AddGroupWebService(api.Codecs, m.HandlerContainer, path, apiGroup)
-	m.addThirdPartyResourceStorage(path, thirdparty.Storage[strings.ToLower(kind)+"s"].(*thirdpartyresourcedataetcd.REST), apiGroup)
+
+	m.addThirdPartyResourceStorage(path, thirdparty.Storage[plural.Resource].(*thirdpartyresourcedataetcd.REST), apiGroup)
 	apiserver.InstallServiceErrorHandler(api.Codecs, m.HandlerContainer, m.NewRequestInfoResolver(), []string{thirdparty.GroupVersion.String()})
 	return nil
 }
 
-func (m *Master) thirdpartyapi(group, kind, version string) *apiserver.APIGroupVersion {
+func (m *Master) thirdpartyapi(group, kind, version, pluralResource string) *apiserver.APIGroupVersion {
 	resourceStorage := thirdpartyresourcedataetcd.NewREST(
 		generic.RESTOptions{
 			Storage:                 m.thirdPartyStorage,
@@ -783,7 +791,7 @@ func (m *Master) thirdpartyapi(group, kind, version string) *apiserver.APIGroupV
 	apiRoot := makeThirdPartyPath("")
 
 	storage := map[string]rest.Storage{
-		strings.ToLower(kind) + "s": resourceStorage,
+		pluralResource: resourceStorage,
 	}
 
 	optionsExternalVersion := registered.GroupOrDie(api.GroupName).GroupVersion

pkg/master/master_test.go

@@ -30,6 +30,7 @@ import (
 	"testing"
 
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/meta"
 	"k8s.io/kubernetes/pkg/api/testapi"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	apiv1 "k8s.io/kubernetes/pkg/api/v1"
@@ -50,6 +51,7 @@ import (
 	"k8s.io/kubernetes/pkg/registry/endpoint"
 	"k8s.io/kubernetes/pkg/registry/namespace"
 	"k8s.io/kubernetes/pkg/registry/registrytest"
+	"k8s.io/kubernetes/pkg/registry/thirdpartyresourcedata"
 	"k8s.io/kubernetes/pkg/runtime"
 	"k8s.io/kubernetes/pkg/storage"
 	"k8s.io/kubernetes/pkg/storage/etcd/etcdtest"
@@ -527,12 +529,12 @@ type FooList struct {
 	Items []Foo `json:"items"`
 }
 
-func initThirdParty(t *testing.T, version string) (*Master, *etcdtesting.EtcdTestServer, *httptest.Server, *assert.Assertions) {
+func initThirdParty(t *testing.T, version, name string) (*Master, *etcdtesting.EtcdTestServer, *httptest.Server, *assert.Assertions) {
 	master, etcdserver, _, assert := newMaster(t)
 
 	api := &extensions.ThirdPartyResource{
 		ObjectMeta: api.ObjectMeta{
-			Name: "foo.company.com",
+			Name: name,
 		},
 		Versions: []extensions.APIVersion{
 			{
@@ -559,10 +561,22 @@ func TestInstallThirdPartyAPIList(t *testing.T) {
 func testInstallThirdPartyAPIListVersion(t *testing.T, version string) {
 	tests := []struct {
 		items []Foo
+		name  string
+		test  string
 	}{
-		{},
+		{
+			name: "foo.company.com",
+			test: "null",
+		},
 		{
 			items: []Foo{},
+			name:  "foo.company.com",
+			test:  "empty",
+		},
+		{
+			items: []Foo{},
+			name:  "policy.company.com",
+			test:  "plurals",
 		},
 		{
 			items: []Foo{
@@ -589,46 +603,61 @@ func testInstallThirdPartyAPIListVersion(t *testing.T, version string) {
 					OtherField: 20,
 				},
 			},
+			name: "foo.company.com",
+			test: "real list",
 		},
 	}
 	for _, test := range tests {
 		func() {
-			master, etcdserver, server, assert := initThirdParty(t, version)
+			master, etcdserver, server, assert := initThirdParty(t, version, test.name)
 			defer server.Close()
 			defer etcdserver.Terminate(t)
 
+			kind, group, err := thirdpartyresourcedata.ExtractApiGroupAndKind(
+				&extensions.ThirdPartyResource{ObjectMeta: api.ObjectMeta{Name: test.name}})
+			assert.NoError(err, test.test)
+
+			plural, _ := meta.KindToResource(unversioned.GroupVersionKind{
+				Group:   group,
+				Version: version,
+				Kind:    kind,
+			})
+
 			if test.items != nil {
-				err := createThirdPartyList(master.thirdPartyStorage, "/ThirdPartyResourceData/company.com/foos/default", test.items)
+				err := createThirdPartyList(master.thirdPartyStorage,
-				if !assert.NoError(err) {
+					fmt.Sprintf("/ThirdPartyResourceData/%s/%s/default", group, plural.Resource),
+					test.items)
+				if !assert.NoError(err, test.test) {
 					return
 				}
 			}
 
-			resp, err := http.Get(server.URL + "/apis/company.com/" + version + "/namespaces/default/foos")
+			resp, err := http.Get(
-			if !assert.NoError(err) {
+				fmt.Sprintf("%s/apis/%s/%s/namespaces/default/%s", server.URL, group, version, plural.Resource))
+			if !assert.NoError(err, test.test) {
 				return
 			}
 			defer resp.Body.Close()
 
-			assert.Equal(http.StatusOK, resp.StatusCode)
+			assert.Equal(http.StatusOK, resp.StatusCode, test.test)
 
 			data, err := ioutil.ReadAll(resp.Body)
-			assert.NoError(err)
+			assert.NoError(err, test.test)
 
 			list := FooList{}
 			if err = json.Unmarshal(data, &list); err != nil {
-				t.Errorf("unexpected error: %v", err)
+				assert.NoError(err, "unexpected error: %v %s", err, test.test)
 			}
 
 			if test.items == nil {
 				if len(list.Items) != 0 {
-					t.Errorf("expected no items, saw: %v", list.Items)
+					assert.NoError(err, "expected no items, saw: %v %s", err, list.Items, test.test)
 				}
 				return
 			}
 
 			if len(list.Items) != len(test.items) {
-				t.Fatalf("unexpected length: %d vs %d", len(list.Items), len(test.items))
+				t.Fatalf("(%s) unexpected length: %d vs %d", test.name, len(list.Items), len(test.items))
 			}
 			// The order of elements in LIST is not guaranteed.
 			mapping := make(map[string]int)
@@ -647,7 +676,7 @@ func testInstallThirdPartyAPIListVersion(t *testing.T, version string) {
 				// We endure the order of items by sorting them (using 'mapping')
 				// so that this function passes.
 				if !reflect.DeepEqual(list.Items[ix], expectedObj) {
-					t.Errorf("expected:\n%#v\nsaw:\n%#v\n", expectedObj, list.Items[ix])
+					t.Errorf("(%s) expected:\n%#v\nsaw:\n%#v\n", test.name, expectedObj, list.Items[ix])
 				}
 			}
 		}()
@@ -703,7 +732,7 @@ func TestInstallThirdPartyAPIGet(t *testing.T) {
 }
 
 func testInstallThirdPartyAPIGetVersion(t *testing.T, version string) {
-	master, etcdserver, server, assert := initThirdParty(t, version)
+	master, etcdserver, server, assert := initThirdParty(t, version, "foo.company.com")
 	defer server.Close()
 	defer etcdserver.Terminate(t)
 
@@ -750,7 +779,7 @@ func TestInstallThirdPartyAPIPost(t *testing.T) {
 }
 
 func testInstallThirdPartyAPIPostForVersion(t *testing.T, version string) {
-	master, etcdserver, server, assert := initThirdParty(t, version)
+	master, etcdserver, server, assert := initThirdParty(t, version, "foo.company.com")
 	defer server.Close()
 	defer etcdserver.Terminate(t)
 
@@ -814,7 +843,7 @@ func TestInstallThirdPartyAPIDelete(t *testing.T) {
 }
 
 func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) {
-	master, etcdserver, server, assert := initThirdParty(t, version)
+	master, etcdserver, server, assert := initThirdParty(t, version, "foo.company.com")
 	defer server.Close()
 	defer etcdserver.Terminate(t)
 
@@ -891,7 +920,7 @@ func TestInstallThirdPartyAPIListOptions(t *testing.T) {
 }
 
 func testInstallThirdPartyAPIListOptionsForVersion(t *testing.T, version string) {
-	_, etcdserver, server, assert := initThirdParty(t, version)
+	_, etcdserver, server, assert := initThirdParty(t, version, "foo.company.com")
 	defer server.Close()
 	defer etcdserver.Terminate(t)
 
@@ -923,7 +952,7 @@ func TestInstallThirdPartyResourceRemove(t *testing.T) {
 }
 
 func testInstallThirdPartyResourceRemove(t *testing.T, version string) {
-	master, etcdserver, server, assert := initThirdParty(t, version)
+	master, etcdserver, server, assert := initThirdParty(t, version, "foo.company.com")
 	defer server.Close()
 	defer etcdserver.Terminate(t)
 
@@ -1044,7 +1073,7 @@ func TestIsTunnelSyncHealthy(t *testing.T) {
 }
 
 func testThirdPartyDiscovery(t *testing.T, version string) {
-	_, etcdserver, server, assert := initThirdParty(t, version)
+	_, etcdserver, server, assert := initThirdParty(t, version, "foo.company.com")
 	defer server.Close()
 	defer etcdserver.Terminate(t)