diff --git a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go index d5503fac5d9..9960600be33 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go @@ -405,84 +405,84 @@ func NewGenericServerResponse(code int, verb string, qualifiedResource schema.Gr // IsNotFound returns true if the specified error was created by NewNotFound. func IsNotFound(err error) bool { - return reasonForError(err) == metav1.StatusReasonNotFound + return ReasonForError(err) == metav1.StatusReasonNotFound } // IsAlreadyExists determines if the err is an error which indicates that a specified resource already exists. func IsAlreadyExists(err error) bool { - return reasonForError(err) == metav1.StatusReasonAlreadyExists + return ReasonForError(err) == metav1.StatusReasonAlreadyExists } // IsConflict determines if the err is an error which indicates the provided update conflicts. func IsConflict(err error) bool { - return reasonForError(err) == metav1.StatusReasonConflict + return ReasonForError(err) == metav1.StatusReasonConflict } // IsInvalid determines if the err is an error which indicates the provided resource is not valid. func IsInvalid(err error) bool { - return reasonForError(err) == metav1.StatusReasonInvalid + return ReasonForError(err) == metav1.StatusReasonInvalid } // IsGone is true if the error indicates the requested resource is no longer available. func IsGone(err error) bool { - return reasonForError(err) == metav1.StatusReasonGone + return ReasonForError(err) == metav1.StatusReasonGone } // IsResourceExpired is true if the error indicates the resource has expired and the current action is // no longer possible. func IsResourceExpired(err error) bool { - return reasonForError(err) == metav1.StatusReasonExpired + return ReasonForError(err) == metav1.StatusReasonExpired } // IsMethodNotSupported determines if the err is an error which indicates the provided action could not // be performed because it is not supported by the server. func IsMethodNotSupported(err error) bool { - return reasonForError(err) == metav1.StatusReasonMethodNotAllowed + return ReasonForError(err) == metav1.StatusReasonMethodNotAllowed } // IsServiceUnavailable is true if the error indicates the underlying service is no longer available. func IsServiceUnavailable(err error) bool { - return reasonForError(err) == metav1.StatusReasonServiceUnavailable + return ReasonForError(err) == metav1.StatusReasonServiceUnavailable } // IsBadRequest determines if err is an error which indicates that the request is invalid. func IsBadRequest(err error) bool { - return reasonForError(err) == metav1.StatusReasonBadRequest + return ReasonForError(err) == metav1.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) == metav1.StatusReasonUnauthorized + return ReasonForError(err) == metav1.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 { - return reasonForError(err) == metav1.StatusReasonForbidden + return ReasonForError(err) == metav1.StatusReasonForbidden } // IsTimeout determines if err is an error which indicates that request times out due to long // processing. func IsTimeout(err error) bool { - return reasonForError(err) == metav1.StatusReasonTimeout + return ReasonForError(err) == metav1.StatusReasonTimeout } // IsServerTimeout determines if err is an error which indicates that the request needs to be retried // by the client. func IsServerTimeout(err error) bool { - return reasonForError(err) == metav1.StatusReasonServerTimeout + return ReasonForError(err) == metav1.StatusReasonServerTimeout } // IsInternalError determines if err is an error which indicates an internal server error. func IsInternalError(err error) bool { - return reasonForError(err) == metav1.StatusReasonInternalError + return ReasonForError(err) == metav1.StatusReasonInternalError } // IsTooManyRequests determines if err is an error which indicates that there are too many requests // that the server cannot handle. func IsTooManyRequests(err error) bool { - if reasonForError(err) == metav1.StatusReasonTooManyRequests { + if ReasonForError(err) == metav1.StatusReasonTooManyRequests { return true } switch t := err.(type) { @@ -536,7 +536,8 @@ func SuggestsClientDelay(err error) (int, bool) { return 0, false } -func reasonForError(err error) metav1.StatusReason { +// ReasonForError returns the HTTP status for a particular error. +func ReasonForError(err error) metav1.StatusReason { switch t := err.(type) { case APIStatus: return t.Status().Reason diff --git a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go index afc26fd31c4..303a9d3f48f 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go @@ -188,8 +188,8 @@ func TestNewInvalid(t *testing.T) { } } -func Test_reasonForError(t *testing.T) { - if e, a := metav1.StatusReasonUnknown, reasonForError(nil); e != a { +func TestReasonForError(t *testing.T) { + if e, a := metav1.StatusReasonUnknown, ReasonForError(nil); e != a { t.Errorf("unexpected reason type: %#v", a) } } diff --git a/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go b/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go index 01014f9d36a..11588abdd58 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go @@ -1552,7 +1552,7 @@ func sortMergeListsByName(mapJSON []byte, dataStruct interface{}) ([]byte, error var m map[string]interface{} err := json.Unmarshal(mapJSON, &m) if err != nil { - return nil, err + return nil, mergepatch.ErrBadJSONDoc } newM, err := sortMergeListsByNameMap(m, reflect.TypeOf(dataStruct)) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index b5d2192e30e..588cd7917d5 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -186,7 +186,7 @@ func patchResource( case types.JSONPatchType, types.MergePatchType: originalJS, patchedJS, err := patchObjectJSON(patchType, codec, currentObject, patchJS, objToUpdate, versionedObj) if err != nil { - return nil, err + return nil, interpretPatchError(err) } originalObjJS, originalPatchedObjJS = originalJS, patchedJS @@ -198,13 +198,13 @@ func patchResource( // Compute once originalPatchBytes, err = strategicpatch.CreateTwoWayMergePatch(originalObjJS, originalPatchedObjJS, versionedObj) if err != nil { - return nil, err + return nil, interpretPatchError(err) } } // Return a fresh map every time originalPatchMap := make(map[string]interface{}) if err := json.Unmarshal(originalPatchBytes, &originalPatchMap); err != nil { - return nil, err + return nil, errors.NewBadRequest(err.Error()) } return originalPatchMap, nil } @@ -242,7 +242,7 @@ func patchResource( getOriginalPatchMap = func() (map[string]interface{}, error) { patchMap := make(map[string]interface{}) if err := json.Unmarshal(patchJS, &patchMap); err != nil { - return nil, err + return nil, errors.NewBadRequest(err.Error()) } return patchMap, nil } @@ -277,7 +277,7 @@ func patchResource( var err error currentPatchMap, err = strategicpatch.CreateTwoWayMergeMapPatch(originalObjMap, currentObjMap, versionedObj) if err != nil { - return nil, err + return nil, interpretPatchError(err) } } else { // Compute current patch. @@ -287,11 +287,11 @@ func patchResource( } currentPatch, err := strategicpatch.CreateTwoWayMergePatch(originalObjJS, currentObjJS, versionedObj) if err != nil { - return nil, err + return nil, interpretPatchError(err) } currentPatchMap = make(map[string]interface{}) if err := json.Unmarshal(currentPatch, ¤tPatchMap); err != nil { - return nil, err + return nil, errors.NewBadRequest(err.Error()) } } @@ -422,7 +422,7 @@ func strategicPatchObject( patchMap := make(map[string]interface{}) if err := json.Unmarshal(patchJS, &patchMap); err != nil { - return err + return errors.NewBadRequest(err.Error()) } if err := applyPatchToObject(codec, defaulter, originalObjMap, patchMap, objToUpdate, versionedObj); err != nil { @@ -456,3 +456,15 @@ func applyPatchToObject( return nil } + +// interpretPatchError interprets the error type and returns an error with appropriate HTTP code. +func interpretPatchError(err error) error { + switch err { + case mergepatch.ErrBadJSONDoc, mergepatch.ErrBadPatchFormatForPrimitiveList, mergepatch.ErrBadPatchFormatForRetainKeys, mergepatch.ErrBadPatchFormatForSetElementOrderList: + return errors.NewBadRequest(err.Error()) + case mergepatch.ErrNoListOfLists, mergepatch.ErrPatchContentNotMatchRetainKeys: + return errors.NewGenericServerResponse(http.StatusUnprocessableEntity, "", schema.GroupResource{}, "", err.Error(), 0, false) + default: + return err + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go index a99fdaf6fed..5e6ca117a27 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go @@ -98,6 +98,29 @@ func TestPatchAnonymousField(t *testing.T) { } } +func TestPatchInvalid(t *testing.T) { + testGV := schema.GroupVersion{Group: "", Version: "v"} + scheme.AddKnownTypes(testGV, &testPatchType{}) + codec := codecs.LegacyCodec(testGV) + defaulter := runtime.ObjectDefaulter(scheme) + + original := &testPatchType{ + TypeMeta: metav1.TypeMeta{Kind: "testPatchType", APIVersion: "v"}, + TestPatchSubType: TestPatchSubType{StringField: "my-value"}, + } + patch := `barbaz` + expectedError := "invalid character 'b' looking for beginning of value" + + actual := &testPatchType{} + err := strategicPatchObject(codec, defaulter, original, []byte(patch), actual, &testPatchType{}) + if apierrors.IsBadRequest(err) == false { + t.Errorf("expected HTTP status: BadRequest, got: %#v", apierrors.ReasonForError(err)) + } + if err.Error() != expectedError { + t.Errorf("expected %#v, got %#v", expectedError, err.Error()) + } +} + type testPatcher struct { t *testing.T