diff --git a/docs/INDEX.md b/docs/INDEX.md
index 1f403bbc..dc938927 100644
--- a/docs/INDEX.md
+++ b/docs/INDEX.md
@@ -71,6 +71,7 @@ Fifteen other open items:
|---|---|
| [`open-asks-priority.md`](design/open-asks-priority.md) | three backlogs are open at once — the gitops-api consumer asks, the maintainer review's unbuilt block (F6, F9, F10), and the config-surface proposal (B1–B6) — and they overlap. Merges them into one ordered queue under four stated tests, and makes one design call against what was asked: **delete Option C sibling inference** rather than ship an off-switch for it, because it lets a human's edit to the repository change the operator's behaviour with nothing in status recording the move, its central guard has already failed once by cascading, and the explainability its own spec made mandatory was never built. That answers the namespace-leak ask by removal, and means `spec.placement.mode` is never built. Open: whether the removal ships with an Event on the first fall-back to canonical |
| [`docs-linting.md`](design/docs-linting.md) | how to mechanize [`style-guide.md`](style-guide.md) with markdownlint-cli2 and Vale. Both are wired into `task lint`, gated on the files [`.docs-lint-scope`](../.docs-lint-scope) lists rather than the whole tree: 102 of 174 files fail markdownlint and 148 of 174 fail Vale, so the two backlogs need different gates. Open: how the scope list grows to cover the tree, the `MD013` limit, and whether `AGENTS.md` and the chart READMEs are in scope |
+| [`attribution-deletion-intent-actor.md`](design/attribution-deletion-intent-actor.md) | a finalized deletion used to be attributed to the controller that cleared the finalizer rather than to the human who asked for it. Reproduced from a two-actor mutation-lab capture (`configmap/deletion-intent-actor`, with a tunable hold between the phases) and pinned by a corpus-driven unit test: the human's `delete` and the controller's finalizer `patch` both return a body carrying the resourceVersion the DELETION stamped, so both facts are filed under the same `(uid, resourceVersion)` key and the index is last-writer-wins — the deleter's fact is not outranked, it is replaced. **Built**: a STICKY removal pointer — a fact about a deletion may not be overwritten by a fact about a write — keyed strictly by uid, consulted ahead of the exact tier for a removal, and bounded by the index's caps rather than the join TTL, because a uid is unique across space and time — in memory, so a restart still re-warms from one TTL of stream retention. It ships the `delete_sticky` value on `attribution_resolutions_total{tier}` and collapses the full-grace wait a `Terminating` object seen on replay used to pay. Records why the cheaper alternatives are second: filing delete facts under the name tier answers at a tier that misstates the evidence, and making the exact entry write-once makes correctness depend on fact arrival order and does nothing for replay. Open: nothing |
| [`attribution-removal-wait-options.md`](design/attribution-removal-wait-options.md) | a removal now waits for evidence about the DELETION rather than accepting the object's last write, which stopped it naming whoever last edited the object as the author of a deletion they did not perform. Enumerates the eight situations a resolution can be in and shows the cost is concentrated in exactly one: a removal for which no delete fact will ever arrive (a graceful pod delete, a status-only removal, a type the audit policy skips) spends the whole grace to return the answer it had at t=0, measured at ~3.1s against ~70ms when evidence is present. Prices five options against that, and recommends a per-route watermark — stop waiting once the fact stream has demonstrably moved past this event — over a second timeout flag whose right value lives in the API server's config rather than ours. Open: the decision, and how common the case is outside the e2e suite |
| [`attribution-metrics-proposal.md`](design/attribution-metrics-proposal.md) | a phased attribution metric surface, revised after review cut an earlier draft of thirteen new families down to a first release that covers health and the unseen loss paths. Splits `result` into `tier` and `actor_kind` (how `commits_total` already models it) and `weak` into `latest` and `resource_version`, taking the break in the release that has broken `result` anyway. Adds watch-queue delay, follower error and last-success health, a `no_attribution_fact` outcome on the existing bounded audit vocabulary, and a decode-error counter for the one loss path with no symptom at all: both transports discard an undecodable stream entry and advance past it with no log and no metric. Records what the first draft got wrong and why, including a proposed series that would have been permanently zero and a gauge that would have counted registrations rather than blocked resolvers. Its Phase 1 is now Phase 1 of [`metrics-observability-plan.md`](design/metrics-observability-plan.md), which absorbed the surface and records the drift it had accumulated; this stays as the reasoning trail. **Phase 1 has shipped** — the migration is in [`UPGRADING.md`](UPGRADING.md). Open: nothing structural |
| [`attribution-publish-and-join.md`](design/attribution-publish-and-join.md) | the reference for what attribution's two halves each do, exactly: the publish side that turns one audit event into zero or one fact and files it under the keys it happens to have, and the join side that walks the tiers strongest-first to name an author for a watch event. A flowchart per half, the tier table, and the two rules that are easy to miss (a removal never answers with a write fact without looking further; an exact-capable event may never fall through to the removal tiers). Also answers whether anything special-cases a type: nothing does, every branch is on the verb or on which fields are present, and the two ConfigMap deletes in the corpus — one answered with the object, one with a `Status` — are the standing argument that a type-based rule would be unsound |
@@ -116,7 +117,7 @@ index. It deleted the `SET`/`GET` fact keys, the 150ms poll loop, and the `delet
expander, replacing the expander with one collection fact that every removal in its scope joins by
uid membership or by scope. That last part is a capability gain rather than a like-for-like swap: a
collection delete the API server sent no response body for used to lose its author entirely.
-`exact_deletecollection_item` is replaced by `collection_uid` and `collection_scope`, and
+`exact_deletecollection_item` is replaced by `deletecollection_body_uid` and `deletecollection_scope`, and
`--author-attribution-transport=memory` runs attribution with no Redis on a single replica.
Shipped as #283, #284, #286 and #287.
diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md
index 3dfe078d..7ccb8be5 100644
--- a/docs/UPGRADING.md
+++ b/docs/UPGRADING.md
@@ -70,7 +70,25 @@ because it covered two different kinds of it:
| `exact_serviceaccount` | `tier="exact"`, `actor_kind="serviceaccount"` |
| `weak` (a UID-latest match) | `tier="latest"` |
| `weak` (the RV-only escape hatch) | `tier="resource_version"` |
-| `collection_uid`, `collection_scope`, `name`, `absent` | `tier=` the same value |
+| `collection_uid` | `tier="deletecollection_body_uid"` |
+| `collection_scope` | `tier="deletecollection_scope"` |
+| `name`, `absent` | `tier=` the same value |
+
+The two collection values are renamed for the **verb that produced the fact** and how it matched, the
+convention the three removal-only tiers now share: `delete_sticky`, `deletecollection_body_uid`,
+`deletecollection_scope`. `body` is in the second because the uid set comes from the response body,
+which is the part the API server may not send; when it does not, the same fact resolves at
+`deletecollection_scope` instead. `latest` and `name` keep their names, because either can hold a
+delete fact or a write and so cannot claim a verb.
+
+`delete_sticky` is a value `result` never had at all, and it comes with a behaviour change. A
+finalized deletion — a human deletes, a controller clears the finalizer — is attributed to the human
+and resolves at `tier="delete_sticky"`. Before this release it named the controller and was counted
+on the exact path (`result="exact_user"` or `result="exact_serviceaccount"`), because the finalizer
+patch's fact carries the resourceVersion the *deletion* stamped and overwrote the deleter's under
+the same key. A dashboard therefore sees the exact path shift toward `delete_sticky` for types that
+carry finalizers, and `commits_total{author_kind}` shift from `serviceaccount` toward `user`. A query
+that enumerates tiers explicitly needs the three new values; `tier!="absent"` covers them already.
`actor_kind` is `user`, `serviceaccount`, or `none`, the vocabulary
`gitopsreverser_commits_total{author_kind}` already uses, and it is available on **every** tier
diff --git a/docs/architecture.md b/docs/architecture.md
index 1e6e355a..71118150 100644
--- a/docs/architecture.md
+++ b/docs/architecture.md
@@ -717,21 +717,30 @@ same `user`/`serviceaccount`/`none` vocabulary `commits_total{author_kind}` uses
| Tier | Key | What it asserts |
|---|---|---|
+| `delete_sticky` | uid (a delete fact, sticky) | who asked for this object's deletion |
| `exact` | uid + rv | this actor produced this exact version |
-| `collection_uid` | uid in a collection fact's set | the API server said this request deleted this object |
+| `deletecollection_body_uid` | uid in a collection fact's set | the API server said this request deleted this object |
| `latest` | uid (the object's own delete fact) | who removed it |
| `name` | namespace + name | the same, for a fact with no uid (an aggregated API's usual shape) |
| `latest` | uid (a write fact) | who last *wrote* it; a fallback for a removal |
-| `collection_scope` | namespace + selector + window | a collection request covering it was made |
+| `deletecollection_scope` | namespace + selector + window | a collection request covering it was made |
| `resource_version` | rv (the escape hatch for a fact with no uid) | a fact for this exact version, unidentified |
| `absent` | none | nothing usable arrived in time |
-Two rules carry most of the behavior. **A removal never returns on a write fact without looking
-further:** the per-object tiers are last-writer-wins, so for a removal they hold whoever last *edited*
-the object, which is not who deleted it; such a match is held as a fallback while the wait continues
-for evidence about the deletion itself. And **an exact-capable event may not fall through to the
-removal tiers:** a create or update presents the resourceVersion its own write produced, so if the
-exact tier misses, the uid pointer may name an older, different author.
+Three rules carry most of the behavior. **A fact about a deletion may not be replaced by a fact about
+a write:** a removal fact takes a sticky, uid-keyed slot that the ordinary last-writer-wins
+structures cannot reach, and a removal asks that slot first. That slot is the one structure the fact
+TTL does not bound: a uid is unique across space and time, so the statement cannot be superseded,
+and its horizon is the index's caps instead. It is still in-memory: a restart re-warms the index
+from one TTL of stream retention like everything else. Without the slot, a finalizer patch's fact
+overwrites the deleter's, because both carry the resourceVersion the *deletion* stamped
+([attribution-deletion-intent-actor.md](design/attribution-deletion-intent-actor.md)). **A removal
+never returns on a write fact without looking further:** the per-object tiers are last-writer-wins,
+so for a removal they hold whoever last *edited* the object, which is not who deleted it; such a
+match is held as a fallback while the wait continues for evidence about the deletion itself. And
+**an exact-capable event may not fall through to the removal tiers:** a create or update presents
+the resourceVersion its own write produced, so if the exact tier misses, the uid pointer may name an
+older, different author.
No branch on this path depends on the type. Every decision is made on the verb, on whether the event
is a removal, and on which fields the event happens to carry. That is why an aggregated API needed
diff --git a/docs/design/attribution-branch-findings.md b/docs/design/attribution-branch-findings.md
index 45955ee4..263a6e90 100644
--- a/docs/design/attribution-branch-findings.md
+++ b/docs/design/attribution-branch-findings.md
@@ -49,7 +49,7 @@ No `uid`. No `resourceVersion`. No `responseObject` to recover either from. Mean
Corpus `flunder/aggregated-api-deletecollection/`. One audit record, name-less, with the selector
visible only in the `requestURI`, and no response body: the collection fact therefore carries no uid
set and the join can only proceed by scope. This is precisely the case the deleted response-body expander
-produced nothing at all for, and it is the case `collection_scope` exists to serve.
+produced nothing at all for, and it is the case `deletecollection_scope` exists to serve.
The scenario deletes three flunders to make the asymmetry unmistakable: three watch `DELETED` events
and three admission records against one audit record that names none of them.
@@ -68,7 +68,7 @@ flowchart TD
F -->|neither: name only| H[name tier
namespace + name]
G --> I[exact tier: uid + rv
latest tier: uid]
- E --> J[collection_uid if a uid set arrived
collection_scope otherwise]
+ E --> J[deletecollection_body_uid if a uid set arrived
deletecollection_scope otherwise]
C --> K[Committer-authored]
H --> L[Attributed to the actor]
diff --git a/docs/design/attribution-deletion-intent-actor.md b/docs/design/attribution-deletion-intent-actor.md
new file mode 100644
index 00000000..8ce3ed19
--- /dev/null
+++ b/docs/design/attribution-deletion-intent-actor.md
@@ -0,0 +1,276 @@
+# A finalized deletion is attributed to the cleanup controller, not to the deleter
+
+> **design** — built. Index: [`../INDEX.md`](../INDEX.md)
+>
+> Status: BUILT. The defect was reproduced from a mutation-lab capture
+> (`configmap/deletion-intent-actor`) and pinned by a corpus-driven unit test
+> ([fact_index_corpus_test.go](../../internal/queue/fact_index_corpus_test.go)); that test now pins
+> the fix. Shipped: the sticky uid-keyed removal slot, its consultation ahead of the exact tier for a
+> removal, the count-based horizon, and the `delete_sticky` tier on
+> `attribution_resolutions_total{tier}`. The measurement and the reasoning below are kept as
+> written — they are why the shape is what it is.
+
+A human deletes an object that carries a finalizer. A controller clears the finalizer. The commit
+that removes the file from Git is authored by **the controller**, and the human who asked for the
+deletion appears nowhere.
+
+```text
+onboard: [DELETE] reposelections/attribution-probe (by system:serviceaccount:gitops-api:tenant-operator)
+```
+
+The `[CREATE]` on the same object, seconds earlier, names the human correctly. This document says
+exactly why, with the measurement, and designs the fix.
+
+## What the deletion-as-intent rule promises
+
+The spec's §2.2 states the intent:
+
+> *The first observation of `deletionTimestamp` (or a `DELETED` event) removes the resource from Git
+> and attributes the removal to the actor who **requested** the deletion.*
+
+The first half holds. [`operationForLiveTargetWatchEvent`](../../internal/watch/target_watch.go)
+renders any object carrying a `deletionTimestamp` as a `DELETE`, so the file goes at the transition
+rather than at the eventual `DELETED`. The second half is the one that does not hold, and the reason
+is not in the watch side at all — it is in what the two audit events look like.
+
+## The measurement
+
+The lab drives one ConfigMap with two real identities: the deletion is requested by
+`alice@example.com` (impersonated), the finalizer is cleared by a real ServiceAccount token,
+`system:serviceaccount::finalizer-controller`. A **hold** between the two phases is the knob:
+the API server batches audit deliveries (`--audit-webhook-batch-max-wait`, 1s here), so the hold
+decides whether both events reach the operator in one batch or two.
+
+Three facts come out of the capture, and only the third is surprising:
+
+1. **The removal verb is a `patch`.** There is exactly one audit `delete` — phase one, the human's —
+ and nothing audits the disappearance itself. This was already known
+ ([`finalizer-delete`](../../test/mutationlab/corpus/configmap/finalizer-delete), catalog row 8).
+2. **The two phases have different actors**, which is the whole premise of the report.
+3. **Both audit events carry the same `resourceVersion`.** The human's `delete` returns the object
+ with `deletionTimestamp` set, at RV *n*. The controller's finalizer `patch` returns the object as
+ it stood — also at RV *n*. The capture's relational tokens make this visible directly: both
+ response bodies read `resourceVersion: `, and so does the deletion-pending watch event.
+
+Fact 3 is the defect. It is not a ranking problem, and no tier ordering can fix it.
+
+## Why the join then names the controller
+
+The publish side files a fact under the strongest key it has
+([`file`](../../internal/queue/fact_index.go)). Both of these facts carry a uid and a
+resourceVersion, so both are filed under **`exact(uid, rv)`** and **`latest(uid)`**. Both index
+structures are last-writer-wins ([`putExact`, `putLatest`](../../internal/queue/fact_index_store.go)):
+
+```go
+s.exact[key] = entry // same (uid, rv) → the controller's fact REPLACES the human's
+s.latest[uid] = entry // same uid → likewise
+```
+
+So once the controller's fact is applied, the human's fact is not outranked — **it is gone**. The
+watch event that renders as the removal is the deletion-pending `MODIFIED`, whose resourceVersion is
+that same colliding value, so it asks the exact tier for exactly the key that was overwritten, gets
+the strongest possible match, and never consults a weaker tier or waits for anything.
+
+The corpus-driven test states both sides of the race:
+
+| Facts applied when the removal resolves | Named actor | Tier |
+|---|---|---|
+| the human's `delete` only (a slow finalizer, separate audit batches) | **the human** | `exact` |
+| both, in the order the API server produced them (a fast finalizer, one batch) | **the controller** | `exact` |
+
+Nothing about the join can tell those two apart. Whether a deletion is attributed correctly depends
+on how quickly a controller cleans up, which is a property of the cluster's workloads.
+
+**The more finalizers an object carries, the likelier the wrong answer**, exactly as the report
+says: each finalizer controller adds another write that can land inside the batch window.
+
+### The second trigger: a hung finalizer plus a restart
+
+A finalizer that hangs — days, when nobody notices — is the *safe* case for as long as the operator
+stays up. The file left Git at the transition, attributed to the human, and the eventual finalizer
+clear and terminal `DELETED` both fold to no-ops against the already-absent path. Slower is better.
+
+It stops being safe the moment the operator is not running at the transition. A restart, a rollout,
+a `GitTarget` created later, a `410` rebuild — after any of them the watch collapses to CURRENT
+state, so there is no transition event to observe. The first observation is of an object already
+`Terminating`, the file is still in Git, that observation renders as a `DELETE`, and a real commit
+lands. The human's fact aged out hours ago, so that commit is authored `unresolved`, or picks up
+whichever fact is still inside the TTL — during cleanup, plausibly the finalizer controller's.
+
+A hang does not cause this. It widens the window in which an ordinary restart does, from seconds to
+days, which turns an unlucky coincidence into an expected one.
+
+### What the current code does get right
+
+Three things already move in the right direction, and the fix should not disturb them:
+
+- **A removal never settles for a write fact.** [`lookupRemoval`](../../internal/queue/fact_index.go)
+ holds a write match as a fallback and keeps waiting for evidence about the deletion; only a fact
+ whose own verb is `delete`/`deletecollection` ends the wait. That is already "prefer the actor of
+ the transition", expressed at the tier level — it just never gets the chance here, because the
+ exact tier answers first.
+- **An object's own delete fact wins from more places** than it used to: by uid, and by
+ `(namespace, name)` when the delete audit event carried no uid at all.
+- **The head-of-line stall that inverted event ordering is fixed at its cause**
+ ([attribution-branch-findings.md](attribution-branch-findings.md)), so the reported log's
+ "controller's window commits first" no longer has that mechanism behind it.
+
+## The fix: a sticky removal pointer
+
+**A fact about a DELETION must not be replaceable by a fact about a WRITE.**
+
+That single rule closes the defect, and it is the rule the tier ladder already believes in — the
+removal path is built entirely around "a fact about the deletion outranks a fact about a write". It
+is simply not enforceable today, because the write can overwrite the deletion's *storage* before any
+ranking happens.
+
+Concretely: file a fact whose verb is a removal into a **removal slot** keyed by uid, and let a
+later non-removal fact fill the ordinary tiers without touching that slot. `lookupRemoval` consults
+the removal slot first. How long the slot lives is its own decision, and not the join TTL — see
+[the horizon](#how-long-it-should-live-and-why-that-is-not-the-ttl).
+
+Sketch, in [`file`](../../internal/queue/fact_index.go):
+
+```go
+case isRemovalVerb(fact.Verb) && fact.UID != "":
+ facts.putRemoval(fact.UID, entry) // sticky: only another REMOVAL fact may replace it
+ // …and the ordinary exact/latest filing as today
+```
+
+and in [`lookupRemoval`](../../internal/queue/fact_index.go), before the exact tier is consulted for
+a non-exact-capable query.
+
+### Why "sticky" and not "ranked"
+
+A ranking change alone (consult a removal tier before the exact tier) does not help while both facts
+live under one key: there would be nothing left to rank. The stickiness is the load-bearing half.
+The ordering change is what makes it reachable.
+
+### What it costs
+
+- **One more entry per deleted object**, held until the caps reclaim it rather than until the TTL
+ does. The index is capped per type and in total, so the cost is bounded by construction and shows
+ up on `attribution_fact_index_evictions_total` if it ever binds.
+- **A deliberate asymmetry**: the removal slot is the only structure in the index that a later fact
+ may not overwrite. That deserves the comment it will get — the reason is that "who deleted this"
+ is a question a later write cannot answer, which is not true of any other tier.
+
+### How long it should live, and why that is not the TTL
+
+The removal pointer should outlive the join TTL, and be bounded by **count rather than time**.
+
+Every other structure in the index must expire for correctness. `exact` and `latest` can be
+superseded by a later write. The name tier must expire because a **name** is reused after a
+delete-and-recreate, which is exactly why it ranks last. A uid-keyed removal statement has neither
+failure mode:
+
+> **A uid is unique across space and time.** "uid X was deleted, and this actor asked for it" can
+> never be superseded, because that object can never be written again, deleted again, or recreated
+> under the same uid.
+
+So holding it for a day is not less accurate than holding it for ten minutes, only more expensive,
+and the question stops being "how long is safe" and becomes "how much memory will we spend". That
+argues for a per-type LRU of recent removals, aged out under pressure rather than by a clock: a
+cluster that deletes rarely keeps its removals for a very long time at no cost, and a busy one keeps
+the most recent, which are the ones a replay is most likely to need. It fits the index's existing
+shape — bounded per type and in total, evictions counted — and it needs no new number for an
+operator to tune.
+
+**Strictly uid-keyed.** The same stickiness on the name tier would be a defect rather than a fix: a
+name reused after a recreate would inherit the previous object's deleter, and a longer horizon makes
+that more likely, not less. The TTL is what bounds that risk today and must keep bounding it.
+
+**The longer horizon is a horizon within one process, and the TTL still bounds what a restart can
+recover.** The pointer is not persisted: the index is warmed from the fact streams, and both halves
+of that are one TTL wide — the transport trims to the retention horizon, and the follower replays
+exactly `i.ttl` ([`Run`](../../internal/queue/fact_index.go),
+[`FollowFacts`](../../internal/queue/fact_stream.go)). So a pointer for a delete older than the TTL
+survives a `410` rebuild, a re-list, and any amount of watch churn, but it does not survive the
+operator process going away. The second trigger is therefore *narrowed* rather than closed: it is
+closed for a restart that happens after the delete fact was seen and inside the retention window,
+and untouched when the operator was not running to see the delete at all. Closing that remainder
+needs the pointer to outlive the process, which is a different change — persistence — and is not
+proposed here.
+
+### The payoff that is not correctness
+
+A `Terminating` object seen on replay resolves `absent` today, which means it waits out the **whole**
+grace window for a fact written days ago that can never arrive — on the shard's single goroutine,
+with every later event queued behind it. That is the head-of-line cost
+[attribution-branch-findings.md](attribution-branch-findings.md) measured. A removal pointer that
+outlives the TTL turns that full-grace wait into an immediate hit, so a cluster carrying objects
+stuck `Terminating` stops paying a grace per replayed event — for as long as the process that saw
+the delete is still running, which is the horizon the section above states exactly.
+
+### Why not simply refuse to overwrite the exact entry
+
+The narrower move is to make the exact tier **write-once**: `(uid, rv)` asserts "this actor produced
+this exact version", and the finalizer patch did not produce that version — it returned it. Keeping
+the first fact filed under a key would name the deleter here without touching the read ladder at all,
+which is a real attraction.
+
+It is not enough, for two reasons.
+
+**It makes correctness depend on arrival order.** First-writer-wins is still a race, only decided
+earlier: it gives the right answer exactly when the deleter's fact is filed first. Within one audit
+batch it is, but a fact's arrival order is not guaranteed in general — an HA control plane audits the
+`delete` on one API server and the finalizer `patch` on another, and those two audit batches are
+independent. "A write may never displace a deletion" needs no ordering assumption, because the
+patch's fact can never enter the slot no matter when it lands.
+
+**It does nothing for the replay case.** The exact tier is TTL-bounded and has to stay that way — it
+holds one entry per write, not one per delete — so the [second trigger](#the-second-trigger-a-hung-finalizer-plus-a-restart)
+and the [head-of-line payoff](#the-payoff-that-is-not-correctness) both need a structure whose
+horizon is a count. That structure is the removal slot, and once it exists, the ordering change is
+the cheap half.
+
+The narrower move remains worth making on its own merits — a mutating request that returns a
+resourceVersion it did not produce should not claim that version's exact key, and a no-op patch does
+exactly that today. It is a separate change with a separate argument, and it is not this one.
+
+### The alternative, and why it is second
+
+**File every delete fact under the name tier as well**, so it survives the uid tiers being
+overwritten. It is smaller — one extra `putName` — and needs no new structure or lookup order.
+
+It is second because it is indirect: it fixes this defect by making a *weaker* tier hold the answer,
+so the removal resolves at `tier="name"` rather than at a tier that says what it is. A reader of the
+metric would see a name-tier match and reasonably conclude the audit event carried no uid, which
+here would be false. It also leaves the exact tier still answering with the controller for the
+window before the name tier is consulted.
+
+## How we will know it worked
+
+The metrics that shipped in the attribution surface make this observable without a repro:
+
+- The evidence mix moves. A finalized deletion that resolves today at `tier="exact"` should resolve
+ at `tier="delete_sticky"` instead:
+ `sum by (tier) (rate(gitopsreverser_attribution_resolutions_total[5m]))`.
+- `commits_total{author_kind}` shifts from `serviceaccount` toward `user` for the affected types —
+ the bottom line the report is actually about.
+- The removal wait collapses for `Terminating` objects seen on replay, which today spend a full
+ grace each:
+ `histogram_quantile(0.95, sum by (le) (rate(gitopsreverser_attribution_resolution_wait_seconds_bucket{event_kind="removal"}[5m])))`.
+- The corpus test inverts: the "both facts applied" case names the human, and the assertion that
+ pinned the defect is now the assertion that pins the fix.
+
+## Reproducing it
+
+```bash
+task lab-corpus-update # captures configmap/deletion-intent-actor (three holds, two actors)
+go test ./internal/queue/ -run TestFactIndex_FinalizerRemovalNamesTheDeleter -v
+```
+
+The hold is tunable — `LAB_FINALIZER_HOLD=0s` makes the controller win every time, a hold longer
+than the audit batch window makes the human win — which is the race itself, made deterministic.
+
+## References
+
+- [attribution-publish-and-join.md](attribution-publish-and-join.md) — what each half does, and the
+ tier ladder this fix extends.
+- [attribution-removal-wait-options.md](attribution-removal-wait-options.md) — why a removal waits
+ for evidence about the deletion rather than accepting the last write.
+- [mutation-capture-lab-design.md](../spec/mutation-capture-lab-design.md) — the lab, and why a
+ captured shape beats an asserted one.
+- [interpreting-metrics.md](../interpreting-metrics.md) — the tier and actor-kind labels the fix is
+ measured on.
diff --git a/docs/design/attribution-publish-and-join.md b/docs/design/attribution-publish-and-join.md
index 3dffc348..493dbf57 100644
--- a/docs/design/attribution-publish-and-join.md
+++ b/docs/design/attribution-publish-and-join.md
@@ -73,9 +73,10 @@ first, and a uid-keyed fact always answers there. A second copy of that same fac
would never be the one read. Storing it anyway costs the entry on every replica following the type,
for the whole TTL, and again on every restart replay, and buys nothing.
-The one branch that files twice is the uid case, and only when it also has a resourceVersion: `exact`
-serves creates and updates, `latest` serves removals, and the two answer different questions about
-the same object: one fact serving two tiers.
+The one branch that files more than once is the uid case: `exact` (when the fact also has a
+resourceVersion) serves creates and updates, `latest` serves removals, and the two answer different
+questions about the same object. A fact whose own verb is a **removal** takes a third structure, the
+sticky removal pointer, which answers a question no later fact can: who asked for the deletion.
So the rule is: keep every field, file under exactly the keys a query could reach you by.
@@ -89,13 +90,15 @@ flowchart TD
A[Watch event] --> B[scope = audit route + group/resource]
B --> C{scope known?}
C -->|no| Z[absent: committer-authored]
- C -->|yes| D{uid and rv,
and exact-capable?}
+ C -->|yes| AA{a removal, and a sticky
removal pointer for its uid?}
+ AA -->|yes| AB[delete_sticky]
+ AA -->|no| D{a fact under
this uid and rv?}
D -->|match| E[exact]
D -->|no match| F{is this a removal?}
F -->|yes| G{uid in a collection's
uid set?}
- G -->|yes| H[collection_uid]
+ G -->|yes| H[deletecollection_body_uid]
G -->|no| I{latest uid
is a DELETE fact?}
I -->|yes| J[latest: the object's own delete]
I -->|no, it is a write| K[hold it as a fallback]
@@ -104,7 +107,7 @@ flowchart TD
L -->|no| N{fallback held?}
N -->|yes| O[latest: last writer]
N -->|no| P{collection covers
this scope + selector?}
- P -->|yes| Q[collection_scope]
+ P -->|yes| Q[deletecollection_scope]
P -->|no| R
F -->|no| R{rv-only hatch?}
@@ -114,6 +117,7 @@ flowchart TD
T -->|no match| Z
style Z fill:#7f1d1d,color:#fff
+ style AB fill:#14532d,color:#fff
style E fill:#14532d,color:#fff
style H fill:#14532d,color:#fff
style J fill:#14532d,color:#fff
@@ -128,12 +132,13 @@ flowchart TD
| Tier | Key | `tier` label | What it asserts |
|---|---|---|---|
+| delete, sticky | uid (a delete fact, sticky) | `delete_sticky` | who asked for this object's deletion |
| exact | uid + rv | `exact` | this actor produced this exact version |
-| collection uid | uid in a collection's set | `collection_uid` | the API server said this request deleted this object |
+| collection uid | uid in a collection's set | `deletecollection_body_uid` | the API server said this request deleted this object |
| latest, delete | uid | `latest` | this object's own delete fact |
| name, delete | namespace + name | `name` | this object's own delete fact, when it has no uid |
| latest, write | uid | `latest` | who last wrote it; a fallback for a removal |
-| collection scope | namespace + selector + window | `collection_scope` | a collection request covering it was made |
+| collection scope | namespace + selector + window | `deletecollection_scope` | a collection request covering it was made |
| rv-only | rv | `resource_version` | a fact with an rv but no uid |
| name | namespace + name | `name` | the only key an aggregated write has |
| absent | none | `absent` | committer-authored |
@@ -141,7 +146,17 @@ flowchart TD
Who the evidence named is the separate `actor_kind` label (`user` / `serviceaccount` / `none`), so
every row above can be asked about either kind of actor.
-### Two rules that are easy to miss
+### Three rules that are easy to miss
+
+**A fact about a DELETION may not be replaced by a fact about a WRITE.** Every ordinary structure
+here is last-writer-wins, and a finalizer patch's fact carries the resourceVersion the DELETION
+stamped, so it lands under the deleter's exact key and the deleter's uid key alike. A removal fact
+therefore also takes a **sticky removal pointer**, keyed by uid, that only another removal fact may
+fill, and a removal consults that pointer before the exact tier. It is the one structure the TTL does
+not bound: a uid is unique across space and time, so the statement can never be superseded, and its
+horizon is the index's caps instead. It is strictly uid-keyed, because the same stickiness on the
+name tier would be a defect: a name is reused after a delete and recreate. See
+[attribution-deletion-intent-actor.md](attribution-deletion-intent-actor.md).
**A removal never returns on a write fact without looking further.** The per-object tiers are
last-writer-wins, so for a removal they hold whoever last EDITED the object, which is not who deleted
@@ -153,6 +168,11 @@ wait immediately.
resourceVersion its own write produced. If the exact tier misses, the `latest` pointer may name an
older, different author, so the lookup skips straight to the rv hatch and the name tier.
+The gate is one-directional, and only one of the two tiers above is gated. The exact tier is tried
+for *any* query carrying a uid and a resourceVersion, a removal included. A removal that misses
+the sticky pointer can therefore still resolve at `exact`, which is exactly what it did before the
+pointer existed. What an exact-capable event may not do is the reverse: reach the tiers below.
+
## The wait, and what changed about it
The two halves are racing, and the watch side reliably wins. The API server batches audit deliveries
@@ -314,7 +334,7 @@ it is the argument for the shape, and the migration is in
[`UPGRADING.md`](../UPGRADING.md#0410--the-attribution-metrics-are-relabelled-and-partly-renamed-breaking-for-queries).
The inconsistency was real rather than cosmetic. `result` should have named the TIER: `weak`,
-`name`, `collection_uid`, `collection_scope`, `absent` all did. `exact` was the only one that also
+`name`, `deletecollection_body_uid`, `deletecollection_scope`, `absent` all did. `exact` was the only one that also
encoded WHO the actor was, which crammed two orthogonal dimensions into one label.
Two consequences followed directly:
@@ -322,7 +342,7 @@ Two consequences followed directly:
- counting exact resolutions meant summing two series, and any new actor kind would have multiplied
them again;
- the actor kind could only be asked of the exact tier. There was no way to ask how many `name`-tier
- or `collection_uid` resolutions named a service account, because that dimension did not exist
+ or `deletecollection_body_uid` resolutions named a service account, because that dimension did not exist
there.
The decisive argument was that the codebase already modeled it correctly one metric over:
diff --git a/docs/design/attribution-removal-wait-options.md b/docs/design/attribution-removal-wait-options.md
index 5703e059..da23bc2d 100644
--- a/docs/design/attribution-removal-wait-options.md
+++ b/docs/design/attribution-removal-wait-options.md
@@ -49,8 +49,8 @@ delete fact, or a collection fact covering it.
| # | Situation | Today | Cost |
|---|---|---|---|
| 1 | Removal; the object's own delete fact is already in the index | returns at once, `weak` | none |
-| 2 | Removal; a collection fact naming its uid is present | returns at once, `collection_uid` | none — measured at ~70ms |
-| 3 | Removal; a collection fact covers its scope | returns at once, `collection_scope` | none |
+| 2 | Removal; a collection fact naming its uid is present | returns at once, `deletecollection_body_uid` | none — measured at ~70ms |
+| 3 | Removal; a collection fact covers its scope | returns at once, `deletecollection_scope` | none |
| 4 | Removal; only a stale WRITE fact is present, and the delete fact arrives during the grace | waits, then returns the deleter | the audit delivery lag, ~1s. **This is the case the fix exists for** |
| 5 | Removal; only a stale WRITE fact is present, and no delete fact ever arrives | waits the FULL grace, then returns the last writer | the whole grace, for an answer available at t=0. **This is the entire cost** |
| 6 | Removal; nothing in the index at all | waits the full grace, then `absent` | unchanged by the fix — this is what the grace has always done |
@@ -104,7 +104,7 @@ supplies one. This is measured, not inferred: corpus `flunder/aggregated-api-wri
| create | nothing: no name, uid or rv | **No fact is published at all.** `AuthorFactFromEvent` rejects it at the "no resolvable name" gate | unchanged, and now COUNTED as `no_attribution_fact` on `audit_events_total` |
| update / patch | the NAME, from the URL path | A fact was published and then **dropped by the index**: with no uid and no rv it could never be joined | **resolves**, on the name tier |
| single delete | the NAME, from the URL path | Same: published, then dropped as unjoinable | **resolves**, on the name tier |
-| deletecollection | namespace and selector | **Works.** A collection fact joins by SCOPE, which needs no uid | unchanged, and now two-tiered (`collection_uid` / `collection_scope`) |
+| deletecollection | namespace and selector | **Works.** A collection fact joins by SCOPE, which needs no uid | unchanged, and now two-tiered (`deletecollection_body_uid` / `deletecollection_scope`) |
So the observation that creates and updates "measure" while deletes do not was half right, and the
truth was less flattering: for an aggregated type, only the COLLECTION delete was attributable at
@@ -179,7 +179,7 @@ REQUEST happened, which means an audit event should exist.
Removals return on the first match again. Collection deletes whose response body carried uids are
still credited correctly, because that tier now outranks the write fact and both are present at once.
-- **For**: no latency cost at all; keeps the measured `collection_uid` win.
+- **For**: no latency cost at all; keeps the measured `deletecollection_body_uid` win.
- **Against**: puts back the mis-attribution for every removal whose delete fact is merely late —
which, given audit batching, is the common case rather than the corner. It trades a correctness
property for latency in the one place the product's core claim lives.
diff --git a/docs/design/metrics-observability-plan.md b/docs/design/metrics-observability-plan.md
index 2962c7f8..abe4a0ee 100644
--- a/docs/design/metrics-observability-plan.md
+++ b/docs/design/metrics-observability-plan.md
@@ -52,8 +52,8 @@ uniformly dark. What is left is a sharper, smaller list:
told to build against the current label names. The cost of renaming a label today is one
[`UPGRADING.md`](../UPGRADING.md) entry; the cost after a release with a published dashboard is a
migration for every consumer. This release has already broken the `result` label anyway — the
-`deletecollection` rework removed `exact_deletecollection_item` and added `collection_uid`,
-`collection_scope`, and `name` — so finishing the job costs the same single migration, and deferring
+`deletecollection` rework removed `exact_deletecollection_item` and added `deletecollection_body_uid`,
+`deletecollection_scope`, and `name` — so finishing the job costs the same single migration, and deferring
it costs a second one later on a label that will have been wrong twice.
So this plan takes the label breaks now, deliberately, and writes them down. It does not take them
@@ -166,11 +166,11 @@ records the metric at the smallest honest boundary.
|---|---|---|---|
| `audit_events_total` | counter | `outcome`, `category`, `group`, `version`, `resource`, `verb` | ✅ shipped — `no_attribution_fact` added in the `dropped` category |
| `audit_eventlists_total` / `_eventlist_events_total` / `_eventlist_duration_seconds` | counter/hist | `outcome` | live, unchanged |
-| `attribution_resolutions_total` | counter | **`tier`**, **`actor_kind`**, `group`, `version`, `resource` | ✅ shipped — `result` is gone |
+| `attribution_resolutions_total` | counter | **`tier`**, **`actor_kind`**, `group`, `version`, `resource` | ✅ shipped — `result` is gone; `tier` gained `delete_sticky`, and the two collection values are named for the `deletecollection` verb |
| `attribution_resolution_wait_seconds` | histogram | **`tier`**, **`event_kind`**, `group`, `version`, `resource` | ✅ shipped — relabelled and split by write/removal |
| `attribution_facts_total` | counter | `op` (`written`/`matched`) | ✅ shipped — renamed from `attribution_fact_events_total` |
| `attribution_fact_index_entries` | gauge | — | ✅ shipped — renamed from `attribution_fact_index_size` |
-| `attribution_fact_index_evictions_total` | counter | `reason` (`per_type`/`total`) | live, unchanged |
+| `attribution_fact_index_evictions_total` | counter | `reason` (`per_type`/`total`) | live, unchanged — and now the removal pointer's only horizon |
| `attribution_fact_stream_gaps_total` | counter | `stream` | live, unchanged |
| `attribution_collection_without_uidset_total` | counter | `reason` (`uid_cap`/`no_uids`) | ✅ shipped — renamed from `attribution_collection_degraded_total` |
| `attribution_fact_stream_decode_errors_total` | counter | `transport` | ✅ shipped — the one loss path that had no symptom at all |
@@ -247,7 +247,7 @@ two metrics disagreed about the shape of one distinction.
| Label | Values |
|---|---|
-| `tier` | `exact`, `latest`, `resource_version`, `name`, `collection_uid`, `collection_scope`, `absent` |
+| `tier` | `exact`, `latest`, `resource_version`, `name`, `delete_sticky`, `deletecollection_body_uid`, `deletecollection_scope`, `absent` |
| `actor_kind` | `user`, `serviceaccount`, `none` |
`weak` split at the same time. It covered both a `latest` (uid) match and the rv-only
diff --git a/docs/interpreting-metrics.md b/docs/interpreting-metrics.md
index 019884a4..ba556407 100644
--- a/docs/interpreting-metrics.md
+++ b/docs/interpreting-metrics.md
@@ -288,13 +288,25 @@ The two labels answer two different questions. **`tier`** names which evidence p
and it is ordered, strongest first. **`actor_kind`** names who that evidence named, in the same
vocabulary `commits_total{author_kind}` uses.
+Three values are named for the **verb that produced the fact** and how it matched:
+`delete_sticky`, `deletecollection_body_uid`, `deletecollection_scope`. Those are the tiers only a
+removal can reach, and naming the source is deliberate: it says where the evidence came from rather
+than what it proves. Two of the three are statements about *this object*; `deletecollection_scope` is
+a statement about a request whose scope covered it, and can name the wrong actor. So
+`tier=~"delete.*"` reads as "resolved on deletion-specific evidence", never as a guarantee that the
+actor named asked for this object's removal.
+
+`latest` and `name` can hold a delete fact too, and are deliberately *not* named for one: either can
+equally hold a write, and a value that could mean either must not claim a verb.
+
| `tier` | Meaning |
| --- | --- |
+| `delete_sticky` | The sticky removal pointer: a fact whose own verb is a delete, filed by UID into a slot no later *write* fact may overwrite. Only a removal consults it, and it is asked before `exact`, because a removal's resourceVersion is the one the deletion stamped — the version a finalizer patch's own fact carries too. It is the only tier the fact TTL does not bound: a UID is unique across space and time, so the statement can never be superseded. |
| `exact` | Exact UID+resourceVersion match: this actor produced this exact version. |
-| `collection_uid` | A removal whose UID was in the set the API server said a `deletecollection` deleted. No over-attribution risk: either the object was in that set or it was not. It outranks `latest`, because `latest` names whoever last *wrote* an object while a removal asks who *deleted* it. |
+| `deletecollection_body_uid` | A removal whose UID was in the set the API server said a `deletecollection` deleted. No over-attribution risk: either the object was in that set or it was not. It outranks `latest`, because `latest` names whoever last *wrote* an object while a removal asks who *deleted* it. |
| `latest` | The UID-latest tier: the object's own last fact, keyed by UID alone. A removal consults it, and a match here describing a *write* is held as a fallback while the wait continues for evidence about the deletion. |
| `name` | A match on `(namespace, name)` for a fact carrying neither a UID nor a resourceVersion — the usual shape of an aggregated API's audit event, and of a delete the API server answered with a `Status`. |
-| `collection_scope` | A removal matched to a `deletecollection` by scope alone — same type and namespace, the request's selector accepting the object's labels, within the collection window. The weakest evidence the join has, and the only one that can name the wrong actor, which is why it is reached only when every more specific tier missed. |
+| `deletecollection_scope` | A removal matched to a `deletecollection` by scope alone — same type and namespace, the request's selector accepting the object's labels, within the collection window. The weakest evidence the join has, and the only one that can name the wrong actor, which is why it is reached only when every more specific tier missed. |
| `resource_version` | The RV-only escape hatch: a fact that carried a resourceVersion and no UID, matched on that version alone. |
| `absent` | No usable fact matched before the grace window elapsed. The resulting live commit is authored as `unknown (attribution unresolved)`. |
@@ -310,7 +322,7 @@ coincidence: an audit event whose user cannot be resolved never becomes a fact a
(counted as a stream decode error below). Every fact that reaches the index therefore names someone,
which is why coverage can be read off the tier alone.
-**Evidence quality, independently of coverage.** A shift from `exact` toward `collection_scope` or
+**Evidence quality, independently of coverage.** A shift from `exact` toward `deletecollection_scope` or
`name` is a quality regression even while coverage holds flat, so it is worth its own panel:
```promql
@@ -319,8 +331,8 @@ sum by (tier) (rate(gitopsreverser_attribution_resolutions_total[5m]))
> **`result` is gone**, and so are `exact_user`, `exact_serviceaccount`, and `weak`. See
> [`UPGRADING.md`](UPGRADING.md) for the old-to-new mapping. `exact_deletecollection_item` went
-> earlier, with the expander and the fact keyspace; `collection_uid` is its closest equivalent and
-> `collection_scope` is new capability rather than a rename. See
+> earlier, with the expander and the fact keyspace; `deletecollection_body_uid` is its closest equivalent and
+> `deletecollection_scope` is new capability rather than a rename. See
> [`attribution-fact-stream.md`](finished/attribution-fact-stream.md).
**Is the grace window paying for itself?** `event_kind` is `write` or `removal`, and the split is
@@ -359,6 +371,12 @@ sum by (reason) (rate(gitopsreverser_attribution_fact_index_evictions_total[5m])
`reason` is `per_type` (one type is hotter than its share) or `total` (the whole index is under
pressure, and eviction falls on the type holding the most).
+This counter is also the removal pointer's only horizon. Every other entry in the index expires on
+the fact TTL; a removal pointer is bounded by these caps instead, so a cluster that deletes rarely
+keeps its removals for a very long time at no cost, and a busy one keeps the most recent — which are
+the ones a replay is likeliest to need. A sustained eviction rate on a type that deletes heavily is
+the signal that its pointers are being reclaimed before a replay can use them.
+
**Is a follower losing facts?** A trim gap means the fact log was trimmed past this process's
position: the facts in the gap are gone for good, and the commits that needed them are authored
unresolved. It is the one loss the transport can see, which is why the transport is a log with
@@ -424,7 +442,7 @@ gitopsreverser_attribution_transport_info
**How often does a collection delete fall back to scope matching?** A `deletecollection` fact
carries the UIDs the API server named, when it sent them, and joins by membership. When it cannot,
-the join falls back to `collection_scope`, which is correct but weaker — so the fallback is counted
+the join falls back to `deletecollection_scope`, which is correct but weaker — so the fallback is counted
rather than inferred:
```promql
diff --git a/docs/spec/deletecollection-attribution-expander.md b/docs/spec/deletecollection-attribution-expander.md
index ce52d925..1b3c974e 100644
--- a/docs/spec/deletecollection-attribution-expander.md
+++ b/docs/spec/deletecollection-attribution-expander.md
@@ -221,8 +221,8 @@ delete — is the one that gains most. Option B (`Co-authored-by`) is no longer
|---|---|---|
| Any delete (single or collection member) | **Remove file at intent time; never commit `deletionTimestamp`** (§2) | Git is intent; reversible invariant; manifests stay re-appliable. |
| Finalizer object | **Removed immediately, attributed to the delete-requester**; finalizer cleanup is runtime no-op in Git | Reframe dissolves the old delay/conflict; controllers still finalize in-cluster. |
-| Collection delete, body present | **One collection fact**, joined by **uid membership** (`collection_uid`) | API server states "these exact objects, by this user." |
-| Collection delete, hollow body | **The same collection fact**, joined by **scope** — type, namespace, selector, window (`collection_scope`) | Bounded by precedence and a short window (§6); resolves what used to degrade. |
+| Collection delete, body present | **One collection fact**, joined by **uid membership** (`deletecollection_body_uid`) | API server states "these exact objects, by this user." |
+| Collection delete, hollow body | **The same collection fact**, joined by **scope** — type, namespace, selector, window (`deletecollection_scope`) | Bounded by precedence and a short window (§6); resolves what used to degrade. |
| Stuck `Terminating` | **Operational status/metric** (§2.5), file already absent | Don't pollute intent with runtime state. |
## 8. Observability & diagnostics
@@ -231,9 +231,9 @@ delete — is the one that gains most. Option B (`Co-authored-by`) is no longer
`op="deletecollection_expanded"` write counter. Two labels replace the one, because the match is now
two-tiered and the tiers carry different confidence:
-- **`collection_uid`** — the removal's uid was in the set the API server said it deleted. No
+- **`deletecollection_body_uid`** — the removal's uid was in the set the API server said it deleted. No
over-attribution risk at all.
-- **`collection_scope`** — matched by namespace, selector, and window alone. Weaker evidence, and
+- **`deletecollection_scope`** — matched by namespace, selector, and window alone. Weaker evidence, and
the reason the window is short.
Both flow onto `AttributionResolutionsTotal{result=…}`, so a dashboard can now separate *precise*
@@ -267,9 +267,9 @@ and the event is a no-op commit anyway).
`attribution_collection_degraded_total{reason="uid_cap"}`; the fact still publishes.
3. A hollow / `Status` / unparseable / absent body still publishes a fact — with no uid set. This is the
case the expander produced nothing at all for.
-4. Join shape, uid tier: a removal whose uid is in the set resolves to the actor as `collection_uid`,
+4. Join shape, uid tier: a removal whose uid is in the set resolves to the actor as `deletecollection_body_uid`,
even though its RV never matches (proves §3).
-5. Join shape, scope tier: a removal with no uid set to consult resolves as `collection_scope` when the
+5. Join shape, scope tier: a removal with no uid set to consult resolves as `deletecollection_scope` when the
namespace, selector and window cover it — and does NOT resolve when the selector rejects its labels,
when it is in another namespace, or when the window has passed.
6. Precedence: an object with its own fact never reaches either collection tier.
@@ -310,7 +310,7 @@ these survive), never a global drop count, so they run against a reused cluster.
finalizer/`DELETED` events fold to no-ops. Unit §9.1.1–9.1.3.
- **§5 collection fact:** one fact per collection request, carrying scope, selector and (when the body
allowed it) the uid set; finalizer items attributed like any other, since §2 removes them at intent;
- defensive parsing; `collection_uid` and `collection_scope` result labels wired. Unit §9.1.4–9.1.9.
+ defensive parsing; `deletecollection_body_uid` and `deletecollection_scope` result labels wired. Unit §9.1.4–9.1.9.
- **E2E §9.2.1–9.2.4** implemented, convergence-asserted; the finalizer showcase (§9.2.2) proves removal-at-intent
with a *different* finalizer-clearing identity.
- **Hard case:** resolved by scope matching (§6), bounded by namespace, selector, precedence and a short
diff --git a/internal/queue/author_fact.go b/internal/queue/author_fact.go
index 5c045624..83743373 100644
--- a/internal/queue/author_fact.go
+++ b/internal/queue/author_fact.go
@@ -58,7 +58,29 @@ const serviceAccountUserPrefix = "system:serviceaccount:"
// resolutions a sum of two series and made the actor kind unaskable of every other tier.
type AttributionResult string
+// Three values are named for the VERB that produced the fact and how it matched — delete_sticky,
+// deletecollection_body_uid, deletecollection_scope — because those three are the tiers reachable
+// only by a removal, and naming the source is the honest thing to do: it says where the evidence came
+// from rather than asserting what it proves. Two of them are statements about this exact object; the
+// third, deletecollection_scope, is a statement about a request whose scope covered it, and can name
+// the wrong actor. A name that claimed otherwise would be read as a guarantee it cannot give.
+//
+// `latest` and `name` can hold a delete fact too, so they are NOT named for one: either can equally
+// hold a write, and a value that could mean either must not claim a verb.
const (
+ // AttributionDeleteSticky is the sticky removal pointer: a fact whose own verb is a delete, filed by
+ // uid into a slot no later WRITE fact may overwrite. It is the strongest evidence a removal can
+ // have about itself, so it is consulted before the exact tier — and only by a removal, because an
+ // exact-capable event asks who produced a version rather than who deleted an object.
+ //
+ // "sticky" is the half of the name that is not the verb, and it is there because the stickiness is
+ // the whole reason this tier can answer at all: every other structure would have been overwritten
+ // by the finalizer patch that followed the delete.
+ //
+ // It is also the only tier the TTL does not bound. A uid is unique across space and time, so the
+ // statement can never be superseded; its horizon is the index's caps instead. See
+ // docs/design/attribution-deletion-intent-actor.md.
+ AttributionDeleteSticky AttributionResult = "delete_sticky"
// AttributionExact is an exact UID+resourceVersion match: this actor produced this exact version.
AttributionExact AttributionResult = "exact"
// AttributionLatest is the uid-latest tier — the object's own last write or its own delete fact,
@@ -69,10 +91,16 @@ const (
// and no uid, matched on that version alone. It and AttributionLatest were one value ("weak")
// and are different evidence, which is why they are now two.
AttributionResourceVersion AttributionResult = "resource_version"
- // AttributionCollectionUID is a removal matched to a deletecollection fact whose uid set
+ // AttributionDeleteCollectionBodyUID is a removal matched to a deletecollection fact whose uid set
// contains this object. There is no over-attribution risk in it: either the API server said it
// deleted this object, or it did not.
- AttributionCollectionUID AttributionResult = "collection_uid"
+ //
+ // "body" is in the name because the uid set comes from the RESPONSE BODY, which is the part the
+ // API server may not send — a proxied collection delete sends none, and a set past
+ // DefaultCollectionUIDCap is dropped. Both land on the scope tier instead, counted on
+ // attribution_collection_without_uidset_total, so the name says which half of the pair answered
+ // and why the other one exists.
+ AttributionDeleteCollectionBodyUID AttributionResult = "deletecollection_body_uid"
// AttributionName is a match on (namespace, name) for a fact that carries neither a uid nor a
// resourceVersion. It is the tier of last resort for a type whose audit event cannot express
// object identity: the kube-apiserver proxies an aggregated-API request and never decodes the
@@ -83,12 +111,12 @@ const (
// recreate where a uid is not, so it can name the author of a previous object that held this
// name. The TTL is what bounds that: the wrong answer requires the recreate to happen inside it.
AttributionName AttributionResult = "name"
- // AttributionCollectionScope is a removal matched to a deletecollection fact by scope alone —
+ // AttributionDeleteCollectionScope is a removal matched to a deletecollection fact by scope alone —
// same type and namespace, selector accepting the object's labels, within the collection window.
// It is the weakest evidence the join has, which is why it is reached only when every more
// specific tier missed. It is also the tier that resolves what the deleted expander gave up on:
// a collection delete the API server sent no response body for.
- AttributionCollectionScope AttributionResult = "collection_scope"
+ AttributionDeleteCollectionScope AttributionResult = "deletecollection_scope"
// AttributionAbsent means no usable author fact matched before the grace elapsed.
AttributionAbsent AttributionResult = "absent"
)
diff --git a/internal/queue/fact_index.go b/internal/queue/fact_index.go
index 3cdc6493..387f613b 100644
--- a/internal/queue/fact_index.go
+++ b/internal/queue/fact_index.go
@@ -274,6 +274,8 @@ func (i *FactIndex) settle(ctx context.Context, fallback AuthorResolution) Autho
// awaitsBetterEvidence reports whether a match should be held as a fallback rather than returned.
//
// It is true for exactly one shape: a REMOVAL matched to a fact that is not about a removal. The
+// sticky removal pointer is about the deletion by construction — only a removal fact is ever filed
+// there — so a match on it ends the wait at once. The
// per-object tiers are last-writer-wins, so for a collection member — whose delete files one fact
// about the collection rather than one per object — they hold whoever edited it last. Both
// collection tiers are about the deletion itself and end the wait, as does a per-object fact whose
@@ -284,7 +286,7 @@ func (q FactQuery) awaitsBetterEvidence(resolution AuthorResolution) bool {
return false
}
switch resolution.Result {
- case AttributionCollectionUID, AttributionCollectionScope:
+ case AttributionDeleteSticky, AttributionDeleteCollectionBodyUID, AttributionDeleteCollectionScope:
return false
case AttributionExact, AttributionLatest, AttributionResourceVersion,
AttributionName, AttributionAbsent:
@@ -299,12 +301,13 @@ func isRemovalVerb(verb string) bool {
// Lookup reads the index once, trying the tiers strongest-first:
//
-// 1. the exact (uid, rv) fact, the only exact-capable join;
-// 2. a collection fact whose uid set contains this object;
-// 3. the last-writer-wins fact for that uid, for a removal whose rv never matches;
-// 4. a collection fact whose scope, selector, and window cover it;
-// 5. the rv-only escape hatch;
-// 6. the (namespace, name) floor.
+// 1. the sticky removal pointer for that uid, for a removal only;
+// 2. the exact (uid, rv) fact, the only exact-capable join;
+// 3. a collection fact whose uid set contains this object;
+// 4. the last-writer-wins fact for that uid, for a removal whose rv never matches;
+// 5. a collection fact whose scope, selector, and window cover it;
+// 6. the rv-only escape hatch;
+// 7. the (namespace, name) floor.
//
// The name tier is last because it is the weakest per-object evidence here: a name is reused after a
// delete and recreate, so it can name the author of a previous object that held it, where a uid
@@ -337,6 +340,18 @@ func (i *FactIndex) Lookup(query FactQuery) AuthorResolution {
return AuthorResolution{Result: AttributionAbsent}
}
+ // The removal pointer is consulted BEFORE the exact tier, and only for an event that is not
+ // exact-capable. That ordering is half of the fix and useless without the other half: the slot is
+ // what preserves the deleter's fact, and being asked first is what makes it reachable. A removal's
+ // own resourceVersion is the one the DELETION stamped, so the exact tier can hold a fact about the
+ // finalizer patch that carries the very same version — a stronger-looking answer to a question it
+ // is not about. An exact-capable event never reaches here, so a create or update still resolves at
+ // the exact tier exactly as before.
+ if !query.ExactCapable {
+ if resolution := stickyRemoval(facts, query); resolution.Result != AttributionAbsent {
+ return resolution
+ }
+ }
if query.UID != "" && query.ResourceVersion != "" {
if fact, found := facts.lookupExact(query.UID, query.ResourceVersion, cutoff); found {
return AuthorResolution{Fact: fact, Result: AttributionExact}
@@ -360,6 +375,19 @@ func (i *FactIndex) Lookup(query FactQuery) AuthorResolution {
return AuthorResolution{Result: AttributionAbsent}
}
+// stickyRemoval reads the head of the removal ladder: the uid-keyed slot holding the fact that this
+// object was DELETED, which no later fact about a write may overwrite. It is the one tier consulted
+// before the exact tier, and only for an event that is not exact-capable.
+func stickyRemoval(facts *scopeFacts, query FactQuery) AuthorResolution {
+ if query.UID == "" {
+ return AuthorResolution{Result: AttributionAbsent}
+ }
+ if fact, found := facts.lookupRemovalPointer(query.UID); found {
+ return AuthorResolution{Fact: fact, Result: AttributionDeleteSticky}
+ }
+ return AuthorResolution{Result: AttributionAbsent}
+}
+
// lookupRemoval reads the tiers only a removal may consult, in the order they rank: the uid set of
// a collection that named this object, the object's own delete fact (by uid, then by name), its
// last-writer fact, and finally a collection whose scope covers it. See Lookup for why uid
@@ -377,7 +405,7 @@ func (i *FactIndex) lookupRemoval(
haveWriteFallback := false
if query.UID != "" {
if fact, found := facts.matchCollectionUID(query, cutoff); found {
- return AuthorResolution{Fact: fact, Result: AttributionCollectionUID}
+ return AuthorResolution{Fact: fact, Result: AttributionDeleteCollectionBodyUID}
}
if fact, found := facts.lookupLatest(query.UID, cutoff); found {
resolution := AuthorResolution{Fact: fact, Result: AttributionLatest}
@@ -410,7 +438,7 @@ func (i *FactIndex) lookupRemoval(
return writeFallback
}
if fact, found := facts.matchCollectionScope(query, now, cutoff, i.collectionWindow); found {
- return AuthorResolution{Fact: fact, Result: AttributionCollectionScope}
+ return AuthorResolution{Fact: fact, Result: AttributionDeleteCollectionScope}
}
return AuthorResolution{Result: AttributionAbsent}
}
@@ -523,6 +551,15 @@ func (i *FactIndex) file(facts *scopeFacts, scope factScope, fact AuthorFact, no
)
}
facts.putLatest(fact.UID, &indexedFact{fact: fact, at: now, seq: i.nextSeq()})
+ if isRemovalVerb(fact.Verb) {
+ // A fact about a DELETION also takes the sticky removal slot, which a later fact about a
+ // WRITE may not overwrite. The ordinary filing above still happens: the slot answers the
+ // removal question, and the tiers keep answering the ones they always did.
+ //
+ // It needs no waiter key of its own, because a fact reaching here always fills the latest
+ // tier too, and a removal query already waits on that key.
+ facts.putRemoval(fact.UID, &indexedFact{fact: fact, at: now, seq: i.nextSeq()})
+ }
return append(keys, factWaiterKey{scope: scope, kind: factKindLatest, value: fact.UID})
case fact.ResourceVersion != "":
facts.putRV(fact.ResourceVersion, &indexedFact{fact: fact, at: now, seq: i.nextSeq()})
diff --git a/internal/queue/fact_index_corpus_test.go b/internal/queue/fact_index_corpus_test.go
new file mode 100644
index 00000000..6bb2d5b0
--- /dev/null
+++ b/internal/queue/fact_index_corpus_test.go
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package queue
+
+import (
+ "encoding/json"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/require"
+ "k8s.io/apimachinery/pkg/runtime/schema"
+ auditv1 "k8s.io/apiserver/pkg/apis/audit/v1"
+ "sigs.k8s.io/yaml"
+)
+
+// These tests drive the real publish-and-join path with audit events CAPTURED FROM KUBERNETES
+// rather than hand-written ones, so a claim about "what the API server emits" cannot drift from
+// what it actually emits. The recordings live in test/mutationlab/corpus and are produced by
+// `task lab-corpus-update`; see test/mutationlab/README.md.
+//
+// The corpus is normalized — uids, resourceVersions and timestamps are replaced with relational
+// tokens like and . That costs nothing here and buys the thing this test needs: the
+// tokens are RELATIONAL, so two events that carried the same resourceVersion still carry the same
+// token, and two that differed still differ. The join treats both as opaque strings, exactly as it
+// treats real ones.
+
+// corpusDeletionIntentDir holds the two-actor finalizer removal: a human deletes, a controller
+// clears the finalizer.
+const corpusDeletionIntentDir = "../../test/mutationlab/corpus/configmap/deletion-intent-actor"
+
+// loadCorpusAuditEvent reads one normalized audit recording and returns it as the audit event the
+// receiver would decode. The timestamp tokens are replaced with a real time, because they are the
+// only normalized fields that must parse as something other than a string.
+func loadCorpusAuditEvent(t *testing.T, dir, file string, at time.Time) auditv1.Event {
+ t.Helper()
+ raw, err := os.ReadFile(filepath.Join(dir, file))
+ if err != nil {
+ t.Skipf("corpus recording %s/%s is not present: %v", dir, file, err)
+ }
+ // Microsecond precision, not RFC3339Nano: the audit event's stageTimestamp is a metav1.MicroTime,
+ // whose parser accepts exactly six fractional digits, while metav1.Time (creationTimestamp and
+ // friends in the embedded object) accepts the same string as ordinary RFC3339.
+ stamped := strings.ReplaceAll(string(raw), "", at.UTC().Format("2006-01-02T15:04:05.000000Z07:00"))
+
+ asJSON, err := yaml.YAMLToJSON([]byte(stamped))
+ require.NoError(t, err, "convert %s to JSON", file)
+
+ var event auditv1.Event
+ require.NoError(t, json.Unmarshal(asJSON, &event), "decode %s", file)
+ return event
+}
+
+// corpusFact reduces one recorded audit event to the fact the stream would carry.
+func corpusFact(t *testing.T, event auditv1.Event) AuthorFact {
+ t.Helper()
+ fact, _, ok := AuthorFactFromEvent(t.Context(), event, 0)
+ require.True(t, ok, "the recorded event produced no fact")
+ return fact
+}
+
+// TestFactIndex_FinalizerRemovalNamesTheDeleter pins the sticky removal pointer against the
+// captured audit events that made it necessary.
+//
+// The shape, measured: a human's `delete` on a finalized object and the controller's `patch` that
+// clears the finalizer BOTH carry a response body, and both bodies carry the SAME resourceVersion —
+// the one the deletion stamped. The two facts are therefore filed under identical keys, and every
+// ordinary structure in the index is last-writer-wins, so the controller's fact never had to
+// outrank the human's: it simply REPLACED it, and no tier ordering could recover a fact that was no
+// longer stored.
+//
+// The watch event that renders as the removal is the deletion-pending MODIFIED, whose
+// resourceVersion is that same value. The fix is that a removal fact also takes a sticky, uid-keyed
+// slot that a later WRITE fact may not overwrite, and that a removal asks that slot first. So the
+// answer no longer depends on whether the controller's fact has landed yet — a race nothing in the
+// join can see. This test pins both sides of that race producing the same author.
+func TestFactIndex_FinalizerRemovalNamesTheDeleter(t *testing.T) {
+ base := time.Now().Add(-time.Second)
+ deleteEvent := loadCorpusAuditEvent(t, corpusDeletionIntentDir, "audit.delete.yaml", base)
+ patchEvent := loadCorpusAuditEvent(t, corpusDeletionIntentDir, "audit.patch.yaml", base.Add(100*time.Millisecond))
+
+ deleteFact := corpusFact(t, deleteEvent)
+ patchFact := corpusFact(t, patchEvent)
+
+ // The premise, restated as an assertion so this test fails loudly if a Kubernetes upgrade ever
+ // stops the two events colliding — at which point the bug below is gone for a better reason.
+ require.NotEmpty(t, deleteFact.UID)
+ require.Equal(t, deleteFact.UID, patchFact.UID, "both events describe one object")
+ require.Equal(t, deleteFact.ResourceVersion, patchFact.ResourceVersion,
+ "the finalizer patch's response carries the resourceVersion the DELETION stamped; "+
+ "that collision is what lets one fact replace the other")
+ require.NotEqual(t, deleteFact.Author, patchFact.Author, "the two phases have different actors")
+
+ // The watch event the deletion-as-intent rule renders as a removal: the object as it looked when
+ // deletionTimestamp appeared, so its resourceVersion is the colliding one. ExactCapable is false
+ // because the operator maps it to a DELETE (operationForLiveTargetWatchEvent).
+ removal := FactQuery{
+ AuditRoute: "prod-eu-1",
+ GroupResource: schema.GroupResource{Resource: "configmaps"},
+ UID: deleteFact.UID,
+ ResourceVersion: deleteFact.ResourceVersion,
+ Namespace: deleteFact.Namespace,
+ Name: deleteFact.Name,
+ ExactCapable: false,
+ }
+
+ t.Run("the deleter is named while their fact is the only one filed", func(t *testing.T) {
+ harness := newFactIndexHarness(t, FactIndexConfig{})
+ key := FactStreamKeyFor("prod-eu-1", schema.GroupResource{Resource: "configmaps"})
+ harness.publish(key, deleteFact)
+ harness.waitForFacts(1)
+
+ resolution := harness.resolve(removal)
+ require.Equal(t, deleteFact.Author, resolution.Fact.Author,
+ "with only the deletion filed, the removal names the actor who requested it")
+ require.Equal(t, AttributionDeleteSticky, resolution.Result,
+ "and it names them from the removal pointer, which the delete fact filled")
+ })
+
+ t.Run("the deleter is still named once the finalizer controller's fact lands", func(t *testing.T) {
+ harness := newFactIndexHarness(t, FactIndexConfig{})
+ key := FactStreamKeyFor("prod-eu-1", schema.GroupResource{Resource: "configmaps"})
+ // One append, in the order the API server produced them — which is what an audit batch
+ // covering both phases delivers, and what a fast finalizer controller guarantees.
+ harness.publish(key, deleteFact, patchFact)
+ harness.waitForFacts(1)
+
+ resolution := harness.resolve(removal)
+
+ // The controller's patch fact has overwritten the exact and latest tiers, exactly as it did
+ // before — that is what a last-writer-wins structure does, and it is correct for the question
+ // those tiers answer. It cannot reach the removal pointer, so the deleter's fact survives, and
+ // a removal asks the pointer first.
+ require.Equal(t, deleteFact.Author, resolution.Fact.Author,
+ "a fact about a WRITE must not replace the fact about the DELETION")
+ require.NotEqual(t, patchFact.Author, resolution.Fact.Author,
+ "the cleanup controller is not who asked for the deletion")
+ require.Equal(t, AttributionDeleteSticky, resolution.Result,
+ "and the tier says which evidence answered: the removal pointer, not the colliding "+
+ "(uid, resourceVersion) the finalizer patch shares with it")
+ })
+}
diff --git a/internal/queue/fact_index_store.go b/internal/queue/fact_index_store.go
index 48a5b00d..f99fdc9d 100644
--- a/internal/queue/fact_index_store.go
+++ b/internal/queue/fact_index_store.go
@@ -8,7 +8,7 @@ import (
"k8s.io/apimachinery/pkg/labels"
)
-// factKind names one of the five match structures a fact can land in. The set is closed: a fact
+// factKind names one of the six match structures a fact can land in. The set is closed: a fact
// that fits none of them is not stored, because a fact nothing can ever join is only memory.
type factKind uint8
@@ -17,6 +17,9 @@ const (
factKindExact factKind = iota
// factKindLatest is (uid), last-writer-wins, serving removals whose rv never matches.
factKindLatest
+ // factKindRemoval is (uid), the sticky removal pointer: the fact that this object was DELETED and
+ // by whom. It is the one structure a later fact may not overwrite — see putRemoval.
+ factKindRemoval
// factKindRV is (rv), the escape hatch for a fact with an rv but no uid.
factKindRV
// factKindName is (namespace, name), the floor for a fact with neither a uid nor an rv. Only an
@@ -136,13 +139,16 @@ type factRef struct {
seq uint64
}
-// scopeFacts is one (route, group/resource)'s four match structures, its oldest-first insertion
+// scopeFacts is one (route, group/resource)'s match structures, its oldest-first insertion
// order, and its entry count. Bounding per scope rather than globally is what keeps a burst on one
// noisy type — a deletecollection over ten thousand objects, a large rollout — from evicting every
// other type's facts.
type scopeFacts struct {
- exact map[exactFactKey]*indexedFact
- latest map[string]*indexedFact
+ exact map[exactFactKey]*indexedFact
+ latest map[string]*indexedFact
+ // removals is the sticky removal pointer, keyed by uid: "this object was deleted, and this actor
+ // asked for it". It is deliberately NOT swept by the TTL — see putRemoval and sweep.
+ removals map[string]*indexedFact
rvOnly map[string]*indexedFact
byName map[nameFactKey]*indexedFact
collections []*indexedCollection
@@ -154,10 +160,11 @@ type scopeFacts struct {
func newScopeFacts() *scopeFacts {
return &scopeFacts{
- exact: map[exactFactKey]*indexedFact{},
- latest: map[string]*indexedFact{},
- rvOnly: map[string]*indexedFact{},
- byName: map[nameFactKey]*indexedFact{},
+ exact: map[exactFactKey]*indexedFact{},
+ latest: map[string]*indexedFact{},
+ removals: map[string]*indexedFact{},
+ rvOnly: map[string]*indexedFact{},
+ byName: map[nameFactKey]*indexedFact{},
}
}
@@ -182,6 +189,27 @@ func (s *scopeFacts) putLatest(uid string, entry *indexedFact) {
s.order = append(s.order, factRef{kind: factKindLatest, uid: uid, seq: entry.seq})
}
+// putRemoval stores the sticky removal pointer for an object: the fact that it was DELETED, and by
+// whom. Only a fact whose own verb is a removal is ever filed here, which is what makes the slot
+// sticky — a later WRITE fact fills the ordinary tiers and cannot reach this one.
+//
+// That stickiness is the fix for a measured defect, and it is the reason the slot has to exist at
+// all rather than the ranking being adjusted. A human's `delete` on a finalized object and the
+// controller's `patch` that clears the finalizer both return a body carrying the resourceVersion the
+// DELETION stamped, so both facts are filed under the same (uid, rv) and the same uid. Every other
+// structure here is last-writer-wins, so the deleter's fact is not outranked by the controller's —
+// it is replaced, and no tier ordering can recover what is no longer stored. See
+// docs/design/attribution-deletion-intent-actor.md.
+//
+// A later removal fact may replace it, because that is a statement about the same question.
+func (s *scopeFacts) putRemoval(uid string, entry *indexedFact) {
+ if _, ok := s.removals[uid]; !ok {
+ s.count++
+ }
+ s.removals[uid] = entry
+ s.order = append(s.order, factRef{kind: factKindRemoval, uid: uid, seq: entry.seq})
+}
+
// putRV stores the rv-only escape hatch.
func (s *scopeFacts) putRV(rv string, entry *indexedFact) {
if _, ok := s.rvOnly[rv]; !ok {
@@ -222,6 +250,25 @@ func (s *scopeFacts) lookupLatest(uid string, cutoff time.Time) (AuthorFact, boo
return liveFact(s.latest[uid], cutoff)
}
+// lookupRemovalPointer reads the sticky removal pointer. It takes NO cutoff, and it is the only lookup here
+// that does not: the pointer's horizon is a count rather than a clock.
+//
+// Every other structure must expire for correctness — `exact` and `latest` can be superseded by a
+// later write, and a NAME is reused after a delete and recreate, which is exactly why the name tier
+// ranks last and why the TTL has to keep bounding it. A uid-keyed removal statement has neither
+// failure mode, because a uid is unique across space and time: "uid X was deleted, and this actor
+// asked for it" can never be superseded, since that object can never be written, deleted, or
+// recreated under the same uid again. Holding it longer is therefore never less accurate, only more
+// expensive, so it is bounded the way memory is bounded — by the per-type and total caps, evicted
+// oldest-first under pressure and counted on attribution_fact_index_evictions_total.
+func (s *scopeFacts) lookupRemovalPointer(uid string) (AuthorFact, bool) {
+ entry, ok := s.removals[uid]
+ if !ok {
+ return AuthorFact{}, false
+ }
+ return entry.fact, true
+}
+
// lookupRV reads the rv-only escape hatch.
func (s *scopeFacts) lookupRV(rv string, cutoff time.Time) (AuthorFact, bool) {
return liveFact(s.rvOnly[rv], cutoff)
@@ -299,6 +346,11 @@ func (s *scopeFacts) sweep(cutoff time.Time) int {
removed++
}
}
+ // s.removals is not swept. Its horizon is the caps rather than the clock, for the reason
+ // lookupRemovalPointer gives: a uid-keyed removal statement can never be superseded. One
+ // consequence is worth naming — a scope that has held a deletion never goes empty, so the index
+ // keeps the (route, group/resource) entry alive until eviction reclaims it. That is bounded by
+ // construction, and it is the same bound the rest of the index already lives under.
kept := s.collections[:0]
for _, entry := range s.collections {
if entry.at.Before(cutoff) {
@@ -343,6 +395,8 @@ func (s *scopeFacts) remove(ref factRef) bool {
return removeKeyed(s.exact, exactFactKey{uid: ref.uid, rv: ref.rv}, ref.seq)
case factKindLatest:
return removeKeyed(s.latest, ref.uid, ref.seq)
+ case factKindRemoval:
+ return removeKeyed(s.removals, ref.uid, ref.seq)
case factKindRV:
return removeKeyed(s.rvOnly, ref.rv, ref.seq)
case factKindName:
@@ -377,8 +431,26 @@ func (s *scopeFacts) removeCollection(seq uint64) bool {
// compact drops eviction references whose entries are gone, so the order does not grow with every
// swept entry.
+//
+// It reallocates rather than reusing the backing array when the array has grown far past what
+// survives, and that is not a micro-optimization. A put REPLACING an entry appends a reference and
+// leaves the old one behind — the reference is what makes replacement safe, since the sequence check
+// is how a stale reference is told from a live one — so an object rewritten a thousand times inside
+// one sweep interval leaves a thousand references and ONE entry. The dead references go here either
+// way; the peak capacity would not, because `s.order[:0]` keeps the array. That memory is the kind
+// worth giving back: `count` never counted it, so neither the per-type cap nor
+// attribution_fact_index_entries can see it.
func (s *scopeFacts) compact() {
+ live := 0
+ for _, ref := range s.order {
+ if s.live(ref) {
+ live++
+ }
+ }
kept := s.order[:0]
+ if cap(s.order) > 2*live+orderCapacitySlack {
+ kept = make([]factRef, 0, live)
+ }
for _, ref := range s.order {
if s.live(ref) {
kept = append(kept, ref)
@@ -387,6 +459,11 @@ func (s *scopeFacts) compact() {
s.order = kept
}
+// orderCapacitySlack is how much spare capacity the eviction order may keep before compaction hands
+// it back. It exists so an ordinary scope, whose order grows and shrinks by a handful of entries per
+// sweep, keeps reusing one small array instead of reallocating on every pass.
+const orderCapacitySlack = 64
+
// live reports whether a reference still names the entry it was taken for.
func (s *scopeFacts) live(ref factRef) bool {
switch ref.kind {
@@ -394,6 +471,8 @@ func (s *scopeFacts) live(ref factRef) bool {
return liveKeyed(s.exact, exactFactKey{uid: ref.uid, rv: ref.rv}, ref.seq)
case factKindLatest:
return liveKeyed(s.latest, ref.uid, ref.seq)
+ case factKindRemoval:
+ return liveKeyed(s.removals, ref.uid, ref.seq)
case factKindRV:
return liveKeyed(s.rvOnly, ref.rv, ref.seq)
case factKindName:
diff --git a/internal/queue/fact_index_test.go b/internal/queue/fact_index_test.go
index 344a81e1..3a466779 100644
--- a/internal/queue/fact_index_test.go
+++ b/internal/queue/fact_index_test.go
@@ -211,6 +211,54 @@ func TestFactIndex_FactsAreAbsentPastTheTTL(t *testing.T) {
require.Zero(t, harness.index.Len())
}
+// TestFactIndex_RewritingOneKeyDoesNotStrandTheEvictionOrder bounds the memory a HOT OBJECT costs.
+//
+// A put that replaces an entry appends an eviction reference and leaves the old one behind — that is
+// what makes replacement safe, since the sequence check is how a stale reference is told from a live
+// one. So an object rewritten N times inside one sweep interval holds ONE entry and N references, and
+// the caps cannot see the difference: they count entries, which is also what the entries gauge
+// reports. The sweep drops the dead references, and must also hand back the capacity they occupied,
+// or a single retry storm leaves its peak resident for the life of the scope.
+func TestFactIndex_RewritingOneKeyDoesNotStrandTheEvictionOrder(t *testing.T) {
+ harness := newFactIndexHarness(t, FactIndexConfig{})
+ const rewrites = 4000
+ facts := make([]AuthorFact, 0, rewrites)
+ for range rewrites {
+ // The same object at the same version, over and over: one exact key and one latest key.
+ facts = append(facts, objectFact("alice", "101"))
+ }
+ harness.publish(factIndexTestStream("prod-eu-1"), facts...)
+
+ // Waiting on the ORDER rather than on Len is the only barrier available here: Len reaches 2 on
+ // the first fact and stays there, so it cannot say the batch has finished applying.
+ scope := factScope{route: "prod-eu-1", groupResource: groupResourceKey("apps", "deployments")}
+ require.Eventually(t, func() bool { return harness.orderCapacity(scope) > rewrites },
+ factIndexTestGrace, factIndexTestBlock,
+ "the references are what the sweep has to reclaim, so the test is only meaningful if they piled up")
+ require.Equal(t, 2, harness.index.Len(), "one exact and one latest entry, however many rewrites landed")
+
+ harness.index.Sweep(time.Now())
+
+ require.Equal(t, 2, harness.index.Len(), "the sweep drops references, never live entries")
+ require.Less(t, harness.orderCapacity(scope), rewrites/2,
+ "and it hands back the capacity rather than keeping the peak for the life of the scope")
+ require.Equal(t, "alice", harness.resolve(objectQuery("prod-eu-1", factIndexTestUID, "101", true)).Fact.Author,
+ "the surviving entry is still joinable through its reference-compacted scope")
+}
+
+// orderCapacity reads one scope's eviction-order capacity under the index lock, which is where the
+// cost of a replaced entry accumulates.
+func (h *factIndexHarness) orderCapacity(scope factScope) int {
+ h.t.Helper()
+ h.index.mu.Lock()
+ defer h.index.mu.Unlock()
+ facts, ok := h.index.scopes[scope]
+ if !ok {
+ return 0
+ }
+ return cap(facts.order)
+}
+
func TestFactIndex_PerTypeCapEvictsOldestFirstAndCountsIt(t *testing.T) {
reader, err := telemetry.InitTestExporter()
require.NoError(t, err)
@@ -331,7 +379,7 @@ func TestFactIndex_CollectionMatchesByUIDMembership(t *testing.T) {
// the API server said it deleted this object or it did not.
for _, uid := range []string{"uid-1", "uid-2"} {
resolution := harness.resolve(objectQuery("prod-eu-1", uid, "999", false))
- require.Equal(t, AttributionCollectionUID, resolution.Result)
+ require.Equal(t, AttributionDeleteCollectionBodyUID, resolution.Result)
require.Equal(t, "alice", resolution.Fact.Author)
}
@@ -352,7 +400,7 @@ func TestFactIndex_CollectionMatchesByScopeAndSelector(t *testing.T) {
matching := objectQuery("prod-eu-1", "uid-1", "999", false)
matching.Labels = map[string]string{"app": "web", "tier": "front"}
resolution := harness.resolve(matching)
- require.Equal(t, AttributionCollectionScope, resolution.Result)
+ require.Equal(t, AttributionDeleteCollectionScope, resolution.Result)
require.Equal(t, "alice", resolution.Fact.Author)
// The selector is the intent the actor expressed: an object it does not select was not part of
@@ -375,7 +423,7 @@ func TestFactIndex_CollectionWithNoSelectorCoversTheWholeNamespace(t *testing.T)
// No selector is what --all means.
unlabelled := objectQuery("prod-eu-1", "uid-9", "999", false)
- require.Equal(t, AttributionCollectionScope, harness.resolve(unlabelled).Result)
+ require.Equal(t, AttributionDeleteCollectionScope, harness.resolve(unlabelled).Result)
}
func TestFactIndex_CollectionScopeMatchStopsAtTheWindow(t *testing.T) {
@@ -383,7 +431,8 @@ func TestFactIndex_CollectionScopeMatchStopsAtTheWindow(t *testing.T) {
harness := newFactIndexHarness(t, FactIndexConfig{CollectionWindow: window})
harness.publish(factIndexTestStream("prod-eu-1"), aliceCollectionFact(""))
harness.waitForFacts(1)
- require.Equal(t, AttributionCollectionScope, harness.resolve(objectQuery("prod-eu-1", "uid-9", "9", false)).Result)
+ scoped := harness.resolve(objectQuery("prod-eu-1", "uid-9", "9", false))
+ require.Equal(t, AttributionDeleteCollectionScope, scoped.Result)
// An unrelated delete later in the same namespace must not be claimed by a collection that has
// long since finished.
@@ -427,7 +476,7 @@ func TestFactIndex_CollectionUIDOutranksAStaleWriteFact(t *testing.T) {
harness.waitForFacts(3)
resolution := harness.resolve(objectQuery("prod-eu-1", factIndexTestUID, "999", false))
- require.Equal(t, AttributionCollectionUID, resolution.Result)
+ require.Equal(t, AttributionDeleteCollectionBodyUID, resolution.Result)
require.Equal(t, "alice", resolution.Fact.Author,
"the removal was caused by alice's collection delete, not by bob's earlier update")
@@ -758,8 +807,8 @@ func TestFactIndex_ARemovalWaitsForEvidenceAboutTheDeletion(t *testing.T) {
}()
resolution := harness.index.Await(t.Context(),
- removalFactQuery("prod-eu-1", factIndexTestUID), 5*time.Second)
- require.Equal(t, AttributionCollectionUID, resolution.Result)
+ removalFactQuery(factIndexTestUID), 5*time.Second)
+ require.Equal(t, AttributionDeleteCollectionBodyUID, resolution.Result)
require.Equal(t, "alice", resolution.Fact.Author,
"a removal must wait for evidence about the deletion, not settle for the last edit")
}
@@ -774,7 +823,7 @@ func TestFactIndex_ARemovalStillNamesTheLastWriterWhenNothingBetterArrives(t *te
start := time.Now()
resolution := harness.index.Await(t.Context(),
- removalFactQuery("prod-eu-1", factIndexTestUID), 150*time.Millisecond)
+ removalFactQuery(factIndexTestUID), 150*time.Millisecond)
require.Equal(t, AttributionLatest, resolution.Result)
require.Equal(t, "bob", resolution.Fact.Author, "waiting must not turn a match into an absence")
@@ -793,16 +842,91 @@ func TestFactIndex_ARemovalsOwnDeleteFactEndsTheWaitAtOnce(t *testing.T) {
start := time.Now()
resolution := harness.index.Await(t.Context(),
- removalFactQuery("prod-eu-1", factIndexTestUID), 5*time.Second)
+ removalFactQuery(factIndexTestUID), 5*time.Second)
require.Equal(t, "carol", resolution.Fact.Author)
require.Less(t, time.Since(start), 2*time.Second, "a delete fact must not be held as a fallback")
}
+// TestFactIndex_TheRemovalPointerOutlivesTheTTL pins the horizon the removal pointer deliberately
+// does not share with the rest of the index.
+//
+// This is the replay case. An object left Terminating by a hung finalizer is first OBSERVED hours
+// later, after a restart, a rollout, or a 410 rebuild collapses the watch to CURRENT state: there is
+// no transition event, the file is still in Git, and that first observation renders as a DELETE.
+// Every ordinary tier holding the deleter's fact aged out long before, so the commit is authored
+// unresolved — after sitting out the whole grace window on the shard's serial goroutine, waiting for
+// a fact that can never arrive. The pointer turns that into an immediate hit.
+func TestFactIndex_TheRemovalPointerOutlivesTheTTL(t *testing.T) {
+ const ttl = 300 * time.Millisecond
+ harness := newFactIndexHarness(t, FactIndexConfig{TTL: ttl})
+ deleteFact := objectFact("alice", "101")
+ deleteFact.Verb = "delete"
+ harness.publish(factIndexTestStream("prod-eu-1"), deleteFact)
+ // Exact, latest, and the removal pointer: one delete fact carrying a uid and an rv fills all three.
+ harness.waitForFacts(3)
+
+ time.Sleep(3 * ttl)
+ require.Equal(t, 2, harness.index.Sweep(time.Now()), "the two TTL-bounded entries are reclaimed")
+
+ // The ordinary tiers are gone, including the exact join for the version the deletion stamped.
+ require.Equal(t, AttributionAbsent,
+ harness.index.Lookup(objectQuery("prod-eu-1", factIndexTestUID, "101", true)).Result)
+
+ resolution := harness.index.Lookup(removalFactQuery(factIndexTestUID))
+ require.Equal(t, AttributionDeleteSticky, resolution.Result,
+ "a removal still reaches the pointer once every TTL-bounded tier has expired")
+ require.Equal(t, "alice", resolution.Fact.Author)
+ require.Equal(t, 1, harness.index.Len(), "and the pointer is what the index is still holding")
+}
+
+// TestFactIndex_ARecreatedNameDoesNotInheritThePreviousDeleter pins why the pointer is keyed on the
+// uid and nothing else.
+//
+// A uid is unique across space and time, which is the whole argument for letting the pointer outlive
+// the TTL: the statement can never be superseded. A NAME has no such property — it is reused after a
+// delete and recreate — so the same stickiness on the name tier would not be this fix, it would be a
+// defect that a longer horizon makes MORE likely rather than less. The name tier stays bounded by
+// the TTL exactly as before.
+func TestFactIndex_ARecreatedNameDoesNotInheritThePreviousDeleter(t *testing.T) {
+ harness := newFactIndexHarness(t, FactIndexConfig{})
+ key := factIndexTestStream("prod-eu-1")
+ deleted := AuthorFact{
+ Namespace: "team-a", UID: "uid-old", Name: "checkout",
+ ResourceVersion: "101", Author: "alice", Verb: "delete",
+ }
+ recreated := AuthorFact{
+ Namespace: "team-a", UID: "uid-new", Name: "checkout",
+ ResourceVersion: "202", Author: "bob", Verb: "update",
+ }
+ harness.publish(key, deleted, recreated)
+ harness.waitForFacts(5)
+
+ // The recreated object is removed in its turn. It shares the name, and nothing else.
+ removal := FactQuery{
+ AuditRoute: "prod-eu-1",
+ GroupResource: deploymentsGroupResource(),
+ UID: "uid-new",
+ ResourceVersion: "999",
+ Namespace: "team-a",
+ Name: "checkout",
+ }
+ resolution := harness.index.Lookup(removal)
+ require.NotEqual(t, "alice", resolution.Fact.Author,
+ "the previous object's deleter must not be inherited through a reused name")
+ require.Equal(t, "bob", resolution.Fact.Author)
+ require.Equal(t, AttributionLatest, resolution.Result, "which leaves the ordinary fallback")
+
+ // The first object's pointer is untouched by any of that: it is a statement about a uid.
+ previous := harness.index.Lookup(removalFactQuery("uid-old"))
+ require.Equal(t, AttributionDeleteSticky, previous.Result)
+ require.Equal(t, "alice", previous.Fact.Author)
+}
+
// removalFactQuery is a DELETE watch event: its resourceVersion is never the one a write produced.
-func removalFactQuery(route, uid string) FactQuery {
+func removalFactQuery(uid string) FactQuery {
return FactQuery{
- AuditRoute: route,
+ AuditRoute: "prod-eu-1",
GroupResource: deploymentsGroupResource(),
UID: uid,
ResourceVersion: "999",
diff --git a/internal/telemetry/exporter.go b/internal/telemetry/exporter.go
index ce1c90e5..68855dc5 100644
--- a/internal/telemetry/exporter.go
+++ b/internal/telemetry/exporter.go
@@ -100,7 +100,8 @@ var (
AuditEventListDurationSeconds metric.Float64Histogram
// AttributionResolutionsTotal counts watch-event attribution resolver outcomes, labelled by
// {tier, actor_kind, group, version, resource}. tier names WHICH evidence answered
- // (exact/latest/resource_version/name/collection_uid/collection_scope/absent) and actor_kind
+ // (delete_sticky/exact/deletecollection_body_uid/latest/name/deletecollection_scope/
+ // resource_version/absent) and actor_kind
// names WHO it named (user/serviceaccount/none) — two orthogonal questions, so they are two
// labels. Match coverage is tier!="absent"; anything narrower reads the collection and name
// tiers as misses.
diff --git a/test/e2e/aggregated_deletecollection_e2e_test.go b/test/e2e/aggregated_deletecollection_e2e_test.go
index 001811f6..2e619946 100644
--- a/test/e2e/aggregated_deletecollection_e2e_test.go
+++ b/test/e2e/aggregated_deletecollection_e2e_test.go
@@ -148,7 +148,7 @@ func collectionResolutionCounts() map[string]float64 {
// silent hole rather than a failure: the spec asserts the author, so it passes either way.
ensurePrometheusClient()
counts := map[string]float64{}
- for _, tier := range []string{"collection_uid", "collection_scope"} {
+ for _, tier := range []string{"deletecollection_body_uid", "deletecollection_scope"} {
n, err := queryPrometheus(fmt.Sprintf(
`sum(max_over_time(gitopsreverser_attribution_resolutions_total{tier=%q}[2h])) or vector(0)`, tier))
if err != nil {
@@ -167,18 +167,18 @@ func reportCollectionTier(before map[string]float64) {
return
}
after := collectionResolutionCounts()
- uid := after["collection_uid"] - before["collection_uid"]
- scope := after["collection_scope"] - before["collection_scope"]
+ uid := after["deletecollection_body_uid"] - before["deletecollection_body_uid"]
+ scope := after["deletecollection_scope"] - before["deletecollection_scope"]
switch {
case scope > 0 && uid == 0:
_, _ = fmt.Fprintf(GinkgoWriter,
- "\nℹ️ aggregated deletecollection resolved by SCOPE (+%.0f collection_scope): the proxied "+
+ "\nℹ️ aggregated deletecollection resolved by SCOPE (+%.0f deletecollection_scope): the proxied "+
"request carried no response body, which is exactly the case the deleted expander "+
"produced nothing for.\n", scope)
case uid > 0:
_, _ = fmt.Fprintf(GinkgoWriter,
- "\nℹ️ aggregated deletecollection resolved by UID membership (+%.0f collection_uid, "+
- "+%.0f collection_scope): this API server DID return the deleted set for a proxied "+
+ "\nℹ️ aggregated deletecollection resolved by UID membership (+%.0f deletecollection_body_uid, "+
+ "+%.0f deletecollection_scope): this API server DID return the deleted set for a proxied "+
"collection delete.\n", uid, scope)
default:
_, _ = fmt.Fprintf(GinkgoWriter,
diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go
index 0d832b30..07c6635a 100644
--- a/test/e2e/e2e_suite_test.go
+++ b/test/e2e/e2e_suite_test.go
@@ -171,11 +171,13 @@ func reportAttributionStats() {
_, _ = fmt.Fprintf(GinkgoWriter, "\n📊 author attribution — %.0f resolutions this run\n", total)
var absent float64
- // The tiers, strongest first. collection_uid and collection_scope replaced the expander's
- // exact_deletecollection_item: the collection match is two-tiered now, and the second tier
- // resolves what used to degrade to committer-authored, so they are worth reading apart.
+ // The tiers, strongest first. The three named for a delete verb are the ones reachable only by a
+ // removal: the sticky pointer, and the two deletecollection tiers that replaced the expander's
+ // exact_deletecollection_item. Those two are worth reading apart because the scope one resolves
+ // what used to degrade to committer-authored.
for _, tier := range []string{
- "exact", "collection_uid", "latest", "name", "collection_scope", "resource_version", "absent",
+ "delete_sticky", "exact", "deletecollection_body_uid", "latest", "name",
+ "deletecollection_scope", "resource_version", "absent",
} {
n, qErr := queryPrometheus(fmt.Sprintf(
`sum(max_over_time(gitopsreverser_attribution_resolutions_total{tier=%q}[2h])) or vector(0)`, tier))
diff --git a/test/mutationlab/README.md b/test/mutationlab/README.md
index dc96da47..7f6283ac 100644
--- a/test/mutationlab/README.md
+++ b/test/mutationlab/README.md
@@ -43,9 +43,19 @@ gap (see [Capturing Intent, Not State](../../docs/spec/mutation-capture-lab-desi
| 15a | Aggregated API delete | `aggregated_api_test.go` · `TestAggregatedAPIDelete` | `flunder/aggregated-api-delete/` | watch DELETED (full object), audit (`delete`, `objectRef` has the NAME from the URL but **no uid**) |
| 15b | Aggregated API deletecollection | `aggregated_api_test.go` · `TestAggregatedAPIDeletecollection` | `flunder/aggregated-api-deletecollection/` | watch ×N, audit ×1 (name-less, selector in `requestURI`, **no response body**), admission ×N (per object) |
| 16 | Watch resync (`410 Gone`) | `watch_transport_test.go` · `TestWatchExpiredResourceVersion` | `configmap/watch-resync/` | watch ERROR (`Status` 410); driver verifies relist recovery |
+| 8a | Finalizer delete, two actors + tunable hold | `deletion_intent_actor_test.go` · `TestDeletionIntentActorRace` | `configmap/deletion-intent-actor/` | watch (MODIFIED + DELETED), audit (`delete` by the human + `patch` by a ServiceAccount) — both audit response bodies carry the **same** resourceVersion |
| 18 | `generateName` create | `configmap_scenarios_test.go` · `TestGenerateNameCreate` | `configmap/generate-name-create/` | watch, audit (`objectRef` has **no name**, response body **does**), admission — the control for rows 15a/15b |
| 17 | Bookmark | `watch_transport_test.go` · `TestWatchBookmark` | `configmap/watch-bookmark/` | watch BOOKMARK with resourceVersion |
+Row 8a is not a catalog row: it extends row 8 to the case the catalog could not express, because
+row 8 drives both phases with one identity. A finalized deletion in production has TWO actors — a
+human asks, a controller cleans up — and the row exists to measure what that does to attribution.
+The finding is in the last column: the human's `delete` and the controller's finalizer `patch` both
+return a body, and both bodies carry the resourceVersion the DELETION stamped, so the two facts they
+produce collide on one key. `LAB_FINALIZER_HOLD` tunes the gap between the phases, which is what
+decides whether both land in one audit batch. See
+[attribution-deletion-intent-actor.md](../../docs/design/attribution-deletion-intent-actor.md).
+
All seventeen catalogued scenarios are now captured. Rows 15a and 15b are not catalog
rows: they extend row 15 to the removal verbs, because the create alone could not say
whether a proxied delete carries a uid (it does not) or whether a proxied
diff --git a/test/mutationlab/corpus/configmap/deletion-intent-actor/admission.delete.yaml b/test/mutationlab/corpus/configmap/deletion-intent-actor/admission.delete.yaml
new file mode 100644
index 00000000..84cd074d
--- /dev/null
+++ b/test/mutationlab/corpus/configmap/deletion-intent-actor/admission.delete.yaml
@@ -0,0 +1,62 @@
+request:
+ dryRun: false
+ kind:
+ group: ""
+ kind: ConfigMap
+ version: v1
+ name: cm-hold-short
+ namespace:
+ object: null
+ oldObject:
+ apiVersion: v1
+ data:
+ key: value
+ kind: ConfigMap
+ metadata:
+ creationTimestamp:
+ finalizers:
+ - mutationlab.configbutler.ai/hold
+ labels:
+ mutationlab.configbutler.ai/scenario: deletion-intent-actor
+ managedFields:
+ - apiVersion: v1
+ fieldsType: FieldsV1
+ fieldsV1:
+ f:data:
+ .: {}
+ f:key: {}
+ f:metadata:
+ f:finalizers:
+ .: {}
+ v:"mutationlab.configbutler.ai/hold": {}
+ f:labels:
+ .: {}
+ f:mutationlab.configbutler.ai/scenario: {}
+ manager: e2e.test
+ operation: Update
+ time:
+ name: cm-hold-short
+ namespace:
+ resourceVersion:
+ uid:
+ operation: DELETE
+ options:
+ apiVersion: meta.k8s.io/v1
+ kind: DeleteOptions
+ requestKind:
+ group: ""
+ kind: ConfigMap
+ version: v1
+ requestResource:
+ group: ""
+ resource: configmaps
+ version: v1
+ resource:
+ group: ""
+ resource: configmaps
+ version: v1
+ uid:
+ userInfo:
+ groups:
+ - system:authenticated
+ username: alice@example.com
diff --git a/test/mutationlab/corpus/configmap/deletion-intent-actor/admission.update.yaml b/test/mutationlab/corpus/configmap/deletion-intent-actor/admission.update.yaml
new file mode 100644
index 00000000..d57092d9
--- /dev/null
+++ b/test/mutationlab/corpus/configmap/deletion-intent-actor/admission.update.yaml
@@ -0,0 +1,98 @@
+request:
+ dryRun: false
+ kind:
+ group: ""
+ kind: ConfigMap
+ version: v1
+ name: cm-hold-short
+ namespace:
+ object:
+ apiVersion: v1
+ data:
+ key: value
+ kind: ConfigMap
+ metadata:
+ creationTimestamp:
+ deletionGracePeriodSeconds: 0
+ deletionTimestamp:
+ labels:
+ mutationlab.configbutler.ai/scenario: deletion-intent-actor
+ managedFields:
+ - apiVersion: v1
+ fieldsType: FieldsV1
+ fieldsV1:
+ f:data:
+ .: {}
+ f:key: {}
+ f:metadata:
+ f:labels:
+ .: {}
+ f:mutationlab.configbutler.ai/scenario: {}
+ manager: e2e.test
+ operation: Update
+ time:
+ name: cm-hold-short
+ namespace:
+ resourceVersion:
+ uid:
+ oldObject:
+ apiVersion: v1
+ data:
+ key: value
+ kind: ConfigMap
+ metadata:
+ creationTimestamp:
+ deletionGracePeriodSeconds: 0
+ deletionTimestamp:
+ finalizers:
+ - mutationlab.configbutler.ai/hold
+ labels:
+ mutationlab.configbutler.ai/scenario: deletion-intent-actor
+ managedFields:
+ - apiVersion: v1
+ fieldsType: FieldsV1
+ fieldsV1:
+ f:data:
+ .: {}
+ f:key: {}
+ f:metadata:
+ f:finalizers:
+ .: {}
+ v:"mutationlab.configbutler.ai/hold": {}
+ f:labels:
+ .: {}
+ f:mutationlab.configbutler.ai/scenario: {}
+ manager: e2e.test
+ operation: Update
+ time:
+ name: cm-hold-short
+ namespace:
+ resourceVersion:
+ uid:
+ operation: UPDATE
+ options:
+ apiVersion: meta.k8s.io/v1
+ kind: UpdateOptions
+ requestKind:
+ group: ""
+ kind: ConfigMap
+ version: v1
+ requestResource:
+ group: ""
+ resource: configmaps
+ version: v1
+ resource:
+ group: ""
+ resource: configmaps
+ version: v1
+ uid:
+ userInfo:
+ extra:
+ authentication.kubernetes.io/credential-id:
+ -
+ groups:
+ - system:serviceaccounts
+ - system:serviceaccounts:lab-actors
+ - system:authenticated
+ uid:
+ username: system:serviceaccount:lab-actors:finalizer-controller
diff --git a/test/mutationlab/corpus/configmap/deletion-intent-actor/audit.delete.yaml b/test/mutationlab/corpus/configmap/deletion-intent-actor/audit.delete.yaml
new file mode 100644
index 00000000..0869219b
--- /dev/null
+++ b/test/mutationlab/corpus/configmap/deletion-intent-actor/audit.delete.yaml
@@ -0,0 +1,55 @@
+annotations:
+ authorization.k8s.io/decision: allow
+ authorization.k8s.io/reason: 'RBAC: allowed by RoleBinding "scenario-human/lab-deletion-intent-actor-9mkc"
+ of Role "scenario-human" to User "alice@example.com"'
+ validate-all.configbutler.ai/scenario: deletion-intent-actor
+auditID:
+impersonatedUser:
+ groups:
+ - system:authenticated
+ username: alice@example.com
+level: RequestResponse
+objectRef:
+ apiVersion: v1
+ name: cm-hold-short
+ namespace:
+ resource: configmaps
+requestObject:
+ apiVersion: meta.k8s.io/__internal
+ kind: DeleteOptions
+requestReceivedTimestamp:
+requestURI: /api/v1/namespaces//configmaps/cm-hold-short
+responseObject:
+ apiVersion: v1
+ data:
+ key: value
+ kind: ConfigMap
+ metadata:
+ creationTimestamp:
+ deletionGracePeriodSeconds: 0
+ deletionTimestamp:
+ finalizers:
+ - mutationlab.configbutler.ai/hold
+ labels:
+ mutationlab.configbutler.ai/scenario: deletion-intent-actor
+ name: cm-hold-short
+ namespace:
+ resourceVersion:
+ uid:
+responseStatus:
+ code: 200
+ metadata: {}
+sourceIPs:
+-
+stage: ResponseComplete
+stageTimestamp:
+user:
+ extra:
+ authentication.kubernetes.io/credential-id:
+ -
+ groups:
+ - system:masters
+ - system:authenticated
+ username: system:admin
+userAgent: e2e.test/v0.0.0 (linux/amd64) kubernetes/$Format
+verb: delete
diff --git a/test/mutationlab/corpus/configmap/deletion-intent-actor/audit.patch.yaml b/test/mutationlab/corpus/configmap/deletion-intent-actor/audit.patch.yaml
new file mode 100644
index 00000000..80dc251b
--- /dev/null
+++ b/test/mutationlab/corpus/configmap/deletion-intent-actor/audit.patch.yaml
@@ -0,0 +1,51 @@
+annotations:
+ authorization.k8s.io/decision: allow
+ authorization.k8s.io/reason: 'RBAC: allowed by RoleBinding "finalizer-controller/lab-deletion-intent-actor-9mkc"
+ of Role "finalizer-controller" to ServiceAccount "finalizer-controller/lab-actors"'
+ validate-all.configbutler.ai/scenario: deletion-intent-actor
+auditID:
+level: RequestResponse
+objectRef:
+ apiVersion: v1
+ name: cm-hold-short
+ namespace:
+ resource: configmaps
+requestObject:
+ metadata:
+ finalizers: null
+requestReceivedTimestamp:
+requestURI: /api/v1/namespaces//configmaps/cm-hold-short
+responseObject:
+ apiVersion: v1
+ data:
+ key: value
+ kind: ConfigMap
+ metadata:
+ creationTimestamp:
+ deletionGracePeriodSeconds: 0
+ deletionTimestamp:
+ labels:
+ mutationlab.configbutler.ai/scenario: deletion-intent-actor
+ name: cm-hold-short
+ namespace:
+ resourceVersion:
+ uid:
+responseStatus:
+ code: 200
+ metadata: {}
+sourceIPs:
+-
+stage: ResponseComplete
+stageTimestamp:
+user:
+ extra:
+ authentication.kubernetes.io/credential-id:
+ -
+ groups:
+ - system:serviceaccounts
+ - system:serviceaccounts:lab-actors
+ - system:authenticated
+ uid:
+ username: system:serviceaccount:lab-actors:finalizer-controller
+userAgent: e2e.test/v0.0.0 (linux/amd64) kubernetes/$Format
+verb: patch
diff --git a/test/mutationlab/corpus/configmap/deletion-intent-actor/watch.deleted.yaml b/test/mutationlab/corpus/configmap/deletion-intent-actor/watch.deleted.yaml
new file mode 100644
index 00000000..5343f337
--- /dev/null
+++ b/test/mutationlab/corpus/configmap/deletion-intent-actor/watch.deleted.yaml
@@ -0,0 +1,35 @@
+object:
+ apiVersion: v1
+ data:
+ key: value
+ kind: ConfigMap
+ metadata:
+ creationTimestamp:
+ deletionGracePeriodSeconds: 0
+ deletionTimestamp:
+ finalizers:
+ - mutationlab.configbutler.ai/hold
+ labels:
+ mutationlab.configbutler.ai/scenario: deletion-intent-actor
+ managedFields:
+ - apiVersion: v1
+ fieldsType: FieldsV1
+ fieldsV1:
+ f:data:
+ .: {}
+ f:key: {}
+ f:metadata:
+ f:finalizers:
+ .: {}
+ v:"mutationlab.configbutler.ai/hold": {}
+ f:labels:
+ .: {}
+ f:mutationlab.configbutler.ai/scenario: {}
+ manager: e2e.test
+ operation: Update
+ time:
+ name: cm-hold-short
+ namespace:
+ resourceVersion:
+ uid:
+type: DELETED
diff --git a/test/mutationlab/corpus/configmap/deletion-intent-actor/watch.modified.yaml b/test/mutationlab/corpus/configmap/deletion-intent-actor/watch.modified.yaml
new file mode 100644
index 00000000..476b253f
--- /dev/null
+++ b/test/mutationlab/corpus/configmap/deletion-intent-actor/watch.modified.yaml
@@ -0,0 +1,35 @@
+object:
+ apiVersion: v1
+ data:
+ key: value
+ kind: ConfigMap
+ metadata:
+ creationTimestamp:
+ deletionGracePeriodSeconds: 0
+ deletionTimestamp:
+ finalizers:
+ - mutationlab.configbutler.ai/hold
+ labels:
+ mutationlab.configbutler.ai/scenario: deletion-intent-actor
+ managedFields:
+ - apiVersion: v1
+ fieldsType: FieldsV1
+ fieldsV1:
+ f:data:
+ .: {}
+ f:key: {}
+ f:metadata:
+ f:finalizers:
+ .: {}
+ v:"mutationlab.configbutler.ai/hold": {}
+ f:labels:
+ .: {}
+ f:mutationlab.configbutler.ai/scenario: {}
+ manager: e2e.test
+ operation: Update
+ time:
+ name: cm-hold-short
+ namespace:
+ resourceVersion:
+ uid:
+type: MODIFIED
diff --git a/test/mutationlab/e2e/deletion_intent_actor_test.go b/test/mutationlab/e2e/deletion_intent_actor_test.go
new file mode 100644
index 00000000..1264397c
--- /dev/null
+++ b/test/mutationlab/e2e/deletion_intent_actor_test.go
@@ -0,0 +1,269 @@
+//go:build mutationlab_e2e
+
+// SPDX-License-Identifier: Apache-2.0
+
+package e2e
+
+import (
+ "context"
+ "encoding/json"
+ "fmt"
+ "os"
+ "testing"
+ "time"
+
+ corev1 "k8s.io/api/core/v1"
+ rbacv1 "k8s.io/api/rbac/v1"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/apimachinery/pkg/types"
+ "k8s.io/client-go/kubernetes"
+
+ "github.com/ConfigButler/gitops-reverser/internal/mutationlab"
+)
+
+// The two actors this scenario drives one object with. They are the shape of the reported
+// mis-attribution: a HUMAN asks for the deletion, and a CONTROLLER's cleanup write is what
+// actually makes the object disappear. Both are real identities to the API server — the human
+// through impersonation, the controller through a ServiceAccount token — so the audit events
+// carry the actors a production cluster would carry, not a single admin twice.
+const (
+ deletionIntentHuman = "alice@example.com"
+ deletionIntentControllerSA = "finalizer-controller"
+)
+
+// deletionIntentHold is how long the controller waits, after the deletion is requested, before
+// clearing the finalizer. It is the knob this scenario exists to turn.
+//
+// The API server batches audit deliveries (--audit-webhook-batch-max-wait, 1s in this cluster), so
+// the hold decides whether the human's `delete` and the controller's `patch` arrive as ONE batch or
+// as two. That is not a detail of the capture; it is the race itself, because the two facts they
+// produce collide on one index key (see the resourceVersion law below), so which one a watch event
+// joins depends on whether the second has landed yet.
+func deletionIntentHold(t *testing.T, fallback time.Duration) time.Duration {
+ t.Helper()
+ raw := os.Getenv("LAB_FINALIZER_HOLD")
+ if raw == "" {
+ return fallback
+ }
+ hold, err := time.ParseDuration(raw)
+ if err != nil {
+ t.Fatalf("LAB_FINALIZER_HOLD=%q is not a duration: %v", raw, err)
+ }
+ return hold
+}
+
+// TestDeletionIntentActorRace captures what Kubernetes emits when a human deletes a finalized
+// object and a controller clears the finalizer — the shape behind "the deletion is attributed to
+// the controller that cleaned up, not to the person who asked for it".
+//
+// Row 8 (finalizer-delete) already established the two-phase removal with one actor. This scenario
+// adds the two things that turn that shape into a mis-attribution: a SECOND actor on the second
+// phase, and a controllable hold between them. Three objects are driven with three holds, because
+// the hold is what decides whether both audit events reach the operator in one batch.
+//
+// The law worth reading twice is the resourceVersion one. Both audit events carry a response body,
+// and both response bodies carry the SAME resourceVersion — the one the deletion stamped. So the
+// two facts they produce are keyed identically, and the later one (the controller's) simply
+// replaces the earlier one (the human's) in the index. It is not that the human's evidence ranks
+// lower; it is that it is no longer there.
+func TestDeletionIntentActorRace(t *testing.T) {
+ h := newHarness(t)
+ ctx := context.Background()
+ s := h.newScenario(ctx, t, "deletion-intent-actor")
+
+ human := h.asActor(t, deletionIntentHuman, "system:authenticated")
+ h.allowConfigMapWrites(ctx, t, s.ns, "scenario-human",
+ rbacv1.Subject{Kind: rbacv1.UserKind, APIGroup: rbacv1.GroupName, Name: deletionIntentHuman})
+ controller := h.asServiceAccount(ctx, t, s.ns, deletionIntentControllerSA)
+
+ // Three holds: none (the controller is already cleaning up when the delete lands), a hold
+ // longer than one audit batch window, and a hold long enough that no batching could merge them.
+ cases := []struct {
+ name string
+ hold time.Duration
+ }{
+ {name: "cm-hold-none", hold: deletionIntentHold(t, 0)},
+ {name: "cm-hold-short", hold: deletionIntentHold(t, 3*time.Second)},
+ {name: "cm-hold-long", hold: deletionIntentHold(t, 8*time.Second)},
+ }
+
+ var committed []mutationlab.Record
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ records := h.driveDeletionIntentRace(ctx, t, s, tc.name, tc.hold, human, controller)
+ moments := assertDeletionIntentLaws(t, records, tc.name)
+ if tc.name == "cm-hold-short" {
+ committed = moments
+ }
+ })
+ }
+
+ // One object's moments are committed. The other two exercise the same laws under a different
+ // hold, which is a timing property rather than a shape one: committing all three would put the
+ // same six files in the corpus three times and add nothing a reader could not infer.
+ if len(committed) == 0 {
+ t.Fatal("no moments captured for the committed hold")
+ }
+ h.syncCorpus(t, "configmap/deletion-intent-actor", committed)
+}
+
+// driveDeletionIntentRace runs one object through the two-actor removal and returns the records it
+// produced. The phases are kept ordered by waiting for the deletion-pending MODIFIED before the
+// controller writes, so the capture measures the hold rather than a scheduling accident.
+func (h *harness) driveDeletionIntentRace(
+ ctx context.Context,
+ t *testing.T,
+ s scenario,
+ name string,
+ hold time.Duration,
+ human, controller kubernetes.Interface,
+) []mutationlab.Record {
+ t.Helper()
+
+ meta := s.meta(name)
+ meta.Finalizers = []string{finalizerName}
+ cm := &corev1.ConfigMap{ObjectMeta: meta, Data: map[string]string{"key": "value"}}
+ if _, err := human.CoreV1().ConfigMaps(s.ns).Create(ctx, cm, metav1.CreateOptions{}); err != nil {
+ t.Fatalf("create %s as the human: %v", name, err)
+ }
+ h.quiesceAndClear(t, s.id, 3)
+
+ // Phase 1, the HUMAN: the finalizer blocks real removal, so this only stamps deletionTimestamp.
+ if err := human.CoreV1().ConfigMaps(s.ns).Delete(ctx, name, metav1.DeleteOptions{}); err != nil {
+ t.Fatalf("delete %s as the human: %v", name, err)
+ }
+ h.drain(t, s.id, drainSpec{
+ minCount: 1, settle: 0, timeout: 60 * time.Second,
+ until: func(rs []mutationlab.Record) bool { return firstDeletionPendingWatch(rs) != nil },
+ })
+
+ // The hold. This is the whole knob: it decides whether the controller's write shares an audit
+ // batch with the human's delete.
+ time.Sleep(hold)
+
+ // Phase 2, the CONTROLLER: clearing the last finalizer is a `patch`, and it is what makes the
+ // object disappear.
+ patch := []byte(`{"metadata":{"finalizers":null}}`)
+ if _, err := controller.CoreV1().ConfigMaps(s.ns).Patch(
+ ctx, name, types.MergePatchType, patch, metav1.PatchOptions{}); err != nil {
+ t.Fatalf("clear finalizer on %s as the controller: %v", name, err)
+ }
+
+ return h.drain(t, s.id, drainSpec{
+ minCount: 6, settle: 3 * time.Second, timeout: 90 * time.Second,
+ until: func(rs []mutationlab.Record) bool { return firstWatch(rs, "DELETED") != nil },
+ })
+}
+
+// assertDeletionIntentLaws checks what the capture says about the two-actor removal and returns the
+// moments worth committing. Every check here is a claim the design note rests on.
+func assertDeletionIntentLaws(t *testing.T, records []mutationlab.Record, name string) []mutationlab.Record {
+ t.Helper()
+
+ auditDelete := firstOp(records, mutationlab.SourceAudit, "delete")
+ auditPatch := firstOp(records, mutationlab.SourceAudit, "patch")
+ pending := firstDeletionPendingWatch(records)
+ deleted := firstWatch(records, "DELETED")
+ admDelete := firstOp(records, mutationlab.SourceAdmission, "DELETE")
+ admUpdate := firstOp(records, mutationlab.SourceAdmission, "UPDATE")
+ for label, r := range map[string]*mutationlab.Record{
+ "audit delete": auditDelete,
+ "audit patch": auditPatch,
+ "watch MODIFIED (deletion-pending)": pending,
+ "watch DELETED (terminal)": deleted,
+ "admission DELETE": admDelete,
+ "admission UPDATE": admUpdate,
+ } {
+ if r == nil {
+ t.Fatalf("%s: missing required moment: %s", name, label)
+ }
+ }
+
+ // Law 1 — two actors, one object. The delete is the human's; the write that removes the object
+ // is the controller's. Impersonation puts the human in impersonatedUser, which is the field the
+ // product reads in preference to user.
+ if got := auditActor(t, auditDelete); got != deletionIntentHuman {
+ t.Errorf("%s: the delete names %q; want the human %q", name, got, deletionIntentHuman)
+ }
+ wantSA := fmt.Sprintf("system:serviceaccount:%s:%s", labActorNamespace, deletionIntentControllerSA)
+ if got := auditActor(t, auditPatch); got != wantSA {
+ t.Errorf("%s: the finalizer patch names %q; want the controller %q", name, got, wantSA)
+ }
+
+ // Law 2 — the removal verb is a patch. There is exactly one audit `delete`, and it belongs to
+ // phase 1; nothing audits the disappearance itself.
+ if got := countAuditVerb(records, "delete"); got != 1 {
+ t.Errorf("%s: %d audit `delete` events; want exactly 1 (phase 1 only)", name, got)
+ }
+
+ // Law 3 — THE FINDING. Both audit events carry a response body, and both bodies carry the SAME
+ // resourceVersion: the one the deletion stamped. The facts they produce therefore share one
+ // (uid, resourceVersion) key, and the join has no way to prefer the earlier one, because the
+ // later one has taken its place.
+ deleteRV := auditResponseResourceVersion(t, auditDelete)
+ patchRV := auditResponseResourceVersion(t, auditPatch)
+ if deleteRV == "" || patchRV == "" {
+ t.Fatalf("%s: an audit event carried no response resourceVersion (delete=%q patch=%q)",
+ name, deleteRV, patchRV)
+ }
+ if deleteRV != patchRV {
+ t.Errorf("%s: the two audit events carry different resourceVersions (delete=%q patch=%q); "+
+ "the key collision this scenario measures does not hold", name, deleteRV, patchRV)
+ }
+
+ // Law 4 — and that shared key is exactly what the watch event joins on. The deletion-pending
+ // MODIFIED is the observation the deletion-as-intent rule renders as a DELETE, and it carries
+ // the same resourceVersion both facts were filed under.
+ if pending.Key.ResourceVersion != deleteRV {
+ t.Errorf("%s: the deletion-pending watch event carries rv %q, the audit facts %q; "+
+ "the transition event does not join the colliding key", name, pending.Key.ResourceVersion, deleteRV)
+ }
+ if pending.Key.ResourceVersion >= deleted.Key.ResourceVersion {
+ t.Errorf("%s: deletion-pending rv %q should precede terminal DELETED rv %q",
+ name, pending.Key.ResourceVersion, deleted.Key.ResourceVersion)
+ }
+
+ return []mutationlab.Record{*admDelete, *admUpdate, *auditDelete, *auditPatch, *pending, *deleted}
+}
+
+// auditActor returns the actor an audit record names, preferring the impersonated subject over the
+// impersonator — the same preference the product's resolveUserInfo applies when it builds a fact.
+func auditActor(t *testing.T, r *mutationlab.Record) string {
+ t.Helper()
+ var event struct {
+ User struct {
+ Username string `json:"username"`
+ } `json:"user"`
+ ImpersonatedUser *struct {
+ Username string `json:"username"`
+ } `json:"impersonatedUser"`
+ }
+ if err := json.Unmarshal(r.Raw, &event); err != nil {
+ t.Fatalf("decode audit record %s: %v", r.ID, err)
+ }
+ if event.ImpersonatedUser != nil && event.ImpersonatedUser.Username != "" {
+ return event.ImpersonatedUser.Username
+ }
+ return event.User.Username
+}
+
+// auditResponseResourceVersion reads the resourceVersion out of an audit event's response body,
+// which is where the product reads it from too (resourceVersionFromEvent prefers the response
+// object, because objectRef.resourceVersion is the precondition rather than the result).
+func auditResponseResourceVersion(t *testing.T, r *mutationlab.Record) string {
+ t.Helper()
+ var event struct {
+ ResponseObject *struct {
+ Metadata struct {
+ ResourceVersion string `json:"resourceVersion"`
+ } `json:"metadata"`
+ } `json:"responseObject"`
+ }
+ if err := json.Unmarshal(r.Raw, &event); err != nil {
+ t.Fatalf("decode audit record %s: %v", r.ID, err)
+ }
+ if event.ResponseObject == nil {
+ return ""
+ }
+ return event.ResponseObject.Metadata.ResourceVersion
+}
diff --git a/test/mutationlab/e2e/harness_test.go b/test/mutationlab/e2e/harness_test.go
index bb57a68a..d16b224a 100644
--- a/test/mutationlab/e2e/harness_test.go
+++ b/test/mutationlab/e2e/harness_test.go
@@ -19,7 +19,10 @@ import (
"testing"
"time"
+ authnv1 "k8s.io/api/authentication/v1"
corev1 "k8s.io/api/core/v1"
+ rbacv1 "k8s.io/api/rbac/v1"
+ apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes"
@@ -70,6 +73,136 @@ func newHarness(t *testing.T) *harness {
return h
}
+// asActor returns a Kubernetes client that acts as username, through impersonation.
+//
+// Impersonation is how a scenario gets more than one identity out of one kubeconfig, and it is
+// not a shortcut around the thing being measured: the API server records an impersonated request
+// with the impersonator in `user` and the impersonated subject in `impersonatedUser`, and the
+// product reads the second in preference to the first (resolveUserInfo). So a fact published from
+// such an event names the actor this scenario meant, which is the whole point of driving one
+// object with two of them.
+func (h *harness) asActor(t *testing.T, username string, groups ...string) kubernetes.Interface {
+ t.Helper()
+ cfg, err := restConfig()
+ if err != nil {
+ t.Fatalf("kube config for actor %q: %v", username, err)
+ }
+ cfg.Impersonate = rest.ImpersonationConfig{UserName: username, Groups: groups}
+ client, err := kubernetes.NewForConfig(cfg)
+ if err != nil {
+ t.Fatalf("kube client for actor %q: %v", username, err)
+ }
+ return client
+}
+
+// labActorNamespace is where scenario ServiceAccount actors live. It is FIXED rather than the
+// scenario's ephemeral namespace, because a ServiceAccount's username embeds its namespace
+// (system:serviceaccount::) and the normalizer folds namespaces only where they appear as
+// a namespace FIELD, not inside a username string. An actor in the scenario namespace would
+// therefore write a fresh run id into the corpus on every capture. It is also the more faithful
+// arrangement: a cleanup controller runs in its own namespace, not in the tenant's.
+const labActorNamespace = "lab-actors"
+
+// asServiceAccount returns a client authenticated as a ServiceAccount in labActorNamespace, granted
+// ConfigMap writes in grantNS.
+//
+// This is the one actor a scenario cannot fake with impersonation and should not want to: the
+// reported mis-attribution names a real controller identity, and that spelling
+// (system:serviceaccount:…) is what the product's actor-kind check keys on. A token makes the API
+// server write that name into the audit event itself, with no impersonation indirection between the
+// request and the record.
+func (h *harness) asServiceAccount(
+ ctx context.Context,
+ t *testing.T,
+ grantNS, name string,
+) kubernetes.Interface {
+ t.Helper()
+ h.ensureNamespace(ctx, t, labActorNamespace)
+
+ sa := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: labActorNamespace}}
+ if _, err := h.kube.CoreV1().ServiceAccounts(labActorNamespace).Create(
+ ctx, sa, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
+ t.Fatalf("create ServiceAccount %s/%s: %v", labActorNamespace, name, err)
+ }
+ h.allowConfigMapWrites(ctx, t, grantNS, name, rbacv1.Subject{
+ Kind: rbacv1.ServiceAccountKind, Name: name, Namespace: labActorNamespace,
+ })
+
+ // No audiences: an empty request asks for the API server's own default audience, which is the
+ // only one it will accept back. Naming one explicitly (even the usual
+ // https://kubernetes.default.svc) mints a token this cluster authenticates as nobody, and the
+ // scenario then fails with a bare Unauthorized that looks like an RBAC problem and is not one.
+ token, err := h.kube.CoreV1().ServiceAccounts(labActorNamespace).CreateToken(
+ ctx, name, &authnv1.TokenRequest{}, metav1.CreateOptions{})
+ if err != nil {
+ t.Fatalf("request token for %s/%s: %v", labActorNamespace, name, err)
+ }
+
+ cfg, err := restConfig()
+ if err != nil {
+ t.Fatalf("kube config for ServiceAccount %s/%s: %v", labActorNamespace, name, err)
+ }
+ // The token replaces every other credential the kubeconfig carries; leaving a client
+ // certificate in place would authenticate as its subject and silently ignore the token.
+ cfg.BearerToken = token.Status.Token
+ cfg.BearerTokenFile = ""
+ cfg.CertData, cfg.KeyData, cfg.CertFile, cfg.KeyFile = nil, nil, "", ""
+ cfg.Username, cfg.Password = "", ""
+ cfg.Impersonate = rest.ImpersonationConfig{}
+
+ client, err := kubernetes.NewForConfig(cfg)
+ if err != nil {
+ t.Fatalf("kube client for ServiceAccount %s/%s: %v", labActorNamespace, name, err)
+ }
+ return client
+}
+
+// ensureNamespace creates ns if it is absent and leaves it in place. Unlike the scenario namespace
+// it is not torn down: it holds actor identities shared across scenarios, and the driver is serial.
+func (h *harness) ensureNamespace(ctx context.Context, t *testing.T, ns string) {
+ t.Helper()
+ obj := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}}
+ if _, err := h.kube.CoreV1().Namespaces().Create(ctx, obj, metav1.CreateOptions{}); err != nil &&
+ !apierrors.IsAlreadyExists(err) {
+ t.Fatalf("create namespace %s: %v", ns, err)
+ }
+}
+
+// allowConfigMapWrites binds one subject to a namespace Role covering the verbs a scenario actor
+// needs on ConfigMaps. Every non-admin actor needs it, impersonated humans included: impersonation
+// authenticates as somebody else, it does not authorize as them, so an impersonated user with no
+// RoleBinding is refused exactly like any other unprivileged subject.
+//
+// It is deliberately namespace-scoped and ConfigMap-only. The scenario is about who the API server
+// records, and a broader grant would only widen what a mistake in the driver could reach.
+func (h *harness) allowConfigMapWrites(
+ ctx context.Context,
+ t *testing.T,
+ ns, roleName string,
+ subject rbacv1.Subject,
+) {
+ t.Helper()
+ role := &rbacv1.Role{
+ ObjectMeta: metav1.ObjectMeta{Name: roleName, Namespace: ns},
+ Rules: []rbacv1.PolicyRule{{
+ APIGroups: []string{""},
+ Resources: []string{"configmaps"},
+ Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"},
+ }},
+ }
+ if _, err := h.kube.RbacV1().Roles(ns).Create(ctx, role, metav1.CreateOptions{}); err != nil {
+ t.Fatalf("create Role %s/%s: %v", ns, roleName, err)
+ }
+ binding := &rbacv1.RoleBinding{
+ ObjectMeta: metav1.ObjectMeta{Name: roleName, Namespace: ns},
+ Subjects: []rbacv1.Subject{subject},
+ RoleRef: rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "Role", Name: roleName},
+ }
+ if _, err := h.kube.RbacV1().RoleBindings(ns).Create(ctx, binding, metav1.CreateOptions{}); err != nil {
+ t.Fatalf("create RoleBinding %s/%s: %v", ns, roleName, err)
+ }
+}
+
func restConfig() (*rest.Config, error) {
rules := clientcmd.NewDefaultClientConfigLoadingRules()
if kc := os.Getenv("LAB_KUBECONFIG"); kc != "" {