From 8cb21671f43c8438beb7116b32c406cdfbce3cec 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. Kubernetes-commit: 072dfcb416fd4e1ddab0a89ac4faf519e268bc96 --- dynamic/scheme.go | 2 +- dynamic/simple.go | 4 +-- features/features.go | 42 ----------------------------- features/known_features.go | 54 ++++++++++++++++++++------------------ rest/client.go | 6 ++--- rest/config.go | 2 +- rest/request.go | 2 +- util/apply/apply.go | 2 +- 8 files changed, 37 insertions(+), 77 deletions(-) diff --git a/dynamic/scheme.go b/dynamic/scheme.go index dbee0531..28316f1d 100644 --- a/dynamic/scheme.go +++ b/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/dynamic/simple.go b/dynamic/simple.go index 6c729982..62b2999c 100644 --- a/dynamic/simple.go +++ b/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/features/features.go b/features/features.go index 19056df1..5ccdcc55 100644 --- a/features/features.go +++ b/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/features/known_features.go b/features/known_features.go index 9a6a7364..a74f6a83 100644 --- a/features/known_features.go +++ b/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/rest/client.go b/rest/client.go index e548eaf5..9bcb99c5 100644 --- a/rest/client.go +++ b/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/rest/config.go b/rest/config.go index c58e9668..aebd990a 100644 --- a/rest/config.go +++ b/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/rest/request.go b/rest/request.go index 46863bb2..65942aa1 100644 --- a/rest/request.go +++ b/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/util/apply/apply.go b/util/apply/apply.go index c135f559..0cc85df6 100644 --- a/util/apply/apply.go +++ b/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 }