Update List implementations to use new signature

The fake client now uses similar GVK detection code as the other
implementations and no longer relies on the Raw object having a
TypeMeta.

Author: grantr

pkg/cache/cache_test.go

@@ -113,7 +113,7 @@ var _ = Describe("Informer Cache", func() {
 		It("should be able to list objects that haven't been watched previously", func() {
 			By("listing all services in the cluster")
 			listObj := &kcorev1.ServiceList{}
-			Expect(informerCache.List(context.Background(), nil, listObj)).To(Succeed())
+			Expect(informerCache.List(context.Background(), listObj)).To(Succeed())
 
 			By("verifying that the returned list contains the Kubernetes service")
 			// NB: kubernetes default service is automatically created in testenv.
@@ -143,8 +143,10 @@ var _ = Describe("Informer Cache", func() {
 			By("listing pods with a particular label")
 			// NB: each pod has a "test-label": <pod-name>
 			out := kcorev1.PodList{}
-			Expect(informerCache.List(context.Background(), client.InNamespace(testNamespaceTwo).
-				MatchingLabels(map[string]string{"test-label": "test-pod-2"}), &out)).To(Succeed())
+			Expect(informerCache.List(context.Background(), &out,
+				client.InNamespace(testNamespaceTwo),
+				client.MatchingLabels(map[string]string{"test-label": "test-pod-2"}),
+			)).To(Succeed())
 
 			By("verifying the returned pods have the correct label")
 			Expect(out.Items).NotTo(BeEmpty())
@@ -161,8 +163,9 @@ var _ = Describe("Informer Cache", func() {
 			// NB: each pod has a "test-label": <pod-name>
 			out := kcorev1.PodList{}
 			labels := map[string]string{"test-label": "test-pod-2"}
-			Expect(informerCache.List(context.Background(),
-				client.MatchingLabels(labels), &out)).To(Succeed())
+			Expect(informerCache.List(context.Background(), &out,
+				client.MatchingLabels(labels),
+			)).To(Succeed())
 
 			By("verifying multiple pods with the same label in different namespaces are returned")
 			Expect(out.Items).NotTo(BeEmpty())
@@ -177,9 +180,9 @@ var _ = Describe("Informer Cache", func() {
 		It("should be able to list objects by namespace", func() {
 			By("listing pods in test-namespace-1")
 			listObj := &kcorev1.PodList{}
-			Expect(informerCache.List(context.Background(),
+			Expect(informerCache.List(context.Background(), listObj,
 				client.InNamespace(testNamespaceOne),
-				listObj)).To(Succeed())
+			)).To(Succeed())
 
 			By("verifying that the returned pods are in test-namespace-1")
 			Expect(listObj.Items).NotTo(BeEmpty())
@@ -317,9 +320,9 @@ var _ = Describe("Informer Cache", func() {
 
 			By("listing Pods with restartPolicyOnFailure")
 			listObj := &kcorev1.PodList{}
-			Expect(informer.List(context.Background(),
+			Expect(informer.List(context.Background(), listObj,
 				client.MatchingField("spec.restartPolicy", "OnFailure"),
-				listObj)).To(Succeed())
+			)).To(Succeed())
 
 			By("verifying that the returned pods have correct restart policy")
 			Expect(listObj.Items).NotTo(BeEmpty())

pkg/cache/informer_cache.go

@@ -57,7 +57,7 @@ func (ip *informerCache) Get(ctx context.Context, key client.ObjectKey, out runt
 }
 
 // List implements Reader
-func (ip *informerCache) List(ctx context.Context, opts *client.ListOptions, out runtime.Object) error {
+func (ip *informerCache) List(ctx context.Context, out runtime.Object, opts ...client.ListOptionFunc) error {
 	itemsPtr, err := apimeta.GetItemsPtr(out)
 	if err != nil {
 		return nil
@@ -87,7 +87,7 @@ func (ip *informerCache) List(ctx context.Context, opts *client.ListOptions, out
 		return err
 	}
 
-	return cache.Reader.List(ctx, opts, out)
+	return cache.Reader.List(ctx, out, opts...)
 }
 
 // GetInformerForKind returns the informer for the GroupVersionKind

pkg/cache/informertest/fake_cache.go

@@ -136,6 +136,6 @@ func (c *FakeInformers) Get(ctx context.Context, key client.ObjectKey, obj runti
 }
 
 // List implements Cache
-func (c *FakeInformers) List(ctx context.Context, opts *client.ListOptions, list runtime.Object) error {
+func (c *FakeInformers) List(ctx context.Context, list runtime.Object, opts ...client.ListOptionFunc) error {
 	return nil
 }

pkg/cache/internal/cache_reader.go

@@ -86,32 +86,35 @@ func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out runtime.O
 }
 
 // List lists items out of the indexer and writes them to out
-func (c *CacheReader) List(ctx context.Context, opts *client.ListOptions, out runtime.Object) error {
+func (c *CacheReader) List(ctx context.Context, out runtime.Object, opts ...client.ListOptionFunc) error {
 	var objs []interface{}
 	var err error
 
-	if opts != nil && opts.FieldSelector != nil {
+	listOpts := client.ListOptions{}
+	listOpts.ApplyOptions(opts)
+
+	if listOpts.FieldSelector != nil {
 		// TODO(directxman12): support more complicated field selectors by
 		// combining multiple indicies, GetIndexers, etc
-		field, val, requiresExact := requiresExactMatch(opts.FieldSelector)
+		field, val, requiresExact := requiresExactMatch(listOpts.FieldSelector)
 		if !requiresExact {
 			return fmt.Errorf("non-exact field matches are not supported by the cache")
 		}
 		// list all objects by the field selector.  If this is namespaced and we have one, ask for the
 		// namespaced index key.  Otherwise, ask for the non-namespaced variant by using the fake "all namespaces"
 		// namespace.
-		objs, err = c.indexer.ByIndex(FieldIndexName(field), KeyToNamespacedKey(opts.Namespace, val))
-	} else if opts != nil && opts.Namespace != "" {
-		objs, err = c.indexer.ByIndex(cache.NamespaceIndex, opts.Namespace)
+		objs, err = c.indexer.ByIndex(FieldIndexName(field), KeyToNamespacedKey(listOpts.Namespace, val))
+	} else if listOpts.Namespace != "" {
+		objs, err = c.indexer.ByIndex(cache.NamespaceIndex, listOpts.Namespace)
 	} else {
 		objs = c.indexer.List()
 	}
 	if err != nil {
 		return err
 	}
 	var labelSel labels.Selector
-	if opts != nil && opts.LabelSelector != nil {
-		labelSel = opts.LabelSelector
+	if listOpts.LabelSelector != nil {
+		labelSel = listOpts.LabelSelector
 	}
 
 	outItems, err := c.getListItems(objs, labelSel)

pkg/client/fake/client.go

@@ -19,7 +19,9 @@ package fake
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"os"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -77,10 +79,23 @@ func (c *fakeClient) Get(ctx context.Context, key client.ObjectKey, obj runtime.
 	return err
 }
 
-func (c *fakeClient) List(ctx context.Context, opts *client.ListOptions, list runtime.Object) error {
-	gvk := opts.Raw.TypeMeta.GroupVersionKind()
+func (c *fakeClient) List(ctx context.Context, obj runtime.Object, opts ...client.ListOptionFunc) error {
+	gvk, err := apiutil.GVKForObject(obj, scheme.Scheme)
+	if err != nil {
+		return err
+	}
+
+	if !strings.HasSuffix(gvk.Kind, "List") {
+		return fmt.Errorf("non-list type %T (kind %q) passed as output", obj, gvk)
+	}
+	// we need the non-list GVK, so chop off the "List" from the end of the kind
+	gvk.Kind = gvk.Kind[:len(gvk.Kind)-4]
+
+	listOpts := client.ListOptions{}
+	listOpts.ApplyOptions(opts)
+
 	gvr, _ := meta.UnsafeGuessKindToResource(gvk)
-	o, err := c.tracker.List(gvr, gvk, opts.Namespace)
+	o, err := c.tracker.List(gvr, gvk, listOpts.Namespace)
 	if err != nil {
 		return err
 	}
@@ -89,7 +104,7 @@ func (c *fakeClient) List(ctx context.Context, opts *client.ListOptions, list ru
 		return err
 	}
 	decoder := scheme.Codecs.UniversalDecoder()
-	_, _, err = decoder.Decode(j, nil, list)
+	_, _, err = decoder.Decode(j, nil, obj)
 	return err
 }
 

pkg/client/fake/client_test.go

@@ -17,15 +17,12 @@ limitations under the License.
 package fake
 
 import (
-	"encoding/json"
-
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -69,22 +66,11 @@ var _ = Describe("Fake client", func() {
 
 	It("should be able to List", func() {
 		By("Listing all deployments in a namespace")
-		list := &metav1.List{}
-		err := cl.List(nil, &client.ListOptions{
-			Namespace: "ns1",
-			Raw: &metav1.ListOptions{
-				TypeMeta: metav1.TypeMeta{
-					APIVersion: "apps/v1",
-					Kind:       "Deployment",
-				},
-			},
-		}, list)
+		list := &appsv1.DeploymentList{}
+		err := cl.List(nil, list, client.InNamespace("ns1"))
 		Expect(err).To(BeNil())
 		Expect(list.Items).To(HaveLen(1))
-		j, err := json.Marshal(dep)
-		Expect(err).To(BeNil())
-		expectedDep := runtime.RawExtension{Raw: j}
-		Expect(list.Items).To(ConsistOf(expectedDep))
+		Expect(list.Items).To(ConsistOf(*dep))
 	})
 
 	It("should be able to Create", func() {
@@ -140,16 +126,8 @@ var _ = Describe("Fake client", func() {
 		Expect(err).To(BeNil())
 
 		By("Listing all deployments in the namespace")
-		list := &metav1.List{}
-		err = cl.List(nil, &client.ListOptions{
-			Namespace: "ns1",
-			Raw: &metav1.ListOptions{
-				TypeMeta: metav1.TypeMeta{
-					APIVersion: "apps/v1",
-					Kind:       "Deployment",
-				},
-			},
-		}, list)
+		list := &appsv1.DeploymentList{}
+		err = cl.List(nil, list, client.InNamespace("ns1"))
 		Expect(err).To(BeNil())
 		Expect(list.Items).To(HaveLen(0))
 	})