feat(api)!: scope by kind — per-item WatchRule source namespaces, cluster-only ClusterWatchRule

.coverage-baseline

@@ -1 +1 @@
-77.0
+77.8

AGENTS.md

@@ -69,6 +69,18 @@ workflow or Dockerfile change is covered by the normal lint gate; you can also r
 - Include JSON tags and field descriptions
 - Run `task manifests` to update CRDs
 - Test CRD installation and usage
+- **Keep CRD descriptions to the contract.** A doc comment on a type or field becomes the
+  `kubectl explain` text, so it should say what the field does, what its values mean, and the
+  gotchas — not why it was designed that way. Put design rationale in a comment block separated
+  from the doc comment **by a blank line**: Go does not treat it as the doc comment, so
+  controller-gen leaves it out of the schema while it stays in the source.
+- **Where that block goes matters, and getting it wrong fails silently.** On a FIELD, put it above
+  the doc comment. On a TYPE, put it above the `+kubebuilder:` marker block, never between the
+  markers and the doc — that displaces the markers and they are dropped without any error
+  (a `+kubebuilder:resource:scope=Cluster` was silently lost this way, flipping a CRD to
+  Namespaced). After editing API comments, confirm only descriptions moved:
+  regenerate into a scratch dir with `controller-gen crd paths=./api/... output:crd:artifacts:config=<dir>`,
+  then compare against `config/crd/bases` with every `description` key stripped from both.
 
 ### Git Operations (`internal/git/`)
 - Handle Git errors gracefully

api/v1alpha3/clusterprovider_types.go

@@ -5,7 +5,6 @@ package v1alpha3
 import (
 	meta "github.com/fluxcd/pkg/apis/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
 )
 
 // DefaultClusterProviderName is the conventionally opinionated ClusterProvider name that an
@@ -39,24 +38,6 @@ type ClusterProviderReference struct {
 	Name string `json:"name"`
 }
 
-// AllowedNamespaces is the deny-by-default namespace-access policy a ClusterProvider carries.
-// A cluster-scoped provider holds a credential that can read a lot of a remote cluster; any
-// GitTarget that references it makes the operator mirror that cluster's state into the target's
-// destination. So which namespaces may reference the provider is authorization, not routing: an
-// empty policy (no names, no selector) means NO namespace may reference the provider. Names and
-// selector are ORed — a namespace is allowed if it is listed OR matches the selector.
-type AllowedNamespaces struct {
-	// Names is an explicit allow-list of namespace names that may reference this provider.
-	// +optional
-	// +listType=set
-	Names []string `json:"names,omitempty"`
-
-	// Selector is a label selector matched against Namespace labels; a namespace whose labels
-	// match may reference this provider. ORed with Names.
-	// +optional
-	Selector *metav1.LabelSelector `json:"selector,omitempty"`
-}
-
 // ClusterProviderSpec defines the desired state of ClusterProvider.
 //
 // kubeConfig is IMMUTABLE and OPTIONAL: which physical cluster a provider name means must not
