Merge pull request #51526 from atlassian/optimize-unstructured-converter

Automatic merge from submit-queue (batch tested with PRs 50392, 52108, 52083, 52134, 51526). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>..

Do deep copy instead of to and from JSON encoding

**What this PR does / why we need it**:
Unstructured converter encodes to JSON and then parses the result into a new object. For `Unstructured` this can be avoided by directly doing a deep copy. It is an optimization.

**Special notes for your reviewer**:
#47889 is somewhat related.

**Release note**:
```release-note
NONE
```
/sig api-machinery

Kubernetes-commit: 8fe960e71018f0e1bf1c427539f180c16147a646

Author: k8s-publish-robot

pkg/apis/meta/v1/unstructured/unstructured.go

@@ -145,33 +145,13 @@ func (u *Unstructured) UnmarshalJSON(b []byte) error {
 	return err
 }
 
-func deepCopyJSON(x interface{}) interface{} {
-	switch x := x.(type) {
-	case map[string]interface{}:
-		clone := make(map[string]interface{}, len(x))
-		for k, v := range x {
-			clone[k] = deepCopyJSON(v)
-		}
-		return clone
-	case []interface{}:
-		clone := make([]interface{}, len(x))
-		for i := range x {
-			clone[i] = deepCopyJSON(x[i])
-		}
-		return clone
-	default:
-		// only non-pointer values (float64, int64, bool, string) are left. These can be copied by-value.
-		return x
-	}
-}
-
 func (in *Unstructured) DeepCopy() *Unstructured {
 	if in == nil {
 		return nil
 	}
 	out := new(Unstructured)
 	*out = *in
-	out.Object = deepCopyJSON(in.Object).(map[string]interface{})
+	out.Object = unstructured.DeepCopyJSON(in.Object)
 	return out
 }
 
@@ -181,7 +161,7 @@ func (in *UnstructuredList) DeepCopy() *UnstructuredList {
 	}
 	out := new(UnstructuredList)
 	*out = *in
-	out.Object = deepCopyJSON(in.Object).(map[string]interface{})
+	out.Object = unstructured.DeepCopyJSON(in.Object)
 	out.Items = make([]Unstructured, len(in.Items))
 	for i := range in.Items {
 		in.Items[i].DeepCopyInto(&out.Items[i])

pkg/conversion/unstructured/BUILD

@@ -14,6 +14,7 @@ go_library(
     deps = [
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library",

pkg/conversion/unstructured/converter.go

@@ -29,6 +29,7 @@ import (
 	"sync/atomic"
 
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/apimachinery/pkg/util/json"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -106,6 +107,8 @@ func NewConverter(mismatchDetection bool) Converter {
 	}
 }
 
+// FromUnstructured converts an object from map[string]interface{} representation into a concrete type.
+// It uses encoding/json/Unmarshaler if object implements it or reflection if not.
 func (c *converterImpl) FromUnstructured(u map[string]interface{}, obj interface{}) error {
 	t := reflect.TypeOf(obj)
 	value := reflect.ValueOf(obj)
@@ -388,19 +391,27 @@ func interfaceFromUnstructured(sv, dv reflect.Value) error {
 	return nil
 }
 
+// ToUnstructured converts an object into map[string]interface{} representation.
+// It uses encoding/json/Marshaler if object implements it or reflection if not.
 func (c *converterImpl) ToUnstructured(obj interface{}) (map[string]interface{}, error) {
-	t := reflect.TypeOf(obj)
-	value := reflect.ValueOf(obj)
-	if t.Kind() != reflect.Ptr || value.IsNil() {
-		return nil, fmt.Errorf("ToUnstructured requires a non-nil pointer to an object, got %v", t)
+	var u map[string]interface{}
+	var err error
+	if unstr, ok := obj.(runtime.Unstructured); ok {
+		u = DeepCopyJSON(unstr.UnstructuredContent())
+	} else {
+		t := reflect.TypeOf(obj)
+		value := reflect.ValueOf(obj)
+		if t.Kind() != reflect.Ptr || value.IsNil() {
+			return nil, fmt.Errorf("ToUnstructured requires a non-nil pointer to an object, got %v", t)
+		}
+		u = map[string]interface{}{}
+		err = toUnstructured(value.Elem(), reflect.ValueOf(&u).Elem())
 	}
-	u := &map[string]interface{}{}
-	err := toUnstructured(value.Elem(), reflect.ValueOf(u).Elem())
 	if c.mismatchDetection {
-		newUnstr := &map[string]interface{}{}
-		newErr := toUnstructuredViaJSON(obj, newUnstr)
+		newUnstr := map[string]interface{}{}
+		newErr := toUnstructuredViaJSON(obj, &newUnstr)
 		if (err != nil) != (newErr != nil) {
-			glog.Fatalf("ToUnstructured unexpected error for %v: error: %v", obj, err)
+			glog.Fatalf("ToUnstructured unexpected error for %v: error: %v; newErr: %v", obj, err, newErr)
 		}
 		if err == nil && !apiequality.Semantic.DeepEqual(u, newUnstr) {
 			glog.Fatalf("ToUnstructured mismatch for %#v, diff: %v", u, diff.ObjectReflectDiff(u, newUnstr))
@@ -409,7 +420,34 @@ func (c *converterImpl) ToUnstructured(obj interface{}) (map[string]interface{},
 	if err != nil {
 		return nil, err
 	}
-	return *u, nil
+	return u, nil
+}
+
+// DeepCopyJSON deep copies the passed value, assuming it is a valid JSON representation i.e. only contains
+// types produced by json.Unmarshal().
+func DeepCopyJSON(x map[string]interface{}) map[string]interface{} {
+	return deepCopyJSON(x).(map[string]interface{})
+}
+
+func deepCopyJSON(x interface{}) interface{} {
+	switch x := x.(type) {
+	case map[string]interface{}:
+		clone := make(map[string]interface{}, len(x))
+		for k, v := range x {
+			clone[k] = deepCopyJSON(v)
+		}
+		return clone
+	case []interface{}:
+		clone := make([]interface{}, len(x))
+		for i, v := range x {
+			clone[i] = deepCopyJSON(v)
+		}
+		return clone
+	case string, int64, bool, float64, nil, encodingjson.Number:
+		return x
+	default:
+		panic(fmt.Errorf("cannot deep copy %T", x))
+	}
 }
 
 func toUnstructuredViaJSON(obj interface{}, u *map[string]interface{}) error {

pkg/conversion/unstructured/testing/converter_test.go

@@ -21,6 +21,7 @@ limitations under the License.
 package testing
 
 import (
+	encodingjson "encoding/json"
 	"fmt"
 	"reflect"
 	"strconv"
@@ -464,6 +465,24 @@ func TestUnrecognized(t *testing.T) {
 	}
 }
 
+func TestDeepCopyJSON(t *testing.T) {
+	src := map[string]interface{}{
+		"a": nil,
+		"b": int64(123),
+		"c": map[string]interface{}{
+			"a": "b",
+		},
+		"d": []interface{}{
+			int64(1), int64(2),
+		},
+		"e": "estr",
+		"f": true,
+		"g": encodingjson.Number("123"),
+	}
+	deepCopy := conversionunstructured.DeepCopyJSON(src)
+	assert.Equal(t, src, deepCopy)
+}
+
 func TestFloatIntConversion(t *testing.T) {
 	unstr := map[string]interface{}{"fd": float64(3)}