From 072dfcb416fd4e1ddab0a89ac4faf519e268bc96 Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Mon, 4 Nov 2024 10:40:19 -0500 Subject: [PATCH] Add CBOR feature gates. For alpha, there is one apiserver feature gate and two client-go feature gates controlling CBOR. They were initially wired to separate test-only feature gate instances in order to prevent them from being configurable at runtime via command-line flags or environment variables (for client-go feature gates outside of Kubernetes components). All of the integration tests required by the KEP as alpha criteria have been implemented. This adds the feature gates to the usual feature gate instances and removes the temporary code to support separate test-only feature gate instances. --- pkg/features/client_adapter.go | 7 +++ pkg/features/versioned_kube_features.go | 4 ++ .../pkg/apiserver/customresource_handler.go | 6 +-- .../pkg/cmd/server/options/options.go | 2 +- .../test/integration/cbor_test.go | 8 +-- .../apiserver/pkg/endpoints/handlers/patch.go | 4 +- .../handlers/responsewriters/writers.go | 2 +- .../apiserver/pkg/endpoints/handlers/watch.go | 6 +-- .../apiserver/pkg/endpoints/installer.go | 2 +- .../apiserver/pkg/features/kube_features.go | 18 ++----- .../src/k8s.io/apiserver/pkg/server/config.go | 2 +- .../apiserver/pkg/server/config_test.go | 2 +- .../pkg/util/feature/feature_gate.go | 12 ----- .../src/k8s.io/client-go/dynamic/scheme.go | 2 +- .../src/k8s.io/client-go/dynamic/simple.go | 4 +- .../src/k8s.io/client-go/features/features.go | 42 --------------- .../client-go/features/known_features.go | 54 ++++++++++--------- staging/src/k8s.io/client-go/rest/client.go | 6 +-- staging/src/k8s.io/client-go/rest/config.go | 2 +- staging/src/k8s.io/client-go/rest/request.go | 2 +- .../src/k8s.io/client-go/util/apply/apply.go | 2 +- .../test_data/versioned_feature_list.yaml | 12 ++--- .../admissionwebhook/admission_test.go | 5 +- .../integration/apiserver/apply/apply_test.go | 4 +- .../apiserver/apply/status_test.go | 5 +- test/integration/apiserver/cbor_test.go | 5 +- test/integration/client/client_test.go | 11 ++-- .../integration/client/dynamic_client_test.go | 11 ++-- test/integration/framework/cbor.go | 36 +------------ 29 files changed, 108 insertions(+), 170 deletions(-) diff --git a/pkg/features/client_adapter.go b/pkg/features/client_adapter.go index de03d78ef2b..a24a1f8730c 100644 --- a/pkg/features/client_adapter.go +++ b/pkg/features/client_adapter.go @@ -67,3 +67,10 @@ func (a *clientAdapter) Add(in map[clientfeatures.Feature]clientfeatures.Feature } return a.mfg.Add(out) } + +// Set implements the unexported interface that client-go feature gate testing expects for +// ek8s.io/client-go/features/testing.SetFeatureDuringTest. This is necessary for integration tests +// to set test overrides for client-go feature gates. +func (a *clientAdapter) Set(name clientfeatures.Feature, enabled bool) error { + return a.mfg.SetFromMap(map[string]bool{string(name): enabled}) +} diff --git a/pkg/features/versioned_kube_features.go b/pkg/features/versioned_kube_features.go index df9c7ab6430..a80f44e0cb3 100644 --- a/pkg/features/versioned_kube_features.go +++ b/pkg/features/versioned_kube_features.go @@ -245,6 +245,10 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate {Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.Beta}, }, + genericfeatures.CBORServingAndStorage: { + {Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha}, + }, + genericfeatures.ConcurrentWatchObjectDecode: { {Version: version.MustParse("1.31"), Default: false, PreRelease: featuregate.Beta}, }, diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index 12a77a204e9..955ba1b5a37 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -327,7 +327,7 @@ func (r *crdHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { string(types.MergePatchType), string(types.ApplyYAMLPatchType), } - if utilfeature.TestOnlyFeatureGate.Enabled(features.TestOnlyCBORServingAndStorage) { + if utilfeature.DefaultFeatureGate.Enabled(features.CBORServingAndStorage) { supportedTypes = append(supportedTypes, string(types.ApplyCBORPatchType)) } @@ -913,7 +913,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd }, } - if utilfeature.TestOnlyFeatureGate.Enabled(features.TestOnlyCBORServingAndStorage) { + if utilfeature.DefaultFeatureGate.Enabled(features.CBORServingAndStorage) { negotiatedSerializer.supportedMediaTypes = append(negotiatedSerializer.supportedMediaTypes, newCBORSerializerInfo(creator, typer)) } @@ -981,7 +981,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd scaleConverter := scale.NewScaleConverter() scaleScope.Subresource = "scale" var opts []serializer.CodecFactoryOptionsMutator - if utilfeature.TestOnlyFeatureGate.Enabled(features.TestOnlyCBORServingAndStorage) { + if utilfeature.DefaultFeatureGate.Enabled(features.CBORServingAndStorage) { opts = append(opts, serializer.WithSerializer(newCBORSerializerInfo)) } scaleScope.Serializer = serializer.NewCodecFactory(scaleConverter.Scheme(), opts...) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/options/options.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/options/options.go index 84ca33a3f31..37f0e31cea9 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/options/options.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/options/options.go @@ -139,7 +139,7 @@ func NewCRDRESTOptionsGetter(etcdOptions genericoptions.EtcdOptions, resourceTra ucbor := cbor.NewSerializer(unstructuredscheme.NewUnstructuredCreator(), unstructuredscheme.NewUnstructuredObjectTyper()) encoder := unstructured.UnstructuredJSONScheme - if utilfeature.TestOnlyFeatureGate.Enabled(features.TestOnlyCBORServingAndStorage) { + if utilfeature.DefaultFeatureGate.Enabled(features.CBORServingAndStorage) { encoder = ucbor } diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/cbor_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/cbor_test.go index 9298767a801..cfacda3aed4 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/cbor_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/cbor_test.go @@ -69,7 +69,7 @@ func TestCBORStorageEnablement(t *testing.T) { func() { t.Log("starting server with feature gate disabled") - featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.TestOnlyFeatureGate, features.TestOnlyCBORServingAndStorage, false) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CBORServingAndStorage, false) tearDown, apiExtensionsClientset, dynamicClient, etcdClient, _, err := fixtures.StartDefaultServerWithClientsAndEtcd(t, "--etcd-prefix", etcdPrefix) if err != nil { t.Fatal(err) @@ -109,7 +109,7 @@ func TestCBORStorageEnablement(t *testing.T) { func() { t.Log("starting server with feature gate enabled") - featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.TestOnlyFeatureGate, features.TestOnlyCBORServingAndStorage, true) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CBORServingAndStorage, true) tearDown, _, dynamicClient, etcdClient, _, err := fixtures.StartDefaultServerWithClientsAndEtcd(t, "--etcd-prefix", etcdPrefix) if err != nil { t.Fatal(err) @@ -159,7 +159,7 @@ func TestCBORStorageEnablement(t *testing.T) { func() { t.Log("starting server with feature gate disabled") - featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.TestOnlyFeatureGate, features.TestOnlyCBORServingAndStorage, false) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CBORServingAndStorage, false) tearDown, _, dynamicClient, _, _, err := fixtures.StartDefaultServerWithClientsAndEtcd(t, "--etcd-prefix", etcdPrefix) if err != nil { t.Fatal(err) @@ -185,7 +185,7 @@ func TestCBORServingEnablement(t *testing.T) { {name: "disabled", enabled: false}, } { t.Run(tc.name, func(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.TestOnlyFeatureGate, features.TestOnlyCBORServingAndStorage, tc.enabled) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CBORServingAndStorage, tc.enabled) tearDown, config, _, err := fixtures.StartDefaultServer(t) if err != nil { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index 097107842a0..acfff1961c9 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -138,7 +138,7 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac case types.ApplyYAMLPatchType: baseContentType = runtime.ContentTypeYAML case types.ApplyCBORPatchType: - if !utilfeature.TestOnlyFeatureGate.Enabled(features.TestOnlyCBORServingAndStorage) { + if !utilfeature.DefaultFeatureGate.Enabled(features.CBORServingAndStorage) { // This request should have already been rejected by the // Content-Type allowlist check. Return 500 because assumptions are // already broken and the feature is not GA. @@ -673,7 +673,7 @@ func (p *patcher) patchResource(ctx context.Context, scope *RequestScope) (runti p.mechanism = newApplyPatcher(p, scope.FieldManager, yaml.Unmarshal, yaml.UnmarshalStrict) p.forceAllowCreate = true case types.ApplyCBORPatchType: - if !utilfeature.TestOnlyFeatureGate.Enabled(features.TestOnlyCBORServingAndStorage) { + if !utilfeature.DefaultFeatureGate.Enabled(features.CBORServingAndStorage) { utilruntime.HandleErrorWithContext(context.TODO(), nil, "CBOR apply requests should be rejected before reaching this point unless the feature gate is enabled.") return nil, false, fmt.Errorf("%v: unimplemented patch type", p.patchType) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go index 9b9e10ad179..3ca5cba8cb6 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go @@ -286,7 +286,7 @@ func WriteObjectNegotiated(s runtime.NegotiatedSerializer, restrictions negotiat audit.LogResponseObject(req.Context(), object, gv, s) var encoder runtime.Encoder - if utilfeature.TestOnlyFeatureGate.Enabled(features.TestOnlyCBORServingAndStorage) { + if utilfeature.DefaultFeatureGate.Enabled(features.CBORServingAndStorage) { encoder = s.EncoderForVersion(runtime.UseNondeterministicEncoding(serializer.Serializer), gv) } else { encoder = s.EncoderForVersion(serializer.Serializer, gv) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go index 938e8ef5b80..8876a19cd62 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go @@ -77,7 +77,7 @@ func serveWatchHandler(watcher watch.Interface, scope *RequestScope, mediaTypeOp } framer := serializer.StreamSerializer.Framer var encoder runtime.Encoder - if utilfeature.TestOnlyFeatureGate.Enabled(features.TestOnlyCBORServingAndStorage) { + if utilfeature.DefaultFeatureGate.Enabled(features.CBORServingAndStorage) { encoder = scope.Serializer.EncoderForVersion(runtime.UseNondeterministicEncoding(serializer.StreamSerializer.Serializer), scope.Kind.GroupVersion()) } else { encoder = scope.Serializer.EncoderForVersion(serializer.StreamSerializer.Serializer, scope.Kind.GroupVersion()) @@ -102,13 +102,13 @@ func serveWatchHandler(watcher watch.Interface, scope *RequestScope, mediaTypeOp if !ok { return nil, fmt.Errorf("no encoder for %q exists in the requested target %#v", serializer.MediaType, contentSerializer) } - if utilfeature.TestOnlyFeatureGate.Enabled(features.TestOnlyCBORServingAndStorage) { + if utilfeature.DefaultFeatureGate.Enabled(features.CBORServingAndStorage) { negotiatedEncoder = contentSerializer.EncoderForVersion(runtime.UseNondeterministicEncoding(info.Serializer), contentKind.GroupVersion()) } else { negotiatedEncoder = contentSerializer.EncoderForVersion(info.Serializer, contentKind.GroupVersion()) } } else { - if utilfeature.TestOnlyFeatureGate.Enabled(features.TestOnlyCBORServingAndStorage) { + if utilfeature.DefaultFeatureGate.Enabled(features.CBORServingAndStorage) { negotiatedEncoder = scope.Serializer.EncoderForVersion(runtime.UseNondeterministicEncoding(serializer.Serializer), contentKind.GroupVersion()) } else { negotiatedEncoder = scope.Serializer.EncoderForVersion(serializer.Serializer, contentKind.GroupVersion()) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index 6a1826b4e33..78e67109f08 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go @@ -895,7 +895,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag string(types.StrategicMergePatchType), string(types.ApplyYAMLPatchType), } - if utilfeature.TestOnlyFeatureGate.Enabled(features.TestOnlyCBORServingAndStorage) { + if utilfeature.DefaultFeatureGate.Enabled(features.CBORServingAndStorage) { supportedTypes = append(supportedTypes, string(types.ApplyCBORPatchType)) } handler := metrics.InstrumentRouteFunc(action.Verb, group, version, resource, subresource, requestScope, metrics.APIServerComponent, deprecated, removedRelease, restfulPatchResource(patcher, reqScope, admit, supportedTypes)) 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 336e29d9936..02debfb7669 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -92,9 +92,7 @@ const ( // // Enables CBOR as a supported encoding for requests and responses, and as the // preferred storage encoding for custom resources. - // - // This feature is currently PRE-ALPHA and MUST NOT be enabled outside of integration tests. - TestOnlyCBORServingAndStorage featuregate.Feature = "TestOnlyCBORServingAndStorage" + CBORServingAndStorage featuregate.Feature = "CBORServingAndStorage" // owner: @serathius // @@ -245,7 +243,6 @@ const ( func init() { runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(defaultKubernetesFeatureGates)) runtime.Must(utilfeature.DefaultMutableFeatureGate.AddVersioned(defaultVersionedKubernetesFeatureGates)) - runtime.Must(utilfeature.TestOnlyMutableFeatureGate.AddVersioned(testOnlyVersionedKubernetesFeatureGates)) } // defaultVersionedKubernetesFeatureGates consists of all known Kubernetes-specific feature keys with VersionedSpecs. @@ -306,6 +303,10 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate {Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.Beta}, }, + CBORServingAndStorage: { + {Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha}, + }, + ConcurrentWatchObjectDecode: { {Version: version.MustParse("1.31"), Default: false, PreRelease: featuregate.Beta}, }, @@ -417,12 +418,3 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate // defaultKubernetesFeatureGates consists of legacy unversioned Kubernetes-specific feature keys. // Please do not add to this struct and use defaultVersionedKubernetesFeatureGates instead. var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{} - -// testOnlyVersionedKubernetesFeatureGates consists of features that require programmatic enablement -// for integration testing, but have not yet graduated to alpha in a release and must not be enabled -// by a runtime option. -var testOnlyVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate.VersionedSpecs{ - TestOnlyCBORServingAndStorage: { - {Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha}, - }, -} diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go index fe9dca9dcc8..b377285560a 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config.go @@ -756,7 +756,7 @@ func (c completedConfig) New(name string, delegationTarget DelegationTarget) (*G return nil, fmt.Errorf("Genericapiserver.New() called with config.Serializer == nil") } allowedMediaTypes := defaultAllowedMediaTypes - if utilfeature.TestOnlyFeatureGate.Enabled(genericfeatures.TestOnlyCBORServingAndStorage) { + if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.CBORServingAndStorage) { allowedMediaTypes = append(allowedMediaTypes, runtime.ContentTypeCBOR) } for _, info := range c.Serializer.SupportedMediaTypes() { diff --git a/staging/src/k8s.io/apiserver/pkg/server/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/config_test.go index f6447eef7b0..1f30143a977 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config_test.go @@ -424,7 +424,7 @@ func TestNewErrorForbiddenSerializer(t *testing.T) { } func TestNewFeatureGatedSerializer(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.TestOnlyFeatureGate, features.TestOnlyCBORServingAndStorage, true) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CBORServingAndStorage, true) config := NewConfig(serializer.NewCodecFactory(scheme, serializer.WithSerializer(func(creater runtime.ObjectCreater, typer runtime.ObjectTyper) runtime.SerializerInfo { return runtime.SerializerInfo{ diff --git a/staging/src/k8s.io/apiserver/pkg/util/feature/feature_gate.go b/staging/src/k8s.io/apiserver/pkg/util/feature/feature_gate.go index 7c061042aab..00a9e099ba7 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/feature/feature_gate.go +++ b/staging/src/k8s.io/apiserver/pkg/util/feature/feature_gate.go @@ -31,15 +31,3 @@ var ( // Top-level commands/options setup that needs to modify this feature gate should use DefaultMutableFeatureGate. DefaultFeatureGate featuregate.FeatureGate = DefaultMutableFeatureGate ) - -var ( - // TestOnlyMutableFeatureGate is a mutable version of TestOnlyFeatureGate. Only top-level - // commands/options setup and the k8s.io/component-base/featuregate/testing package should - // make use of this. - TestOnlyMutableFeatureGate featuregate.MutableVersionedFeatureGate = featuregate.NewFeatureGate() - - // TestOnlyFeatureGate is a shared global FeatureGate for features that have not yet - // graduated to alpha and require programmatic feature enablement for pre-alpha integration - // testing without exposing the feature as a runtime option. - TestOnlyFeatureGate featuregate.FeatureGate = TestOnlyMutableFeatureGate -) diff --git a/staging/src/k8s.io/client-go/dynamic/scheme.go b/staging/src/k8s.io/client-go/dynamic/scheme.go index dbee05312ea..28316f1dd5c 100644 --- a/staging/src/k8s.io/client-go/dynamic/scheme.go +++ b/staging/src/k8s.io/client-go/dynamic/scheme.go @@ -53,7 +53,7 @@ func newBasicNegotiatedSerializer() basicNegotiatedSerializer { }, }, } - if features.TestOnlyFeatureGates.Enabled(features.TestOnlyClientAllowsCBOR) { + if features.FeatureGates().Enabled(features.ClientsAllowCBOR) { supportedMediaTypes = append(supportedMediaTypes, runtime.SerializerInfo{ MediaType: "application/cbor", MediaTypeType: "application", diff --git a/staging/src/k8s.io/client-go/dynamic/simple.go b/staging/src/k8s.io/client-go/dynamic/simple.go index 6c72998255b..62b2999ca93 100644 --- a/staging/src/k8s.io/client-go/dynamic/simple.go +++ b/staging/src/k8s.io/client-go/dynamic/simple.go @@ -49,9 +49,9 @@ func ConfigFor(inConfig *rest.Config) *rest.Config { config.ContentType = "application/json" config.AcceptContentTypes = "application/json" - if features.TestOnlyFeatureGates.Enabled(features.TestOnlyClientAllowsCBOR) { + if features.FeatureGates().Enabled(features.ClientsAllowCBOR) { config.AcceptContentTypes = "application/json;q=0.9,application/cbor;q=1" - if features.TestOnlyFeatureGates.Enabled(features.TestOnlyClientPrefersCBOR) { + if features.FeatureGates().Enabled(features.ClientsPreferCBOR) { config.ContentType = "application/cbor" } } diff --git a/staging/src/k8s.io/client-go/features/features.go b/staging/src/k8s.io/client-go/features/features.go index 19056df147b..5ccdcc55f6c 100644 --- a/staging/src/k8s.io/client-go/features/features.go +++ b/staging/src/k8s.io/client-go/features/features.go @@ -18,8 +18,6 @@ package features import ( "errors" - "fmt" - "sync" "sync/atomic" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -143,43 +141,3 @@ var ( // should use AddFeaturesToExistingFeatureGates followed by ReplaceFeatureGates. featureGates = &atomic.Value{} ) - -// TestOnlyFeatureGates is a distinct registry of pre-alpha client features that must not be -// included in runtime wiring to command-line flags or environment variables. It exists as a risk -// mitigation to allow only programmatic enablement of CBOR serialization for integration testing -// purposes. -// -// TODO: Once all required integration test coverage is complete, this will be deleted and the -// test-only feature gates will be replaced by normal feature gates. -var TestOnlyFeatureGates = &testOnlyFeatureGates{ - features: map[Feature]bool{ - TestOnlyClientAllowsCBOR: false, - TestOnlyClientPrefersCBOR: false, - }, -} - -type testOnlyFeatureGates struct { - lock sync.RWMutex - features map[Feature]bool -} - -func (t *testOnlyFeatureGates) Enabled(feature Feature) bool { - t.lock.RLock() - defer t.lock.RUnlock() - - enabled, ok := t.features[feature] - if !ok { - panic(fmt.Sprintf("test-only feature %q not recognized", feature)) - } - return enabled -} - -func (t *testOnlyFeatureGates) Set(feature Feature, enabled bool) error { - t.lock.Lock() - defer t.lock.Unlock() - if _, ok := t.features[feature]; !ok { - return fmt.Errorf("test-only feature %q not recognized", feature) - } - t.features[feature] = enabled - return nil -} diff --git a/staging/src/k8s.io/client-go/features/known_features.go b/staging/src/k8s.io/client-go/features/known_features.go index 9a6a7364573..a74f6a83335 100644 --- a/staging/src/k8s.io/client-go/features/known_features.go +++ b/staging/src/k8s.io/client-go/features/known_features.go @@ -28,6 +28,31 @@ const ( // of code conflicts because changes are more likely to be scattered // across the file. + // owner: @benluddy + // kep: https://kep.k8s.io/4222 + // alpha: 1.32 + // + // If disabled, clients configured to accept "application/cbor" will instead accept + // "application/json" with the same relative preference, and clients configured to write + // "application/cbor" or "application/apply-patch+cbor" will instead write + // "application/json" or "application/apply-patch+yaml", respectively. + ClientsAllowCBOR Feature = "ClientsAllowCBOR" + + // owner: @benluddy + // kep: https://kep.k8s.io/4222 + // alpha: 1.32 + // + // If enabled, and only if ClientsAllowCBOR is also enabled, the default request content + // type (if not explicitly configured) and the dynamic client's request content type both + // become "application/cbor" instead of "application/json". The default content type for + // apply patch requests becomes "application/apply-patch+cbor" instead of + // "application/apply-patch+yaml". + ClientsPreferCBOR Feature = "ClientsPreferCBOR" + + // owner: @nilekhc + // alpha: v1.30 + InformerResourceVersion Feature = "InformerResourceVersion" + // owner: @p0lyn0mial // beta: v1.30 // @@ -37,31 +62,6 @@ const ( // The feature is disabled in Beta by default because // it will only be turned on for selected control plane component(s). WatchListClient Feature = "WatchListClient" - - // owner: @nilekhc - // alpha: v1.30 - InformerResourceVersion Feature = "InformerResourceVersion" - - // owner: @benluddy - // kep: https://kep.k8s.io/4222 - // - // If disabled, clients configured to accept "application/cbor" will instead accept - // "application/json" with the same relative preference, and clients configured to write - // "application/cbor" or "application/apply-patch+cbor" will instead write - // "application/json" or "application/apply-patch+yaml", respectively. - // - // This feature is currently PRE-ALPHA and MUST NOT be enabled outside of integration tests. - TestOnlyClientAllowsCBOR Feature = "TestOnlyClientAllowsCBOR" - - // owner: @benluddy - // kep: https://kep.k8s.io/4222 - // - // If enabled AND TestOnlyClientAllowsCBOR is also enabled, the default request content type - // (if not explicitly configured) and the dynamic client's request content type both become - // "application/cbor". - // - // This feature is currently PRE-ALPHA and MUST NOT be enabled outside of integration tests. - TestOnlyClientPrefersCBOR Feature = "TestOnlyClientPrefersCBOR" ) // defaultKubernetesFeatureGates consists of all known Kubernetes-specific feature keys. @@ -70,6 +70,8 @@ const ( // After registering with the binary, the features are, by default, controllable using environment variables. // For more details, please see envVarFeatureGates implementation. var defaultKubernetesFeatureGates = map[Feature]FeatureSpec{ - WatchListClient: {Default: false, PreRelease: Beta}, + ClientsAllowCBOR: {Default: false, PreRelease: Alpha}, + ClientsPreferCBOR: {Default: false, PreRelease: Alpha}, InformerResourceVersion: {Default: false, PreRelease: Alpha}, + WatchListClient: {Default: false, PreRelease: Beta}, } diff --git a/staging/src/k8s.io/client-go/rest/client.go b/staging/src/k8s.io/client-go/rest/client.go index e548eaf5694..9bcb99c549b 100644 --- a/staging/src/k8s.io/client-go/rest/client.go +++ b/staging/src/k8s.io/client-go/rest/client.go @@ -128,7 +128,7 @@ func NewRESTClient(baseURL *url.URL, versionedAPIPath string, config ClientConte } func scrubCBORContentConfigIfDisabled(content ClientContentConfig) ClientContentConfig { - if clientfeatures.TestOnlyFeatureGates.Enabled(clientfeatures.TestOnlyClientAllowsCBOR) { + if clientfeatures.FeatureGates().Enabled(clientfeatures.ClientsAllowCBOR) { return content } @@ -258,7 +258,7 @@ type requestClientContentConfigProvider struct { // GetClientContentConfig returns the ClientContentConfig that should be used for new requests by // this client. func (p *requestClientContentConfigProvider) GetClientContentConfig() ClientContentConfig { - if !clientfeatures.TestOnlyFeatureGates.Enabled(clientfeatures.TestOnlyClientAllowsCBOR) { + if !clientfeatures.FeatureGates().Enabled(clientfeatures.ClientsAllowCBOR) { return p.base } @@ -280,7 +280,7 @@ func (p *requestClientContentConfigProvider) GetClientContentConfig() ClientCont // UnsupportedMediaType reports that the server has responded to a request with HTTP 415 Unsupported // Media Type. func (p *requestClientContentConfigProvider) UnsupportedMediaType(requestContentType string) { - if !clientfeatures.TestOnlyFeatureGates.Enabled(clientfeatures.TestOnlyClientAllowsCBOR) { + if !clientfeatures.FeatureGates().Enabled(clientfeatures.ClientsAllowCBOR) { return } diff --git a/staging/src/k8s.io/client-go/rest/config.go b/staging/src/k8s.io/client-go/rest/config.go index c58e9668b83..aebd990af4a 100644 --- a/staging/src/k8s.io/client-go/rest/config.go +++ b/staging/src/k8s.io/client-go/rest/config.go @@ -683,7 +683,7 @@ func CopyConfig(config *Config) *Config { // This is supported ONLY for use by clients generated with client-gen. The caller is responsible // for ensuring that the CodecFactory argument was constructed using the Scheme argument. func CodecFactoryForGeneratedClient(scheme *runtime.Scheme, codecs serializer.CodecFactory) serializer.CodecFactory { - if !features.TestOnlyFeatureGates.Enabled(features.TestOnlyClientAllowsCBOR) { + if !features.FeatureGates().Enabled(features.ClientsAllowCBOR) { // NOTE: This assumes client-gen will not generate CBOR-enabled Codecs as long as // the feature gate exists. return codecs diff --git a/staging/src/k8s.io/client-go/rest/request.go b/staging/src/k8s.io/client-go/rest/request.go index 46863bb21dc..65942aa1233 100644 --- a/staging/src/k8s.io/client-go/rest/request.go +++ b/staging/src/k8s.io/client-go/rest/request.go @@ -160,7 +160,7 @@ func NewRequest(c *RESTClient) *Request { contentTypeNotSet := len(contentConfig.ContentType) == 0 if contentTypeNotSet { contentConfig.ContentType = "application/json" - if clientfeatures.TestOnlyFeatureGates.Enabled(clientfeatures.TestOnlyClientAllowsCBOR) && clientfeatures.TestOnlyFeatureGates.Enabled(clientfeatures.TestOnlyClientPrefersCBOR) { + if clientfeatures.FeatureGates().Enabled(clientfeatures.ClientsAllowCBOR) && clientfeatures.FeatureGates().Enabled(clientfeatures.ClientsPreferCBOR) { contentConfig.ContentType = "application/cbor" } } diff --git a/staging/src/k8s.io/client-go/util/apply/apply.go b/staging/src/k8s.io/client-go/util/apply/apply.go index c135f5590cb..0cc85df6c53 100644 --- a/staging/src/k8s.io/client-go/util/apply/apply.go +++ b/staging/src/k8s.io/client-go/util/apply/apply.go @@ -35,7 +35,7 @@ func NewRequest(client rest.Interface, applyConfiguration interface{}) (*rest.Re pt := types.ApplyYAMLPatchType marshal := json.Marshal - if features.TestOnlyFeatureGates.Enabled(features.TestOnlyClientAllowsCBOR) && features.TestOnlyFeatureGates.Enabled(features.TestOnlyClientPrefersCBOR) { + if features.FeatureGates().Enabled(features.ClientsAllowCBOR) && features.FeatureGates().Enabled(features.ClientsPreferCBOR) { pt = types.ApplyCBORPatchType marshal = cbor.Marshal } diff --git a/test/featuregates_linter/test_data/versioned_feature_list.yaml b/test/featuregates_linter/test_data/versioned_feature_list.yaml index eec43e298dc..35377737809 100644 --- a/test/featuregates_linter/test_data/versioned_feature_list.yaml +++ b/test/featuregates_linter/test_data/versioned_feature_list.yaml @@ -170,6 +170,12 @@ lockToDefault: false preRelease: Beta version: "1.32" +- name: CBORServingAndStorage + versionedSpecs: + - default: false + lockToDefault: false + preRelease: Alpha + version: "1.32" - name: CloudControllerManagerWebhook versionedSpecs: - default: false @@ -1286,12 +1292,6 @@ lockToDefault: false preRelease: Beta version: "1.32" -- name: TestOnlyCBORServingAndStorage - versionedSpecs: - - default: false - lockToDefault: false - preRelease: Alpha - version: "1.32" - name: TopologyAwareHints versionedSpecs: - default: false diff --git a/test/integration/apiserver/admissionwebhook/admission_test.go b/test/integration/apiserver/admissionwebhook/admission_test.go index a211bc6adca..f3c05f93f32 100644 --- a/test/integration/apiserver/admissionwebhook/admission_test.go +++ b/test/integration/apiserver/admissionwebhook/admission_test.go @@ -53,6 +53,8 @@ import ( "k8s.io/apimachinery/pkg/util/wait" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/client-go/dynamic" + clientfeatures "k8s.io/client-go/features" + clientfeaturestesting "k8s.io/client-go/features/testing" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/util/retry" @@ -458,7 +460,8 @@ func TestWebhookAdmissionWithoutWatchCache(t *testing.T) { func TestWebhookAdmissionWithCBOR(t *testing.T) { framework.EnableCBORServingAndStorageForTest(t) - framework.SetTestOnlyCBORClientFeatureGatesForTest(t, true, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsAllowCBOR, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsPreferCBOR, true) testWebhookAdmission(t, false, func(t testing.TB, config *rest.Config) { config.Wrap(framework.AssertRequestResponseAsCBOR(t)) }) diff --git a/test/integration/apiserver/apply/apply_test.go b/test/integration/apiserver/apply/apply_test.go index 058867a026d..ac2750e2be8 100644 --- a/test/integration/apiserver/apply/apply_test.go +++ b/test/integration/apiserver/apply/apply_test.go @@ -46,6 +46,8 @@ import ( appsv1ac "k8s.io/client-go/applyconfigurations/apps/v1" corev1ac "k8s.io/client-go/applyconfigurations/core/v1" metav1ac "k8s.io/client-go/applyconfigurations/meta/v1" + clientfeatures "k8s.io/client-go/features" + clientfeaturestesting "k8s.io/client-go/features/testing" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" restclient "k8s.io/client-go/rest" @@ -1057,7 +1059,7 @@ func TestPatchVeryLargeObject(t *testing.T) { // syntax suffix for application/apply-patch and with CBOR enabled. func TestPatchVeryLargeObjectCBORApply(t *testing.T) { framework.EnableCBORServingAndStorageForTest(t) - framework.SetTestOnlyCBORClientFeatureGatesForTest(t, true, false) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsAllowCBOR, true) client, closeFn := setup(t) defer closeFn() diff --git a/test/integration/apiserver/apply/status_test.go b/test/integration/apiserver/apply/status_test.go index 322faeec241..6fbff8db175 100644 --- a/test/integration/apiserver/apply/status_test.go +++ b/test/integration/apiserver/apply/status_test.go @@ -29,6 +29,8 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/json" "k8s.io/client-go/dynamic" + clientfeatures "k8s.io/client-go/features" + clientfeaturestesting "k8s.io/client-go/features/testing" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" apiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" @@ -99,7 +101,8 @@ func TestApplyStatus(t *testing.T) { // TestApplyStatus makes sure that applying the status works for all known types. func TestApplyStatusWithCBOR(t *testing.T) { framework.EnableCBORServingAndStorageForTest(t) - framework.SetTestOnlyCBORClientFeatureGatesForTest(t, true, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsAllowCBOR, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsPreferCBOR, true) testApplyStatus(t, func(t testing.TB, config *rest.Config) { config.Wrap(framework.AssertRequestResponseAsCBOR(t)) }) diff --git a/test/integration/apiserver/cbor_test.go b/test/integration/apiserver/cbor_test.go index 9368bd36b52..fbd16b1d468 100644 --- a/test/integration/apiserver/cbor_test.go +++ b/test/integration/apiserver/cbor_test.go @@ -36,6 +36,8 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer/cbor/direct" "k8s.io/apimachinery/pkg/runtime/serializer/streaming" "k8s.io/apimachinery/pkg/types" + clientfeatures "k8s.io/client-go/features" + clientfeaturestesting "k8s.io/client-go/features/testing" "k8s.io/client-go/kubernetes/scheme" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" @@ -57,7 +59,8 @@ func TestNondeterministicResponseEncoding(t *testing.T) { const NTrials = 40 framework.EnableCBORServingAndStorageForTest(t) - framework.SetTestOnlyCBORClientFeatureGatesForTest(t, true, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsAllowCBOR, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsPreferCBOR, true) server := kubeapiservertesting.StartTestServerOrDie(t, nil, framework.DefaultTestServerFlags(), framework.SharedEtcd()) t.Cleanup(server.TearDownFn) diff --git a/test/integration/client/client_test.go b/test/integration/client/client_test.go index 5ba0e869bf8..bda92bdd731 100644 --- a/test/integration/client/client_test.go +++ b/test/integration/client/client_test.go @@ -51,6 +51,8 @@ import ( corev1ac "k8s.io/client-go/applyconfigurations/core/v1" metav1ac "k8s.io/client-go/applyconfigurations/meta/v1" "k8s.io/client-go/discovery" + clientfeatures "k8s.io/client-go/features" + clientfeaturestesting "k8s.io/client-go/features/testing" "k8s.io/client-go/gentype" "k8s.io/client-go/kubernetes" clientscheme "k8s.io/client-go/kubernetes/scheme" @@ -1754,7 +1756,8 @@ func TestClientCBOREnablement(t *testing.T) { } t.Run(tc.name, func(t *testing.T) { - framework.SetTestOnlyCBORClientFeatureGatesForTest(t, tc.allowed, tc.preferred) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsAllowCBOR, tc.allowed) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsPreferCBOR, tc.preferred) config := rest.CopyConfig(server.ClientConfig) config.ContentType = tc.configuredContentType @@ -1795,7 +1798,8 @@ func TestClientCBOREnablement(t *testing.T) { func TestCBORWithTypedClient(t *testing.T) { framework.EnableCBORServingAndStorageForTest(t) - framework.SetTestOnlyCBORClientFeatureGatesForTest(t, true, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsAllowCBOR, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsPreferCBOR, true) server := kubeapiservertesting.StartTestServerOrDie(t, nil, framework.DefaultTestServerFlags(), framework.SharedEtcd()) t.Cleanup(server.TearDownFn) @@ -1986,7 +1990,8 @@ func TestCBORWithTypedClient(t *testing.T) { } func TestUnsupportedMediaTypeCircuitBreaker(t *testing.T) { - framework.SetTestOnlyCBORClientFeatureGatesForTest(t, true, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsAllowCBOR, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsPreferCBOR, true) server := kubeapiservertesting.StartTestServerOrDie(t, nil, framework.DefaultTestServerFlags(), framework.SharedEtcd()) t.Cleanup(server.TearDownFn) diff --git a/test/integration/client/dynamic_client_test.go b/test/integration/client/dynamic_client_test.go index 03a4ea4b953..13abc33c5d0 100644 --- a/test/integration/client/dynamic_client_test.go +++ b/test/integration/client/dynamic_client_test.go @@ -41,6 +41,8 @@ import ( metav1ac "k8s.io/client-go/applyconfigurations/meta/v1" "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" + clientfeatures "k8s.io/client-go/features" + clientfeaturestesting "k8s.io/client-go/features/testing" clientset "k8s.io/client-go/kubernetes" clientscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -145,7 +147,8 @@ func TestDynamicClientWatch(t *testing.T) { func TestDynamicClientWatchWithCBOR(t *testing.T) { framework.EnableCBORServingAndStorageForTest(t) - framework.SetTestOnlyCBORClientFeatureGatesForTest(t, true, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsAllowCBOR, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsPreferCBOR, true) result := kubeapiservertesting.StartTestServerOrDie(t, nil, framework.DefaultTestServerFlags(), framework.SharedEtcd()) defer result.TearDownFn() @@ -554,7 +557,8 @@ func TestDynamicClientCBOREnablement(t *testing.T) { } t.Run(tc.name, func(t *testing.T) { - framework.SetTestOnlyCBORClientFeatureGatesForTest(t, tc.allowed, tc.preferred) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsAllowCBOR, tc.allowed) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsPreferCBOR, tc.preferred) config := rest.CopyConfig(server.ClientConfig) config.Wrap(func(rt http.RoundTripper) http.RoundTripper { @@ -591,7 +595,8 @@ func TestDynamicClientCBOREnablement(t *testing.T) { } func TestUnsupportedMediaTypeCircuitBreakerDynamicClient(t *testing.T) { - framework.SetTestOnlyCBORClientFeatureGatesForTest(t, true, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsAllowCBOR, true) + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.ClientsPreferCBOR, true) server := kubeapiservertesting.StartTestServerOrDie(t, nil, framework.DefaultTestServerFlags(), framework.SharedEtcd()) t.Cleanup(server.TearDownFn) diff --git a/test/integration/framework/cbor.go b/test/integration/framework/cbor.go index 234cd8d0758..10ef193498c 100644 --- a/test/integration/framework/cbor.go +++ b/test/integration/framework/cbor.go @@ -31,53 +31,19 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/features" utilfeature "k8s.io/apiserver/pkg/util/feature" - clientfeatures "k8s.io/client-go/features" "k8s.io/client-go/transport" featuregatetesting "k8s.io/component-base/featuregate/testing" aggregatorscheme "k8s.io/kube-aggregator/pkg/apiserver/scheme" "k8s.io/kubernetes/pkg/api/legacyscheme" ) -// SetTestOnlyCBORClientFeatureGatesForTest overrides the CBOR client feature gates in the test-only -// client feature gate instance for the duration of a test. The CBOR client feature gates are -// temporarily registered in their own feature gate instance that does not include runtime wiring to -// command-line flags or environment variables in order to mitigate the risk of enabling a new -// encoding before all integration tests have been demonstrated to pass. -// -// This will be removed as an alpha requirement. The client feature gates will be registered with -// the existing feature gate instance and tests will use -// k8s.io/client-go/features/testing.SetFeatureDuringTest (which unlike -// k8s.io/component-base/featuregate/testing.SetFeatureGateDuringTest does not accept a feature gate -// instance as a parameter). -func SetTestOnlyCBORClientFeatureGatesForTest(tb testing.TB, allowed, preferred bool) { - originalAllowed := clientfeatures.TestOnlyFeatureGates.Enabled(clientfeatures.TestOnlyClientAllowsCBOR) - tb.Cleanup(func() { - if err := clientfeatures.TestOnlyFeatureGates.Set(clientfeatures.TestOnlyClientAllowsCBOR, originalAllowed); err != nil { - tb.Fatal(err) - } - }) - if err := clientfeatures.TestOnlyFeatureGates.Set(clientfeatures.TestOnlyClientAllowsCBOR, allowed); err != nil { - tb.Fatal(err) - } - - originalPreferred := clientfeatures.TestOnlyFeatureGates.Enabled(clientfeatures.TestOnlyClientPrefersCBOR) - tb.Cleanup(func() { - if err := clientfeatures.TestOnlyFeatureGates.Set(clientfeatures.TestOnlyClientPrefersCBOR, originalPreferred); err != nil { - tb.Fatal(err) - } - }) - if err := clientfeatures.TestOnlyFeatureGates.Set(clientfeatures.TestOnlyClientPrefersCBOR, preferred); err != nil { - tb.Fatal(err) - } -} - // EnableCBORForTest patches global state to enable the CBOR serializer and reverses those changes // at the end of the test. As a risk mitigation, integration tests are initially written this way so // that integration tests can be implemented fully and incrementally before exposing options // (including feature gates) that can enable CBOR at runtime. After integration test coverage is // complete, feature gates will be introduced to completely supersede this mechanism. func EnableCBORServingAndStorageForTest(tb testing.TB) { - featuregatetesting.SetFeatureGateDuringTest(tb, utilfeature.TestOnlyFeatureGate, features.TestOnlyCBORServingAndStorage, true) + featuregatetesting.SetFeatureGateDuringTest(tb, utilfeature.DefaultFeatureGate, features.CBORServingAndStorage, true) newCBORSerializerInfo := func(creater runtime.ObjectCreater, typer runtime.ObjectTyper) runtime.SerializerInfo { return runtime.SerializerInfo{