Fix error for strategic merge patch of custom resources

We need the go struct tags `patchMergeKey` and `patchStrategy`
for fields that support a strategic merge patch. For native
resources, we can easily figure out these tags since we know
the fields.

Because custom resources are decoded as Unstructured and
because we're missing the metadata about how to handle
each field in a strategic merge patch, we can't find the
go struct tags. Hence, we can't easily  do a strategic merge
for custom resources.

So we should fail fast and return an error.

staging/src/k8s.io/apimachinery/pkg/util/mergepatch/errors.go

@@ -29,6 +29,7 @@ var (
 	ErrBadPatchFormatForRetainKeys          = errors.New("invalid patch format of retainKeys")
 	ErrBadPatchFormatForSetElementOrderList = errors.New("invalid patch format of setElementOrder list")
 	ErrPatchContentNotMatchRetainKeys       = errors.New("patch content doesn't match retainKeys list")
+	ErrUnsupportedStrategicMergePatchFormat = errors.New("strategic merge patch format is not supported")
 )
 
 func ErrNoMergeKey(m map[string]interface{}, k string) error {

staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/BUILD

@@ -25,6 +25,7 @@ go_library(
     srcs = ["patch.go"],
     importpath = "k8s.io/apimachinery/pkg/util/strategicpatch",
     deps = [
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/mergepatch:go_default_library",
         "//vendor/k8s.io/apimachinery/third_party/forked/golang/json:go_default_library",

staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go

@@ -22,6 +22,7 @@ import (
 	"sort"
 	"strings"
 
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/json"
 	"k8s.io/apimachinery/pkg/util/mergepatch"
 	forkedjson "k8s.io/apimachinery/third_party/forked/golang/json"
@@ -828,7 +829,7 @@ func handleUnmarshal(j []byte) (map[string]interface{}, error) {
 	return m, nil
 }
 
-// StrategicMergePatch applies a strategic merge patch. The original and patch documents
+// StrategicMergeMapPatch applies a strategic merge patch. The original and patch documents
 // must be JSONMap. A patch can be created from an original and modified document by
 // calling CreateTwoWayMergeMapPatch.
 // Warning: the original and patch JSONMap objects are mutated by this function and should not be reused.
@@ -837,6 +838,17 @@ func StrategicMergeMapPatch(original, patch JSONMap, dataStruct interface{}) (JS
 	if err != nil {
 		return nil, err
 	}
+
+	// We need the go struct tags `patchMergeKey` and `patchStrategy` for fields that support a strategic merge patch.
+	// For native resources, we can easily figure out these tags since we know the fields.
+
+	// Because custom resources are decoded as Unstructured and because we're missing the metadata about how to handle
+	// each field in a strategic merge patch, we can't find the go struct tags. Hence, we can't easily  do a strategic merge
+	// for custom resources. So we should fail fast and return an error.
+	if _, ok := dataStruct.(*unstructured.Unstructured); ok {
+		return nil, mergepatch.ErrUnsupportedStrategicMergePatchFormat
+	}
+
 	mergeOptions := MergeOptions{
 		MergeParallelList:    true,
 		IgnoreUnmatchedNulls: true,
@@ -1532,7 +1544,7 @@ func findMapInSliceBasedOnKeyValue(m []interface{}, key string, value interface{
 	for k, v := range m {
 		typedV, ok := v.(map[string]interface{})
 		if !ok {
-			return nil, 0, false, fmt.Errorf("value for key %v is not a map.", k)
+			return nil, 0, false, fmt.Errorf("value for key %v is not a map", k)
 		}
 
 		valueToMatch, ok := typedV[key]

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD

@@ -21,6 +21,7 @@ go_test(
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",

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

@@ -444,7 +444,7 @@ func applyPatchToObject(
 ) error {
 	patchedObjMap, err := strategicpatch.StrategicMergeMapPatch(originalMap, patchMap, versionedObj)
 	if err != nil {
-		return err
+		return interpretPatchError(err)
 	}
 
 	// Rather than serialize the patched map to JSON, then decode it to an object, we go directly from a map to an object
@@ -460,7 +460,7 @@ func applyPatchToObject(
 // 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:
+	case mergepatch.ErrBadJSONDoc, mergepatch.ErrBadPatchFormatForPrimitiveList, mergepatch.ErrBadPatchFormatForRetainKeys, mergepatch.ErrBadPatchFormatForSetElementOrderList, mergepatch.ErrUnsupportedStrategicMergePatchFormat:
 		return errors.NewBadRequest(err.Error())
 	case mergepatch.ErrNoListOfLists, mergepatch.ErrPatchContentNotMatchRetainKeys:
 		return errors.NewGenericServerResponse(http.StatusUnprocessableEntity, "", schema.GroupResource{}, "", err.Error(), 0, false)

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

@@ -29,6 +29,7 @@ import (
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
@@ -113,7 +114,39 @@ func TestPatchInvalid(t *testing.T) {
 
 	actual := &testPatchType{}
 	err := strategicPatchObject(codec, defaulter, original, []byte(patch), actual, &testPatchType{})
-	if apierrors.IsBadRequest(err) == false {
+	if !apierrors.IsBadRequest(err) {
+		t.Errorf("expected HTTP status: BadRequest, got: %#v", apierrors.ReasonForError(err))
+	}
+	if err.Error() != expectedError {
+		t.Errorf("expected %#v, got %#v", expectedError, err.Error())
+	}
+}
+
+func TestPatchCustomResource(t *testing.T) {
+	testGV := schema.GroupVersion{Group: "mygroup.example.com", Version: "v1beta1"}
+	scheme.AddKnownTypes(testGV, &unstructured.Unstructured{})
+	codec := codecs.LegacyCodec(testGV)
+	defaulter := runtime.ObjectDefaulter(scheme)
+
+	original := &unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"apiVersion": "mygroup.example.com/v1beta1",
+			"kind":       "Noxu",
+			"metadata": map[string]interface{}{
+				"namespace": "Namespaced",
+				"name":      "foo",
+			},
+			"spec": map[string]interface{}{
+				"num": "10",
+			},
+		},
+	}
+	patch := `{"spec":{"num":"20"}}`
+	expectedError := "strategic merge patch format is not supported"
+
+	actual := &unstructured.Unstructured{}
+	err := strategicPatchObject(codec, defaulter, original, []byte(patch), actual, &unstructured.Unstructured{})
+	if !apierrors.IsBadRequest(err) {
 		t.Errorf("expected HTTP status: BadRequest, got: %#v", apierrors.ReasonForError(err))
 	}
 	if err.Error() != expectedError {