diff --git a/pkg/apis/apidiscovery/types.go b/pkg/apis/apidiscovery/types.go index 246655078ae..3749aa1618c 100644 --- a/pkg/apis/apidiscovery/types.go +++ b/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 diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index 5b349eaec35..b00851f2ffd 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/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"), }, }, diff --git a/staging/src/k8s.io/api/apidiscovery/v2beta1/generated.proto b/staging/src/k8s.io/api/apidiscovery/v2beta1/generated.proto index aa08b4978c2..a09af750ba3 100644 --- a/staging/src/k8s.io/api/apidiscovery/v2beta1/generated.proto +++ b/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. diff --git a/staging/src/k8s.io/api/apidiscovery/v2beta1/types.go b/staging/src/k8s.io/api/apidiscovery/v2beta1/types.go index 1aff3e3702f..83429377304 100644 --- a/staging/src/k8s.io/api/apidiscovery/v2beta1/types.go +++ b/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 diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index 2011292927d..042bd802f1a 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/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{ diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer_test.go index 68ea6056d8b..03d06f90947 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer_test.go +++ b/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{ diff --git a/staging/src/k8s.io/client-go/discovery/aggregated_discovery.go b/staging/src/k8s.io/client-go/discovery/aggregated_discovery.go index 7470259dc86..f72c42051b9 100644 --- a/staging/src/k8s.io/client-go/discovery/aggregated_discovery.go +++ b/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) diff --git a/staging/src/k8s.io/client-go/discovery/aggregated_discovery_test.go b/staging/src/k8s.io/client-go/discovery/aggregated_discovery_test.go index c0c8d0ea6e7..a5004804c1a 100644 --- a/staging/src/k8s.io/client-go/discovery/aggregated_discovery_test.go +++ b/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{ diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_discovery.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_discovery.go index 3690006fd57..974388f9731 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_discovery.go +++ b/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{} + } + } + } } } diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_discovery_test.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_discovery_test.go index 3f37ffa4f1e..44bdfa5c9b9 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_discovery_test.go +++ b/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) } }) diff --git a/test/integration/apiserver/discovery/discovery_test.go b/test/integration/apiserver/discovery/discovery_test.go index 5c983e8fcbb..7ce49612430 100644 --- a/test/integration/apiserver/discovery/discovery_test.go +++ b/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) {