From d77b95723c4fb67c87a0ec8c09d4054ae2c77ee1 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 14 Nov 2017 22:25:24 -0500 Subject: [PATCH] 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{}