diff --git a/staging/src/k8s.io/client-go/rest/client.go b/staging/src/k8s.io/client-go/rest/client.go index 393a4166913..159caa13fab 100644 --- a/staging/src/k8s.io/client-go/rest/client.go +++ b/staging/src/k8s.io/client-go/rest/client.go @@ -238,7 +238,8 @@ func (c *RESTClient) Delete() *Request { // APIVersion returns the APIVersion this RESTClient is expected to use. func (c *RESTClient) APIVersion() schema.GroupVersion { - return c.content.GetClientContentConfig().GroupVersion + config, _ := c.content.GetClientContentConfig() + return config.GroupVersion } // requestClientContentConfigProvider observes HTTP 415 (Unsupported Media Type) responses to detect @@ -257,25 +258,35 @@ type requestClientContentConfigProvider struct { } // GetClientContentConfig returns the ClientContentConfig that should be used for new requests by -// this client. -func (p *requestClientContentConfigProvider) GetClientContentConfig() ClientContentConfig { +// this client and true if the request ContentType was selected by default. +func (p *requestClientContentConfigProvider) GetClientContentConfig() (ClientContentConfig, bool) { + config := p.base + + defaulted := config.ContentType == "" + if defaulted { + config.ContentType = "application/json" + } + if !clientfeatures.FeatureGates().Enabled(clientfeatures.ClientsAllowCBOR) { - return p.base + return config, defaulted + } + + if defaulted && clientfeatures.FeatureGates().Enabled(clientfeatures.ClientsPreferCBOR) { + config.ContentType = "application/cbor" } if sawUnsupportedMediaTypeForCBOR := p.sawUnsupportedMediaTypeForCBOR.Load(); !sawUnsupportedMediaTypeForCBOR { - return p.base + return config, defaulted } - if mediaType, _, _ := mime.ParseMediaType(p.base.ContentType); mediaType != runtime.ContentTypeCBOR { - return p.base + if mediaType, _, _ := mime.ParseMediaType(config.ContentType); mediaType != runtime.ContentTypeCBOR { + return config, defaulted } - config := p.base - // The default ClientContentConfig sets ContentType to CBOR and the client has previously - // received an HTTP 415 in response to a CBOR request. Override ContentType to JSON. + // The effective ContentType is CBOR and the client has previously received an HTTP 415 in + // response to a CBOR request. Override ContentType to JSON. config.ContentType = runtime.ContentTypeJSON - return config + return config, defaulted } // UnsupportedMediaType reports that the server has responded to a request with HTTP 415 Unsupported diff --git a/staging/src/k8s.io/client-go/rest/request.go b/staging/src/k8s.io/client-go/rest/request.go index 65942aa1233..0ec90ad188b 100644 --- a/staging/src/k8s.io/client-go/rest/request.go +++ b/staging/src/k8s.io/client-go/rest/request.go @@ -156,14 +156,10 @@ func NewRequest(c *RESTClient) *Request { timeout = c.Client.Timeout } - contentConfig := c.content.GetClientContentConfig() - contentTypeNotSet := len(contentConfig.ContentType) == 0 - if contentTypeNotSet { - contentConfig.ContentType = "application/json" - if clientfeatures.FeatureGates().Enabled(clientfeatures.ClientsAllowCBOR) && clientfeatures.FeatureGates().Enabled(clientfeatures.ClientsPreferCBOR) { - contentConfig.ContentType = "application/cbor" - } - } + // A request needs to know whether the content type was explicitly configured or selected by + // default in order to support the per-request Protobuf override used by clients generated + // with --prefers-protobuf. + contentConfig, contentTypeDefaulted := c.content.GetClientContentConfig() r := &Request{ c: c, @@ -176,7 +172,7 @@ func NewRequest(c *RESTClient) *Request { warningHandler: c.warningHandler, contentConfig: contentConfig, - contentTypeNotSet: contentTypeNotSet, + contentTypeNotSet: contentTypeDefaulted, } r.setAcceptHeader() diff --git a/test/e2e/apimachinery/protocol.go b/test/e2e/apimachinery/protocol.go index b40f52275a0..491f5c15b97 100644 --- a/test/e2e/apimachinery/protocol.go +++ b/test/e2e/apimachinery/protocol.go @@ -26,12 +26,21 @@ import ( v1 "k8s.io/api/core/v1" apierrors "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" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic" + clientfeatures "k8s.io/client-go/features" + clientfeaturestesting "k8s.io/client-go/features/testing" "k8s.io/client-go/kubernetes" - admissionapi "k8s.io/pod-security-admission/api" - + aggregatorclientset "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset" + "k8s.io/kubernetes/test/e2e/feature" "k8s.io/kubernetes/test/e2e/framework" + imageutils "k8s.io/kubernetes/test/utils/image" + admissionapi "k8s.io/pod-security-admission/api" + samplev1alpha1 "k8s.io/sample-apiserver/pkg/apis/wardle/v1alpha1" + samplev1alpha1client "k8s.io/sample-apiserver/pkg/generated/clientset/versioned/typed/wardle/v1alpha1" ) var _ = SIGDescribe("client-go should negotiate", func() { @@ -96,3 +105,67 @@ var _ = SIGDescribe("client-go should negotiate", func() { }) } }) + +var _ = SIGDescribe("CBOR", feature.CBOR, func() { + f := framework.NewDefaultFramework("cbor") + f.NamespacePodSecurityLevel = admissionapi.LevelBaseline + + // Must be serial to avoid conflict with other tests that set up a sample apiserver. + f.It("clients remain compatible with the 1.17 sample-apiserver", f.WithSerial(), func(ctx context.Context) { + clientfeaturestesting.SetFeatureDuringTest(g.GinkgoTB(), clientfeatures.ClientsAllowCBOR, true) + clientfeaturestesting.SetFeatureDuringTest(g.GinkgoTB(), clientfeatures.ClientsPreferCBOR, true) + + clientConfig, err := framework.LoadConfig() + framework.ExpectNoError(err) + + aggregatorClient, err := aggregatorclientset.NewForConfig(clientConfig) + framework.ExpectNoError(err) + + dynamicClient, err := dynamic.NewForConfig(clientConfig) + framework.ExpectNoError(err) + + objectNames := generateSampleAPIServerObjectNames(f.Namespace.Name) + g.DeferCleanup(func(ctx context.Context) { + cleanupSampleAPIServer(ctx, f.ClientSet, aggregatorClient, objectNames, samplev1alpha1.SchemeGroupVersion.Version+"."+samplev1alpha1.SchemeGroupVersion.Group) + }) + SetUpSampleAPIServer(ctx, f, aggregatorClient, imageutils.GetE2EImage(imageutils.APIServer), objectNames, samplev1alpha1.SchemeGroupVersion.Group, samplev1alpha1.SchemeGroupVersion.Version) + + flunder := samplev1alpha1.Flunder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-flunder", + }, + } + + g.By("making requests with a generated client", func() { + sampleClient, err := samplev1alpha1client.NewForConfig(clientConfig) + framework.ExpectNoError(err) + + _, err = sampleClient.Flunders(f.Namespace.Name).List(ctx, metav1.ListOptions{LabelSelector: "a,!a"}) + framework.ExpectNoError(err, "Failed to list with generated client") + + _, err = sampleClient.Flunders(f.Namespace.Name).Create(ctx, &flunder, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}) + o.Expect(err).To(o.MatchError(apierrors.IsUnsupportedMediaType, "Expected 415 (Unsupported Media Type) response on first write with generated client")) + + _, err = sampleClient.Flunders(f.Namespace.Name).Create(ctx, &flunder, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}) + framework.ExpectNoError(err, "Expected subsequent writes to succeed with generated client") + }) + + g.By("making requests with a dynamic client", func() { + unstructuredFlunderContent, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&flunder) + framework.ExpectNoError(err) + unstructuredFlunder := &unstructured.Unstructured{Object: unstructuredFlunderContent} + + flunderDynamicClient := dynamicClient.Resource(samplev1alpha1.SchemeGroupVersion.WithResource("flunders")).Namespace(f.Namespace.Name) + + list, err := flunderDynamicClient.List(ctx, metav1.ListOptions{LabelSelector: "a,!a"}) + framework.ExpectNoError(err, "Failed to list with dynamic client") + o.Expect(list.GetObjectKind().GroupVersionKind()).To(o.Equal(samplev1alpha1.SchemeGroupVersion.WithKind("FlunderList"))) + + _, err = flunderDynamicClient.Create(ctx, unstructuredFlunder, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}) + o.Expect(err).To(o.MatchError(apierrors.IsUnsupportedMediaType, "Expected 415 (Unsupported Media Type) response on first write with dynamic client")) + + _, err = flunderDynamicClient.Create(ctx, unstructuredFlunder, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}) + framework.ExpectNoError(err, "Expected subsequent writes to succeed with dynamic client") + }) + }) +}) diff --git a/test/e2e/feature/feature.go b/test/e2e/feature/feature.go index 557d0930cee..d9cf9d47f46 100644 --- a/test/e2e/feature/feature.go +++ b/test/e2e/feature/feature.go @@ -34,6 +34,10 @@ var ( // TODO: document the feature (owning SIG, when to use this feature for a test) BoundServiceAccountTokenVolume = framework.WithFeature(framework.ValidFeatures.Add("BoundServiceAccountTokenVolume")) + // Owner: sig-api-machinery + // Marks tests that exercise the CBOR data format for serving or storage. + CBOR = framework.WithFeature(framework.ValidFeatures.Add("CBOR")) + // TODO: document the feature (owning SIG, when to use this feature for a test) CloudProvider = framework.WithFeature(framework.ValidFeatures.Add("CloudProvider")) diff --git a/test/integration/client/client_test.go b/test/integration/client/client_test.go index 54cad7d10b3..7f51c941798 100644 --- a/test/integration/client/client_test.go +++ b/test/integration/client/client_test.go @@ -2037,44 +2037,60 @@ func TestUnsupportedMediaTypeCircuitBreaker(t *testing.T) { server := kubeapiservertesting.StartTestServerOrDie(t, nil, framework.DefaultTestServerFlags(), framework.SharedEtcd()) t.Cleanup(server.TearDownFn) - config := rest.CopyConfig(server.ClientConfig) - config.ContentType = "application/cbor" - config.AcceptContentTypes = "application/json" + for _, tc := range []struct { + name string + contentType string + }{ + { + name: "default content type", + contentType: "", + }, + { + name: "explicit content type", + contentType: "application/cbor", + }, + } { + t.Run(tc.name, func(t *testing.T) { + config := rest.CopyConfig(server.ClientConfig) + config.ContentType = tc.contentType + config.AcceptContentTypes = "application/json" - client, err := corev1client.NewForConfig(config) - if err != nil { - t.Fatal(err) - } + client, err := corev1client.NewForConfig(config) + if err != nil { + t.Fatal(err) + } - if _, err := client.Namespaces().Create( - context.TODO(), - &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-client-415"}}, - metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}, - ); !apierrors.IsUnsupportedMediaType(err) { - t.Errorf("expected to receive unsupported media type on first cbor request, got: %v", err) - } + if _, err := client.Namespaces().Create( + context.TODO(), + &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-client-415"}}, + metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}, + ); !apierrors.IsUnsupportedMediaType(err) { + t.Errorf("expected to receive unsupported media type on first cbor request, got: %v", err) + } - // Requests from this client should fall back from application/cbor to application/json. - if _, err := client.Namespaces().Create( - context.TODO(), - &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-client-415"}}, - metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}, - ); err != nil { - t.Errorf("expected to receive nil error on subsequent cbor request, got: %v", err) - } + // Requests from this client should fall back from application/cbor to application/json. + if _, err := client.Namespaces().Create( + context.TODO(), + &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-client-415"}}, + metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}, + ); err != nil { + t.Errorf("expected to receive nil error on subsequent cbor request, got: %v", err) + } - // The circuit breaker trips on a per-client basis, so it should not begin tripped for a - // fresh client with identical config. - client, err = corev1client.NewForConfig(config) - if err != nil { - t.Fatal(err) - } + // The circuit breaker trips on a per-client basis, so it should not begin tripped for a + // fresh client with identical config. + client, err = corev1client.NewForConfig(config) + if err != nil { + t.Fatal(err) + } - if _, err := client.Namespaces().Create( - context.TODO(), - &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-client-415"}}, - metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}, - ); !apierrors.IsUnsupportedMediaType(err) { - t.Errorf("expected to receive unsupported media type on cbor request with fresh client, got: %v", err) + if _, err := client.Namespaces().Create( + context.TODO(), + &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-client-415"}}, + metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}, + ); !apierrors.IsUnsupportedMediaType(err) { + t.Errorf("expected to receive unsupported media type on cbor request with fresh client, got: %v", err) + } + }) } }