:fakeclient: Don't return items on invalid selector

Currently, if a List call to the fake client references an invalid
fieldSelector, we will return an error and put all items into the passed
List. Avoid putting anything into the passed list if the selector is
invalid.

Author: alvaroaleman

pkg/client/fake/client.go

@@ -598,22 +598,23 @@ func (c *fakeClient) List(ctx context.Context, obj client.ObjectList, opts ...cl
 	if err != nil {
 		return err
 	}
-	zero(obj)
-	if err := json.Unmarshal(j, obj); err != nil {
+	objCopy := obj.DeepCopyObject().(client.ObjectList)
+	zero(objCopy)
+	if err := json.Unmarshal(j, objCopy); err != nil {
+		return err
+	}
+
+	objs, err := meta.ExtractList(objCopy)
+	if err != nil {
 		return err
 	}
 
 	if listOpts.LabelSelector == nil && listOpts.FieldSelector == nil {
-		return nil
+		return meta.SetList(obj, objs)
 	}
 
 	// If we're here, either a label or field selector are specified (or both), so before we return
 	// the list we must filter it. If both selectors are set, they are ANDed.
-	objs, err := meta.ExtractList(obj)
-	if err != nil {
-		return err
-	}
-
 	filteredList, err := c.filterList(objs, gvk, listOpts.LabelSelector, listOpts.FieldSelector)
 	if err != nil {
 		return err

pkg/client/fake/client_test.go

@@ -1319,16 +1319,20 @@ var _ = Describe("Fake client", func() {
 					listOpts := &client.ListOptions{
 						FieldSelector: fields.OneTermEqualSelector("key", "val"),
 					}
-					err := cl.List(context.Background(), &corev1.ConfigMapList{}, listOpts)
+					list := &corev1.ConfigMapList{}
+					err := cl.List(context.Background(), list, listOpts)
 					Expect(err).To(HaveOccurred())
+					Expect(list.Items).To(BeEmpty())
 				})
 
 				It("errors when there's no Index matching the field name", func() {
 					listOpts := &client.ListOptions{
 						FieldSelector: fields.OneTermEqualSelector("spec.paused", "false"),
 					}
-					err := cl.List(context.Background(), &appsv1.DeploymentList{}, listOpts)
+					list := &appsv1.DeploymentList{}
+					err := cl.List(context.Background(), list, listOpts)
 					Expect(err).To(HaveOccurred())
+					Expect(list.Items).To(BeEmpty())
 				})
 
 				It("errors when field selector uses two requirements", func() {
@@ -1337,8 +1341,10 @@ var _ = Describe("Fake client", func() {
 							fields.OneTermEqualSelector("spec.replicas", "1"),
 							fields.OneTermEqualSelector("spec.strategy.type", string(appsv1.RecreateDeploymentStrategyType)),
 						)}
-					err := cl.List(context.Background(), &appsv1.DeploymentList{}, listOpts)
+					list := &appsv1.DeploymentList{}
+					err := cl.List(context.Background(), list, listOpts)
 					Expect(err).To(HaveOccurred())
+					Expect(list.Items).To(BeEmpty())
 				})
 
 				It("returns two deployments that match the only field selector requirement", func() {