Merge pull request #128539 from benluddy/cbor-feature-gates

KEP-4222: Add CBOR feature gates.

Kubernetes-commit: a28f14089cfa47ef9c57f9f283e1504a68f616d6

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",

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"
 		}
 	}

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

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},
 }

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
 	}
 

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

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"
 		}
 	}

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
 	}