DESIGN.md: ground Stage 3 against real source before writing any code
CI / test (push) Successful in 1m34s
CI / test (push) Successful in 1m34s
Three real findings, same discipline as Stages 1/2: - Dead man's switches gained PUT update-in-place in terdut-server v0.33.0 (internal/api/teams.go's handleUpdateTeamDeadman, whose own doc comment names terdut-operator as the reason it was added) -- §5's table still described delete-and-recreate, written before that landed. Also: no unique-name constraint server-side at all, so this resource's idempotent-create step is GET-list-and-match-by-name, not adopt-on-409 the way Team/service-accounts work. - §4.3's username->user_id resolution needs an endpoint: GET /api/users, confirmed open to any authenticated caller (router.go's own "readable by anyone signed in"), so the team-scoped credential already in hand is enough -- no new server-side capability needed here, unlike TEAM-LOOKUP.md's gap. - §5's "a child never needs to chain up to TerdutServer" claim wasn't actually true as written -- a child still needs the server's URL to make any call, and the only way to get one was reading TerdutServer directly. Fixed at the root: TerdutTeam.status now carries serverEndpoint too (resolved once, by TerdutTeam's own controller, same reconcile as teamID/credentialsSecretRef), so the claim holds literally and child controllers need no terdutservers RBAC at all.
This commit is contained in:
@@ -231,6 +231,7 @@ status:
|
||||
conditions: [...]
|
||||
teamID: 42 # the server-side ID; needed by every child object's controller
|
||||
credentialsSecretRef: {name: platform-oncall.platform-team-credentials, key: token} # see §6; this team's own scoped key, always in the OPERATOR's own namespace (not stored here — same reasoning as TerdutServer's, §4.1)
|
||||
serverEndpoint: "http://terdut.platform-oncall.svc:8080" # resolved once, here, from spec.serverRef -- see §5: this is what actually makes "a child never needs to chain up to TerdutServer" true, not just a stated intent. Set alongside teamID/credentialsSecretRef, same reconcile.
|
||||
observedGeneration: 1
|
||||
```
|
||||
|
||||
@@ -266,7 +267,20 @@ One `TerdutEscalationRule` per team — the server itself models a policy as
|
||||
one row (`escalation_policies`) with an owned list of levels, so a
|
||||
one-CRD-to-one-policy mapping (not one-CRD-per-level) matches the server's
|
||||
own aggregate and lets the whole thing be reconciled with the single
|
||||
`PUT /api/teams/{teamID}/escalation` the API actually exposes (see §5).
|
||||
`PUT /api/teams/{teamID}/escalation` the API actually exposes (see §5). Not
|
||||
enforced at admission (no webhooks in v1, §1) — two `TerdutEscalationRule`s
|
||||
naming the same team would both `PUT` it and clobber each other every
|
||||
reconcile; a footgun worth this one sentence, not a technical guard.
|
||||
|
||||
A `username` target is resolved to the `user_id` `PUT /api/teams/{teamID}/escalation`
|
||||
actually requires (confirmed against source: `escalationTargetJSON.UserID
|
||||
*int64`, no username field at all) via `GET /api/users` — confirmed open to
|
||||
any authenticated caller, not gated by team membership or admin
|
||||
(`internal/api/router.go`'s own comment: "readable by anyone signed in"), so
|
||||
the team-scoped credential this controller already holds is enough; no
|
||||
extra RBAC-equivalent server-side needed. Re-resolved every reconcile rather
|
||||
than cached, in case a username is renamed. An unresolvable username is
|
||||
`Ready: False, reason: UnknownUser`, naming which one.
|
||||
|
||||
### 4.4 `TerdutDeadmanSwitch`
|
||||
|
||||
@@ -282,6 +296,12 @@ status:
|
||||
switchID: 7
|
||||
```
|
||||
|
||||
No unique-name constraint server-side (§5's table row, confirmed against
|
||||
source) — the controller's own idempotent-create step is a `GET`-list and
|
||||
name match, not a conflict to recover from. `name` is optional, same as the
|
||||
API: left empty, terdut-server derives it from `matcher`'s own canonical
|
||||
form, and that's what the lookup matches against too.
|
||||
|
||||
### 4.5 `TerdutAlertSource`
|
||||
|
||||
```yaml
|
||||
@@ -356,7 +376,7 @@ resource a full update verb, so reconciliation strategy is per-resource:
|
||||
|---|---|---|
|
||||
| Team | POST create, PUT rename, DELETE, PUT oidc-groups, GET by name | Real update-in-place: diff spec vs. last-applied, PUT the changed pieces. `GET /api/teams?name=` (terdut-server's `TEAM-LOOKUP.md`, landed 2026-10-01) is what makes the idempotent-create general rule below actually true for Team — confirmed by checking: until that endpoint existed, an instance-scoped service account had no way to recover a team's id after a 409, unlike every other resource in this table, where the adopt-on-conflict rule had a real lookup to call. |
|
||||
| Escalation policy | GET/PUT whole-policy | Update-in-place: PUT the full desired policy every reconcile that finds drift; cheap because whole-policy is small and already loaded whole server-side. |
|
||||
| Dead man's switch | POST create, DELETE — **no PUT** | Delete-and-recreate on any spec diff other than `name`. The controller diffs against `status` (which mirrors what was last successfully applied) rather than re-reading the server every reconcile, to avoid a spurious recreate from field reordering. |
|
||||
| Dead man's switch | POST create, **PUT update-in-place** (added in terdut-server v0.33.0), DELETE, GET-list | Real update-in-place, same shape as Team/Escalation: PUT the whole switch every reconcile once its id is known. No unique-name constraint server-side (confirmed against source — `handleCreateTeamDeadman` has no conflict handling at all, unlike Team/service-account creation), so idempotent-create here can't rely on a 409 to adopt from: before POSTing, `GET /api/teams/{teamID}/deadman/switches` and match by `name` first: found → adopt its id; not found → POST. `handleUpdateTeamDeadman`'s own doc comment (terdut-server) confirms the motivation directly: "Added alongside create/delete so an automated caller (terdut-operator) can reconcile a spec change without deleting and recreating the switch, which would otherwise ... needlessly rotate its id for no reason a reconciler's diff should ever manufacture." An earlier draft of this row, written before that endpoint existed, described delete-and-recreate; corrected here, confirmed against source rather than left stale. |
|
||||
| Integration (alert source) | POST create, PATCH rename, DELETE | Rename via PATCH; any other spec change (kind) is delete-and-recreate, which **rotates the webhook key** — called out loudly in the CRD's field docs and in a `Warning` event, since it breaks whatever sends to the old URL/key until the new Secret is picked up. (See the general rule below for what happens if the Secret is lost with *no* spec change.) |
|
||||
|
||||
General rules for every controller:
|
||||
@@ -394,13 +414,17 @@ General rules for every controller:
|
||||
- **Owner chain for status resolution, not API calls**: `TerdutTeam`'s
|
||||
controller does not call any other controller; every child CRD's
|
||||
controller independently resolves its own `teamRef` → `TerdutTeam.status`
|
||||
for both the `teamID` and the `credentialsSecretRef` it needs to call the
|
||||
API (§6) — it never needs to chain further up to `TerdutServer` at all,
|
||||
since the team's own scoped credential is everything a child resource's
|
||||
controller requires. If the referenced `TerdutTeam` isn't `Ready` yet
|
||||
(which includes not having a `credentialsSecretRef` set), the child
|
||||
requeues with backoff and reports `Ready: False, reason: WaitingForTeam`
|
||||
— no cross-controller RPC.
|
||||
for the `teamID`, `credentialsSecretRef`, and `serverEndpoint` it needs to
|
||||
call the API (§6) — it never needs to chain further up to `TerdutServer`
|
||||
at all, not just as a stated intent but literally: `serverEndpoint` is
|
||||
resolved once, by `TerdutTeam`'s own controller, and stored in its status
|
||||
specifically so no child ever needs its own `TerdutServer` RBAC (`get`/
|
||||
`list`/`watch` on `terdutservers`) to find out where to send a request —
|
||||
the team's own scoped credential plus that one status field is everything
|
||||
a child resource's controller requires. If the referenced `TerdutTeam`
|
||||
isn't `Ready` yet (which includes not having a `credentialsSecretRef` set),
|
||||
the child requeues with backoff and reports `Ready: False, reason:
|
||||
WaitingForTeam` — no cross-controller RPC.
|
||||
- **Cross-namespace `serverRef` is re-checked every reconcile, not just at
|
||||
creation**: `TerdutTeam`'s controller reads the target `TerdutServer`'s
|
||||
`spec.allowedTeams` (and, under `Selector`, a `Get` on its own `Namespace`
|
||||
|
||||
Reference in New Issue
Block a user