diff --git a/pkg/webhook/conversion/conversion.go b/pkg/webhook/conversion/conversion.go index f9bd5cb2db..05760cbbc4 100644 --- a/pkg/webhook/conversion/conversion.go +++ b/pkg/webhook/conversion/conversion.go @@ -174,7 +174,10 @@ func (wh *Webhook) convertViaHub(src, dst conversion.Convertible) error { // getHub returns an instance of the Hub for passed-in object's group/kind. func (wh *Webhook) getHub(obj runtime.Object) (conversion.Hub, error) { - gvks := objectGVKs(wh.scheme, obj) + gvks, err := objectGVKs(wh.scheme, obj) + if err != nil { + return nil, err + } if len(gvks) == 0 { return nil, fmt.Errorf("error retrieving gvks for object : %v", obj) } @@ -223,7 +226,10 @@ func (wh *Webhook) allocateDstObject(apiVersion, kind string) (runtime.Object, e func IsConvertible(scheme *runtime.Scheme, obj runtime.Object) (bool, error) { var hubs, spokes, nonSpokes []runtime.Object - gvks := objectGVKs(scheme, obj) + gvks, err := objectGVKs(scheme, obj) + if err != nil { + return false, err + } if len(gvks) == 0 { return false, fmt.Errorf("error retrieving gvks for object : %v", obj) } @@ -273,18 +279,27 @@ func IsConvertible(scheme *runtime.Scheme, obj runtime.Object) (bool, error) { } // objectGVKs returns all (Group,Version,Kind) for the Group/Kind of given object. -func objectGVKs(scheme *runtime.Scheme, obj runtime.Object) []schema.GroupVersionKind { - var gvks []schema.GroupVersionKind - - objGVK := obj.GetObjectKind().GroupVersionKind() +func objectGVKs(scheme *runtime.Scheme, obj runtime.Object) ([]schema.GroupVersionKind, error) { + // NB: we should not use `obj.GetObjectKind().GroupVersionKind()` to get the + // GVK here, since it is parsed from apiVersion and kind fields and it may + // return empty GVK if obj is an uninitialized object. + objGVKs, _, err := scheme.ObjectKinds(obj) + if err != nil { + return nil, err + } + if len(objGVKs) != 1 { + return nil, fmt.Errorf("expect to get only one GVK for %v", obj) + } + objGVK := objGVKs[0] knownTypes := scheme.AllKnownTypes() + var gvks []schema.GroupVersionKind for gvk := range knownTypes { if objGVK.GroupKind() == gvk.GroupKind() { gvks = append(gvks, gvk) } } - return gvks + return gvks, nil } // PartialImplementationError represents an error due to partial conversion diff --git a/pkg/webhook/conversion/conversion_test.go b/pkg/webhook/conversion/conversion_test.go index 1f2e1dc0e3..64ca7b575d 100644 --- a/pkg/webhook/conversion/conversion_test.go +++ b/pkg/webhook/conversion/conversion_test.go @@ -29,6 +29,7 @@ import ( appsv1beta1 "k8s.io/api/apps/v1beta1" apix "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" kscheme "k8s.io/client-go/kubernetes/scheme" @@ -312,6 +313,27 @@ var _ = Describe("IsConvertible", func() { Expect(jobsv3.AddToScheme(scheme)).To(Succeed()) }) + It("should not error for uninitialized types", func() { + obj := &jobsv2.ExternalJob{} + + ok, err := IsConvertible(scheme, obj) + Expect(err).NotTo(HaveOccurred()) + Expect(ok).To(BeTrue()) + }) + + It("should not error for unstructured types", func() { + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "ExternalJob", + "apiVersion": "jobs.testprojects.kb.io/v2", + }, + } + + ok, err := IsConvertible(scheme, obj) + Expect(err).NotTo(HaveOccurred()) + Expect(ok).To(BeTrue()) + }) + It("should return true for convertible types", func() { obj := &jobsv2.ExternalJob{ TypeMeta: metav1.TypeMeta{