🐛 Modify multinamespaced cache to support cluster scoped resources (#1418)

* 🐛 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>

* Add restmapper to multinamespaced cache

* Use restmapper to identify scope of the object

Modify multinamespaced cache to accept restmapper, which
can be used to identify the scope of the object and handle
the cluster scoped objects accordingly.

* Rename fileter.go to objectutil.go

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>

Author: varshaprasad96

pkg/cache/cache_test.go

@@ -505,6 +505,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() {

pkg/cache/multi_namespace_cache.go

@@ -29,14 +29,19 @@ import (
 	"k8s.io/client-go/rest"
 	toolscache "k8s.io/client-go/tools/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/internal/objectutil"
 )
 
 // 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
+const globalCache = "_cluster-scope"
+
 // 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 +50,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
@@ -54,7 +61,7 @@ func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc {
 			}
 			caches[ns] = c
 		}
-		return &multiNamespaceCache{namespaceToCache: caches, Scheme: opts.Scheme}, nil
+		return &multiNamespaceCache{namespaceToCache: caches, Scheme: opts.Scheme, RESTMapper: opts.Mapper}, nil
 	}
 }
 
@@ -65,6 +72,7 @@ func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc {
 type multiNamespaceCache struct {
 	namespaceToCache map[string]Cache
 	Scheme           *runtime.Scheme
+	RESTMapper       meta.RESTMapper
 }
 
 var _ Cache = &multiNamespaceCache{}
@@ -127,6 +135,17 @@ func (c *multiNamespaceCache) IndexField(ctx context.Context, obj client.Object,
 }
 
 func (c *multiNamespaceCache) Get(ctx context.Context, key client.ObjectKey, obj client.Object) error {
+	isNamespaced, err := objectutil.IsAPINamespaced(obj, c.Scheme, c.RESTMapper)
+	if err != nil {
+		return err
+	}
+
+	if !isNamespaced {
+		// Look into the global cache to fetch the object
+		cache := c.namespaceToCache[globalCache]
+		return cache.Get(ctx, key, obj)
+	}
+
 	cache, ok := c.namespaceToCache[key.Namespace]
 	if !ok {
 		return fmt.Errorf("unable to get: %v because of unknown namespace for the cache", key)
@@ -138,6 +157,18 @@ func (c *multiNamespaceCache) Get(ctx context.Context, key client.ObjectKey, obj
 func (c *multiNamespaceCache) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
 	listOpts := client.ListOptions{}
 	listOpts.ApplyOptions(opts)
+
+	isNamespaced, err := objectutil.IsAPINamespaced(list, c.Scheme, c.RESTMapper)
+	if err != nil {
+		return err
+	}
+
+	if !isNamespaced {
+		// Look at the global cache to get the objects with the specified GVK
+		cache := c.namespaceToCache[globalCache]
+		return cache.List(ctx, list, opts...)
+	}
+
 	if listOpts.Namespace != corev1.NamespaceAll {
 		cache, ok := c.namespaceToCache[listOpts.Namespace]
 		if !ok {

pkg/client/namespaced_client.go

@@ -57,6 +57,7 @@ func (n *namespacedClient) RESTMapper() meta.RESTMapper {
 
 // isNamespaced returns true if the object is namespace scoped.
 // For unstructured objects the gvk is found from the object itself.
+// TODO: this is repetitive code. Remove this and use ojectutil.IsNamespaced.
 func isNamespaced(c Client, obj runtime.Object) (bool, error) {
 	var gvk schema.GroupVersionKind
 	var err error

pkg/internal/objectutil/filter.go pkg/internal/objectutil/objectutil.go

@@ -17,9 +17,15 @@ limitations under the License.
 package objectutil
 
 import (
+	"errors"
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/api/meta"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 )
 
 // FilterWithLabels returns a copy of the items in objs matching labelSel
@@ -40,3 +46,28 @@ func FilterWithLabels(objs []runtime.Object, labelSel labels.Selector) ([]runtim
 	}
 	return outItems, nil
 }
+
+// IsAPINamespaced returns true if the object is namespace scoped.
+// For unstructured objects the gvk is found from the object itself.
+func IsAPINamespaced(obj runtime.Object, scheme *runtime.Scheme, restmapper apimeta.RESTMapper) (bool, error) {
+	gvk, err := apiutil.GVKForObject(obj, scheme)
+	if err != nil {
+		return false, err
+	}
+
+	restmapping, err := restmapper.RESTMapping(schema.GroupKind{Group: gvk.Group, Kind: gvk.Kind})
+	if err != nil {
+		return false, fmt.Errorf("failed to get restmapping: %w", err)
+	}
+
+	scope := restmapping.Scope.Name()
+
+	if scope == "" {
+		return false, errors.New("Scope cannot be identified. Empty scope returned")
+	}
+
+	if scope != meta.RESTScopeNameRoot {
+		return true, nil
+	}
+	return false, nil
+}