From 95d3d4a22d705ef6bf2d494c065743d356914e8d Mon Sep 17 00:00:00 2001 From: Jefftree Date: Thu, 13 Mar 2025 17:58:06 +0000 Subject: [PATCH] Gate apidiscovery/v2beta1 serving with a feature gate --- pkg/features/versioned_kube_features.go | 5 + .../endpoints/discovery/aggregated/handler.go | 10 ++ .../discovery/aggregated/handler_test.go | 4 + .../discovery/aggregated/negotiation.go | 6 + .../discovery/aggregated/wrapper_test.go | 6 + .../apiserver/pkg/features/kube_features.go | 12 ++ .../discovery/aggregated_discovery.go | 2 +- .../discovery/cached/memory/memcache_test.go | 2 +- .../reference/versioned_feature_list.yaml | 10 ++ .../apiserver/discovery/discovery_test.go | 7 -- .../apiserver/discovery/framework.go | 103 ------------------ 11 files changed, 55 insertions(+), 112 deletions(-) diff --git a/pkg/features/versioned_kube_features.go b/pkg/features/versioned_kube_features.go index a5d4898dd31..e8c139e5080 100644 --- a/pkg/features/versioned_kube_features.go +++ b/pkg/features/versioned_kube_features.go @@ -213,6 +213,11 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate {Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha}, }, + genericfeatures.AggregatedDiscoveryRemoveBetaType: { + {Version: version.MustParse("1.0"), Default: false, PreRelease: featuregate.GA}, + {Version: version.MustParse("1.33"), Default: true, PreRelease: featuregate.Deprecated}, + }, + genericfeatures.AllowParsingUserUIDFromCertAuth: { {Version: version.MustParse("1.33"), Default: true, PreRelease: featuregate.Beta}, }, diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/handler.go b/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/handler.go index 338be85a287..3ea59e873fe 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/handler.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/handler.go @@ -29,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/version" apidiscoveryv2conversion "k8s.io/apiserver/pkg/apis/apidiscovery/v2" + genericfeatures "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/endpoints/handlers/responsewriters" @@ -40,6 +41,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" ) @@ -538,6 +540,14 @@ func (rdm *resourceDiscoveryManager) serveHTTP(resp http.ResponseWriter, req *ht resp.WriteHeader(http.StatusInternalServerError) return } + + if mediaType.Convert.GroupVersion() == apidiscoveryv2beta1.SchemeGroupVersion && + utilfeature.DefaultFeatureGate.Enabled(genericfeatures.AggregatedDiscoveryRemoveBetaType) { + klog.Errorf("aggregated discovery version v2beta1 is removed. Please update to use v2") + resp.WriteHeader(http.StatusNotFound) + return + } + targetGV = mediaType.Convert.GroupVersion() if len(etag) > 0 { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/handler_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/handler_test.go index 34b0cd37ae7..295540a5694 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/handler_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/handler_test.go @@ -43,6 +43,9 @@ import ( "k8s.io/apimachinery/pkg/version" apidiscoveryv2conversion "k8s.io/apiserver/pkg/apis/apidiscovery/v2" discoveryendpoint "k8s.io/apiserver/pkg/endpoints/discovery/aggregated" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" ) var scheme = runtime.NewScheme() @@ -187,6 +190,7 @@ func TestBasicResponseProtobuf(t *testing.T) { // V2Beta1 should still be served func TestV2Beta1SkewSupport(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AggregatedDiscoveryRemoveBetaType, false) manager := discoveryendpoint.NewResourceManager("apis") apis := fuzzAPIGroups(1, 3, 10) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/negotiation.go b/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/negotiation.go index 9900021a432..57d1d610367 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/negotiation.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/negotiation.go @@ -18,6 +18,9 @@ package aggregated import ( "k8s.io/apimachinery/pkg/runtime/schema" + + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" ) // Interface is from "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" @@ -37,6 +40,9 @@ func (discoveryEndpointRestrictions) AllowsStreamSchema(s string) bool { return // IsAggregatedDiscoveryGVK checks if a provided GVK is the GVK for serving aggregated discovery. func IsAggregatedDiscoveryGVK(gvk *schema.GroupVersionKind) bool { if gvk != nil { + if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.AggregatedDiscoveryRemoveBetaType) { + return gvk.Group == "apidiscovery.k8s.io" && gvk.Version == "v2" && gvk.Kind == "APIGroupDiscoveryList" + } return gvk.Group == "apidiscovery.k8s.io" && (gvk.Version == "v2beta1" || gvk.Version == "v2") && gvk.Kind == "APIGroupDiscoveryList" } return false diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/wrapper_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/wrapper_test.go index 91071b6470e..08d754897e6 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/wrapper_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/wrapper_test.go @@ -24,6 +24,9 @@ import ( "testing" "github.com/stretchr/testify/assert" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" ) const discoveryPath = "/apis" @@ -103,6 +106,9 @@ func TestAggregationEnabled(t *testing.T) { } for _, tc := range testCases { + if tc.accept == aggregatedV2Beta1JSONAccept || tc.accept == aggregatedV2Beta1ProtoAccept { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AggregatedDiscoveryRemoveBetaType, false) + } body := fetchPath(wrapped, discoveryPath, tc.accept) assert.Equal(t, tc.expected, body) } diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go index d2cfc6f218c..e7fab291aa4 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -34,6 +34,13 @@ const ( // of code conflicts because changes are more likely to be scattered // across the file. + // owner: @jefftree + // + // Remove the v2beta1 apidiscovery.k8s.io/v2beta1 group version. Aggregated + // discovery implements its own handlers and follows a different lifecycle than + // traditional k8s resources. + AggregatedDiscoveryRemoveBetaType featuregate.Feature = "AggregatedDiscoveryRemoveBetaType" + // owner: @modulitos // // Allow user.DefaultInfo.UID to be set from x509 cert during cert auth. @@ -250,6 +257,11 @@ func init() { // Entries are alphabetized and separated from each other with blank lines to avoid sweeping gofmt changes // when adding or removing one entry. var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate.VersionedSpecs{ + AggregatedDiscoveryRemoveBetaType: { + {Version: version.MustParse("1.0"), Default: false, PreRelease: featuregate.GA}, + {Version: version.MustParse("1.33"), Default: true, PreRelease: featuregate.Deprecated}, + }, + AllowParsingUserUIDFromCertAuth: { {Version: version.MustParse("1.33"), Default: true, PreRelease: featuregate.Beta}, }, 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 f5eaaedab3a..82d46b48954 100644 --- a/staging/src/k8s.io/client-go/discovery/aggregated_discovery.go +++ b/staging/src/k8s.io/client-go/discovery/aggregated_discovery.go @@ -156,7 +156,7 @@ func convertAPISubresource(parent metav1.APIResource, in apidiscovery.APISubreso return result, nil } -// Please note the functions below will be removed in v1.33. They facilitate conversion +// Please note the functions below will be removed in v1.35. They facilitate conversion // between the deprecated type apidiscoveryv2beta1.APIGroupDiscoveryList. // SplitGroupsAndResourcesV2Beta1 transforms "aggregated" discovery top-level structure into diff --git a/staging/src/k8s.io/client-go/discovery/cached/memory/memcache_test.go b/staging/src/k8s.io/client-go/discovery/cached/memory/memcache_test.go index f33ded83501..1412a961ba9 100644 --- a/staging/src/k8s.io/client-go/discovery/cached/memory/memcache_test.go +++ b/staging/src/k8s.io/client-go/discovery/cached/memory/memcache_test.go @@ -1425,7 +1425,7 @@ func TestMemCacheAggregatedServerGroups(t *testing.T) { return } // Content-type is "aggregated" discovery format. - w.Header().Set("Content-Type", discovery.AcceptV2Beta1) + w.Header().Set("Content-Type", discovery.AcceptV2) w.WriteHeader(http.StatusOK) w.Write(output) })) diff --git a/test/compatibility_lifecycle/reference/versioned_feature_list.yaml b/test/compatibility_lifecycle/reference/versioned_feature_list.yaml index fec5eb228e1..50b60fae655 100644 --- a/test/compatibility_lifecycle/reference/versioned_feature_list.yaml +++ b/test/compatibility_lifecycle/reference/versioned_feature_list.yaml @@ -1,6 +1,16 @@ # This file is generated by compatibility_lifecycle tool. # Do not edit manually. Run hack/update-featuregates.sh to regenerate. +- name: AggregatedDiscoveryRemoveBetaType + versionedSpecs: + - default: false + lockToDefault: false + preRelease: GA + version: "1.0" + - default: true + lockToDefault: false + preRelease: Deprecated + version: "1.33" - name: AllowDNSOnlyNodeCSR versionedSpecs: - default: true diff --git a/test/integration/apiserver/discovery/discovery_test.go b/test/integration/apiserver/discovery/discovery_test.go index cf41ffe620e..f6d7ad7a02d 100644 --- a/test/integration/apiserver/discovery/discovery_test.go +++ b/test/integration/apiserver/discovery/discovery_test.go @@ -388,7 +388,6 @@ func TestCRD(t *testing.T) { applyCRD(makeCRDSpec(stableGroup, "Foo", false, []string{"v1", "v1alpha1", "v1beta1", "v2"})), waitForGroupVersionsV1([]metav1.GroupVersion{stableV1, stableV1alpha1, stableV1beta1, stableV2}), waitForGroupVersionsV2([]metav1.GroupVersion{stableV1, stableV1alpha1, stableV1beta1, stableV2}), - waitForGroupVersionsV2Beta1([]metav1.GroupVersion{stableV1, stableV1alpha1, stableV1beta1, stableV2}), }, }, { @@ -398,7 +397,6 @@ func TestCRD(t *testing.T) { applyCRD(makeCRDSpec(stableGroup, "Foo", false, []string{"v1", "v1alpha1", "v1beta1", "v2"})), waitForGroupVersionsV1([]metav1.GroupVersion{stableV1, stableV1alpha1, stableV1beta1, stableV2}), waitForGroupVersionsV2([]metav1.GroupVersion{stableV1, stableV1alpha1, stableV1beta1, stableV2}), - waitForGroupVersionsV2Beta1([]metav1.GroupVersion{stableV1, stableV1alpha1, stableV1beta1, stableV2}), deleteObject{ GroupVersionResource: metav1.GroupVersionResource(apiextensionsv1.SchemeGroupVersion.WithResource("customresourcedefinitions")), Name: "foos.stable.example.com", @@ -466,7 +464,6 @@ func TestCRD(t *testing.T) { // Wait for GV to appear in both discovery documents waitForGroupVersionsV1([]metav1.GroupVersion{stableV2, stableV1alpha1}), waitForGroupVersionsV2([]metav1.GroupVersion{stableV2, stableV1alpha1}), - waitForGroupVersionsV2Beta1([]metav1.GroupVersion{stableV2, stableV1alpha1}), applyAPIService( apiregistrationv1.APIServiceSpec{ @@ -485,13 +482,11 @@ func TestCRD(t *testing.T) { // We should now have stable v1 available waitForGroupVersionsV1([]metav1.GroupVersion{stableV1}), waitForGroupVersionsV2([]metav1.GroupVersion{stableV1}), - waitForGroupVersionsV2Beta1([]metav1.GroupVersion{stableV1}), // The CRD group-versions not served by the aggregated // apiservice should still be availablee waitForGroupVersionsV1([]metav1.GroupVersion{stableV2, stableV1alpha1}), waitForGroupVersionsV2([]metav1.GroupVersion{stableV2, stableV1alpha1}), - waitForGroupVersionsV2Beta1([]metav1.GroupVersion{stableV2, stableV1alpha1}), // Remove API service. Show we have switched to CRD deleteObject{ @@ -502,11 +497,9 @@ func TestCRD(t *testing.T) { // Show that we still have stable v1 since it is in the CRD waitForGroupVersionsV1([]metav1.GroupVersion{stableV2, stableV1alpha1}), waitForGroupVersionsV2([]metav1.GroupVersion{stableV2, stableV1alpha1}), - waitForGroupVersionsV2Beta1([]metav1.GroupVersion{stableV2, stableV1alpha1}), waitForAbsentGroupVersionsV1([]metav1.GroupVersion{stableV1}), waitForAbsentGroupVersionsV2([]metav1.GroupVersion{stableV1}), - waitForAbsentGroupVersionsV2Beta1([]metav1.GroupVersion{stableV1}), }, }, { diff --git a/test/integration/apiserver/discovery/framework.go b/test/integration/apiserver/discovery/framework.go index 0ed6dd49889..460525fd9ca 100644 --- a/test/integration/apiserver/discovery/framework.go +++ b/test/integration/apiserver/discovery/framework.go @@ -37,14 +37,12 @@ import ( aggregator "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset" apidiscoveryv2 "k8s.io/api/apidiscovery/v2" - apidiscoveryv2beta1 "k8s.io/api/apidiscovery/v2beta1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" ) const acceptV1JSON = "application/json" -const acceptV2Beta1JSON = "application/json;g=apidiscovery.k8s.io;v=v2beta1;as=APIGroupDiscoveryList" const acceptV2JSON = "application/json;g=apidiscovery.k8s.io;v=v2;as=APIGroupDiscoveryList" const maxTimeout = 10 * time.Second @@ -95,15 +93,9 @@ type waitForAbsentGroupVersionsV1 []metav1.GroupVersion // Wait for groupversions to appear in v2 discovery type waitForGroupVersionsV2 []metav1.GroupVersion -// Wait for groupversions to appear in v2beta1 discovery -type waitForGroupVersionsV2Beta1 []metav1.GroupVersion - // Wait for groupversions to disappear from v2 discovery type waitForAbsentGroupVersionsV2 []metav1.GroupVersion -// Wait for groupversions to disappear from v2beta1 discovery -type waitForAbsentGroupVersionsV2Beta1 []metav1.GroupVersion - type waitForStaleGroupVersionsV2 []metav1.GroupVersion type waitForFreshGroupVersionsV2 []metav1.GroupVersion @@ -308,23 +300,6 @@ func (w waitForGroupVersionsV2) Do(ctx context.Context, client testClient) error return nil } -func (w waitForGroupVersionsV2Beta1) Do(ctx context.Context, client testClient) error { - err := WaitForV2Beta1ResultWithCondition(ctx, client, func(result apidiscoveryv2beta1.APIGroupDiscoveryList) bool { - for _, gv := range w { - if FindGroupVersionV2Beta1(result, gv) == nil { - return false - } - } - - return true - }) - - if err != nil { - return fmt.Errorf("waiting for groupversions v2 (%v): %w", w, err) - } - return nil -} - func (w waitForAbsentGroupVersionsV2) Do(ctx context.Context, client testClient) error { err := WaitForResultWithCondition(ctx, client, func(result apidiscoveryv2.APIGroupDiscoveryList) bool { for _, gv := range w { @@ -342,23 +317,6 @@ func (w waitForAbsentGroupVersionsV2) Do(ctx context.Context, client testClient) return nil } -func (w waitForAbsentGroupVersionsV2Beta1) Do(ctx context.Context, client testClient) error { - err := WaitForV2Beta1ResultWithCondition(ctx, client, func(result apidiscoveryv2beta1.APIGroupDiscoveryList) bool { - for _, gv := range w { - if FindGroupVersionV2Beta1(result, gv) != nil { - return false - } - } - - return true - }) - - if err != nil { - return fmt.Errorf("waiting for absent groupversions v2 (%v): %w", w, err) - } - return nil -} - func (w waitForGroupVersionsV1) Do(ctx context.Context, client testClient) error { err := WaitForV1GroupsWithCondition(ctx, client, func(result metav1.APIGroupList) bool { for _, gv := range w { @@ -551,29 +509,6 @@ func FetchV2Discovery(ctx context.Context, client testClient) (apidiscoveryv2.AP return groupList, nil } -func FetchV2Beta1Discovery(ctx context.Context, client testClient) (apidiscoveryv2beta1.APIGroupDiscoveryList, error) { - result, err := client. - Discovery(). - RESTClient(). - Get(). - AbsPath("/apis"). - SetHeader("Accept", acceptV2Beta1JSON). - Do(ctx). - Raw() - - if err != nil { - return apidiscoveryv2beta1.APIGroupDiscoveryList{}, fmt.Errorf("failed to fetch v2 discovery: %w", err) - } - - groupList := apidiscoveryv2beta1.APIGroupDiscoveryList{} - err = json.Unmarshal(result, &groupList) - if err != nil { - return apidiscoveryv2beta1.APIGroupDiscoveryList{}, fmt.Errorf("failed to parse v2 discovery: %w", err) - } - - return groupList, nil -} - func FetchV1DiscoveryGroups(ctx context.Context, client testClient) (metav1.APIGroupList, error) { return FetchV1DiscoveryGroupsAtPath(ctx, client, "/apis") } @@ -705,28 +640,6 @@ func WaitForResultWithCondition(ctx context.Context, client testClient, conditio }) } -func WaitForV2Beta1ResultWithCondition(ctx context.Context, client testClient, condition func(result apidiscoveryv2beta1.APIGroupDiscoveryList) bool) error { - // Keep repeatedly fetching document from aggregator. - // Check to see if it contains our service within a reasonable amount of time - return wait.PollUntilContextTimeout( - ctx, - 250*time.Millisecond, - maxTimeout, - true, - func(ctx context.Context) (done bool, err error) { - groupList, err := FetchV2Beta1Discovery(ctx, client) - if err != nil { - return false, err - } - - if condition(groupList) { - return true, nil - } - - return false, nil - }) -} - func WaitForV1GroupsWithCondition(ctx context.Context, client testClient, condition func(result metav1.APIGroupList) bool) error { // Keep repeatedly fetching document from aggregator. // Check to see if it contains our service within a reasonable amount of time @@ -804,19 +717,3 @@ func FindGroupVersionV2(discovery apidiscoveryv2.APIGroupDiscoveryList, gv metav return nil } - -func FindGroupVersionV2Beta1(discovery apidiscoveryv2beta1.APIGroupDiscoveryList, gv metav1.GroupVersion) *apidiscoveryv2beta1.APIVersionDiscovery { - for _, documentGroup := range discovery.Items { - if documentGroup.Name != gv.Group { - continue - } - - for _, documentVersion := range documentGroup.Versions { - if documentVersion.Version == gv.Version { - return &documentVersion - } - } - } - - return nil -}