Track ownerrefs between cluster-scoped and namespaced objects

Signed-off-by: Jonathan Ogilvie <[email protected]>

Author: jcogilvie

pkg/cache/cluster.go

@@ -1065,7 +1065,7 @@ func (c *clusterCache) IterateHierarchyV2(keys []kube.ResourceKey, action func(r
 	}
 	for namespace, namespaceKeys := range keysPerNamespace {
 		nsNodes := c.nsIndex[namespace]
-		graph := buildGraph(nsNodes)
+		graph := buildGraph(nsNodes, c.resources)
 		visited := make(map[kube.ResourceKey]int)
 		for _, key := range namespaceKeys {
 			visited[key] = 0
@@ -1095,7 +1095,7 @@ func (c *clusterCache) IterateHierarchyV2(keys []kube.ResourceKey, action func(r
 	}
 }
 
-func buildGraph(nsNodes map[kube.ResourceKey]*Resource) map[kube.ResourceKey]map[types.UID]*Resource {
+func buildGraph(nsNodes map[kube.ResourceKey]*Resource, allResources map[kube.ResourceKey]*Resource) map[kube.ResourceKey]map[types.UID]*Resource {
 	// Prepare to construct a graph
 	nodesByUID := make(map[types.UID][]*Resource, len(nsNodes))
 	for _, node := range nsNodes {
@@ -1106,6 +1106,7 @@ func buildGraph(nsNodes map[kube.ResourceKey]*Resource) map[kube.ResourceKey]map
 	graph := make(map[kube.ResourceKey]map[types.UID]*Resource)
 
 	// Loop through all nodes, calling each one "childNode," because we're only bothering with it if it has a parent.
+	// First process nodes in the current namespace
 	for _, childNode := range nsNodes {
 		for i, ownerRef := range childNode.OwnerRefs {
 			// First, backfill UID of inferred owner child references.
@@ -1115,7 +1116,16 @@ func buildGraph(nsNodes map[kube.ResourceKey]*Resource) map[kube.ResourceKey]map
 					// APIVersion is invalid, so we couldn't find the parent.
 					continue
 				}
-				graphKeyNode, ok := nsNodes[kube.ResourceKey{Group: group.Group, Kind: ownerRef.Kind, Namespace: childNode.Ref.Namespace, Name: ownerRef.Name}]
+				// Try same-namespace lookup first (preserves existing behavior)
+				sameNSKey := kube.ResourceKey{Group: group.Group, Kind: ownerRef.Kind, Namespace: childNode.Ref.Namespace, Name: ownerRef.Name}
+				graphKeyNode, ok := nsNodes[sameNSKey]
+				
+				// If not found and we have cross-namespace capabilities, try cluster-scoped lookup
+				if !ok && allResources != nil {
+					clusterScopedKey := kube.ResourceKey{Group: group.Group, Kind: ownerRef.Kind, Namespace: "", Name: ownerRef.Name}
+					graphKeyNode, ok = allResources[clusterScopedKey]
+				}
+				
 				if !ok {
 					// No resource found with the given graph key, so move on.
 					continue
@@ -1126,6 +1136,18 @@ func buildGraph(nsNodes map[kube.ResourceKey]*Resource) map[kube.ResourceKey]map
 
 			// Now that we have the UID of the parent, update the graph.
 			uidNodes, ok := nodesByUID[ownerRef.UID]
+			if !ok && allResources != nil {
+				// If parent not found in current namespace, check if it exists in allResources
+				// and create a temporary uidNodes list for cross-namespace relationships
+				for _, parentCandidate := range allResources {
+					if parentCandidate.Ref.UID == ownerRef.UID {
+						uidNodes = []*Resource{parentCandidate}
+						ok = true
+						break
+					}
+				}
+			}
+			
 			if ok {
 				for _, uidNode := range uidNodes {
 					// Update the graph for this owner to include the child.
@@ -1148,6 +1170,33 @@ func buildGraph(nsNodes map[kube.ResourceKey]*Resource) map[kube.ResourceKey]map
 			}
 		}
 	}
+	
+	// Second pass: process cross-namespace children if allResources is provided
+	if allResources != nil {
+		for _, childNode := range allResources {
+			// Skip if already processed in the current namespace
+			if _, exists := nsNodes[childNode.ResourceKey()]; exists {
+				continue
+			}
+			
+			// Check if this child has a parent in the current namespace
+			for _, ownerRef := range childNode.OwnerRefs {
+				group, err := schema.ParseGroupVersion(ownerRef.APIVersion)
+				if err != nil {
+					continue
+				}
+				parentKey := kube.ResourceKey{Group: group.Group, Kind: ownerRef.Kind, Namespace: "", Name: ownerRef.Name}
+				if parentNode, exists := nsNodes[parentKey]; exists {
+					// Found a cross-namespace relationship
+					if _, ok := graph[parentNode.ResourceKey()]; !ok {
+						graph[parentNode.ResourceKey()] = make(map[types.UID]*Resource)
+					}
+					graph[parentNode.ResourceKey()][childNode.Ref.UID] = childNode
+				}
+			}
+		}
+	}
+	
 	return graph
 }
 

pkg/cache/cluster_test.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/client-go/dynamic/fake"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -1157,6 +1158,102 @@ func TestIterateHierachyV2(t *testing.T) {
 	})
 }
 
+// TestIterateHierarchyV2_CrossNamespaceOwnerReference tests that cross-namespace 
+// owner references work correctly, specifically cluster-scoped resources with 
+// namespaced children
+func TestIterateHierarchyV2_CrossNamespaceOwnerReference(t *testing.T) {
+	cluster := newCluster(t)
+	
+	// Create cluster-scoped parent resource (ProviderRevision)
+	parentUID := types.UID("parent-uid-123")
+	clusterScopedParent := &Resource{
+		Ref: corev1.ObjectReference{
+			APIVersion: "pkg.crossplane.io/v1",
+			Kind:       "ProviderRevision",
+			Name:       "provider-aws-cloudformation-3b2c213545b8",
+			UID:        parentUID,
+			// No namespace = cluster-scoped
+		},
+		OwnerRefs: []metav1.OwnerReference{},
+	}
+
+	// Create namespaced child with ownerReference to cluster-scoped parent
+	namespacedChild := &Resource{
+		Ref: corev1.ObjectReference{
+			APIVersion: "apps/v1",
+			Kind:       "Deployment",
+			Name:       "provider-aws-cloudformation-3b2c213545b8",
+			Namespace:  "crossplane-system",
+			UID:        types.UID("child-uid-456"),
+		},
+		OwnerRefs: []metav1.OwnerReference{{
+			APIVersion: "pkg.crossplane.io/v1",
+			Kind:       "ProviderRevision",
+			Name:       "provider-aws-cloudformation-3b2c213545b8",
+			// UID: parentUID, // Don't set UID - let it be resolved via cross-namespace lookup
+		}},
+	}
+
+	// Create cluster-scoped child with ownerReference to cluster-scoped parent (this should work already)
+	clusterScopedChild := &Resource{
+		Ref: corev1.ObjectReference{
+			APIVersion: "rbac.authorization.k8s.io/v1",
+			Kind:       "ClusterRole",
+			Name:       "crossplane:provider:provider-aws-cloudformation-3b2c213545b8:system",
+			UID:        types.UID("child-uid-789"),
+			// No namespace = cluster-scoped
+		},
+		OwnerRefs: []metav1.OwnerReference{{
+			APIVersion: "pkg.crossplane.io/v1",
+			Kind:       "ProviderRevision",
+			Name:       "provider-aws-cloudformation-3b2c213545b8",
+			UID:        parentUID,
+		}},
+	}
+
+	// Add all resources to cluster cache
+	cluster.setNode(clusterScopedParent)
+	cluster.setNode(namespacedChild)
+	cluster.setNode(clusterScopedChild)
+
+	// Test hierarchy traversal starting from cluster-scoped parent
+	var visitedResources []*Resource
+	cluster.IterateHierarchyV2(
+		[]kube.ResourceKey{clusterScopedParent.ResourceKey()},
+		func(resource *Resource, namespaceResources map[kube.ResourceKey]*Resource) bool {
+			visitedResources = append(visitedResources, resource)
+			return true
+		},
+	)
+
+	// Should visit parent + both children (3 resources)
+	assert.Equal(t, 3, len(visitedResources), "Should visit parent and both children")
+	
+	visitedNames := make([]string, len(visitedResources))
+	for i, res := range visitedResources {
+		visitedNames[i] = res.Ref.Name
+	}
+	
+	// Check we have the expected resources by type and namespace combination
+	foundParent := false
+	foundClusterChild := false
+	foundNamespacedChild := false
+	
+	for _, res := range visitedResources {
+		if res.Ref.Kind == "ProviderRevision" && res.Ref.Namespace == "" {
+			foundParent = true
+		} else if res.Ref.Kind == "ClusterRole" && res.Ref.Namespace == "" {
+			foundClusterChild = true
+		} else if res.Ref.Kind == "Deployment" && res.Ref.Namespace == "crossplane-system" {
+			foundNamespacedChild = true
+		}
+	}
+	
+	assert.True(t, foundParent, "Should visit ProviderRevision parent")
+	assert.True(t, foundClusterChild, "Should visit ClusterRole child")
+	assert.True(t, foundNamespacedChild, "Should visit Deployment child (this tests the fix)")
+}
+
 // Test_watchEvents_Deadlock validates that starting watches will not create a deadlock
 // caused by using improper locking in various callback methods when there is a high load on the
 // system.
@@ -1273,7 +1370,7 @@ func BenchmarkBuildGraph(b *testing.B) {
 	testResources := buildTestResourceMap()
 	b.ResetTimer()
 	for n := 0; n < b.N; n++ {
-		buildGraph(testResources)
+		buildGraph(testResources, nil)
 	}
 }