Skip to content

Commit 66e3e8c

Browse files
committed
Fix non-cache client list
This fixes the non-cache client list, which had a null pointer dereference (opts.Namespace), incorrect resource handling (need the resource for the items, not the list itself), and incorrect handling of nil list options (the underlying metav1.ListOptions needs to always be non-nil, even if empty).
1 parent c5c7e4d commit 66e3e8c

File tree

4 files changed

+41
-7
lines changed

4 files changed

+41
-7
lines changed

pkg/client/client.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,12 @@ func (c *client) List(ctx context.Context, opts *ListOptions, obj runtime.Object
142142
if err != nil {
143143
return err
144144
}
145+
namespace := ""
146+
if opts != nil {
147+
namespace = opts.Namespace
148+
}
145149
return r.Get().
146-
NamespaceIfScoped(opts.Namespace, r.isNamespaced()).
150+
NamespaceIfScoped(namespace, r.isNamespaced()).
147151
Resource(r.resource()).
148152
Body(obj).
149153
VersionedParams(opts.AsListOptions(), c.paramCodec).

pkg/client/client_cache.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package client
1818

1919
import (
2020
"reflect"
21+
"strings"
2122
"sync"
2223

2324
"k8s.io/apimachinery/pkg/api/meta"
@@ -49,11 +50,18 @@ type clientCache struct {
4950
}
5051

5152
// newResource maps obj to a Kubernetes Resource and constructs a client for that Resource.
53+
// If the object is a list, the resource represents the item's type instead.
5254
func (c *clientCache) newResource(obj runtime.Object) (*resourceMeta, error) {
5355
gvk, err := apiutil.GVKForObject(obj, c.scheme)
5456
if err != nil {
5557
return nil, err
5658
}
59+
60+
if strings.HasSuffix(gvk.Kind, "List") && meta.IsListType(obj) {
61+
// if this was a list, treat it as a request for the item's resource
62+
gvk.Kind = gvk.Kind[:len(gvk.Kind)-4]
63+
}
64+
5765
client, err := apiutil.RESTClientForGVK(gvk, c.config, c.codecs)
5866
if err != nil {
5967
return nil, err
@@ -65,7 +73,8 @@ func (c *clientCache) newResource(obj runtime.Object) (*resourceMeta, error) {
6573
return &resourceMeta{Interface: client, mapping: mapping, gvk: gvk}, nil
6674
}
6775

68-
// getResource returns a raw rest.Client for the given object type.
76+
// getResource returns the resource meta information for the given type of object.
77+
// If the object is a list, the resource represents the item's type instead.
6978
func (c *clientCache) getResource(obj runtime.Object) (*resourceMeta, error) {
7079
typ := reflect.TypeOf(obj)
7180

pkg/client/client_test.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@ package client_test
1818

1919
import (
2020
"context"
21-
2221
"fmt"
23-
2422
"sync/atomic"
2523

2624
. "github.com/onsi/ginkgo"
@@ -353,7 +351,26 @@ var _ = Describe("Client", func() {
353351

354352
Describe("List", func() {
355353
It("should fetch collection of objects", func() {
354+
By("creating an initial object")
355+
dep, err := clientset.AppsV1().Deployments(ns).Create(dep)
356+
Expect(err).NotTo(HaveOccurred())
357+
358+
cl, err := client.New(cfg, client.Options{})
359+
Expect(err).NotTo(HaveOccurred())
356360

361+
By("listing all objects of that type in the cluster")
362+
deps := &appsv1.DeploymentList{}
363+
Expect(cl.List(context.Background(), nil, deps)).NotTo(HaveOccurred())
364+
365+
Expect(deps.Items).NotTo(BeEmpty())
366+
hasDep := false
367+
for _, item := range deps.Items {
368+
if item.Name == dep.Name && item.Namespace == dep.Namespace {
369+
hasDep = true
370+
break
371+
}
372+
}
373+
Expect(hasDep).To(BeTrue())
357374
})
358375

359376
It("should return an empty list if there are no matching objects", func() {

pkg/client/interfaces.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,14 @@ func (o *ListOptions) SetFieldSelector(selRaw string) error {
127127
// This may mutate the Raw field.
128128
func (o *ListOptions) AsListOptions() *metav1.ListOptions {
129129
if o == nil {
130-
return nil
130+
return &metav1.ListOptions{}
131+
}
132+
if o.LabelSelector != nil {
133+
o.Raw.LabelSelector = o.LabelSelector.String()
134+
}
135+
if o.FieldSelector != nil {
136+
o.Raw.FieldSelector = o.FieldSelector.String()
131137
}
132-
o.Raw.LabelSelector = o.LabelSelector.String()
133-
o.Raw.FieldSelector = o.FieldSelector.String()
134138
return o.Raw
135139
}
136140

0 commit comments

Comments
 (0)