Fix content type fallback when a client defaults to CBOR.

With the ClientsAllowCBOR client-go feature gate enabled, a 415 response to a CBOR-encoded REST
causes all subsequent requests from the client to fall back to a JSON request encoding. This
mechanism had only worked as intended when CBOR was explicitly configured in the
ClientContentConfig. When both ClientsAllowCBOR and ClientsPreferCBOR are enabled, an
unconfigured (empty) content type defaults to CBOR instead of JSON. Both ways of configuring a
client to use the CBOR request encoding are now subject to the same fallback mechanism.
This commit is contained in:
Ben Luddy 2024-11-07 00:05:03 -05:00
parent c9024e7ae6
commit a77f4c7ba2
No known key found for this signature in database
GPG Key ID: A6551E73A5974C30
3 changed files with 77 additions and 54 deletions

View File

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

View File

@ -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()

View File

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