Cache's ByObject is now a map Object -> options

This is a breaking change and revisits some of the previous changes
we've made. It brings the options ByObject to a top level map, which now
contains all the options the internal informer can be configured with.
In addition, it removes the ObjectAll object, which was super confusing
and only used to disable the deep copy for every object; instead it's
now possible to disable deep copy through a top level option.

Signed-off-by: Vince Prignano <[email protected]>

Author: vincepri

pkg/cache/cache.go

@@ -38,6 +38,7 @@ import (
 	kscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	kcache "k8s.io/client-go/tools/cache"
+	"k8s.io/utils/pointer"
 
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -119,13 +120,10 @@ var _ = Describe("Informer Cache", func() {
 var _ = Describe("Multi-Namespace Informer Cache", func() {
 	CacheTest(cache.MultiNamespacedCacheBuilder([]string{testNamespaceOne, testNamespaceTwo, "default"}), cache.Options{})
 })
-var _ = Describe("Informer Cache without DeepCopy", func() {
+
+var _ = Describe("Informer Cache without global DeepCopy", func() {
 	CacheTest(cache.New, cache.Options{
-		View: cache.ViewOptions{
-			ByObject: cache.ViewByObject{
-				UnsafeDisableDeepCopy: cache.DisableDeepCopyByObject{cache.ObjectAll{}: true},
-			},
-		},
+		UnsafeDisableDeepCopy: pointer.Bool(true),
 	})
 })
 
@@ -190,48 +188,46 @@ var _ = Describe("Cache with transformers", func() {
 
 		By("creating the informer cache")
 		informerCache, err = cache.New(cfg, cache.Options{
-			View: cache.ViewOptions{
-				DefaultTransform: func(i interface{}) (interface{}, error) {
-					obj := i.(runtime.Object)
-					Expect(obj).NotTo(BeNil())
+			DefaultTransform: func(i interface{}) (interface{}, error) {
+				obj := i.(runtime.Object)
+				Expect(obj).NotTo(BeNil())
 
-					accessor, err := meta.Accessor(obj)
-					Expect(err).To(BeNil())
-					annotations := accessor.GetAnnotations()
-
-					if _, exists := annotations["transformed"]; exists {
-						// Avoid performing transformation multiple times.
-						return i, nil
-					}
+				accessor, err := meta.Accessor(obj)
+				Expect(err).To(BeNil())
+				annotations := accessor.GetAnnotations()
 
-					if annotations == nil {
-						annotations = make(map[string]string)
-					}
-					annotations["transformed"] = "default"
-					accessor.SetAnnotations(annotations)
+				if _, exists := annotations["transformed"]; exists {
+					// Avoid performing transformation multiple times.
 					return i, nil
-				},
-				ByObject: cache.ViewByObject{
-					Transform: cache.TransformByObject{
-						&corev1.Pod{}: func(i interface{}) (interface{}, error) {
-							obj := i.(runtime.Object)
-							Expect(obj).NotTo(BeNil())
-							accessor, err := meta.Accessor(obj)
-							Expect(err).To(BeNil())
-
-							annotations := accessor.GetAnnotations()
-							if _, exists := annotations["transformed"]; exists {
-								// Avoid performing transformation multiple times.
-								return i, nil
-							}
-
-							if annotations == nil {
-								annotations = make(map[string]string)
-							}
-							annotations["transformed"] = "explicit"
-							accessor.SetAnnotations(annotations)
+				}
+
+				if annotations == nil {
+					annotations = make(map[string]string)
+				}
+				annotations["transformed"] = "default"
+				accessor.SetAnnotations(annotations)
+				return i, nil
+			},
+			ByObject: map[client.Object]cache.ByObject{
+				&corev1.Pod{}: {
+					Transform: func(i interface{}) (interface{}, error) {
+						obj := i.(runtime.Object)
+						Expect(obj).NotTo(BeNil())
+						accessor, err := meta.Accessor(obj)
+						Expect(err).To(BeNil())
+
+						annotations := accessor.GetAnnotations()
+						if _, exists := annotations["transformed"]; exists {
+							// Avoid performing transformation multiple times.
 							return i, nil
-						},
+						}
+
+						if annotations == nil {
+							annotations = make(map[string]string)
+						}
+						annotations["transformed"] = "explicit"
+						accessor.SetAnnotations(annotations)
+						return i, nil
 					},
 				},
 			},
@@ -370,13 +366,9 @@ var _ = Describe("Cache with selectors", func() {
 		}
 
 		opts := cache.Options{
-			View: cache.ViewOptions{
-				DefaultSelector: cache.ObjectSelector{Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceTwo)},
-				ByObject: cache.ViewByObject{
-					Selectors: cache.SelectorsByObject{
-						&corev1.ServiceAccount{}: {Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceOne)},
-					},
-				},
+			DefaultFieldSelector: fields.OneTermEqualSelector("metadata.namespace", testNamespaceTwo),
+			ByObject: map[client.Object]cache.ByObject{
+				&corev1.ServiceAccount{}: {Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceOne)},
 			},
 		}
 
@@ -798,7 +790,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
 
 				It("should be able to restrict cache to a namespace", func() {
 					By("creating a namespaced cache")
-					namespacedCache, err := cache.New(cfg, cache.Options{View: cache.ViewOptions{Namespaces: []string{testNamespaceOne}}})
+					namespacedCache, err := cache.New(cfg, cache.Options{Namespaces: []string{testNamespaceOne}})
 					Expect(err).NotTo(HaveOccurred())
 
 					By("running the cache and waiting for it to sync")
@@ -1079,7 +1071,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
 
 				It("should be able to restrict cache to a namespace", func() {
 					By("creating a namespaced cache")
-					namespacedCache, err := cache.New(cfg, cache.Options{View: cache.ViewOptions{Namespaces: []string{testNamespaceOne}}})
+					namespacedCache, err := cache.New(cfg, cache.Options{Namespaces: []string{testNamespaceOne}})
 					Expect(err).NotTo(HaveOccurred())
 
 					By("running the cache and waiting for it to sync")
@@ -1233,14 +1225,10 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
 				By("creating the cache")
 				builder := cache.BuilderWithOptions(
 					cache.Options{
-						View: cache.ViewOptions{
-							ByObject: cache.ViewByObject{
-								Selectors: cache.SelectorsByObject{
-									&corev1.Pod{}: {
-										Label: labels.Set(tc.labelSelectors).AsSelector(),
-										Field: fields.Set(tc.fieldSelectors).AsSelector(),
-									},
-								},
+						ByObject: map[client.Object]cache.ByObject{
+							&corev1.Pod{}: {
+								Label: labels.Set(tc.labelSelectors).AsSelector(),
+								Field: fields.Set(tc.fieldSelectors).AsSelector(),
 							},
 						},
 					},
@@ -1822,12 +1810,11 @@ func isKubeService(svc metav1.Object) bool {
 }
 
 func isPodDisableDeepCopy(opts cache.Options) bool {
-	if d, ok := opts.View.ByObject.UnsafeDisableDeepCopy[&corev1.Pod{}]; ok {
-		return d
-	} else if d, ok = opts.View.ByObject.UnsafeDisableDeepCopy[cache.ObjectAll{}]; ok {
-		return d
-	} else if d, ok = opts.View.ByObject.UnsafeDisableDeepCopy[&cache.ObjectAll{}]; ok {
-		return d
+	if opts.ByObject[&corev1.Pod{}].UnsafeDisableDeepCopy != nil && *opts.ByObject[&corev1.Pod{}].UnsafeDisableDeepCopy {
+		return true
+	}
+	if opts.UnsafeDisableDeepCopy != nil && *opts.UnsafeDisableDeepCopy {
+		return true
 	}
 	return false
 }