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>
Mirrors charts/terdut-server's own deployment.yaml change: v0.36.0 put
the sweeper, the notifier and the migration runner each behind a
Postgres advisory lock, and gave incident creation its own conflict
resolution, so the Recreate strategy and replicas-stays-at-1 guidance
this controller carried (explicitly tracking that chart's comment)
are no longer load-bearing.
spec.replicas' +kubebuilder:default moves 1 -> 2 (config/crd/bases and
the chart's CRD template regenerated via controller-gen and
kubebuilder's helm plugin respectively, then hand-verified identical
to the generator's own output rather than trusting a bulk regen --
the plugin's --output-dir charts writes a fresh charts/chart scaffold
rather than updating charts/terdut-operator in place, so only the
diff was taken, not the whole tree). terdutserver_deployment.go's
same-value fallback (reachable only for a TerdutServer stored before
this default existed) moves with it, and its Strategy changes from
Recreate to RollingUpdate with no explicit maxUnavailable/maxSurge --
the 25%/25% default rounds to 0/1 at replicas: 2, already
zero-downtime.
DESIGN.md's three places asserting multi-replica isn't a supported
topology (the illustrative spec.replicas YAML, spec.pod.affinity's
rationale, and the HPA deferred-feature note) are corrected to match;
the HPA note now gives its own standing reason (no scaling metric or
bounds decided yet) rather than a contradiction that no longer holds.
The chart's optional terdutServer.replicas sample value moves 1 -> 2
alongside it. image.tag must be v0.36.0 or newer for any of this to
hold -- stated in both the CRD field's doc comment and the chart
value's comment, not enforced in code, same stance the chart takes on
every other version-coupled assumption.
Co-authored-by: Claude <noreply@anthropic.com>
spec.pod (api/v1alpha1/terdutserver_types.go): annotations, nodeSelector,
tolerations, affinity, topologySpreadConstraints, resources, pod and
container securityContext, serviceAccountName, extraEnv/extraEnvFrom,
extraVolumes/extraVolumeMounts, imagePullSecrets, and an optional
disruptionBudget. All direct corev1 passthrough -- no wrapper types buy
anything for any of these, matching how CloudNativePG and the Zalando
postgres-operator both expose the same knobs, and matching this repo's
own SweeperSpec precedent ("wrap only when a round-trip through a
different type buys something"). affinity is pure user-supplied
passthrough, not a toggle-plus-generated-default the way a multi-replica
cluster operator's pod anti-affinity usually is: this operator never
auto-generates one, since spec.replicas above 1 isn't a supported
topology (the sweeper/notifier singleton constraint). Considered and
declined for this round: priorityClassName, pod labels beyond
annotations, and a HorizontalPodAutoscaler -- the last of those would
directly contradict the singleton constraint above.
disruptionBudget is the one field here that isn't a plain PodTemplateSpec
knob: when set, the controller now reconciles a PodDisruptionBudget
selecting the TerdutServer's own pods (new terdutserver_pdb.go); clearing
it deletes any it previously created. New RBAC marker on
poddisruptionbudgets to match.
Driven by a public-release pass: looking past this project's own use case
at what a mature, general-purpose operator CRD exposes here (researched
against Zalando postgres-operator and CloudNativePG specifically), not
just the fields this install happened to need.
Separately, and found while answering a question about exposing
TerdutServer through Istio instead of Gateway API: spec.networking's own
doc comment quietly promised a Gateway API HTTPRoute this operator would
build eventually ("a near-term follow-up, not deferred"). That promise is
wrong for a public release -- an operator managing someone's ingress
mechanism for them is a worse default than not touching it at all, and a
surprise HTTPRoute appearing once that follow-up eventually landed would
have been exactly backwards for an Istio (or plain-Ingress, or
intentionally-unexposed) install. Made the non-goal explicit and
permanent instead (DESIGN.md §1), removed the dead `gatewayListener`
field it was the only consumer of (zero runtime call sites anywhere --
setting it already had no effect, so this is a schema cleanup, not a
behavior change), and corrected ROADMAP.md's framing. hostname/servicePort
stay: both are live (TERDUT_PUBLIC_URL, container/Service port), this
operator just never acts on hostname for exposure. Added
examples/networking (Gateway API HTTPRoute, Istio VirtualService) showing
how to expose the plain ClusterIP Service the operator already creates --
outside the operator itself, as illustrations, not as something
examples/demo applies automatically.
No new terdut-server version requirement: both changes are CRD/controller-
only, nothing about the API this operator's bootstrap flow depends on
changed.
Found while standing up terdut-demo (Ryuvia/charts#275), a second real
TerdutServer against the same Authentik provider as production: OIDCSpec
has no trustEmail override, so a demo install copying production's OIDC
config otherwise verbatim silently runs with the wrong default for it.
Not fixed here -- recorded in §13 as a real, found gap, not a decision,
same as the mid-life teamRef note already there.
No operator-mode section existed here before -- terdut-server's own
README.md documents the feature, but this repo's design doc never
mentioned it. Added as §6 point 7, confirmed against source
(internal/api/middleware.go's OperatorModeBlock, router.go's opMode
wrapper): it blocks human writes to exactly the resources this
operator's CRDs manage (team identity, OIDC-group binding, escalation,
dead man's switches, integrations), and nothing else -- team membership,
invites, and the on-call schedule/rota stay human-editable regardless,
confirmed from the router rather than assumed from the README's prose
alone.
Covers webhook Secret generation/ownership (DESIGN.md §4.5, §7), the
WebhookSecretLost fail-closed condition, and the kind-change
delete-and-recreate rotation path.
Idempotent-create here is deliberately neither adopt-on-409
(Team/service-account) nor list-and-match-by-name (TerdutDeadmanSwitch):
terdut-server shows the webhook key exactly once, at creation, and never
again, so no server-side lookup could ever recover it after a crash.
Instead the generated webhook Secret itself -- written immediately after
the POST, before status is ever touched -- is this CR's only durable
record that a create already succeeded; found with status.integrationID
still unset on a later reconcile, it's read back directly rather than
POSTing a second, orphaned integration. Found missing with
status.integrationID *set* instead, that's the already-designed
WebhookSecretLost case: fail closed, not self-healed, since the key is
genuinely gone and recreating it would rotate a live webhook URL with no
spec change to explain why.
Renaming (PATCH) never touches the key, so it's applied unconditionally
every reconcile, same as the escalation policy's whole-policy PUT. A
spec.kind change is the one case with no in-place update verb at all:
DELETE the old integration, delete the stale webhook Secret, then run the
same create path fresh -- fires a Warning event since this breaks whatever
still sends to the old URL.
Also: fakeTerdutServer grows POST/PATCH/DELETE .../integrations routes
behind a new handleIntegrationSubPath, split out of handleTeamSubPath to
stay under gocyclo's threshold; three goconst-flagged test literals
("does-not-exist", "unready") and one unparam-flagged test helper
parameter (bootstrapReadyTerdutServer's always-"default" namespace) get
shared/removed now that a fourth same-shaped caller made the repetition
concrete enough for the linter to flag.
DESIGN.md §13 gains one honest gap found while grounding this stage, not
introduced by it: no child CRD specially detects a mid-life teamRef
change; all three always resolve spec.teamRef fresh and trust the
already-stored server-side id remains valid there.
make fmt lint test build all clean; internal/controller envtest coverage
holds at 71.6%.
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.
§5's Team row now cites GET /api/teams?name= (terdut-server's
TEAM-LOOKUP.md, landed today) -- without it the idempotent-create general
rule's claim that every resource here has a real lookup to adopt-on-409
through wasn't actually true for Team specifically, confirmed by tracing
it before writing any TerdutTeam code, same as Stage 1's bootstrap flow.
Also: TerdutTeam.status.credentialsSecretRef drops namespace for key,
matching the TerdutServer fix from Stage 1 -- same reasoning, missed
there originally.
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.
Removes the premise Stage 1's bring-your-own credential design was built
on. Confirmed with the user directly: this operator creates and owns
every TerdutServer it manages; there is no hand-deployed or chart-deployed
install it's expected to target or migrate.
- §1: states this explicitly -- the root the rest of this commit hangs off.
- §4.1: spec.credentialsSecretRef (bring-your-own input) removed entirely,
not kept as unused flexibility. status.credentialsSecretRef stays as
pure output.
- §6: self-registration is now the *only* bootstrap path, not one of two --
and, since it's now load-bearing rather than a fallback with an easy
escape hatch, closed the two real crash windows in it properly rather
than leaving them as theoretical gaps: a checkpoint Secret for the raw
admin key between /api/bootstrap and minting the service account, and
adopt-on-409 (§5's general rule) if a prior interrupted attempt already
got that far. A checkpoint lost after being used crosses into the same
fail-closed territory §5's webhook-Secret-loss rule already established
-- same recovery (delete and recreate), not a new, one-off workaround.
- §10: dropped the migrate-an-existing-install narrative and the
chart-Job-vs-operator bootstrap race question entirely -- both
presupposed an install the operator might adopt or race against, which
doesn't exist. Kept the installer-chart framing on its own.
- §13: dropped the now-stale "Helm chart migration execution" deferred item.
§8 (Postgres) needed no change -- it already described both the DSN and
Zalando paths as co-equal, full-design detail, with no sequencing between
them to remove.
It's always the operator's own namespace by construction now (§6), never
anything else, so there was nothing for the field to vary -- key varies
instead (fixed 'token' when self-generated, whatever a human chose when
adopted from spec.credentialsSecretRef).
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.
- Add .gitignore (build artifacts, editor swapfiles, envtest testbin).
- Remove the stray .DESIGN.md.swp that was sitting untracked in the repo.
- Carries the DESIGN.md §5/§9/§13 edits from the secret-loss discussion:
fail-closed (not self-healed) TerdutAlertSource webhook Secret loss, and
the corrected RBAC section (the webhook Secret lives in the CR's tenant
namespace, not the operator's own namespace as an earlier draft claimed).
/api/bootstrap is single-shot per install (gated on COUNT(*) FROM
users, confirmed against internal/api/users.go and the chart's
bootstrap-job.yaml), not per identity — the two-identity bootstrap
plan and the delete-Secret-to-rotate runbook this section described
don't work against that. Rewrites §6 points 1/5/6 around a dedicated,
repeatable service-account credential instead (proposed server-side in
terdut-server's new SERVICE-ACCOUNTS.md), notes in §9 that Secret
mirroring is RBAC-sound but still hands out a server-admin-equivalent
credential per consenting namespace, and flags in §10 that chart-vs-
operator bootstrap ownership blocks §6 and needs deciding first.
Updates §13 to mark the service-account type as v1-blocking rather
than a someday improvement, and adds a version-discovery endpoint to
the same list (both this operator and terdut-tui currently detect
server capability by route-probing).
Niklas Ye