diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index 3458e4960d..4ae74b883e 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -56,9 +56,9 @@ type fakeClient struct { scheme *runtime.Scheme restMapper meta.RESTMapper - // indexes maps each GroupVersionResource (GVR) to the indexes registered for that GVR. + // indexes maps each GroupVersionKind (GVK) to the indexes registered for that GVK. // The inner map maps from index name to IndexerFunc. - indexes map[schema.GroupVersionResource]map[string]client.IndexerFunc + indexes map[schema.GroupVersionKind]map[string]client.IndexerFunc schemeWriteLock sync.Mutex } @@ -102,9 +102,9 @@ type ClientBuilder struct { initRuntimeObjects []runtime.Object objectTracker testing.ObjectTracker - // indexes maps each GroupVersionResource (GVR) to the indexes registered for that GVR. + // indexes maps each GroupVersionKind (GVK) to the indexes registered for that GVK. // The inner map maps from index name to IndexerFunc. - indexes map[schema.GroupVersionResource]map[string]client.IndexerFunc + indexes map[schema.GroupVersionKind]map[string]client.IndexerFunc } // WithScheme sets this builder's internal scheme. @@ -147,27 +147,40 @@ func (f *ClientBuilder) WithObjectTracker(ot testing.ObjectTracker) *ClientBuild return f } -// WithIndex can be optionally used to register an index with name `name` and indexer `indexer` for -// API objects of GroupVersionResource `gvr` in the fake client. -// It can be invoked multiple times, both with different GroupVersionResource or the same one. -// Invoking WithIndex twice with the same `name` and `gvr` will panic. -func (f *ClientBuilder) WithIndex(gvr schema.GroupVersionResource, name string, indexer client.IndexerFunc) *ClientBuilder { +// WithIndex can be optionally used to register an index with name `field` and indexer `extractValue` +// for API objects of the same GroupVersionKind (GVK) as `obj` in the fake client. +// It can be invoked multiple times, both with objects of the same GVK or different ones. +// Invoking WithIndex twice with the same `field` and GVK (via `obj`) arguments will panic. +// WithIndex retrieves the GVK of `obj` using the scheme registered via WithScheme if +// WithScheme was previously invoked, the default scheme otherwise. +func (f *ClientBuilder) WithIndex(obj runtime.Object, field string, extractValue client.IndexerFunc) *ClientBuilder { + objScheme := f.scheme + if objScheme == nil { + objScheme = scheme.Scheme + } + + gvk, err := apiutil.GVKForObject(obj, objScheme) + if err != nil { + panic(err) + } + // If this is the first index being registered, we initialize the map storing all the indexes. if f.indexes == nil { - f.indexes = make(map[schema.GroupVersionResource]map[string]client.IndexerFunc) + f.indexes = make(map[schema.GroupVersionKind]map[string]client.IndexerFunc) } - // If this is the first index being registered for the input GroupVersionResource, we initialize - // the map storing the indexes for that GroupVersionResource. - if f.indexes[gvr] == nil { - f.indexes[gvr] = make(map[string]client.IndexerFunc) + // If this is the first index being registered for the GroupVersionKind of `obj`, we initialize + // the map storing the indexes for that GroupVersionKind. + if f.indexes[gvk] == nil { + f.indexes[gvk] = make(map[string]client.IndexerFunc) } - if _, nameAlreadyTaken := f.indexes[gvr][name]; nameAlreadyTaken { - panic(fmt.Errorf("indexer conflict: index name %s is already registered for GroupVersionResource %v", name, gvr)) + if _, fieldAlreadyIndexed := f.indexes[gvk][field]; fieldAlreadyIndexed { + panic(fmt.Errorf("indexer conflict: field %s for GroupVersionKind %v is already indexed", + field, gvk)) } - f.indexes[gvr][name] = indexer + f.indexes[gvk][field] = extractValue return f } @@ -469,7 +482,7 @@ func (c *fakeClient) List(ctx context.Context, obj client.ObjectList, opts ...cl return err } - filteredList, err := c.filterList(objs, gvr, listOpts.LabelSelector, listOpts.FieldSelector) + filteredList, err := c.filterList(objs, gvk, listOpts.LabelSelector, listOpts.FieldSelector) if err != nil { return err } @@ -477,7 +490,7 @@ func (c *fakeClient) List(ctx context.Context, obj client.ObjectList, opts ...cl return meta.SetList(obj, filteredList) } -func (c *fakeClient) filterList(list []runtime.Object, gvr schema.GroupVersionResource, ls labels.Selector, fs fields.Selector) ([]runtime.Object, error) { +func (c *fakeClient) filterList(list []runtime.Object, gvk schema.GroupVersionKind, ls labels.Selector, fs fields.Selector) ([]runtime.Object, error) { // Filter the objects with the label selector filteredList := list if ls != nil { @@ -490,7 +503,7 @@ func (c *fakeClient) filterList(list []runtime.Object, gvr schema.GroupVersionRe // Filter the result of the previous pass with the field selector if fs != nil { - objsFilteredByField, err := c.filterWithFields(filteredList, gvr, fs) + objsFilteredByField, err := c.filterWithFields(filteredList, gvk, fs) if err != nil { return nil, err } @@ -500,7 +513,7 @@ func (c *fakeClient) filterList(list []runtime.Object, gvr schema.GroupVersionRe return filteredList, nil } -func (c *fakeClient) filterWithFields(list []runtime.Object, gvr schema.GroupVersionResource, fs fields.Selector) ([]runtime.Object, error) { +func (c *fakeClient) filterWithFields(list []runtime.Object, gvk schema.GroupVersionKind, fs fields.Selector) ([]runtime.Object, error) { // We only allow filtering on the basis of a single field to ensure consistency with the // behavior of the cache reader (which we're faking here). fieldKey, fieldVal, requiresExact := selector.RequiresExactMatch(fs) @@ -510,18 +523,14 @@ func (c *fakeClient) filterWithFields(list []runtime.Object, gvr schema.GroupVer } // Field selection is mimicked via indexes, so there's no sane answer this function can give - // if there are no indexes registered for the GroupVersionResource of the objects in the list. - indexes, listGVRHasIndexes := c.indexes[gvr] - if !listGVRHasIndexes { - return nil, fmt.Errorf("List on GroupVersionResource %v specifies field selector, but no "+ - "indexes for that GroupResourceVersion are defined", gvr) - } - - indexExtractor, found := indexes[fieldKey] - if !found { - return nil, fmt.Errorf("no index with name %s was registered", fieldKey) + // if there are no indexes registered for the GroupVersionKind of the objects in the list. + indexes := c.indexes[gvk] + if len(indexes) == 0 || indexes[fieldKey] == nil { + return nil, fmt.Errorf("List on GroupVersionKind %v specifies selector on field %s, but no "+ + "index with name %s has been registered for GroupVersionKind %v", gvk, fieldKey, fieldKey, gvk) } + indexExtractor := indexes[fieldKey] filteredList := make([]runtime.Object, 0, len(list)) for _, obj := range list { if c.objMatchesFieldSelector(obj, indexExtractor, fieldVal) { diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go index cc8afae2b5..570cd744ad 100644 --- a/pkg/client/fake/client_test.go +++ b/pkg/client/fake/client_test.go @@ -28,7 +28,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes/fake" appsv1 "k8s.io/api/apps/v1" @@ -100,7 +99,7 @@ var _ = Describe("Fake client", func() { } }) - AssertClientWOIndexBehavior := func() { + AssertClientWithoutIndexBehavior := func() { It("should be able to Get", func() { By("Getting a deployment") namespacedName := types.NamespacedName{ @@ -981,7 +980,7 @@ var _ = Describe("Fake client", func() { WithObjects(dep, dep2, cm). Build() }) - AssertClientWOIndexBehavior() + AssertClientWithoutIndexBehavior() }) Context("with given scheme", func() { @@ -996,7 +995,7 @@ var _ = Describe("Fake client", func() { WithLists(&appsv1.DeploymentList{Items: []appsv1.Deployment{*dep, *dep2}}). Build() }) - AssertClientWOIndexBehavior() + AssertClientWithoutIndexBehavior() }) Context("with Indexes", func() { @@ -1022,23 +1021,18 @@ var _ = Describe("Fake client", func() { return []string{string(dep.Spec.Strategy.Type)} } - var ( - depGVR schema.GroupVersionResource - cb *ClientBuilder - ) - + var cb *ClientBuilder BeforeEach(func() { cb = NewClientBuilder(). WithObjects(dep, dep2, cm). - WithIndex(depGVR, "spec.replicas", depReplicasIndexer) - depGVR = appsv1.SchemeGroupVersion.WithResource("deployments") + WithIndex(&appsv1.Deployment{}, "spec.replicas", depReplicasIndexer) }) Context("client has just one Index", func() { BeforeEach(func() { cl = cb.Build() }) Context("behavior that doesn't use an Index", func() { - AssertClientWOIndexBehavior() + AssertClientWithoutIndexBehavior() }) Context("filtered List using field selector", func() { @@ -1120,11 +1114,11 @@ var _ = Describe("Fake client", func() { Context("client has two Indexes", func() { BeforeEach(func() { - cl = cb.WithIndex(depGVR, "spec.strategy.type", depStrategyTypeIndexer).Build() + cl = cb.WithIndex(&appsv1.Deployment{}, "spec.strategy.type", depStrategyTypeIndexer).Build() }) Context("behavior that doesn't use an Index", func() { - AssertClientWOIndexBehavior() + AssertClientWithoutIndexBehavior() }) Context("filtered List using field selector", func() { @@ -1229,16 +1223,14 @@ var _ = Describe("Fake client", func() { }) var _ = Describe("Fake client builder", func() { - It("panics when an index with the same name and GroupVersionResource is registered twice", func() { - // We need any realistic GroupVersionResource, the choice of deployments is arbitrary. - gvr := appsv1.SchemeGroupVersion.WithResource("deployments") - - cb := NewClientBuilder().WithIndex(gvr, + It("panics when an index with the same name and GroupVersionKind is registered twice", func() { + // We need any realistic GroupVersionKind, the choice of apps/v1 Deployment is arbitrary. + cb := NewClientBuilder().WithIndex(&appsv1.Deployment{}, "test-name", func(client.Object) []string { return nil }) Expect(func() { - cb.WithIndex(gvr, + cb.WithIndex(&appsv1.Deployment{}, "test-name", func(client.Object) []string { return []string{"foo"} }) }).To(Panic())