Simplify cache.objectTypeForListObject

This changeset streamlines and simplifies the logic inside
objectTypeForListObject. This method is used to retrieve from a List
object, its respective actual resource GVK and a new instance of the
object.

In the previous iteration, the use of reflection was complicated to
follow and looking at the fields in the struct passed in, instead of
using the information we already have in the Scheme to create a new
instance of the expected object and return.

In addition to all of the above, we're also now handling
PartialObjectMetadataList(s).

Signed-off-by: Vince Prignano <[email protected]>

Author: vincepri

pkg/cache/informer_cache.go

@@ -19,10 +19,10 @@ package cache
 import (
 	"context"
 	"fmt"
-	"reflect"
 	"strings"
 
 	apimeta "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"
@@ -97,36 +97,28 @@ func (ip *informerCache) objectTypeForListObject(list client.ObjectList) (*schem
 		return nil, nil, err
 	}
 
-	// we need the non-list GVK, so chop off the "List" from the end of the kind
-	if strings.HasSuffix(gvk.Kind, "List") && apimeta.IsListType(list) {
-		gvk.Kind = gvk.Kind[:len(gvk.Kind)-4]
-	}
+	// We need the non-list GVK, so chop off the "List" from the end of the kind.
+	gvk.Kind = strings.TrimSuffix(gvk.Kind, "List")
 
-	_, isUnstructured := list.(*unstructured.UnstructuredList)
-	var cacheTypeObj runtime.Object
-	if isUnstructured {
+	// Handle unstructured.UnstructuredList.
+	if _, isUnstructured := list.(*unstructured.UnstructuredList); isUnstructured {
 		u := &unstructured.Unstructured{}
 		u.SetGroupVersionKind(gvk)
-		cacheTypeObj = u
-	} else {
-		itemsPtr, err := apimeta.GetItemsPtr(list)
-		if err != nil {
-			return nil, nil, err
-		}
-		// http://knowyourmeme.com/memes/this-is-fine
-		elemType := reflect.Indirect(reflect.ValueOf(itemsPtr)).Type().Elem()
-		if elemType.Kind() != reflect.Ptr {
-			elemType = reflect.PtrTo(elemType)
-		}
-
-		cacheTypeValue := reflect.Zero(elemType)
-		var ok bool
-		cacheTypeObj, ok = cacheTypeValue.Interface().(runtime.Object)
-		if !ok {
-			return nil, nil, fmt.Errorf("cannot get cache for %T, its element %T is not a runtime.Object", list, cacheTypeValue.Interface())
-		}
+		return &gvk, u, nil
+	}
+	// Handle metav1.PartialObjectMetadataList.
+	if _, isPartialObjectMetadata := list.(*metav1.PartialObjectMetadataList); isPartialObjectMetadata {
+		pom := &metav1.PartialObjectMetadata{}
+		pom.SetGroupVersionKind(gvk)
+		return &gvk, pom, nil
 	}
 
+	// Any other list type should have a corresponding non-list type registered
+	// in the scheme. Use that to create a new instance of the non-list type.
+	cacheTypeObj, err := ip.scheme.New(gvk)
+	if err != nil {
+		return nil, nil, err
+	}
 	return &gvk, cacheTypeObj, nil
 }
 

pkg/cache/informer_cache_unit_test.go

@@ -21,6 +21,7 @@ import (
 	. "github.com/onsi/gomega"
 
 	corev1 "k8s.io/api/core/v1"
+	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"
@@ -55,7 +56,21 @@ var _ = Describe("ip.objectTypeForListObject", func() {
 		referenceUnstructured := &unstructured.Unstructured{}
 		referenceUnstructured.SetGroupVersionKind(*gvk)
 		Expect(obj).To(Equal(referenceUnstructured))
+	})
+
+	It("should find the object type for partial object metadata lists", func() {
+		partialList := &metav1.PartialObjectMetadataList{}
+		partialList.APIVersion = ("v1")
+		partialList.Kind = "PodList"
 
+		gvk, obj, err := ip.objectTypeForListObject(partialList)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(gvk.Group).To(Equal(""))
+		Expect(gvk.Version).To(Equal("v1"))
+		Expect(gvk.Kind).To(Equal("Pod"))
+		referencePartial := &metav1.PartialObjectMetadata{}
+		referencePartial.SetGroupVersionKind(*gvk)
+		Expect(obj).To(Equal(referencePartial))
 	})
 
 	It("should find the object type of a list with a slice of literals items field", func() {
@@ -64,9 +79,8 @@ var _ = Describe("ip.objectTypeForListObject", func() {
 		Expect(gvk.Group).To(Equal(""))
 		Expect(gvk.Version).To(Equal("v1"))
 		Expect(gvk.Kind).To(Equal("Pod"))
-		var referencePod *corev1.Pod
+		referencePod := &corev1.Pod{}
 		Expect(obj).To(Equal(referencePod))
-
 	})
 
 	It("should find the object type of a list with a slice of pointers items field", func() {
@@ -88,7 +102,7 @@ var _ = Describe("ip.objectTypeForListObject", func() {
 			Expect(gvk.Group).To(Equal(itemPointerSliceTypeGroupName))
 			Expect(gvk.Version).To(Equal(itemPointerSliceTypeVersion))
 			Expect(gvk.Kind).To(Equal("UnconventionalListType"))
-			var referenceObject *controllertest.UnconventionalListType
+			referenceObject := &controllertest.UnconventionalListType{}
 			Expect(obj).To(Equal(referenceObject))
 		})
 	})