Traced the actual flow against terdut-server's real source before writing any Stage 1 controller code, rather than trusting this section's own prior description of it: - internal/api/middleware.go's AuthMiddleware hard-rejects with 401 any request carrying neither a Bearer token nor a session cookie, before handleListServiceAccounts' own (more permissive) internal check ever runs. So "on 403, self-lookup via GET /api/service-accounts?name=" -- this section's described fallback -- cannot work unauthenticated; an earlier draft of this section assumed otherwise. - That only actually matters in the rare case where this TerdutServer's own controller loses the /api/bootstrap race... except Stage 1's own setup (ROADMAP.md) guarantees it loses every time: terdut-server is deployed via its existing chart, which runs its own bootstrap Job, before the TerdutServer CR or its controller exist at all. The self-registration flow was never going to complete for the one scenario Stage 1 actually exercises. Fix: spec.credentialsSecretRef (§4.1), bring-your-own -- a human mints an instance-scoped service account once, manually, with their own admin session, and hands the controller that Secret directly. This is now the primary, expected path; self-registration on a genuinely fresh install (where this controller might actually win the race) stays as the fallback it was always meant to be, not the only path. Also corrected: this section's opening paragraph still said "v1-blocking, not v1-shippable" pending SERVICE-ACCOUNTS.md landing -- confirmed shipped (internal/api/service_accounts.go, migration 014) since Stage 0's work on this repo; stale framing removed.
This commit is contained in:
@@ -169,6 +169,15 @@ spec:
|
|||||||
adminGroup: ""
|
adminGroup: ""
|
||||||
sessionMaxAge: 12h
|
sessionMaxAge: 12h
|
||||||
passwordLogin: true
|
passwordLogin: true
|
||||||
|
# Bring-your-own instance credential: a human mints this once, manually, with
|
||||||
|
# their own admin session (`POST /api/service-accounts {name: ..., scope:
|
||||||
|
# instance}`) and creates this Secret themselves, in the OPERATOR's own
|
||||||
|
# namespace (same namespace status.credentialsSecretRef below would otherwise
|
||||||
|
# point into). When set and the Secret exists, the controller adopts it
|
||||||
|
# directly and skips bootstrap entirely -- see §6 for why this is required,
|
||||||
|
# not optional polish, whenever terdut-server was already bootstrapped by its
|
||||||
|
# chart or a human before this CR existed (the normal case, not an edge one).
|
||||||
|
credentialsSecretRef: {name: "", key: token}
|
||||||
# Consent for TerdutTeams in OTHER namespaces to set serverRef at this
|
# Consent for TerdutTeams in OTHER namespaces to set serverRef at this
|
||||||
# TerdutServer. Same-namespace TerdutTeams never need this. Modeled on
|
# TerdutServer. Same-namespace TerdutTeams never need this. Modeled on
|
||||||
# Gateway API's Gateway.spec.allowedListeners.namespaces (the ListenerSet
|
# Gateway API's Gateway.spec.allowedListeners.namespaces (the ListenerSet
|
||||||
@@ -190,6 +199,17 @@ Field-for-field this is the chart's `values.yaml` reshaped as a spec — the
|
|||||||
operator absorbs the chart's Deployment/Service/bootstrap-job templates, so
|
operator absorbs the chart's Deployment/Service/bootstrap-job templates, so
|
||||||
existing installs have a direct mapping when migrating (see §10).
|
existing installs have a direct mapping when migrating (see §10).
|
||||||
|
|
||||||
|
`spec.credentialsSecretRef` is an *input* (bring-your-own), distinct from
|
||||||
|
`status.credentialsSecretRef`'s *output* (generated-by-the-controller) —
|
||||||
|
when the input is set, the controller treats it as the credential outright
|
||||||
|
and echoes its name back into `status.credentialsSecretRef` rather than
|
||||||
|
generating a second Secret alongside it. Left unset, the controller falls
|
||||||
|
back to the self-registration flow (§6) — which only actually completes if
|
||||||
|
this `TerdutServer`'s own first reconcile is the one that wins the
|
||||||
|
`/api/bootstrap` race against a genuinely empty install; see §6 for why
|
||||||
|
that fallback is the exception, not the common case, and why this field
|
||||||
|
exists at all rather than being deferred as nice-to-have.
|
||||||
|
|
||||||
`spec.database` fields are `+kubebuilder:validation:XValidation` guarded to
|
`spec.database` fields are `+kubebuilder:validation:XValidation` guarded to
|
||||||
be mutually exclusive (`dsn` xor `postgresClusterRef`); mirrors the chart's
|
be mutually exclusive (`dsn` xor `postgresClusterRef`); mirrors the chart's
|
||||||
"chart provisions no database" stance — this operator provisions no database
|
"chart provisions no database" stance — this operator provisions no database
|
||||||
@@ -399,11 +419,30 @@ General rules for every controller:
|
|||||||
|
|
||||||
## 6. Bootstrap & authentication to terdut-server's API
|
## 6. Bootstrap & authentication to terdut-server's API
|
||||||
|
|
||||||
terdut-server has no first-class "service account" token type today — API
|
terdut-server's scoped service-account credential type
|
||||||
keys belong to a real user row (`api_keys.user_id`). This section depends on
|
(`terdut-server`'s `SERVICE-ACCOUNTS.md`) has shipped — confirmed against
|
||||||
a server-side feature request (`terdut-server`'s `SERVICE-ACCOUNTS.md`) that
|
source: `internal/api/service_accounts.go`, migration
|
||||||
adds one; treat this whole section as v1-blocking, not v1-shippable, until
|
`014_service_accounts.sql`, and `internal/api/router.go` wiring it in under
|
||||||
that lands (§10, §13).
|
`AuthMiddleware`. This section is no longer blocked on it; the "v1-blocking"
|
||||||
|
framing here was accurate when this section was first written and is stale
|
||||||
|
now.
|
||||||
|
|
||||||
|
**`GET /api/service-accounts?name=` is not an unauthenticated lookup, unlike
|
||||||
|
an earlier draft of this section assumed — confirmed against
|
||||||
|
`internal/api/middleware.go`'s `AuthMiddleware`, which hard-rejects any
|
||||||
|
request carrying neither a Bearer token nor a session cookie with `401`
|
||||||
|
before any handler (including this one's own internal, more permissive
|
||||||
|
name-filter check) ever runs.** This matters beyond a technicality: it means
|
||||||
|
the self-registration flow below (point 1) only ever completes for the
|
||||||
|
`TerdutServer` whose own controller happens to win the `/api/bootstrap` race
|
||||||
|
on a genuinely empty install. Every other case — including Stage 1's own
|
||||||
|
setup (`ROADMAP.md`): terdut-server deployed by its existing chart, which
|
||||||
|
runs its own bootstrap Job, *before* the `TerdutServer` CR or its controller
|
||||||
|
ever exist — leaves the controller with no credential and no authenticated
|
||||||
|
way to get one. `spec.credentialsSecretRef` (§4.1) exists to make that the
|
||||||
|
normal path, not an unhandled edge case: a human mints an instance-scoped
|
||||||
|
service account once, manually, with their own admin session, and hands the
|
||||||
|
controller that Secret directly.
|
||||||
|
|
||||||
**Every credential the operator holds — the one instance-scoped key per
|
**Every credential the operator holds — the one instance-scoped key per
|
||||||
`TerdutServer`, and one team-scoped key per `TerdutTeam` — lives in a Secret
|
`TerdutServer`, and one team-scoped key per `TerdutTeam` — lives in a Secret
|
||||||
@@ -425,22 +464,35 @@ when `allowedTeams` narrows, no "OwnerReferences can't cross namespaces so
|
|||||||
track it in status instead" workaround — none of that machinery is needed
|
track it in status instead" workaround — none of that machinery is needed
|
||||||
when nothing ever crosses into a tenant namespace in the first place.
|
when nothing ever crosses into a tenant namespace in the first place.
|
||||||
|
|
||||||
1. **Bootstrap, confirmed against source** (`internal/api/users.go`'s
|
1. **First reconcile checks `spec.credentialsSecretRef` before anything
|
||||||
|
else.** Set and the Secret exists: adopt it as-is, set
|
||||||
|
`status.credentialsSecretRef` to the same reference, `Bootstrapped: True`,
|
||||||
|
done — no API call made at all. This is the path every Stage 1 install
|
||||||
|
actually takes (`ROADMAP.md`): terdut-server deployed by its existing
|
||||||
|
chart, bootstrapped by that chart's own Job, before this CR exists.
|
||||||
|
Unset (or the named Secret doesn't exist yet): fall through to
|
||||||
|
self-registration, confirmed against source (`internal/api/users.go`'s
|
||||||
`handleBootstrap`): `/api/bootstrap` is single-shot *per install*, gated
|
`handleBootstrap`): `/api/bootstrap` is single-shot *per install*, gated
|
||||||
on `SELECT COUNT(*) FROM users` — once non-zero, every call 403s
|
on `SELECT COUNT(*) FROM users` — once non-zero, every call 403s
|
||||||
regardless of identity, exactly as `charts/terdut-server`'s own
|
regardless of identity, exactly as `charts/terdut-server`'s own
|
||||||
`bootstrap-job.yaml` already assumes (403 → "already bootstrapped,
|
`bootstrap-job.yaml` already assumes (403 → "already bootstrapped,
|
||||||
nothing to do", exit 0). So the `TerdutServer` controller's first-reconcile
|
nothing to do", exit 0). Call `/api/bootstrap`; on `201`, its response
|
||||||
flow is: call `/api/bootstrap` **only if nothing has bootstrapped yet**
|
(`{"user": ..., "api_key": {"key": "<raw>", ...}}`) hands back a real,
|
||||||
(a genuinely empty install, e.g. chart's bootstrap Job disabled in favor
|
usable admin key directly — use it for exactly one further call,
|
||||||
of the operator owning this — see §10); on 403, look itself up instead —
|
`POST /api/service-accounts {name: "terdut-operator", scope: "instance"}`,
|
||||||
`GET /api/service-accounts?name=terdut-operator` — and self-register via
|
and keep *that* key, not the raw admin one, as the lasting credential.
|
||||||
`POST /api/service-accounts` (instance scope) if it isn't already
|
**On `403`, there is no further fallback**: per this section's opening
|
||||||
registered. This also resolves the race if both the chart's Job and this
|
note, `GET /api/service-accounts?name=` needs a credential this
|
||||||
controller call `/api/bootstrap` against the same fresh install: whichever
|
controller does not have, so self-lookup cannot run unauthenticated. Set
|
||||||
loses treats 403 as "go get my own credential the other way," not an
|
`Ready: False, reason: WaitingForCredential` with an event telling the
|
||||||
error.
|
human to mint an instance-scoped service account with their own admin
|
||||||
2. The resulting instance-scoped key is written to a generated Secret in the
|
session and set `spec.credentialsSecretRef`, and requeue with backoff —
|
||||||
|
this is the expected, steady-state outcome whenever the operator loses
|
||||||
|
(or never entered) the bootstrap race, not a transient error to retry
|
||||||
|
past.
|
||||||
|
2. On the self-registration path only (point 1's bring-your-own path
|
||||||
|
generates nothing — it adopts the human-provided Secret directly): the
|
||||||
|
resulting instance-scoped key is written to a generated Secret in the
|
||||||
**operator's own namespace** (e.g. `<serverRef.namespace>.<serverRef.name>-instance-credentials`),
|
**operator's own namespace** (e.g. `<serverRef.namespace>.<serverRef.name>-instance-credentials`),
|
||||||
referenced back from `TerdutServer.status.credentialsSecretRef: {name,
|
referenced back from `TerdutServer.status.credentialsSecretRef: {name,
|
||||||
namespace}` (§4.1) — `namespace` is part of the reference now, since it's
|
namespace}` (§4.1) — `namespace` is part of the reference now, since it's
|
||||||
|
|||||||
Reference in New Issue
Block a user