diff --git a/CLAUDE.md b/CLAUDE.md index 1463497..ab32028 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -453,30 +453,23 @@ Three things about that are easy to get wrong: scales back. `reissueDidsEnroll` does the same dance against the dids daemon and is the template. The mediator and dids daemons are separate Deployments and stay up, so `vta_only` sessions are unaffected; what goes down is this - stack's own VTA and its VTC. Reading costs the same window, which is why - **nothing here stores a copy of the ACL** — `pnm acl list` answers that against - the running VTA for free, and any copy would be stale within minutes anyway. -- **A grant row is an event, not a permission.** The DID a co-admin submits is - the temporary `did:key` from `pnm setup`, and PNM swaps it for a long-lived - one on first connect (`POST /acl/swap`, which preserves role, contexts **and - label**). `vta_admin_grants.did` therefore goes stale by design, and `label` - is the only human-readable field that survives the move — which is why the API - requires one: it is what somebody reads at a `pnm acl list` prompt. Nothing - here tracks where the entry moved to. + stack's own VTA and its VTC. Every successful grant captures `vta acl list` + while the store is already offline; explicit refresh uses another maintenance + window. The database snapshot is dated and never treated as authoritative. +- **The submitted DID rotates.** The DID a co-admin submits is the temporary + `did:key` from `pnm setup`, and PNM swaps it for a long-lived one on first + connect (`POST /acl/swap`, which preserves role, contexts and label). The next + ACL refresh follows that move. Labels are optional, but remain the only + human-readable way to attribute the rotated entry. - **One window at a time.** `runVtaAclJob` holds a process-wide `TryLock` for the - whole window, and the grant route additionally refuses when a live `pending` - row exists (the cross-replica half — the lock is in-process). Two concurrent - grants would scale the same VTA down twice and run two Jobs under one name, - each able to delete the other's. - -**Platform stack only, and `runVtaAclJob` enforces it** — it refuses any session -whose `domain_type` is not `platform`, on top of the routes already resolving -only that session. Everything under it is session-generic by construction (the -table is keyed by `session_id`, the K8s names derive from `session.ID`), so -without that check a per-session route wired to it later would silently hand out -unrestricted super admin on a stack the farm merely operates. Widening the scope -is gated on the approval flow of design §7.4 — the `pending` status and -`requested_by` column exist for it — not on deleting that check. + whole window. `vta_acl_snapshots.maintenance_started_at` is the cross-replica + lock, acquired atomically before anything is scaled and expired after the + maximum Job/restart budget. Two concurrent operations would otherwise scale + the same VTA down twice and run two Jobs under one name. + +The platform route resolves only the platform session; the owner route resolves +through the authenticated user's own `user_id`. The shared Job helper assumes +the caller has already established that authority. Design: `docs/platform-stack-admin-grant-design.md` (§7 is the section to read). diff --git a/docs/platform-stack-admin-grant-design.md b/docs/platform-stack-admin-grant-design.md index bd426da..142aeb5 100644 --- a/docs/platform-stack-admin-grant-design.md +++ b/docs/platform-stack-admin-grant-design.md @@ -34,7 +34,7 @@ Prerequisite reading: [`full-stack-setup-design.md`](full-stack-setup-design.md) | The **platform stack** only — one session, owned by the `platform` system account | every other `full_stack`; every `vta_only` (§10.1) | | Granting `role=admin` with **no contexts** — unrestricted, identical to `pnm-bootstrap` | context-scoped grants, `initiator`/`application`/`reader`, expiry (§10.2) | | Adding an admin | **removing** one — that is `pnm acl delete` against the live VTA (§10.4) | -| A record of **what was added from here** | any copy of the VTA's own admin list (§7.1) | +| A synchronized, dated copy of the VTA ACL | treating that snapshot as authoritative (§7.1) | | Accepting ~60–120s of VTA downtime per operation (§3) | zero-downtime grants (§10.3 records what that costs) | --- @@ -101,40 +101,18 @@ back would cost. ## 4. Data model -Migration `000027_vta_admin_grants`. - -```sql -CREATE TABLE vta_admin_grants ( - id BIGSERIAL PRIMARY KEY, - session_id BIGINT NOT NULL REFERENCES setup_sessions(id) ON DELETE CASCADE, - did TEXT NOT NULL, - label TEXT NOT NULL DEFAULT '', - status TEXT NOT NULL DEFAULT 'pending' - CHECK (status IN ('pending','granted','failed')), - error_msg TEXT NOT NULL DEFAULT '', - requested_by BIGINT NULL REFERENCES admins(id) ON DELETE SET NULL, - granted_at TIMESTAMPTZ NULL, - created_at TIMESTAMPTZ NOT NULL DEFAULT now(), - updated_at TIMESTAMPTZ NOT NULL DEFAULT now() -); - --- One live grant per DID per session. Partial, so a failed attempt stays as --- history and the same DID can be retried without deleting the first record. -CREATE UNIQUE INDEX vta_admin_grants_live_unique - ON vta_admin_grants (session_id, did) - WHERE status IN ('pending','granted'); -``` - -Named for the session, not for the platform stack, because the mechanism is -session-generic and only the *route* is narrowed (§1). Generalising later is a -route addition, not a migration. +Migration `000031_vta_acl_snapshots` stores the last complete `vta acl list` +result in `vta_acl_snapshots` and `vta_acl_entries`. The snapshot timestamp is +shown in the UI so cached data is never confused with the VTA's current state. -`ON DELETE CASCADE`: the grants describe a store that is deleted with the -session. There is nothing to orphan. +`vta_acl_snapshots.maintenance_started_at` is also the cross-replica lock for +all offline ACL operations. An atomic conditional upsert acquires it, and a +15-minute stale threshold prevents an interrupted request from wedging future +maintenance indefinitely. -**No `role` or `contexts` column.** Every row is an unrestricted admin — that is -the feature. Adding a column that only ever holds one value invites a second -value without the authorization work §7.4 would need. +Migration `000032_drop_vta_admin_grants` removes the old grant-event table. Its +submitted DIDs became stale after PNM rotation, it was no longer displayed, and +its locking responsibility is now covered by the snapshot row. --- @@ -144,22 +122,18 @@ All admin-cookie only, all under the existing `/api/v1/admin` group. | Method | Path | Notes | | --- | --- | --- | -| `GET` | `/api/v1/admin/platform-stack/admins` | The grant rows — what was added from here. Never blocks, never causes downtime. | -| `POST` | `/api/v1/admin/platform-stack/admins` | `{did, label, confirm}` → grants. Synchronous; 60–120s. 409 while another grant holds the window (§7.6). | - -`confirm` must equal the platform stack's label, mirroring the guard on -`DELETE /admin/setup-sessions/:id` and enforced at the API, not the UI. It is -the speed bump on an irreversible privilege grant that also takes production -down for a minute — see §7.4 for why a speed bump and not a second approver. +| `GET` | `/api/v1/admin/platform-stack/admins` | Last complete ACL snapshot. Never blocks or causes downtime. | +| `POST` | `/api/v1/admin/platform-stack/admins` | `{did, label?}` → grants and refreshes the snapshot. | +| `POST` | `/api/v1/admin/platform-stack/admins/refresh` | Runs `vta acl list`, synchronizes the snapshot, and restarts the VTA. | Validation on `did`: must start with `did:`, must be `did:key:` (the VTA's DI proof verifier is `did:key`-only, so anything else produces an ACL entry that -can never authenticate), and must not already hold a live grant (409). +can never authenticate). The offline Job asks the VTA whether it is already +present and reports that as a successful no-op. -Synchronous, following `reissueDidsEnroll`. The row is written `pending` -**before** the k8s work starts, so a client that times out at an ingress proxy -has not lost the operation — `GET` still shows it, and it lands `granted` or -`failed` regardless of who is listening. +The operation is synchronous, following `reissueDidsEnroll`. A client timeout +does not cancel the deferred VTA restart, but the client must read or refresh the +ACL snapshot to determine the final state. --- @@ -168,7 +142,7 @@ has not lost the operation — `GET` still shows it, and it lands `granted` or One helper, `runVtaAclJob(ctx, session, cmd)`. ``` -0. refuse unless domain_type = platform (§1); TryLock or 409 (§7.6) +0. caller resolves and authorizes the session; acquire both locks or return 409 (§7.6) 1. resolve ns, deployment name (k8s.FSVtaName), selector "app=fs-vta,session-id=" 2. ScaleComponentDeployment(vta, 0) 3. defer: ScaleComponentDeployment(vta, 1) + WaitForComponentDeploymentReady @@ -196,13 +170,12 @@ fi ``` `VTAFARM_ALREADY_PRESENT` is not an error — it is the idempotent outcome, and -the row still lands `granted`. +the response returns `already_present: true`. A condition's exit status does not trigger `set -e`, so the probe stays a test -rather than a failure. `set -e` itself is now defensive rather than load-bearing -— the import is the last command, so its status is the script's — and it stays in -front of whoever appends a line next, since a trailing command would otherwise -mask a failed import and have the API report a grant that never happened. +rather than a failure. `set -e` ensures a failed import stops before the +trailing ACL list and end marker can mask it and make the API report a grant +that never happened. --- @@ -227,34 +200,29 @@ and parked in their keyring. On their first authenticated command PNM rotates: `POST /acl/swap` atomically moves the entry — same role, same contexts — onto a fresh long-lived DID and deletes the temp (`vta-sdk/src/session.rs:1107`). -So minutes after a successful grant, `vta_admin_grants.did` names a DID that is -no longer in the ACL, while the co-admin holds full super admin under a DID this -farm has never seen. +Minutes after a successful grant, the submitted DID may no longer be in the ACL, +while the co-admin holds full super admin under its rotated replacement. This is not a bug to fix, it is the protocol working. What follows from it: -- A grant row is **a record of an event**, not a statement of current access. - The UI must label it that way (`granted `) and must not present it as - the admin list. -- The **label is required** for exactly this reason. `POST /acl/swap` carries +- The ACL snapshot is a dated cache, not a statement of current access. The UI + displays its synchronization time and offers an explicit refresh. +- An optional label remains useful because `POST /acl/swap` carries role, contexts and label onto the new entry (`vta-service/src/operations/acl.rs`, `with_label(old.label.clone())`), and of - those the label is the only human-readable one. Grant without it and the ACL - holds a did:key nobody can attribute to a person. + those the label is the only human-readable one. - The next explicit ACL refresh follows the DID to where it moved. Until then, the displayed snapshot remains clearly dated. - **This is why removal is not built here.** Deleting a granted DID after a rotation would remove nothing; a removal that works must target a DID from the - freshly synchronized VTA ACL, not the original grant row. + freshly synchronized VTA ACL, not the originally submitted DID. It is the strongest argument for keeping removal out of scope (§10.4). Attributing a rotated entry to a person is not something this side can do: `vta acl list` prints DID / role / label / contexts / created, so the only - handle is the **label** — which the swap does preserve - (`with_label(old.label.clone())`), and which is why the API requires one. An - operator at a `pnm` prompt reads that label with the whole ACL in front of them - and decides. A form cannot do better, and would take a maintenance window to do - worse. + handle is the optional label, which the swap preserves + (`with_label(old.label.clone())`). Without one, the full DID remains visible + but cannot be attributed to a person by this service. ### 7.3 Nothing stops the last admin being removed @@ -282,19 +250,14 @@ effect. It is accepted here because the platform stack is the farm's own stack, run by the same operators, on a cluster where those operators already have PVC access — the authority exists whether or not there is a button for it. -Two things make it accountable rather than silent: - -- `requested_by` records which admin, and the row is permanent. -- `confirm` (§5) means it cannot be a stray click or a CSRF-shaped accident. - A second-approver flow (`pending` → approved by someone already holding a VTA credential) was considered and deferred. It remains the right shape for any future **admin-cookie** route that can reach a customer's stack, where the argument above does not hold. The owner-facing `/setup/{id}/admins` route added later is different: it resolves the session through the authenticated user's -own `user_id`, so it cannot grant on somebody else's VTA. The `pending` status -and `requested_by` column still leave room for an approval transition if that -broader admin route is ever added. +own `user_id`, so it cannot grant on somebody else's VTA. If durable per-actor +grant auditing or approval is required later, it should use a purpose-built +immutable audit/approval model rather than stale ACL identifiers. ### 7.5 A failed scale-back leaves the stack down @@ -324,25 +287,22 @@ Two guards, because one does not cover it: `TryLock`, not `Lock`: a caller who queued would sit through one outage and then start another, and a queue of these is a queue of outages. Refusing with 409 says the true thing — nothing is broken, come back in a minute. -- The grant route additionally refuses while a live `pending` row exists for the - session. The lock is in-process and cannot see a second API replica; the row is - written before any Kubernetes work and can. Bounded by `aclJobStale` (15 min, - past the Job's own `ActiveDeadlineSeconds`) so a request that died mid-window - cannot wedge the route permanently. +- `runVtaAclJob` also acquires `vta_acl_snapshots.maintenance_started_at` with an + atomic conditional upsert. That lock is visible across API replicas and is + bounded by `aclJobStale` (15 minutes, past the Job and restart budget), so a + request that dies mid-window cannot wedge the route permanently. -Both sit inside `runVtaAclJob` and the grant handler rather than in middleware, -so nothing can reach the window by another path. +Both sit inside `runVtaAclJob` rather than in middleware, so grant and refresh +cannot bypass them through another route. ## 8. Frontend `src/pages/admin/PlatformStackView.tsx`, one new section below the existing stack detail. Client methods in `src/lib/api.ts` alongside `getPlatformStack`. -- **Add admin** — the primary action, and the reason the page exists. A - `did:key` field, a **required** label ("who is this?"), and a confirm input - taking the stack label. The copy has to state three things plainly: the grant - is **unrestricted super admin**, the VTA will be **down for about a minute**, - and the DID being pasted is expected to change once its holder connects. +- **Add admin** — a `did:key` field and optional label. No typed confirmation; + the authenticated admin action submits directly and temporarily stops and + restarts the VTA. Worth spelling out the flow it sits in, because a co-admin doing this for the first time will otherwise stop halfway: run `pnm setup --name ` locally, @@ -352,17 +312,12 @@ stack detail. Client methods in `src/lib/api.ts` alongside `getPlatformStack`. A 409 while another grant is running is not an error state — say "another admin is updating the ACL, try again in a minute" and keep the form filled in. -- **Added from here** — the `vta_admin_grants` rows, labelled as events (§7.2). - Empty on a new stack, and it says why: the first administrator was set during - provisioning and was never a row here. - - This is the only list on the page, so it also carries the pointer to the real - one: `pnm acl list` for the VTA's actual administrators, `pnm acl delete ` - to remove one. Both belong next to the rows they qualify, not in a footnote. +- **Live ACL** — the dated database snapshot from `vta acl list`, with the same + explicit refresh action and full-DID presentation as the owner portal. ## 9. Phasing -1. ~~Migration + model + `runVtaAclJob`, with `GET`.~~ **Done.** +1. ~~Snapshot migration + `runVtaAclJob`, with `GET`.~~ **Done.** 2. ~~`POST` (grant), with the concurrency guards of §7.6.~~ **Done.** Revoke was cut — see §10.4. 3. ~~Frontend section.~~ **Done** — `src/pages/admin/PlatformStackAdmins.tsx`, @@ -370,7 +325,7 @@ stack detail. Client methods in `src/lib/api.ts` alongside `getPlatformStack`. The owner portal additionally exposes a dated ACL snapshot and an explicit refresh action. Refresh uses the same offline Job and therefore carries the -same one-minute maintenance window as a grant. +same maintenance window as a grant. The shared `runVtaAclJob` machinery now also backs the owner-only `POST /setup/{id}/admins` route. That route performs its own ownership and @@ -405,12 +360,10 @@ it with no downtime and no code here. Building it into the API would mean answering "which of these entries is the person I want to remove" — and after a rotation the only handle is a label somebody typed (§7.2). An operator at a `pnm` prompt has the full ACL in front of them and can decide; a form cannot. The -schema keeps no `revoked` status, so this is an addition rather than a -resurrection if it is ever wanted. - -**10.5 Keeping any copy of the VTA's ACL.** Dropped along with the snapshot it -was built for — §7.1 has the reasoning. A corollary: the once-planned fourth -phase, capturing the ACL during `fsStepImportAdminDid`, is dropped too. It would -have put a parse of `vta acl list` in the provisioning critical path, where a -changed output format or a renamed flag fails the Job, fails the step and fails -the whole stack build — for a display nobody needs. +service keeps no local permission ledger, so this would be a new operation if it +is ever wanted. + +**10.5 Treating the ACL snapshot as live state.** The database stores only the +last complete explicit synchronization. It is intentionally not refreshed from +the provisioning critical path or on ordinary page loads, both to avoid extra +downtime and to keep output parsing failures from blocking stack creation. diff --git a/go.mod b/go.mod index f534b28..8599acb 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,6 @@ require ( github.com/golang-jwt/jwt/v5 v5.3.1 github.com/golang-migrate/migrate/v4 v4.19.1 github.com/google/uuid v1.6.0 - github.com/jackc/pgx/v5 v5.9.2 github.com/joho/godotenv v1.5.1 golang.org/x/net v0.55.0 golang.org/x/sync v0.20.0 @@ -59,6 +58,7 @@ require ( github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect + github.com/jackc/pgx/v5 v5.9.2 // indirect github.com/jackc/puddle/v2 v2.2.2 // indirect github.com/jinzhu/inflection v1.0.0 // indirect github.com/jinzhu/now v1.1.5 // indirect diff --git a/internal/apidocs/openapi.yaml b/internal/apidocs/openapi.yaml index 3e95775..0318b80 100644 --- a/internal/apidocs/openapi.yaml +++ b/internal/apidocs/openapi.yaml @@ -2351,54 +2351,21 @@ paths: /api/v1/admin/platform-stack/admins: get: - summary: Co-admins on the platform stack's VTA + summary: Read the platform VTA's synchronized ACL description: | - **What was added from here** — a history of events, not the VTA's - current admin list. Nothing here keeps a copy of that list. - - The two differ in both directions and it matters: - - - A `granted` DID usually is **not** in the VTA's ACL any more. The DID - a co-admin submits is the temporary `did:key` `pnm setup` minted, and - PNM swaps it for a fresh long-lived one on first connect - (`POST /acl/swap` moves the entry, carrying role, contexts and label, - then deletes the temporary). So minutes after a successful grant this - DID names something the ACL no longer holds, while its holder has full - super admin under a DID this API has never seen. That is the protocol - working, not a failure. - - Admins added out of band — the `pnm-bootstrap` entry from - provisioning, or anything an operator added with `pnm acl create` — - never appear here at all. - - For who can act on the VTA right now, run `pnm acl list` against it. - - Free — serves stored state, never touches the cluster. + Returns the last complete `vta acl list` snapshot stored for the + platform stack. Reading the snapshot does not stop the VTA. Before the + first refresh, `synced_at` is null and `entries` is empty. tags: [Admin] security: - CookieAuthAdmin: [] responses: "200": - description: What was added from here + description: Last synchronized ACL snapshot content: application/json: schema: - type: object - properties: - id: { type: string } - label: { type: string } - grants: - type: array - items: - type: object - properties: - did: { type: string } - label: { type: string } - status: - type: string - enum: [pending, granted, failed] - error_msg: { type: string } - granted_at: { type: string, format: date-time, nullable: true } - created_at: { type: string, format: date-time } + $ref: "#/components/schemas/SessionAclSnapshot" "404": description: No platform stack exists content: @@ -2427,11 +2394,8 @@ paths: read the vault, mint keys, and **remove any other admin including the one who granted them** — the VTA has no last-admin protection. - **This stops the VTA for roughly a minute** (see the refresh route). - Synchronous, so the request blocks for the whole window. The grant row - is written `pending` *before* the Kubernetes work starts, so a client - that times out at a proxy has not lost the operation — it still lands - `granted` or `failed`, and `GET .../admins` shows the outcome. + This temporarily stops and restarts the VTA. The request is synchronous + and refreshes the stored ACL snapshot before returning. `did` must be a `did:key` — the form `pnm setup` mints, and the only one the VTA's REST login can verify. The expected flow: the co-admin runs @@ -2440,7 +2404,7 @@ paths: Expect that DID to **stop being their DID** on their first connect — PNM rotates onto a fresh long-lived one and the ACL entry moves with it. - That is normal; see `GET .../admins`. + That is normal; the next ACL refresh shows the replacement DID. `already_present: true` means the DID was already in the ACL and nothing changed. Not an error — reported so a UI can say so rather than imply it @@ -2463,7 +2427,7 @@ paths: application/json: schema: type: object - required: [did, label, confirm] + required: [did] properties: did: type: string @@ -2472,14 +2436,7 @@ paths: label: type: string maxLength: 64 - description: | - Who this key belongs to. **Required** — PNM rotates the DID - away on first connect and the label is the only - human-readable field that survives the move, so an entry - granted without one cannot be attributed to anyone later. - confirm: - type: string - description: Must equal the platform stack's label. + description: Optional human-readable name that survives PNM key rotation. responses: "200": description: Granted @@ -2493,18 +2450,15 @@ paths: already_present: { type: boolean } warning: type: string - description: Present only when the VTA failed to restart. The stack is down. + description: Present when snapshot synchronization or VTA restart needs attention. "400": - description: Bad `did`, oversized label, or missing/non-matching `confirm` + description: Bad `did` or invalid label content: application/json: schema: $ref: "#/components/schemas/Error" "409": - description: | - Either this DID already has a pending or granted entry, or another - admin's grant is mid-window. Both are retryable; neither means - damage. + description: Another ACL operation is in progress content: application/json: schema: @@ -2516,11 +2470,40 @@ paths: schema: $ref: "#/components/schemas/Error" "502": - description: The Kubernetes work failed; the grant row is marked `failed` + description: ACL maintenance Job or VTA restart failed + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "401": + description: Missing or invalid token + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "403": + description: Insufficient role content: application/json: schema: $ref: "#/components/schemas/Error" + + /api/v1/admin/platform-stack/admins/refresh: + post: + summary: Refresh the platform VTA's ACL snapshot + description: | + Stops the platform VTA, runs `vta acl list` against its local store, + atomically replaces the database snapshot, then restarts the VTA. + tags: [Admin] + security: + - CookieAuthAdmin: [] + responses: + "200": + description: ACL synchronized and the VTA is ready again + content: + application/json: + schema: + $ref: "#/components/schemas/SessionAclSnapshot" "401": description: Missing or invalid token content: @@ -2533,6 +2516,24 @@ paths: application/json: schema: $ref: "#/components/schemas/Error" + "404": + description: No platform stack exists + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "409": + description: Platform stack is not running or another ACL operation is in progress + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "502": + description: ACL maintenance Job, output parsing, or VTA restart failed + content: + application/json: + schema: + $ref: "#/components/schemas/Error" /api/v1/admin/setup/images: get: @@ -5287,7 +5288,7 @@ paths: schema: $ref: "#/components/schemas/Error" "409": - description: Session is not running, the DID already has a live grant, or another grant is in progress + description: Session is not running or another ACL operation is in progress content: application/json: schema: diff --git a/internal/handler/admin_platform_stack_admins.go b/internal/handler/admin_platform_stack_admins.go index 0024e8f..2699d36 100644 --- a/internal/handler/admin_platform_stack_admins.go +++ b/internal/handler/admin_platform_stack_admins.go @@ -46,11 +46,10 @@ const ( aclRestartTimeout = 3 * time.Minute aclReadyTimeout = 2 * time.Minute - // aclJobStale bounds how long a `pending` row can block the next attempt. - // Matches ComponentJobSpec's default ActiveDeadlineSeconds (600s) plus the - // restart budget: past that, no Job of ours can still be running, so a row - // still sitting at `pending` belongs to a request that died without - // finishing — most likely a replica that was killed mid-window. + // aclJobStale bounds how long a database maintenance lock can block the next + // attempt. It matches ComponentJobSpec's default ActiveDeadlineSeconds + // (600s) plus the restart budget: past that, no Job of ours can still be + // running, so a held lock belongs to a request that died without releasing it. aclJobStale = 15 * time.Minute ) @@ -122,58 +121,32 @@ func (h *SetupHandler) platformSession(c *gin.Context) *model.SetupSession { } // ListPlatformStackAdmins — GET /api/v1/admin/platform-stack/admins. -// -// Serves stored state only: never stops the VTA, never blocks. -// -// This is a history of what was added from here, **not** the VTA's current -// admin list — and the two genuinely differ. A granted DID stops being the -// holder's DID on their first connect (PNM rotates and `POST /acl/swap` moves -// the entry), and admins added out of band never appear here at all. For who -// can act on the VTA right now, `pnm acl list`. +// Serves the last complete `vta acl list` snapshot without causing downtime. func (h *SetupHandler) ListPlatformStackAdmins(c *gin.Context) { session := h.platformSession(c) if session == nil { return } - var grants []model.VtaAdminGrant - if err := h.db.Where("session_id = ?", session.ID). - Order("created_at DESC").Find(&grants).Error; err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to read grants"}) + response, err := h.sessionAclSnapshot(session.ID) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to read the VTA ACL snapshot"}) return } - if grants == nil { - grants = []model.VtaAdminGrant{} - } - - c.JSON(http.StatusOK, gin.H{ - "id": session.VtaName, - "label": session.VtaName, - "grants": grants, - }) + c.JSON(http.StatusOK, response) } -// requireStackConfirm gates the one route that takes the VTA down, mirroring -// the guard on DELETE /admin/setup-sessions/:id: the caller must name the stack. -// -// Enforced here rather than in the UI. Adding an admin is both an irreversible -// privilege grant and a minute of downtime on the flagship stack — neither is -// something a stray click should be able to cause. -// -// Takes the already-bound value rather than reading the body itself, because -// gin's ShouldBindJSON consumes it: the grant route carries `did` and `label` -// alongside `confirm` and has to bind all three in one pass. A missing or -// malformed body leaves Confirm at "", which fails here exactly as a wrong -// value does — both mean "not confirmed". -func requireStackConfirm(c *gin.Context, session *model.SetupSession, confirm string) bool { - if confirm != session.VtaName { - c.JSON(http.StatusBadRequest, gin.H{ - "error": "this stops the platform stack's VTA for about a minute — " + - `send {"confirm": "` + session.VtaName + `"} to proceed`, - }) - return false +// RefreshPlatformStackAdmins — POST /api/v1/admin/platform-stack/admins/refresh. +func (h *SetupHandler) RefreshPlatformStackAdmins(c *gin.Context) { + session := h.platformSession(c) + if session == nil { + return } - return true + if session.Status != "running" { + c.JSON(http.StatusConflict, gin.H{"error": "platform stack must be in running status"}) + return + } + h.refreshVtaAclSnapshot(c, session) } // aclRestartError marks a failure of the deferred scale-back-up, so callers can diff --git a/internal/handler/admin_platform_stack_grant.go b/internal/handler/admin_platform_stack_grant.go index 85eddf9..13c360c 100644 --- a/internal/handler/admin_platform_stack_grant.go +++ b/internal/handler/admin_platform_stack_grant.go @@ -1,19 +1,13 @@ package handler import ( - "errors" - "fmt" "log" "net/http" "regexp" "strings" - "time" "github.com/gin-gonic/gin" - "github.com/jackc/pgx/v5/pgconn" - "gorm.io/gorm" - "github.com/ic3software/vtafarm-api/internal/middleware" "github.com/ic3software/vtafarm-api/internal/model" ) @@ -46,9 +40,8 @@ const alreadyPresentMarker = "VTAFARM_ALREADY_PRESENT" // Adds `did` to the platform stack's VTA ACL as an **unrestricted admin** — the // same authority step_import_admin_did gave the stack's first admin (§2). // -// Synchronous and slow (60–120s). The grant row is written `pending` before any -// Kubernetes work starts, so a client that times out at a proxy has not lost -// the operation: it still lands `granted` or `failed`, and GET shows it. +// Synchronous and slow (60–120s). The VTA ACL is authoritative; a successful +// operation also synchronizes the database snapshot returned by GET. func (h *SetupHandler) GrantPlatformStackAdmin(c *gin.Context) { session := h.platformSession(c) if session == nil { @@ -56,14 +49,10 @@ func (h *SetupHandler) GrantPlatformStackAdmin(c *gin.Context) { } var body struct { - Did string `json:"did"` - Label string `json:"label"` - Confirm string `json:"confirm"` + Did string `json:"did"` + Label string `json:"label"` } _ = c.ShouldBindJSON(&body) - if !requireStackConfirm(c, session, body.Confirm) { - return - } did := strings.TrimSpace(body.Did) if !didKeyRe.MatchString(did) { @@ -72,23 +61,7 @@ func (h *SetupHandler) GrantPlatformStackAdmin(c *gin.Context) { }) return } - // Required, not decorative. The DID in this request stops being the - // holder's DID on their first connect — `POST /acl/swap` moves the entry - // onto a freshly minted one — and of everything on that entry the label is - // the only human-readable field that survives the move - // (vta-service/src/operations/acl.rs `with_label(old.label.clone())`). - // - // Grant without one and the ACL ends up holding an unidentifiable did:key — - // and since removal happens at a `pnm acl list` prompt, that label is what - // the person deciding is reading. Cheaper to insist here. label := strings.TrimSpace(body.Label) - if label == "" { - c.JSON(http.StatusBadRequest, gin.H{ - "error": "label is required — it is the only identifier that survives PNM's key rotation, " + - "and without it this entry cannot be attributed to anyone later", - }) - return - } if len(label) > 64 { c.JSON(http.StatusBadRequest, gin.H{"error": "label must be 64 characters or fewer"}) return @@ -98,104 +71,26 @@ func (h *SetupHandler) GrantPlatformStackAdmin(c *gin.Context) { return } - h.grantVtaAdmin(c, session, did, label, callingAdminID(c), "platform admin") + h.grantVtaAdmin(c, session, did, label, "platform admin") } // grantVtaAdmin performs the shared, session-scoped grant operation after the // caller-specific route has established authority and validated its request. -// The platform route records an admins.id; the user route passes nil because -// ownership is already fixed by setup_sessions.user_id. func (h *SetupHandler) grantVtaAdmin( c *gin.Context, session *model.SetupSession, did, label string, - requestedBy *uint, actor string, ) { - // A request can disappear with its API replica after writing `pending` but - // before recording a terminal state. Past the Job + restart budget nothing - // from that request can still be running, so retire it before the live-grant - // and cross-replica concurrency checks below. This also releases the partial - // unique index that makes one pending row per session an atomic lock. - staleBefore := time.Now().Add(-aclJobStale) - if err := h.db.Model(&model.VtaAdminGrant{}). - Where("session_id = ? AND status = ? AND created_at <= ?", session.ID, model.GrantPending, staleBefore). - Updates(map[string]any{ - "status": model.GrantFailed, - "error_msg": "grant expired before completion", - "updated_at": time.Now(), - }).Error; err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to expire stale grants"}) - return - } - - // Not idempotent by design: a second grant of a live DID is a mistake worth - // surfacing, not a no-op worth hiding, because it costs a window either - // way. The partial unique index is the real gate; this is the readable - // error in front of it. - var existing model.VtaAdminGrant - err := h.db.Where("session_id = ? AND did = ? AND status IN ?", - session.ID, did, []string{model.GrantPending, model.GrantGranted}).First(&existing).Error - if err == nil { - c.JSON(http.StatusConflict, gin.H{ - "error": fmt.Sprintf("this DID already has a %s grant on this VTA", existing.Status), - }) - return - } - if !errors.Is(err, gorm.ErrRecordNotFound) { - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to check existing grants"}) - return - } - - // The cross-replica half of the mutual exclusion in runVtaAclJob. That lock - // is in-process, so it only serialises callers hitting the same API pod. The - // read gives a useful error in the common case; the database's one-pending- - // per-session partial unique index closes the two-replicas-read-zero race. - // - // Bounded by aclJobStale so a request that died mid-window cannot wedge the - // route permanently — past that deadline no Job of ours can still be alive. - var inFlight int64 - if countErr := h.db.Model(&model.VtaAdminGrant{}). - Where("session_id = ? AND status = ? AND created_at > ?", - session.ID, model.GrantPending, staleBefore). - Count(&inFlight).Error; countErr != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to check for an in-flight grant"}) - return - } - if inFlight > 0 { - c.JSON(http.StatusConflict, gin.H{"error": errAclJobBusy.Error()}) - return - } - - grant := model.VtaAdminGrant{ - SessionID: session.ID, - Did: did, - Label: label, - Status: model.GrantPending, - RequestedBy: requestedBy, - } - if createErr := h.db.Create(&grant).Error; createErr != nil { - // Lost the cross-replica race to one of the two partial unique indexes - // (same DID, or any pending grant on the session). - var pgErr *pgconn.PgError - if errors.As(createErr, &pgErr) && pgErr.Code == "23505" { - c.JSON(http.StatusConflict, gin.H{"error": errAclJobBusy.Error()}) - return - } - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create admin grant"}) - return - } log.Printf("[vta-admins] granting super admin on session %d to %s (requested by %s)", session.ID, did, actor) logs, restartErr, runErr := h.runVtaAclJob(c.Request.Context(), session, grantCmd(did, label)) if runErr != nil { - h.markGrant(&grant, model.GrantFailed, runErr.Error()) respondAclJobError(c, session, runErr, restartErr) return } - h.markGrant(&grant, model.GrantGranted, "") warnings := make([]string, 0, 2) if entries, parseErr := parseVtaAclList(logs); parseErr != nil { warnings = append(warnings, "The PNM was linked, but the ACL snapshot could not be parsed. Use Refresh live ACL to retry.") @@ -206,7 +101,7 @@ func (h *SetupHandler) grantVtaAdmin( resp := gin.H{ "did": did, - "status": grant.Status, + "status": "granted", // The caller asked for this DID to hold super admin; it already did. // Reported rather than swallowed so a UI can say "already an admin" // instead of implying it just changed something. @@ -226,10 +121,8 @@ func (h *SetupHandler) grantVtaAdmin( // status does not trigger `set -e`, so the probe stays a test rather than a // failure. // -// `set -e` is defensive rather than load-bearing now that the import is the last -// thing to run: with a trailing command it would be the difference between -// reporting a grant and reporting the truth, so it stays in front of the next -// person who appends a line. +// `set -e` ensures a failed import stops before the trailing ACL list and marker +// can make the shell script appear successful. func grantCmd(did, label string) string { importCmd := "vta import-did --role admin --did " + shellQuote(did) if label != "" { @@ -245,34 +138,3 @@ func grantCmd(did, label string) string { "vta acl list 2>&1\n" + "echo " + aclListEndMarker + "\n" } - -// markGrant moves a grant row to its terminal state, keeping the in-memory copy -// in step so the caller can report from it. -func (h *SetupHandler) markGrant(grant *model.VtaAdminGrant, status, errMsg string) { - now := time.Now() - updates := map[string]any{"status": status, "error_msg": errMsg, "updated_at": now} - if status == model.GrantGranted { - updates["granted_at"] = now - grant.GrantedAt = &now - } - if err := h.db.Model(&model.VtaAdminGrant{}).Where("id = ?", grant.ID).Updates(updates).Error; err != nil { - log.Printf("[vta-admins] error: failed to mark grant %d as %s: %v", grant.ID, status, err) - } - grant.Status = status - grant.ErrorMsg = errMsg -} - -// callingAdminID reads the admin's id from the request. On an admin-cookie -// route the JWT's UserID claim is an admins.id — admins are their own table -// (see handler/admin_enroll.go, which mints the token from admin.ID). -func callingAdminID(c *gin.Context) *uint { - v, ok := c.Get(middleware.ContextUserID) - if !ok { - return nil - } - id, ok := v.(uint) - if !ok { - return nil - } - return &id -} diff --git a/internal/handler/admin_platform_stack_grant_test.go b/internal/handler/admin_platform_stack_grant_test.go index 5ba3ee3..b61ccaa 100644 --- a/internal/handler/admin_platform_stack_grant_test.go +++ b/internal/handler/admin_platform_stack_grant_test.go @@ -146,10 +146,8 @@ func TestSortVtaAclEntriesNewestFirst(t *testing.T) { } } -// The label is what identifies the entry after PNM rotates the DID away, so it -// has to reach the ACL. The handler rejects an empty one; grantCmd still omits -// the flag rather than passing an empty string, for any caller that gets there -// another way. +// A label identifies the entry after PNM rotates the DID away. It is optional, +// so an empty value must omit the flag rather than pass an empty string. func TestGrantCmdCarriesTheLabel(t *testing.T) { withLabel := grantCmd("did:key:z6MkTest", "alice") if !strings.Contains(withLabel, "--label 'alice'") { diff --git a/internal/handler/setup_acl.go b/internal/handler/setup_acl.go index f451708..127ca47 100644 --- a/internal/handler/setup_acl.go +++ b/internal/handler/setup_acl.go @@ -56,6 +56,10 @@ func (h *SetupHandler) RefreshSessionAdmins(c *gin.Context) { return } + h.refreshVtaAclSnapshot(c, session) +} + +func (h *SetupHandler) refreshVtaAclSnapshot(c *gin.Context, session *model.SetupSession) { logs, restartErr, runErr := h.runVtaAclJob(c.Request.Context(), session, aclListCmd()) if runErr != nil { respondAclJobError(c, session, runErr, restartErr) diff --git a/internal/handler/setup_admins.go b/internal/handler/setup_admins.go index 771b85f..b549552 100644 --- a/internal/handler/setup_admins.go +++ b/internal/handler/setup_admins.go @@ -15,9 +15,8 @@ import ( // platform stack. // // The operation is synchronous and briefly stops the VTA because its ACL store -// is held under an exclusive lock by the running process. A pending grant row -// is written before that maintenance window, so a disconnected client does not -// lose the operation and a second API replica can reject overlapping work. +// is held under an exclusive lock by the running process. The shared snapshot +// maintenance lock prevents overlapping work across API replicas. func (h *SetupHandler) GrantSessionAdmin(c *gin.Context) { session := h.userSession(c) if session == nil { @@ -44,7 +43,7 @@ func (h *SetupHandler) GrantSessionAdmin(c *gin.Context) { return } - h.grantVtaAdmin(c, session, did, additionalPnmLabel(did), nil, "session owner") + h.grantVtaAdmin(c, session, did, additionalPnmLabel(did), "session owner") } // PNM rotates away from the submitted DID on first connect, so the ACL entry diff --git a/internal/model/vta_admin_grant.go b/internal/model/vta_admin_grant.go deleted file mode 100644 index 191a198..0000000 --- a/internal/model/vta_admin_grant.go +++ /dev/null @@ -1,53 +0,0 @@ -package model - -import "time" - -// The lifecycle of one grant. Written pending before any Kubernetes work -// starts, so a client that times out during the maintenance window has not -// lost the operation — the row is the record, the HTTP response is not. A live -// pending row is also how a second API replica knows to refuse a concurrent -// grant, which the in-process lock cannot see across pods. -// -// There is deliberately no revoked state: removing an admin is `pnm acl delete` -// against the live VTA, not something this side does. -const ( - GrantPending = "pending" - GrantGranted = "granted" - GrantFailed = "failed" -) - -// VtaAdminGrant is one attempt by this farm to put a DID in a stack's VTA ACL -// as an unrestricted admin — the same authority `step_import_admin_did` gives -// the stack's first admin (docs/platform-stack-admin-grant-design.md §2). -// -// There is no role or contexts field: every grant is unrestricted admin, which -// is the whole feature. See the migration for why that is a deliberate absence -// rather than a default. -// -// **A row is an event, not a permission.** The DID here is the temporary -// did:key `pnm setup` minted, and PNM rotates off it on first connect, so this -// value goes stale by design (§7.2). Nothing on this side tracks where the entry -// moved to — `pnm acl list` against the running VTA is what answers who can act -// on it now. -type VtaAdminGrant struct { - ID uint `json:"-" gorm:"primaryKey;autoIncrement"` - SessionID uint `json:"-" gorm:"column:session_id;not null;index"` - - Did string `json:"did" gorm:"not null"` - Label string `json:"label" gorm:"not null;default:''"` - - Status string `json:"status" gorm:"not null;default:pending"` - ErrorMsg string `json:"error_msg,omitempty" gorm:"not null;default:''"` - - // RequestedBy is populated for the platform-admin route with an admins.id - // (the admin cookie's JWT carries it as UserID). It is nil for an owner - // adding a PNM to their own session; setup_sessions.user_id already records - // that actor. Nullable also lets the row outlive an admin who asked for it. - RequestedBy *uint `json:"-" gorm:"column:requested_by"` - GrantedAt *time.Time `json:"granted_at,omitempty"` - - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` -} - -func (VtaAdminGrant) TableName() string { return "vta_admin_grants" } diff --git a/internal/router/router.go b/internal/router/router.go index 101fc9b..c847c2e 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -220,6 +220,7 @@ func Setup( // docs/platform-stack-admin-grant-design.md §1 and §7.4. adminAuth.GET("/admin/platform-stack/admins", sh.ListPlatformStackAdmins) adminAuth.POST("/admin/platform-stack/admins", sh.GrantPlatformStackAdmin) + adminAuth.POST("/admin/platform-stack/admins/refresh", sh.RefreshPlatformStackAdmins) // Cluster capacity overview: CPU/memory/storage totals per node plus // how many more sessions of each mode still fit. dashH := handler.NewDashboardHandler(k8sClient) diff --git a/migrations/000032_drop_vta_admin_grants.down.sql b/migrations/000032_drop_vta_admin_grants.down.sql new file mode 100644 index 0000000..cd7ccf2 --- /dev/null +++ b/migrations/000032_drop_vta_admin_grants.down.sql @@ -0,0 +1,25 @@ +-- Rolling back recreates the schema only. Rows removed by the up migration +-- cannot be recovered. +CREATE TABLE vta_admin_grants ( + id BIGSERIAL PRIMARY KEY, + session_id BIGINT NOT NULL REFERENCES setup_sessions(id) ON DELETE CASCADE, + did TEXT NOT NULL, + label TEXT NOT NULL DEFAULT '', + status TEXT NOT NULL DEFAULT 'pending' + CHECK (status IN ('pending', 'granted', 'failed')), + error_msg TEXT NOT NULL DEFAULT '', + requested_by BIGINT NULL REFERENCES admins(id) ON DELETE SET NULL, + granted_at TIMESTAMPTZ NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE UNIQUE INDEX vta_admin_grants_live_unique + ON vta_admin_grants (session_id, did) + WHERE status IN ('pending', 'granted'); + +CREATE INDEX vta_admin_grants_session_idx ON vta_admin_grants (session_id); + +CREATE UNIQUE INDEX vta_admin_grants_one_pending_per_session + ON vta_admin_grants (session_id) + WHERE status = 'pending'; diff --git a/migrations/000032_drop_vta_admin_grants.up.sql b/migrations/000032_drop_vta_admin_grants.up.sql new file mode 100644 index 0000000..c784ada --- /dev/null +++ b/migrations/000032_drop_vta_admin_grants.up.sql @@ -0,0 +1,5 @@ +-- The VTA ACL is authoritative and vta_acl_entries stores its last complete +-- synchronized view. Grant rows contain temporary DIDs that become stale after +-- PNM key rotation, while vta_acl_snapshots.maintenance_started_at now provides +-- the cross-replica lock that pending grant rows previously supplied. +DROP TABLE IF EXISTS vta_admin_grants;