diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD index 87846313d46..64d3b14d241 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD @@ -76,6 +76,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/strategicpatch:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/audit:go_default_library", 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 c33f181661a..7988b0a2fa4 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -36,6 +36,7 @@ import ( "k8s.io/apimachinery/pkg/util/mergepatch" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/strategicpatch" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/authorization/authorizer" @@ -318,7 +319,9 @@ func (p *jsonPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (r // Construct the resulting typed, unversioned object. objToUpdate := p.restPatcher.New() if err := runtime.DecodeInto(p.codec, patchedObjJS, objToUpdate); err != nil { - return nil, err + return nil, errors.NewInvalid(schema.GroupKind{}, "", field.ErrorList{ + field.Invalid(field.NewPath("patch"), string(patchedObjJS), err.Error()), + }) } if p.fieldManager != nil { @@ -583,7 +586,9 @@ func applyPatchToObject( // Rather than serialize the patched map to JSON, then decode it to an object, we go directly from a map to an object if err := runtime.DefaultUnstructuredConverter.FromUnstructured(patchedObjMap, objToUpdate); err != nil { - return err + return errors.NewInvalid(schema.GroupKind{}, "", field.ErrorList{ + field.Invalid(field.NewPath("patch"), fmt.Sprintf("%+v", patchMap), err.Error()), + }) } // Decoding from JSON to a versioned object would apply defaults, so we do the same here defaulter.Default(objToUpdate) diff --git a/test/integration/apiserver/apiserver_test.go b/test/integration/apiserver/apiserver_test.go index 212209b9303..e61c699dda1 100644 --- a/test/integration/apiserver/apiserver_test.go +++ b/test/integration/apiserver/apiserver_test.go @@ -148,6 +148,70 @@ var cascDel = ` } ` +func Test4xxStatusCodeInvalidPatch(t *testing.T) { + _, client, closeFn := setup(t) + defer closeFn() + + obj := []byte(`{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "name": "deployment", + "labels": {"app": "nginx"} + }, + "spec": { + "selector": { + "matchLabels": { + "app": "nginx" + } + }, + "template": { + "metadata": { + "labels": { + "app": "nginx" + } + }, + "spec": { + "containers": [{ + "name": "nginx", + "image": "nginx:latest" + }] + } + } + } + }`) + + resp, err := client.CoreV1().RESTClient().Post(). + AbsPath("/apis/apps/v1"). + Namespace("default"). + Resource("deployments"). + Body(obj).Do().Get() + if err != nil { + t.Fatalf("Failed to create object: %v: %v", err, resp) + } + result := client.CoreV1().RESTClient().Patch(types.MergePatchType). + AbsPath("/apis/apps/v1"). + Namespace("default"). + Resource("deployments"). + Name("deployment"). + Body([]byte(`{"metadata":{"annotations":{"foo":["bar"]}}}`)).Do() + var statusCode int + result.StatusCode(&statusCode) + if statusCode != 422 { + t.Fatalf("Expected status code to be 422, got %v (%#v)", statusCode, result) + } + result = client.CoreV1().RESTClient().Patch(types.StrategicMergePatchType). + AbsPath("/apis/apps/v1"). + Namespace("default"). + Resource("deployments"). + Name("deployment"). + Body([]byte(`{"metadata":{"annotations":{"foo":["bar"]}}}`)).Do() + result.StatusCode(&statusCode) + if statusCode != 422 { + t.Fatalf("Expected status code to be 422, got %v (%#v)", statusCode, result) + } +} + // Tests that the apiserver returns 202 status code as expected. func Test202StatusCode(t *testing.T) { s, clientSet, closeFn := setup(t)