Add JSONPatch tests

liggitt · Oct 20, 2021 · 8bc736a · 8bc736a
1 parent fb910cd
commit 8bc736a
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 17 deletions.
diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/types.go b/staging/src/k8s.io/apimachinery/pkg/runtime/types.go
@@ -42,6 +42,7 @@ type TypeMeta struct {
 
 const (
 	ContentTypeJSON                    string = "application/json"
+	ContentTypeJSONPatch               string = "application/json-patch+json"
 	ContentTypeJSONMergePatch          string = "application/merge-patch+json"
 	ContentTypeJSONStrategicMergePatch string = "application/strategic-merge-patch+json"
 	ContentTypeYAML                    string = "application/yaml"

diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go
@@ -467,9 +467,12 @@ func fieldValidation(req *http.Request) (runtime.FieldValidationDirective, error
 		return runtime.IgnoreFieldValidation, nil
 	}
 
-	supportedContentTypes := []string{runtime.ContentTypeJSON, runtime.ContentTypeJSONMergePatch, runtime.ContentTypeJSONStrategicMergePatch, runtime.ContentTypeYAML}
+	// TODO: Should we blocklist unsupportedContentTypes (just protobuf) rather than allowlisting everything that isn't protobuf?
+	// TODO: Is there a better way to determine if something is JSON or YAML by its media type suffix rather than adding all these
+	// ContentTypes to the runtime package?
+	supportedContentTypes := []string{runtime.ContentTypeJSON, runtime.ContentTypeJSONPatch, runtime.ContentTypeJSONMergePatch, runtime.ContentTypeJSONStrategicMergePatch, runtime.ContentTypeYAML}
 	contentType := req.Header.Get("Content-Type")
-	// TODO: not sure if it is okay to assume empty content type is a valid one
+	// TODO: Is it okay to assume empty content type is a valid one?
 	supported := true
 	if contentType != "" {
 		supported = false

diff --git a/test/integration/apiserver/field_validation_test.go b/test/integration/apiserver/field_validation_test.go
@@ -550,21 +550,20 @@ func TestFieldValidationPatchCRD(t *testing.T) {
 			patchType: types.MergePatchType,
 			body:      `{"metadata":{"finalizers":["test-finalizer","another-one"]}, "spec":{"foo": "bar"}}`,
 		},
-		// TODO: figure out how to test JSONPatch
-		//{
-		//	name:        "jsonPatchStrictValidation",
-		//	patchType:   types.JSONPatchType,
-		//	params:      map[string]string{"validate": "strict"},
-		//	body:        // TODO
-		//	errContains: "failed with unknown fields",
-		//},
-		//{
-		//	name:        "jsonPatchNoValidation",
-		//	patchType:   types.JSONPatchType,
-		//	params:      map[string]string{},
-		//	body:        // TODO
-		//	errContains: "",
-		//},
+		{
+			name:      "json-patch-strict-validation",
+			patchType: types.JSONPatchType,
+			opts: metav1.PatchOptions{
+				FieldValidation: "Strict",
+			},
+			body:        `[{"op": "add", "path": "/spec/foo", "value": "bar"}]`,
+			errContains: "unknown field",
+		},
+		{
+			name:      "json-patch-strict-validation",
+			patchType: types.JSONPatchType,
+			body:      `[{"op": "add", "path": "/spec/foo", "value": "bar"}]`,
+		},
 	}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {