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 Nov 23, 2024
1 parent b88f351 commit 4b3d7f8
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 10 deletions.
16 changes: 9 additions & 7 deletions pkg/client/fake/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Check failure on line 613 in pkg/client/fake/client.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `meta.SetList` is not checked (errcheck)

Check failure on line 613 in pkg/client/fake/client.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `meta.SetList` is not checked (errcheck)
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
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 4b3d7f8

Please sign in to comment.