3 Commits

Author SHA1 Message Date
Niklas Ye 62664c93ff Keep the instance credential across a TerdutServer delete, and adopt it on recreate
Deleting a TerdutServer removed the credential Secrets but never touched the
database, so a recreated one found a server that was already bootstrapped and
no key for it: /api/bootstrap answered 403 and the operator stopped at
BootstrapStateLost, whose message and DESIGN.md both said "delete and
recreate". That is how the terdut-demo install on the cluster got stuck on
2026-10-03: Helm's cleanupOnFail deleted its TerdutServer after a failed
upgrade, the recreate found the bootstrapped database, and it sat at Ready:
False for five days until the database was reset by hand. Recreating cannot
fix it, because the finalizer clears Secrets and the database is not its to
reset, so "a fresh create starts clean" was only ever true when the database
went with it.

spec.credentials.deletionPolicy is Retain by default: the finalizer keeps the
instance credential Secret (Delete removes it, as before). The bootstrap
checkpoint is always removed. Before calling /api/bootstrap, reconcile now
looks for the retained Secret and asks the server for the operator's own
service account with its token. Accepted: adopt it and skip bootstrap.
Rejected with 401/403: the Secret outlived a database reset, so ignore it and
bootstrap like a first install, which replaces it. Any other error retries.
terdut-server's own tests already call that endpoint with an instance-scoped
key, so the permission is not new.

BootstrapStateLost is still the answer when the server is bootstrapped and no
credential it accepts survives, but its message now names the Secret to
restore and says that recreating does not clear the database. DESIGN.md §6
says the same, and the chart passes the setting through as
terdutServer.credentials.deletionPolicy.

A retained Secret of a TerdutServer that is gone for good is an orphan to
delete by hand. It is inert: nothing adopts it unless the server accepts the
token.

Checked on the kind demo with a locally built image against the real
terdut-server v0.43.0: deleting the TerdutServer kept the Secret, recreating it
reached Ready with the same credential (identical hash) and both TerdutTeams
came back Ready with their original ids. The controller specs cover adoption,
a rejected token after a reset, the bootstrapped-and-rejected failure, and
both deletion policies.

Co-authored-by: Claude <noreply@anthropic.com>
2026-10-08 21:09:56 +02:00
Niklas Ye c9c52af2f7 Stage 2: TerdutTeam (create, mint team credential, rename/oidc-groups, delete)
CI / test (push) Successful in 1m41s
Implements ROADMAP.md Stage 2 against terdut-server's now-real
GET /api/teams?name= (TEAM-LOOKUP.md, landed in terdut-server just
before this commit) -- without it, the adopt-on-409 pattern this
controller depends on for team creation had no server-side lookup to
call, the same gap TerdutServer's own bootstrap flow hit and fixed in
Stage 1.

- api/v1alpha1: TerdutTeamSpec per DESIGN.md §4.2 (serverRef, displayName,
  oidc). status.credentialsSecretRef drops namespace for key, matching
  the fix already applied to TerdutServer's.
