From 916622105daa9813ea2377becfa32d701624eb2b Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Thu, 22 Feb 2018 11:12:56 +0100 Subject: [PATCH 1/5] apimachinery: normal conversion code path for Unstructured in ConvertToVersion --- .../pkg/runtime/serializer/versioning/versioning.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go index b717fe8fe6..a9b13d21f6 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go @@ -167,10 +167,14 @@ func (c *codec) Decode(data []byte, defaultGVK *schema.GroupVersionKind, into ru // conversion if necessary. Unversioned objects (according to the ObjectTyper) are output as is. func (c *codec) Encode(obj runtime.Object, w io.Writer) error { switch obj.(type) { - case *runtime.Unknown, runtime.Unstructured: + case *runtime.Unknown: return c.encoder.Encode(obj, w) } + // Note: for runtime.Unstructured, the typer will return the GVK in the object and we will do conversion as normal below. + // For the normal runtime.Scheme converter the conversion will be a no-op for Unstructured. For CustomResources + // the conversion will actually do something. + gvks, isUnversioned, err := c.typer.ObjectKinds(obj) if err != nil { return err From 556f8ccbdde616cfdfa2c0df30d877ef1cb52ed5 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Thu, 22 Feb 2018 16:54:35 +0100 Subject: [PATCH 2/5] apimachinery duct tape: in versioning codec avoid conversion roundtrip for same GVK --- .../runtime/serializer/versioning/versioning.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go index a9b13d21f6..aedec75ee0 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go @@ -18,6 +18,7 @@ package versioning import ( "io" + "reflect" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -166,15 +167,21 @@ func (c *codec) Decode(data []byte, defaultGVK *schema.GroupVersionKind, into ru // Encode ensures the provided object is output in the appropriate group and version, invoking // conversion if necessary. Unversioned objects (according to the ObjectTyper) are output as is. func (c *codec) Encode(obj runtime.Object, w io.Writer) error { - switch obj.(type) { + switch obj := obj.(type) { case *runtime.Unknown: return c.encoder.Encode(obj, w) + case runtime.Unstructured: + // avoid conversion roundtrip if GVK is the right one already + objGVK := obj.GetObjectKind().GroupVersionKind() + targetGVK, ok := c.encodeVersion.KindForGroupVersionKinds([]schema.GroupVersionKind{objGVK}) + if !ok { + return runtime.NewNotRegisteredErrForTarget(reflect.TypeOf(obj).Elem(), c.encodeVersion) + } + if targetGVK == objGVK { + return c.encoder.Encode(obj, w) + } } - // Note: for runtime.Unstructured, the typer will return the GVK in the object and we will do conversion as normal below. - // For the normal runtime.Scheme converter the conversion will be a no-op for Unstructured. For CustomResources - // the conversion will actually do something. - gvks, isUnversioned, err := c.typer.ObjectKinds(obj) if err != nil { return err From ca9d1f728b2bc08b3f8cb35f3b32ffdf4c12d765 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Thu, 5 Apr 2018 13:47:44 +0200 Subject: [PATCH 3/5] apimachinery duct tape: handle empty unstructured GV in versioning codec gracefully --- .../pkg/runtime/serializer/versioning/versioning.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go index aedec75ee0..004e5c3371 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go @@ -171,8 +171,11 @@ func (c *codec) Encode(obj runtime.Object, w io.Writer) error { case *runtime.Unknown: return c.encoder.Encode(obj, w) case runtime.Unstructured: - // avoid conversion roundtrip if GVK is the right one already + // avoid conversion roundtrip if GVK is the right one already or is empty (yes, this is a hack, but the old behaviour we rely on in kubectl) objGVK := obj.GetObjectKind().GroupVersionKind() + if len(objGVK.Version) == 0 { + return c.encoder.Encode(obj, w) + } targetGVK, ok := c.encodeVersion.KindForGroupVersionKinds([]schema.GroupVersionKind{objGVK}) if !ok { return runtime.NewNotRegisteredErrForTarget(reflect.TypeOf(obj).Elem(), c.encodeVersion) From 0fc2c4844454be18961ad1b4446ef3c9540d20e6 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Thu, 22 Feb 2018 18:01:48 +0100 Subject: [PATCH 4/5] kubectl: add JSON fallback codec to cope with more strict stock versioning codec --- pkg/kubectl/cmd/apply_test.go | 6 +++--- pkg/kubectl/cmd/util/factory_client_access.go | 4 +++- pkg/kubectl/scheme/scheme.go | 3 ++- .../pkg/apis/meta/v1/unstructured/helpers.go | 15 +++++++++++++++ .../src/k8s.io/apimachinery/pkg/runtime/error.go | 8 ++++++++ .../runtime/serializer/versioning/versioning.go | 3 +-- 6 files changed, 32 insertions(+), 7 deletions(-) diff --git a/pkg/kubectl/cmd/apply_test.go b/pkg/kubectl/cmd/apply_test.go index 84a5b3da78..536329b7b0 100644 --- a/pkg/kubectl/cmd/apply_test.go +++ b/pkg/kubectl/cmd/apply_test.go @@ -178,7 +178,7 @@ func annotateRuntimeObject(t *testing.T, originalObj, currentObj runtime.Object, originalLabels := originalAccessor.GetLabels() originalLabels["DELETE_ME"] = "DELETE_ME" originalAccessor.SetLabels(originalLabels) - original, err := runtime.Encode(testapi.Default.Codec(), originalObj) + original, err := runtime.Encode(unstructured.JSONFallbackEncoder{Encoder: testapi.Default.Codec()}, originalObj) if err != nil { t.Fatal(err) } @@ -194,7 +194,7 @@ func annotateRuntimeObject(t *testing.T, originalObj, currentObj runtime.Object, } currentAnnotations[api.LastAppliedConfigAnnotation] = string(original) currentAccessor.SetAnnotations(currentAnnotations) - current, err := runtime.Encode(testapi.Default.Codec(), currentObj) + current, err := runtime.Encode(unstructured.JSONFallbackEncoder{Encoder: testapi.Default.Codec()}, currentObj) if err != nil { t.Fatal(err) } @@ -971,7 +971,7 @@ func TestUnstructuredIdempotentApply(t *testing.T) { initTestErrorHandler(t) serversideObject := readUnstructuredFromFile(t, filenameWidgetServerside) - serversideData, err := runtime.Encode(testapi.Default.Codec(), serversideObject) + serversideData, err := runtime.Encode(unstructured.JSONFallbackEncoder{Encoder: testapi.Default.Codec()}, serversideObject) if err != nil { t.Fatal(err) } diff --git a/pkg/kubectl/cmd/util/factory_client_access.go b/pkg/kubectl/cmd/util/factory_client_access.go index 831ec8b05a..e414c0fdc6 100644 --- a/pkg/kubectl/cmd/util/factory_client_access.go +++ b/pkg/kubectl/cmd/util/factory_client_access.go @@ -45,6 +45,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilflag "k8s.io/apiserver/pkg/util/flag" @@ -662,5 +663,6 @@ func InternalVersionDecoder() runtime.Decoder { } func InternalVersionJSONEncoder() runtime.Encoder { - return legacyscheme.Codecs.LegacyCodec(legacyscheme.Registry.EnabledVersions()...) + encoder := legacyscheme.Codecs.LegacyCodec(legacyscheme.Registry.EnabledVersions()...) + return unstructured.JSONFallbackEncoder{Encoder: encoder} } diff --git a/pkg/kubectl/scheme/scheme.go b/pkg/kubectl/scheme/scheme.go index d2e7ca978c..1140f60904 100644 --- a/pkg/kubectl/scheme/scheme.go +++ b/pkg/kubectl/scheme/scheme.go @@ -21,6 +21,7 @@ import ( "k8s.io/apimachinery/pkg/apimachinery/announced" "k8s.io/apimachinery/pkg/apimachinery/registered" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" @@ -50,5 +51,5 @@ var Versions = []schema.GroupVersion{} // DefaultJSONEncoder returns a default encoder for our scheme func DefaultJSONEncoder() runtime.Encoder { - return Codecs.LegacyCodec(Versions...) + return unstructured.JSONFallbackEncoder{Encoder: Codecs.LegacyCodec(Versions...)} } 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 08705ac841..d1fe97f00d 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 @@ -433,6 +433,21 @@ func (s unstructuredJSONScheme) decodeToList(data []byte, list *UnstructuredList return nil } +type JSONFallbackEncoder struct { + runtime.Encoder +} + +func (c JSONFallbackEncoder) Encode(obj runtime.Object, w io.Writer) error { + err := c.Encoder.Encode(obj, w) + if runtime.IsNotRegisteredError(err) { + switch obj.(type) { + case *Unstructured, *UnstructuredList: + return UnstructuredJSONScheme.Encode(obj, w) + } + } + return err +} + // UnstructuredObjectConverter is an ObjectConverter for use with // Unstructured objects. Since it has no schema or type information, // it will only succeed for no-op conversions. This is provided as a diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/error.go b/staging/src/k8s.io/apimachinery/pkg/runtime/error.go index 86b24840f0..7787966021 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/error.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/error.go @@ -41,10 +41,18 @@ func NewNotRegisteredErrForTarget(t reflect.Type, target GroupVersioner) error { return ¬RegisteredErr{t: t, target: target} } +func NewNotRegisteredGVKErrForTarget(gvk schema.GroupVersionKind, target GroupVersioner) error { + return ¬RegisteredErr{gvk: gvk, target: target} +} + func (k *notRegisteredErr) Error() string { if k.t != nil && k.target != nil { return fmt.Sprintf("%v is not suitable for converting to %q", k.t, k.target) } + nullGVK := schema.GroupVersionKind{} + if k.gvk != nullGVK && k.target != nil { + return fmt.Sprintf("%q is not suitable for converting to %q", k.gvk.GroupVersion(), k.target) + } if k.t != nil { return fmt.Sprintf("no kind is registered for the type %v", k.t) } diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go index 004e5c3371..48df6b5dd1 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go @@ -18,7 +18,6 @@ package versioning import ( "io" - "reflect" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -178,7 +177,7 @@ func (c *codec) Encode(obj runtime.Object, w io.Writer) error { } targetGVK, ok := c.encodeVersion.KindForGroupVersionKinds([]schema.GroupVersionKind{objGVK}) if !ok { - return runtime.NewNotRegisteredErrForTarget(reflect.TypeOf(obj).Elem(), c.encodeVersion) + return runtime.NewNotRegisteredGVKErrForTarget(objGVK, c.encodeVersion) } if targetGVK == objGVK { return c.encoder.Encode(obj, w) From 0fdd3d04d2251343b9998bb691e459dc3ca89111 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Thu, 22 Feb 2018 18:47:02 +0100 Subject: [PATCH 5/5] Update bazel --- pkg/kubectl/scheme/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/kubectl/scheme/BUILD b/pkg/kubectl/scheme/BUILD index cdd9333404..c22cb88089 100644 --- a/pkg/kubectl/scheme/BUILD +++ b/pkg/kubectl/scheme/BUILD @@ -39,6 +39,7 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/apimachinery/announced:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apimachinery/registered:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",