✨ Allow configuring more granular cache filtering

This change implements most of the [cache options design][0], in
particular it is now possible to:
* Configure Namespaces per type
* Configure filtering per namespace

What is still missing is to allow configuring namespace-level default
settings that will be used for all namespaces not explicitly configured.
There is nothing in the way of doing that as a follow-up.

The implementation slightly derivates from the design document in that
the filter settings in `ByGVK` do not use the `Config` struct but
instead are directly embedded. This allows us to not break compatibility
and is more ergonomic to use.

The implementation is based on a new and internal per-type delegating
"meta cache".

[0]: https://github.com/kubernetes-sigs/controller-runtime/blob/main/designs/cache_options.md

examples/scratch-env/go.sum

@@ -1251,6 +1251,7 @@ golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u0
 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
 golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
 golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e/go.mod h1:Kr81I6Kryrl9sr8s2FK3vxD90NdsKWRuOIl2O4CvYbA=
+golang.org/x/exp v0.0.0-20220827204233-334a2380cb91 h1:tnebWN09GYg9OLPss1KXj8txwZc6X6uMr6VFdcGNbHw=
 golang.org/x/exp v0.0.0-20220827204233-334a2380cb91/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE=
 golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs=
 golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=

go.mod

@@ -9,12 +9,14 @@ require (
 	github.com/go-logr/logr v1.2.4
 	github.com/go-logr/zapr v1.2.4
 	github.com/google/go-cmp v0.5.9
+	github.com/google/gofuzz v1.2.0
 	github.com/onsi/ginkgo/v2 v2.11.0
 	github.com/onsi/gomega v1.27.10
 	github.com/prometheus/client_golang v1.16.0
 	github.com/prometheus/client_model v0.4.0
 	go.uber.org/goleak v1.2.1
 	go.uber.org/zap v1.24.0
+	golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e
 	golang.org/x/sys v0.10.0
 	gomodules.xyz/jsonpatch/v2 v2.3.0
 	k8s.io/api v0.28.0-beta.0
@@ -40,7 +42,6 @@ require (
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/google/gnostic-models v0.6.8 // indirect
-	github.com/google/gofuzz v1.2.0 // indirect
 	github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 // indirect
 	github.com/google/uuid v1.3.0 // indirect
 	github.com/imdario/mergo v0.3.6 // indirect

go.sum

@@ -128,6 +128,8 @@ go.uber.org/zap v1.24.0/go.mod h1:2kMP+WWQ8aoFoedH3T2sq6iJ2yDWpHbP0f6MQbS9Gkg=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e h1:+WEEuIdZHnUeJJmEUjyYC2gfUMj69yZXw17EnHg/otA=
+golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e/go.mod h1:Kr81I6Kryrl9sr8s2FK3vxD90NdsKWRuOIl2O4CvYbA=
 golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=

pkg/cache/cache.go

@@ -22,15 +22,16 @@ import (
 	"net/http"
 	"time"
 
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	toolscache "k8s.io/client-go/tools/cache"
+	"k8s.io/utils/pointer"
 
 	"sigs.k8s.io/controller-runtime/pkg/cache/internal"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -144,9 +145,13 @@ type Options struct {
 	// instead of `reconcile.Result{}`.
 	SyncPeriod *time.Duration
 
-	// Namespaces restricts the cache's ListWatch to the desired namespaces
-	// Per default ListWatch watches all namespaces
-	Namespaces []string
+	// DefaultNamespaces maps namespace names to cache configs. If set, only
+	// the namespaces in here will be watched and it will by used to default
+	// ByObject.Namespaces for all objects if that is nil.
+	//
+	// The options in the Config that are nil will be defaulted from
+	// the respective Default* settings.
+	DefaultNamespaces map[string]Config
 
 	// DefaultLabelSelector will be used as a label selector for all object types
 	// unless they have a more specific selector set in ByObject.
@@ -160,20 +165,39 @@ type Options struct {
 	// unless they have a more specific transform set in ByObject.
 	DefaultTransform toolscache.TransformFunc
 
-	// UnsafeDisableDeepCopy indicates not to deep copy objects during get or
-	// list objects for EVERY object.
+	// DefaultUnsafeDisableDeepCopy is the default for UnsafeDisableDeepCopy
+	// for everything that doesn't specify this.
+	//
 	// Be very careful with this, when enabled you must DeepCopy any object before mutating it,
 	// otherwise you will mutate the object in the cache.
 	//
 	// This is a global setting for all objects, and can be overridden by the ByObject setting.
-	UnsafeDisableDeepCopy *bool
+	DefaultUnsafeDisableDeepCopy *bool
 
 	// ByObject restricts the cache's ListWatch to the desired fields per GVK at the specified object.
+	// object, this will fall through to Default* settings.
 	ByObject map[client.Object]ByObject
 }
 
 // ByObject offers more fine-grained control over the cache's ListWatch by object.
 type ByObject struct {
+	// Namespaces maps a namespace name to cache configs. If set, only the
+	// namespaces in this map will be cached.
+	//
+	// Settings in the map value that are unset will be defaulted.
+	// Use an empty value for the specific setting to prevent that.
+	//
+	// A nil map allows to default this to the cache's DefaultNamespaces setting.
+	// An empty map prevents this and means that all namespaces will be cached.
+	//
+	// The defaulting follows the following precedence order:
+	// 1. ByObject
+	// 2. DefaultNamespaces[namespace]
+	// 3. Default*
+	//
+	// This must be unset for cluster-scoped objects.
+	Namespaces map[string]Config
+
 	// Label represents a label selector for the object.
 	Label labels.Selector
 
@@ -194,48 +218,118 @@ type ByObject struct {
 	UnsafeDisableDeepCopy *bool
 }
 
+// Config describes all potential options for a given watch.
+type Config struct {
+	// LabelSelector specifies a label selector. A nil value allows to
+	// default this.
+	//
+	// Set to labels.Everything() if you don't want this defaulted.
+	LabelSelector labels.Selector
+
+	// FieldSelector specifics a field selector. A nil value allows to
+	// default this.
+	//
+	// Set to fields.Everything() if you don't want this defaulted.
+	FieldSelector fields.Selector
+
+	// Transform specifies a transform func. A nil value allows to default
+	// this.
+	//
+	// Set to an empty func to prevent this:
+	// func(in interface{}) (interface{}, error) { return in, nil }
+	Transform toolscache.TransformFunc
+
+	// UnsafeDisableDeepCopy specifies if List and Get requests against the
+	// cache should not DeepCopy. A nil value allows to default this.
+	UnsafeDisableDeepCopy *bool
+}
+
 // NewCacheFunc - Function for creating a new cache from the options and a rest config.
 type NewCacheFunc func(config *rest.Config, opts Options) (Cache, error)
 
 // New initializes and returns a new Cache.
-func New(config *rest.Config, opts Options) (Cache, error) {
-	if len(opts.Namespaces) == 0 {
-		opts.Namespaces = []string{metav1.NamespaceAll}
+func New(cfg *rest.Config, opts Options) (Cache, error) {
+	opts, err := defaultOpts(cfg, opts)
+	if err != nil {
+		return nil, err
 	}
-	if len(opts.Namespaces) > 1 {
-		return newMultiNamespaceCache(config, opts)
+
+	newCacheFunc := newCache(cfg, opts)
+
+	var defaultCache Cache
+	if len(opts.DefaultNamespaces) > 0 {
+		defaultConfig := optionDefaultsToConfig(&opts)
+		defaultCache = newMultiNamespaceCache(newCacheFunc, opts.Scheme, opts.Mapper, opts.DefaultNamespaces, &defaultConfig)
+	} else {
+		defaultCache = newCacheFunc(optionDefaultsToConfig(&opts), corev1.NamespaceAll)
 	}
 
-	opts, err := defaultOpts(config, opts)
-	if err != nil {
-		return nil, err
+	if len(opts.ByObject) == 0 {
+		return defaultCache, nil
 	}
 
-	byGVK, err := convertToInformerOptsByGVK(opts.ByObject, opts.Scheme)
-	if err != nil {
-		return nil, err
+	delegating := &delegatingByTypeCache{
+		scheme:       opts.Scheme,
+		caches:       make(map[schema.GroupVersionKind]Cache, len(opts.ByObject)),
+		defaultCache: defaultCache,
+	}
+
+	for obj, config := range opts.ByObject {
+		gvk, err := apiutil.GVKForObject(obj, opts.Scheme)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get GVK for type %T: %w", obj, err)
+		}
+		var cache Cache
+		if len(config.Namespaces) > 0 {
+			cache = newMultiNamespaceCache(newCacheFunc, opts.Scheme, opts.Mapper, config.Namespaces, nil)
+		} else {
+			cache = newCacheFunc(byObjectToConfig(config), corev1.NamespaceAll)
+		}
+		delegating.caches[gvk] = cache
 	}
-	// Set the default selector and transform.
-	byGVK[schema.GroupVersionKind{}] = internal.InformersOptsByGVK{
-		Selector: internal.Selector{
-			Label: opts.DefaultLabelSelector,
-			Field: opts.DefaultFieldSelector,
-		},
+
+	return delegating, nil
+}
+
+func optionDefaultsToConfig(opts *Options) Config {
+	return Config{
+		LabelSelector:         opts.DefaultLabelSelector,
+		FieldSelector:         opts.DefaultFieldSelector,
 		Transform:             opts.DefaultTransform,
-		UnsafeDisableDeepCopy: opts.UnsafeDisableDeepCopy,
+		UnsafeDisableDeepCopy: opts.DefaultUnsafeDisableDeepCopy,
+	}
+}
+
+func byObjectToConfig(byObject ByObject) Config {
+	return Config{
+		LabelSelector:         byObject.Label,
+		FieldSelector:         byObject.Field,
+		Transform:             byObject.Transform,
+		UnsafeDisableDeepCopy: byObject.UnsafeDisableDeepCopy,
 	}
+}
 
-	return &informerCache{
-		scheme: opts.Scheme,
-		Informers: internal.NewInformers(config, &internal.InformersOpts{
-			HTTPClient:   opts.HTTPClient,
-			Scheme:       opts.Scheme,
-			Mapper:       opts.Mapper,
-			ResyncPeriod: *opts.SyncPeriod,
-			Namespace:    opts.Namespaces[0],
-			ByGVK:        byGVK,
-		}),
-	}, nil
+type newCacheFunc func(config Config, namespace string) Cache
+
+func newCache(restConfig *rest.Config, opts Options) newCacheFunc {
+	return func(config Config, namespace string) Cache {
+		return &informerCache{
+			scheme: opts.Scheme,
+			Informers: internal.NewInformers(restConfig, &internal.InformersOpts{
+				HTTPClient:   opts.HTTPClient,
+				Scheme:       opts.Scheme,
+				Mapper:       opts.Mapper,
+				ResyncPeriod: *opts.SyncPeriod,
+				Namespace:    namespace,
+				Selector: internal.Selector{
+					Label: config.LabelSelector,
+					Field: config.FieldSelector,
+				},
+				Transform:             config.Transform,
+				UnsafeDisableDeepCopy: pointer.BoolDeref(config.UnsafeDisableDeepCopy, false),
+			}),
+		}
+	}
 }
 
 func defaultOpts(config *rest.Config, opts Options) (Options, error) {
@@ -262,31 +356,70 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) {
 		}
 	}
 
+	for namespace, cfg := range opts.DefaultNamespaces {
+		cfg = defaultConfig(cfg, optionDefaultsToConfig(&opts))
+		opts.DefaultNamespaces[namespace] = cfg
+	}
+
+	for obj, byObject := range opts.ByObject {
+		isNamespaced, err := apiutil.IsObjectNamespaced(obj, opts.Scheme, opts.Mapper)
+		if err != nil {
+			return opts, fmt.Errorf("failed to determine if %T is namespaced: %w", obj, err)
+		}
+		if !isNamespaced && byObject.Namespaces != nil {
+			return opts, fmt.Errorf("type %T is not namespaced, but its ByObject.Namespaces setting is not nil", obj)
+		}
+
+		// Default the namespace-level configs first, because they need to use the undefaulted type-level config.
+		for namespace, config := range byObject.Namespaces {
+			// 1. Default from the undefaulted type-level config
+			config = defaultConfig(config, byObjectToConfig(byObject))
+
+			// 2. Default from the namespace-level config. This was defaulted from the global default config earlier, but
+			//    might not have an entry for the current namespace.
+			if defaultNamespaceSettings, hasDefaultNamespace := opts.DefaultNamespaces[namespace]; hasDefaultNamespace {
+				config = defaultConfig(config, defaultNamespaceSettings)
+			}
+
+			// 3. Default from the global defaults
+			config = defaultConfig(config, optionDefaultsToConfig(&opts))
+
+			byObject.Namespaces[namespace] = config
+		}
+
+		defaultedConfig := defaultConfig(byObjectToConfig(byObject), optionDefaultsToConfig(&opts))
+		byObject.Label = defaultedConfig.LabelSelector
+		byObject.Field = defaultedConfig.FieldSelector
+		byObject.Transform = defaultedConfig.Transform
+		byObject.UnsafeDisableDeepCopy = defaultedConfig.UnsafeDisableDeepCopy
+
+		if byObject.Namespaces == nil {
+			byObject.Namespaces = opts.DefaultNamespaces
+		}
+
+		opts.ByObject[obj] = byObject
+	}
+
 	// Default the resync period to 10 hours if unset
 	if opts.SyncPeriod == nil {
 		opts.SyncPeriod = &defaultSyncPeriod
 	}
 	return opts, nil
 }
 
-func convertToInformerOptsByGVK(in map[client.Object]ByObject, scheme *runtime.Scheme) (map[schema.GroupVersionKind]internal.InformersOptsByGVK, error) {
-	out := map[schema.GroupVersionKind]internal.InformersOptsByGVK{}
-	for object, byObject := range in {
-		gvk, err := apiutil.GVKForObject(object, scheme)
-		if err != nil {
-			return nil, err
-		}
-		if _, ok := out[gvk]; ok {
-			return nil, fmt.Errorf("duplicate cache options for GVK %v, cache.Options.ByObject has multiple types with the same GroupVersionKind", gvk)
-		}
-		out[gvk] = internal.InformersOptsByGVK{
-			Selector: internal.Selector{
-				Field: byObject.Field,
-				Label: byObject.Label,
-			},
-			Transform:             byObject.Transform,
-			UnsafeDisableDeepCopy: byObject.UnsafeDisableDeepCopy,
-		}
+func defaultConfig(toDefault, defaultFrom Config) Config {
+	if toDefault.LabelSelector == nil {
+		toDefault.LabelSelector = defaultFrom.LabelSelector
 	}
-	return out, nil
+	if toDefault.FieldSelector == nil {
+		toDefault.FieldSelector = defaultFrom.FieldSelector
+	}
+	if toDefault.Transform == nil {
+		toDefault.Transform = defaultFrom.Transform
+	}
+	if toDefault.UnsafeDisableDeepCopy == nil {
+		toDefault.UnsafeDisableDeepCopy = defaultFrom.UnsafeDisableDeepCopy
+	}
+
+	return toDefault
 }