Merge pull request #28431 from brendandburns/thirdparty2

Automatic merge from submit-queue

Fix a problem with multiple APIs clobbering each other in registration.

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

@kubernetes/sig-api-machinery 

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

pkg/apimachinery/registered/registered.go

@@ -55,6 +55,10 @@ var (
 )
 
 func init() {
+	loadKubeAPIVersions()
+}
+
+func loadKubeAPIVersions() {
 	// Env var KUBE_API_VERSIONS is a comma separated list of API versions that
 	// should be registered in the scheme.
 	kubeAPIVersions := os.Getenv("KUBE_API_VERSIONS")
@@ -70,6 +74,16 @@ func init() {
 	}
 }
 
+// Resets everything to clean room for the start of a test
+func clearForTesting() {
+	registeredVersions = map[unversioned.GroupVersion]struct{}{}
+	thirdPartyGroupVersions = []unversioned.GroupVersion{}
+	enabledVersions = map[unversioned.GroupVersion]struct{}{}
+	groupMetaMap = map[string]*apimachinery.GroupMeta{}
+	envRequestedVersions = []unversioned.GroupVersion{}
+	loadKubeAPIVersions()
+}
+
 // RegisterVersions adds the given group versions to the list of registered group versions.
 func RegisterVersions(availableVersions []unversioned.GroupVersion) {
 	for _, v := range availableVersions {
@@ -207,12 +221,7 @@ func AddThirdPartyAPIGroupVersions(gvs ...unversioned.GroupVersion) []unversione
 	}
 	RegisterVersions(filteredGVs)
 	EnableVersions(filteredGVs...)
-	next := make([]unversioned.GroupVersion, len(gvs))
-	for ix := range filteredGVs {
-		next[ix] = filteredGVs[ix]
-	}
-	thirdPartyGroupVersions = next
-
+	thirdPartyGroupVersions = append(thirdPartyGroupVersions, filteredGVs...)
 	return skippedGVs
 }
 

pkg/apimachinery/registered/registered_test.go

@@ -23,6 +23,78 @@ import (
 	"k8s.io/kubernetes/pkg/apimachinery"
 )
 
+func TestAddThirdPartyVersionsBasic(t *testing.T) {
+	clearForTesting()
+
+	registered := []unversioned.GroupVersion{
+		{
+			Group:   "",
+			Version: "v1",
+		},
+	}
+	skipped := registered
+	thirdParty := []unversioned.GroupVersion{
+		{
+			Group:   "company.com",
+			Version: "v1",
+		},
+		{
+			Group:   "company.com",
+			Version: "v2",
+		},
+	}
+	gvs := append(registered, thirdParty...)
+
+	RegisterVersions(registered)
+	wasSkipped := AddThirdPartyAPIGroupVersions(gvs...)
+	if len(wasSkipped) != len(skipped) {
+		t.Errorf("Expected %v, found %v", skipped, wasSkipped)
+	}
+	for ix := range wasSkipped {
+		found := false
+		for _, gv := range skipped {
+			if gv.String() == wasSkipped[ix].String() {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Errorf("Couldn't find %v in %v", wasSkipped[ix], skipped)
+		}
+	}
+	for _, gv := range thirdParty {
+		if !IsThirdPartyAPIGroupVersion(gv) {
+			t.Errorf("Expected %v to be third party.", gv)
+		}
+	}
+}
+
+func TestAddThirdPartyVersionsMultiple(t *testing.T) {
+	clearForTesting()
+
+	thirdParty := []unversioned.GroupVersion{
+		{
+			Group:   "company.com",
+			Version: "v1",
+		},
+		{
+			Group:   "company.com",
+			Version: "v2",
+		},
+	}
+	for _, gv := range thirdParty {
+		wasSkipped := AddThirdPartyAPIGroupVersions(gv)
+		if len(wasSkipped) != 0 {
+			t.Errorf("Expected length 0, found %v", wasSkipped)
+		}
+	}
+	for _, gv := range thirdParty {
+		if !IsThirdPartyAPIGroupVersion(gv) {
+			t.Errorf("Expected %v to be third party.", gv)
+		}
+	}
+}
+
 func TestAllPreferredGroupVersions(t *testing.T) {
 	testCases := []struct {
 		groupMetas []apimachinery.GroupMeta