Add indexes to fake client

Allow registration of indexes into the fake client to allow usage
of field selectors when doing a List. The indexing is done lazily
by doing a normal list and then filtering the results by computing
the index value for each list item.

To enable the main change for this commit, some refactorings are
performed: an internal and unexported function that checks
whether a field selector is in the form key=val or key==val is made
internal and exported to be shared between real and faked code to
ensure loyalty. Unit tests for it are added.

Co-authored-by: Paul Eichler <peichler@anynines.com>

Author: matteoolivi

pkg/cache/internal/cache_reader.go

@@ -23,12 +23,11 @@ import (
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
-	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/selection"
 	"k8s.io/client-go/tools/cache"
+	"sigs.k8s.io/controller-runtime/pkg/internal/field/selector"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -116,7 +115,7 @@ func (c *CacheReader) List(_ context.Context, out client.ObjectList, opts ...cli
 	case listOpts.FieldSelector != nil:
 		// TODO(directxman12): support more complicated field selectors by
 		// combining multiple indices, GetIndexers, etc
-		field, val, requiresExact := requiresExactMatch(listOpts.FieldSelector)
+		field, val, requiresExact := selector.RequiresExactMatch(listOpts.FieldSelector)
 		if !requiresExact {
 			return fmt.Errorf("non-exact field matches are not supported by the cache")
 		}
@@ -186,19 +185,6 @@ func objectKeyToStoreKey(k client.ObjectKey) string {
 	return k.Namespace + "/" + k.Name
 }
 
-// requiresExactMatch checks if the given field selector is of the form `k=v` or `k==v`.
-func requiresExactMatch(sel fields.Selector) (field, val string, required bool) {
-	reqs := sel.Requirements()
-	if len(reqs) != 1 {
-		return "", "", false
-	}
-	req := reqs[0]
-	if req.Operator != selection.Equals && req.Operator != selection.DoubleEquals {
-		return "", "", false
-	}
-	return req.Field, req.Value, true
-}
-
 // FieldIndexName constructs the name of the index over the given field,
 // for use with an indexer.
 func FieldIndexName(field string) string {

pkg/client/fake/client.go

@@ -30,13 +30,16 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/fields"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	utilrand "k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/testing"
+	"sigs.k8s.io/controller-runtime/pkg/internal/field/selector"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
@@ -49,9 +52,14 @@ type versionedTracker struct {
 }
 
 type fakeClient struct {
-	tracker         versionedTracker
-	scheme          *runtime.Scheme
-	restMapper      meta.RESTMapper
+	tracker    versionedTracker
+	scheme     *runtime.Scheme
+	restMapper meta.RESTMapper
+
+	// indexes maps each GroupVersionResource (GVR) to the indexes registered for that GVR.
+	// The inner map maps from index name to IndexerFunc.
+	indexes map[schema.GroupVersionResource]map[string]client.IndexerFunc
+
 	schemeWriteLock sync.Mutex
 }
 
@@ -93,6 +101,10 @@ type ClientBuilder struct {
 	initLists          []client.ObjectList
 	initRuntimeObjects []runtime.Object
 	objectTracker      testing.ObjectTracker
+
+	// indexes maps each GroupVersionResource (GVR) to the indexes registered for that GVR.
+	// The inner map maps from index name to IndexerFunc.
+	indexes map[schema.GroupVersionResource]map[string]client.IndexerFunc
 }
 
 // WithScheme sets this builder's internal scheme.
@@ -135,6 +147,31 @@ 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 {
+	// 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)
+	}
+
+	// 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 _, nameAlreadyTaken := f.indexes[gvr][name]; nameAlreadyTaken {
+		panic(fmt.Errorf("indexer conflict: index name %s is already registered for GroupVersionResource %v", name, gvr))
+	}
+
+	f.indexes[gvr][name] = indexer
+
+	return f
+}
+
 // Build builds and returns a new fake client.
 func (f *ClientBuilder) Build() client.WithWatch {
 	if f.scheme == nil {
@@ -171,6 +208,7 @@ func (f *ClientBuilder) Build() client.WithWatch {
 		tracker:    tracker,
 		scheme:     f.scheme,
 		restMapper: f.restMapper,
+		indexes:    f.indexes,
 	}
 }
 
@@ -420,21 +458,92 @@ func (c *fakeClient) List(ctx context.Context, obj client.ObjectList, opts ...cl
 		return err
 	}
 
-	if listOpts.LabelSelector != nil {
-		objs, err := meta.ExtractList(obj)
+	if listOpts.LabelSelector == nil && listOpts.FieldSelector == nil {
+		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, gvr, listOpts.LabelSelector, listOpts.FieldSelector)
+	if err != nil {
+		return err
+	}
+
+	return meta.SetList(obj, filteredList)
+}
+
+func (c *fakeClient) filterList(list []runtime.Object, gvr schema.GroupVersionResource, ls labels.Selector, fs fields.Selector) ([]runtime.Object, error) {
+	// Filter the objects with the label selector
+	filteredList := list
+	if ls != nil {
+		objsFilteredByLabel, err := objectutil.FilterWithLabels(list, ls)
 		if err != nil {
-			return err
+			return nil, err
 		}
-		filteredObjs, err := objectutil.FilterWithLabels(objs, listOpts.LabelSelector)
+		filteredList = objsFilteredByLabel
+	}
+
+	// Filter the result of the previous pass with the field selector
+	if fs != nil {
+		objsFilteredByField, err := c.filterWithFields(filteredList, gvr, fs)
 		if err != nil {
-			return err
+			return nil, err
 		}
-		err = meta.SetList(obj, filteredObjs)
-		if err != nil {
-			return err
+		filteredList = objsFilteredByField
+	}
+
+	return filteredList, nil
+}
+
+func (c *fakeClient) filterWithFields(list []runtime.Object, gvr schema.GroupVersionResource, 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)
+	if !requiresExact {
+		return nil, fmt.Errorf("field selector %s is not in one of the two supported forms \"key==val\" or \"key=val\"",
+			fs)
+	}
+
+	// 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)
+	}
+
+	filteredList := make([]runtime.Object, 0, len(list))
+	for _, obj := range list {
+		if c.objMatchesFieldSelector(obj, indexExtractor, fieldVal) {
+			filteredList = append(filteredList, obj)
 		}
 	}
-	return nil
+	return filteredList, nil
+}
+
+func (c *fakeClient) objMatchesFieldSelector(o runtime.Object, extractIndex client.IndexerFunc, val string) bool {
+	obj, isClientObject := o.(client.Object)
+	if !isClientObject {
+		panic(fmt.Errorf("expected object %v to be of type client.Object, but it's not", o))
+	}
+
+	for _, extractedVal := range extractIndex(obj) {
+		if extractedVal == val {
+			return true
+		}
+	}
+
+	return false
 }
 
 func (c *fakeClient) Scheme() *runtime.Scheme {