From 557f9ddfe6300ecdbdd70ea67dca63bf6b710ea7 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Mon, 13 Nov 2017 21:28:57 -0500 Subject: [PATCH 1/5] Move unstructured conversion into pkg/runtime Scheme conversion should support unstructured conversion natively to allow going from unstructured to typed and back. It is not a higher level responsibility to do that conversion because the scheme is the only one who knows what types it supports. --- hack/.golint_failures | 1 - pkg/api/testing/BUILD | 1 - pkg/api/testing/unstructured_test.go | 13 +++-- staging/src/k8s.io/api/Godeps/Godeps.json | 4 ++ .../Godeps/Godeps.json | 4 -- .../pkg/apis/meta/v1/unstructured/BUILD | 1 - .../pkg/apis/meta/v1/unstructured/helpers.go | 11 ++-- .../apis/meta/v1/unstructured/unstructured.go | 7 ++- .../meta/v1/unstructured/unstructured_list.go | 3 +- .../k8s.io/apimachinery/pkg/conversion/BUILD | 1 - .../pkg/conversion/unstructured/BUILD | 39 -------------- .../pkg/conversion/unstructured/doc.go | 19 ------- .../pkg/conversion/unstructured/testing/BUILD | 29 ----------- .../src/k8s.io/apimachinery/pkg/runtime/BUILD | 9 ++++ .../unstructured => runtime}/converter.go | 52 ++++++++++++------- .../testing => runtime}/converter_test.go | 34 +++++++----- .../src/k8s.io/apiserver/Godeps/Godeps.json | 4 -- .../apiserver/pkg/endpoints/handlers/BUILD | 1 - .../apiserver/pkg/endpoints/handlers/patch.go | 9 ++-- .../src/k8s.io/client-go/Godeps/Godeps.json | 4 -- .../k8s.io/kube-aggregator/Godeps/Godeps.json | 4 -- staging/src/k8s.io/metrics/Godeps/Godeps.json | 16 ------ .../sample-apiserver/Godeps/Godeps.json | 4 -- .../sample-controller/Godeps/Godeps.json | 8 --- 24 files changed, 85 insertions(+), 193 deletions(-) delete mode 100644 staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/BUILD delete mode 100644 staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/doc.go delete mode 100644 staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/BUILD rename staging/src/k8s.io/apimachinery/pkg/{conversion/unstructured => runtime}/converter.go (92%) rename staging/src/k8s.io/apimachinery/pkg/{conversion/unstructured/testing => runtime}/converter_test.go (93%) diff --git a/hack/.golint_failures b/hack/.golint_failures index dab25848dd0..6c8bd346931 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -505,7 +505,6 @@ staging/src/k8s.io/apimachinery/pkg/apis/meta/v1alpha1 staging/src/k8s.io/apimachinery/pkg/apis/testapigroup staging/src/k8s.io/apimachinery/pkg/apis/testapigroup/v1 staging/src/k8s.io/apimachinery/pkg/conversion -staging/src/k8s.io/apimachinery/pkg/conversion/unstructured staging/src/k8s.io/apimachinery/pkg/labels staging/src/k8s.io/apimachinery/pkg/runtime/schema staging/src/k8s.io/apimachinery/pkg/runtime/serializer diff --git a/pkg/api/testing/BUILD b/pkg/api/testing/BUILD index 149b71937be..9c04ee34584 100644 --- a/pkg/api/testing/BUILD +++ b/pkg/api/testing/BUILD @@ -97,7 +97,6 @@ go_test( "//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/conversion:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/conversion/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/protobuf:go_default_library", diff --git a/pkg/api/testing/unstructured_test.go b/pkg/api/testing/unstructured_test.go index c3cc41424a9..005b5d4a9ae 100644 --- a/pkg/api/testing/unstructured_test.go +++ b/pkg/api/testing/unstructured_test.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/api/testing/fuzzer" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metaunstruct "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/conversion/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/json" @@ -99,14 +98,14 @@ func doRoundTrip(t *testing.T, group testapi.TestGroup, kind string) { return } - newUnstr, err := unstructured.DefaultConverter.ToUnstructured(item) + newUnstr, err := runtime.NewTestUnstructuredConverter(apiequality.Semantic).ToUnstructured(item) if err != nil { t.Errorf("ToUnstructured failed: %v", err) return } newObj := reflect.New(reflect.TypeOf(item).Elem()).Interface().(runtime.Object) - err = unstructured.DefaultConverter.FromUnstructured(newUnstr, newObj) + err = runtime.DefaultUnstructuredConverter.FromUnstructured(newUnstr, newObj) if err != nil { t.Errorf("FromUnstructured failed: %v", err) return @@ -146,7 +145,7 @@ func TestRoundTripWithEmptyCreationTimestamp(t *testing.T) { } t.Logf("Testing: %v in %v", kind, groupKey) - unstrBody, err := unstructured.DefaultConverter.ToUnstructured(item) + unstrBody, err := runtime.DefaultUnstructuredConverter.ToUnstructured(item) if err != nil { t.Fatalf("ToUnstructured failed: %v", err) } @@ -163,7 +162,7 @@ func TestRoundTripWithEmptyCreationTimestamp(t *testing.T) { // attempt to re-convert unstructured object - conversion should not fail // based on empty metadata fields, such as creationTimestamp newObj := reflect.New(reflect.TypeOf(item).Elem()).Interface().(runtime.Object) - err = unstructured.DefaultConverter.FromUnstructured(unstructObj.Object, newObj) + err = runtime.NewTestUnstructuredConverter(apiequality.Semantic).FromUnstructured(unstructObj.Object, newObj) if err != nil { t.Fatalf("FromUnstructured failed: %v", err) } @@ -176,12 +175,12 @@ func BenchmarkToFromUnstructured(b *testing.B) { size := len(items) b.ResetTimer() for i := 0; i < b.N; i++ { - unstr, err := unstructured.DefaultConverter.ToUnstructured(&items[i%size]) + unstr, err := runtime.NewTestUnstructuredConverter(apiequality.Semantic).ToUnstructured(&items[i%size]) if err != nil { b.Fatalf("unexpected error: %v", err) } obj := v1.Pod{} - if err := unstructured.DefaultConverter.FromUnstructured(unstr, &obj); err != nil { + if err := runtime.NewTestUnstructuredConverter(apiequality.Semantic).FromUnstructured(unstr, &obj); err != nil { b.Fatalf("unexpected error: %v", err) } } diff --git a/staging/src/k8s.io/api/Godeps/Godeps.json b/staging/src/k8s.io/api/Godeps/Godeps.json index 83b361acac5..ffa35e2d980 100644 --- a/staging/src/k8s.io/api/Godeps/Godeps.json +++ b/staging/src/k8s.io/api/Godeps/Godeps.json @@ -182,6 +182,10 @@ "ImportPath": "k8s.io/apimachinery/pkg/util/intstr", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, + { + "ImportPath": "k8s.io/apimachinery/pkg/util/json", + "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + }, { "ImportPath": "k8s.io/apimachinery/pkg/util/net", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" diff --git a/staging/src/k8s.io/apiextensions-apiserver/Godeps/Godeps.json b/staging/src/k8s.io/apiextensions-apiserver/Godeps/Godeps.json index 7f61204935c..bd34b09887e 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/Godeps/Godeps.json +++ b/staging/src/k8s.io/apiextensions-apiserver/Godeps/Godeps.json @@ -702,10 +702,6 @@ "ImportPath": "k8s.io/apimachinery/pkg/conversion/queryparams", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, - { - "ImportPath": "k8s.io/apimachinery/pkg/conversion/unstructured", - "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" - }, { "ImportPath": "k8s.io/apimachinery/pkg/fields", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" 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 870ef7b906c..7ae9cb0f9d8 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 @@ -32,7 +32,6 @@ go_library( importpath = "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured", deps = [ "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/conversion/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/types:go_default_library", diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/helpers.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/helpers.go index ac4355bf577..5c9f1a55509 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/helpers.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/helpers.go @@ -24,7 +24,6 @@ import ( "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/conversion/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -39,7 +38,7 @@ func NestedFieldCopy(obj map[string]interface{}, fields ...string) (interface{}, if !ok { return nil, false } - return unstructured.DeepCopyJSONValue(val), true + return runtime.DeepCopyJSONValue(val), true } func nestedFieldNoCopy(obj map[string]interface{}, fields ...string) (interface{}, bool) { @@ -131,7 +130,7 @@ func NestedSlice(obj map[string]interface{}, fields ...string) ([]interface{}, b return nil, false } if _, ok := val.([]interface{}); ok { - return unstructured.DeepCopyJSONValue(val).([]interface{}), true + return runtime.DeepCopyJSONValue(val).([]interface{}), true } return nil, false } @@ -165,7 +164,7 @@ func NestedMap(obj map[string]interface{}, fields ...string) (map[string]interfa return nil, false } if m, ok := val.(map[string]interface{}); ok { - return unstructured.DeepCopyJSON(m), true + return runtime.DeepCopyJSON(m), true } return nil, false } @@ -173,7 +172,7 @@ func NestedMap(obj map[string]interface{}, fields ...string) (map[string]interfa // SetNestedField sets the value of a nested field to a deep copy of the value provided. // Returns false if value cannot be set because one of the nesting levels is not a map[string]interface{}. func SetNestedField(obj map[string]interface{}, value interface{}, fields ...string) bool { - return setNestedFieldNoCopy(obj, unstructured.DeepCopyJSONValue(value), fields...) + return setNestedFieldNoCopy(obj, runtime.DeepCopyJSONValue(value), fields...) } func setNestedFieldNoCopy(obj map[string]interface{}, value interface{}, fields ...string) bool { @@ -288,8 +287,6 @@ func setOwnerReference(src metav1.OwnerReference) map[string]interface{} { return ret } -var converter = unstructured.NewConverter(false) - // UnstructuredJSONScheme is capable of converting JSON data into the Unstructured // type, which can be used for generic access to objects without a predefined scheme. // TODO: move into serializer/json. 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 56d18a32e85..c635f34409b 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 @@ -21,7 +21,6 @@ import ( "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/conversion/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -112,7 +111,7 @@ func (in *Unstructured) DeepCopy() *Unstructured { } out := new(Unstructured) *out = *in - out.Object = unstructured.DeepCopyJSON(in.Object) + out.Object = runtime.DeepCopyJSON(in.Object) return out } @@ -348,7 +347,7 @@ func (u *Unstructured) GetInitializers() *metav1.Initializers { return nil } out := &metav1.Initializers{} - if err := converter.FromUnstructured(obj, out); err != nil { + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj, out); err != nil { utilruntime.HandleError(fmt.Errorf("unable to retrieve initializers for object: %v", err)) } return out @@ -359,7 +358,7 @@ func (u *Unstructured) SetInitializers(initializers *metav1.Initializers) { RemoveNestedField(u.Object, "metadata", "initializers") return } - out, err := converter.ToUnstructured(initializers) + out, err := runtime.DefaultUnstructuredConverter.ToUnstructured(initializers) if err != nil { utilruntime.HandleError(fmt.Errorf("unable to retrieve initializers for object: %v", err)) } 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 11270a6bf83..4db4162ac29 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 @@ -20,7 +20,6 @@ import ( "bytes" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/conversion/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -80,7 +79,7 @@ func (u *UnstructuredList) DeepCopy() *UnstructuredList { } out := new(UnstructuredList) *out = *u - out.Object = unstructured.DeepCopyJSON(u.Object) + out.Object = runtime.DeepCopyJSON(u.Object) out.Items = make([]Unstructured, len(u.Items)) for i := range u.Items { u.Items[i].DeepCopyInto(&out.Items[i]) diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/BUILD b/staging/src/k8s.io/apimachinery/pkg/conversion/BUILD index 72add7598f6..184dafbff1d 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/conversion/BUILD @@ -45,7 +45,6 @@ filegroup( srcs = [ ":package-srcs", "//staging/src/k8s.io/apimachinery/pkg/conversion/queryparams:all-srcs", - "//staging/src/k8s.io/apimachinery/pkg/conversion/unstructured:all-srcs", ], tags = ["automanaged"], ) diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/BUILD b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/BUILD deleted file mode 100644 index 91c1bb00bdf..00000000000 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/BUILD +++ /dev/null @@ -1,39 +0,0 @@ -package(default_visibility = ["//visibility:public"]) - -load( - "@io_bazel_rules_go//go:def.bzl", - "go_library", -) - -go_library( - name = "go_default_library", - srcs = [ - "converter.go", - "doc.go", - ], - importpath = "k8s.io/apimachinery/pkg/conversion/unstructured", - 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", - ], -) - -filegroup( - name = "package-srcs", - srcs = glob(["**"]), - tags = ["automanaged"], - visibility = ["//visibility:private"], -) - -filegroup( - name = "all-srcs", - srcs = [ - ":package-srcs", - "//staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing:all-srcs", - ], - tags = ["automanaged"], -) diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/doc.go b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/doc.go deleted file mode 100644 index cd40e74bed3..00000000000 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/doc.go +++ /dev/null @@ -1,19 +0,0 @@ -/* -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 provides conversion from runtime objects -// to map[string]interface{} representation. -package unstructured // import "k8s.io/apimachinery/pkg/conversion/unstructured" diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/BUILD b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/BUILD deleted file mode 100644 index eaeca02a7ed..00000000000 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/BUILD +++ /dev/null @@ -1,29 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_test") - -go_test( - name = "go_default_test", - srcs = ["converter_test.go"], - importpath = "k8s.io/apimachinery/pkg/conversion/unstructured/testing", - deps = [ - "//vendor/github.com/stretchr/testify/assert:go_default_library", - "//vendor/github.com/stretchr/testify/require:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/conversion/unstructured:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library", - ], -) - -filegroup( - name = "package-srcs", - srcs = glob(["**"]), - tags = ["automanaged"], - visibility = ["//visibility:private"], -) - -filegroup( - name = "all-srcs", - srcs = [":package-srcs"], - tags = ["automanaged"], - visibility = ["//visibility:public"], -) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/BUILD b/staging/src/k8s.io/apimachinery/pkg/runtime/BUILD index 2085e643fd6..93c6dcbfc77 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/BUILD @@ -19,6 +19,7 @@ go_library( "codec.go", "codec_check.go", "conversion.go", + "converter.go", "doc.go", "embedded.go", "error.go", @@ -37,10 +38,13 @@ go_library( importpath = "k8s.io/apimachinery/pkg/runtime", deps = [ "//vendor/github.com/gogo/protobuf/proto:go_default_library", + "//vendor/github.com/golang/glog:go_default_library", "//vendor/k8s.io/apimachinery/pkg/conversion:go_default_library", "//vendor/k8s.io/apimachinery/pkg/conversion/queryparams:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", ], ) @@ -48,6 +52,7 @@ go_test( name = "go_default_xtest", srcs = [ "conversion_test.go", + "converter_test.go", "embedded_test.go", "extension_test.go", "scheme_test.go", @@ -56,13 +61,17 @@ go_test( deps = [ "//vendor/github.com/google/gofuzz:go_default_library", "//vendor/github.com/spf13/pflag:go_default_library", + "//vendor/github.com/stretchr/testify/assert:go_default_library", + "//vendor/github.com/stretchr/testify/require:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", "//vendor/k8s.io/apimachinery/pkg/conversion: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", "//vendor/k8s.io/apimachinery/pkg/runtime/testing:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library", ], ) diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go b/staging/src/k8s.io/apimachinery/pkg/runtime/converter.go similarity index 92% rename from staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go rename to staging/src/k8s.io/apimachinery/pkg/runtime/converter.go index 077d173d8c1..b10fe4858bf 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/converter.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package unstructured +package runtime import ( "bytes" @@ -27,19 +27,18 @@ import ( "strings" "sync" "sync/atomic" + "time" - apiequality "k8s.io/apimachinery/pkg/api/equality" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/apimachinery/pkg/conversion" "k8s.io/apimachinery/pkg/util/json" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "github.com/golang/glog" ) -// Converter is an interface for converting between interface{} +// UnstructuredConverter is an interface for converting between interface{} // and map[string]interface representation. -type Converter interface { +type UnstructuredConverter interface { ToUnstructured(obj interface{}) (map[string]interface{}, error) FromUnstructured(u map[string]interface{}, obj interface{}) error } @@ -78,7 +77,16 @@ var ( float64Type = reflect.TypeOf(float64(0)) boolType = reflect.TypeOf(bool(false)) fieldCache = newFieldsCache() - DefaultConverter = NewConverter(parseBool(os.Getenv("KUBE_PATCH_CONVERSION_DETECTOR"))) + + // DefaultUnstructuredConverter performs unstructured to Go typed object conversions. + DefaultUnstructuredConverter = &unstructuredConverter{ + mismatchDetection: parseBool(os.Getenv("KUBE_PATCH_CONVERSION_DETECTOR")), + comparison: conversion.EqualitiesOrDie( + func(a, b time.Time) bool { + return a.UTC() == b.UTC() + }, + ), + } ) func parseBool(key string) bool { @@ -92,24 +100,30 @@ func parseBool(key string) bool { return value } -// ConverterImpl knows how to convert between interface{} and +// unstructuredConverter knows how to convert between interface{} and // Unstructured in both ways. -type converterImpl struct { +type unstructuredConverter struct { // If true, we will be additionally running conversion via json // to ensure that the result is true. // This is supposed to be set only in tests. mismatchDetection bool + // comparison is the default test logic used to compare + comparison conversion.Equalities } -func NewConverter(mismatchDetection bool) Converter { - return &converterImpl{ - mismatchDetection: mismatchDetection, +// NewTestUnstructuredConverter creates an UnstructuredConverter that accepts JSON typed maps and translates them +// to Go types via reflection. It performs mismatch detection automatically and is intended for use by external +// test tools. Use DefaultUnstructuredConverter if you do not explicitly need mismatch detection. +func NewTestUnstructuredConverter(comparison conversion.Equalities) UnstructuredConverter { + return &unstructuredConverter{ + mismatchDetection: true, + comparison: comparison, } } // 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 { +func (c *unstructuredConverter) FromUnstructured(u map[string]interface{}, obj interface{}) error { t := reflect.TypeOf(obj) value := reflect.ValueOf(obj) if t.Kind() != reflect.Ptr || value.IsNil() { @@ -122,8 +136,8 @@ func (c *converterImpl) FromUnstructured(u map[string]interface{}, obj interface if (err != nil) != (newErr != nil) { glog.Fatalf("FromUnstructured unexpected error for %v: error: %v", u, err) } - if err == nil && !apiequality.Semantic.DeepEqual(obj, newObj) { - glog.Fatalf("FromUnstructured mismatch for %#v, diff: %v", obj, diff.ObjectReflectDiff(obj, newObj)) + if err == nil && !c.comparison.DeepEqual(obj, newObj) { + glog.Fatalf("FromUnstructured mismatch\nobj1: %#v\nobj2: %#v", obj, newObj) } } return err @@ -393,10 +407,10 @@ func interfaceFromUnstructured(sv, dv reflect.Value) error { // 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) { +func (c *unstructuredConverter) ToUnstructured(obj interface{}) (map[string]interface{}, error) { var u map[string]interface{} var err error - if unstr, ok := obj.(runtime.Unstructured); ok { + if unstr, ok := obj.(Unstructured); ok { u = DeepCopyJSON(unstr.UnstructuredContent()) } else { t := reflect.TypeOf(obj) @@ -413,8 +427,8 @@ func (c *converterImpl) ToUnstructured(obj interface{}) (map[string]interface{}, if (err != nil) != (newErr != nil) { 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)) + if err == nil && !c.comparison.DeepEqual(u, newUnstr) { + glog.Fatalf("ToUnstructured mismatch\nobj1: %#v\nobj2: %#v", u, newUnstr) } } if err != nil { diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/converter_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/converter_test.go similarity index 93% rename from staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/converter_test.go rename to staging/src/k8s.io/apimachinery/pkg/runtime/converter_test.go index f367c6310b6..1e9515102cf 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing/converter_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/converter_test.go @@ -18,7 +18,7 @@ limitations under the License. // Unstructured type depends on unstructured converter package but we want to test how the converter handles // the Unstructured type so we need to import both. -package testing +package runtime_test import ( encodingjson "encoding/json" @@ -26,9 +26,11 @@ import ( "reflect" "strconv" "testing" + "time" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - conversionunstructured "k8s.io/apimachinery/pkg/conversion/unstructured" + "k8s.io/apimachinery/pkg/conversion" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/json" @@ -36,6 +38,12 @@ import ( "github.com/stretchr/testify/require" ) +var simpleEquality = conversion.EqualitiesOrDie( + func(a, b time.Time) bool { + return a.UTC() == b.UTC() + }, +) + // Definte a number of test types. type A struct { A int `json:"aa,omitempty"` @@ -137,14 +145,15 @@ func doRoundTrip(t *testing.T, item interface{}) { return } - newUnstr, err := conversionunstructured.DefaultConverter.ToUnstructured(item) + // TODO: should be using mismatch detection but fails due to another error + newUnstr, err := runtime.DefaultUnstructuredConverter.ToUnstructured(item) if err != nil { t.Errorf("ToUnstructured failed: %v", err) return } newObj := reflect.New(reflect.TypeOf(item).Elem()).Interface() - err = conversionunstructured.DefaultConverter.FromUnstructured(newUnstr, newObj) + err = runtime.NewTestUnstructuredConverter(simpleEquality).FromUnstructured(newUnstr, newObj) if err != nil { t.Errorf("FromUnstructured failed: %v", err) return @@ -239,10 +248,9 @@ func TestRoundTrip(t *testing.T) { } for i := range testCases { - doRoundTrip(t, testCases[i].obj) - if t.Failed() { - break - } + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + doRoundTrip(t, testCases[i].obj) + }) } } @@ -265,7 +273,7 @@ func doUnrecognized(t *testing.T, jsonData string, item interface{}, expectedErr return } newObj := reflect.New(reflect.TypeOf(item).Elem()).Interface() - err = conversionunstructured.DefaultConverter.FromUnstructured(unstr, newObj) + err = runtime.NewTestUnstructuredConverter(simpleEquality).FromUnstructured(unstr, newObj) if (err != nil) != (expectedErr != nil) { t.Errorf("Unexpected error in FromUnstructured: %v, expected: %v", err, expectedErr) } @@ -479,7 +487,7 @@ func TestDeepCopyJSON(t *testing.T) { "f": true, "g": encodingjson.Number("123"), } - deepCopy := conversionunstructured.DeepCopyJSON(src) + deepCopy := runtime.DeepCopyJSON(src) assert.Equal(t, src, deepCopy) } @@ -487,7 +495,7 @@ func TestFloatIntConversion(t *testing.T) { unstr := map[string]interface{}{"fd": float64(3)} var obj F - if err := conversionunstructured.DefaultConverter.FromUnstructured(unstr, &obj); err != nil { + if err := runtime.NewTestUnstructuredConverter(simpleEquality).FromUnstructured(unstr, &obj); err != nil { t.Errorf("Unexpected error in FromUnstructured: %v", err) } @@ -525,7 +533,7 @@ func TestCustomToUnstructured(t *testing.T) { tc := tc t.Run(tc.Data, func(t *testing.T) { t.Parallel() - result, err := conversionunstructured.DefaultConverter.ToUnstructured(&G{ + result, err := runtime.NewTestUnstructuredConverter(simpleEquality).ToUnstructured(&G{ CustomValue1: CustomValue{data: []byte(tc.Data)}, CustomValue2: &CustomValue{data: []byte(tc.Data)}, CustomPointer1: CustomPointer{data: []byte(tc.Data)}, @@ -550,7 +558,7 @@ func TestCustomToUnstructuredTopLevel(t *testing.T) { obj := obj t.Run(strconv.Itoa(i), func(t *testing.T) { t.Parallel() - result, err := conversionunstructured.DefaultConverter.ToUnstructured(obj) + result, err := runtime.NewTestUnstructuredConverter(simpleEquality).ToUnstructured(obj) require.NoError(t, err) assert.Equal(t, expected, result) }) diff --git a/staging/src/k8s.io/apiserver/Godeps/Godeps.json b/staging/src/k8s.io/apiserver/Godeps/Godeps.json index 1e0d2090888..717b1127f6f 100644 --- a/staging/src/k8s.io/apiserver/Godeps/Godeps.json +++ b/staging/src/k8s.io/apiserver/Godeps/Godeps.json @@ -954,10 +954,6 @@ "ImportPath": "k8s.io/apimachinery/pkg/conversion/queryparams", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, - { - "ImportPath": "k8s.io/apimachinery/pkg/conversion/unstructured", - "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" - }, { "ImportPath": "k8s.io/apimachinery/pkg/fields", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD index 92b7b282322..7e2cdcc1b19 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD @@ -60,7 +60,6 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/apis/meta/internalversion:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1alpha1:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/conversion/unstructured:go_default_library", "//vendor/k8s.io/apimachinery/pkg/fields:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index 8db9f40c93a..1ac736d09dd 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -27,7 +27,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/conversion/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -232,7 +231,7 @@ func patchResource( return nil, err } // Capture the original object map and patch for possible retries. - originalMap, err := unstructured.DefaultConverter.ToUnstructured(currentVersionedObject) + originalMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(currentVersionedObject) if err != nil { return nil, err } @@ -278,7 +277,7 @@ func patchResource( if err != nil { return nil, err } - currentObjMap, err := unstructured.DefaultConverter.ToUnstructured(currentVersionedObject) + currentObjMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(currentVersionedObject) if err != nil { return nil, err } @@ -425,7 +424,7 @@ func strategicPatchObject( objToUpdate runtime.Object, versionedObj runtime.Object, ) error { - originalObjMap, err := unstructured.DefaultConverter.ToUnstructured(originalObject) + originalObjMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(originalObject) if err != nil { return err } @@ -458,7 +457,7 @@ func applyPatchToObject( } // Rather than serialize the patched map to JSON, then decode it to an object, we go directly from a map to an object - if err := unstructured.DefaultConverter.FromUnstructured(patchedObjMap, objToUpdate); err != nil { + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(patchedObjMap, objToUpdate); err != nil { return err } // Decoding from JSON to a versioned object would apply defaults, so we do the same here diff --git a/staging/src/k8s.io/client-go/Godeps/Godeps.json b/staging/src/k8s.io/client-go/Godeps/Godeps.json index 917b30d09e7..32a753405b9 100644 --- a/staging/src/k8s.io/client-go/Godeps/Godeps.json +++ b/staging/src/k8s.io/client-go/Godeps/Godeps.json @@ -546,10 +546,6 @@ "ImportPath": "k8s.io/apimachinery/pkg/conversion/queryparams", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, - { - "ImportPath": "k8s.io/apimachinery/pkg/conversion/unstructured", - "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" - }, { "ImportPath": "k8s.io/apimachinery/pkg/fields", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" diff --git a/staging/src/k8s.io/kube-aggregator/Godeps/Godeps.json b/staging/src/k8s.io/kube-aggregator/Godeps/Godeps.json index 77163536018..80920dbf316 100644 --- a/staging/src/k8s.io/kube-aggregator/Godeps/Godeps.json +++ b/staging/src/k8s.io/kube-aggregator/Godeps/Godeps.json @@ -666,10 +666,6 @@ "ImportPath": "k8s.io/apimachinery/pkg/conversion/queryparams", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, - { - "ImportPath": "k8s.io/apimachinery/pkg/conversion/unstructured", - "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" - }, { "ImportPath": "k8s.io/apimachinery/pkg/fields", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" diff --git a/staging/src/k8s.io/metrics/Godeps/Godeps.json b/staging/src/k8s.io/metrics/Godeps/Godeps.json index f04be36963f..155f834ab7d 100644 --- a/staging/src/k8s.io/metrics/Godeps/Godeps.json +++ b/staging/src/k8s.io/metrics/Godeps/Godeps.json @@ -14,10 +14,6 @@ "ImportPath": "github.com/PuerkitoBio/urlesc", "Rev": "5bd2802263f21d8788851d5305584c82a5c75d7e" }, - { - "ImportPath": "github.com/davecgh/go-spew/spew", - "Rev": "782f4967f2dc4564575ca782fe2d04090b5faca8" - }, { "ImportPath": "github.com/emicklei/go-restful", "Rev": "ff4f55a206334ef123e4f79bbf348980da81ca46" @@ -306,10 +302,6 @@ "ImportPath": "k8s.io/api/storage/v1beta1", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, - { - "ImportPath": "k8s.io/apimachinery/pkg/api/equality", - "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" - }, { "ImportPath": "k8s.io/apimachinery/pkg/api/errors", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" @@ -354,10 +346,6 @@ "ImportPath": "k8s.io/apimachinery/pkg/conversion/queryparams", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, - { - "ImportPath": "k8s.io/apimachinery/pkg/conversion/unstructured", - "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" - }, { "ImportPath": "k8s.io/apimachinery/pkg/fields", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" @@ -410,10 +398,6 @@ "ImportPath": "k8s.io/apimachinery/pkg/util/clock", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, - { - "ImportPath": "k8s.io/apimachinery/pkg/util/diff", - "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" - }, { "ImportPath": "k8s.io/apimachinery/pkg/util/errors", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" diff --git a/staging/src/k8s.io/sample-apiserver/Godeps/Godeps.json b/staging/src/k8s.io/sample-apiserver/Godeps/Godeps.json index d08413cd72d..41c1836a982 100644 --- a/staging/src/k8s.io/sample-apiserver/Godeps/Godeps.json +++ b/staging/src/k8s.io/sample-apiserver/Godeps/Godeps.json @@ -666,10 +666,6 @@ "ImportPath": "k8s.io/apimachinery/pkg/conversion/queryparams", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, - { - "ImportPath": "k8s.io/apimachinery/pkg/conversion/unstructured", - "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" - }, { "ImportPath": "k8s.io/apimachinery/pkg/fields", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" diff --git a/staging/src/k8s.io/sample-controller/Godeps/Godeps.json b/staging/src/k8s.io/sample-controller/Godeps/Godeps.json index b6412341fc7..4368edf042c 100644 --- a/staging/src/k8s.io/sample-controller/Godeps/Godeps.json +++ b/staging/src/k8s.io/sample-controller/Godeps/Godeps.json @@ -342,10 +342,6 @@ "ImportPath": "k8s.io/api/storage/v1beta1", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, - { - "ImportPath": "k8s.io/apimachinery/pkg/api/equality", - "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" - }, { "ImportPath": "k8s.io/apimachinery/pkg/api/errors", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" @@ -382,10 +378,6 @@ "ImportPath": "k8s.io/apimachinery/pkg/conversion/queryparams", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, - { - "ImportPath": "k8s.io/apimachinery/pkg/conversion/unstructured", - "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" - }, { "ImportPath": "k8s.io/apimachinery/pkg/fields", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" From e7e313d6d23f4e4619944049a265270353c19cf8 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Mon, 13 Nov 2017 22:24:50 -0500 Subject: [PATCH 2/5] Scheme should support Unstructured conversion natively To reduce the impact of transitioning away from internal, make Unstructured conversion a natural part of the scheme. Convert and ConvertToVersion now automatically manage converting unstructured objects into versioned types as necessary. Alter the signature of Unstructured to make set possible. --- .../apis/meta/v1/unstructured/unstructured.go | 6 +- .../meta/v1/unstructured/unstructured_list.go | 29 ++- .../apimachinery/pkg/runtime/interfaces.go | 6 +- .../k8s.io/apimachinery/pkg/runtime/scheme.go | 120 +++++++++-- .../apimachinery/pkg/runtime/scheme_test.go | 197 ++++++++++++++---- .../apimachinery/pkg/runtime/testing/types.go | 105 ++++++++++ 6 files changed, 402 insertions(+), 61 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 c635f34409b..2ee7dad0e20 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 @@ -49,8 +49,6 @@ var _ runtime.Unstructured = &Unstructured{} func (obj *Unstructured) GetObjectKind() schema.ObjectKind { return obj } -func (obj *Unstructured) IsUnstructuredObject() {} - func (obj *Unstructured) IsList() bool { if obj.Object != nil { _, ok := obj.Object["items"] @@ -90,6 +88,10 @@ func (obj *Unstructured) UnstructuredContent() map[string]interface{} { return obj.Object } +func (obj *Unstructured) SetUnstructuredContent(content map[string]interface{}) { + obj.Object = content +} + // MarshalJSON ensures that the unstructured object produces proper // JSON when passed to Go's standard JSON library. func (u *Unstructured) MarshalJSON() ([]byte, error) { 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 4db4162ac29..57d78a09dcc 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 @@ -41,8 +41,6 @@ type UnstructuredList struct { func (u *UnstructuredList) GetObjectKind() schema.ObjectKind { return u } -func (u *UnstructuredList) IsUnstructuredObject() {} - func (u *UnstructuredList) IsList() bool { return true } func (u *UnstructuredList) EachListItem(fn func(runtime.Object) error) error { @@ -73,6 +71,33 @@ func (u *UnstructuredList) UnstructuredContent() map[string]interface{} { return out } +// SetUnstructuredContent obeys the conventions of List and keeps Items and the items +// array in sync. If items is not an array of objects in the incoming map, then any +// mismatched item will be removed. +func (obj *UnstructuredList) SetUnstructuredContent(content map[string]interface{}) { + obj.Object = content + if content == nil { + obj.Items = nil + return + } + items, ok := obj.Object["items"].([]interface{}) + if !ok || items == nil { + items = []interface{}{} + } + unstructuredItems := make([]Unstructured, 0, len(items)) + newItems := make([]interface{}, 0, len(items)) + for _, item := range items { + o, ok := item.(map[string]interface{}) + if !ok { + continue + } + unstructuredItems = append(unstructuredItems, Unstructured{Object: o}) + newItems = append(newItems, o) + } + obj.Items = unstructuredItems + obj.Object["items"] = newItems +} + func (u *UnstructuredList) DeepCopy() *UnstructuredList { if u == nil { return nil diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go b/staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go index c90eef5ac38..9d00f1650e9 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go @@ -233,13 +233,13 @@ type Object interface { // Unstructured objects store values as map[string]interface{}, with only values that can be serialized // to JSON allowed. type Unstructured interface { - // IsUnstructuredObject is a marker interface to allow objects that can be serialized but not introspected - // to bypass conversion. - IsUnstructuredObject() + Object // UnstructuredContent returns a non-nil, mutable map of the contents of this object. Values may be // []interface{}, map[string]interface{}, or any primitive type. Contents are typically serialized to // and from JSON. UnstructuredContent() map[string]interface{} + // SetUnstructuredContent updates the object content to match the provided map. + SetUnstructuredContent(map[string]interface{}) // IsList returns true if this type is a list or matches the list convention - has an array called "items". IsList() bool // EachListItem should pass a single item out of the list as an Object to the provided function. Any diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go index 3be721ca9fa..d8d84ca4017 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go @@ -338,7 +338,7 @@ func (s *Scheme) AddConversionFuncs(conversionFuncs ...interface{}) error { return nil } -// Similar to AddConversionFuncs, but registers conversion functions that were +// AddGeneratedConversionFuncs registers conversion functions that were // automatically generated. func (s *Scheme) AddGeneratedConversionFuncs(conversionFuncs ...interface{}) error { for _, f := range conversionFuncs { @@ -396,10 +396,67 @@ func (s *Scheme) Default(src Object) { // testing of conversion functions. Returns an error if the conversion isn't // possible. You can call this with types that haven't been registered (for example, // a to test conversion of types that are nested within registered types). The -// context interface is passed to the convertor. -// TODO: identify whether context should be hidden, or behind a formal context/scope -// interface +// context interface is passed to the convertor. Convert also supports Unstructured +// types and will convert them intelligently. func (s *Scheme) Convert(in, out interface{}, context interface{}) error { + unstructuredIn, okIn := in.(Unstructured) + unstructuredOut, okOut := out.(Unstructured) + switch { + case okIn && okOut: + // converting unstructured input to an unstructured output is a straight copy - unstructured + // is a "smart holder" and the contents are passed by reference between the two objects + unstructuredOut.SetUnstructuredContent(unstructuredIn.UnstructuredContent()) + return nil + + case okOut: + // if the output is an unstructured object, use the standard Go type to unstructured + // conversion. The object must not be internal. + obj, ok := in.(Object) + if !ok { + return fmt.Errorf("unable to convert object type %T to Unstructured, must be a runtime.Object", in) + } + gvk, unversioned, err := s.ObjectKind(obj) + if err != nil { + return err + } + + // if no conversion is necessary, convert immediately + if unversioned || gvk.Version != APIVersionInternal { + content, err := DefaultUnstructuredConverter.ToUnstructured(in) + if err != nil { + return err + } + unstructuredOut.SetUnstructuredContent(content) + return nil + } + + // attempt to convert the object to an external version first. + target, ok := context.(GroupVersioner) + if !ok { + return fmt.Errorf("unable to convert the internal object type %T to Unstructured without providing a preferred version to convert to", in) + } + // Convert is implicitly unsafe, so we don't need to perform a safe conversion + versioned, err := s.UnsafeConvertToVersion(obj, target) + if err != nil { + return err + } + content, err := DefaultUnstructuredConverter.ToUnstructured(versioned) + if err != nil { + return err + } + unstructuredOut.SetUnstructuredContent(content) + return nil + + case okIn: + // converting an unstructured object to any type is modeled by first converting + // the input to a versioned type, then running standard conversions + typed, err := s.unstructuredToTyped(unstructuredIn) + if err != nil { + return err + } + in = typed + } + flags, meta := s.generateConvertMeta(in) meta.Context = context if flags == 0 { @@ -408,8 +465,8 @@ func (s *Scheme) Convert(in, out interface{}, context interface{}) error { return s.converter.Convert(in, out, flags, meta) } -// Converts the given field label and value for an kind field selector from -// versioned representation to an unversioned one. +// ConvertFieldLabel alters the given field label and value for an kind field selector from +// versioned representation to an unversioned one or returns an error. func (s *Scheme) ConvertFieldLabel(version, kind, label, value string) (string, string, error) { if s.fieldLabelConversionFuncs[version] == nil { return DefaultMetaV1FieldSelectorConversion(label, value) @@ -439,15 +496,30 @@ func (s *Scheme) UnsafeConvertToVersion(in Object, target GroupVersioner) (Objec // convertToVersion handles conversion with an optional copy. func (s *Scheme) convertToVersion(copy bool, in Object, target GroupVersioner) (Object, error) { - // determine the incoming kinds with as few allocations as possible. - t := reflect.TypeOf(in) - if t.Kind() != reflect.Ptr { - return nil, fmt.Errorf("only pointer types may be converted: %v", t) - } - t = t.Elem() - if t.Kind() != reflect.Struct { - return nil, fmt.Errorf("only pointers to struct types may be converted: %v", t) + var t reflect.Type + + if u, ok := in.(Unstructured); ok { + typed, err := s.unstructuredToTyped(u) + if err != nil { + return nil, err + } + + in = typed + // unstructuredToTyped returns an Object, which must be a pointer to a struct. + t = reflect.TypeOf(in).Elem() + + } else { + // determine the incoming kinds with as few allocations as possible. + t = reflect.TypeOf(in) + if t.Kind() != reflect.Ptr { + return nil, fmt.Errorf("only pointer types may be converted: %v", t) + } + t = t.Elem() + if t.Kind() != reflect.Struct { + return nil, fmt.Errorf("only pointers to struct types may be converted: %v", t) + } } + kinds, ok := s.typeToGVK[t] if !ok || len(kinds) == 0 { return nil, NewNotRegisteredErrForType(t) @@ -463,7 +535,6 @@ func (s *Scheme) convertToVersion(copy bool, in Object, target GroupVersioner) ( } return copyAndSetTargetKind(copy, in, unversionedKind) } - return nil, NewNotRegisteredErrForTarget(t, target) } @@ -501,6 +572,25 @@ func (s *Scheme) convertToVersion(copy bool, in Object, target GroupVersioner) ( return out, nil } +// unstructuredToTyped attempts to transform an unstructured object to a typed +// object if possible. It will return an error if conversion is not possible, or the versioned +// Go form of the object. Note that this conversion will lose fields. +func (s *Scheme) unstructuredToTyped(in Unstructured) (Object, error) { + // the type must be something we recognize + gvks, _, err := s.ObjectKinds(in) + if err != nil { + return nil, err + } + typed, err := s.New(gvks[0]) + if err != nil { + return nil, err + } + if err := DefaultUnstructuredConverter.FromUnstructured(in.UnstructuredContent(), typed); err != nil { + return nil, fmt.Errorf("unable to convert unstructured object to %v: %v", gvks[0], err) + } + return typed, nil +} + // generateConvertMeta constructs the meta value we pass to Convert. func (s *Scheme) generateConvertMeta(in interface{}) (conversion.FieldMatchingFlags, *conversion.Meta) { return s.converter.DefaultMeta(reflect.TypeOf(in)) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go index 3e623f64217..7c460e26b0d 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go @@ -17,6 +17,7 @@ limitations under the License. package runtime_test import ( + "fmt" "reflect" "strings" "testing" @@ -126,8 +127,61 @@ func TestScheme(t *testing.T) { t.Errorf("Expected %v, got %v", e, a) } + // Test convert internal to unstructured + unstructuredObj := &runtimetesting.Unstructured{} + err = scheme.Convert(simple, unstructuredObj, nil) + if err == nil || !strings.Contains(err.Error(), "to Unstructured without providing a preferred version to convert to") { + t.Fatalf("Unexpected non-error: %v", err) + } + err = scheme.Convert(simple, unstructuredObj, schema.GroupVersion{Group: "test.group", Version: "testExternal"}) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if e, a := simple.TestString, unstructuredObj.Object["testString"].(string); e != a { + t.Errorf("Expected %v, got %v", e, a) + } + if e := unstructuredObj.GetObjectKind().GroupVersionKind(); !reflect.DeepEqual(e, schema.GroupVersionKind{Group: "test.group", Version: "testExternal", Kind: "Simple"}) { + t.Errorf("Unexpected object kind: %#v", e) + } + + // Test convert external to unstructured + unstructuredObj = &runtimetesting.Unstructured{} + err = scheme.Convert(external, unstructuredObj, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if e, a := simple.TestString, unstructuredObj.Object["testString"].(string); e != a { + t.Errorf("Expected %v, got %v", e, a) + } + if e := unstructuredObj.GetObjectKind().GroupVersionKind(); !reflect.DeepEqual(e, schema.GroupVersionKind{Group: "test.group", Version: "testExternal", Kind: "Simple"}) { + t.Errorf("Unexpected object kind: %#v", e) + } + + // Test convert unstructured to unstructured + uIn := &runtimetesting.Unstructured{Object: map[string]interface{}{ + "test": []interface{}{"other", "test"}, + }} + uOut := &runtimetesting.Unstructured{} + err = scheme.Convert(uIn, uOut, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if !reflect.DeepEqual(uIn.Object, uOut.Object) { + t.Errorf("Unexpected object contents: %#v", uOut.Object) + } + + // Test convert unstructured to structured + externalOut := &runtimetesting.ExternalSimple{} + err = scheme.Convert(unstructuredObj, externalOut, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if !reflect.DeepEqual(external, externalOut) { + t.Errorf("Unexpected object contents: %#v", externalOut) + } + // Encode and Convert should each have caused an increment. - if e, a := 2, internalToExternalCalls; e != a { + if e, a := 3, internalToExternalCalls; e != a { t.Errorf("Expected %v, got %v", e, a) } // DecodeInto and Decode should each have caused an increment because of a conversion @@ -367,6 +421,7 @@ func GetTestScheme() *runtime.Scheme { internalGV := schema.GroupVersion{Version: "__internal"} externalGV := schema.GroupVersion{Version: "v1"} alternateExternalGV := schema.GroupVersion{Group: "custom", Version: "v1"} + alternateInternalGV := schema.GroupVersion{Group: "custom", Version: "__internal"} differentExternalGV := schema.GroupVersion{Group: "other", Version: "v2"} s := runtime.NewScheme() @@ -380,10 +435,15 @@ func GetTestScheme() *runtime.Scheme { s.AddKnownTypeWithName(internalGV.WithKind("TestType3"), &runtimetesting.TestType1{}) s.AddKnownTypeWithName(externalGV.WithKind("TestType3"), &runtimetesting.ExternalTestType1{}) s.AddKnownTypeWithName(externalGV.WithKind("TestType4"), &runtimetesting.ExternalTestType1{}) + s.AddKnownTypeWithName(alternateInternalGV.WithKind("TestType3"), &runtimetesting.TestType1{}) s.AddKnownTypeWithName(alternateExternalGV.WithKind("TestType3"), &runtimetesting.ExternalTestType1{}) s.AddKnownTypeWithName(alternateExternalGV.WithKind("TestType5"), &runtimetesting.ExternalTestType1{}) s.AddKnownTypeWithName(differentExternalGV.WithKind("TestType1"), &runtimetesting.ExternalTestType1{}) s.AddUnversionedTypes(externalGV, &runtimetesting.UnversionedType{}) + + s.AddConversionFuncs(func(in *runtimetesting.TestType1, out *runtimetesting.ExternalTestType1, s conversion.Scope) { + out.A = in.A + }) return s } @@ -540,6 +600,28 @@ func TestConvertToVersion(t *testing.T) { gv: schema.GroupVersion{Version: "__internal"}, out: &runtimetesting.TestType1{A: "test"}, }, + // converts from unstructured to internal + { + scheme: GetTestScheme(), + in: &runtimetesting.Unstructured{Object: map[string]interface{}{ + "apiVersion": "custom/v1", + "kind": "TestType3", + "A": "test", + }}, + gv: schema.GroupVersion{Version: "__internal"}, + out: &runtimetesting.TestType1{A: "test"}, + }, + // converts from unstructured to external + { + scheme: GetTestScheme(), + in: &runtimetesting.Unstructured{Object: map[string]interface{}{ + "apiVersion": "custom/v1", + "kind": "TestType3", + "A": "test", + }}, + gv: schema.GroupVersion{Group: "custom", Version: "v1"}, + out: &runtimetesting.ExternalTestType1{MyWeirdCustomEmbeddedVersionKindField: runtimetesting.MyWeirdCustomEmbeddedVersionKindField{APIVersion: "custom/v1", ObjectKind: "TestType3"}, A: "test"}, + }, // prefers the best match { scheme: GetTestScheme(), @@ -711,51 +793,88 @@ func TestConvertToVersion(t *testing.T) { }, } for i, test := range testCases { - original := test.in.DeepCopyObject() - out, err := test.scheme.ConvertToVersion(test.in, test.gv) - switch { - case test.errFn != nil: - if !test.errFn(err) { - t.Errorf("%d: unexpected error: %v", i, err) + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + original := test.in.DeepCopyObject() + out, err := test.scheme.ConvertToVersion(test.in, test.gv) + switch { + case test.errFn != nil: + if !test.errFn(err) { + t.Fatalf("unexpected error: %v", err) + } + return + case err != nil: + t.Fatalf("unexpected error: %v", err) + } + if out == test.in { + t.Fatalf("ConvertToVersion should always copy out: %#v", out) } - continue - case err != nil: - t.Errorf("%d: unexpected error: %v", i, err) - continue - } - if out == test.in { - t.Errorf("%d: ConvertToVersion should always copy out: %#v", i, out) - continue - } - if test.same { - if !reflect.DeepEqual(original, test.in) { - t.Errorf("%d: unexpected mutation of input: %s", i, diff.ObjectReflectDiff(original, test.in)) - continue + if test.same { + if !reflect.DeepEqual(original, test.in) { + t.Fatalf("unexpected mutation of input: %s", diff.ObjectReflectDiff(original, test.in)) + } + if !reflect.DeepEqual(out, test.out) { + t.Fatalf("unexpected out: %s", diff.ObjectReflectDiff(out, test.out)) + } + unsafe, err := test.scheme.UnsafeConvertToVersion(test.in, test.gv) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !reflect.DeepEqual(unsafe, test.out) { + t.Fatalf("unexpected unsafe: %s", diff.ObjectReflectDiff(unsafe, test.out)) + } + if unsafe != test.in { + t.Fatalf("UnsafeConvertToVersion should return same object: %#v", unsafe) + } + return } if !reflect.DeepEqual(out, test.out) { - t.Errorf("%d: unexpected out: %s", i, diff.ObjectReflectDiff(out, test.out)) - continue + t.Fatalf("unexpected out: %s", diff.ObjectReflectDiff(out, test.out)) } - unsafe, err := test.scheme.UnsafeConvertToVersion(test.in, test.gv) - if err != nil { - t.Errorf("%d: unexpected error: %v", i, err) - continue + }) + } +} + +func TestConvert(t *testing.T) { + testCases := []struct { + scheme *runtime.Scheme + in runtime.Object + into runtime.Object + gv runtime.GroupVersioner + out runtime.Object + errFn func(error) bool + }{ + // converts from internal to unstructured, given a target version + { + scheme: GetTestScheme(), + in: &runtimetesting.TestType1{A: "test"}, + into: &runtimetesting.Unstructured{}, + out: &runtimetesting.Unstructured{Object: map[string]interface{}{ + "myVersionKey": "custom/v1", + "myKindKey": "TestType3", + "A": "test", + }}, + gv: schema.GroupVersion{Group: "custom", Version: "v1"}, + }, + } + for i, test := range testCases { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + err := test.scheme.Convert(test.in, test.into, test.gv) + switch { + case test.errFn != nil: + if !test.errFn(err) { + t.Fatalf("unexpected error: %v", err) + } + return + case err != nil: + t.Fatalf("unexpected error: %v", err) + return } - if !reflect.DeepEqual(unsafe, test.out) { - t.Errorf("%d: unexpected unsafe: %s", i, diff.ObjectReflectDiff(unsafe, test.out)) - continue + + if !reflect.DeepEqual(test.into, test.out) { + t.Fatalf("unexpected out: %s", diff.ObjectReflectDiff(test.into, test.out)) } - if unsafe != test.in { - t.Errorf("%d: UnsafeConvertToVersion should return same object: %#v", i, unsafe) - continue - } - continue - } - if !reflect.DeepEqual(out, test.out) { - t.Errorf("%d: unexpected out: %s", i, diff.ObjectReflectDiff(out, test.out)) - continue - } + }) } } 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 c051fb1d277..f7345db0caf 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/testing/types.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/testing/types.go @@ -17,8 +17,11 @@ limitations under the License. package testing import ( + "fmt" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/json" ) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -213,3 +216,105 @@ func (obj *MyWeirdCustomEmbeddedVersionKindField) GroupVersionKind() schema.Grou func (obj *TestType2) GetObjectKind() schema.ObjectKind { return schema.EmptyObjectKind } func (obj *ExternalTestType2) GetObjectKind() schema.ObjectKind { return schema.EmptyObjectKind } + +// +k8s:deepcopy-gen=false +type Unstructured struct { + // Object is a JSON compatible map with string, float, int, bool, []interface{}, or + // map[string]interface{} + // children. + Object map[string]interface{} +} + +var _ runtime.Unstructured = &Unstructured{} + +func (obj *Unstructured) GetObjectKind() schema.ObjectKind { return obj } + +func (obj *Unstructured) IsList() bool { + if obj.Object != nil { + _, ok := obj.Object["items"] + return ok + } + return false +} + +func (obj *Unstructured) EachListItem(fn func(runtime.Object) error) error { + if obj.Object == nil { + return fmt.Errorf("content is not a list") + } + field, ok := obj.Object["items"] + if !ok { + return fmt.Errorf("content is not a list") + } + items, ok := field.([]interface{}) + if !ok { + return nil + } + for _, item := range items { + child, ok := item.(map[string]interface{}) + if !ok { + return fmt.Errorf("items member is not an object") + } + if err := fn(&Unstructured{Object: child}); err != nil { + return err + } + } + return nil +} + +func (obj *Unstructured) UnstructuredContent() map[string]interface{} { + if obj.Object == nil { + obj.Object = make(map[string]interface{}) + } + return obj.Object +} + +func (obj *Unstructured) SetUnstructuredContent(content map[string]interface{}) { + obj.Object = content +} + +// MarshalJSON ensures that the unstructured object produces proper +// JSON when passed to Go's standard JSON library. +func (u *Unstructured) MarshalJSON() ([]byte, error) { + return json.Marshal(u.Object) +} + +// UnmarshalJSON ensures that the unstructured object properly decodes +// JSON when passed to Go's standard JSON library. +func (u *Unstructured) UnmarshalJSON(b []byte) error { + return json.Unmarshal(b, &u.Object) +} + +func (in *Unstructured) DeepCopyObject() runtime.Object { + return in.DeepCopy() +} + +func (in *Unstructured) DeepCopy() *Unstructured { + if in == nil { + return nil + } + out := new(Unstructured) + *out = *in + out.Object = runtime.DeepCopyJSON(in.Object) + return out +} + +func (u *Unstructured) GroupVersionKind() schema.GroupVersionKind { + apiVersion, ok := u.Object["apiVersion"].(string) + if !ok { + return schema.GroupVersionKind{} + } + gv, err := schema.ParseGroupVersion(apiVersion) + if err != nil { + return schema.GroupVersionKind{} + } + kind, ok := u.Object["kind"].(string) + if ok { + return gv.WithKind(kind) + } + return schema.GroupVersionKind{} +} + +func (u *Unstructured) SetGroupVersionKind(gvk schema.GroupVersionKind) { + u.Object["apiVersion"] = gvk.GroupVersion().String() + u.Object["kind"] = gvk.Kind +} From d77b95723c4fb67c87a0ec8c09d4054ae2c77ee1 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 14 Nov 2017 22:25:24 -0500 Subject: [PATCH 3/5] Scheme should provide ObjectTyper for Unstructured objects as well This will allow us to recognize unstructured objects in the absence of server side discovery info. --- .../meta/internalversion/register_test.go | 8 ++--- .../k8s.io/apimachinery/pkg/runtime/scheme.go | 26 ++++++++------ .../apimachinery/pkg/runtime/scheme_test.go | 17 +++++++++ .../pkg/runtime/serializer/json/json.go | 3 +- .../apimachinery/pkg/runtime/testing/BUILD | 1 + .../client-go/discovery/unstructured.go | 36 +++++-------------- 6 files changed, 48 insertions(+), 43 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/register_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/register_test.go index 8a6fefa4173..8116f807435 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/register_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/register_test.go @@ -59,11 +59,11 @@ func TestListOptions(t *testing.T) { } // verify kind registration - if gvk, unversioned, err := scheme.ObjectKind(in); err != nil || unversioned || gvk != metav1.SchemeGroupVersion.WithKind("ListOptions") { - t.Errorf("unexpected: %v %v %v", gvk, unversioned, err) + if gvks, unversioned, err := scheme.ObjectKinds(in); err != nil || unversioned || gvks[0] != metav1.SchemeGroupVersion.WithKind("ListOptions") { + t.Errorf("unexpected: %v %v %v", gvks[0], unversioned, err) } - if gvk, unversioned, err := scheme.ObjectKind(out); err != nil || unversioned || gvk != SchemeGroupVersion.WithKind("ListOptions") { - t.Errorf("unexpected: %v %v %v", gvk, unversioned, err) + if gvks, unversioned, err := scheme.ObjectKinds(out); err != nil || unversioned || gvks[0] != SchemeGroupVersion.WithKind("ListOptions") { + t.Errorf("unexpected: %v %v %v", gvks[0], unversioned, err) } actual = &metav1.ListOptions{} diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go index d8d84ca4017..08b7553810b 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go @@ -217,19 +217,22 @@ func (s *Scheme) AllKnownTypes() map[schema.GroupVersionKind]reflect.Type { return s.gvkToType } -// ObjectKind returns the group,version,kind of the go object and true if this object -// is considered unversioned, or an error if it's not a pointer or is unregistered. -func (s *Scheme) ObjectKind(obj Object) (schema.GroupVersionKind, bool, error) { - gvks, unversionedType, err := s.ObjectKinds(obj) - if err != nil { - return schema.GroupVersionKind{}, false, err - } - return gvks[0], unversionedType, nil -} - // ObjectKinds returns all possible group,version,kind of the go object, true if the // object is considered unversioned, or an error if it's not a pointer or is unregistered. func (s *Scheme) ObjectKinds(obj Object) ([]schema.GroupVersionKind, bool, error) { + // Unstructured objects are always considered to have their declared GVK + if _, ok := obj.(Unstructured); ok { + // we require that the GVK be populated in order to recognize the object + gvk := obj.GetObjectKind().GroupVersionKind() + if len(gvk.Kind) == 0 { + return nil, false, NewMissingKindErr("unstructured object has no kind") + } + if len(gvk.Version) == 0 { + return nil, false, NewMissingVersionErr("unstructured object has no version") + } + return []schema.GroupVersionKind{gvk}, false, nil + } + v, err := conversion.EnforcePtr(obj) if err != nil { return nil, false, err @@ -415,10 +418,11 @@ func (s *Scheme) Convert(in, out interface{}, context interface{}) error { if !ok { return fmt.Errorf("unable to convert object type %T to Unstructured, must be a runtime.Object", in) } - gvk, unversioned, err := s.ObjectKind(obj) + gvks, unversioned, err := s.ObjectKinds(obj) if err != nil { return err } + gvk := gvks[0] // if no conversion is necessary, convert immediately if unversioned || gvk.Version != APIVersionInternal { diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go index 7c460e26b0d..24743dcaecb 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go @@ -143,6 +143,9 @@ func TestScheme(t *testing.T) { if e := unstructuredObj.GetObjectKind().GroupVersionKind(); !reflect.DeepEqual(e, schema.GroupVersionKind{Group: "test.group", Version: "testExternal", Kind: "Simple"}) { t.Errorf("Unexpected object kind: %#v", e) } + if gvks, unversioned, err := scheme.ObjectKinds(unstructuredObj); err != nil || !reflect.DeepEqual(gvks[0], schema.GroupVersionKind{Group: "test.group", Version: "testExternal", Kind: "Simple"}) || unversioned { + t.Errorf("Scheme did not recognize unversioned: %v, %#v %t", err, gvks, unversioned) + } // Test convert external to unstructured unstructuredObj = &runtimetesting.Unstructured{} @@ -188,6 +191,20 @@ func TestScheme(t *testing.T) { if e, a := 2, externalToInternalCalls; e != a { t.Errorf("Expected %v, got %v", e, a) } + + // Verify that unstructured types must have V and K set + emptyObj := &runtimetesting.Unstructured{Object: make(map[string]interface{})} + if _, _, err := scheme.ObjectKinds(emptyObj); !runtime.IsMissingKind(err) { + t.Errorf("unexpected error: %v", err) + } + emptyObj.SetGroupVersionKind(schema.GroupVersionKind{Kind: "Test"}) + if _, _, err := scheme.ObjectKinds(emptyObj); !runtime.IsMissingVersion(err) { + t.Errorf("unexpected error: %v", err) + } + emptyObj.SetGroupVersionKind(schema.GroupVersionKind{Kind: "Test", Version: "v1"}) + if _, _, err := scheme.ObjectKinds(emptyObj); err != nil { + t.Errorf("unexpected error: %v", err) + } } func TestBadJSONRejection(t *testing.T) { diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go index ce3d77c2b3b..8a217f32e31 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go @@ -150,9 +150,10 @@ func (s *Serializer) Decode(originalData []byte, gvk *schema.GroupVersionKind, i } if into != nil { + _, isUnstructured := into.(runtime.Unstructured) types, _, err := s.typer.ObjectKinds(into) switch { - case runtime.IsNotRegisteredError(err): + case runtime.IsNotRegisteredError(err), isUnstructured: if err := jsoniter.ConfigFastest.Unmarshal(data, into); err != nil { return nil, actual, err } diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/testing/BUILD b/staging/src/k8s.io/apimachinery/pkg/runtime/testing/BUILD index a03585c8625..cee23e55778 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/testing/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/testing/BUILD @@ -16,6 +16,7 @@ go_library( deps = [ "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library", ], ) diff --git a/staging/src/k8s.io/client-go/discovery/unstructured.go b/staging/src/k8s.io/client-go/discovery/unstructured.go index ee72d668bcf..4a0fd643fa6 100644 --- a/staging/src/k8s.io/client-go/discovery/unstructured.go +++ b/staging/src/k8s.io/client-go/discovery/unstructured.go @@ -49,28 +49,22 @@ func NewUnstructuredObjectTyper(groupResources []*APIGroupResources) *Unstructur return dot } -// ObjectKind returns the group,version,kind of the provided object, or an error -// if the object in not runtime.Unstructured or has no group,version,kind -// information. -func (d *UnstructuredObjectTyper) ObjectKind(obj runtime.Object) (schema.GroupVersionKind, error) { - if _, ok := obj.(runtime.Unstructured); !ok { - return schema.GroupVersionKind{}, fmt.Errorf("type %T is invalid for dynamic object typer", obj) - } - - return obj.GetObjectKind().GroupVersionKind(), nil -} - // ObjectKinds returns a slice of one element with the group,version,kind of the // provided object, or an error if the object is not runtime.Unstructured or // has no group,version,kind information. unversionedType will always be false // because runtime.Unstructured object should always have group,version,kind // information set. func (d *UnstructuredObjectTyper) ObjectKinds(obj runtime.Object) (gvks []schema.GroupVersionKind, unversionedType bool, err error) { - gvk, err := d.ObjectKind(obj) - if err != nil { - return nil, false, err + if _, ok := obj.(runtime.Unstructured); !ok { + return nil, false, fmt.Errorf("type %T is invalid for dynamic object typer", obj) + } + gvk := obj.GetObjectKind().GroupVersionKind() + if len(gvk.Kind) == 0 { + return nil, false, runtime.NewMissingKindErr("unstructured object has no kind") + } + if len(gvk.Version) == 0 { + return nil, false, runtime.NewMissingVersionErr("unstructured object has no version") } - return []schema.GroupVersionKind{gvk}, false, nil } @@ -80,16 +74,4 @@ func (d *UnstructuredObjectTyper) Recognizes(gvk schema.GroupVersionKind) bool { return d.registered[gvk] } -// IsUnversioned returns false always because runtime.Unstructured objects -// should always have group,version,kind information set. ok will be true if the -// object's group,version,kind is api.Registry. -func (d *UnstructuredObjectTyper) IsUnversioned(obj runtime.Object) (unversioned bool, ok bool) { - gvk, err := d.ObjectKind(obj) - if err != nil { - return false, false - } - - return false, d.registered[gvk] -} - var _ runtime.ObjectTyper = &UnstructuredObjectTyper{} From 5519a7c5e917b951fbb3f14b9a38b57bab7311ac Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 16 Nov 2017 15:45:59 -0500 Subject: [PATCH 4/5] Make edit test use subtests to identify flake --- pkg/kubectl/cmd/edit_test.go | 164 +++++++++++++++++------------------ 1 file changed, 82 insertions(+), 82 deletions(-) diff --git a/pkg/kubectl/cmd/edit_test.go b/pkg/kubectl/cmd/edit_test.go index 4362826cc38..eef95fa5a26 100644 --- a/pkg/kubectl/cmd/edit_test.go +++ b/pkg/kubectl/cmd/edit_test.go @@ -193,94 +193,94 @@ func TestEdit(t *testing.T) { } for _, testcaseName := range testcases.List() { - t.Logf("Running testcase: %s", testcaseName) - i = 0 - name = testcaseName - testcase = EditTestCase{} - testcaseDir := filepath.Join("testdata", "edit", "testcase-"+name) - testcaseData, err := ioutil.ReadFile(filepath.Join(testcaseDir, "test.yaml")) - if err != nil { - t.Fatalf("%s: %v", name, err) - } - if err := yaml.Unmarshal(testcaseData, &testcase); err != nil { - t.Fatalf("%s: %v", name, err) - } - - f, tf, _, _ := cmdtesting.NewAPIFactory() - tf.Printer = &testPrinter{} - tf.UnstructuredClientForMappingFunc = func(mapping *meta.RESTMapping) (resource.RESTClient, error) { - versionedAPIPath := "" - if mapping.GroupVersionKind.Group == "" { - versionedAPIPath = "/api/" + mapping.GroupVersionKind.Version - } else { - versionedAPIPath = "/apis/" + mapping.GroupVersionKind.Group + "/" + mapping.GroupVersionKind.Version + t.Run(testcaseName, func(t *testing.T) { + i = 0 + name = testcaseName + testcase = EditTestCase{} + testcaseDir := filepath.Join("testdata", "edit", "testcase-"+name) + testcaseData, err := ioutil.ReadFile(filepath.Join(testcaseDir, "test.yaml")) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + if err := yaml.Unmarshal(testcaseData, &testcase); err != nil { + t.Fatalf("%s: %v", name, err) } - return &fake.RESTClient{ - VersionedAPIPath: versionedAPIPath, - NegotiatedSerializer: unstructuredSerializer, - Client: fake.CreateHTTPClient(reqResp), - }, nil - } - if len(testcase.Namespace) > 0 { - tf.Namespace = testcase.Namespace - } - tf.ClientConfig = defaultClientConfig() - tf.Command = "edit test cmd invocation" - buf := bytes.NewBuffer([]byte{}) - errBuf := bytes.NewBuffer([]byte{}) + f, tf, _, _ := cmdtesting.NewAPIFactory() + tf.Printer = &testPrinter{} + tf.UnstructuredClientForMappingFunc = func(mapping *meta.RESTMapping) (resource.RESTClient, error) { + versionedAPIPath := "" + if mapping.GroupVersionKind.Group == "" { + versionedAPIPath = "/api/" + mapping.GroupVersionKind.Version + } else { + versionedAPIPath = "/apis/" + mapping.GroupVersionKind.Group + "/" + mapping.GroupVersionKind.Version + } + return &fake.RESTClient{ + VersionedAPIPath: versionedAPIPath, + NegotiatedSerializer: unstructuredSerializer, + Client: fake.CreateHTTPClient(reqResp), + }, nil + } - var cmd *cobra.Command - switch testcase.Mode { - case "edit": - cmd = NewCmdEdit(f, buf, errBuf) - case "create": - cmd = NewCmdCreate(f, buf, errBuf) - cmd.Flags().Set("edit", "true") - case "edit-last-applied": - cmd = NewCmdApplyEditLastApplied(f, buf, errBuf) - default: - t.Errorf("%s: unexpected mode %s", name, testcase.Mode) - continue - } - if len(testcase.Filename) > 0 { - cmd.Flags().Set("filename", filepath.Join(testcaseDir, testcase.Filename)) - } - if len(testcase.Output) > 0 { - cmd.Flags().Set("output", testcase.Output) - } - if len(testcase.OutputPatch) > 0 { - cmd.Flags().Set("output-patch", testcase.OutputPatch) - } - if len(testcase.SaveConfig) > 0 { - cmd.Flags().Set("save-config", testcase.SaveConfig) - } + if len(testcase.Namespace) > 0 { + tf.Namespace = testcase.Namespace + } + tf.ClientConfig = defaultClientConfig() + tf.Command = "edit test cmd invocation" + buf := bytes.NewBuffer([]byte{}) + errBuf := bytes.NewBuffer([]byte{}) - cmdutil.BehaviorOnFatal(func(str string, code int) { - errBuf.WriteString(str) - if testcase.ExpectedExitCode != code { - t.Errorf("%s: expected exit code %d, got %d: %s", name, testcase.ExpectedExitCode, code, str) + var cmd *cobra.Command + switch testcase.Mode { + case "edit": + cmd = NewCmdEdit(f, buf, errBuf) + case "create": + cmd = NewCmdCreate(f, buf, errBuf) + cmd.Flags().Set("edit", "true") + case "edit-last-applied": + cmd = NewCmdApplyEditLastApplied(f, buf, errBuf) + default: + t.Fatalf("%s: unexpected mode %s", name, testcase.Mode) + } + if len(testcase.Filename) > 0 { + cmd.Flags().Set("filename", filepath.Join(testcaseDir, testcase.Filename)) + } + if len(testcase.Output) > 0 { + cmd.Flags().Set("output", testcase.Output) + } + if len(testcase.OutputPatch) > 0 { + cmd.Flags().Set("output-patch", testcase.OutputPatch) + } + if len(testcase.SaveConfig) > 0 { + cmd.Flags().Set("save-config", testcase.SaveConfig) + } + + cmdutil.BehaviorOnFatal(func(str string, code int) { + errBuf.WriteString(str) + if testcase.ExpectedExitCode != code { + t.Errorf("%s: expected exit code %d, got %d: %s", name, testcase.ExpectedExitCode, code, str) + } + }) + + cmd.Run(cmd, testcase.Args) + + stdout := buf.String() + stderr := errBuf.String() + + for _, s := range testcase.ExpectedStdout { + if !strings.Contains(stdout, s) { + t.Errorf("%s: expected to see '%s' in stdout\n\nstdout:\n%s\n\nstderr:\n%s", name, s, stdout, stderr) + } + } + for _, s := range testcase.ExpectedStderr { + if !strings.Contains(stderr, s) { + t.Errorf("%s: expected to see '%s' in stderr\n\nstdout:\n%s\n\nstderr:\n%s", name, s, stdout, stderr) + } + } + if i < len(testcase.Steps) { + t.Errorf("%s: saw %d steps, testcase included %d additional steps that were not exercised", name, i, len(testcase.Steps)-i) } }) - - cmd.Run(cmd, testcase.Args) - - stdout := buf.String() - stderr := errBuf.String() - - for _, s := range testcase.ExpectedStdout { - if !strings.Contains(stdout, s) { - t.Errorf("%s: expected to see '%s' in stdout\n\nstdout:\n%s\n\nstderr:\n%s", name, s, stdout, stderr) - } - } - for _, s := range testcase.ExpectedStderr { - if !strings.Contains(stderr, s) { - t.Errorf("%s: expected to see '%s' in stderr\n\nstdout:\n%s\n\nstderr:\n%s", name, s, stdout, stderr) - } - } - if i < len(testcase.Steps) { - t.Errorf("%s: saw %d steps, testcase included %d additional steps that were not exercised", name, i, len(testcase.Steps)-i) - } } } From f8cc69cafe34ae516de3c100c5e5a6333a1be9a6 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 16 Nov 2017 18:02:41 -0500 Subject: [PATCH 5/5] kubectl apply edit-last-applied should fail when version is missing If a user edits last-applied-configuration and it doesn't have an apiVersion or kind, the command should error out. Now that we check for kind and version on the unstructured object typer, the previously passing test now correctly fails. Add a new explicit failure test and make the existing test pass. --- .../0.request | 0 .../0.response | 21 +++++++++ .../1.request | 0 .../1.response | 35 +++++++++++++++ .../2.edited | 38 ++++++++++++++++ .../2.original | 36 ++++++++++++++++ .../3.edited | 41 ++++++++++++++++++ .../3.original | 40 +++++++++++++++++ .../4.request | 7 +++ .../4.response | 21 +++++++++ .../5.request | 7 +++ .../5.response | 35 +++++++++++++++ .../test.yaml | 43 +++++++++++++++++++ .../2.edited | 1 + .../4.request | 10 ++--- 15 files changed, 330 insertions(+), 5 deletions(-) create mode 100755 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/0.request create mode 100755 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/0.response create mode 100755 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/1.request create mode 100755 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/1.response create mode 100755 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/2.edited create mode 100755 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/2.original create mode 100644 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/3.edited create mode 100644 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/3.original create mode 100755 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/4.request create mode 100755 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/4.response create mode 100755 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/5.request create mode 100755 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/5.response create mode 100755 pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/test.yaml diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/0.request b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/0.request new file mode 100755 index 00000000000..e69de29bb2d diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/0.response b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/0.response new file mode 100755 index 00000000000..75d223aadcc --- /dev/null +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/0.response @@ -0,0 +1,21 @@ +{ + "kind": "ConfigMap", + "apiVersion": "v1", + "metadata": { + "name": "cm1", + "namespace": "myproject", + "selfLink": "/api/v1/namespaces/myproject/configmaps/cm1", + "uid": "cc08a131-3d6f-11e7-8ef0-c85b76034b7b", + "resourceVersion": "3518", + "creationTimestamp": "2017-05-20T15:20:03Z", + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"data\":{\"baz\":\"qux\",\"foo\":\"changed-value\",\"new-data\":\"new-value\",\"new-data2\":\"new-value\"},\"kind\":\"ConfigMap\",\"metadata\":{\"annotations\":{},\"name\":\"cm1\",\"namespace\":\"myproject\"}}\n" + } + }, + "data": { + "baz": "qux", + "foo": "changed-value", + "new-data": "new-value", + "new-data2": "new-value" + } +} \ No newline at end of file diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/1.request b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/1.request new file mode 100755 index 00000000000..e69de29bb2d diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/1.response b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/1.response new file mode 100755 index 00000000000..9703a466ab8 --- /dev/null +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/1.response @@ -0,0 +1,35 @@ +{ + "kind": "Service", + "apiVersion": "v1", + "metadata": { + "name": "svc1", + "namespace": "myproject", + "selfLink": "/api/v1/namespaces/myproject/services/svc1", + "uid": "d8b96f0b-3d6f-11e7-8ef0-c85b76034b7b", + "resourceVersion": "3525", + "creationTimestamp": "2017-05-20T15:20:24Z", + "labels": { + "app": "svc1", + "new-label": "foo" + }, + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "{\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"labels\":{\"app\":\"svc1\",\"new-label\":\"foo\"},\"name\":\"svc1\",\"namespace\":\"myproject\"},\"spec\":{\"ports\":[{\"name\":\"80\",\"port\":81,\"protocol\":\"TCP\",\"targetPort\":81}],\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n" + } + }, + "spec": { + "ports": [ + { + "name": "80", + "protocol": "TCP", + "port": 81, + "targetPort": 81 + } + ], + "clusterIP": "172.30.32.183", + "type": "ClusterIP", + "sessionAffinity": "None" + }, + "status": { + "loadBalancer": {} + } +} \ No newline at end of file diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/2.edited b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/2.edited new file mode 100755 index 00000000000..1769ec8faed --- /dev/null +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/2.edited @@ -0,0 +1,38 @@ +# Please edit the 'last-applied-configuration' annotations below. +# Lines beginning with a '#' will be ignored, and an empty file will abort the edit. +# +apiVersion: v1 +items: +- apiVersion: v1 + data: + baz: qux + foo: changed-value + new-data: new-value + new-data2: new-value + new-data3: newivalue + kind: ConfigMap + metadata: + annotations: {} + name: cm1 + namespace: myproject +- kind: Service + metadata: + annotations: {} + labels: + app: svc1 + new-label: foo + new-label2: foo2 + name: svc1 + namespace: myproject + spec: + ports: + - name: "80" + port: 82 + protocol: TCP + targetPort: 81 + sessionAffinity: None + type: ClusterIP + status: + loadBalancer: {} +kind: List +metadata: {} diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/2.original b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/2.original new file mode 100755 index 00000000000..82770327ca1 --- /dev/null +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/2.original @@ -0,0 +1,36 @@ +# Please edit the 'last-applied-configuration' annotations below. +# Lines beginning with a '#' will be ignored, and an empty file will abort the edit. +# +apiVersion: v1 +items: +- apiVersion: v1 + data: + baz: qux + foo: changed-value + new-data: new-value + new-data2: new-value + kind: ConfigMap + metadata: + annotations: {} + name: cm1 + namespace: myproject +- kind: Service + metadata: + annotations: {} + labels: + app: svc1 + new-label: foo + name: svc1 + namespace: myproject + spec: + ports: + - name: "80" + port: 81 + protocol: TCP + targetPort: 81 + sessionAffinity: None + type: ClusterIP + status: + loadBalancer: {} +kind: List +metadata: {} diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/3.edited b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/3.edited new file mode 100644 index 00000000000..e5a7d241ecf --- /dev/null +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/3.edited @@ -0,0 +1,41 @@ +# Please edit the 'last-applied-configuration' annotations below. +# Lines beginning with a '#' will be ignored, and an empty file will abort the edit. +# +# The edited file had a syntax error: error converting YAML to JSON: yaml: line 12: could not find expected ':' +# +apiVersion: v1 +items: +- apiVersion: v1 + data: + baz: qux + foo: changed-value + new-data: new-value + new-data2: new-value + new-data3: newivalue + kind: ConfigMap + metadata: + annotations: {} + name: cm1 + namespace: myproject +- kind: Service + apiVersion: v1 + metadata: + annotations: {} + labels: + app: svc1 + new-label: foo + new-label2: foo2 + name: svc1 + namespace: myproject + spec: + ports: + - name: "80" + port: 82 + protocol: TCP + targetPort: 81 + sessionAffinity: None + type: ClusterIP + status: + loadBalancer: {} +kind: List +metadata: {} diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/3.original b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/3.original new file mode 100644 index 00000000000..28250a20995 --- /dev/null +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/3.original @@ -0,0 +1,40 @@ +# Please edit the 'last-applied-configuration' annotations below. +# Lines beginning with a '#' will be ignored, and an empty file will abort the edit. +# +# The edited file had a syntax error: unable to get type info from the object "*unstructured.Unstructured": Object 'apiVersion' is missing in 'unstructured object has no version' +# +apiVersion: v1 +items: +- apiVersion: v1 + data: + baz: qux + foo: changed-value + new-data: new-value + new-data2: new-value + new-data3: newivalue + kind: ConfigMap + metadata: + annotations: {} + name: cm1 + namespace: myproject +- kind: Service + metadata: + annotations: {} + labels: + app: svc1 + new-label: foo + new-label2: foo2 + name: svc1 + namespace: myproject + spec: + ports: + - name: "80" + port: 82 + protocol: TCP + targetPort: 81 + sessionAffinity: None + type: ClusterIP + status: + loadBalancer: {} +kind: List +metadata: {} diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/4.request b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/4.request new file mode 100755 index 00000000000..162c97e69ec --- /dev/null +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/4.request @@ -0,0 +1,7 @@ +{ + "metadata": { + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"data\":{\"baz\":\"qux\",\"foo\":\"changed-value\",\"new-data\":\"new-value\",\"new-data2\":\"new-value\",\"new-data3\":\"newivalue\"},\"kind\":\"ConfigMap\",\"metadata\":{\"annotations\":{},\"name\":\"cm1\",\"namespace\":\"myproject\"}}\n" + } + } +} \ No newline at end of file diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/4.response b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/4.response new file mode 100755 index 00000000000..e2115785efb --- /dev/null +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/4.response @@ -0,0 +1,21 @@ +{ + "kind": "ConfigMap", + "apiVersion": "v1", + "metadata": { + "name": "cm1", + "namespace": "myproject", + "selfLink": "/api/v1/namespaces/myproject/configmaps/cm1", + "uid": "cc08a131-3d6f-11e7-8ef0-c85b76034b7b", + "resourceVersion": "3554", + "creationTimestamp": "2017-05-20T15:20:03Z", + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"data\":{\"baz\":\"qux\",\"foo\":\"changed-value\",\"new-data\":\"new-value\",\"new-data2\":\"new-value\",\"new-data3\":\"newivalue\"},\"kind\":\"ConfigMap\",\"metadata\":{\"annotations\":{},\"name\":\"cm1\",\"namespace\":\"myproject\"}}\n" + } + }, + "data": { + "baz": "qux", + "foo": "changed-value", + "new-data": "new-value", + "new-data2": "new-value" + } +} \ No newline at end of file diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/5.request b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/5.request new file mode 100755 index 00000000000..a4768bcf4dc --- /dev/null +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/5.request @@ -0,0 +1,7 @@ +{ + "metadata": { + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"labels\":{\"app\":\"svc1\",\"new-label\":\"foo\",\"new-label2\":\"foo2\"},\"name\":\"svc1\",\"namespace\":\"myproject\"},\"spec\":{\"ports\":[{\"name\":\"80\",\"port\":82,\"protocol\":\"TCP\",\"targetPort\":81}],\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n" + } + } +} \ No newline at end of file diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/5.response b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/5.response new file mode 100755 index 00000000000..c538b4333a4 --- /dev/null +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/5.response @@ -0,0 +1,35 @@ +{ + "kind": "Service", + "apiVersion": "v1", + "metadata": { + "name": "svc1", + "namespace": "myproject", + "selfLink": "/api/v1/namespaces/myproject/services/svc1", + "uid": "d8b96f0b-3d6f-11e7-8ef0-c85b76034b7b", + "resourceVersion": "3555", + "creationTimestamp": "2017-05-20T15:20:24Z", + "labels": { + "app": "svc1", + "new-label": "foo" + }, + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"labels\":{\"app\":\"svc1\",\"new-label\":\"foo\",\"new-label2\":\"foo2\"},\"name\":\"svc1\",\"namespace\":\"myproject\"},\"spec\":{\"ports\":[{\"name\":\"80\",\"port\":82,\"protocol\":\"TCP\",\"targetPort\":81}],\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n" + } + }, + "spec": { + "ports": [ + { + "name": "80", + "protocol": "TCP", + "port": 81, + "targetPort": 81 + } + ], + "clusterIP": "172.30.32.183", + "type": "ClusterIP", + "sessionAffinity": "None" + }, + "status": { + "loadBalancer": {} + } +} \ No newline at end of file diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/test.yaml b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/test.yaml new file mode 100755 index 00000000000..56b93abd66b --- /dev/null +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/test.yaml @@ -0,0 +1,43 @@ +description: if the user omits an API version, edit will fail +mode: edit-last-applied +args: +- configmaps/cm1 +- service/svc1 +namespace: "myproject" +expectedStdout: +- configmap "cm1" edited +- service "svc1" edited +expectedExitCode: 0 +steps: +- type: request + expectedMethod: GET + expectedPath: /api/v1/namespaces/myproject/configmaps/cm1 + expectedInput: 0.request + resultingStatusCode: 200 + resultingOutput: 0.response +- type: request + expectedMethod: GET + expectedPath: /api/v1/namespaces/myproject/services/svc1 + expectedInput: 1.request + resultingStatusCode: 200 + resultingOutput: 1.response +- type: edit + expectedInput: 2.original + resultingOutput: 2.edited +- type: edit + expectedInput: 3.original + resultingOutput: 3.edited +- type: request + expectedMethod: PATCH + expectedPath: /api/v1/namespaces/myproject/configmaps/cm1 + expectedContentType: application/merge-patch+json + expectedInput: 4.request + resultingStatusCode: 200 + resultingOutput: 4.response +- type: request + expectedMethod: PATCH + expectedPath: /api/v1/namespaces/myproject/services/svc1 + expectedContentType: application/merge-patch+json + expectedInput: 5.request + resultingStatusCode: 200 + resultingOutput: 5.response diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list/2.edited b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list/2.edited index 1769ec8faed..490d5fbb04d 100755 --- a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list/2.edited +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list/2.edited @@ -16,6 +16,7 @@ items: name: cm1 namespace: myproject - kind: Service + apiVersion: v1 metadata: annotations: {} labels: diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list/4.request b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list/4.request index 5c9c261898c..5c8be23d1b6 100755 --- a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list/4.request +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list/4.request @@ -1,7 +1,7 @@ { - "metadata": { - "annotations": { - "kubectl.kubernetes.io/last-applied-configuration": "{\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"labels\":{\"app\":\"svc1\",\"new-label\":\"foo\",\"new-label2\":\"foo2\"},\"name\":\"svc1\",\"namespace\":\"myproject\"},\"spec\":{\"ports\":[{\"name\":\"80\",\"port\":82,\"protocol\":\"TCP\",\"targetPort\":81}],\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n" - } - } + "metadata": { + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"labels\":{\"app\":\"svc1\",\"new-label\":\"foo\",\"new-label2\":\"foo2\"},\"name\":\"svc1\",\"namespace\":\"myproject\"},\"spec\":{\"ports\":[{\"name\":\"80\",\"port\":82,\"protocol\":\"TCP\",\"targetPort\":81}],\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n" + } + } } \ No newline at end of file