Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
bc41a2e
docs(readme): correct quick start, document Kustomize support, add ro…
sunib Jul 19, 2026
4a0ab92
chore(deps): refresh the version pins Dependabot cannot see, add Reno…
sunib Jul 19, 2026
aad22b3
docs(design): decide the e2e Git server — stay on Gitea, adopt no SDK
sunib Jul 19, 2026
6459ea0
fix(e2e): unpin Gitea to 12.7.0 by deleting the CSRF scraper
sunib Jul 19, 2026
8698a7d
docs(spec): record where validation lives, repo-wide
sunib Jul 20, 2026
b1788b5
docs(finished): trim the multi-cluster attribution record to its deci…
sunib Jul 20, 2026
8f253a2
docs(design): authorize WatchRule source namespaces on the provider
sunib Jul 20, 2026
0e240b0
docs(api): correct the AllowsNamespace enforcement comment
sunib Jul 20, 2026
219e2a0
docs: getting one step further and fixing kcp version mistake
sunib Jul 20, 2026
a8883d1
docs: current stand
sunib Jul 20, 2026
c5c2ec5
Merge remote-tracking branch 'origin/main' into docs-validation-place…
sunib Jul 20, 2026
956ecb6
fix(e2e): pin kcp-operator to chart 0.7.7, not appVersion 0.8.3
sunib Jul 20, 2026
5df7a64
docs(design): split the WatchRule source-namespace design into four P…
sunib Jul 20, 2026
953952b
docs: Folding in more findings
sunib Jul 20, 2026
1b7aeeb
fix(resync): scope the mark-and-sweep by namespace, not only by type
sunib Jul 20, 2026
9dc9df8
docs(design): refresh the source-namespace workstream pages
sunib Jul 20, 2026
4f37759
docs(design): resolve the review contradictions in the source-namespa…
sunib Jul 20, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions api/v1alpha3/clusterprovider_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,13 @@ func (p *ClusterProvider) IsInCluster() bool {
// 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. This is the single authorization predicate shared by the admission webhook and the
// reconcile-time refusal, so the two can never diverge. A malformed selector is a configuration
// error surfaced to the caller (not a silent allow).
// 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
// 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).
func (p *ClusterProvider) AllowsNamespace(nsName string, nsLabels map[string]string) (bool, error) {
policy := p.Spec.AllowedNamespaces
if policy == nil {
Expand Down
5 changes: 4 additions & 1 deletion docs/INDEX.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ misled. Full list in [`spec/README.md`](spec/README.md); the ones that carry a
| [`unsupported-folder-refusal-plan.md`](spec/unsupported-folder-refusal-plan.md) | `GitPathAccepted`, and refusing what we cannot own |
| [`commitrequest-design.md`](spec/commitrequest-design.md) | the CommitRequest window and its conditions |
| [`commitrequest-admission-authorship.md`](spec/commitrequest-admission-authorship.md) | how a real Kubernetes user becomes a commit author |
| [`where-validation-lives.md`](spec/where-validation-lives.md) | schema → CEL → **the reconciler**; a webhook only for what exists solely at admission |
| [`e2e-serial-registry.md`](spec/e2e-serial-registry.md) | which e2e specs must run Serial, and why |

## What is being decided now — [`design/`](design/)
Expand All @@ -66,7 +67,7 @@ says what we support and refuse** — and then its
kustomize field taxonomy, the write boundary, the orchestrator/expansion line, and
how secrets are handled.

Nine other open items:
Eleven other open items:

| Doc | Open question |
|---|---|
Expand All @@ -79,6 +80,8 @@ Nine other open items:
| [`e2e-finish-plan.md`](design/e2e-finish-plan.md) | remaining e2e harness work |
| [`residual-e2e-flakes-2026-06-19.md`](design/residual-e2e-flakes-2026-06-19.md) | Flake B still open |
| [`sensitive-resource-diagnostics-follow-up.md`](design/sensitive-resource-diagnostics-follow-up.md) | deferred diagnostics |
| [`e2e-git-server-choice.md`](design/e2e-git-server-choice.md) | stay on Gitea or move to Forgejo — the `_csrf` pin is fixable in place on both, so the migration is now a preference call, not a fix; also why we adopt no SDK either way |
| [`watchrule-source-namespace/`](design/watchrule-source-namespace/README.md) | letting a WatchRule address a differently-named namespace on its source cluster — a deny-by-default `allowedSourceNamespaces` on the **GitTarget** (so scope is per-tenant, not a provider-wide union), unlocked by a false-by-default delegation flag on the ClusterProvider. Split into five implementable PRs: three prerequisite scope fixes (the namespace-blind resync sweep that would delete other namespaces' manifests, the cluster-wide/named stream collapse, and ClusterWatchRule's unchecked GitTarget attachment), the field and its gate, and the ceiling that makes the allow-list bind ClusterWatchRule too — required because a multi-tenant deployment runs a ClusterWatchRule per tenant from day one to capture CRDs |

## Deferred, but still wanted — [`future/`](future/)

Expand Down
239 changes: 239 additions & 0 deletions docs/design/watchrule-source-namespace/README.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
# PR 1 — the resync sweep must be scoped by namespace, not only by GVR

> Phase 1 of [source-namespace addressing](README.md). **Depends on:** nothing.
> **Blocked every other PR in this folder.** Bug fix — no API change, no CRD regeneration.
>
> **Status: landed.** This was the prerequisite that makes namespace fan-out safe: until it landed,
> any change that let one GitTarget watch a GVR in more than one namespace would **delete Git
> content**. The rest of this page is the record of what was wrong and what shipped; sections below
> are past-tense by design, so a regression is recognisable against them.

## The defect, as it stood before this PR

A per-namespace replay produced a `desired` set covering **one** namespace, but the resulting
mark-and-sweep was scoped by **(group, resource) only**. Every managed document of that type in every
*other* namespace was therefore absent from `desired`, and a sweep deletes what is absent.

The scope was dropped in one place. `targetWatchSpecs` already built per-namespace watch keys:

~~~go
key := targetWatchKey{GVR: wt.GVR, Namespace: ns} // namespace is known here
~~~

but `enqueueReplayResync` passed only the GVR onward:

~~~go
resultCh, enqueued, err := m.EventRouter.enqueueScopedResync(
ctx, gitDest, key.GVR, desired, revision, false) // key.Namespace dropped
~~~

`enqueueScopedResync` then set `scope := gvr`, and `resyncPlan` built a predicate that never looked
at the namespace:

~~~go
inScope := func(ri types.ResourceIdentifier) bool {
return ri.Group == gvr.Group && ri.Resource == gvr.Resource
}
~~~

`ri` is a `types.ResourceIdentifier`, which **already carried `Namespace`**. The information was
present at both ends and discarded in the middle.

## What the tree looks like now

The scope travels end to end as one value:

- `git.ResyncScope` carries GVR **and** Namespace, and owns the match predicate
([types.go](../../../internal/git/types.go)) — `ResyncRequest` and `PendingWrite` both hold it, so
the two halves of a scope cannot be separated in transit.
- `resyncScopeForWatchKey` is the single watch-key → scope conversion
([event_router.go:207-213](../../../internal/watch/event_router.go#L207-L213)), and
`enqueueReplayResync` passes `resyncScopeForWatchKey(key)`
([target_watch.go:585-586](../../../internal/watch/target_watch.go#L585-L586)), so `key.Namespace`
is preserved rather than dropped.
- `resyncPlan` matches through `scope.Matches`
([resync_flush.go:475](../../../internal/git/resync_flush.go#L475)); an empty `Namespace` keeps
the whole-GVR meaning a genuinely cluster-wide stream needs.
- `resyncHealKey` separates namespaces, so a parked heal for one no longer replaces another's.

## Why it was latent, and live the moment anything else lands

It could not fire before this PR because a GitTarget could only ever watch one namespace per GVR:
`WatchRule.targetRef` is a `LocalTargetReference` with no namespace field, so every WatchRule using a
target lives in that target's namespace, and a ClusterWatchRule's `""` key collapses the whole type
to all-namespaces (which is a correct whole-GVR sweep). One named namespace, or none.

Each of the remaining PRs breaks that invariant, independently — which is why this one went first:

| PR | New source of multi-namespace fan-out on one GVR |
|---|---|
| [PR 2](pr2-stream-scope-collapse.md) | Named and cluster-wide selections become distinct concurrent streams for the same GVR. |
| [PR 4](pr4-source-namespace-field.md) | Two WatchRules in the target's namespace can carry different `sourceNamespace` values. |
| [PR 5](pr5-clusterwatchrule-source-ceiling.md) | A declared ceiling expands one cluster-wide selection into N per-namespace selections. |

So this was not a defect to fix opportunistically alongside the feature. It is the load-bearing floor
under all three, and the failure mode is silent data loss in a tenant's repository — a replay for
`team-a` removing `team-b`'s manifests of the same type.

## The fix that shipped

**Thread the namespace through the scope, and match on it.** Concretely:

1. The scoped-resync request carries a namespace alongside its GVR — `enqueueScopedResync` takes a
`git.ResyncScope` instead of a bare `gvr`, and carries it into `git.ResyncRequest` and
`PendingWrite`.
2. `resyncPlan`'s predicate became `scope.Matches`: when the scope names a namespace it requires
`ri.Namespace == scope.Namespace` in addition to group and resource. An empty scope namespace
keeps the whole-GVR meaning, which is what a genuinely cluster-wide stream needs.
3. Every other `enqueueScopedResync` caller was audited for the same drop. The `heal: true` path and
any other scoped-resync producer passes a scope consistent with the `desired` set it built, and
`resyncHealKey` includes the namespace so two namespaces' parked heals no longer collide.

The invariant now held, and stated in the code comment: **the sweep scope must be exactly the scope
the `desired` set was gathered over.** A `desired` narrower than its sweep scope deletes; a `desired`
wider than its sweep scope silently leaves content unmanaged. This is the rule that was violated.

Having one conversion function (`resyncScopeForWatchKey`) rather than a namespace parameter threaded
by hand is the part that makes it stay fixed: there is no second place for a caller to forget.

### Alternative considered: coalesce into one authoritative snapshot

Rather than making the scope finer, make `desired` wider: gather every watched namespace for a GVR
into a single snapshot and keep sweeping by GVR. This is attractive because it yields one
authoritative picture per type and removes a class of partial-scope reasoning entirely.

It is rejected for this PR because it couples the replay lifecycles of independent streams: one
namespace's watch failing or resuming late would hold up or falsify the whole type's snapshot, and
streams start and stop independently by design. Scope-narrowing is also the smaller, more directly
testable change. Revisit coalescing if per-namespace replay volume becomes the problem.

## Revocation leaves prior content — a decision, not an oversight

Stopping a stream does not remove what it already wrote. When a namespace leaves a watch set — a
[PR 5](pr5-clusterwatchrule-source-ceiling.md) ceiling tightening, a WatchRule deletion, a revoked
label — its manifests remain in Git.

**Recommended: retain, and make it visible.** Deleting a tenant's manifests as a side effect of a
policy edit is destructive, hard to undo in the moment, and easy to trigger by accident (a typo in a
selector). Retention is also the safe direction under the failure mode in
[PR 5](pr5-clusterwatchrule-source-ceiling.md#2b-unknown-is-not-empty): if an unavailable selector were
ever read as an empty allow-list, a sweep-on-revocation would erase the target's entire namespaced
content.

The cost is real and must be documented rather than glossed: after a revocation, Git holds manifests
from a namespace the policy no longer admits, and no automatic process removes them. Removing them is
a deliberate operator action. Whichever way this is settled, it must be settled explicitly and
covered by a test — the failure to avoid is discovering the behavior in production.

## Tests that shipped

- **`TestResync_NamespaceScopedSweepLeavesSiblingNamespacesAlone`** — a GitTarget managing one GVR in
`team-a` and `team-b`, replaying only `team-a`; `team-b`'s manifests survive untouched. This is the
test that failed before the fix and is the whole point of the PR.
- **`TestResync_NamespaceScopedSweepStillDropsOrphansInItsOwnNamespace`** — an object removed from
`team-a` while `team-a` replays is still swept. The fix narrows the sweep; it must not turn it off.
- **`TestResync_ClusterWideScopeStillSweepsEveryNamespace`** — a genuinely cluster-wide stream (empty
scope namespace) still sweeps every namespace for its type, so PR 2's cluster-wide half is
unaffected.
- **`TestResyncScopeForWatchKey_CarriesBothHalvesOfTheScope`** — the scope/`desired` agreement
invariant asserted directly, so a future caller that drops the namespace again fails here rather
than in a tenant's repo.
- **`TestResyncScope_MatchesRespectsTypeAndNamespace`**,
**`TestResyncHealKey_SeparatesNamespacesOfTheSameType`**,
**`TestResyncScope_StringIsNilSafeAndNamesTheNamespace`** — the predicate, the heal-key split, and
nil-safe formatting.

Verified by reverting the namespace half of `ResyncScope.Matches`:
`TestResync_NamespaceScopedSweepLeavesSiblingNamespacesAlone` and the sibling-namespace row of
`TestResyncScope_MatchesRespectsTypeAndNamespace` both fail without the fix.

## Done — with one item carried forward

- ✅ A scoped resync carries a namespace end to end, and the plan predicate honours it.
- ✅ Multi-namespace replay is proven non-destructive by test.
- ✅ `task lint`, `task test`, `task test-e2e` pass.
- ⏭ **Retention-on-revocation is documented above but not yet enforced by a test.** Nothing in this
PR can revoke a namespace — no code path removes one from a watch set yet — so the test has no
subject until [PR 5](pr5-clusterwatchrule-source-ceiling.md) introduces ceiling tightening. It is
carried as `TestCeiling_UnknownScopeRetainsPreviousAndDoesNotSweep` and the revocation envtest in
PR 5's plan. Recording it here rather than silently dropping it.
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# PR 2 — a cluster-wide selection must not collapse named-namespace scoping

> Phase 2 of [source-namespace addressing](README.md). **Depends on:**
> [PR 1](pr1-namespace-scoped-resync.md) — this PR is the first thing that makes a GitTarget watch one
> GVR in two namespaces at once, which is unsafe until the resync sweep is namespace-scoped.
> **Blocks:** PR 4 (the gate is only as good as the stream scoping underneath it) and PR 5.
> Bug fix — no API change, no CRD regeneration.
>
> **Status: landed.** `SnapshotNamespaces` is now `WatchedType.WatchScopes`, which returns every
> namespace scope including the cluster-wide `""` instead of collapsing to it, and both read sites
> project one stream per scope: `targetWatchSpecs` keys a watch per scope with that scope's own
> operation set, and `snapshotGVRsFromTable` emits one `snapshotGVR` per scope (its `namespaces`
> slice became a single `namespace`, matching `targetWatchKey`). The old assertion was deleted and
> replaced; the three replacements fail against the pre-fix collapse.

## The defect

`SnapshotNamespaces()` returns `nil` when a `WatchedType` has the `""` namespace key, and `nil`
means *all namespaces* at every read site
([watched_type_table.go:78-92](../../../internal/watch/watched_type_table.go#L78-L92)). So a
WatchRule scoped to one namespace and a ClusterWatchRule scoped cluster-wide, on the **same GVR and
the same GitTarget**, fold into one `WatchedType` — and the cluster-wide entry wins. The named
namespace survives only in the plan hash.

The operation sets collapse the same way, not just the namespaces: `targetWatchSpecs` uses
`operationSpec(wt.NamespaceOps[""])` and discards the per-namespace op sets
([target_watch.go:214-224](../../../internal/watch/target_watch.go#L214-L224)). A `CREATE`-only
named rule co-resident with an `UPDATE` cluster-wide rule loses its filter too.

### Why it matters to this workstream

Under [PR 4](pr4-source-namespace-field.md) this is a gate bypass. A ClusterWatchRule may
legitimately select every source namespace once its GitTarget passes provider admission — but a
co-resident WatchRule must not silently inherit that cluster-wide stream. Otherwise a WatchRule
authorized only for `repo-config` receives events from every namespace the credential can read, and
its `allowedSourceNamespaces` check passed only *before* the data plane widened it.

[PR 5](pr5-clusterwatchrule-source-ceiling.md) removes the `""` key **for the namespaced selections**
of any target with a declared ceiling — `scope: Cluster` rules keep emitting `""`, because a
namespace allow-list cannot constrain cluster-scoped types. So the collapse cannot trigger for a
namespaced GVR under a ceiling, but a target that mirrors a GVR both cluster-scoped and namespaced is
not covered by that, and the far more common undeclared case is not covered at all. This PR is what
governs both.

### This behavior is currently asserted as intended

`TestBuildWatchedTypeTable_ClusterWideOverridesNamedNamespaces`
([watched_type_table_test.go:64-82](../../../internal/watch/watched_type_table_test.go#L64-L82))
documents it as "matching the historic `gvrSnapshotEntry` collapse". So this is a design-intent
versus security-intent conflict, not an oversight. The fix must consciously **replace** that test,
not work around it — leaving it green would mean the fix did not land.

## Verified mechanism

`buildWatchedTypeTable`
([watched_type_table.go:128-155](../../../internal/watch/watched_type_table.go#L128-L155)) is a pure
union: both the `""` key (ClusterWatchRule) and `"team-a"` (WatchRule) land in the same
`namespaceOps` map for the same GVR. The collapse happens later, at *read* time — `ClusterWide()`
merely tests for presence of the `""` key
([watched_type_table.go:73-76](../../../internal/watch/watched_type_table.go#L73-L76)), so
`SnapshotNamespaces()` short-circuits to `nil`. Two read sites consume that `nil` as all-namespaces:

- `targetWatchSpecs` ([target_watch.go:214-224](../../../internal/watch/target_watch.go#L214-L224))
- `snapshotGVRsFromTable` ([scope_resolve.go:180](../../../internal/watch/scope_resolve.go#L180))

The `team-a` entry survives in `NamespaceOps` for the plan hash only.

## The fix

Keep cluster-wide and named selections as distinct streams for the same GVR, or subtract the named
scope from the cluster-wide one — and preserve the per-namespace operation sets either way. Both
read sites above must agree; a fix applied to one of them is worse than no fix, because the plan hash
and the running streams then disagree.

> **Do not land this before [PR 1](pr1-namespace-scoped-resync.md).** Distinct concurrent streams for
> one GVR is precisely the fan-out the resync sweep mishandles: the named stream's replay carries a
> `desired` set for one namespace, while the sweep it triggers is scoped to the whole type. Fixing
> the collapse first therefore converts a silent over-watch into silent deletion of the cluster-wide
> stream's manifests.

## Tests

- **Replacement for the old assertion:** a WatchRule scoped to `team-a` and a cluster-wide
ClusterWatchRule on the same GVR and GitTarget must **not** collapse to one all-namespaces stream.
This replaces `TestBuildWatchedTypeTable_ClusterWideOverridesNamedNamespaces`.
- **Operation sets:** a `CREATE`-only named rule co-resident with an `UPDATE` cluster-wide rule
preserves both op sets.
- **Both read sites:** assert on `targetWatchSpecs` *and* `snapshotGVRsFromTable`, so a fix that
lands in one path only is caught here rather than as a resync anomaly later.

## Done when

- The old test is deleted, not skipped, and its replacement asserts non-collapse.
- Named and cluster-wide streams for one GVR are independently observable in both read paths.
- `task lint`, `task test`, `task test-e2e` pass.
Loading