From e82e8b4e893610289dffd6a4b3e368a09ce41e7a Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Tue, 3 Apr 2018 22:13:29 +1000 Subject: [PATCH] Make UnstructuredContent return contents without mutating the source --- pkg/printers/internalversion/describe_test.go | 4 +-- .../pkg/apis/meta/v1/unstructured/BUILD | 1 + .../apis/meta/v1/unstructured/unstructured.go | 2 +- .../meta/v1/unstructured/unstructured_list.go | 20 +++++++------ .../meta/v1/unstructured/unstructured_test.go | 30 +++++++++++++++++++ .../apimachinery/pkg/runtime/converter.go | 3 +- .../apimachinery/pkg/runtime/interfaces.go | 4 +-- .../apimachinery/pkg/runtime/testing/types.go | 2 +- 8 files changed, 49 insertions(+), 17 deletions(-) create mode 100644 staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_test.go diff --git a/pkg/printers/internalversion/describe_test.go b/pkg/printers/internalversion/describe_test.go index a49b812d9fc..487325415e2 100644 --- a/pkg/printers/internalversion/describe_test.go +++ b/pkg/printers/internalversion/describe_test.go @@ -2133,14 +2133,14 @@ URL: http://localhost "name": "MyName", "namespace": "MyNamespace", "creationTimestamp": "2017-04-01T00:00:00Z", - "resourceVersion": 123, + "resourceVersion": int64(123), "uid": "00000000-0000-0000-0000-000000000001", "dummy3": "present", }, "items": []interface{}{ map[string]interface{}{ "itemBool": true, - "itemInt": 42, + "itemInt": int64(42), }, }, "url": "http://localhost", diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/BUILD b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/BUILD index 22c1acee07d..5f55c32d94e 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/BUILD @@ -11,6 +11,7 @@ go_test( srcs = [ "helpers_test.go", "unstructured_list_test.go", + "unstructured_test.go", ], embed = [":go_default_library"], deps = [ 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 2a13330490a..b56437955d0 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 @@ -82,7 +82,7 @@ func (obj *Unstructured) EachListItem(fn func(runtime.Object) error) error { func (obj *Unstructured) UnstructuredContent() map[string]interface{} { if obj.Object == nil { - obj.Object = make(map[string]interface{}) + return make(map[string]interface{}) } return obj.Object } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_list.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_list.go index 57d78a09dcc..fbcb386ee47 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_list.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_list.go @@ -53,19 +53,21 @@ func (u *UnstructuredList) EachListItem(fn func(runtime.Object) error) error { } // UnstructuredContent returns a map contain an overlay of the Items field onto -// the Object field. Items always overwrites overlay. Changing "items" in the -// returned object will affect items in the underlying Items field, but changing -// the "items" slice itself will have no effect. -// TODO: expose SetUnstructuredContent on runtime.Unstructured that allows -// items to be changed. +// the Object field. Items always overwrites overlay. func (u *UnstructuredList) UnstructuredContent() map[string]interface{} { - out := u.Object - if out == nil { - out = make(map[string]interface{}) + out := make(map[string]interface{}, len(u.Object)+1) + + // shallow copy every property but items + for k, v := range u.Object { + if k == "items" { + continue + } + out[k] = v } + items := make([]interface{}, len(u.Items)) for i, item := range u.Items { - items[i] = item.Object + items[i] = item.UnstructuredContent() } out["items"] = items return out diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_test.go new file mode 100644 index 00000000000..4021bedca09 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_test.go @@ -0,0 +1,30 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package unstructured + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNilUnstructuredContent(t *testing.T) { + var u Unstructured + content := u.UnstructuredContent() + expContent := make(map[string]interface{}) + assert.EqualValues(t, expContent, content) +} diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/converter.go b/staging/src/k8s.io/apimachinery/pkg/runtime/converter.go index f6f7c10de62..429857e88e9 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/converter.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/converter.go @@ -411,8 +411,7 @@ func (c *unstructuredConverter) ToUnstructured(obj interface{}) (map[string]inte var u map[string]interface{} var err error if unstr, ok := obj.(Unstructured); ok { - // UnstructuredContent() mutates the object so we need to make a copy first - u = unstr.DeepCopyObject().(Unstructured).UnstructuredContent() + u = unstr.UnstructuredContent() } else { t := reflect.TypeOf(obj) value := reflect.ValueOf(obj) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go b/staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go index 9d00f1650e9..cfe819e0a28 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go @@ -234,9 +234,9 @@ type Object interface { // to JSON allowed. type Unstructured interface { Object - // UnstructuredContent returns a non-nil, mutable map of the contents of this object. Values may be + // UnstructuredContent returns a non-nil map with this object's contents. Values may be // []interface{}, map[string]interface{}, or any primitive type. Contents are typically serialized to - // and from JSON. + // and from JSON. SetUnstructuredContent should be used to mutate the contents. UnstructuredContent() map[string]interface{} // SetUnstructuredContent updates the object content to match the provided map. SetUnstructuredContent(map[string]interface{}) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/testing/types.go b/staging/src/k8s.io/apimachinery/pkg/runtime/testing/types.go index 0694e701122..b8d67061d4b 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/testing/types.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/testing/types.go @@ -263,7 +263,7 @@ func (obj *Unstructured) EachListItem(fn func(runtime.Object) error) error { func (obj *Unstructured) UnstructuredContent() map[string]interface{} { if obj.Object == nil { - obj.Object = make(map[string]interface{}) + return make(map[string]interface{}) } return obj.Object }