🐛 fix(webhook): validate spec-derived GVKs when the profile is missing

internal/collect/collection_filter.go

@@ -25,7 +25,9 @@ type NamespaceDefaults struct {
 	Excluded []string
 }
 
-// ScopeCeiling is the resolved tenancy guardrail for namespace and GVK checks.
+// ScopeCeiling is the resolved tenancy guardrail for namespace checks at collect
+// time. GVKs are not filtered here: they are enforced at admission and, as
+// backstop, at reconcile (ScopeGVKDenied).
 type ScopeCeiling struct {
 	AllowedNamespaces []string
 	DeniedNamespaces  []string

internal/controller/kollectclustertarget_unit_test.go

@@ -137,3 +137,72 @@ func TestKollectClusterTargetReconciler_deniedGVKDegrades(t *testing.T) {
 		t.Fatalf("Degraded = %+v, want reason %s", cond, scopeReasonGVKDenied)
 	}
 }
+
+// A permitted profile targetGVK does not launder a denied resourceRules GVK:
+// CollectRuleGVKs returns the rule GVKs and ignores the profile GVK entirely
+// once resourceRules is set, so the reconcile check has to see the rules.
+func TestKollectClusterTargetReconciler_deniedRuleGVKDegrades(t *testing.T) {
+	t.Parallel()
+
+	scheme := runtime.NewScheme()
+	if err := kollectdevv1alpha1.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+	if err := corev1.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+
+	clusterScope := &kollectdevv1alpha1.KollectClusterScope{
+		ObjectMeta: metav1.ObjectMeta{Name: "platform"},
+		Spec: kollectdevv1alpha1.KollectClusterScopeSpec{
+			ScopeCeilingSpec: kollectdevv1alpha1.ScopeCeilingSpec{
+				AllowedGVKs: []kollectdevv1alpha1.GroupVersionKind{
+					{Group: "apps", Version: "v1", Kind: "Deployment"},
+				},
+			},
+		},
+	}
+	profile := &kollectdevv1alpha1.KollectProfile{
+		ObjectMeta: metav1.ObjectMeta{Name: "deployments", Namespace: "kollect-system"},
+		Spec: kollectdevv1alpha1.KollectProfileSpec{
+			TargetGVK: kollectdevv1alpha1.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
+		},
+	}
+	ct := &kollectdevv1alpha1.KollectClusterTarget{
+		ObjectMeta: metav1.ObjectMeta{Name: "cluster-rules", Generation: 1},
+		Spec: kollectdevv1alpha1.KollectClusterTargetSpec{
+			ProfileRef: kollectdevv1alpha1.NamespacedObjectReference{Name: "deployments", Namespace: "kollect-system"},
+			NamespaceSelector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{"team": "platform"},
+			},
+			CollectionFilterSpec: kollectdevv1alpha1.CollectionFilterSpec{
+				ResourceRules: []kollectdevv1alpha1.ResourceRule{
+					{GVK: kollectdevv1alpha1.GroupVersionKind{Version: "v1", Kind: "Secret"}},
+				},
+			},
+		},
+	}
+
+	c := fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithStatusSubresource(ct).
+		WithObjects(clusterScope, profile, ct).
+		Build()
+
+	r := &KollectClusterTargetReconciler{Client: c, Scheme: scheme}
+	if _, err := r.Reconcile(context.Background(), reconcile.Request{
+		NamespacedName: types.NamespacedName{Name: ct.Name},
+	}); err != nil {
+		t.Fatalf("reconcile: %v", err)
+	}
+
+	var got kollectdevv1alpha1.KollectClusterTarget
+	if err := c.Get(context.Background(), types.NamespacedName{Name: ct.Name}, &got); err != nil {
+		t.Fatal(err)
+	}
+
+	cond := apimeta.FindStatusCondition(got.Status.Conditions, conditionDegraded)
+	if cond == nil || cond.Reason != scopeReasonGVKDenied {
+		t.Fatalf("Degraded = %+v, want reason %s", cond, scopeReasonGVKDenied)
+	}
+}

internal/webhook/v1alpha1/kollectclustertarget_scope_webhook_extra_test.go

@@ -94,3 +94,60 @@ func TestKollectClusterTargetValidator_validateClusterScope(t *testing.T) {
 		t.Fatalf("missing in-scope profile should still admit: %v", err)
 	}
 }
