Report each blocking dependent once on delete

internal/webhook/deletion_validator.go

@@ -129,6 +129,10 @@ func (v *DeletionValidator) Handle(ctx context.Context, req admission.Request) a
 		blockers = append(blockers, dependents...)
 	}
 
+	// A dependent that references the deleted resource through more than one
+	// field matches once per field; collapse those into a single mention.
+	blockers = dedupeBlockers(blockers)
+
 	if len(blockers) > 0 {
 		msg := fmt.Sprintf("cannot delete %s/%s: still referenced by %s",
 			req.Resource.Resource, req.Name, strings.Join(blockers, ", "))
@@ -183,13 +187,54 @@ func (v *DeletionValidator) listDependents(
 	for _, item := range list.Items {
 		val := fieldpath.Resolve(item.Object, entry.MatchedField.FieldPath)
 		if val == targetName {
-			blockers = append(blockers, fmt.Sprintf("%s/%s", entry.State.DependentGVK.Kind, item.GetName()))
+			// A namespaced target is listed within its own namespace, so names
+			// are already unique and the namespace segment would be redundant.
+			// A cluster-scoped target is listed across all namespaces, so qualify
+			// with the dependent's namespace to keep same-named objects distinct.
+			depNamespace := ""
+			if namespace == "" {
+				depNamespace = item.GetNamespace()
+			}
+			blockers = append(blockers, blockerID(entry.State.DependentGVK.Kind, depNamespace, item.GetName()))
 		}
 	}
 
 	return blockers, nil
 }
 
+// blockerID renders a stable, human-readable identifier for a dependent object.
+// When a namespace is supplied it is included so that two objects sharing a
+// name in different namespaces are not conflated — this matters for
+// cluster-scoped deletions, where dependents are listed across all namespaces.
+func blockerID(kind, namespace, name string) string {
+	if namespace != "" {
+		return fmt.Sprintf("%s/%s/%s", kind, namespace, name)
+	}
+
+	return fmt.Sprintf("%s/%s", kind, name)
+}
+
+// dedupeBlockers removes duplicate dependent identifiers while preserving
+// first-seen order, so each referencing object is reported exactly once
+// regardless of how many fields or rules reference the deleted resource.
+func dedupeBlockers(blockers []string) []string {
+	if len(blockers) == 0 {
+		return blockers
+	}
+
+	seen := make(map[string]bool, len(blockers))
+	out := make([]string, 0, len(blockers))
+	for _, b := range blockers {
+		if seen[b] {
+			continue
+		}
+		seen[b] = true
+		out = append(out, b)
+	}
+
+	return out
+}
+
 // objectFromRequest extracts the unstructured object from the admission
 // request's OldObject (for DELETE) or Object (for other operations).
 func objectFromRequest(req admission.Request) (*unstructured.Unstructured, error) {

internal/webhook/deletion_validator_test.go

@@ -4,11 +4,17 @@
 package webhook
 
 import (
+	"context"
 	"encoding/json"
+	"reflect"
+	"sort"
 	"testing"
 
 	admissionv1 "k8s.io/api/admission/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	dynamicfake "k8s.io/client-go/dynamic/fake"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
@@ -121,6 +127,159 @@ func TestSkipProtection_WrongValue(t *testing.T) {
 	}
 }
 
+func vpcPeering(namespace, name, targetVPC string) *unstructured.Unstructured {
+	return &unstructured.Unstructured{Object: map[string]any{
+		"apiVersion": "network.test.io/v1",
+		"kind":       "VPCPeering",
+		"metadata":   map[string]any{"name": name, "namespace": namespace},
+		"spec":       map[string]any{"targetVpcRef": map[string]any{"name": targetVPC}},
+	}}
+}
+
+func TestListDependents_NamespaceQualification(t *testing.T) {
+	vpcGVR := schema.GroupVersionResource{Group: "network.test.io", Version: "v1", Resource: "vpcs"}
+	vpcPeeringGVR := schema.GroupVersionResource{Group: "network.test.io", Version: "v1", Resource: "vpcpeerings"}
+
+	entry := RuleEntry{
+		State: &RuleState{
+			DependentGVR: vpcPeeringGVR,
+			DependentGVK: schema.GroupVersionKind{Group: "network.test.io", Version: "v1", Kind: "VPCPeering"},
+		},
+		MatchedField: IndexedField{FieldPath: ".spec.targetVpcRef.name", TargetGVR: vpcGVR},
+	}
+
+	newClient := func(objs ...runtime.Object) *dynamicfake.FakeDynamicClient {
+		return dynamicfake.NewSimpleDynamicClientWithCustomListKinds(
+			runtime.NewScheme(),
+			map[schema.GroupVersionResource]string{vpcPeeringGVR: "VPCPeeringList"},
+			objs...,
+		)
+	}
+
+	v := &DeletionValidator{}
+
+	t.Run("namespaced target renders clean Kind/name", func(t *testing.T) {
+		// Namespaced target: dependents are listed within the target's namespace
+		// only, so names are unique and the namespace segment is redundant.
+		dyn := newClient(vpcPeering("team-a", "peer1", "my-vpc"))
+
+		got, err := v.listDependents(context.Background(), dyn, entry, "my-vpc", "team-a")
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		want := []string{"VPCPeering/peer1"}
+		assertBlockers(t, got, want)
+	})
+
+	t.Run("cluster-scoped target keeps same-named dependents distinct", func(t *testing.T) {
+		// Cluster-scoped target (namespace ""): dependents are listed across all
+		// namespaces, so two same-named objects must not be conflated.
+		dyn := newClient(
+			vpcPeering("team-a", "peer1", "global-vpc"),
+			vpcPeering("team-b", "peer1", "global-vpc"),
+		)
+
+		got, err := v.listDependents(context.Background(), dyn, entry, "global-vpc", "")
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		want := []string{"VPCPeering/team-a/peer1", "VPCPeering/team-b/peer1"}
+		assertBlockers(t, got, want)
+	})
+}
+
+func assertBlockers(t *testing.T, got, want []string) {
+	t.Helper()
+	sort.Strings(got)
+	sort.Strings(want)
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("blockers = %v, want %v", got, want)
+	}
+}
+
+func TestBlockerID(t *testing.T) {
+	tests := []struct {
+		name      string
+		kind      string
+		namespace string
+		objName   string
+		want      string
+	}{
+		{
+			name:    "cluster-scoped dependent has no namespace segment",
+			kind:    "VPCPeering",
+			objName: "peer1",
+			want:    "VPCPeering/peer1",
+		},
+		{
+			name:      "namespaced dependent includes its namespace",
+			kind:      "Order",
+			namespace: "team-a",
+			objName:   "order1",
+			want:      "Order/team-a/order1",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := blockerID(tt.kind, tt.namespace, tt.objName); got != tt.want {
+				t.Errorf("blockerID(%q, %q, %q) = %q, want %q", tt.kind, tt.namespace, tt.objName, got, tt.want)
+			}
+		})
+	}
+
+	// Same name in different namespaces must not collapse to one identity,
+	// which would hide a genuine blocker for cluster-scoped deletions.
+	if a, b := blockerID("Order", "team-a", "order1"), blockerID("Order", "team-b", "order1"); a == b {
+		t.Errorf("distinct dependents collided: both rendered %q", a)
+	}
+}
+
+func TestDedupeBlockers(t *testing.T) {
+	tests := []struct {
+		name string
+		in   []string
+		want []string
+	}{
+		{
+			name: "nil",
+			in:   nil,
+			want: nil,
+		},
+		{
+			name: "no duplicates preserves order",
+			in:   []string{"VPCPeering/peer1", "VPCPeering/peer2"},
+			want: []string{"VPCPeering/peer1", "VPCPeering/peer2"},
+		},
+		{
+			// A single dependent referencing the target via multiple fields
+			// must be listed once.
+			name: "same dependent collapsed to one",
+			in:   []string{"VPCPeering/peer1", "VPCPeering/peer1", "VPCPeering/peer1"},
+			want: []string{"VPCPeering/peer1"},
+		},
+		{
+			name: "keeps distinct dependents, first-seen order",
+			in:   []string{"VPCPeering/peer1", "VPCPeering/peer2", "VPCPeering/peer1"},
+			want: []string{"VPCPeering/peer1", "VPCPeering/peer2"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := dedupeBlockers(tt.in)
+			if len(got) != len(tt.want) {
+				t.Fatalf("dedupeBlockers(%v) = %v, want %v", tt.in, got, tt.want)
+			}
+			for i := range got {
+				if got[i] != tt.want[i] {
+					t.Errorf("index %d = %q, want %q", i, got[i], tt.want[i])
+				}
+			}
+		})
+	}
+}
+
 func FuzzObjectFromRequest(f *testing.F) {
 	for _, seed := range [][]byte{
 		nil,

internal/webhook/rule_registry.go

@@ -142,10 +142,20 @@ func (r *RuleRegistry) AllTargetGVRs() []schema.GroupVersionResource {
 
 // rebuildTargetIndex rebuilds the byTarget reverse index from scratch.
 // Must be called with the write lock held.
+//
+// A rule key is added to a GVR's list at most once, even when the rule
+// references that GVR via multiple fields. FindByTargetGVR expands the matching
+// fields per key; indexing the key once per field would otherwise produce an
+// N-squared cross product of entries for a single rule.
 func (r *RuleRegistry) rebuildTargetIndex() {
 	r.byTarget = make(map[schema.GroupVersionResource][]string)
 	for key, state := range r.rules {
+		seen := make(map[schema.GroupVersionResource]bool)
 		for _, f := range state.IndexFields {
+			if seen[f.TargetGVR] {
+				continue
+			}
+			seen[f.TargetGVR] = true
 			r.byTarget[f.TargetGVR] = append(r.byTarget[f.TargetGVR], key)
 		}
 	}

internal/webhook/rule_registry_test.go

@@ -115,6 +115,39 @@ func TestRuleRegistry_MultipleRulesSameTarget(t *testing.T) {
 	}
 }
 
+func TestRuleRegistry_MultipleFieldsSameTarget(t *testing.T) {
+	r := NewRuleRegistry()
+
+	vpcGVR := schema.GroupVersionResource{Group: "network.test.io", Version: "v1", Resource: "vpcs"}
+
+	// A VPCPeering references the same target type (vpcs) via two fields.
+	state := &RuleState{
+		IndexFields: []IndexedField{
+			{FieldPath: ".spec.sourceVpcRef.name", TargetGVR: vpcGVR},
+			{FieldPath: ".spec.targetVpcRef.name", TargetGVR: vpcGVR},
+		},
+	}
+
+	r.Register("cluster1/vpcpeering-deps", state)
+
+	// FindByTargetGVR must return exactly one entry per matching field — not the
+	// N-squared cross product produced when the rule key is indexed once per field.
+	entries := r.FindByTargetGVR(vpcGVR)
+	if len(entries) != 2 {
+		t.Fatalf("expected 2 entries (one per field), got %d", len(entries))
+	}
+
+	seen := map[string]int{}
+	for _, e := range entries {
+		seen[e.MatchedField.FieldPath]++
+	}
+	for _, f := range state.IndexFields {
+		if seen[f.FieldPath] != 1 {
+			t.Errorf("field %q appeared %d times, want 1", f.FieldPath, seen[f.FieldPath])
+		}
+	}
+}
+
 func TestRuleRegistry_UnregisterNonexistent(t *testing.T) {
 	r := NewRuleRegistry()
 	// Should not panic.