feat(api)!: move commit batching onto GitTarget, delete allowedSourceNamespaces, and make sourceNamespace "*" cluster-wide

.coverage-baseline

@@ -1 +1 @@
-77.4
+76.9

.markdownlint-cli2.jsonc

@@ -49,7 +49,8 @@
         "jsonc",
         "http",
         "gitignore",
-        "dockerfile"
+        "dockerfile",
+        "csharp"
       ]
     },
 

README.md

@@ -260,7 +260,7 @@ kubectl describe gitprovider,gittarget,watchrule -n gitops-reverser-quickstart-d
 ```
 
 Two `GitTarget` conditions stop the data plane and are worth recognizing: `ClusterProviderNotFound`
-(the `default` `ClusterProvider` is missing) and `NamespaceNotAuthorized` (its `allowedNamespaces`
+(the `default` `ClusterProvider` is missing) and `NamespaceNotAuthorized` (its `accessFrom`
 selector does not cover the demo namespace).
 
 To tear the demo down: `helm uninstall gitops-reverser -n gitops-reverser` and

api/v1alpha3/audit_route_test.go

@@ -62,8 +62,8 @@ func TestClusterProvider_AuditRoute_SharedBySeveralProviders(t *testing.T) {
 	delegating := ClusterProvider{
 		ObjectMeta: metav1.ObjectMeta{Name: "srcns-delegating"},
 		Spec: ClusterProviderSpec{
-			AllowSourceNamespaceOverride: true,
-			Attribution:                  &ClusterProviderAttribution{AuditRoute: "default"},
+			AllowAnySourceNamespace: true,
+			Attribution:             &ClusterProviderAttribution{AuditRoute: "default"},
 		},
 	}
 

api/v1alpha3/clusterprovider_types.go

@@ -68,11 +68,41 @@ type ClusterProviderSpec struct {
 	// +optional
 	KubeConfig *meta.KubeConfigReference `json:"kubeConfig,omitempty"`
 
-	// 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.
+	// Design rationale, kept out of the generated CRD description by the blank line below.
+	//
+	// This is the one namespace policy that survived the source-scope deletion, and it survived
+	// because the boundary it draws is available nowhere else. Source-cluster RBAC bounds what a
+	// CREDENTIAL may read; it cannot express which control-plane tenant may WIELD that credential,
+	// because the tenant is not a subject in the source cluster at all. Deleting it would make a
+	// shared source credential usable from any namespace that can create a GitTarget.
+	//
+	// Its selector is affordable in a way the deleted source-side one was not: it reads
+	// CONTROL-cluster Namespace labels, locally, with no cross-cluster call and no degradation
+	// path. Both halves stay.
+	//
+	// The rename is what makes it readable now that the two allowed*Namespaces fields no longer sit
+	// side by side to disambiguate each other. See docs/design/source-scope-simplification.md.
+
+	// AccessFrom 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
+	AccessFrom *NamespaceMatcher `json:"accessFrom,omitempty"`
+
+	// Design rationale, kept out of the generated CRD description by the blank line below.
+	//
+	// Retained-and-refused rather than deleted: CRD pruning happens on write, so a deleted field
+	// would be dropped from a re-applied manifest with no error, leaving a deny-by-default policy
+	// silently admitting nothing and every GitTarget through it failing for a reason the manifest
+	// does not show.
+
+	// AllowedNamespaces is RENAMED to accessFrom, with the same shape and the same semantics.
+	// Setting this field is rejected.
+	//
+	// Deprecated: use spec.accessFrom. Removed at v1alpha4.
+	// +optional
+	// +kubebuilder:validation:XValidation:rule="false",message="spec.allowedNamespaces is renamed spec.accessFrom. Same shape, same semantics: rename the key."
 	AllowedNamespaces *NamespaceMatcher `json:"allowedNamespaces,omitempty"`
 
 	// Design rationale, kept out of the generated CRD description by the blank line below.
@@ -88,23 +118,45 @@ type ClusterProviderSpec struct {
 	// 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.
+	// The name keeps "Source" deliberately. This object carries two namespace planes, and an
+	// allowAnyNamespace sitting directly beneath accessFrom would read as a modifier on it.
+	// allowCrossNamespace was the other candidate, borrowing Flux's --no-cross-namespace-refs
+	// vocabulary, and it was not taken: in Flux the phrase means references across namespaces in
+	// ONE cluster, while here the far side is a namespace in a DIFFERENT cluster. "Crossing" is
+	// literally true only for the in-cluster provider; "any" is literally true for both.
+	//
+	// It stays a boolean because there are two states and no third one is in view: impersonation
+	// and source-side selectors are both out, so an enum would only leave room for something
+	// nobody can name.
 
-	// 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.
+	// AllowAnySourceNamespace 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.
 	//
-	// 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.
+	// It grants no access by itself: the source credential's own RBAC remains the hard maximum, and
+	// a request it permits still fails 403 if the credential cannot read that namespace. 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 "*", which reaches every namespace the
+	// credential can read. It does not apply to ClusterWatchRule, which selects no namespaces at
+	// all.
 	// +optional
 	// +kubebuilder:default=false
-	AllowSourceNamespaceOverride bool `json:"allowSourceNamespaceOverride,omitempty"`
+	AllowAnySourceNamespace bool `json:"allowAnySourceNamespace,omitempty"`
+
+	// Design rationale, kept out of the generated CRD description by the blank line below.
+	//
+	// Retained-and-refused rather than deleted, and this one matters most of the three: pruning a
+	// stored `true` would silently REVOKE a delegation, stalling every cross-namespace WatchRule
+	// through this provider with a message about a flag the manifest still appears to set.
+
+	// AllowSourceNamespaceOverride is RENAMED to allowAnySourceNamespace: same type, same default,
+	// same semantics. Setting this field is rejected.
+	//
+	// Deprecated: use spec.allowAnySourceNamespace. Removed at v1alpha4.
+	// +optional
+	// +kubebuilder:validation:XValidation:rule="false",message="spec.allowSourceNamespaceOverride is renamed spec.allowAnySourceNamespace. Same type, same default, same semantics: rename the key."
+	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.
@@ -178,7 +230,7 @@ type ClusterProviderStatus struct {
 
 // 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
+// namespace-access authorization (spec.accessFrom), and per-cluster status. Its NAME is the
 // cluster's identity for the watch data plane, and the DEFAULT for its audit route: attribution
 // facts are partitioned by spec.attribution.auditRoute, which falls back to this name. Several
 // providers may name one cluster by declaring one route, which is what an API server with a single
@@ -187,7 +239,7 @@ type ClusterProviderStatus struct {
 // in-cluster-ness follows from spec.kubeConfig (omitted = in-cluster) rather than from the name.
 //
 // 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
+// it only from a namespace spec.accessFrom admits — deny-by-default, enforced at admission
 // and again before any watch starts.
 type ClusterProvider struct {
 	metav1.TypeMeta `json:",inline"`
@@ -240,30 +292,29 @@ func (p *ClusterProvider) AuditRoute() string {
 }
 
 // AllowsNamespace reports whether a namespace (by name and labels) may reference this provider
-// from a GitTarget, per spec.allowedNamespaces. It is DENY-BY-DEFAULT: a provider with no
-// allowedNamespaces policy (neither names nor selector) admits no namespace. Names and selector
-// are ORed. Enforced on every reconcile and NOWHERE else: checkSourceAuthorization in
+// from a GitTarget, per spec.accessFrom. It is DENY-BY-DEFAULT: a provider with no accessFrom
+// policy (neither names nor selector) admits no namespace. Names and selector are ORed. Enforced on
+// every reconcile and NOWHERE else: checkSourceAuthorization in
 // internal/controller/gittarget_source_cluster.go is the only non-test caller, and it returns
 // before DeclareForGitTarget, so an unauthorized target starts no watch and writes no Git.
-// Reconcile-time is deliberate rather than incidental — it re-evaluates continuously, so it also
+//
+// Reconcile-time is deliberate rather than incidental: it re-evaluates continuously, so it also
 // 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).
+// selector is a configuration error surfaced to the caller, never 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.
+// The labels passed here are always CONTROL-cluster Namespace labels. This is now the only caller
+// of NamespaceMatcher.Matches: the source-side twin was deleted with
+// GitTarget.spec.allowedSourceNamespaces.
 func (p *ClusterProvider) AllowsNamespace(nsName string, nsLabels map[string]string) (bool, error) {
-	return p.Spec.AllowedNamespaces.Matches(nsName, nsLabels)
+	return p.Spec.AccessFrom.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
+// AllowsAnySourceNamespace 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 (p *ClusterProvider) AllowsAnySourceNamespace() bool {
+	return p.Spec.AllowAnySourceNamespace
 }
 
 func init() {

api/v1alpha3/clusterwatchrule_types.go

@@ -171,10 +171,10 @@ type ClusterWatchRuleStatus struct {
 
 // 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.
+// Cluster-scoped objects have no namespace, so no namespace policy is 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.
 
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
@@ -194,7 +194,7 @@ type ClusterWatchRuleStatus struct {
 // 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.
+// spec.rules[].sourceNamespace, whose "*" reaches every namespace the source credential can read.
 type ClusterWatchRule struct {
 	metav1.TypeMeta `json:",inline"`
 

api/v1alpha3/gitprovider_types.go

@@ -42,11 +42,25 @@ type GitProviderSpec struct {
 	// +kubebuilder:validation:items:MinLength=1
 	AllowedBranches []string `json:"allowedBranches"`
 
-	// Push controls how events are coalesced into commits before pushing.
+	// 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 sets it
+	// FAILS, with a message naming where it went. Deleting it outright would be silent: CRD pruning
+	// happens on write, so the value would be dropped without an error and the provider would
+	// quietly commit on a cadence nobody asked for. See docs/design/gittarget-api-wave.md
+	// § "Version strategy: stay v1alpha3".
+
+	// Push is REMOVED: commit batching is a property of the folder being written, not of the
+	// connection, so spec.push.commitWindow is now GitTarget.spec.commit.window. Setting this
+	// field is rejected.
+	//
+	// Deprecated: use GitTarget.spec.commit.window. Removed at v1alpha4.
 	// +optional
+	// +kubebuilder:validation:XValidation:rule="false",message="spec.push.commitWindow has moved to GitTarget.spec.commit.window; commit batching belongs to the folder being written, not to the connection. Remove spec.push here and set spec.commit.window on each GitTarget that needs a window other than 5s."
 	Push *PushStrategy `json:"push,omitempty"`
 
-	// Commit configures commit identity, message formatting, and signing behavior.
+	// Commit configures the commit identity and signing behavior this connection uses. Message
+	// formatting moved to GitTarget.spec.commit.message.
 	// +optional
 	Commit *CommitSpec `json:"commit,omitempty"`
 }
@@ -154,16 +168,28 @@ type GitProviderStatus struct {
 	SigningPublicKey string `json:"signingPublicKey,omitempty"`
 }
 
-// CommitSpec configures how gitops-reverser creates commits for a GitProvider.
+// CommitSpec configures the commit identity and signing a GitProvider uses. Message formatting
+// lives on the GitTarget (spec.commit.message), because it describes the folder rather than the
+// connection.
 type CommitSpec struct {
 	// Committer configures the operator identity written as the commit committer.
 	// When signing is enabled, Email must be a verified address on the account
 	// that owns the signing key.
 	// +optional
 	Committer *CommitterSpec `json:"committer,omitempty"`
 
-	// Message configures commit message formatting.
+	// Design rationale, kept out of the generated CRD description by the blank line below.
+	//
+	// Retained-and-refused rather than deleted, for the same reason as spec.push above: a pruned
+	// field is a silent behavior change, and a rejected one is an apply-time error naming the fix.
+
+	// Message is REMOVED: how a commit is phrased is a property of the folder being written, not of
+	// the connection, so it is now GitTarget.spec.commit.message with the same three templates.
+	// Setting this field is rejected.
+	//
+	// Deprecated: use GitTarget.spec.commit.message. Removed at v1alpha4.
 	// +optional
+	// +kubebuilder:validation:XValidation:rule="false",message="spec.commit.message has moved to GitTarget.spec.commit.message, with the same eventTemplate/reconcileTemplate/groupTemplate fields. Remove it here and set it on each GitTarget whose commits it should phrase."
 	Message *CommitMessageSpec `json:"message,omitempty"`
 
 	// Signing configures commit signing.
@@ -184,10 +210,12 @@ type CommitterSpec struct {
 	Email string `json:"email,omitempty"`
 }
 
-// CommitMessageSpec configures commit message formatting.
+// CommitMessageSpec configures commit message formatting. It is set on
+// GitTarget.spec.commit.message; the identically-shaped GitProvider.spec.commit.message is
+// retained only to reject a manifest that still sets it there.
 type CommitMessageSpec struct {
 	// EventTemplate is a Go text/template string for per-event commit messages
-	// (used when commitWindow is "0s"; one event per commit).
+	// (used when spec.commit.window is "0s"; one event per commit).
 	// Available variables: Operation, Group, Version, Resource, Namespace, Name,
 	// APIVersion, Username, GitTarget.
 	// +optional