diff --git a/internal/collect/collection_filter.go b/internal/collect/collection_filter.go index fc7065ca..2c17b97f 100644 --- a/internal/collect/collection_filter.go +++ b/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 diff --git a/internal/controller/kollectclustertarget_unit_test.go b/internal/controller/kollectclustertarget_unit_test.go index e22a82d4..1763d58e 100644 --- a/internal/controller/kollectclustertarget_unit_test.go +++ b/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) + } +} diff --git a/internal/webhook/v1alpha1/kollectclustertarget_scope_webhook_extra_test.go b/internal/webhook/v1alpha1/kollectclustertarget_scope_webhook_extra_test.go index a0588ce7..ebb57332 100644 --- a/internal/webhook/v1alpha1/kollectclustertarget_scope_webhook_extra_test.go +++ b/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") + } +} diff --git a/internal/webhook/v1alpha1/kollectclustertarget_webhook.go b/internal/webhook/v1alpha1/kollectclustertarget_webhook.go index c8142d7c..1f1680ca 100644 --- a/internal/webhook/v1alpha1/kollectclustertarget_webhook.go +++ b/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 + } } }