From 51e653dc68e47584ab571fc214a7aa5950918831 Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Mon, 11 Sep 2017 10:18:22 +1000 Subject: [PATCH] Do deep copy instead of to and from JSON encoding --- .../apis/meta/v1/unstructured/unstructured.go | 24 +------- .../pkg/conversion/unstructured/BUILD | 1 + .../pkg/conversion/unstructured/converter.go | 58 +++++++++++++++---- .../unstructured/testing/converter_test.go | 19 ++++++ 4 files changed, 70 insertions(+), 32 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go index 588230f4090..7213b7a1662 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go +++ b/staging/src/k8s.io/apimachinery/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]) diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/BUILD b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/BUILD index a98e4232620..d10d733c358 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/BUILD +++ b/staging/src/k8s.io/apimachinery/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", diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go index 84dca4ac7c2..d0e625d2c17 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go +++ b/staging/src/k8s.io/apimachinery/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 { diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/converter_test.go b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/converter_test.go index 1338e4e430b..f367c6310b6 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/converter_test.go +++ b/staging/src/k8s.io/apimachinery/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)}