Design: passwordRotation for postgres/mariadb via a credentials-owning controller

[scooby87]

Context

cozystack/cozystack#4078 drops plaintext users[].password from the postgres and mariadb apps: passwords are now always chart-generated into the <release>-credentials Secret and read only from there. That part is sound and stays in #4078. The PR also introduced passwordRotation — an integer counter a tenant bumps to regenerate every managed password — and review showed that piece needs a different foundation, so it is being split out here for its own design discussion before implementation.

Why rotation cannot live in the chart render

While the credentials Secret is rendered by the chart, its cleartext value is part of the Helm release manifest, and helm-controller retains the last MaxHistory revisions of that manifest. A passwordRotation bump performed in response to a leak therefore does not retire the leaked password: it stays in up to several stored release revisions until unrelated upgrades push it out of history. Rotation whose purpose is to revoke, but which cannot revoke, is worse than no rotation — the credential the tenant used to control in one scrubbable place now lives in several the tenant cannot see. This is Helm behaviour, not something a template can work around.

Three further problems share the same root — there is no durable state and no actor outside the render:

• No completion signal. A tenant writes an integer and nothing observable follows: no status, no condition, no event, no way to answer "did the new password reach the database". If the applying Job/operator step fails, the Secret advertises a credential the database never received and nothing says so.
• Fragile lookup. Rotation state is a counter compared against a value read back with Helm's lookup, which returns empty during any render without API access; the template cannot distinguish "first install" from "the read did not happen", and both paths regenerate and overwrite the Secret while the database keeps the old password — a silent credential split.
• Two mechanisms, and root outside both. Postgres converges through a post-upgrade hook Job; MariaDB through the operator reacting to a watch-labelled Secret. Different windows, different failure modes, one values field. MariaDB root is excluded from rotation entirely (the operator only applies rootPasswordSecretKeyRef at datadir bootstrap), so the feature does not cover the most privileged account on that engine.

Proposed direction

Give rotation an owner outside the Helm render: a controller that creates and holds the credentials Secret, with the chart referencing it by name rather than rendering it. The controller applies the rotation and reports the outcome in status. That also dissolves the problems above by construction — the swallowed first bump exists only because there is nowhere to keep the baseline, and the missing completion signal only because there is nothing to carry it.

Requirements for the design:

• The credentials Secret is owned by the controller, not by the chart render, so its value never enters the Helm release manifest or its retained history.
• Rotation reports a completion signal (a status/condition on the app CR) distinguishing "requested", "applied to the database", and "failed".
• A single mechanism covers both engines, or the per-engine difference is explicit and justified.
• root (MariaDB) is covered, or its exclusion is a deliberate, documented decision rather than an accident of the delivery path.
• The baseline for an existing release is durable state, so the first bump is not spent establishing it.

Open questions