@@ -75,21 +56,55 @@ type AllowedNamespaces struct {
 // but permits the empty string; an empty name can never resolve a Secret, so reject it here.
 // +kubebuilder:validation:XValidation:rule="!has(self.kubeConfig) || !has(self.kubeConfig.secretRef) || size(self.kubeConfig.secretRef.name) > 0",message="spec.kubeConfig.secretRef.name must not be empty"
 type ClusterProviderSpec struct {
-	// KubeConfig names the SOURCE CLUSTER this provider represents and the credentials to reach
-	// it (Flux's meta.KubeConfigReference, embedded verbatim). OMITTED means the operator's own
-	// in-cluster cluster, for any provider name. IMMUTABLE. The referenced Secret is
-	// resolved from the operator's namespace — the credential for a cluster never has to live on
-	// that cluster. When secretRef.key is empty the resolver reads "value" then "value.yaml"
-	// (Flux's order). Only secretRef is honored (configMapRef is rejected); unsafe kubeconfigs
-	// (exec auth, insecure-skip-tls-verify) are rejected with a Validated=False reason unless the
+	// KubeConfig names the SOURCE CLUSTER this provider represents and the credentials to reach it
+	// (Flux's meta.KubeConfigReference, embedded verbatim). OMITTED means the operator's own
+	// in-cluster cluster, for any provider name. IMMUTABLE.
+	//
+	// The referenced Secret is resolved from the operator's namespace, so a cluster's credential
+	// never has to live on that cluster. Only secretRef is honored; configMapRef is rejected. When
+	// secretRef.key is empty the resolver reads "value" then "value.yaml" (Flux's order). Unsafe
+	// kubeconfigs (exec auth, insecure-skip-tls-verify) are rejected with Validated=False unless the
 	// operator opts in via flags.
 	// +optional
 	KubeConfig *meta.KubeConfigReference `json:"kubeConfig,omitempty"`
 
-	// AllowedNamespaces is the deny-by-default policy for which namespaces may reference this
-	// provider from a GitTarget. Empty (or omitted) means no namespace may reference it.
+	// AllowedNamespaces is the deny-by-default policy for which CONTROL-CLUSTER namespaces may
+	// reference this provider from a GitTarget. Empty (or omitted) means no namespace may
+	// reference it. Its selector matches labels on Namespaces in the control cluster — the
+	// cluster the operator's own CRs live in — never on the source cluster this provider names.
 	// +optional
-	AllowedNamespaces *AllowedNamespaces `json:"allowedNamespaces,omitempty"`
+	AllowedNamespaces *NamespaceMatcher `json:"allowedNamespaces,omitempty"`
+
+	// Design rationale, kept out of the generated CRD description by the blank line below.
+	//
+	// Remote and in-cluster providers use the same mechanism but deserve very different sign-off.
+	// For a REMOTE provider the config-plane namespace and the source namespace are on different
+	// clusters, so their sharing a name never was a boundary and naming one widens nothing. For an
+	// IN-CLUSTER provider (kubeConfig omitted) the same-name coupling WAS the boundary: setting this
+	// deliberately bypasses live namespace RBAC, letting the owner of an admitted GitTarget in one
+	// namespace mirror another namespace's objects — read through the operator's own cluster-wide
+	// credential — into a Git destination they control. That is legitimate for a cluster-admin to
+	// grant on purpose, and must never happen by default or as a side effect of another field,
+	// which is why this exists and defaults to false. LOCALITY is not the switch: in-cluster-ness
+	// follows from spec.kubeConfig, and neither that nor the provider's name decides this.
+	//
+	// A wildcard needs the flag for the same reason a named namespace does: it requests the
+	// target's policy SET, so a later policy edit could otherwise widen the watch with no
+	// platform-admin opt-in.
+
+	// AllowSourceNamespaceOverride delegates SOURCE-namespace selection to the GitTargets this
+	// provider admits. While false (the default) a WatchRule mirroring through this provider may
+	// watch only its OWN namespace, whatever any GitTarget policy says.
+	//
+	// It grants no access by itself: an admitted GitTarget must still admit the namespace in its
+	// spec.allowedSourceNamespaces, and the source credential's own RBAC remains the hard maximum.
+	// What it delegates is the AUTHORITY to choose, so set it only when the owners of admitted
+	// GitTargets are trusted to pick a subset of what that credential may read. Every
+	// cross-namespace request needs it, including a rules[].sourceNamespace of "*". It does not
+	// apply to ClusterWatchRule, which selects no namespaces at all.
+	// +optional
+	// +kubebuilder:default=false
+	AllowSourceNamespaceOverride bool `json:"allowSourceNamespaceOverride,omitempty"`
 
 	// QPS overrides the operator's outgoing kube-client query-per-second throttle for this
 	// cluster's watches and discovery. Omitted, the operator-wide --source-cluster-qps applies.
@@ -132,19 +147,17 @@ type ClusterProviderStatus struct {
 // +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].message`,priority=1
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 
-// ClusterProvider is the cluster-scoped, read-side peer of GitProvider: it names a SOURCE cluster
-// a GitTarget mirrors FROM, and is the home for that cluster's connectivity credential
-// (spec.kubeConfig), namespace-access authorization (spec.allowedNamespaces), and per-cluster
-// status. Its NAME is the cluster's identity everywhere: the /audit-webhook/<name> ingress route
-// and the attribution fact-index key. No name is special: "default" is merely the name
-// GitTarget.spec.clusterProviderRef defaults to when omitted. Whether a provider is the operator's
-// own cluster or a remote follows from spec.kubeConfig (omitted = in-cluster), not from its name,
-// so "default" may just as well name a remote cluster.
+// ClusterProvider is the cluster-scoped, read-side peer of GitProvider: it names a SOURCE cluster a
+// GitTarget mirrors FROM, and owns that cluster's connectivity credential (spec.kubeConfig),
+// namespace-access authorization (spec.allowedNamespaces), and per-cluster status. Its NAME is the
+// cluster's identity everywhere — the /audit-webhook/<name> ingress route and the attribution
+// fact-index key. No name is special: "default" is merely what an omitted
+// GitTarget.spec.clusterProviderRef points at, and it may just as well name a remote cluster, since
+// in-cluster-ness follows from spec.kubeConfig (omitted = in-cluster) rather than from the name.
 //
-// Security model:
-//   - ClusterProvider is cluster-scoped and requires platform-admin permissions to create.
-//   - A GitTarget may reference it only from a namespace its spec.allowedNamespaces admits
-//     (deny-by-default), enforced at admission AND before any watch starts.
+// It is cluster-scoped and requires platform-admin permissions to create. A GitTarget may reference
+// it only from a namespace spec.allowedNamespaces admits — deny-by-default, enforced at admission
+// and again before any watch starts.
 type ClusterProvider struct {
 	metav1.TypeMeta `json:",inline"`
 
@@ -188,26 +201,21 @@ func (p *ClusterProvider) IsInCluster() bool {
 // covers a policy tightened after the GitTarget was created, which an admission webhook could not
 // see. There is no admission webhook for this (docs/spec/where-validation-lives.md). A malformed
 // selector is a configuration error surfaced to the caller (not a silent allow).
+// A malformed selector is a configuration error surfaced to the caller (not a silent allow).
+//
+// It is one of two thin wrappers over NamespaceMatcher.Matches — the other being
+// GitTarget.AllowsSourceNamespace — so the control-cluster and source-cluster policies can never
+// drift in their deny-by-default, names-OR-selector semantics. The labels passed here are always
+// CONTROL-cluster Namespace labels.
 func (p *ClusterProvider) AllowsNamespace(nsName string, nsLabels map[string]string) (bool, error) {
-	policy := p.Spec.AllowedNamespaces
-	if policy == nil {
-		return false, nil
-	}
-	for _, n := range policy.Names {
-		if n == nsName {
-			return true, nil
-		}
-	}
-	if policy.Selector != nil {
-		sel, err := metav1.LabelSelectorAsSelector(policy.Selector)
-		if err != nil {
-			return false, err
-		}
-		if sel.Matches(labels.Set(nsLabels)) {
-			return true, nil
-		}
-	}
-	return false, nil
+	return p.Spec.AllowedNamespaces.Matches(nsName, nsLabels)
+}
+
+// AllowsSourceNamespaceOverride reports whether this provider delegates source-namespace selection
+// to the GitTargets it admits. See the field's documentation: false (the default) means a WatchRule
+// mirroring through this provider may watch only its own namespace.
+func (p *ClusterProvider) AllowsSourceNamespaceOverride() bool {
+	return p.Spec.AllowSourceNamespaceOverride
 }
 
 func init() {

api/v1alpha3/clusterwatchrule_types.go

@@ -6,8 +6,11 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// ResourceScope defines the scope of resources.
-// +kubebuilder:validation:Enum=Cluster;Namespaced
+// ResourceScope names a Kubernetes resource's scope. It is an INTERNAL matching vocabulary: the
+// resolver uses both constants to align a rule's selector with the discovered scope of each type
+// (a WatchRule always resolves Namespaced records, a ClusterWatchRule always Cluster ones). The
+// only field that still exposes it — ClusterResourceRule.scope — is narrowed to Cluster alone, so
+// "Namespaced" is no longer a public choice anywhere in the API.
 type ResourceScope string
 
 const (
@@ -50,17 +53,17 @@ type ClusterWatchRuleSpec struct {
 	// +required
 	TargetRef NamespacedTargetReference `json:"targetRef"`
 
-	// Rules define which resources to watch.
+	// Rules define which CLUSTER-SCOPED resources to watch.
 	// Multiple rules create a logical OR - a resource matching ANY rule is watched.
-	// Each rule can specify cluster-scoped or namespaced resources.
+	// A rule that resolves to no cluster-scoped type simply watches nothing; use a WatchRule with
+	// spec.rules[].sourceNamespace for namespaced resources.
 	// +required
 	// +kubebuilder:validation:MinItems=1
 	Rules []ClusterResourceRule `json:"rules"`
 }
 
-// ClusterResourceRule defines which resources to watch with scope control.
-// Each rule independently specifies whether it watches cluster-scoped or
-// namespaced resources.
+// ClusterResourceRule defines which CLUSTER-SCOPED resources to watch. It deliberately has no
+// sourceNamespace: cluster-scoped objects have no namespace, so there is nothing to select.
 type ClusterResourceRule struct {
 	// Operations to watch. If empty, watches all operations (CREATE, UPDATE, DELETE).
 	// Supports: CREATE, UPDATE, DELETE, or * (wildcard for all operations).
@@ -108,13 +111,43 @@ type ClusterResourceRule struct {
 	// +kubebuilder:validation:items:Pattern=`^[^/]*$`
 	Resources []string `json:"resources"`
 
-	// Scope defines whether this rule watches Cluster-scoped or Namespaced resources.
-	// - "Cluster": For cluster-scoped resources (Nodes, ClusterRoles, CRDs, etc.).
-	// - "Namespaced": For namespaced resources (Pods, Deployments, Secrets, etc.),
-	//                 across all namespaces.
-	// +required
-	// +kubebuilder:validation:Enum=Cluster;Namespaced
-	Scope ResourceScope `json:"scope"`
+	// Design rationale, kept out of the generated CRD description by the blank line below.
+	//
+	// The field is retained in the schema purely so that re-applying a manifest that still says
+	// "Namespaced" FAILS. Deleting it outright would be worse and silent twice over: CRD pruning
+	// happens on write, so the value would be dropped without an error and the rule would quietly
+	// stop mirroring namespaced objects; and a stored pre-release object would keep its value in
+	// etcd with no Go field left to read, leaving the controller nothing to refuse. The narrowed
+	// enum rejects it at admission, and the compile path refuses a stored value.
+
+	// Scope is REMOVED as a choice: a ClusterWatchRule is cluster-scoped only, so "Cluster" is the
+	// only accepted value and also the default, making the field omittable. To watch NAMESPACED
+	// resources, use a WatchRule in the tenant namespace and set spec.rules[].sourceNamespace.
+	//
+	// Deprecated: ClusterWatchRule is cluster-scope-only; use WatchRule with
+	// spec.rules[].sourceNamespace for namespaced resources. Removed one release from now, or at
+	// v1beta1.
+	// +optional
+	// +kubebuilder:default=Cluster
+	// +kubebuilder:validation:Enum=Cluster
+	Scope ResourceScope `json:"scope,omitempty"`
+}
+
+// DeclaresNamespacedScope reports whether a STORED ClusterWatchRule still selects namespaced
+// resources through the removed scope choice. Admission rejects the value, but an object written
+// before this release keeps it in etcd, so the compile path must refuse it rather than let the
+// rule resolve as if it had asked for cluster scope.
+//
+// It keys on the STORED value, not on what the selector happens to resolve: `resources: ["*"]`
+// legitimately resolves cluster-scoped records, so inferring the refusal from the resolution would
+// be ambiguous exactly where it matters.
+func (s *ClusterWatchRuleSpec) DeclaresNamespacedScope() bool {
+	for i := range s.Rules {
+		if s.Rules[i].Scope != "" && s.Rules[i].Scope != ResourceScopeCluster {
+			return true
+		}
+	}
+	return false
 }
 
 // ClusterWatchRuleStatus defines the observed state of ClusterWatchRule.
@@ -136,6 +169,13 @@ type ClusterWatchRuleStatus struct {
 	Streams *WatchRuleStreamsStatus `json:"streams,omitempty"`
 }
 
+// Design rationale, kept out of the generated CRD description by the blank line below.
+//
+// Cluster-scoped objects have no namespace, so GitTarget.spec.allowedSourceNamespaces is neither
+// consulted nor a bound for them: a ClusterWatchRule is intentionally cluster-global and is limited
+// only by its source credential's Kubernetes RBAC. Isolating cluster-scoped objects between tenants
+// therefore takes separate credentials/ClusterProviders, not a namespace allow-list.
+
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:scope=Cluster
@@ -147,20 +187,14 @@ type ClusterWatchRuleStatus struct {
 // +kubebuilder:printcolumn:name="StreamsRunning",type=string,JSONPath=`.status.conditions[?(@.type=="StreamsRunning")].status`,priority=1
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 
-// ClusterWatchRule watches resources across the entire cluster.
-// It provides the ability to audit both cluster-scoped resources (Nodes, ClusterRoles, CRDs)
-// and namespaced resources across multiple namespaces with per-rule filtering.
-//
-// Security model:
-//   - ClusterWatchRule is cluster-scoped and requires cluster-admin permissions
-//   - It references a GitTarget via targetRef (namespace required)
-//   - Each rule can independently specify Cluster or Namespaced scope
+// ClusterWatchRule selects CLUSTER-SCOPED resources on the source cluster its GitTarget mirrors
+// from — Nodes, PersistentVolumes, StorageClasses, ClusterRoles, CRDs, and the like. Scope is
+// carried by the rule KIND, so it has no per-rule scope choice and no source-namespace selection.
 //
-// Use cases:
-//   - Audit cluster infrastructure (Nodes, PersistentVolumes, StorageClasses)
-//   - Audit RBAC changes (ClusterRoles, ClusterRoleBindings)
-//   - Audit CRD installations and updates
-//   - Audit resources across multiple namespaces (e.g., all production namespaces)
+// It is cluster-scoped and requires cluster-admin permissions. Its targetRef names a GitTarget
+// (namespace required), whose namespace must be admitted by that target's ClusterProvider. To
+// mirror NAMESPACED resources use a WatchRule in the tenant namespace and set
+// spec.rules[].sourceNamespace, whose "*" reaches every namespace the GitTarget admits.
 type ClusterWatchRule struct {
 	metav1.TypeMeta `json:",inline"`