+
+func TestKollectClusterTargetValidator_missingProfileSpecDerivedChecks(t *testing.T) {
+	t.Parallel()
+
+	scheme := runtime.NewScheme()
+	if err := kollectdevv1alpha1.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+
+	clusterScope := &kollectdevv1alpha1.KollectClusterScope{
+		ObjectMeta: metav1.ObjectMeta{Name: "platform"},
+		Spec: kollectdevv1alpha1.KollectClusterScopeSpec{
+			ScopeCeilingSpec: kollectdevv1alpha1.ScopeCeilingSpec{
+				AllowedGVKs: []kollectdevv1alpha1.GroupVersionKind{
+					{Group: "apps", Version: "v1", Kind: "Deployment"},
+				},
+			},
+			AllowedStaticRefNamespaces: []string{"kollect-system"},
+		},
+	}
+
+	// No KollectProfile object: profileRef points at one that does not exist yet.
+	cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(clusterScope).Build()
+	v := &kollectClusterTargetValidator{client: cl}
+
+	base := &kollectdevv1alpha1.KollectClusterTarget{
+		ObjectMeta: metav1.ObjectMeta{Name: "ct"},
+		Spec: kollectdevv1alpha1.KollectClusterTargetSpec{
+			ProfileRef: kollectdevv1alpha1.NamespacedObjectReference{Name: "not-created-yet", Namespace: "kollect-system"},
+			NamespaceSelector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{"team": "platform"},
+			},
+		},
+	}
+	if err := v.validateClusterScope(context.Background(), base); err != nil {
+		t.Fatalf("in-scope target must stay admissible before its profile exists: %v", err)
+	}
+
+	// resourceRules GVKs come from the spec, not the profile — CollectRuleGVKs
+	// ignores the profile GVK entirely once resourceRules is set, so a missing
+	// profile is no reason to skip them.
+	deniedRuleGVK := base.DeepCopy()
+	deniedRuleGVK.Spec.ResourceRules = []kollectdevv1alpha1.ResourceRule{
+		{GVK: kollectdevv1alpha1.GroupVersionKind{Version: "v1", Kind: "Secret"}},
+	}
+	if err := v.validateClusterScope(context.Background(), deniedRuleGVK); err == nil {
+		t.Fatal("expected resourceRules GVK violation when the profile is missing")
+	}
+
+	// allowedStaticRefNamespaces is spec-derived too; #304 added the check but no
+	// fixture exercised it, because allowedStaticRefNamespaces was unset there.
+	deniedRefNS := base.DeepCopy()
+	deniedRefNS.Spec.ProfileRef.Namespace = "tenant-b"
+	if err := v.validateClusterScope(context.Background(), deniedRefNS); err == nil {
+		t.Fatal("expected profileRef namespace violation when the profile is missing")
+	}
+}

internal/webhook/v1alpha1/kollectclustertarget_webhook.go

@@ -83,24 +83,27 @@ func (v *kollectClusterTargetValidator) validateClusterScope(
 		return nil
 	}
 
+	var profileGVK kollectdevv1alpha1.GroupVersionKind
+	profileResolved := true
 	profile, err := resolveClusterTargetProfileForWebhook(ctx, v.client, target.Spec.ProfileRef)
-	if err != nil {
-		if apierrors.IsNotFound(err) {
-			intentNS := scope.NormalizeNamespaceList(target.Spec.IncludedNamespaces)
-			if nsErr := scope.ValidateClusterScopeNamespaces(binding.Scope, intentNS); nsErr != nil {
-				return nsErr
-			}
-
-			return scope.ValidateClusterScopeStaticRefNamespace(binding.Scope, target.Spec.ProfileRef.Namespace)
-		}
-
+	switch {
+	case err == nil:
+		profileGVK = profile.Spec.TargetGVK
+	case apierrors.IsNotFound(err):
+		// A ClusterTarget may be applied before its KollectProfile. Only the
+		// profile targetGVK is unknowable here; reconcile re-checks it and
+		// degrades with ScopeGVKDenied. Everything below is spec-derived and
+		// stays enforced so the checks cannot silently diverge per branch.
+		profileResolved = false
+	default:
 		return err
 	}
 
-	gvks := scope.CollectRuleGVKs(target.Spec.CollectionFilterSpec, profile.Spec.TargetGVK)
-	for _, gvk := range gvks {
-		if err := scope.ValidateClusterScopeGVKs(binding.Scope, gvk); err != nil {
-			return err
+	if profileResolved || len(target.Spec.ResourceRules) > 0 {
+		for _, gvk := range scope.CollectRuleGVKs(target.Spec.CollectionFilterSpec, profileGVK) {
+			if err := scope.ValidateClusterScopeGVKs(binding.Scope, gvk); err != nil {
+				return err
+			}
 		}
 	}