From fd22deacd0687f0d17a936f9411df3682d5d2987 Mon Sep 17 00:00:00 2001
From: varshaprasad96 <varshaprasad96@gmail.com>
Date: Mon, 8 Mar 2021 12:37:24 -0800
Subject: [PATCH] :sparkles: Modify multinamespaced cache to support cluster
 scoped resources

This PR modifies the multinamespacedcache implementation to:
- create a global cache mapping for an empty namespace, so that when
cluster scoped resources are fetched, namespace is not required.
- deduplicate the objects in the `List` call, based on
unique combination of resource name and namespace.

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>
---
 pkg/cache/cache_test.go            | 33 ++++++++++++++++++++++++
 pkg/cache/multi_namespace_cache.go | 40 +++++++++++++++++++++++++++---
 2 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go
index d23db63f75..18cd27e1da 100644
--- a/pkg/cache/cache_test.go
+++ b/pkg/cache/cache_test.go
@@ -494,6 +494,39 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
 					err := informerCache.Get(context.Background(), svcKey, svc)
 					Expect(err).To(HaveOccurred())
 				})
+				It("test multinamespaced cache for cluster scoped resources", func() {
+					By("creating a multinamespaced cache to watch specific namespaces")
+					multi := cache.MultiNamespacedCacheBuilder([]string{"default", testNamespaceOne})
+					m, err := multi(cfg, cache.Options{})
+					Expect(err).NotTo(HaveOccurred())
+
+					By("running the cache and waiting it for sync")
+					go func() {
+						defer GinkgoRecover()
+						Expect(m.Start(informerCacheCtx)).To(Succeed())
+					}()
+					Expect(m.WaitForCacheSync(informerCacheCtx)).NotTo(BeFalse())
+
+					By("should be able to fetch cluster scoped resource")
+					node := &kcorev1.Node{}
+
+					By("verifying that getting the node works with an empty namespace")
+					key1 := client.ObjectKey{Namespace: "", Name: testNodeOne}
+					Expect(m.Get(context.Background(), key1, node)).To(Succeed())
+
+					By("verifying if the cluster scoped resources are not duplicated")
+					nodeList := &unstructured.UnstructuredList{}
+					nodeList.SetGroupVersionKind(schema.GroupVersionKind{
+						Group:   "",
+						Version: "v1",
+						Kind:    "NodeList",
+					})
+					Expect(m.List(context.Background(), nodeList)).To(Succeed())
+
+					By("verifying the node list is not empty")
+					Expect(nodeList.Items).NotTo(BeEmpty())
+					Expect(len(nodeList.Items)).To(BeEquivalentTo(1))
+				})
 			})
 			Context("with metadata-only objects", func() {
 				It("should be able to list objects that haven't been watched previously", func() {
diff --git a/pkg/cache/multi_namespace_cache.go b/pkg/cache/multi_namespace_cache.go
index f0e18c09b0..006c171870 100644
--- a/pkg/cache/multi_namespace_cache.go
+++ b/pkg/cache/multi_namespace_cache.go
@@ -34,9 +34,13 @@ import (
 // NewCacheFunc - Function for creating a new cache from the options and a rest config
 type NewCacheFunc func(config *rest.Config, opts Options) (Cache, error)
 
+// a new global namespaced cache to handle cluster scoped resources
+var globalCache = ""
+
 // MultiNamespacedCacheBuilder - Builder function to create a new multi-namespaced cache.
 // This will scope the cache to a list of namespaces. Listing for all namespaces
-// will list for all the namespaces that this knows about. Note that this is not intended
+// will list for all the namespaces that this knows about. By default this will create
+// a global cache for cluster scoped resource (having empty namespace). Note that this is not intended
 // to be used for excluding namespaces, this is better done via a Predicate. Also note that
 // you may face performance issues when using this with a high number of namespaces.
 func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc {
@@ -45,6 +49,8 @@ func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc {
 		if err != nil {
 			return nil, err
 		}
+		// create a cache for cluster scoped resources
+		namespaces = append(namespaces, globalCache)
 		caches := map[string]Cache{}
 		for _, ns := range namespaces {
 			opts.Namespace = ns
@@ -143,7 +149,16 @@ func (c *multiNamespaceCache) List(ctx context.Context, list client.ObjectList,
 		if !ok {
 			return fmt.Errorf("unable to get: %v because of unknown namespace for the cache", listOpts.Namespace)
 		}
-		return cache.List(ctx, list, opts...)
+		err := cache.List(ctx, list, opts...)
+		if err != nil {
+			return err
+		}
+		items, err := apimeta.ExtractList(list)
+		if err != nil {
+			return err
+		}
+		uniqueItems := removeDuplicates(items)
+		return apimeta.SetList(list, uniqueItems)
 	}
 
 	listAccessor, err := meta.ListAccessor(list)
@@ -174,9 +189,28 @@ func (c *multiNamespaceCache) List(ctx context.Context, list client.ObjectList,
 		// The last list call should have the most correct resource version.
 		resourceVersion = accessor.GetResourceVersion()
 	}
+
+	uniqueItems := removeDuplicates(allItems)
 	listAccessor.SetResourceVersion(resourceVersion)
 
-	return apimeta.SetList(list, allItems)
+	return apimeta.SetList(list, uniqueItems)
+}
+
+// removeDuplicates removes the duplicate objects obtained from all namespaces so that
+// the resulting list has objects with unique name and namespace.
+func removeDuplicates(items []runtime.Object) []runtime.Object {
+	objects := make(map[string]bool)
+	unique := []runtime.Object{}
+
+	for _, obj := range items {
+		metaObj, _ := meta.Accessor(obj)
+		key := metaObj.GetName() + " " + metaObj.GetNamespace()
+		if _, value := objects[key]; !value {
+			objects[key] = true
+			unique = append(unique, obj)
+		}
+	}
+	return unique
 }
 
 // multiNamespaceInformer knows how to handle interacting with the underlying informer across multiple namespaces