diff --git a/docs/api-conventions.md b/docs/api-conventions.md index b829f6c2e7..7a8d862fab 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -232,6 +232,8 @@ TODO: Plugins, extensions, nested kinds, headers HTTP Status codes ----------------- +The server will respond with HTTP status codes that match the HTTP spec. See the section below for a breakdown of the types of status codes the server will send. + The following HTTP status codes may be returned by the API. #### Success codes @@ -253,6 +255,11 @@ The following HTTP status codes may be returned by the API. * Indicates the requested is invalid. * Suggested client recovery behavior: * Do not retry. Fix the request. +* `401 StatusUnauthorized` + * Indicates that the server can be reached and understood the request, but refuses to take any further action, because the client must provide authorization. If the client has provided authorization, the server is indicating the provided authorization is unsuitable or invalid. + * Suggested client recovery behavior + * If the user has not supplied authorization information, prompt them for the appropriate credentials + * If the user has supplied authorization information, inform them their credentials were rejected and optionally prompt them again. * `403 StatusForbidden` * Indicates that the server can be reached and understood the request, but refuses to take any further action, because it is configured to deny access for some reason to the requested resource by the client. * Suggested client recovery behavior @@ -295,7 +302,7 @@ The following HTTP status codes may be returned by the API. * Increase the value of the timeout param and retry with exponential backoff Response Status Kind ---------------------- +-------------------- Kubernetes will always return the ```Status``` kind from any API endpoint when an error occurs. Clients SHOULD handle these types of objects when appropriate. @@ -304,7 +311,7 @@ A ```Status``` kind will be returned by the API in two cases: * When an operation is not successful (i.e. when the server would return a non 2xx HTTP status code). * When a HTTP ```DELETE``` call is successful. -The status object is encoded as JSON and provided as the body of the response. The status object contains fields for humans and machine consumers of the API to get more detailed information for the cause of the failure. The information in the status object supplements, but does not override, the HTTP status code's meaning. +The status object is encoded as JSON and provided as the body of the response. The status object contains fields for humans and machine consumers of the API to get more detailed information for the cause of the failure. The information in the status object supplements, but does not override, the HTTP status code's meaning. When fields in the status object have the same meaning as generally defined HTTP headers and that header is returned with the response, the header should be considered as having higher priority. **Example:** ```JSON @@ -341,6 +348,19 @@ Content-Length: 144 ```details``` may contain extended data associated with the reason. Each reason may define its own extended details. This field is optional and the data returned is not guaranteed to conform to any schema except that defined by the reason type. Possible values for the ```reason``` and ```details``` fields: +* `BadRequest` + * Indicates that the request itself was invalid, because the request doesn't make any sense, for example deleting a read-only object. + * This is different than `status reason` `Invalid` above which indicates that the API call could possibly succeed, but the data was invalid. + * API calls that return BadRequest can never succeed. + * Http status code: `400 StatusBadRequest` +* `Unauthorized` + * Indicates that the server can be reached and understood the request, but refuses to take any further action without the client providing appropriate authorization. If the client has provided authorization, this error indicates the provided credentials are insufficient or invalid. + * Details (optional): + * `kind string` + * The kind attribute of the unauthorized resource (on some operations may differ from the requested resource). + * `id string` + * The identifier of the unauthorized resource. + * HTTP status code: `401 StatusUnauthorized` * `Forbidden` * Indicates that the server can be reached and understood the request, but refuses to take any further action, because it is configured to deny access for some reason to the requested resource by the client. * Details (optional): @@ -378,6 +398,10 @@ Possible values for the ```reason``` and ```details``` fields: * `causes` * One or more `StatusCause` entries indicating the data in the provided resource that was invalid. The `reason`, `message`, and `field` attributes will be set. * HTTP status code: `422 StatusUnprocessableEntity` +* `Timeout` + * Indicates that the request could not be completed within the given time. Clients may receive this response if the server has decided to rate limit the client, or if the server is overloaded and cannot process the request at this time. + * Http status code: `429 TooManyRequests` + * The server should set the `Retry-After` HTTP header and return `retryAfterSeconds` in the details field of the object. A value of `0` is the default. * `ServerTimeout` * Indicates that the server can be reached and understood the request, but cannot complete the action in a reasonable time. This maybe due to temporary server load or a transient communication issue with another server. * Details (optional): @@ -385,15 +409,8 @@ Possible values for the ```reason``` and ```details``` fields: * The kind attribute of the resource being acted on. * `id string` * The operation that is being attempted. - * Http status code: `500 StatusInternalServerError` -* `Timeout` - * Indicates that the request could not be completed within the given time. Clients can get this response ONLY when they specified a timeout param in the request. The request might succeed with an increased value of timeout param. + * The server should set the `Retry-After` HTTP header and return `retryAfterSeconds` in the details field of the object. A value of `0` is the default. * Http status code: `504 StatusServerTimeout` -* `BadRequest` - * Indicates that the request itself was invalid, because the request doesn't make any sense, for example deleting a read-only object. - * This is different than `status reason` `Invalid` above which indicates that the API call could possibly succeed, but the data was invalid. - * API calls that return BadRequest can never succeed. - * Http status code: `400 StatusBadRequest` * `MethodNotAllowed` * Indicates that that the action the client attempted to perform on the resource was not supported by the code. * For instance, attempting to delete a resource that can only be created. diff --git a/pkg/api/errors/errors.go b/pkg/api/errors/errors.go index f0c3ed9c89..5ebabfc33a 100644 --- a/pkg/api/errors/errors.go +++ b/pkg/api/errors/errors.go @@ -105,6 +105,21 @@ func NewAlreadyExists(kind, name string) error { }} } +// NewUnauthorized returns an error indicating the client is not authorized to perform the requested +// action. +func NewUnauthorized(reason string) error { + message := reason + if len(message) == 0 { + message = "not authorized" + } + return &StatusError{api.Status{ + Status: api.StatusFailure, + Code: http.StatusUnauthorized, + Reason: api.StatusReasonUnauthorized, + Message: message, + }} +} + // NewForbidden returns an error indicating the requested action was forbidden func NewForbidden(kind, name string, err error) error { return &StatusError{api.Status{ @@ -183,14 +198,15 @@ func NewMethodNotSupported(kind, action string) error { // NewServerTimeout returns an error indicating the requested action could not be completed due to a // transient error, and the client should try again. -func NewServerTimeout(kind, operation string) error { +func NewServerTimeout(kind, operation string, retryAfterSeconds int) error { return &StatusError{api.Status{ Status: api.StatusFailure, Code: http.StatusInternalServerError, Reason: api.StatusReasonServerTimeout, Details: &api.StatusDetails{ - Kind: kind, - ID: operation, + Kind: kind, + ID: operation, + RetryAfterSeconds: retryAfterSeconds, }, Message: fmt.Sprintf("The %s operation against %s could not be completed at this time, please try again.", operation, kind), }} @@ -211,12 +227,15 @@ func NewInternalError(err error) error { // NewTimeoutError returns an error indicating that a timeout occurred before the request // could be completed. Clients may retry, but the operation may still complete. -func NewTimeoutError(message string) error { +func NewTimeoutError(message string, retryAfterSeconds int) error { return &StatusError{api.Status{ Status: api.StatusFailure, Code: StatusServerTimeout, Reason: api.StatusReasonTimeout, Message: fmt.Sprintf("Timeout: %s", message), + Details: &api.StatusDetails{ + RetryAfterSeconds: retryAfterSeconds, + }, }} } @@ -251,6 +270,12 @@ func IsBadRequest(err error) bool { return reasonForError(err) == api.StatusReasonBadRequest } +// IsUnauthorized determines if err is an error which indicates that the request is unauthorized and +// requires authentication by the user. +func IsUnauthorized(err error) bool { + return reasonForError(err) == api.StatusReasonUnauthorized +} + // IsForbidden determines if err is an error which indicates that the request is forbidden and cannot // be completed as requested. func IsForbidden(err error) bool { @@ -275,6 +300,21 @@ func IsUnexpectedObjectError(err error) bool { return ok } +// SuggestsClientDelay returns true if this error suggests a client delay as well as the +// suggested seconds to wait, or false if the error does not imply a wait. +func SuggestsClientDelay(err error) (int, bool) { + switch t := err.(type) { + case *StatusError: + if t.Status().Details != nil { + switch t.Status().Reason { + case api.StatusReasonServerTimeout, api.StatusReasonTimeout: + return t.Status().Details.RetryAfterSeconds, true + } + } + } + return 0, false +} + func reasonForError(err error) api.StatusReason { switch t := err.(type) { case *StatusError: diff --git a/pkg/api/errors/errors_test.go b/pkg/api/errors/errors_test.go index f73f1c99ad..1c257a51f0 100644 --- a/pkg/api/errors/errors_test.go +++ b/pkg/api/errors/errors_test.go @@ -69,9 +69,18 @@ func TestErrorNew(t *testing.T) { if !IsForbidden(NewForbidden("test", "2", errors.New("reason"))) { t.Errorf("expected to be %s", api.StatusReasonForbidden) } - if !IsServerTimeout(NewServerTimeout("test", "reason")) { + if !IsUnauthorized(NewUnauthorized("reason")) { + t.Errorf("expected to be %s", api.StatusReasonUnauthorized) + } + if !IsServerTimeout(NewServerTimeout("test", "reason", 0)) { t.Errorf("expected to be %s", api.StatusReasonServerTimeout) } + if time, ok := SuggestsClientDelay(NewServerTimeout("test", "doing something", 10)); time != 10 || !ok { + t.Errorf("expected to be %s", api.StatusReasonServerTimeout) + } + if time, ok := SuggestsClientDelay(NewTimeoutError("test reason", 10)); time != 10 || !ok { + t.Errorf("expected to be %s", api.StatusReasonTimeout) + } if !IsMethodNotSupported(NewMethodNotSupported("foo", "delete")) { t.Errorf("expected to be %s", api.StatusReasonMethodNotAllowed) } diff --git a/pkg/api/rest/create.go b/pkg/api/rest/create.go index 9089554a9d..fad73a4958 100644 --- a/pkg/api/rest/create.go +++ b/pkg/api/rest/create.go @@ -85,7 +85,7 @@ func CheckGeneratedNameError(strategy RESTCreateStrategy, err error, obj runtime return err } - return errors.NewServerTimeout(kind, "POST") + return errors.NewServerTimeout(kind, "POST", 0) } // objectMetaAndKind retrieves kind and ObjectMeta from a runtime object, or returns an error. diff --git a/pkg/api/types.go b/pkg/api/types.go index ead5838221..fc802f1002 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1243,6 +1243,8 @@ type StatusDetails struct { // The Causes array includes more details associated with the StatusReason // failure. Not all StatusReasons may provide detailed causes. Causes []StatusCause `json:"causes,omitempty"` + // If specified, the time in seconds before the operation should be retried. + RetryAfterSeconds int `json:"retryAfterSeconds,omitempty"` } // Values of Status.Status @@ -1263,6 +1265,13 @@ const ( // Status code 500. StatusReasonUnknown StatusReason = "" + // StatusReasonUnauthorized means the server can be reached and understood the request, but requires + // the user to present appropriate authorization credentials (identified by the WWW-Authenticate header) + // in order for the action to be completed. If the user has specified credentials on the request, the + // server considers them insufficient. + // Status code 401 + StatusReasonUnauthorized StatusReason = "Unauthorized" + // StatusReasonForbidden means the server can be reached and understood the request, but refuses // to take any further action. It is the result of the server being configured to deny access for some reason // to the requested resource by the client. @@ -1319,12 +1328,17 @@ const ( // Details (optional): // "kind" string - the kind attribute of the resource being acted on. // "id" string - the operation that is being attempted. + // "retryAfterSeconds" int - the number of seconds before the operation should be retried // Status code 500 StatusReasonServerTimeout StatusReason = "ServerTimeout" // StatusReasonTimeout means that the request could not be completed within the given time. - // Clients can get this response only when they specified a timeout param in the request. - // The request might succeed with an increased value of timeout param. + // Clients can get this response only when they specified a timeout param in the request, + // or if the server cannot complete the operation within a reasonable amount of time. + // The request might succeed with an increased value of timeout param. The client *should* + // wait at least the number of seconds specified by the retryAfterSeconds field. + // Details (optional): + // "retryAfterSeconds" int - the number of seconds before the operation should be retried // Status code 504 StatusReasonTimeout StatusReason = "Timeout" diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 21ddbebcf8..7de5e55489 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -1073,6 +1073,8 @@ type StatusDetails struct { // The Causes array includes more details associated with the StatusReason // failure. Not all StatusReasons may provide detailed causes. Causes []StatusCause `json:"causes,omitempty" description:"the Causes array includes more details associated with the StatusReason failure; not all StatusReasons may provide detailed causes"` + // If specified, the time in seconds before the operation should be retried. + RetryAfterSeconds int `json:"retryAfterSeconds,omitempty" description:"the number of seconds before the client should attempt to retry this operation"` } // Values of Status.Status diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index b9855c5f52..e3d8a15209 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -1087,6 +1087,8 @@ type StatusDetails struct { // The Causes array includes more details associated with the StatusReason // failure. Not all StatusReasons may provide detailed causes. Causes []StatusCause `json:"causes,omitempty" description:"the Causes array includes more details associated with the StatusReason failure; not all StatusReasons may provide detailed causes"` + // If specified, the time in seconds before the operation should be retried. + RetryAfterSeconds int `json:"retryAfterSeconds,omitempty" description:"the number of seconds before the client should attempt to retry this operation"` } // Values of Status.Status diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index 4aa59a49c6..f97bad0f5e 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -1229,6 +1229,8 @@ type StatusDetails struct { // The Causes array includes more details associated with the StatusReason // failure. Not all StatusReasons may provide detailed causes. Causes []StatusCause `json:"causes,omitempty" description:"the Causes array includes more details associated with the StatusReason failure; not all StatusReasons may provide detailed causes"` + // If specified, the time in seconds before the operation should be retried. + RetryAfterSeconds int `json:"retryAfterSeconds,omitempty" description:"the number of seconds before the client should attempt to retry this operation"` } // Values of Status.Status diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index df363a6fba..25ae6c98a0 100644 --- a/pkg/apiserver/resthandler.go +++ b/pkg/apiserver/resthandler.go @@ -436,7 +436,7 @@ func finishRequest(timeout time.Duration, fn resultFunc) (result runtime.Object, case err = <-errCh: return nil, err case <-time.After(timeout): - return nil, errors.NewTimeoutError("request did not complete within allowed duration") + return nil, errors.NewTimeoutError("request did not complete within allowed duration", 0) } } diff --git a/pkg/client/request.go b/pkg/client/request.go index 58a1a8477e..adec12529c 100644 --- a/pkg/client/request.go +++ b/pkg/client/request.go @@ -22,6 +22,7 @@ import ( "fmt" "io" "io/ioutil" + "mime" "net/http" "net/url" "path" @@ -448,11 +449,10 @@ func (r *Request) Watch() (watch.Interface, error) { return nil, err } if resp.StatusCode != http.StatusOK { - var body []byte - if resp.Body != nil { - body, _ = ioutil.ReadAll(resp.Body) + if _, _, err := r.transformResponse(resp, req, nil); err != nil { + return nil, err } - return nil, fmt.Errorf("for request '%+v', got status: %v\nbody: %v", req.URL, resp.StatusCode, string(body)) + return nil, fmt.Errorf("for request '%+v', got status: %v", req.URL, resp.StatusCode) } return watch.NewStreamWatcher(watchjson.NewDecoder(resp.Body, r.codec)), nil } @@ -603,12 +603,18 @@ func (r *Request) Do() Result { if err != nil { return Result{err: err} } - respBody, created, err := r.transformResponse(body, r.resp, r.req) + respBody, created, err := r.transformResponse(r.resp, r.req, body) return Result{respBody, created, err, r.codec} } -// transformResponse converts an API response into a structured API object. -func (r *Request) transformResponse(body []byte, resp *http.Response, req *http.Request) ([]byte, bool, error) { +// transformResponse converts an API response into a structured API object. If body is nil, the response +// body will be read to try and gather more response data. +func (r *Request) transformResponse(resp *http.Response, req *http.Request, body []byte) ([]byte, bool, error) { + if body == nil && resp.Body != nil { + if data, err := ioutil.ReadAll(resp.Body); err == nil { + body = data + } + } // Did the server give us a status response? isStatusResponse := false var status api.Status @@ -621,26 +627,7 @@ func (r *Request) transformResponse(body []byte, resp *http.Response, req *http. // no-op, we've been upgraded case resp.StatusCode < http.StatusOK || resp.StatusCode > http.StatusPartialContent: if !isStatusResponse { - var err error - err = &UnexpectedStatusError{ - Request: req, - Response: resp, - Body: string(body), - } - // TODO: handle other error classes we know about - switch resp.StatusCode { - case http.StatusConflict: - if req.Method == "POST" { - err = errors.NewAlreadyExists(r.resource, r.resourceName) - } else { - err = errors.NewConflict(r.resource, r.resourceName, err) - } - case http.StatusNotFound: - err = errors.NewNotFound(r.resource, r.resourceName) - case http.StatusBadRequest: - err = errors.NewBadRequest(err.Error()) - } - return nil, false, err + return nil, false, r.transformUnstructuredResponseError(resp, req, body) } return nil, false, errors.FromObject(&status) } @@ -657,6 +644,94 @@ func (r *Request) transformResponse(body []byte, resp *http.Response, req *http. return body, created, nil } +// transformUnstructuredResponseError handles an error from the server that is not in a structured form. +// It is expected to transform any response that is not recognizable as a clear server sent error from the +// K8S API using the information provided with the request. In practice, HTTP proxies and client libraries +// introduce a level of uncertainty to the responses returned by servers that in common use result in +// unexpected responses. The rough structure is: +// +// 1. Assume the server sends you something sane - JSON + well defined error objects + proper codes +// - this is the happy path +// - when you get this output, trust what the server sends +// 2. Guard against empty fields / bodies in received JSON and attempt to cull sufficient info from them to +// generate a reasonable facsimile of the original failure. +// - Be sure to use a distinct error type or flag that allows a client to distinguish between this and error 1 above +// 3. Handle true disconnect failures / completely malformed data by moving up to a more generic client error +// 4. Distinguish between various connection failures like SSL certificates, timeouts, proxy errors, unexpected +// initial contact, the presence of mismatched body contents from posted content types +// - Give these a separate distinct error type and capture as much as possible of the original message +// +// TODO: introduce further levels of refinement that allow a client to distinguish between 1 and 2-3. +// TODO: introduce transformation of generic http.Client.Do() errors that separates 4. +func (r *Request) transformUnstructuredResponseError(resp *http.Response, req *http.Request, body []byte) error { + if body == nil && resp.Body != nil { + if data, err := ioutil.ReadAll(resp.Body); err == nil { + body = data + } + } + var err error = &UnexpectedStatusError{ + Request: req, + Response: resp, + Body: string(body), + } + message := "unknown" + if isTextResponse(resp) { + message = strings.TrimSpace(string(body)) + } + // TODO: handle other error classes we know about + switch resp.StatusCode { + case http.StatusConflict: + if req.Method == "POST" { + err = errors.NewAlreadyExists(r.resource, r.resourceName) + } else { + err = errors.NewConflict(r.resource, r.resourceName, err) + } + case http.StatusNotFound: + err = errors.NewNotFound(r.resource, r.resourceName) + case http.StatusBadRequest: + err = errors.NewBadRequest(message) + case http.StatusUnauthorized: + err = errors.NewUnauthorized(message) + case http.StatusForbidden: + err = errors.NewForbidden(r.resource, r.resourceName, err) + case errors.StatusUnprocessableEntity: + err = errors.NewInvalid(r.resource, r.resourceName, nil) + case errors.StatusServerTimeout: + retryAfterSeconds, _ := retryAfterSeconds(resp) + err = errors.NewServerTimeout(r.resource, r.verb, retryAfterSeconds) + case errors.StatusTooManyRequests: + retryAfterSeconds, _ := retryAfterSeconds(resp) + err = errors.NewServerTimeout(r.resource, r.verb, retryAfterSeconds) + case http.StatusInternalServerError: + err = errors.NewInternalError(fmt.Errorf(message)) + } + return err +} + +// isTextResponse returns true if the response appears to be a textual media type. +func isTextResponse(resp *http.Response) bool { + contentType := resp.Header.Get("Content-Type") + if len(contentType) == 0 { + return true + } + media, _, err := mime.ParseMediaType(contentType) + if err != nil { + return false + } + return strings.HasPrefix(media, "text/") +} + +// retryAfterSeconds returns the value of the Retry-After header and true, or 0 and false if +// the header was missing or not a valid number. +func retryAfterSeconds(resp *http.Response) (int, bool) { + if h := resp.Header.Get("Retry-After"); len(h) > 0 { + if i, err := strconv.Atoi(h); err == nil { + return i, true + } + } + return 0, false +} + // Result contains the result of calling Request.Do(). type Result struct { body []byte diff --git a/pkg/client/request_test.go b/pkg/client/request_test.go index f026702630..bafb75492f 100644 --- a/pkg/client/request_test.go +++ b/pkg/client/request_test.go @@ -222,6 +222,7 @@ func TestTransformResponse(t *testing.T) { Data []byte Created bool Error bool + ErrFn func(err error) bool }{ {Response: &http.Response{StatusCode: 200}, Data: []byte{}}, {Response: &http.Response{StatusCode: 201}, Data: []byte{}, Created: true}, @@ -230,6 +231,30 @@ func TestTransformResponse(t *testing.T) { {Response: &http.Response{StatusCode: 422}, Error: true}, {Response: &http.Response{StatusCode: 409}, Error: true}, {Response: &http.Response{StatusCode: 404}, Error: true}, + {Response: &http.Response{StatusCode: 401}, Error: true}, + { + Response: &http.Response{ + StatusCode: 401, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: ioutil.NopCloser(bytes.NewReader(invalid)), + }, + Error: true, + ErrFn: func(err error) bool { + return err.Error() != "aaaaa" && apierrors.IsUnauthorized(err) + }, + }, + { + Response: &http.Response{ + StatusCode: 401, + Header: http.Header{"Content-Type": []string{"text/any"}}, + Body: ioutil.NopCloser(bytes.NewReader(invalid)), + }, + Error: true, + ErrFn: func(err error) bool { + return err.Error() == "aaaaa" && apierrors.IsUnauthorized(err) + }, + }, + {Response: &http.Response{StatusCode: 403}, Error: true}, {Response: &http.Response{StatusCode: 200, Body: ioutil.NopCloser(bytes.NewReader(invalid))}, Data: invalid}, {Response: &http.Response{StatusCode: 200, Body: ioutil.NopCloser(bytes.NewReader(invalid))}, Data: invalid}, } @@ -242,10 +267,22 @@ func TestTransformResponse(t *testing.T) { if err != nil { t.Errorf("failed to read body of response: %v", err) } - response, created, err := r.transformResponse(body, test.Response, &http.Request{}) + response, created, err := r.transformResponse(test.Response, &http.Request{}, body) hasErr := err != nil if hasErr != test.Error { t.Errorf("%d: unexpected error: %t %v", i, test.Error, err) + } else if hasErr && test.Response.StatusCode > 399 { + status, ok := err.(APIStatus) + if !ok { + t.Errorf("%d: response should have been transformable into APIStatus: %v", i, err) + continue + } + if status.Status().Code != test.Response.StatusCode { + t.Errorf("%d: status code did not match response: %#v", i, status.Status()) + } + } + if test.ErrFn != nil && !test.ErrFn(err) { + t.Errorf("%d: error function did not match: %v", i, err) } if !(test.Data == nil && response == nil) && !api.Semantic.DeepDerivative(test.Data, response) { t.Errorf("%d: unexpected response: %#v %#v", i, test.Data, response) @@ -320,7 +357,7 @@ func TestTransformUnstructuredError(t *testing.T) { if err != nil { t.Errorf("failed to read body: %v", err) } - _, _, err = r.transformResponse(body, testCase.Res, testCase.Req) + _, _, err = r.transformResponse(testCase.Res, testCase.Req, body) if !testCase.ErrFn(err) { t.Errorf("unexpected error: %v", err) continue @@ -344,6 +381,7 @@ func TestRequestWatch(t *testing.T) { testCases := []struct { Request *Request Err bool + ErrFn func(error) bool Empty bool }{ { @@ -365,12 +403,48 @@ func TestRequestWatch(t *testing.T) { }, { Request: &Request{ + codec: testapi.Codec(), client: clientFunc(func(req *http.Request) (*http.Response, error) { return &http.Response{StatusCode: http.StatusForbidden}, nil }), baseURL: &url.URL{}, }, Err: true, + ErrFn: func(err error) bool { + return apierrors.IsForbidden(err) + }, + }, + { + Request: &Request{ + codec: testapi.Codec(), + client: clientFunc(func(req *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusUnauthorized}, nil + }), + baseURL: &url.URL{}, + }, + Err: true, + ErrFn: func(err error) bool { + return apierrors.IsUnauthorized(err) + }, + }, + { + Request: &Request{ + codec: testapi.Codec(), + client: clientFunc(func(req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusUnauthorized, + Body: ioutil.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(testapi.Codec(), &api.Status{ + Status: api.StatusFailure, + Reason: api.StatusReasonUnauthorized, + })))), + }, nil + }), + baseURL: &url.URL{}, + }, + Err: true, + ErrFn: func(err error) bool { + return apierrors.IsUnauthorized(err) + }, }, { Request: &Request{ @@ -416,6 +490,9 @@ func TestRequestWatch(t *testing.T) { t.Errorf("%d: expected %t, got %t: %v", i, testCase.Err, hasErr, err) continue } + if testCase.ErrFn != nil && !testCase.ErrFn(err) { + t.Errorf("%d: error not valid: %v", i, err) + } if hasErr && watch != nil { t.Errorf("%d: watch should be nil when error is returned", i) continue diff --git a/pkg/client/restclient_test.go b/pkg/client/restclient_test.go index e9c3ede61c..e5e1706d3d 100644 --- a/pkg/client/restclient_test.go +++ b/pkg/client/restclient_test.go @@ -225,7 +225,13 @@ func TestDoRequestSuccess(t *testing.T) { } func TestDoRequestFailed(t *testing.T) { - status := &api.Status{Status: api.StatusFailure, Reason: api.StatusReasonInvalid, Details: &api.StatusDetails{ID: "test", Kind: "test"}} + status := &api.Status{ + Code: http.StatusNotFound, + Status: api.StatusFailure, + Reason: api.StatusReasonNotFound, + Message: " \"\" not found", + Details: &api.StatusDetails{}, + } expectedBody, _ := latest.Codec.Encode(status) fakeHandler := util.FakeHandler{ StatusCode: 404, @@ -253,7 +259,7 @@ func TestDoRequestFailed(t *testing.T) { } actual := ss.Status() if !reflect.DeepEqual(status, &actual) { - t.Errorf("Unexpected mis-match. Expected %#v. Saw %#v", status, actual) + t.Errorf("Unexpected mis-match: %s", util.ObjectDiff(status, &actual)) } }