Skip to content

Commit

Permalink
:fakeclient: Don't return items on invalid selector
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
alvaroaleman committed Jan 7, 2025
1 parent b88f351 commit c49e843
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 10 deletions.
15 changes: 8 additions & 7 deletions pkg/client/fake/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -599,21 +599,22 @@ func (c *fakeClient) List(ctx context.Context, obj client.ObjectList, opts ...cl
return err
}
zero(obj)
if err := json.Unmarshal(j, obj); err != nil {
objCopy := obj.DeepCopyObject().(client.ObjectList)
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
Expand Down
12 changes: 9 additions & 3 deletions pkg/client/fake/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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() {
Expand Down

0 comments on commit c49e843

Please sign in to comment.