Add test-only client feature gates for CBOR.

As with the apiserver feature gate for CBOR as a serving and storage encoding, the client feature
gates for CBOR are being initially added through a test-only feature gate instance that is not wired
to environment variables or to command-line flags and is intended only to be enabled
programmatically from integration tests. The test-only instance will be removed as part of alpha
graduation and replaced by conventional client feature gating.

staging/src/k8s.io/client-go/dynamic/scheme.go

@@ -21,7 +21,9 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/runtime/serializer/cbor"
 	"k8s.io/apimachinery/pkg/runtime/serializer/json"
+	"k8s.io/client-go/features"
 )
 
 var basicScheme = runtime.NewScheme()
@@ -35,11 +37,8 @@ func init() {
 	metav1.AddToGroupVersion(parameterScheme, versionV1)
 }
 
-// basicNegotiatedSerializer is used to handle discovery and error handling serialization
+func newBasicNegotiatedSerializer() basicNegotiatedSerializer {
-type basicNegotiatedSerializer struct{}
+	supportedMediaTypes := []runtime.SerializerInfo{
-
-func (s basicNegotiatedSerializer) SupportedMediaTypes() []runtime.SerializerInfo {
-	return []runtime.SerializerInfo{
 		{
 			MediaType:        "application/json",
 			MediaTypeType:    "application",
@@ -54,6 +53,27 @@ func (s basicNegotiatedSerializer) SupportedMediaTypes() []runtime.SerializerInf
 			},
 		},
 	}
+	if features.TestOnlyFeatureGates.Enabled(features.TestOnlyClientAllowsCBOR) {
+		supportedMediaTypes = append(supportedMediaTypes, runtime.SerializerInfo{
+			MediaType:        "application/cbor",
+			MediaTypeType:    "application",
+			MediaTypeSubType: "cbor",
+			Serializer:       cbor.NewSerializer(unstructuredCreater{basicScheme}, unstructuredTyper{basicScheme}),
+			StreamSerializer: &runtime.StreamSerializerInfo{
+				Serializer: cbor.NewSerializer(basicScheme, basicScheme, cbor.Transcode(false)),
+				Framer:     cbor.NewFramer(),
+			},
+		})
+	}
+	return basicNegotiatedSerializer{supportedMediaTypes: supportedMediaTypes}
+}
+
+type basicNegotiatedSerializer struct {
+	supportedMediaTypes []runtime.SerializerInfo
+}
+
+func (s basicNegotiatedSerializer) SupportedMediaTypes() []runtime.SerializerInfo {
+	return s.supportedMediaTypes
 }
 
 func (s basicNegotiatedSerializer) EncoderForVersion(encoder runtime.Encoder, gv runtime.GroupVersioner) runtime.Encoder {

staging/src/k8s.io/client-go/dynamic/simple.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/watch"
+	"k8s.io/client-go/features"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/util/consistencydetector"
 	"k8s.io/client-go/util/watchlist"
@@ -45,9 +46,17 @@ var _ Interface = &DynamicClient{}
 // appropriate dynamic client defaults set.
 func ConfigFor(inConfig *rest.Config) *rest.Config {
 	config := rest.CopyConfig(inConfig)
-	config.AcceptContentTypes = "application/json"
+
 	config.ContentType = "application/json"
-	config.NegotiatedSerializer = basicNegotiatedSerializer{} // this gets used for discovery and error handling types
+	config.AcceptContentTypes = "application/json"
+	if features.TestOnlyFeatureGates.Enabled(features.TestOnlyClientAllowsCBOR) {
+		config.AcceptContentTypes = "application/json;q=0.9,application/cbor;q=1"
+		if features.TestOnlyFeatureGates.Enabled(features.TestOnlyClientPrefersCBOR) {
+			config.ContentType = "application/cbor"
+		}
+	}
+
+	config.NegotiatedSerializer = newBasicNegotiatedSerializer()
 	if config.UserAgent == "" {
 		config.UserAgent = rest.DefaultKubernetesUserAgent()
 	}

staging/src/k8s.io/client-go/features/features.go

@@ -18,9 +18,11 @@ package features
 
 import (
 	"errors"
+	"fmt"
+	"sync"
+	"sync/atomic"
 
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-	"sync/atomic"
 )
 
 // NOTE: types Feature, FeatureSpec, prerelease (and its values)
@@ -141,3 +143,43 @@ 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
+}

staging/src/k8s.io/client-go/features/known_features.go

@@ -41,6 +41,27 @@ const (
 	// 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.

test/integration/client/dynamic_client_test.go

@@ -20,11 +20,13 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"net/http"
 	"reflect"
 	"testing"
 	"time"
 
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/fields"
@@ -38,6 +40,7 @@ import (
 	"k8s.io/client-go/dynamic"
 	clientset "k8s.io/client-go/kubernetes"
 	clientscheme "k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/rest"
 	kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
 	"k8s.io/kubernetes/test/integration/framework"
 )
@@ -55,12 +58,12 @@ func TestDynamicClient(t *testing.T) {
 	resource := schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}
 
 	// Create a Pod with the normal client
-	pod := &v1.Pod{
+	pod := &corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: "test",
 		},
-		Spec: v1.PodSpec{
+		Spec: corev1.PodSpec{
-			Containers: []v1.Container{
+			Containers: []corev1.Container{
 				{
 					Name:  "test",
 					Image: "test-image",
@@ -134,16 +137,16 @@ func TestDynamicClientWatch(t *testing.T) {
 		t.Fatalf("unexpected error creating dynamic client: %v", err)
 	}
 
-	resource := v1.SchemeGroupVersion.WithResource("events")
+	resource := corev1.SchemeGroupVersion.WithResource("events")
 
-	mkEvent := func(i int) *v1.Event {
+	mkEvent := func(i int) *corev1.Event {
 		name := fmt.Sprintf("event-%v", i)
-		return &v1.Event{
+		return &corev1.Event{
 			ObjectMeta: metav1.ObjectMeta{
 				Namespace: "default",
 				Name:      name,
 			},
-			InvolvedObject: v1.ObjectReference{
+			InvolvedObject: corev1.ObjectReference{
 				Namespace: "default",
 				Name:      name,
 			},
@@ -276,24 +279,171 @@ func TestUnstructuredExtract(t *testing.T) {
 
 }
 
-func unstructuredToPod(obj *unstructured.Unstructured) (*v1.Pod, error) {
+func unstructuredToPod(obj *unstructured.Unstructured) (*corev1.Pod, error) {
 	json, err := runtime.Encode(unstructured.UnstructuredJSONScheme, obj)
 	if err != nil {
 		return nil, err
 	}
-	pod := new(v1.Pod)
+	pod := new(corev1.Pod)
-	err = runtime.DecodeInto(clientscheme.Codecs.LegacyCodec(v1.SchemeGroupVersion), json, pod)
+	err = runtime.DecodeInto(clientscheme.Codecs.LegacyCodec(corev1.SchemeGroupVersion), json, pod)
 	pod.Kind = ""
 	pod.APIVersion = ""
 	return pod, err
 }
 
-func unstructuredToEvent(obj *unstructured.Unstructured) (*v1.Event, error) {
+func unstructuredToEvent(obj *unstructured.Unstructured) (*corev1.Event, error) {
 	json, err := runtime.Encode(unstructured.UnstructuredJSONScheme, obj)
 	if err != nil {
 		return nil, err
 	}
-	event := new(v1.Event)
+	event := new(corev1.Event)
-	err = runtime.DecodeInto(clientscheme.Codecs.LegacyCodec(v1.SchemeGroupVersion), json, event)
+	err = runtime.DecodeInto(clientscheme.Codecs.LegacyCodec(corev1.SchemeGroupVersion), json, event)
 	return event, err
 }
+
+func TestDynamicClientCBOREnablement(t *testing.T) {
+	for _, tc := range []struct {
+		name                    string
+		serving                 bool
+		allowed                 bool
+		preferred               bool
+		wantRequestContentType  string
+		wantRequestAccept       string
+		wantResponseContentType string
+		wantResponseStatus      int
+		wantStatusError         bool
+	}{
+		{
+			name:                    "sends cbor accepts both gets cbor",
+			serving:                 true,
+			allowed:                 true,
+			preferred:               true,
+			wantRequestContentType:  "application/cbor",
+			wantRequestAccept:       "application/json;q=0.9,application/cbor;q=1",
+			wantResponseContentType: "application/cbor",
+			wantResponseStatus:      http.StatusCreated,
+			wantStatusError:         false,
+		},
+		{
+			name:                    "sends cbor accepts both gets 415",
+			serving:                 false,
+			allowed:                 true,
+			preferred:               true,
+			wantRequestContentType:  "application/cbor",
+			wantRequestAccept:       "application/json;q=0.9,application/cbor;q=1",
+			wantResponseContentType: "application/json",
+			wantResponseStatus:      http.StatusUnsupportedMediaType,
+			wantStatusError:         true,
+		},
+		{
+			name:                    "sends json accepts both gets cbor",
+			serving:                 true,
+			allowed:                 true,
+			preferred:               false,
+			wantRequestContentType:  "application/json",
+			wantRequestAccept:       "application/json;q=0.9,application/cbor;q=1",
+			wantResponseContentType: "application/cbor",
+			wantResponseStatus:      http.StatusCreated,
+			wantStatusError:         false,
+		},
+		{
+			name:                    "sends json accepts both gets json",
+			serving:                 false,
+			allowed:                 true,
+			preferred:               false,
+			wantRequestContentType:  "application/json",
+			wantRequestAccept:       "application/json;q=0.9,application/cbor;q=1",
+			wantResponseContentType: "application/json",
+			wantResponseStatus:      http.StatusCreated,
+			wantStatusError:         false,
+		},
+		{
+			name:                    "sends json accepts json gets json with serving enabled",
+			serving:                 true,
+			allowed:                 false,
+			preferred:               false,
+			wantRequestContentType:  "application/json",
+			wantRequestAccept:       "application/json",
+			wantResponseContentType: "application/json",
+			wantResponseStatus:      http.StatusCreated,
+			wantStatusError:         false,
+		},
+		{
+			name:                    "sends json accepts json gets json with serving disabled",
+			serving:                 false,
+			allowed:                 false,
+			preferred:               false,
+			wantRequestContentType:  "application/json",
+			wantRequestAccept:       "application/json",
+			wantResponseContentType: "application/json",
+			wantResponseStatus:      http.StatusCreated,
+			wantStatusError:         false,
+		},
+		{
+			name:                    "sends json without both gates enabled",
+			serving:                 true,
+			allowed:                 false,
+			preferred:               true,
+			wantRequestContentType:  "application/json",
+			wantRequestAccept:       "application/json",
+			wantResponseContentType: "application/json",
+			wantResponseStatus:      http.StatusCreated,
+			wantStatusError:         false,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.serving {
+				framework.EnableCBORServingAndStorageForTest(t)
+			}
+
+			server := kubeapiservertesting.StartTestServerOrDie(t, nil, framework.DefaultTestServerFlags(), framework.SharedEtcd())
+			defer server.TearDownFn()
+
+			framework.SetTestOnlyCBORClientFeatureGatesForTest(t, tc.allowed, tc.preferred)
+
+			config := rest.CopyConfig(server.ClientConfig)
+			config.Wrap(func(rt http.RoundTripper) http.RoundTripper {
+				return roundTripperFunc(func(request *http.Request) (*http.Response, error) {
+					response, err := rt.RoundTrip(request)
+					if got := response.Request.Header.Get("Content-Type"); got != tc.wantRequestContentType {
+						t.Errorf("want request content type %q, got %q", tc.wantRequestContentType, got)
+					}
+					if got := response.Request.Header.Get("Accept"); got != tc.wantRequestAccept {
+						t.Errorf("want request accept %q, got %q", tc.wantRequestAccept, got)
+					}
+					if got := response.Header.Get("Content-Type"); got != tc.wantResponseContentType {
+						t.Errorf("want response content type %q, got %q", tc.wantResponseContentType, got)
+					}
+					if got := response.StatusCode; got != tc.wantResponseStatus {
+						t.Errorf("want response status %d, got %d", tc.wantResponseStatus, got)
+					}
+					return response, err
+				})
+			})
+			client, err := dynamic.NewForConfig(config)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			_, err = client.Resource(corev1.SchemeGroupVersion.WithResource("namespaces")).Create(
+				context.TODO(),
+				&unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"metadata": map[string]interface{}{
+							"name": "test-dynamic-client-cbor-enablement",
+						},
+					},
+				},
+				metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}},
+			)
+			switch {
+			case tc.wantStatusError && errors.IsUnsupportedMediaType(err):
+				// ok
+			case !tc.wantStatusError && err == nil:
+				// ok
+			default:
+				t.Errorf("unexpected error: %v", err)
+			}
+		})
+	}
+}

test/integration/framework/cbor.go

@@ -26,11 +26,45 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/serializer/cbor"
 	"k8s.io/apiserver/pkg/features"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	clientfeatures "k8s.io/client-go/features"
 	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