UPSTREAM: 84963: Fix json patch limit check

Origin-commit: 0531b518f48894152d3407f081557340c4a72bdd

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go

@@ -292,7 +292,7 @@ func (p *jsonPatcher) applyJSPatch(versionedJS []byte) (patchedJS []byte, retErr
 		// TODO(liggitt): drop this once golang json parser limits stack depth (https://github.com/golang/go/issues/31789)
 		if len(p.patchJS) > 1024*1024 {
 			v := []interface{}{}
-			if err := json.Unmarshal(p.patchJS, v); err != nil {
+			if err := json.Unmarshal(p.patchJS, &v); err != nil {
 				return nil, errors.NewBadRequest(fmt.Sprintf("error decoding patch: %v", err))
 			}
 		}
@@ -316,7 +316,7 @@ func (p *jsonPatcher) applyJSPatch(versionedJS []byte) (patchedJS []byte, retErr
 		// TODO(liggitt): drop this once golang json parser limits stack depth (https://github.com/golang/go/issues/31789)
 		if len(p.patchJS) > 1024*1024 {
 			v := map[string]interface{}{}
-			if err := json.Unmarshal(p.patchJS, v); err != nil {
+			if err := json.Unmarshal(p.patchJS, &v); err != nil {
 				return nil, errors.NewBadRequest(fmt.Sprintf("error decoding patch: %v", err))
 			}
 		}

test/integration/apiserver/max_request_body_bytes_test.go

@@ -93,6 +93,14 @@ func TestMaxResourceSize(t *testing.T) {
 			t.Errorf("expected success or bad request err, got %v", err)
 		}
 	})
+	t.Run("JSONPatchType should handle a valid patch just under the max limit", func(t *testing.T) {
+		patchBody := []byte(`[{"op":"add","path":"/foo","value":0` + strings.Repeat(" ", 3*1024*1024-100) + `}]`)
+		err = rest.Patch(types.JSONPatchType).AbsPath(fmt.Sprintf("/api/v1/namespaces/default/secrets/test")).
+			Body(patchBody).Do().Error()
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
+	})
 	t.Run("MergePatchType should handle a patch just under the max limit", func(t *testing.T) {
 		patchBody := []byte(`{"value":` + strings.Repeat("[", 3*1024*1024/2-100) + strings.Repeat("]", 3*1024*1024/2-100) + `}`)
 		err = rest.Patch(types.MergePatchType).AbsPath(fmt.Sprintf("/api/v1/namespaces/default/secrets/test")).
@@ -101,6 +109,14 @@ func TestMaxResourceSize(t *testing.T) {
 			t.Errorf("expected success or bad request err, got %v", err)
 		}
 	})
+	t.Run("MergePatchType should handle a valid patch just under the max limit", func(t *testing.T) {
+		patchBody := []byte(`{"value":0` + strings.Repeat(" ", 3*1024*1024-100) + `}`)
+		err = rest.Patch(types.MergePatchType).AbsPath(fmt.Sprintf("/api/v1/namespaces/default/secrets/test")).
+			Body(patchBody).Do().Error()
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
+	})
 	t.Run("StrategicMergePatchType should handle a patch just under the max limit", func(t *testing.T) {
 		patchBody := []byte(`{"value":` + strings.Repeat("[", 3*1024*1024/2-100) + strings.Repeat("]", 3*1024*1024/2-100) + `}`)
 		err = rest.Patch(types.StrategicMergePatchType).AbsPath(fmt.Sprintf("/api/v1/namespaces/default/secrets/test")).
@@ -109,12 +125,12 @@ func TestMaxResourceSize(t *testing.T) {
 			t.Errorf("expected success or bad request err, got %v", err)
 		}
 	})
-	t.Run("ApplyPatchType should handle a patch just under the max limit", func(t *testing.T) {
-		patchBody := []byte(`{"value":` + strings.Repeat("[", 3*1024*1024/2-100) + strings.Repeat("]", 3*1024*1024/2-100) + `}`)
-		err = rest.Patch(types.ApplyPatchType).Param("fieldManager", "test").AbsPath(fmt.Sprintf("/api/v1/namespaces/default/secrets/test")).
+	t.Run("StrategicMergePatchType should handle a valid patch just under the max limit", func(t *testing.T) {
+		patchBody := []byte(`{"value":0` + strings.Repeat(" ", 3*1024*1024-100) + `}`)
+		err = rest.Patch(types.StrategicMergePatchType).AbsPath(fmt.Sprintf("/api/v1/namespaces/default/secrets/test")).
 			Body(patchBody).Do().Error()
-		if err != nil && !errors.IsBadRequest(err) {
-			t.Errorf("expected success or bad request err, got %#v", err)
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
 		}
 	})
 	t.Run("Delete should limit the request body size", func(t *testing.T) {