Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/INDEX.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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.

Expand Down
20 changes: 19 additions & 1 deletion docs/UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 17 additions & 8 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
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
Expand Down
4 changes: 2 additions & 2 deletions docs/design/attribution-branch-findings.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -68,7 +68,7 @@ flowchart TD

F -->|neither: name only| H[name tier<br/>namespace + name]
G --> I[exact tier: uid + rv<br/>latest tier: uid]
E --> J[collection_uid if a uid set arrived<br/>collection_scope otherwise]
E --> J[deletecollection_body_uid if a uid set arrived<br/>deletecollection_scope otherwise]

C --> K[Committer-authored]
H --> L[Attributed to the actor]
Expand Down
Loading