• Where the controller lives (extend an existing operator vs a small dedicated one) and how it holds the Secret relative to Flux/Helm ownership.
• How rotation interacts with a restore into a copy (the recovered roles carry the source hashes until reconciled — see #4078's restore handling).
• Whether root rotation on MariaDB is feasible at all given rootPasswordSecretKeyRef is bootstrap-only, or needs an out-of-band reset.

Links

• PR carrying the plaintext-drop half: feat(postgres,mariadb)!: drop plaintext user passwords cozystack#4078
• The review that motivated the split is on that PR (architectural note on the rotation shape).

[scooby87]

Design review — gaps to close before implementation

Reviewed the proposal adversarially (two independent passes over the current postgres/mariadb code). The direction and the root-cause diagnosis are right, but the design under-specifies the hardest parts. Below: two blockers, then the items that must be resolved before implementation, then minor notes. None of these say "don't do it" — they say the design isn't complete enough to build from yet.

Blockers (without these the stated goal is not reached)

Migration / ownership handoff is undefined. Today <release>-credentials is Helm-managed (it carries meta.helm.sh/release-name and is part of the release manifest). The moment the chart stops rendering it, the next helm upgrade sees it removed from the manifest and prunes it — while rootPasswordSecretKeyRef, the User CRs and the app pods still reference it by name. That is a window with no Secret: password loss / downtime. This is the same class of trap the repo has already hit (kafka resource-policy: keep → uninstall hang; the db-autoscaler SSA-handoff saga). The design needs an explicit ownership handoff: how the controller adopts the existing Secret (rewrite managed-by metadata before the chart stops rendering it? resource-policy: keep? a one-time adopt step?), with a guaranteed order and no absence window.

The whole password surface must leave the render, not one Secret. The motivation targets the credentials Secret, but postgres also historically rendered the password into the <release>-init-script Secret (ALTER ROLE ... WITH PASSWORD '<literal>'), which is equally part of the release history. That init-script copy is already removed in PR #4078 (the init Job now reads the password from the mounted Secret at run time). State the requirement explicitly — no password-bearing surface is chart-rendered — and note the init-script half is handled by #4078, so the design reads as complete rather than covering only one Secret.

Must resolve before implementation

Bootstrap / ordering on a fresh install is circular. The chart references the Secret by name, but the controller has not created it yet. MariaDB is the sharp case: rootPasswordSecretKeyRef is applied only at datadir bootstrap, with no second chance — if the controller has not created the Secret by first bootstrap, root is never set. Define a hard order: the controller creates the Secret before the DB operator reaches bootstrap. And define how the controller even knows a release needs a Secret if it is not the chart that renders it — see the trigger object below.

The delivery channel for the rotation intent is undefined. passwordRotation: N lives in Helm values, which the controller does not watch. If the Secret is controller-owned and not chart-rendered, the counter never reaches the controller. So the chart still has to render something the controller watches — a small trigger object (Helm-owned, carrying the counter + engine + target users). Pin that object down; it also solves release discovery for the bootstrap problem above.

"A single mechanism for both engines" is a false requirement — there are three paths, and one has a two-writer conflict. postgres: the controller runs ALTER ROLE (SQL). mariadb non-root: mariadb-operator reconciles the User CR from a watch-labelled Secret (completion lives on the User CR status, not observable by the controller directly). mariadb root: neither (bootstrap-only). If the controller writes SQL for both engines to get one completion signal, mariadb non-root then has two writers (operator User CR + controller SQL) → conflict/flapping. Resolve it: either the controller is the only writer (drop User CRs from the password loop) or accept per-engine mechanisms with root handled separately. Reframe as a single control plane (trigger + status) with per-engine appliers.

Who applies the password, and how "applied" is observed. A completion signal is only meaningful if something watches the real database change. State whether the controller itself applies + verifies, or delegates to the init Job / operator, per engine.

root rotation on MariaDB must be a decision, not an open question. rootPasswordSecretKeyRef is bootstrap-only, so changing root in the Secret never reaches the live server. The only path is the controller connecting as root and running ALTER USER 'root' IDENTIFIED BY .... Make that the design; otherwise "root covered" is unmet and the feature again skips the most privileged account.

Restore-into-a-copy is "rotation without a bump" — the counter model misses it. A restore brings the source password hashes back into the roles, while a controller would generate a fresh Secret → the Secret advertises a password the database does not have (the exact failure the feature exists to prevent, structurally, on restore). The counter does not change on a restore, so the controller must detect the restore itself (an annotation / generation, not the counter) and force a full re-apply of all passwords. (This interacts with #4078's restore handling.)

A central controller owning credentials across all tenant namespaces is a blast-radius / RBAC question. Owning <release>-credentials in every tenant namespace needs cluster-wide secrets get/create/update/watch; RBAC cannot scope by Secret name, so the controller can read every Secret in every tenant namespace. A compromise of the controller = every database password on the platform, which no single component aggregates today. Address it: per-tenant scoping, or an explicit trade-off with a narrowed scope (dedicated SA, egress, namespace scoping).

Minor

• "Cleartext never enters retained history" overclaims — say "no longer retained via Helm's MaxHistory revisions". The live Secret is still in etcd, in etcd backups, and readable by tenant-scoped subjects that already have get on it (dashboard RBAC). State the threat-model boundary.
• "Applied" ≠ "revoked" for existing sessions: ALTER ROLE ... PASSWORD does not drop open connections, so a leaked password keeps working on a live connection until it closes. For a leak-response feature, note that real revocation needs a session kill (pg_terminate_backend / KILL).
• The account-coverage matrix is uneven and undocumented: the postgres superuser is CNPG-managed (<release>-superuser, out of rotation scope); mariadb tries to include root. A per-engine table of which accounts the feature covers would help.
• Make explicit that the trigger stays on the app CR and the durable baseline lives in the controller-owned Secret's annotation/status (outside the render) — that is what removes the swallowed-first-bump.

What is sound

The root-cause analysis is correct: a chart-rendered Secret's cleartext is retained through Helm revisions and a template cannot work around it. Tying the missing completion signal and the fragile lookup to the same cause (no durable state, no actor outside the render) is accurate. Requiring a durable baseline, and requiring the root exclusion to be a deliberate decision rather than an accident of the delivery path, are the right framings.

[lexfrei]

Premises check out against main (df20f69e5).

The lookup fallback is current main behaviour, not something #4078 introduces. mariadb/templates/secret.yaml:1 and postgres/templates/init-script.yaml:1 read the Secret with lookup, then fall back to randAlphaNum 16 at secret.yaml:19 when the key is missing. Silent regeneration is live today.

Root is already answered in the tree. mariadb/templates/mariadb.yaml:20-28 documents that a missed bootstrap window leaves the install-time root, passwordless and local only, with no path back. So root needs the controller connecting and running ALTER USER, or it stays uncovered. Not really open.

Also passwordRotation is already fully out of #4078. Grep for rotat over the whole diff returns one hit, a removed comment line. Title still says "add rotation".

Two related items not linked here: #37 (SecretRef for external credentials) proposes a cozy-lib helper for the same generate-or-reuse lookup idiom, worth reconciling before either lands, and #69 (platform lifecycle operator) as a candidate host for the controller.

[scooby87]

Resolved design (RFC) — passwordRotation via a credentials-owning controller

This turns the open questions above into concrete decisions so implementation can start. Two of them (D3 and D6) carry standing-privilege trade-offs and want an explicit API-owner call — they are flagged. Everything else is a recommendation ready to build against once signed off.

Scope

In: rotation of chart-managed database passwords for apps.cozystack.io/Postgres and apps.cozystack.io/MariaDB, so a bump actually revokes the old credential and reports when it landed.

Out (already shipped in cozystack/cozystack#4078, unchanged here): dropping the plaintext users[].password field, always-generating passwords, and removing the password from the postgres init-script SQL (applied at run time from the mounted Secret). This RFC builds on that state.

D1 — Controller home and trigger

A new small controller, password-rotation-controller, shipped as a packages/system component (it is engine-agnostic and must not live inside either database operator). It watches the tenant app CRs (Postgres, MariaDB) and reconciles their credentials Secret.

The trigger is a field on the app CR — spec.passwordRotation (integer) returns, but it is read by the controller, never by the chart. The chart no longer renders anything password-related except the reference by name. Discovery works because the controller watches the app CRs directly, so it does not depend on the chart rendering a marker.

D2 — Secret ownership and migration

The controller creates and owns <release>-credentials; the chart references it by name and never renders it. Ownership is expressed with an ownerReference to the app CR plus a controller-managed field manager.

Migration for existing releases (the blocker): the controller adopts the current chart-rendered Secret before the chart drops it. On first reconcile of an existing release it (1) reads the live Secret, (2) strips the meta.helm.sh/* labels/annotations and sets its own owner, (3) records the adopted passwords as the baseline. Helm then no longer considers the object part of the release, so a subsequent chart upgrade that stops rendering it does not prune it. Order is guaranteed by doing the adoption in the controller's reconcile, which runs independently of the Helm upgrade; the chart change that stops rendering the Secret ships only in a release after the controller is present (a two-step rollout: controller first, chart-stops-rendering second).

D3 — Who applies the password, and the completion signal ⚠️ needs API-owner call

Recommendation: the controller is the single writer, but it applies through a short-lived per-rotation Job rather than by holding standing DB connectivity. On a rotation the controller spawns a Job (reusing the existing init-Job image/pattern) that runs ALTER ROLE/ALTER USER ... IDENTIFIED BY for each managed user, reads the new password from the mounted Secret, and exits; the controller watches the Job to completion and writes the condition. This keeps DB access ephemeral (a Job, as today) instead of giving the controller a persistent superuser connection to every tenant database.

This makes the applier engine-uniform (SQL for both) and removes the two-writer conflict: for MariaDB the operator's User CR is taken out of the password loop (the controller-run Job owns password application; the User CR keeps managing existence/grants). Completion is observable because the controller owns the Job.

The alternative — the controller connecting directly to each DB as superuser/root — is simpler to write but gives the controller a standing high-privilege connection to every managed database. That is the trade-off to decide.

D4 — MariaDB root

root is rotated in-band by the same per-rotation Job connecting as root and running ALTER USER 'root'@... IDENTIFIED BY ... against the live server. rootPasswordSecretKeyRef stays the bootstrap-time source; the Job is what changes root afterwards. This is a deliberate decision (not an open question): without it root is uncoverable, which is the gap the chart version already had.

D5 — Restore interplay

A restore into a copy is "rotation without a bump": the recovered roles carry the source hashes while the Secret would advertise fresh values. The controller detects a restore from the app's restore signal (the bootstrap/restore annotation the backup driver already sets — a generation/annotation, not the counter) and forces a full re-apply of every managed password through the same Job, converging the roles onto the Secret. This reuses the completion path, so the restore case reports the same condition.

D6 — RBAC and blast radius ⚠️ needs API-owner call

The controller needs secrets get/list/watch/create/update in tenant namespaces and the ability to create Jobs there. Kubernetes RBAC cannot scope secrets by name, so a cluster-wide grant lets the controller read every Secret in every tenant namespace — a single component that aggregates all database passwords, which nothing does today. Recommendation: run it with a narrowed, per-tenant-namespace RoleBinding set (managed the way other per-tenant system access is), a dedicated ServiceAccount, and an egress policy restricting it to the tenant DBs. The residual trade-off (the controller can, by construction, reach every managed credential) is inherent to centralizing rotation and should be accepted explicitly or scoped further.

D7 — Bootstrap ordering on a fresh install

The controller creates <release>-credentials on first reconcile of the app CR, before the database operator reaches datadir bootstrap. For MariaDB (root applied only once, at bootstrap) this ordering is mandatory; the controller reconciling the app CR on creation, and the DB operator waiting on the referenced Secret, gives the required order.

API surface

• spec.passwordRotation: int on the app CR — the trigger (read by the controller).
• status.conditions on the app CR: CredentialsRotation with reasons Pending / Applied / Failed, plus observedRotation echoing the last applied counter — this is the completion signal and the durable baseline in one place.
• The controller-owned Secret carries the baseline annotation (last applied counter) so a first bump is never spent establishing state.

Security boundary (stated, not overclaimed)

Rotation removes the retention of a revoked password through Helm's MaxHistory revisions. The live Secret still exists in etcd, in etcd backups, and is readable by tenant-scoped subjects that already have get on it. Rotation revokes at the database level (new password set) and overwrites the Secret; it does not terminate existing sessions — a leaked password keeps working on an open connection until it closes, so a full leak response also needs a session kill (pg_terminate_backend / KILL). Account coverage is per-engine: the postgres superuser is CNPG-managed (<release>-superuser, out of scope); MariaDB root is in scope via D4.

Testing

envtest for the controller reconcile (adoption, rotation, restore-detect, failure→condition); unit tests for the per-engine Job rendering; an e2e that bumps the counter on a live release and asserts the old password stops authenticating and the new one works, and that a restore re-applies.

Remaining for API-owner sign-off

1. D3: ephemeral apply-Job (recommended) vs controller-held DB connection.
2. D6: per-tenant-namespace scoping (recommended) vs cluster-wide, and whether the aggregate-credential-access trade-off is acceptable.
3. New packages/system controller + the spec.passwordRotation field returning to the API (the same API-owner gate the rest of this epic carries).

Once D3 and D6 are decided I can formalize this as a design-doc PR in this repo and start the implementation incrementally (controller skeleton + adoption first, then per-engine apply, then restore-detect).

[scooby87]

Superseded by the design-proposal PR #72, which folds in the resolved design and the review from this thread. Continuing the discussion there.