Merge pull request #119835 from liggitt/mitigate-aggregated-discovery-npe

Avoid returning nil responseKind in v1beta1 aggregated discovery

pkg/apis/apidiscovery/types.go

@@ -85,7 +85,7 @@ type APIResourceDiscovery struct {
 	Resource string
 	// responseKind describes the group, version, and kind of the serialization schema for the object type this endpoint typically returns.
 	// APIs may return other objects types at their discretion, such as error conditions, requests for alternate representations, or other operation specific behavior.
-	// This value will be null if an APIService reports subresources but supports no operations on the parent resource
+	// This value will be null or empty if an APIService reports subresources but supports no operations on the parent resource
 	ResponseKind *v1.GroupVersionKind
 	// scope indicates the scope of a resource, either Cluster or Namespaced
 	Scope ResourceScope
@@ -134,7 +134,7 @@ type APISubresourceDiscovery struct {
 	// for this resource across all versions.
 	Subresource string
 	// responseKind describes the group, version, and kind of the serialization schema for the object type this endpoint typically returns.
-	// Some subresources do not return normal resources, these will have null return types.
+	// Some subresources do not return normal resources, these will have null or empty return types.
 	ResponseKind *v1.GroupVersionKind
 	// acceptedTypes describes the kinds that this endpoint accepts.
 	// Subresources may accept the standard content types or define

pkg/generated/openapi/zz_generated.openapi.go

@@ -4428,7 +4428,7 @@ func schema_k8sio_api_apidiscovery_v2beta1_APIResourceDiscovery(ref common.Refer
 					},
 					"responseKind": {
 						SchemaProps: spec.SchemaProps{
-							Description: "responseKind describes the group, version, and kind of the serialization schema for the object type this endpoint typically returns. APIs may return other objects types at their discretion, such as error conditions, requests for alternate representations, or other operation specific behavior. This value will be null if an APIService reports subresources but supports no operations on the parent resource",
+							Description: "responseKind describes the group, version, and kind of the serialization schema for the object type this endpoint typically returns. APIs may return other objects types at their discretion, such as error conditions, requests for alternate representations, or other operation specific behavior. This value will be null or empty if an APIService reports subresources but supports no operations on the parent resource",
 							Ref:         ref("k8s.io/apimachinery/pkg/apis/meta/v1.GroupVersionKind"),
 						},
 					},
@@ -4556,7 +4556,7 @@ func schema_k8sio_api_apidiscovery_v2beta1_APISubresourceDiscovery(ref common.Re
 					},
 					"responseKind": {
 						SchemaProps: spec.SchemaProps{
-							Description: "responseKind describes the group, version, and kind of the serialization schema for the object type this endpoint typically returns. Some subresources do not return normal resources, these will have null return types.",
+							Description: "responseKind describes the group, version, and kind of the serialization schema for the object type this endpoint typically returns. Some subresources do not return normal resources, these will have null or empty return types.",
 							Ref:         ref("k8s.io/apimachinery/pkg/apis/meta/v1.GroupVersionKind"),
 						},
 					},

staging/src/k8s.io/api/apidiscovery/v2beta1/generated.proto

@@ -71,7 +71,7 @@ message APIResourceDiscovery {
 
   // responseKind describes the group, version, and kind of the serialization schema for the object type this endpoint typically returns.
   // APIs may return other objects types at their discretion, such as error conditions, requests for alternate representations, or other operation specific behavior.
-  // This value will be null if an APIService reports subresources but supports no operations on the parent resource
+  // This value will be null or empty if an APIService reports subresources but supports no operations on the parent resource
   optional k8s.io.apimachinery.pkg.apis.meta.v1.GroupVersionKind responseKind = 2;
 
   // scope indicates the scope of a resource, either Cluster or Namespaced
@@ -111,7 +111,7 @@ message APISubresourceDiscovery {
   optional string subresource = 1;
 
   // responseKind describes the group, version, and kind of the serialization schema for the object type this endpoint typically returns.
-  // Some subresources do not return normal resources, these will have null return types.
+  // Some subresources do not return normal resources, these will have null or empty return types.
   optional k8s.io.apimachinery.pkg.apis.meta.v1.GroupVersionKind responseKind = 2;
 
   // acceptedTypes describes the kinds that this endpoint accepts.

staging/src/k8s.io/api/apidiscovery/v2beta1/types.go

@@ -92,7 +92,7 @@ type APIResourceDiscovery struct {
 	Resource string `json:"resource" protobuf:"bytes,1,opt,name=resource"`
 	// responseKind describes the group, version, and kind of the serialization schema for the object type this endpoint typically returns.
 	// APIs may return other objects types at their discretion, such as error conditions, requests for alternate representations, or other operation specific behavior.
-	// This value will be null if an APIService reports subresources but supports no operations on the parent resource
+	// This value will be null or empty if an APIService reports subresources but supports no operations on the parent resource
 	ResponseKind *v1.GroupVersionKind `json:"responseKind,omitempty" protobuf:"bytes,2,opt,name=responseKind"`
 	// scope indicates the scope of a resource, either Cluster or Namespaced
 	Scope ResourceScope `json:"scope" protobuf:"bytes,3,opt,name=scope"`
@@ -141,7 +141,7 @@ type APISubresourceDiscovery struct {
 	// for this resource across all versions.
 	Subresource string `json:"subresource" protobuf:"bytes,1,opt,name=subresource"`
 	// responseKind describes the group, version, and kind of the serialization schema for the object type this endpoint typically returns.
-	// Some subresources do not return normal resources, these will have null return types.
+	// Some subresources do not return normal resources, these will have null or empty return types.
 	ResponseKind *v1.GroupVersionKind `json:"responseKind,omitempty" protobuf:"bytes,2,opt,name=responseKind"`
 	// acceptedTypes describes the kinds that this endpoint accepts.
 	// Subresources may accept the standard content types or define

staging/src/k8s.io/apiserver/pkg/endpoints/installer.go

@@ -127,6 +127,9 @@ func ConvertGroupVersionIntoToDiscovery(list []metav1.APIResource) ([]apidiscove
 			apiResourceList = append(apiResourceList, apidiscoveryv2beta1.APIResourceDiscovery{
 				Resource: split[0],
 				Scope:    scope,
+				// avoid nil panics in v0.26.0-v0.26.3 client-go clients
+				// see https://github.com/kubernetes/kubernetes/issues/118361
+				ResponseKind: &metav1.GroupVersionKind{},
 			})
 			parentidx = len(apiResourceList) - 1
 			parentResources[split[0]] = parentidx
@@ -140,6 +143,9 @@ func ConvertGroupVersionIntoToDiscovery(list []metav1.APIResource) ([]apidiscove
 		subresource := apidiscoveryv2beta1.APISubresourceDiscovery{
 			Subresource: split[1],
 			Verbs:       r.Verbs,
+			// avoid nil panics in v0.26.0-v0.26.3 client-go clients
+			// see https://github.com/kubernetes/kubernetes/issues/118361
+			ResponseKind: &metav1.GroupVersionKind{},
 		}
 		if r.Kind != "" {
 			subresource.ResponseKind = &metav1.GroupVersionKind{

staging/src/k8s.io/apiserver/pkg/endpoints/installer_test.go

@@ -302,6 +302,8 @@ func TestConvertAPIResourceToDiscovery(t *testing.T) {
 				{
 					Resource: "cronjobs",
 					Scope:    apidiscoveryv2beta1.ScopeNamespace,
+					// populated to avoid nil panics
+					ResponseKind: &metav1.GroupVersionKind{},
 					Subresources: []apidiscoveryv2beta1.APISubresourceDiscovery{{
 						Subresource: "status",
 						ResponseKind: &metav1.GroupVersionKind{
@@ -314,6 +316,32 @@ func TestConvertAPIResourceToDiscovery(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "Test with subresource with missing kind",
+			resources: []metav1.APIResource{
+				{
+					Name:       "cronjobs/status",
+					Namespaced: true,
+					Group:      "batch",
+					Version:    "v1",
+					Verbs:      []string{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"},
+				},
+			},
+			wantAPIResourceDiscovery: []apidiscoveryv2beta1.APIResourceDiscovery{
+				{
+					Resource: "cronjobs",
+					Scope:    apidiscoveryv2beta1.ScopeNamespace,
+					// populated to avoid nil panics
+					ResponseKind: &metav1.GroupVersionKind{},
+					Subresources: []apidiscoveryv2beta1.APISubresourceDiscovery{{
+						Subresource: "status",
+						// populated to avoid nil panics
+						ResponseKind: &metav1.GroupVersionKind{},
+						Verbs:        []string{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"},
+					}},
+				},
+			},
+		},
 		{
 			name: "Test with mismatch parent and subresource scope",
 			resources: []metav1.APIResource{

staging/src/k8s.io/client-go/discovery/aggregated_discovery.go

@@ -111,6 +111,8 @@ func convertAPIGroup(g apidiscovery.APIGroupDiscovery) (
 	return group, gvResources, failedGVs
 }
 
+var emptyKind = metav1.GroupVersionKind{}
+
 // convertAPIResource tranforms a APIResourceDiscovery to an APIResource. We are
 // resilient to missing GVK, since this resource might be the parent resource
 // for a subresource. If the parent is missing a GVK, it is not returned in
@@ -125,7 +127,7 @@ func convertAPIResource(in apidiscovery.APIResourceDiscovery) (metav1.APIResourc
 		Categories:   in.Categories,
 	}
 	var err error
-	if in.ResponseKind != nil {
+	if in.ResponseKind != nil && (*in.ResponseKind) != emptyKind {
 		result.Group = in.ResponseKind.Group
 		result.Version = in.ResponseKind.Version
 		result.Kind = in.ResponseKind.Kind
@@ -140,7 +142,7 @@ func convertAPIResource(in apidiscovery.APIResourceDiscovery) (metav1.APIResourc
 // convertAPISubresource tranforms a APISubresourceDiscovery to an APIResource.
 func convertAPISubresource(parent metav1.APIResource, in apidiscovery.APISubresourceDiscovery) (metav1.APIResource, error) {
 	result := metav1.APIResource{}
-	if in.ResponseKind == nil {
+	if in.ResponseKind == nil || (*in.ResponseKind) == emptyKind {
 		return result, fmt.Errorf("subresource %s/%s missing GVK", parent.Name, in.Subresource)
 	}
 	result.Name = fmt.Sprintf("%s/%s", parent.Name, in.Subresource)

staging/src/k8s.io/client-go/discovery/aggregated_discovery_test.go

@@ -610,6 +610,76 @@ func TestSplitGroupsAndResources(t *testing.T) {
 			},
 			expectedFailedGVs: map[schema.GroupVersion]error{},
 		},
+		{
+			name: "Aggregated discovery with single subresource and parent empty GVK",
+			agg: apidiscovery.APIGroupDiscoveryList{
+				Items: []apidiscovery.APIGroupDiscovery{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "external.metrics.k8s.io",
+						},
+						Versions: []apidiscovery.APIVersionDiscovery{
+							{
+								Version: "v1beta1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										// resilient to empty GVK for parent
+										Resource:         "*",
+										Scope:            apidiscovery.ScopeNamespace,
+										SingularResource: "",
+										ResponseKind:     &metav1.GroupVersionKind{},
+										Subresources: []apidiscovery.APISubresourceDiscovery{
+											{
+												Subresource: "other-external-metric",
+												ResponseKind: &metav1.GroupVersionKind{
+													Kind: "MetricValueList",
+												},
+												Verbs: []string{"get"},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedGroups: metav1.APIGroupList{
+				Groups: []metav1.APIGroup{
+					{
+						Name: "external.metrics.k8s.io",
+						Versions: []metav1.GroupVersionForDiscovery{
+							{
+								GroupVersion: "external.metrics.k8s.io/v1beta1",
+								Version:      "v1beta1",
+							},
+						},
+						PreferredVersion: metav1.GroupVersionForDiscovery{
+							GroupVersion: "external.metrics.k8s.io/v1beta1",
+							Version:      "v1beta1",
+						},
+					},
+				},
+			},
+			expectedGVResources: map[schema.GroupVersion]*metav1.APIResourceList{
+				{Group: "external.metrics.k8s.io", Version: "v1beta1"}: {
+					GroupVersion: "external.metrics.k8s.io/v1beta1",
+					APIResources: []metav1.APIResource{
+						// Since parent GVK was nil, it is NOT returned--only the subresource.
+						{
+							Name:         "*/other-external-metric",
+							SingularName: "",
+							Namespaced:   true,
+							Group:        "",
+							Version:      "",
+							Kind:         "MetricValueList",
+							Verbs:        []string{"get"},
+						},
+					},
+				},
+			},
+			expectedFailedGVs: map[schema.GroupVersion]error{},
+		},
 		{
 			name: "Aggregated discovery with multiple subresources",
 			agg: apidiscovery.APIGroupDiscoveryList{

staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_discovery.go

@@ -309,6 +309,18 @@ func (dm *discoveryManager) fetchFreshDiscoveryForService(gv metav1.GroupVersion
 		for _, g := range parsed.Items {
 			for _, v := range g.Versions {
 				discoMap[metav1.GroupVersion{Group: g.Name, Version: v.Version}] = v
+				for i := range v.Resources {
+					// avoid nil panics in v0.26.0-v0.26.3 client-go clients
+					// see https://github.com/kubernetes/kubernetes/issues/118361
+					if v.Resources[i].ResponseKind == nil {
+						v.Resources[i].ResponseKind = &metav1.GroupVersionKind{}
+					}
+					for j := range v.Resources[i].Subresources {
+						if v.Resources[i].Subresources[j].ResponseKind == nil {
+							v.Resources[i].Subresources[j].ResponseKind = &metav1.GroupVersionKind{}
+						}
+					}
+				}
 			}
 		}
 

staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_discovery_test.go

@@ -18,9 +18,10 @@ package apiserver
 
 import (
 	"context"
+	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
-	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -28,8 +29,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
 	fuzz "github.com/google/gofuzz"
 	"github.com/stretchr/testify/require"
+
 	apidiscoveryv2beta1 "k8s.io/api/apidiscovery/v2beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -62,22 +65,94 @@ func waitForQueueComplete(stopCh <-chan struct{}, dm *discoveryManager) bool {
 func TestBasic(t *testing.T) {
 	service1 := discoveryendpoint.NewResourceManager("apis")
 	service2 := discoveryendpoint.NewResourceManager("apis")
+	service3 := discoveryendpoint.NewResourceManager("apis")
 	apiGroup1 := fuzzAPIGroups(2, 5, 25)
 	apiGroup2 := fuzzAPIGroups(2, 5, 50)
+	apiGroup3 := apidiscoveryv2beta1.APIGroupDiscoveryList{Items: []apidiscoveryv2beta1.APIGroupDiscovery{
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "weird.example.com"},
+			Versions: []apidiscoveryv2beta1.APIVersionDiscovery{
+				{
+					Version:   "v1",
+					Freshness: "Current",
+					Resources: []apidiscoveryv2beta1.APIResourceDiscovery{
+						{
+							Resource: "parent-missing-kind",
+							Subresources: []apidiscoveryv2beta1.APISubresourceDiscovery{
+								{Subresource: "subresource-missing-kind"},
+							},
+						},
+						{
+							Resource:     "parent-empty-kind",
+							ResponseKind: &metav1.GroupVersionKind{},
+							Subresources: []apidiscoveryv2beta1.APISubresourceDiscovery{
+								{Subresource: "subresource-empty-kind", ResponseKind: &metav1.GroupVersionKind{}},
+							},
+						},
+						{
+							Resource:     "parent-with-kind",
+							ResponseKind: &metav1.GroupVersionKind{Kind: "ParentWithKind"},
+							Subresources: []apidiscoveryv2beta1.APISubresourceDiscovery{
+								{Subresource: "subresource-with-kind", ResponseKind: &metav1.GroupVersionKind{Kind: "SubresourceWithKind"}},
+							},
+						},
+					},
+				},
+			},
+		},
+	}}
+	apiGroup3WithFixup := apidiscoveryv2beta1.APIGroupDiscoveryList{Items: []apidiscoveryv2beta1.APIGroupDiscovery{
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "weird.example.com"},
+			Versions: []apidiscoveryv2beta1.APIVersionDiscovery{
+				{
+					Version:   "v1",
+					Freshness: "Current",
+					Resources: []apidiscoveryv2beta1.APIResourceDiscovery{
+						{
+							Resource:     "parent-missing-kind",
+							ResponseKind: &metav1.GroupVersionKind{}, // defaulted by aggregator
+							Subresources: []apidiscoveryv2beta1.APISubresourceDiscovery{
+								{Subresource: "subresource-missing-kind", ResponseKind: &metav1.GroupVersionKind{}}, // defaulted by aggregator
+							},
+						},
+						{
+							Resource:     "parent-empty-kind",
+							ResponseKind: &metav1.GroupVersionKind{},
+							Subresources: []apidiscoveryv2beta1.APISubresourceDiscovery{
+								{Subresource: "subresource-empty-kind", ResponseKind: &metav1.GroupVersionKind{}},
+							},
+						},
+						{
+							Resource:     "parent-with-kind",
+							ResponseKind: &metav1.GroupVersionKind{Kind: "ParentWithKind"},
+							Subresources: []apidiscoveryv2beta1.APISubresourceDiscovery{
+								{Subresource: "subresource-with-kind", ResponseKind: &metav1.GroupVersionKind{Kind: "SubresourceWithKind"}},
+							},
+						},
+					},
+				},
+			},
+		},
+	}}
 	service1.SetGroups(apiGroup1.Items)
 	service2.SetGroups(apiGroup2.Items)
+	service3.SetGroups(apiGroup3.Items)
 	aggregatedResourceManager := discoveryendpoint.NewResourceManager("apis")
 	aggregatedManager := newDiscoveryManager(aggregatedResourceManager)
 
 	for _, g := range apiGroup1.Items {
+		versionPriority := int32(len(g.Versions) + 1)
 		for _, v := range g.Versions {
+			versionPriority--
 			aggregatedManager.AddAPIService(&apiregistrationv1.APIService{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: v.Version + "." + g.Name,
 				},
 				Spec: apiregistrationv1.APIServiceSpec{
-					Group:   g.Name,
-					Version: v.Version,
+					Group:           g.Name,
+					Version:         v.Version,
+					VersionPriority: versionPriority,
 					Service: &apiregistrationv1.ServiceReference{
 						Name: "service1",
 					},
@@ -87,14 +162,17 @@ func TestBasic(t *testing.T) {
 	}
 
 	for _, g := range apiGroup2.Items {
+		versionPriority := int32(len(g.Versions) + 1)
 		for _, v := range g.Versions {
+			versionPriority--
 			aggregatedManager.AddAPIService(&apiregistrationv1.APIService{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: v.Version + "." + g.Name,
 				},
 				Spec: apiregistrationv1.APIServiceSpec{
-					Group:   g.Name,
-					Version: v.Version,
+					Group:           g.Name,
+					Version:         v.Version,
+					VersionPriority: versionPriority,
 					Service: &apiregistrationv1.ServiceReference{
 						Name: "service2",
 					},
@@ -103,6 +181,26 @@ func TestBasic(t *testing.T) {
 		}
 	}
 
+	for _, g := range apiGroup3.Items {
+		versionPriority := int32(len(g.Versions) + 1)
+		for _, v := range g.Versions {
+			versionPriority--
+			aggregatedManager.AddAPIService(&apiregistrationv1.APIService{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: v.Version + "." + g.Name,
+				},
+				Spec: apiregistrationv1.APIServiceSpec{
+					Group:           g.Name,
+					Version:         v.Version,
+					VersionPriority: versionPriority,
+					Service: &apiregistrationv1.ServiceReference{
+						Name: "service3",
+					},
+				},
+			}, service3)
+		}
+	}
+
 	testCtx, testCancel := context.WithCancel(context.Background())
 	defer testCancel()
 
@@ -116,9 +214,11 @@ func TestBasic(t *testing.T) {
 	}
 	checkAPIGroups(t, apiGroup1, parsed)
 	checkAPIGroups(t, apiGroup2, parsed)
+	checkAPIGroups(t, apiGroup3WithFixup, parsed)
 }
 
 func checkAPIGroups(t *testing.T, api apidiscoveryv2beta1.APIGroupDiscoveryList, response *apidiscoveryv2beta1.APIGroupDiscoveryList) {
+	t.Helper()
 	if len(response.Items) < len(api.Items) {
 		t.Errorf("expected to check for at least %d groups, only have %d groups in response", len(api.Items), len(response.Items))
 	}
@@ -128,6 +228,10 @@ func checkAPIGroups(t *testing.T, api apidiscoveryv2beta1.APIGroupDiscoveryList,
 			if knownGroup.Name == possibleGroup.Name {
 				t.Logf("found %s", knownGroup.Name)
 				found = true
+				diff := cmp.Diff(knownGroup, possibleGroup)
+				if len(diff) > 0 {
+					t.Error(diff)
+				}
 			}
 		}
 		if found == false {
@@ -287,6 +391,10 @@ func TestLegacyFallbackNoCache(t *testing.T) {
 				GroupVersion: "stable.example.com/v1alpha1",
 				Version:      "v1alpha1",
 			},
+			{
+				GroupVersion: "stable.example.com/v2alpha1",
+				Version:      "v2alpha1",
+			},
 		},
 	})
 
@@ -347,6 +455,17 @@ func TestLegacyFallbackNoCache(t *testing.T) {
 			legacyResourceHandlerV1Beta1.ServeHTTP(w, r)
 		} else if r.URL.Path == "/apis/stable.example.com/v1alpha1" {
 			legacyResourceHandlerV1Alpha1.ServeHTTP(w, r)
+		} else if r.URL.Path == "/apis/stable.example.com/v2alpha1" {
+			// serve the most minimal discovery doc that could have worked prior to aggregated discovery
+			json.NewEncoder(w).Encode(&metav1.APIResourceList{
+				GroupVersion: "stable.example.com/v2alpha1",
+				APIResources: []metav1.APIResource{
+					{Name: "parent-without-kind"},
+					{Name: "missing-parent/subresource-without-parent", Kind: "SubresourceWithoutParent"},
+					{Name: "parent-without-kind/subresource", Kind: "Subresource"},
+					{Name: "parent-without-kind/subresource-without-kind"},
+				},
+			})
 		} else if r.URL.Path == "/apis" {
 			rootAPIsHandler.ServeHTTP(w, r)
 		} else {
@@ -392,6 +511,18 @@ func TestLegacyFallbackNoCache(t *testing.T) {
 			},
 		},
 	}, handlerFunc)
+	aggregatedManager.AddAPIService(&apiregistrationv1.APIService{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "v2alpha1.stable.example.com",
+		},
+		Spec: apiregistrationv1.APIServiceSpec{
+			Group:   "stable.example.com",
+			Version: "v2alpha1",
+			Service: &apiregistrationv1.ServiceReference{
+				Name: "test-service",
+			},
+		},
+	}, handlerFunc)
 
 	testCtx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -403,38 +534,68 @@ func TestLegacyFallbackNoCache(t *testing.T) {
 	// includes the legacy resources
 	_, _, doc := fetchPath(aggregatedResourceManager, "")
 
-	aggregatedVersions := []apidiscoveryv2beta1.APIVersionDiscovery{}
-	for _, resource := range resources {
-		converted, err := endpoints.ConvertGroupVersionIntoToDiscovery([]metav1.APIResource{resource})
+	mustConvert := func(r []metav1.APIResource) []apidiscoveryv2beta1.APIResourceDiscovery {
+		converted, err := endpoints.ConvertGroupVersionIntoToDiscovery(r)
 		require.NoError(t, err)
-		aggregatedVersions = append(aggregatedVersions, apidiscoveryv2beta1.APIVersionDiscovery{
-			Version:   resource.Version,
-			Resources: converted,
-			Freshness: apidiscoveryv2beta1.DiscoveryFreshnessCurrent,
-		})
+		return converted
 	}
-	sort.Sort(byVersion(aggregatedVersions))
-	aggregatedDiscovery := []apidiscoveryv2beta1.APIGroupDiscovery{{
+	expectAggregatedDiscovery := []apidiscoveryv2beta1.APIGroupDiscovery{{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: resources["v1"].Group,
+			Name: "stable.example.com",
+		},
+		Versions: []apidiscoveryv2beta1.APIVersionDiscovery{
+			{
+				Version:   "v1",
+				Resources: mustConvert([]metav1.APIResource{resources["v1"]}),
+				Freshness: apidiscoveryv2beta1.DiscoveryFreshnessCurrent,
+			},
+			{
+				Version:   "v1beta1",
+				Resources: mustConvert([]metav1.APIResource{resources["v1beta1"]}),
+				Freshness: apidiscoveryv2beta1.DiscoveryFreshnessCurrent,
+			},
+			{
+				Version: "v2alpha1",
+				Resources: []apidiscoveryv2beta1.APIResourceDiscovery{
+					{
+						Resource:     "parent-without-kind",
+						ResponseKind: &metav1.GroupVersionKind{}, // defaulted
+						Scope:        "Cluster",
+						Subresources: []apidiscoveryv2beta1.APISubresourceDiscovery{
+							{
+								Subresource:  "subresource",
+								ResponseKind: &metav1.GroupVersionKind{Kind: "Subresource"},
+							},
+							{
+								Subresource:  "subresource-without-kind",
+								ResponseKind: &metav1.GroupVersionKind{}, // defaulted
+							},
+						},
+					},
+					{
+						Resource:     "missing-parent",
+						ResponseKind: &metav1.GroupVersionKind{}, // defaulted
+						Scope:        "Cluster",
+						Subresources: []apidiscoveryv2beta1.APISubresourceDiscovery{
+							{
+								Subresource:  "subresource-without-parent",
+								ResponseKind: &metav1.GroupVersionKind{Kind: "SubresourceWithoutParent"},
+							},
+						},
+					},
+				},
+				Freshness: apidiscoveryv2beta1.DiscoveryFreshnessCurrent,
+			},
+			{
+				Version:   "v1alpha1",
+				Resources: mustConvert([]metav1.APIResource{resources["v1alpha1"]}),
+				Freshness: apidiscoveryv2beta1.DiscoveryFreshnessCurrent,
+			},
 		},
-		Versions: aggregatedVersions,
 	}}
-	require.Equal(t, doc.Items, aggregatedDiscovery)
+	require.Equal(t, doc.Items, expectAggregatedDiscovery)
 }
 
-type byVersion []apidiscoveryv2beta1.APIVersionDiscovery
-
-var versionMap = map[string]int{
-	"v1":       1,
-	"v1beta1":  2,
-	"v1alpha1": 3,
-}
-
-func (a byVersion) Len() int           { return len(a) }
-func (a byVersion) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
-func (a byVersion) Less(i, j int) bool { return versionMap[a[i].Version] < versionMap[a[j].Version] }
-
 func TestLegacyFallback(t *testing.T) {
 	aggregatedResourceManager := discoveryendpoint.NewResourceManager("apis")
 	rootAPIsHandler := discovery.NewRootAPIsHandler(discovery.DefaultAddresses{DefaultAddress: "192.168.1.1"}, scheme.Codecs)
@@ -603,9 +764,14 @@ func fuzzAPIGroups(atLeastNumGroups, maxNumGroups int, seed int64) apidiscoveryv
 		c.Fuzz(&atLeastOne)
 		o.Versions = append(o.Versions, atLeastOne)
 
-		o.TypeMeta = metav1.TypeMeta{
-			Kind:       "APIGroupDiscovery",
-			APIVersion: "v1",
+		// clear invalid fuzzed values
+		o.TypeMeta = metav1.TypeMeta{}
+		// truncate object meta to just name
+		o.ObjectMeta = metav1.ObjectMeta{Name: o.ObjectMeta.Name}
+		// fix version freshness value, make versions unique and non-empty
+		for i := range o.Versions {
+			o.Versions[i].Freshness = "Current"
+			o.Versions[i].Version = fmt.Sprintf("v%d", i+1)
 		}
 	})
 

test/integration/apiserver/discovery/discovery_test.go

@@ -27,6 +27,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/require"
+
 	apidiscoveryv2beta1 "k8s.io/api/apidiscovery/v2beta1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextensions "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
@@ -97,6 +98,28 @@ var (
 		},
 	}
 
+	basicTestGroupWithFixup = apidiscoveryv2beta1.APIGroupDiscovery{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "stable.example.com",
+		},
+		Versions: []apidiscoveryv2beta1.APIVersionDiscovery{
+			{
+				Version: "v1",
+				Resources: []apidiscoveryv2beta1.APIResourceDiscovery{
+					{
+						Resource:   "jobs",
+						Verbs:      []string{"create", "list", "watch", "delete"},
+						ShortNames: []string{"jz"},
+						Categories: []string{"all"},
+						// aggregator will populate this with a non-nil value
+						ResponseKind: &metav1.GroupVersionKind{},
+					},
+				},
+				Freshness: apidiscoveryv2beta1.DiscoveryFreshnessCurrent,
+			},
+		},
+	}
+
 	stableGroup    = "stable.example.com"
 	stableV1       = metav1.GroupVersion{Group: stableGroup, Version: "v1"}
 	stableV1alpha1 = metav1.GroupVersion{Group: stableGroup, Version: "v1alpha1"}
@@ -232,7 +255,7 @@ func TestAggregatedAPIServiceDiscovery(t *testing.T) {
 
 	// Keep repeatedly fetching document from aggregator.
 	// Check to see if it contains our service within a reasonable amount of time
-	require.NoError(t, WaitForGroups(ctx, client, basicTestGroup))
+	require.NoError(t, WaitForGroups(ctx, client, basicTestGroupWithFixup))
 }
 
 func runTestCases(t *testing.T, cases []testCase) {