- internal/controller:
  - terdutteam_controller.go: resolves serverRef (same-namespace by
    default; cross-namespace gated by the target TerdutServer's
    spec.allowedTeams, DESIGN.md §4.6), waits for that TerdutServer to be
    Bootstrapped (no cross-controller RPC -- reads its
    status.credentialsSecretRef directly, DESIGN.md §5), creates the team
    and mints its team-scoped credential using the TerdutServer's
    instance-scoped one, then applies rename/oidc-groups with the
    team-scoped credential every reconcile (both are idempotent PUTs of
    the whole resource -- applied unconditionally rather than diffed
    against a stored last-applied value, same "cheap because it's small"
    reasoning §5 already gives the escalation policy's whole-policy PUT).
  - terdutteam_allowedteams.go: the §4.6 consent check in isolation from
    any client, unit-tested directly against hand-built inputs.
  - terdutteam_bootstrap.go: create-or-adopt-on-409 for the team itself
    (via TEAM-LOOKUP.md) and for its team-scoped service account (via the
    same GET-by-name+mint-new-key pattern Stage 1 already uses for the
    instance account).
  - secrets.go: extracted TerdutServer's write/read-credential-Secret
    helpers into free functions, now shared by both controllers rather
    than duplicated.
  - Finalizer deletes the team server-side (owner-gated, needs the
    team-scoped credential -- confirmed against source that an
    instance-scoped one does not satisfy requireTeamOwner, same finding
    as TEAM-LOOKUP.md's) and cleans up its credentials Secret. A team
    created but never fully reconciled to Ready (no team-scoped
    credential ever minted) is left orphaned server-side on delete -- a
    known, documented limitation (terdut-server has no delete path that
    doesn't require owner-equivalent access), not a silent gap.
- internal/tdclient: Team type, CreateTeam, GetTeamByName, RenameTeam,
  DeleteTeam, SetTeamOIDCGroups, CreateTeamServiceAccount -- matching
  terdut-server's real handlers' shapes field-for-field, same as Stage
  1's client additions.
- Tests: envtest covering the happy path, both not-ready reasons
  (ServerRefNotFound, WaitingForServer), cross-namespace allow/deny
  (default-closed and explicit All), both adopt-on-409 paths (team
  itself, team-scoped service account), and deletion. Extended the shared
  fakeTerdutServer (Stage 1) with team endpoints rather than writing a
  second, separately-drifting fake. 72.8%/30.6% coverage, 0 lint issues.

Verified locally: make fmt lint test build all clean.
2026-10-01 11:09:40 +02:00
Niklas Ye 8064876cb1 Stage 1: TerdutServer full lifecycle (Deployment, Service, both database
CI / test (push) Successful in 1m46s
paths, self-registration bootstrap)

Replaces the bring-your-own-only Stage 1 (commit 1be7cf2) wholesale, per
the redesign in the previous two commits: the operator creates every
server it manages, so self-registration (DESIGN.md §6) is the only
bootstrap path, and Deployment/Service/database management builds
together with it (ROADMAP.md Stage 1) rather than behind a separate
later stage.

Grounded in terdut-server's actual chart (charts/terdut-server/templates/
deployment.yaml, values.yaml), not reconstructed from DESIGN.md's
illustrative YAML alone -- env var names, the password-via-PGPASSWORD
convention, the Recreate deployment strategy, /healthz probes, and the
TERDUT_OPERATOR_MODE=true decision (always on here, unlike the chart's
default-off: every write this operator's own future controllers make
goes through a service account already) all match that source exactly.

- api/v1alpha1: full TerdutServerSpec (image, replicas, networking,
  database, sweeper, deadman, notify, oidc, passwordLogin, allowedTeams).
  spec.database is a oneOf (dsn xor postgresClusterRef) via CEL
  XValidation. No spec.credentialsSecretRef -- removed entirely in the
  prior redesign commit, not carried forward.
- internal/controller:
  - terdutserver_deployment.go: Deployment + Service via CreateOrUpdate,
    owned (OwnerReference), env built field-for-field against the chart.
  - terdutserver_database.go: both §8 paths. The Zalando path resolves
    the postgresql.acid.zalan.do CR by convention (database/role both
    "terdut", matching every DESIGN.md example) and only ever confirms
    its generated credentials Secret exists -- never reads the value,
    same "wire a secretKeyRef, don't read it" posture the DSN path takes.
    classifyClusterGetError is its own function specifically so the
    CRD-not-installed case (meta.IsNoMatchError) is unit-testable without
    a real client.
  - terdutserver_bootstrap.go: self-registration, checkpointed against
    both real crash windows (DESIGN.md §6 point 1) -- an admin-key
    checkpoint Secret, and adopt-via-GET+mint-new-key on a 409 from
    creating the service account. BootstrapStateLost is its own error
    type so Reconcile can route it to a condition instead of an infinite
    retry.
  - terdutserver_controller.go: ties it together -- finalizer add, DB
    resolution, Deployment/Service reconcile, wait for a ready replica,
    bootstrap, Ready/Bootstrapped/DatabaseReady conditions. Finalizer on
    delete only removes the generated Secrets: terdut-server's API can't
    delete a user or service account, only revoke keys, so there's
    nothing server-side to undo.
- internal/tdclient: added Bootstrap, CreateInstanceServiceAccount,
  GetServiceAccountByName, CreateServiceAccountKey, matching
  terdut-server's real handlers' request/response shapes (internal/api/
  users.go, service_accounts.go in that repo) field-for-field.
- Tests: envtest suite covering the full DSN-path lifecycle end to end
  (finalizer -> Deployment/Service -> simulated readiness -> real
  bootstrap against an httptest.Server fake), the adopt-on-409 recovery
  path, BootstrapStateLost, both Zalando outcomes (cluster not found;
  cluster + Secret found -> real DSN -> Ready), and deletion. A minimal
  test-only stub of the Zalando CRD (internal/controller/testdata) lets
  envtest create fixture objects without a real postgres-operator
  installed. 74.0%/44.7% coverage, 0 lint issues.
- Two things scoped down from §8's full ambition, called out in code and
  ROADMAP.md rather than silently dropped: no live watch on the
  Zalando-generated Secret for rotation (periodic resync notices
  eventually, not immediately), no Gateway API HTTPRoute creation from
  spec.networking (would add a new dependency; nothing about proving
  bootstrap works depends on external ingress existing). Both are
  near-term follow-ups.

Verified locally: make fmt lint test build all clean.
2026-10-01 09:11:56 +02:00