From 4b3d7f806a0a713a18766775602243db0a2f13bf Mon Sep 17 00:00:00 2001
From: Alvaro Aleman <alvaroaleman@users.noreply.github.com>
Date: Sat, 23 Nov 2024 15:14:02 -0500
Subject: [PATCH] :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.
---
 pkg/client/fake/client.go      | 16 +++++++++-------
 pkg/client/fake/client_test.go | 12 +++++++++---
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go
index 05b52d0c5f..cccf9ab622 100644
--- a/pkg/client/fake/client.go
+++ b/pkg/client/fake/client.go
@@ -598,22 +598,24 @@ 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 {
+		meta.SetList(obj, objs)
 		return nil
 	}
 
 	// 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
diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go
index a23489756a..80aca53b7a 100644
--- a/pkg/client/fake/client_test.go
+++ b/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() {