Skip to content

Commit fe1eda0

Browse files
enjenj
committed
client-go/rest: move content type wiring from client to request
Signed-off-by: Monis Khan <[email protected]>
1 parent 66e3401 commit fe1eda0

File tree

4 files changed

+70
-64
lines changed

4 files changed

+70
-64
lines changed

‎staging/src/k8s.io/client-go/rest/client.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,6 @@ type RESTClient struct {
105105
// NewRESTClient creates a new RESTClient. This client performs generic REST functions
106106
// such as Get, Put, Post, and Delete on specified paths.
107107
func NewRESTClient(baseURL *url.URL, versionedAPIPath string, config ClientContentConfig, rateLimiter flowcontrol.RateLimiter, client *http.Client) (*RESTClient, error) {
108-
if len(config.ContentType) == 0 {
109-
config.ContentType = "application/json"
110-
}
111-
112108
base := *baseURL
113109
if !strings.HasSuffix(base.Path, "/") {
114110
base.Path += "/"

‎staging/src/k8s.io/client-go/rest/request.go‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ func defaultRequestRetryFn(maxRetries int) WithRetry {
100100
type Request struct {
101101
c *RESTClient
102102

103+
contentConfig ClientContentConfig
104+
contentTypeNotSet bool
105+
103106
warningHandler WarningHandler
104107

105108
rateLimiter flowcontrol.RateLimiter
@@ -153,6 +156,12 @@ func NewRequest(c *RESTClient) *Request {
153156
timeout = c.Client.Timeout
154157
}
155158

159+
contentConfig := c.content
160+
contentTypeNotSet := len(contentConfig.ContentType) == 0
161+
if contentTypeNotSet {
162+
contentConfig.ContentType = "application/json"
163+
}
164+
156165
r := &Request{
157166
c: c,
158167
rateLimiter: c.rateLimiter,
@@ -162,6 +171,9 @@ func NewRequest(c *RESTClient) *Request {
162171
maxRetries: 10,
163172
retryFn: defaultRequestRetryFn,
164173
warningHandler: c.warningHandler,
174+
175+
contentConfig: contentConfig,
176+
contentTypeNotSet: contentTypeNotSet,
165177
}
166178

167179
switch {
@@ -371,7 +383,7 @@ func (r *Request) Param(paramName, s string) *Request {
371383
// VersionedParams will not write query parameters that have omitempty set and are empty. If a
372384
// parameter has already been set it is appended to (Params and VersionedParams are additive).
373385
func (r *Request) VersionedParams(obj runtime.Object, codec runtime.ParameterCodec) *Request {
374-
return r.SpecificallyVersionedParams(obj, codec, r.c.content.GroupVersion)
386+
return r.SpecificallyVersionedParams(obj, codec, r.contentConfig.GroupVersion)
375387
}
376388

377389
func (r *Request) SpecificallyVersionedParams(obj runtime.Object, codec runtime.ParameterCodec, version schema.GroupVersion) *Request {
@@ -464,7 +476,7 @@ func (r *Request) Body(obj interface{}) *Request {
464476
if reflect.ValueOf(t).IsNil() {
465477
return r
466478
}
467-
encoder, err := r.c.content.Negotiator.Encoder(r.c.content.ContentType, nil)
479+
encoder, err := r.contentConfig.Negotiator.Encoder(r.contentConfig.ContentType, nil)
468480
if err != nil {
469481
r.err = err
470482
return r
@@ -476,7 +488,7 @@ func (r *Request) Body(obj interface{}) *Request {
476488
}
477489
r.body = nil
478490
r.bodyBytes = data
479-
r.SetHeader("Content-Type", r.c.content.ContentType)
491+
r.SetHeader("Content-Type", r.contentConfig.ContentType)
480492
default:
481493
r.err = fmt.Errorf("unknown type used for body: %+v", obj)
482494
}
@@ -944,7 +956,7 @@ func (r *Request) newStreamWatcher(resp *http.Response) (watch.Interface, runtim
944956
if err != nil {
945957
klog.V(4).Infof("Unexpected content type from the server: %q: %v", contentType, err)
946958
}
947-
objectDecoder, streamingSerializer, framer, err := r.c.content.Negotiator.StreamDecoder(mediaType, params)
959+
objectDecoder, streamingSerializer, framer, err := r.contentConfig.Negotiator.StreamDecoder(mediaType, params)
948960
if err != nil {
949961
return nil, nil, err
950962
}
@@ -1310,15 +1322,15 @@ func (r *Request) transformResponse(ctx context.Context, resp *http.Response, re
13101322
var decoder runtime.Decoder
13111323
contentType := resp.Header.Get("Content-Type")
13121324
if len(contentType) == 0 {
1313-
contentType = r.c.content.ContentType
1325+
contentType = r.contentConfig.ContentType
13141326
}
13151327
if len(contentType) > 0 {
13161328
var err error
13171329
mediaType, params, err := mime.ParseMediaType(contentType)
13181330
if err != nil {
13191331
return Result{err: errors.NewInternalError(err)}
13201332
}
1321-
decoder, err = r.c.content.Negotiator.Decoder(mediaType, params)
1333+
decoder, err = r.contentConfig.Negotiator.Decoder(mediaType, params)
13221334
if err != nil {
13231335
// if we fail to negotiate a decoder, treat this as an unstructured error
13241336
switch {
@@ -1445,7 +1457,7 @@ func (r *Request) newUnstructuredResponseError(body []byte, isTextResponse bool,
14451457
}
14461458
var groupResource schema.GroupResource
14471459
if len(r.resource) > 0 {
1448-
groupResource.Group = r.c.content.GroupVersion.Group
1460+
groupResource.Group = r.contentConfig.GroupVersion.Group
14491461
groupResource.Resource = r.resource
14501462
}
14511463
return errors.NewGenericServerResponse(

‎staging/src/k8s.io/client-go/rest/request_test.go‎

Lines changed: 50 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import (
4343
"github.com/stretchr/testify/require"
4444

4545
"github.com/google/go-cmp/cmp"
46+
4647
v1 "k8s.io/api/core/v1"
4748
apiequality "k8s.io/apimachinery/pkg/api/equality"
4849
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -66,7 +67,7 @@ import (
6667

6768
func TestNewRequestSetsAccept(t *testing.T) {
6869
r := NewRequestWithClient(&url.URL{Path: "/path/"}, "", ClientContentConfig{}, nil).Verb("get")
69-
if r.headers.Get("Accept") != "" {
70+
if r.headers.Get("Accept") != "application/json, */*" {
7071
t.Errorf("unexpected headers: %#v", r.headers)
7172
}
7273
r = NewRequestWithClient(&url.URL{Path: "/path/"}, "", ClientContentConfig{ContentType: "application/other"}, nil).Verb("get")
@@ -112,9 +113,9 @@ func TestRequestWithErrorWontChange(t *testing.T) {
112113
gvCopy := v1.SchemeGroupVersion
113114
original := Request{
114115
err: errors.New("test"),
115-
c: &RESTClient{
116-
content: ClientContentConfig{GroupVersion: gvCopy},
117-
},
116+
c: &RESTClient{},
117+
118+
contentConfig: ClientContentConfig{GroupVersion: gvCopy},
118119
}
119120
r := original
120121
changed := r.Param("foo", "bar").
@@ -236,7 +237,7 @@ func TestRequestParam(t *testing.T) {
236237
}
237238

238239
func TestRequestVersionedParams(t *testing.T) {
239-
r := (&Request{c: &RESTClient{content: ClientContentConfig{GroupVersion: v1.SchemeGroupVersion}}}).Param("foo", "a")
240+
r := (&Request{c: &RESTClient{}, contentConfig: ClientContentConfig{GroupVersion: v1.SchemeGroupVersion}}).Param("foo", "a")
240241
if !reflect.DeepEqual(r.params, url.Values{"foo": []string{"a"}}) {
241242
t.Errorf("should have set a param: %#v", r)
242243
}
@@ -252,7 +253,7 @@ func TestRequestVersionedParams(t *testing.T) {
252253
}
253254

254255
func TestRequestVersionedParamsFromListOptions(t *testing.T) {
255-
r := &Request{c: &RESTClient{content: ClientContentConfig{GroupVersion: v1.SchemeGroupVersion}}}
256+
r := &Request{c: &RESTClient{}, contentConfig: ClientContentConfig{GroupVersion: v1.SchemeGroupVersion}}
256257
r.VersionedParams(&metav1.ListOptions{ResourceVersion: "1"}, scheme.ParameterCodec)
257258
if !reflect.DeepEqual(r.params, url.Values{
258259
"resourceVersion": []string{"1"},
@@ -272,7 +273,7 @@ func TestRequestVersionedParamsFromListOptions(t *testing.T) {
272273

273274
func TestRequestVersionedParamsWithInvalidScheme(t *testing.T) {
274275
parameterCodec := runtime.NewParameterCodec(runtime.NewScheme())
275-
r := (&Request{c: &RESTClient{content: ClientContentConfig{GroupVersion: v1.SchemeGroupVersion}}})
276+
r := &Request{c: &RESTClient{}, contentConfig: ClientContentConfig{GroupVersion: v1.SchemeGroupVersion}}
276277
r.VersionedParams(&v1.PodExecOptions{Stdin: false, Stdout: true},
277278
parameterCodec)
278279

@@ -336,7 +337,7 @@ func TestRequestBody(t *testing.T) {
336337
}
337338

338339
// test unencodable api object
339-
r = (&Request{c: &RESTClient{content: defaultContentConfig()}}).Body(&NotAnAPIObject{})
340+
r = (&Request{c: &RESTClient{}, contentConfig: defaultContentConfig()}).Body(&NotAnAPIObject{})
340341
if r.err == nil || r.body != nil {
341342
t.Errorf("should have set err and left body nil: %#v", r)
342343
}
@@ -901,11 +902,10 @@ func TestTransformUnstructuredError(t *testing.T) {
901902
t.Run("", func(t *testing.T) {
902903
_, ctx := ktesting.NewTestContext(t)
903904
r := &Request{
904-
c: &RESTClient{
905-
content: defaultContentConfig(),
906-
},
907-
resourceName: testCase.Name,
908-
resource: testCase.Resource,
905+
contentConfig: defaultContentConfig(),
906+
c: &RESTClient{},
907+
resourceName: testCase.Name,
908+
resource: testCase.Resource,
909909
}
910910
result := r.transformResponse(ctx, testCase.Res, testCase.Req)
911911
err := result.err
@@ -989,9 +989,9 @@ func TestRequestWatch(t *testing.T) {
989989
{
990990
name: "server returns forbidden with json content",
991991
Request: &Request{
992+
contentConfig: defaultContentConfig(),
992993
c: &RESTClient{
993-
content: defaultContentConfig(),
994-
base: &url.URL{},
994+
base: &url.URL{},
995995
},
996996
},
997997
serverReturns: []responseErr{
@@ -1018,9 +1018,9 @@ func TestRequestWatch(t *testing.T) {
10181018
{
10191019
name: "server returns forbidden without content",
10201020
Request: &Request{
1021+
contentConfig: defaultContentConfig(),
10211022
c: &RESTClient{
1022-
content: defaultContentConfig(),
1023-
base: &url.URL{},
1023+
base: &url.URL{},
10241024
},
10251025
},
10261026
serverReturns: []responseErr{
@@ -1038,9 +1038,9 @@ func TestRequestWatch(t *testing.T) {
10381038
{
10391039
name: "server returns unauthorized",
10401040
Request: &Request{
1041+
contentConfig: defaultContentConfig(),
10411042
c: &RESTClient{
1042-
content: defaultContentConfig(),
1043-
base: &url.URL{},
1043+
base: &url.URL{},
10441044
},
10451045
},
10461046
serverReturns: []responseErr{
@@ -1058,9 +1058,9 @@ func TestRequestWatch(t *testing.T) {
10581058
{
10591059
name: "server returns unauthorized",
10601060
Request: &Request{
1061+
contentConfig: defaultContentConfig(),
10611062
c: &RESTClient{
1062-
content: defaultContentConfig(),
1063-
base: &url.URL{},
1063+
base: &url.URL{},
10641064
},
10651065
},
10661066
serverReturns: []responseErr{
@@ -1254,9 +1254,9 @@ func TestRequestStream(t *testing.T) {
12541254
},
12551255
{
12561256
Request: &Request{
1257+
contentConfig: defaultContentConfig(),
12571258
c: &RESTClient{
1258-
content: defaultContentConfig(),
1259-
base: &url.URL{},
1259+
base: &url.URL{},
12601260
},
12611261
},
12621262
serverReturns: []responseErr{
@@ -1273,9 +1273,9 @@ func TestRequestStream(t *testing.T) {
12731273
},
12741274
{
12751275
Request: &Request{
1276+
contentConfig: defaultContentConfig(),
12761277
c: &RESTClient{
1277-
content: defaultContentConfig(),
1278-
base: &url.URL{},
1278+
base: &url.URL{},
12791279
},
12801280
},
12811281
serverReturns: []responseErr{
@@ -2957,12 +2957,12 @@ func testRequestWithRetry(t *testing.T, key string, doFunc func(ctx context.Cont
29572957
})
29582958

29592959
req := &Request{
2960-
verb: test.verb,
2961-
body: test.body,
2962-
bodyBytes: test.bodyBytes,
2960+
verb: test.verb,
2961+
body: test.body,
2962+
bodyBytes: test.bodyBytes,
2963+
contentConfig: defaultContentConfig(),
29632964
c: &RESTClient{
2964-
content: defaultContentConfig(),
2965-
Client: client,
2965+
Client: client,
29662966
},
29672967
backoff: &noSleepBackOff{},
29682968
maxRetries: test.maxRetries,
@@ -3251,11 +3251,11 @@ func testRetryWithRateLimiterBackoffAndMetrics(t *testing.T, key string, doFunc
32513251
t.Fatalf("Wrong test setup - did not find expected for: %s", key)
32523252
}
32533253
req := &Request{
3254-
verb: "GET",
3255-
bodyBytes: []byte{},
3254+
verb: "GET",
3255+
bodyBytes: []byte{},
3256+
contentConfig: defaultContentConfig(),
32563257
c: &RESTClient{
32573258
base: base,
3258-
content: defaultContentConfig(),
32593259
Client: client,
32603260
rateLimiter: interceptor,
32613261
},
@@ -3387,12 +3387,12 @@ func testWithRetryInvokeOrder(t *testing.T, key string, doFunc func(ctx context.
33873387
t.Fatalf("Wrong test setup - did not find expected for: %s", key)
33883388
}
33893389
req := &Request{
3390-
verb: "GET",
3391-
bodyBytes: []byte{},
3390+
verb: "GET",
3391+
bodyBytes: []byte{},
3392+
contentConfig: defaultContentConfig(),
33923393
c: &RESTClient{
3393-
base: base,
3394-
content: defaultContentConfig(),
3395-
Client: client,
3394+
base: base,
3395+
Client: client,
33963396
},
33973397
pathPrefix: "/api/v1",
33983398
rateLimiter: flowcontrol.NewFakeAlwaysRateLimiter(),
@@ -3562,12 +3562,12 @@ func testWithWrapPreviousError(t *testing.T, doFunc func(ctx context.Context, r
35623562
t.Fatalf("Failed to create new HTTP request - %v", err)
35633563
}
35643564
req := &Request{
3565-
verb: "GET",
3566-
bodyBytes: []byte{},
3565+
verb: "GET",
3566+
bodyBytes: []byte{},
3567+
contentConfig: defaultContentConfig(),
35673568
c: &RESTClient{
3568-
base: base,
3569-
content: defaultContentConfig(),
3570-
Client: client,
3569+
base: base,
3570+
Client: client,
35713571
},
35723572
pathPrefix: "/api/v1",
35733573
rateLimiter: flowcontrol.NewFakeAlwaysRateLimiter(),
@@ -3986,11 +3986,11 @@ func TestRetryableConditions(t *testing.T) {
39863986

39873987
u, _ := url.Parse("http://localhost:123" + "/apis")
39883988
req := &Request{
3989-
verb: verb,
3989+
verb: verb,
3990+
contentConfig: defaultContentConfig(),
39903991
c: &RESTClient{
3991-
base: u,
3992-
content: defaultContentConfig(),
3993-
Client: client,
3992+
base: u,
3993+
Client: client,
39943994
},
39953995
backoff: &noSleepBackOff{},
39963996
maxRetries: 2,
@@ -4030,10 +4030,10 @@ func TestRequestConcurrencyWithRetry(t *testing.T) {
40304030
})
40314031

40324032
req := &Request{
4033-
verb: "POST",
4033+
verb: "POST",
4034+
contentConfig: defaultContentConfig(),
40344035
c: &RESTClient{
4035-
content: defaultContentConfig(),
4036-
Client: client,
4036+
Client: client,
40374037
},
40384038
backoff: &noSleepBackOff{},
40394039
maxRetries: 9, // 10 attempts in total, including the first

‎staging/src/k8s.io/client-go/rest/request_watchlist_test.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,7 @@ func TestWatchListSuccess(t *testing.T) {
200200
ctx := context.Background()
201201
fakeWatcher := watch.NewFake()
202202
target := &Request{
203-
c: &RESTClient{
204-
content: ClientContentConfig{},
205-
},
203+
c: &RESTClient{},
206204
}
207205

208206
go func(watchEvents []watch.Event) {

0 commit comments

Comments
 (0)