Merge pull request kubernetes#119871 from liggitt/automated-cherry-pi…

…ck-of-#119835-upstream-release-1.26

Automated cherry pick of kubernetes#119835: Avoid returning nil responseKind in v1beta1 aggregated

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

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/go.mod

@@ -8,6 +8,7 @@ require (
 	github.com/davecgh/go-spew v1.1.1
 	github.com/emicklei/go-restful/v3 v3.9.0
 	github.com/gogo/protobuf v1.3.2
+	github.com/google/go-cmp v0.5.9
 	github.com/google/gofuzz v1.1.0
 	github.com/spf13/cobra v1.6.0
 	github.com/spf13/pflag v1.0.5
@@ -46,7 +47,6 @@ require (
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/google/cel-go v0.12.6 // indirect
 	github.com/google/gnostic v0.5.7-v3refs // indirect
-	github.com/google/go-cmp v0.5.9 // indirect
 	github.com/google/uuid v1.3.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0 // indirect

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{}
+						}
+					}
+				}
 			}
 		}