Merge pull request #1326 from joelanford/bug/ignore-namespace-cluster…

…-scoped-get

🐛 ignore namespace key when getting cluster-scoped resources

pkg/cache/cache_test.go

@@ -35,6 +35,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+const testNodeOne = "test-node-1"
 const testNamespaceOne = "test-namespace-1"
 const testNamespaceTwo = "test-namespace-2"
 const testNamespaceThree = "test-namespace-3"
@@ -98,6 +99,8 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
 			By("creating three pods")
 			cl, err := client.New(cfg, client.Options{})
 			Expect(err).NotTo(HaveOccurred())
+			err = ensureNode(testNodeOne, cl)
+			Expect(err).NotTo(HaveOccurred())
 			err = ensureNamespace(testNamespaceOne, cl)
 			Expect(err).NotTo(HaveOccurred())
 			err = ensureNamespace(testNamespaceTwo, cl)
@@ -413,17 +416,33 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
 					Expect(out.Items).Should(HaveLen(1))
 					Expect(out.Items[0].GetNamespace()).To(Equal(testNamespaceOne))
 
-					By("listing all namespaces - should still be able to get a cluster-scoped resource")
-					namespaceList := &unstructured.UnstructuredList{}
-					namespaceList.SetGroupVersionKind(schema.GroupVersionKind{
+					By("listing all nodes - should still be able to list a cluster-scoped resource")
+					nodeList := &unstructured.UnstructuredList{}
+					nodeList.SetGroupVersionKind(schema.GroupVersionKind{
 						Group:   "",
 						Version: "v1",
-						Kind:    "NamespaceList",
+						Kind:    "NodeList",
 					})
-					Expect(namespacedCache.List(context.Background(), namespaceList)).To(Succeed())
+					Expect(namespacedCache.List(context.Background(), nodeList)).To(Succeed())
+
+					By("verifying the node list is not empty")
+					Expect(nodeList.Items).NotTo(BeEmpty())
+
+					By("getting a node - should still be able to get a cluster-scoped resource")
+					node := &unstructured.Unstructured{}
+					node.SetGroupVersionKind(schema.GroupVersionKind{
+						Group:   "",
+						Version: "v1",
+						Kind:    "Node",
+					})
+
+					By("verifying that getting the node works with an empty namespace")
+					key1 := client.ObjectKey{Namespace: "", Name: testNodeOne}
+					Expect(namespacedCache.Get(context.Background(), key1, node)).To(Succeed())
 
-					By("verifying the namespace list is not empty")
-					Expect(namespaceList.Items).NotTo(BeEmpty())
+					By("verifying that the namespace is ignored when getting a cluster-scoped resource")
+					key2 := client.ObjectKey{Namespace: "random", Name: testNodeOne}
+					Expect(namespacedCache.Get(context.Background(), key2, node)).To(Succeed())
 				})
 
 				It("should deep copy the object unless told otherwise", func() {
@@ -607,17 +626,33 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
 					Expect(out.Items).Should(HaveLen(1))
 					Expect(out.Items[0].GetNamespace()).To(Equal(testNamespaceOne))
 
-					By("listing all namespaces - should still be able to get a cluster-scoped resource")
-					namespaceList := &kmetav1.PartialObjectMetadataList{}
-					namespaceList.SetGroupVersionKind(schema.GroupVersionKind{
+					By("listing all nodes - should still be able to list a cluster-scoped resource")
+					nodeList := &kmetav1.PartialObjectMetadataList{}
+					nodeList.SetGroupVersionKind(schema.GroupVersionKind{
 						Group:   "",
 						Version: "v1",
-						Kind:    "NamespaceList",
+						Kind:    "NodeList",
 					})
-					Expect(namespacedCache.List(context.Background(), namespaceList)).To(Succeed())
+					Expect(namespacedCache.List(context.Background(), nodeList)).To(Succeed())
+
+					By("verifying the node list is not empty")
+					Expect(nodeList.Items).NotTo(BeEmpty())
+
+					By("getting a node - should still be able to get a cluster-scoped resource")
+					node := &kmetav1.PartialObjectMetadata{}
+					node.SetGroupVersionKind(schema.GroupVersionKind{
+						Group:   "",
+						Version: "v1",
+						Kind:    "Node",
+					})
+
+					By("verifying that getting the node works with an empty namespace")
+					key1 := client.ObjectKey{Namespace: "", Name: testNodeOne}
+					Expect(namespacedCache.Get(context.Background(), key1, node)).To(Succeed())
 
-					By("verifying the namespace list is not empty")
-					Expect(namespaceList.Items).NotTo(BeEmpty())
+					By("verifying that the namespace is ignored when getting a cluster-scoped resource")
+					key2 := client.ObjectKey{Namespace: "random", Name: testNodeOne}
+					Expect(namespacedCache.Get(context.Background(), key2, node)).To(Succeed())
 				})
 
 				It("should deep copy the object unless told otherwise", func() {
@@ -1079,6 +1114,23 @@ func ensureNamespace(namespace string, client client.Client) error {
 	return err
 }
 
+func ensureNode(name string, client client.Client) error {
+	node := kcorev1.Node{
+		ObjectMeta: kmetav1.ObjectMeta{
+			Name: name,
+		},
+		TypeMeta: kmetav1.TypeMeta{
+			Kind:       "Node",
+			APIVersion: "v1",
+		},
+	}
+	err := client.Create(context.TODO(), &node)
+	if errors.IsAlreadyExists(err) {
+		return nil
+	}
+	return err
+}
+
 //nolint:interfacer
 func isKubeService(svc kmetav1.Object) bool {
 	// grumble grumble linters grumble grumble

pkg/cache/internal/cache_reader.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/selection"
 	"k8s.io/client-go/tools/cache"
+
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -42,10 +43,16 @@ type CacheReader struct {
 
 	// groupVersionKind is the group-version-kind of the resource.
 	groupVersionKind schema.GroupVersionKind
+
+	// scopeName is the scope of the resource (namespaced or cluster-scoped).
+	scopeName apimeta.RESTScopeName
 }
 
 // Get checks the indexer for the object and writes a copy of it if found
 func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out client.Object) error {
+	if c.scopeName == apimeta.RESTScopeNameRoot {
+		key.Namespace = ""
+	}
 	storeKey := objectKeyToStoreKey(key)
 
 	// Lookup the object from the indexer cache

pkg/cache/internal/informers_map.go

@@ -212,9 +212,13 @@ func (ip *specificInformersMap) addInformerToMap(gvk schema.GroupVersionKind, ob
 	ni := cache.NewSharedIndexInformer(lw, obj, resyncPeriod(ip.resync)(), cache.Indexers{
 		cache.NamespaceIndex: cache.MetaNamespaceIndexFunc,
 	})
+	rm, err := ip.mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
+	if err != nil {
+		return nil, false, err
+	}
 	i := &MapEntry{
 		Informer: ni,
-		Reader:   CacheReader{indexer: ni.GetIndexer(), groupVersionKind: gvk},
+		Reader:   CacheReader{indexer: ni.GetIndexer(), groupVersionKind: gvk, scopeName: rm.Scope.Name()},
 	}
 	ip.informersByGVK[gvk] = i