[myasnikovdaniil]

Adds design proposal for user secrets API: platform generates credentials of managed applications on request, shows plaintext once in the response and stores only what engine needs.

Today charts generate passwords at render time and keep them in <release>-credentials, which every use member and ServiceAccount of every ancestor tenant can read again at any time, so deployment that needs one-time disclosure cant offer managed databases at all. Proposal moves generation into cozystack api. mint on per-account Credential generates password, writes into account Secret what engine needs and returns plaintext once, lost response is answered by another mint. Mint and revoke are for admin and above.

Platform accounts (mariadb root, clickhouse backup user, opensearch admin) stop being visible to tenants. Attribution comes from kube-apiserver audit log that cozystack already collects, and small record per account shows where live password came from and who changed it recently. It does not say whether engine applied a mint, that is an open question.

Kind declares its accounts once in spec.credentials of its ApplicationDefinition, which tenant cannot edit. API knows four password formats and no engines or kinds. format and account sources are structs with kind from the first release, so webhook formats and account sources can be added later without breaking the block. They are named in the proposal and not built.

First wave is postgres, mariadb and clickhouse, where account Secret holds only verifier. Waves 2 and 3 get same mint with plaintext kept in account Secret that no tenant can read, wave 2 engines move to verifiers later as separate work. Conversion is a numbered platform migration (pre-upgrade hook and a Job), and it needs a platform variant with migrations enabled.

example.md shows what definition, chart and API write for postgres, clickhouse and a kind outside the tree, and walks one postgres application through whole cycle.

Open questions are the engine applied signal and platform passwords in Helm history.

Tenant supplied secrets are #82. Supersedes #72, builds on cozystack/cozystack#4078 and closes cozystack/cozystack#4164 for converted kinds.

Before review

• New proposal, not revision of merged one, so no decision record needed.

DCO

• Commits are signed off (git commit --signoff).

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 57c2342f-98d6-4475-be24-acd6ab772e5a

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[scooby87]

NOT LGTM — direction review, request changes.

This is a careful, unusually well-defended proposal: most of the obvious attacks on it are already parried in the text (SSA managedFields non-release on template removal, consume-before-send one-time semantics, an explicit trust boundary instead of a wished-away one). I verified the factual base against the tree at the referenced baseline and it holds — the four disclosure routes, the tenantresource=true marker, the render-time lookup+randAlphaNum generation, and the postgres SQL-applier (no CNPG managed-roles, so no two-writer race) are all described accurately.

One gap does not close, and it sits in the headline guarantee, so it blocks approval of the boundary rather than being a detail: one-time disclosure and attributable audit do not extend to the Phase 1 primary delivery flow, and the gap admits a principal the proposal itself places outside the trust boundary. Detail is inline on details.md:156.

The rest are non-blocking — clarifications and cross-proposal reconciliation the author already invites. They are posted inline where they anchor.

Still strong (verified against code)

The four routes are named precisely: render-time generation in packages/apps/postgres/templates/init-script.yaml (lookup + randAlphaNum 16 fallback, rendered into <release>-credentials), the tenantsecrets projection and its internal.cozystack.io/tenantresource=true marker in pkg/registry/core/tenantsecret/rest.go (an ApplicationDefinition really does select on it), the per-release raw grants, and Helm history. The CSPRNG-with-≥32-chars requirement is a real improvement over today's non-cryptographic 16-char randAlphaNum. The managedFields caution is correct and implementable (the controller force-takes ownership once the chart stops rendering). Bootstrap-cycle awareness for the MariaDB admission gate is right. Credit where the design refuses to overclaim — it does that repeatedly.

[scooby87]

[MAJOR] One-time disclosure and attributable audit do not hold for the Phase 1 delivery flow, against an out-of-boundary principal

The headline guarantee is scoped to "tenant-facing interfaces" (README.md:10), and the trust boundary places tenants, their other members, and their ancestors outside it, with only "authorized workload operators" inside (README.md:231, details.md:359). Phase 1 delivery, though, writes a fixed-layout plain Secret into a namespace of a managed Kubernetes cluster the tenant administers (this line; README.md:190), and the tenant owns RBAC inside that cluster.

The custody carve-out only names a person who can "change a consuming image, exec into its process, read its memory, or administer its VM" (README.md:157, details.md:160). But get secrets in the destination namespace is a distinct RBAC fact from exec / image-change / VM-admin. So a tenant member with get secret there but without workload-modify rights — a read-only auditor, a use-tier member, a CI ServiceAccount scoped to secret-read — is simultaneously (a) outside the trust boundary per the tenant-member exclusion, (b) denied collect by the authorization table (use = No), and (c) able to read the delivered credential repeatedly via kubectl get secret. Even the tenant admin, who may collect once through the API, reads the delivered copy without limit. The mechanism therefore does not enforce the boundary it declares.

The second core guarantee falls in the same gap: reads of the delivered Secret happen in the tenant's cluster and are invisible to the platform journal, so "every collection is recorded" (README.md:55) does not cover them. Two stated invariants fail for the same Phase 1 row that details.md:234 marks mandatory.

This blocks approval of the boundary because the proposal asks reviewers to approve the invariants and the surfaces, and a load-bearing invariant is defined against a boundary the delivery adapter cannot enforce. The fix is contract-level, not code: pick one and write it down — (a) state that one-time and audit are platform-API properties that do not bound managed-cluster-side exposure, and reword each "closes the route" claim to match; (b) require the delivery adapter to constrain destination RBAC or deliver through a projection the tenant cannot get (hard in the tenant's own cluster); or (c) widen the custody carve-out to anyone holding get secret in the destination namespace and reconcile the trust-boundary sentence that currently puts tenant members outside.

[myasnikovdaniil]

I don't think this holds, and I'm not rewording the guarantees for it.

The principal isn't out of boundary. The sentence you cite puts "cluster administrators, and whoever controls a consuming workload" inside the boundary (README.md:231; details.md:359 says the same with "authorized workload operators"). In a managed cluster the tenant administers, the tenant is the cluster administrator, and anyone holding get secret in the destination namespace holds it because that administrator granted it — so that reader is inside the boundary for that copy. The "tenants, their other members, and their ancestors are outside" clause is about the management-cluster custody points: platform-side raw reads, the projection, per-release grants, Helm history. Those are the four routes, and those are what the proposal closes. Reading the clause as governing RBAC inside the tenant's own cluster is what produces the contradiction; the text claims no authority there.

One-time disclosure is already an API property, not a custody claim. README.md:12 says it bounds what tenant-facing interfaces hand out and explicitly does not claim the platform cannot recover a password or that a recipient forgets it; details.md:164 says the platform cannot claim control of a copy that landed elsewhere. There is no unmet claim to reconcile and no "closes the route" sentence that overreaches.

The audit claim is defined over a closed catalogue of platform operations (README.md:202, details.md:253): issuance, collection, regeneration, revocation, delivery. collection is the named API operation. A kubectl get secret in the tenant's cluster is not a collection event, and the proposal never claims to observe reads at a destination — it claims to record the delivery, which it does.

Mechanically: whoever can read a destination's runtime material holds the credential. That is true of every delivery mechanism — the same-namespace SecretRef consumer, ESO, CSI, an env var, a mounted file. It is a property of delivery, not of this destination, and no wording makes delivery secret from the destination. The delivered copy is the tenant's own credential landing in a cluster the tenant already administers and where the tenant already reads every Secret, and the channel isn't new: Flux already applies every addon HelmRelease there through <release>-admin-kubeconfig / super-admin.svc (packages/apps/kubernetes/templates/cluster.yaml:229).

If your actual objection is that README.md:231 reads ambiguously about whose cluster administrators it means, say so and I'll treat that as the finding — that's one sentence, not the boundary. What I won't do is narrow the delivery scope. Tenants don't run pods in their own namespace, so removing this row leaves phase 1 with no path at all from a generated credential to a tenant's own workload, which is the gap that naming a first-wave consumer was meant to close.

[scooby87]

You're right, and I'm withdrawing this. The delivered credential is the tenant's own, landing in a cluster the tenant administers — the owner reading their own password in their own runtime is the credential working, not a leak. One-time is an API property by construction (README.md:12), the audit claim is over the named collection operation, and no delivery mechanism (SecretRef, ESO, CSI, a mounted file) audits destination-side reads.

The only residue is the one you already offered to take: README.md:231 names "cluster administrators" unqualified, which is exactly what let me misread it as the tenant's own workload cluster — one clause naming which cluster's administrators the boundary means would close it.

[scooby87]

[MINOR] Phase 1 completion is gated on COSI revocation the proposal itself flags as unproven

This section and the Open Questions concede that if the pinned COSI driver cannot request and verify replacement/revocation, the row is unavailable for strict collection and its absence "blocks completion of the first wave." That sits against the rule that a phase is complete only when all rows agreed for it pass (README.md:198, details.md:225), so the Phase 1 milestone hangs on a capability many COSI drivers do not implement. Either move COSI to a later wave, or write an explicit carve-out for adapter-blocked rows so a single immature external driver does not stall the milestone.

[myasnikovdaniil]

The carve-out you're asking for is already the rule. A phase is complete when all rows agreed for that phase pass (README.md:198, details.md:225), the table is explicitly a proposal open to redistribution, and an absent adapter "reports its capabilities as false and rejects a mutation before staging live material". An adapter that can't prove replacement and verified revocation therefore isn't an agreed row, and its absence doesn't hold the milestone.

The README.md:263 line you're reading as a gate is an Open question, and it says the absence blocks completion of the first wave as currently proposed — which is what an open question is, and the resolution is exactly the redistribution the rules above already allow.

It also isn't "many COSI drivers". It's one pinned image, ghcr.io/seaweedfs/seaweedfs-cosi-driver:v0.3.1 (packages/system/seaweedfs/values.yaml:255-258), already pinned above the vendored v0.1.2 default because that one ignored accessPolicy. What that build does on revocation has one answer, and I'd rather get it than move the row pre-emptively.

[scooby87]

Fair — "many COSI drivers" was wrong; it is the one pinned seaweedfs-cosi-driver:v0.3.1, and "rows agreed for that phase" plus "an absent adapter reports its capabilities as false" is the carve-out I asked for. Withdrawing.

One optional nit: README.md:263's "absence blocks completion of the first wave" still reads as a hard gate against that rule — a half clause ("as currently proposed; the resolution is the redistribution the rules already allow") would remove the contradiction-read.

[scooby87]

[MINOR] No non-disruptive break-glass read after the collection window

For a GitOps or headless flow the credential is generated and delivered by a binding and never collected by a human. After the window (default 24h) the only way for an operator to obtain the password by hand — emergency access, an external tool — is regeneration, which rotates the live password and can break pooled consumers. Today an operator can always read <release>-credentials. This is a deliberate tradeoff, but the day-2 consequence (no non-disruptive break-glass) is significant enough to state explicitly in User-facing changes or Open questions rather than leave implicit.

[myasnikovdaniil]

This isn't right as stated — regeneration is not the only manual route.

The platform holds the active version's material; it has to, since delivery follows the active version and the controller writes destination copies from it (details.md:157). README.md:12 deliberately does not claim the platform cannot recover a password, details.md:291 treats retained authentication material as an operator-visible retention exception, and details.md:281 already names "emergency platform-admin intervention" as a separately audited operational procedure. So a platform operator keeps a non-disruptive read after the window closes.

What the window ends is the tenant-facing entitlement — the point of one-time collection — and that consequence is stated where it belongs (details.md:97: expiry leaves the credential usable by its workloads but not collectible, and an administrator cannot reopen a spent version). The <release>-credentials read you're comparing against doesn't disappear either; it stops being reachable through tenant-facing routes. That's the intended delta, and since the document promises tenant-side break-glass nowhere, there's no overclaim to correct.

[scooby87]

Agreed — the platform-operator read stands (details.md:281, :291), so "regeneration is the only route" was wrong at that level. What remains is purely tenant-facing: "read a password I already have running" is not an operation this API offers a tenant, and the GitOps/headless path makes that the common case. That is a consequence worth surfacing in User-facing changes, not a defect. Withdrawing as a finding.

[scooby87]

[MINOR] Cross-namespace finalization on tenant/namespace deletion is under-specified

The record lives in a platform namespace, bound to the tenant-namespace UID and HelmRelease UID through explicit UID mappings and controller finalization, never cross-namespace owner references. That leaves two footguns unaddressed: on a forced tenant-namespace deletion with the controller down, platform-namespace Secrets holding candidate bundles leak; and a finalizer the controller cannot clear can wedge the namespace in Terminating. Specify where the finalizers sit and the behavior when the controller is unavailable.

[myasnikovdaniil]

Agreed, this one is a real gap. details.md:8 says cross-namespace relationships use UID mappings and controller finalization, and details.md:340 requires that namespace deletion not erase the platform-side records before cleanup resolves — but neither says where the finalizers sit nor what happens while the controller is unavailable, and "issuer outages leave visible finalization failures" covers a different case.

I'll specify both. Finalizers sit only on the platform-side records, never on the tenant namespace, its Secrets, or the HelmRelease, so an unavailable controller cannot wedge a namespace in Terminating — your second footgun becomes structurally impossible rather than handled. That leaves the first as a recovery path: a forced tenant-namespace deletion with the controller down leaves platform-side records and candidate bundles with nothing referencing them, and on recovery the controller resolves a record whose bound tenant-namespace UID no longer exists and runs the details.md:340 deletion path — fence executors, revoke or remove engine accounts, then remove controller-owned material — instead of reading a missing namespace as MaterialMissing.

[scooby87]

Thanks — your resolution closes it. Finalizers only on the platform-side records (never the tenant namespace, its Secrets, or the HelmRelease) makes the Terminating-wedge structurally impossible, and having the dangling record run the details.md:340 deletion path instead of reading a missing namespace as MaterialMissing handles the leak. Keeping this as the accepted change; nothing further from me once it lands in the text.

[myasnikovdaniil]

Already in a381944.

[scooby87]

[MINOR] Name the resource-policy:keep teardown-hang as a required handoff test

Stage A adds helm.sh/resource-policy: keep to credential Secrets. There is prior art in this org (the Kafka ZK->KRaft migration) where keep hung helm uninstall / app deletion by orphaning resources that held teardown open. The acceptance plan covers this only under a generic "uninstall" bullet; given the precedent, make the uninstall / teardown-hang interaction an explicit case of the pinned handoff test rather than folding it into the general list.

[myasnikovdaniil]

The text already requires this, and the precedent isn't Kafka.

details.md:315 doesn't fold uninstall into a generic bullet — it requires acceptance to exercise "pruning, uninstall, field transfer, release retry, interrupted handoff, and the pinned helm-controller behavior", which is the pairing you're asking for, and details.md:405 lists pruning/uninstall again in the upgrade/rollback row.

On the prior art: Kafka's ZooKeeper→KRaft migration used a Helm pre-upgrade Job gated by a deployed-version ConfigMap (422d54ee2, templates/migration-hook.yaml), and resource-policy appears nowhere in packages/apps/kafka/ in its history. The matching case is SeaweedFS — platform migration 43/53 stamps helm.sh/resource-policy: keep on the filer-metadata CNPG Cluster so a <name>-system upgrade can't prune it, and because keep is permanent it also survives the <name>-db release's own uninstall, leaving the Cluster, its Postgres pods and its PVCs orphaned; that needed a dedicated reclaim hook (packages/extra/seaweedfs/templates/hooks/cleanup.yaml, covered by tests/cleanup_test.yaml). packages/extra/etcd/templates/datastore.yaml carries the same annotation on its TLS Secrets.

Useful to know, and it confirms the requirement that's already there — the failure was silent orphaning rather than a hung uninstall.

[scooby87]

You're right on both counts: details.md:315 already pairs uninstall with the pinned-helm-controller behavior, and the precedent is SeaweedFS (migration 43/53 stamping keep, surviving the <name>-db uninstall, orphaning the CNPG Cluster and its PVCs, needing a reclaim hook) plus etcd — not Kafka. Withdrawing the "name it as a test" ask.

That same SeaweedFS mechanism does surface a separate wording issue on the Stage-B ownership gate, which I have raised as a new comment on README.md:216.

[scooby87]

[MINOR] Elevate the fleet-wide-controller trust concentration into the decision table

The design is honest that per-namespace RoleBindings do not partition the controller's compromise scope. For a platform whose value is tenant isolation, concentrating all credential-generation authority in one controller is a first-order architectural decision and a new highest-value target — today no single component holds every tenant's plaintext-generation authority. It currently lives in Model prose; it belongs in the Proposed choices table alongside Ownership and Audit so reviewers weigh it as one of the choices to argue.

[myasnikovdaniil]

The premise is wrong, so moving the statement would put a true paragraph under a false heading.

"Today no single component holds every tenant's plaintext-generation authority" — one does. Generation happens at Helm render time (packages/apps/postgres/templates/init-script.yaml:1,14, lookup plus randAlphaNum 16), and the identity that renders and applies it for every tenant release in every tenant namespace is Flux, bound to cluster-admin (packages/core/flux-aio/manifests/fluxcd.yaml:7831-7843). The concentration is the status quo. This proposal moves that authority off a cluster-admin general-purpose controller onto a dedicated identity with per-applier material and target limits — a reduction, not a new highest-value target.

details.md:23 already states the load-bearing part: fleet-wide material access, per-namespace RoleBindings not partitioning compromise scope, tenant-isolated instances as a possible later deployment option and explicitly not a claimed security property. The Proposed choices table is for choices where a reviewer picks between alternatives; presenting a reduction there as a new concentration would misinform the reviewers it exists to inform. If you want it in the table anyway, argue it against today's cluster-admin renderer and I'll add it in those terms.

[scooby87]

This is the strongest correction, and you are right — my premise was false. Flux at cluster-admin already renders and applies every tenant's randAlphaNum password across every namespace, so the fleet-wide concentration is the status quo and this proposal reduces it onto a dedicated identity with per-applier limits. It does not belong in the choices table as a new concentration. Withdrawing.

[scooby87]

[MINOR] Reconcile the rotation-trigger model with the #4078 dependency

The proposal states there is no counter in application values that triggers rotation (README.md:122, details.md:89), while #4078 — named here as a dependency — adds a passwordRotation mechanism through a Secret annotation marker, which is arguably the same class of external marker-trigger. In the current tree #4078 is not merged (inline users[].password is still present in both charts), so there is no in-code conflict yet, but the direction needs settling: does this proposal inherit, deprecate, or supersede #4078's rotation path with CredentialOperation Regenerate?

[myasnikovdaniil]

#4078 doesn't add a rotation trigger, it removes one — there's nothing to reconcile.

From its own description: a password or passwordRotation key left in a tenant's values is accepted-but-inert with an admission warning, and rotation is deliberately out of scope there because a chart-rendered credentials Secret keeps its cleartext in Helm release history for MaxHistory revisions, so rotation built on it cannot revoke a leaked password — it defers that to a controller that owns the Secret. passwordRotation isn't a field anywhere in main today.

So README.md:122 and details.md:89 ("no mutable counter in app values acts as a rotation trigger") agree with #4078 rather than conflicting with it. The dependency is what the PR description says it is: #4078 drops inline users[].password, this proposal supplies the lifecycle, and CredentialOperation Regenerate is the mechanism #4078 points forward to.

[scooby87]

Correct — #4078 removes an inert accepted-but-warned key rather than adding a rotation trigger, so README.md:122 and details.md:89 agree with it rather than conflicting. Nothing to reconcile. Withdrawing.

[scooby87]

[NIT] Clarify the API-version story across the five new kinds and #73

All five kinds are shown as core.cozystack.io/v1alpha2, including the brand-new Credential. Explain why v1alpha2 rather than v1alpha1 for genuinely new resources, and how this composes with the v1 transition in #73, which is said to have to carry the reference and removed-field changes.

[myasnikovdaniil]

details.md:7 answers the first half: v1alpha2 distinguishes the changed TenantSecret contract and is a proposed version, not an existing endpoint. It carries the new kinds because the group moves as a unit — core.cozystack.io serves only v1alpha1 today (pkg/apis/core/v1alpha1), and TenantSecret's write-only shape isn't compatible with what v1alpha1 serves, so the group takes a new alpha version and the new kinds are born in it rather than splitting the group across two alpha versions for the length of the transition. #73 then carries all five to v1 together, which is where the reference and removed-field changes land.

I've kept #73 out of the document text on purpose while it's unmerged and its shape can still change; the cross-reference lives in the PR description.

[scooby87]

That resolves it — the group moves as a unit to v1alpha2 because TenantSecret's write-only shape is incompatible with the served v1alpha1, and #73 carries all five kinds to v1. Withdrawing. Only suggestion: that rationale currently lives only in this reply; one sentence in the doc would pre-empt the same question from the next reader.

[scooby87]

[MAJOR] README.md:216 and details.md:311 describe the Stage-B ownership step with opposite mechanics, and since README.md:12 makes this document "the contract to review," the contradiction sits in the contract itself.

README states it as a precondition the coordinator confirms: "confirms the Secret was orphaned rather than pruned and that Helm released managedFields ownership, and only then the controller adopts." details.md:311 states the opposite: "Do not assume removing a template automatically releases SSA ownership. Only after Helm no longer renders or manages those bytes may the migration controller perform a … ownership transfer."

Grounded in this repo's own migrations: helm.sh/resource-policy: keep gates pruning only — on template removal Helm leaves the kept object's managedFields manager entry and meta.helm.sh/release-name intact (migration 56:175: "Helm's prune gates ONLY on helm.sh/resource-policy=keep — it does not consult the live meta.helm.sh/release-name"). Ownership is force-transferred, never released: migration 50:524 does del(.metadata.managedFields, …) | kubectl apply --server-side --force-conflicts. Under the default helm-controller SSA mode (postgres carries no helm-server-side-apply override in-tree), a coordinator waiting to confirm that Helm released managedFields ownership never sees it released, and Stage B blocks adoption for every app.

The design itself is sound — details.md:311's force-transfer is exactly what migrations 50/56 already ship. The fix is a README↔details reconciliation: drop "Helm released managedFields ownership" as a confirmable precondition and describe the recorded, force-conflicts ownership transfer the controller performs, as details already does.

[myasnikovdaniil]

You were right, README contradicted details here. Fixed in fe2ad72, README now describes the same forced-conflicts transfer as details.md:311 and doesn't wait for helm to release managedFields anymore, keep gates pruning only.

[scooby87]

Reconciliation after myasnikovdaniil's replies. Net: my original blocker is withdrawn, but request-changes stands on a different, code-grounded basis.

Withdrawn after the rebuttals (all correct): the delivery / one-time-disclosure MAJOR (details.md:156), COSI phase gating (details.md:213), break-glass (details.md:97), fleet-wide-controller placement (details.md:23), the #4078 rotation-trigger reconciliation (README.md:19), and the API-version question (details.md:7). The resource-policy: keep item (details.md:309) is also withdrawn — the requirement is already in details.md:315 and the precedent is SeaweedFS, not Kafka.

Still open:

• [MAJOR] Stage-B ownership handoff: README.md:216 states "confirm Helm released managedFields ownership" as a precondition, which details.md:311 explicitly says not to assume and which this repo's own migrations 50/56 show never happens (keep gates pruning only; ownership is force-transferred). Since README.md:12 makes README the contract under review, the contradiction is contract-level. Detail inline on README.md:216.
• [MINOR, accepted] Cross-namespace finalizer placement on tenant/namespace deletion (details.md:11) — agreed and to be specified.

Keeping the review at request-changes on those two; happy to flip once README.md:216 is reconciled onto the details.md mechanism.

[IvanHunters]

Verdict

NOT LGTM

Three statements in the contract don't hold as written. The design under them is careful and mostly refuses to overclaim, which is why the few places it does overclaim are worth naming rather than letting through.

Two of the three are in the paragraph fe2ad72 added. So the fix for one review item shipped a sentence that claims more than you argued for yourself earlier in this thread.

The Stage-B ownership handoff that was blocking at the previous head is closed here, and I checked the text rather than taking the reply for it: the README now describes the same forced-conflicts transfer as details.md:311, and says the transfer is performed rather than awaited. I haven't re-raised the six items withdrawn earlier in the thread. On details.md:7 I'm adding the code check nobody ran, not reopening the conclusion.

Findings

• [MAJOR] design-proposals/user-secrets-api/README.md:212, two claims in this sentence are not supported by the document
• [MAJOR] design-proposals/user-secrets-api/details.md:319, strictOnly declines the only destination phase 1 delivers to
• [MAJOR] design-proposals/user-secrets-api/README.md:218, the per-adapter deadline has no per-tenant enforcement point
• [MINOR] design-proposals/user-secrets-api/details.md:7, the v1alpha2 rationale rests on a constraint the server does not impose
• [MINOR] design-proposals/user-secrets-api/README.md:164, Failed and Revoked are terminal, but the prose requires exits from both
• [MINOR] design-proposals/user-secrets-api/details.md:68, no read verb for credentialbindings, which three other places require
• [MINOR] design-proposals/user-secrets-api/details.md:11, plaintext residue when the controller is down at application delete
• [MINOR] design-proposals/user-secrets-api/details.md:281, fail-closed journal threshold has no stated scope
• [MINOR] design-proposals/user-secrets-api/README.md:33, "There is no application UID to owner-reference"

Still open

• Two things I did not verify, both outside this repo: the dashboard pre-fetch claim (README.md:37), and MariaDB ACCOUNT LOCK surviving an operator restart, which you already carry as an open item.
• What I re-derived against 67b4e23ca holds: all four disclosure routes, tenantsecrets returning whole data and stamping the outward marker on write, the ancestor-ServiceAccount grants on raw credential Secrets, Kafka having no user surface, and every link to #37 / #71 / #72 / #73 / cozystack#4078.

[IvanHunters]

[MAJOR] two claims in this sentence are not supported by the document

This sentence is new in fe2ad72 and makes two claims the rest of the document does not support.

"A tenant cannot re-read a password that is already running." The first sentence of this same paragraph says a binding delivers the credential into a namespace of the tenant's cluster, and README.md:231 says a tenant's own managed cluster is that tenant's runtime, where "whoever the tenant lets read a delivered copy there holds the credential". A local admin is granted "Within approved destinations" under Authorize delivery (README.md:146) plus edit authority on the destination (details.md:156), so after the collection window they create a CredentialBinding into a namespace of their own cluster, read the fixed-layout Secret with their own cluster access, and delete the binding. That is a repeatable read of a running password, initiated by the tenant, through tenant-facing routes, with no regeneration. The justification clause fails for the same reason: POST credentialbindings is one of those routes, and it still yields the material.

The precise formulation is already yours, twice: README.md:12 scopes one-time disclosure to what tenant-facing interfaces hand out, and details.md:164 says the platform cannot claim control of a copy that landed elsewhere. This sentence overshoots both, and it does so in the section a deployment reads to decide whether the platform meets its policy.

Aggravating it: "approved destinations" is what bounds that path, and nothing defines it. It appears at README.md:54, README.md:146 and details.md:139 with no approval object, no policy field, and no named approver.

"A platform operator keeps an audited, non-disruptive read as an operational procedure." This is its only occurrence in either file. The event catalog is explicitly closed (README.md:202, details.md:253) and carries no operator-disclosure class; the authorization tables have no row for it; details.md has no procedure for it. An operator reading the version Secret directly is covered only by kube-apiserver audit, which README.md:38 says nothing collects today, and details.md:277 concedes the journal supplements API audit rather than replacing evidence of raw access. So the stated recovery path for what this sentence itself calls the common case produces nothing in the attributable trail the proposal is selling. Give it an operation, an event class and an authorization row, or drop the word "audited".

[myasnikovdaniil]

Both claims removed. Sentence now says only what tenant-facing routes hand out, and states the route you found: an admin with source use plus edit authority on the destination binds a delivery and reads the copy there, Security already gives that to tenant runtime, and it is recorded as a delivery event. "Approved destinations" is gone from all three places, there was no approval object behind it, so they carry the rule from the delivery section instead.

On the operator read I dropped the whole clause rather than give it an event class. Management cluster admins are inside the trust boundary by this document own definition, and Context already says platform collects no api audit at all, so an event class for an action that does not pass through this api and that we cannot restrict would be claiming a control we don't have.

[IvanHunters]

[MAJOR] strictOnly declines the only destination phase 1 delivers to

strictOnly "makes unconverted secret-bearing services unavailable rather than legacy", and details.md:217 gives the consequence for managed Kubernetes: such an installation declines that class rather than create a cluster whose owner has no access. That app is secret-bearing (packages/apps/kubernetes/templates/dashboard-resourcemap.yaml:18 grants {{ .Release.Name }}-admin-kubeconfig), and you place its adapter after phase 1 (README.md:190).

Now set that beside two other statements in this document. details.md:156: tenants cannot run pods in their own namespace, so their workloads live in managed clusters and VMs. README.md:194: the proposed phase 1 rows are PostgreSQL and MariaDB users, COSI bucket keys, and delivery into a namespace of the tenant's managed Kubernetes cluster, and details.md:234 marks VM guests, other tenants and external stores unsupported.

A strictOnly phase-1 installation therefore declines the only destination phase 1 delivers to. What the tenant is left with is a 24-hour collection window and, after it, regeneration of a password with nowhere to go. The one other consumption form you name, a co-located managed application through its SecretRef (README.md:155), is qualified "where the consumer adapter exists" and is not among the phase 1 rows, so it does not close the gap as written.

Second, the two files disagree on whether declining is what happens or one option among several. README.md:190 states the outcome: existing admin access stays Legacy "or the installation declines that unsupported class under strictOnly". details.md:217 makes it discretionary: such installations "may decline that unsupported class". Since README.md:12 makes this document the contract, an implementer gets two different answers to "can a strictOnly deployment create a tenant Kubernetes cluster at all".

One of three has to give, and the proposal should say which: strictOnly keeps managed Kubernetes as an explicit exception, phase 1 gains a second delivery destination, or the option ships only once the Kubernetes adapter does.

[myasnikovdaniil]

Confirmed, chart does grant it.

I took a fourth option instead of your three, because the two admin routes need different classification. oidc exec-only kubeconfig carries no token and no private key, cluster-admin comes from a ClusterRoleBinding keyed on the email claim and the bootstrap job prunes stale ones, so it conforms already and phase 1 just classifies it and adds nothing. Static super-admin kubeconfig is break-glass by the chart own notes, stays Legacy, and strictOnly now declines only a cluster whose single admin route is that certificate. may/does split is gone.

Checking this turned up something that changes the open question: kamaji KubeconfigGenerator crd is already installed by the platform, takes the certificate common name and organisations as fields, and is created by no chart. So retirement has two candidates, not one, and they are not equal, ca rotation rejects the old certificate authentication while a generator identity can be de-authorized by deleting a binding with the certificate still authenticating.

[IvanHunters]

[MAJOR] the per-adapter deadline has no per-tenant enforcement point

"The minor release after the one that ships a service's adapter blocks upgrades for tenants that still hold legacy credentials in that service" reads as a per-tenant block, and nothing in this codebase can express one today.

Platform version is a single cozystack-version ConfigMap in cozy-system, stamped by packages/core/platform/images/migrations/migrations/lib/cozystack-version.sh:35: one per management cluster, with no per-tenant dimension. packages/core/platform/images/migrations/run-migrations.sh walks migrations in sequence and exits non-zero on the first failure (:42, :48) without advancing the stored version. A deadline enforced there means one tenant's unregenerated password freezes every other tenant's platform upgrade on that cluster, with no operator path out short of hand-editing the ConfigMap.

There is also no Go-side consumer of that version to hang a per-tenant gate on, at 37e22dc3:

$ grep -rn "cozystack-version" --include="*.go" pkg/ cmd/ internal/ | wc -l
0

This is the shape that has bitten cozystack before: a fleet-wide hook that can exit non-zero on one tenant's stored state deadlocks the whole fleet. The working form is warn-pin-skip: record the offending tenant, report it, let the rest advance.

Name the layer that enforces the deadline (platform migration, admission on the application, something else), and require it to degrade per tenant rather than fleet-wide.

[myasnikovdaniil]

Confirmed on every point.

Deadline is enforced against the per-account credential records now, they already carry a tenant binding, and it is warn-pin-skip. I also named what gets refused, because "blocks upgrades" was doing the same work "approved destinations" was doing elsewhere: creating or updating an application of that service kind in that tenant, until it regenerates. Running workloads untouched. Rollout step 4 still had the old fleet-wide wording, fixed too.

[IvanHunters]

[MINOR] the v1alpha2 rationale rests on a constraint the server does not impose

The rationale for putting the new kinds in v1alpha2 rests on "the group moves as a unit", and the aggregated server does not impose that. pkg/apiserver/apiserver.go:238 installs core storage as coreApiGroupInfo.VersionedResourcesStorageMap["v1alpha1"] = coreV1alpha1Storage, a map keyed by version, so a v1alpha2 entry can serve tenantsecrets and the Credential* kinds while tenantnamespaces, tenantmodules, options and taps stay v1alpha1-only. Kinds sitting at different versions inside one group is ordinary Kubernetes (autoscaling/v1 beside v2).

This was raised and withdrawn earlier in the thread on the strength of that sentence, so I am adding the part that was not checked rather than reopening the conclusion.

Worth noting where it leads: serving v1alpha2 for only the changed and new kinds is splitting the group across two alpha versions, which is the outcome the rationale says it avoids. Genuinely moving the group as a unit means v1alpha2 also serves the four existing kinds, and the document says nothing about their v1alpha2 shape or conversion, nor is it in Open questions. Since you are asking the API owners to arbitrate exactly this, either narrow the new version to TenantSecret and let the Credential* kinds be born in v1alpha1, or state the v1alpha2 contract for every kind in the group.

[myasnikovdaniil]

Rationale is gone, the server does not impose it.

Not settling it in either direction. Text says only what the incompatibility forces, write-only TenantSecret needs a version of its own, and both options are in open questions for you and the other api owners.

[IvanHunters]

[MINOR] Failed and Revoked are terminal, but the prose requires exits from both

The diagram gives Failed and Revoked no outgoing edge, and prose elsewhere requires an exit from both.

Usable --> Prepared: explicit regeneration followed by Prepared --> Failed: known failure before mutation strands an account whose old password still works and whose workloads are still running in a terminal state, with no edge back to Usable and no retry edge.

details.md:207 requires, for a revoked MariaDB account, that "Regeneration unlocks the account together with the verified new password", a Revoked --> transition the diagram forbids. The failure table's "recover when valid" row (details.md:369) needs the same thing from a blocked bootstrap.

Since details.md:59 derives capabilities from lifecycle state, the diagram is normative rather than illustrative, so the missing edges are a specification gap and not a drawing detail.

[myasnikovdaniil]

Agreed, and it is a spec gap rather than a drawing detail since capabilities derive from lifecycle state. Both diagrams carry Failed --> Usable, Failed --> Requested and Revoked --> Prepared now, and prose says Failed is only reachable before an engine mutation, which is what makes the first one safe.

[IvanHunters]

[MINOR] no read verb for credentialbindings, which three other places require

The interface table lists only POST/PUT/DELETE credentialbindings, with no GET/LIST/WATCH row, and three other places require reading them:

• README.md:54 promises observable delivery state as a goal;
• delivery records deliveredVersion and adoptedVersion (README.md:155, details.md:158), which someone has to observe;
• the authorization tables grant workload identity "Binding metadata" (README.md:150, details.md:143), a read that no listed verb serves.

As written, a tenant asking "did my cluster adopt the rotated password" has no endpoint to ask. The README interface table (README.md:115-120) additionally omits the GET/LIST/WATCH credentialoperations row that details.md:65 does carry, while README.md:210 specifies a create-poll-collect sequence against it.

[myasnikovdaniil]

Added to both tables, plus the credentialoperations read row README was missing.

[IvanHunters]

[MINOR] plaintext residue when the controller is down at application delete

Placing finalizers only on platform-side records is the right call for tenant deletion, and it closes the wedge case discussed earlier in this thread. Application deletion after the handoff is a different sequence and is not covered.

After stage B, <release>-credentials is controller-owned and still carries helm.sh/resource-policy: keep (README.md:216). Delete the application and Helm is forbidden to prune that Secret, no tenant-side finalizer holds the deletion, and only the controller is obliged to clean it up (details.md:340). A controller that is down or wedged at that moment leaves live plaintext sitting in the tenant namespace, with "visible finalization failures" as the only signal.

Each half is stated; the combination is not. One line on how long that residue may persist, and what reconciles it when the controller returns, would close it.

[myasnikovdaniil]

Right, each half was there and the combination was not. Controller drops the keep annotation when it adopts the object, it only ever gated helm pruning and helm no longer renders the secret, and removal hangs off the finalizer on the platform-side record. So a controller that is down delays it without losing it.

[IvanHunters]

[MINOR] fail-closed journal threshold has no stated scope

Two rules combine into an availability coupling whose blast radius is not stated.

details.md:257: API denials and discovery requests that do not mutate a record "use the same durable journal directly", and sampling of repetitive discovery traffic is explicitly optional and not required of the initial controller (README.md:204, details.md:255). This line: admission fails closed with 503 AuditUnavailable when the journal buffer "reaches the reserved capacity threshold", refusing supply and collection first.

So cheap read traffic writes the same durable journal whose fullness switches off disclosure. A view-tier principal, the lowest authority in the table, can loop LIST/WATCH inside the separate discovery rate budget, which details.md:287 scopes to protecting entitlement and health reads rather than journal capacity, and details.md:289 forbids dropping events to make room.

The threshold's scope is never stated: per record, per tenant, or platform-wide. Under the platform-wide reading, one tenant's read traffic suspends supply and collection for everyone. Either name the scope, or say that discovery events may be shed under pressure while the mutating trail stays complete.

[myasnikovdaniil]

Took your second option, plus the scope. Threshold is per tenant, platform-wide fail-closed only when the journal as a whole cannot accept a write. Non-mutating discovery and denial events shed first and the shedding is recorded, the outbox trail never. I had to scope the per-record cap sentence as well, it said an event must never be dropped to make room, so it read as forbidding exactly that shedding.

[IvanHunters]

[MINOR] "There is no application UID to owner-reference"

pkg/registry/apps/application/rest.go:1385 builds the virtual object with UID: hr.GetUID(), so an Application does report a UID: the backing HelmRelease's.

The design is unaffected: details.md:9 already binds records to the tenant namespace UID, the HelmRelease UID and a server-assigned incarnation, which is the right answer. Only this Context sentence is wrong, and it is worth fixing because a reviewer checking your premises hits this line before anything else. The real constraint is something like: the Application reports the backing HelmRelease's UID, so an ownerReference to it would be indistinguishable from one to the HelmRelease.

[myasnikovdaniil]

Fixed with your formulation. Model section carried the same wrong reason.

[myasnikovdaniil]

All nine addressed.

Two of my own claims did not survive checking them. I wrote that the static kubeconfig is a certificate in system:masters and built the no-de-authorization argument on that group, but the tree does not establish it, the chart README documents super-admin.conf only as carrying extended administrative permissions and kamaji generates it rather than kubeadm. Which group it carries is a stated prerequisite now, since it decides whether de-authorization exists for that certificate at all.

Same paragraph called client-CA rotation the only mechanism that retires it. kamaji KubeconfigGenerator crd is installed by the platform today, takes the certificate common name and organisations as fields, and is created by no chart, so there is a second and much cheaper mechanism, issue the tenant-facing credential under a named identity and withdraw its binding. Weaker guarantee though, it withdraws authorization while the certificate still authenticates, which this proposal already makes the evidence distinguish. Both are written up and the choice goes to the api and code owners.

mariadb ACCOUNT LOCK stays open as before. Dashboard pre-fetch claim I will check separately instead of leaving it resting on my own earlier reading.

scooby87 your review still sits at request-changes on two items, I think both are closed now. Stage-B handoff is reconciled onto the details mechanism, README describes the same forced-conflicts transfer and says the transfer is performed, not awaited, and IvanHunters checked the text rather than taking my word for it. The finalizer item you accepted is specified, and the round above added the case it did not cover, deletion of the application after handoff where keep forbids helm from pruning and no tenant-side finalizer holds the deletion.

[Timofei Larkin (lllamnyp)]

Verdict

NOT LGTM, as a direction review against head aebc3c1. The problem statement holds and the four read routes are named correctly; the round-4 fixes are real and I have no quarrel with them. The design over-builds the generated-credential half by an order of magnitude, and the tenant-supplied half belongs in a different document. Both follow from one fact the proposal does not use.

The fact

Every first-wave engine stores a verifier, and every first-wave operator accepts one:

• PostgreSQL stores a SCRAM-SHA-256 verifier and ALTER ROLE ... PASSWORD accepts an already-encoded SCRAM-SHA-256$... literal. CloudNativePG's declarative role management says it outright: passwords "already provided in MD5 or SCRAM-SHA-256 shadow form are forwarded unchanged", and disablePassword sets the password to NULL.
• mariadb-operator's User and MariaDB resources carry passwordHashSecretKeyRef next to passwordSecretKeyRef, and the vendored CRD states the hash is re-applied on change when the Secret carries the watch label.
• The ClickHouse chart already renders password_sha256_hex.

So the platform never needs to retain the plaintext of a database password after the response that created it. One-time disclosure stops being an entitlement the server has to guard and becomes a property of what is stored: there is nothing to collect a second time.

What the generated-credential design reduces to

The shape is Kubernetes' TokenRequest: a create-style call that mints a value, returns it in the response, and persists only what is needed to verify it.

• One operation. A create on a credential resource, or a subresource on the application, that generates the password from the CSPRNG, computes the engine's verifier, writes the verifier into the existing <release>-credentials Secret under the account's key, and returns the plaintext in the response body. Nothing else is stored. Regeneration is the same call again. Revocation is the same call with the plaintext discarded, or disablePassword.
• No controller for generation, no candidate state machine, no fencing. The state machine in "Generation and activation" (README.md:161, details.md:167) exists because plaintext bundles are persisted and delivered later. With verifiers the apply step is the operator's ordinary reconcile: CNPG managed.roles[].passwordSecret for PostgreSQL, passwordHashSecretKeyRef for MariaDB. The PostgreSQL applier Job (details.md:207) is not needed; the chart moves from init-job SQL to managed.roles, which is a chart change, not a new component. The MariaDB section (details.md:211) keeps its native writer, which is right, and swaps one field name.
• No entitlement, window or collect handler. "One-time collection" (README.md:126, details.md:94) and its conditional-write commitment protocol go away. A lost response after a successful mint is answered by minting again, exactly as it is for a GitHub token.
• No journal. The apiserver's own Metadata-level audit event for the create names the initiator and never sees the body. A status field with the last issuance time and initiator covers the service-history requirement. The transactional outbox, exporter, credentialevents kind and the per-tenant fail-closed threshold are not needed for this class.
• Bootstrap ordering is free. The API writes the verifier Secret synchronously before it writes the HelmRelease, so MariaDB's datadir-time root read finds it.
• Read routes two, three and four become moot for this class. A Secret holding verifiers can sit in Helm history, behind the per-release Role and in the tenantsecrets projection without disclosing anything. Removing the grants (cozystack/cozystack#4164) and the projection entries stays worth doing as hygiene, but it is no longer a prerequisite for the guarantee. Route one is cozystack/cozystack#4078.
• users: declares existence, minting is separate. An application's users map creates roles; a role with no minted credential cannot log in. The dashboard chains the two calls so the user experience is unchanged, and the application create response never carries a password.

Generation has to move into the API server in any case: Sprig has no salted PBKDF2, so a chart cannot compute a SCRAM verifier. That settles the "who generates" question the proposal spends a section on.

The two alternatives the proposal rejects, and why the rejections do not hold here

"A subresource on the virtual application with no durable records" (details.md:427) is rejected because it "cannot provide stable account incarnation, consumption state, ordering, or recovery". Each of those is a need created by holding plaintext. Consumption state exists only because a value can be collected later; with verifiers there is none. Ordering and recovery are the operator's reconcile of a Secret it already watches. Account incarnation is the username key in the Secret plus the application's users map, which is what the operator keys on anyway. The one thing a durable record would still carry is "previously exposed until rotated" for migrated accounts, and that is an annotation on the Secret.

"Require Vault or KMS, or promise that no component can recover plaintext" (details.md:437) is rejected because external custody "does not remove delivering bytes to password-consuming engines". For the first-wave engines that premise is wrong: they consume verifiers, not bytes. The promise is not that no component can recover the plaintext; it is that no component holds it after the create response returns, which is checkable.

What stays

• The migration. Existing releases hold plaintext in the Secret and in Helm history. Stages A and B (README.md:219 onward, details.md:311 onward) survive as a one-shot platform migration: compute verifiers from the plaintext the platform already has, rewrite the Secret, take field ownership with the forced server-side apply the existing migrations use, and mark those accounts as previously exposed until rotated. A migration script, not a resident controller.
• The four-route table as the evidence obligation for each class.
• Per-class honesty in the coverage table (details.md:229). Verifier-only holds for PostgreSQL, MariaDB and ClickHouse. It does not hold for S3 keys (details.md:237), where SeaweedFS verifies SigV4 with the plaintext secret; for the static break-glass kubeconfig, which is a private key; or for engines whose operators reconcile from plaintext today, which is Kafka under Strimzi, RabbitMQ under the topology operator and MongoDB under Percona unless one of them turns out to accept a hash. For those rows "once" is a policy on an interface and the table should say so per row rather than promise it globally.
• The managed Kubernetes classification from the round-4 fix (README.md:195): the OIDC route as a public projection and the static certificate as a separate class with its retirement mechanism open. That analysis is correct and independent of everything above.
• What is given up, stated. Verification before publication, logging in with the new password and confirming the old one is rejected, is lost in the synchronous shape. The API reports "issued"; "applied" is the operator's condition. I would accept that trade and would like the document to say it is a trade.

Phase 2 comes out

Tenant-supplied secrets are a storage problem, not a credential lifecycle problem. The platform direction for that problem is OpenBao as the store, with #37's {name, key} reference syntax as the consumer side of whatever the store materialises into the tenant namespace. The write-only TenantSecret version, CredentialInputRecord, supplier grants and the "Private inputs" section (README.md:132, details.md:112) are a second answer to the same question, and the two cannot coexist. Please remove phase 2 from this proposal and keep only the cross-reference to #37. That also closes the open question on where the new kinds are served (README.md:272): with no changed TenantSecret contract there is nothing that needs a v1alpha2 at all.

For the next revision

1. Reframe the proposal as "minted database credentials": one create operation, verifier-only storage for the classes that support it, operator-native apply, apiserver audit.
2. Replace the five kinds with one resource or subresource and drop CredentialOperation, CredentialBinding and CredentialEvent. Delivery to tenant clusters is a separate concern once secrets have a store.
3. Keep the migration as a numbered platform migration and keep the previously-exposed marking.
4. Rewrite the coverage table with a "verifier-only" column, and move the plaintext-by-necessity classes to a later proposal that sits on the store.
5. Verify two operator behaviours on the pinned versions before the text claims them: that CNPG managed.roles with a SCRAM verifier reconciles a role the init path created, and that mariadb-operator 25.10.2 re-applies passwordHashSecretKeyRef on Secret change with the watch label. The CRD text and the CNPG docs say yes; a live check should say so too.

Speaking as one of the two API owners the Rollout section names: the reduced shape is one I would approve. The current shape is not, because most of its surface exists to manage plaintext the platform does not need to hold.

[myasnikovdaniil]

Verifier claims check out against 8799a4e4a. postgres doesn't even need managed.roles for it, ALTER ROLE ... PASSWORD 'SCRAM-SHA-256$...' is stored as is, so the existing applier can carry a verifier without restructuring the chart.

I'm putting the mechanism into the design. Neither version is right for me as it stands: yours is correct for accounts where a verifier exists and leaves the rest unanswered, mine carries machinery that verifiers make unnecessary. Where that line sits per class is the open question.

postgres, mariadb users, clickhouse and opensearch take a verifier today, and clickhouse and opensearch already do it while keeping plaintext for no engine reason. nats, rabbitmq, redis and valkey have an engine that takes one but our operator path doesn't. kafka the same, and it has no user surface yet anyway. mongodb can't, server computes scram from plaintext and there is no api to set a precomputed one. cosi keys can't, sigv4 signs with the secret itself. outline access keys can't, shadowsocks derives the cipher key from the password. qdrant api key is compared literally. harbor bootstraps admin from plaintext and its redis needs the value on both ends.

Managed kubernetes has no password at all, static kubeconfig is a client certificate and talos secrets are ca keys and a bootstrap token, so that class keeps issuance and retirement at the ca. Virtual machines are the awkward one. sshKeys are public keys and fine as they are, but cloudInit is a free form string in the application spec and the example in our own values.yaml is password: ubuntu, readable by anyone who can get the application. cloud-init takes a crypt hash, so a verifier form exists, but user data is untyped and no schema check or reference field catches a password inside it.

Coverage table gets a verifier column, and every row that can't do it names the component holding plaintext and why. Without that the first implementation pr walks straight into it.

Verifier doesn't clear routes two, three and four, because service accounts sit in the same secret. mariadb root goes into <release>-credentials next to every tenant user and the pinned crd has only rootPasswordSecretKeyRef, no hash variant, so the operator needs that plaintext. clickhouse backup user is in the same secret and read from it by backup cronjob and by sidecar. Both are granted by name to use tier and to every ancestor tenant serviceaccount through <release>-dashboard-resources Role (cozystack/cozystack#4164).

Same reason "platform never needs to retain the plaintext" holds for tenant facing accounts only. Platform is its own client too: clickhouse backup account, mariadb root, opensearch admin that the operator authenticates with, harbor redis-password that flux reads through valuesFrom on every reconcile. Those have a machine consumer reading the value forever. That class has no home in the reduced shape and none in #82 either, and it is the one with live exposure right now, so wherever we land it needs rows of its own.

Journal can be shrinked a lot. cozystack-api runs with no audit flags at all, kube-apiserver does record aggregated api calls, talos default policy is a single catch-all Metadata rule, and cozystack does collect that log, fluent-bit tails /var/log/audit/kube/*.log into victorialogs and monitoring-agents is a default package. Context bullet in this proposal says otherwise, it is wrong and I'll fix it.

On a 1.7.0-alpha.1 stand the event for an aggregated api call is complete. A get on core.cozystack.io/v1alpha1/namespaces/tenant-root/tenantsecrets/<name>/collect records objectRef with resource, namespace, name, apiGroup, apiVersion and subresource: collect, next to verb, response code, user.username, groups, authentication.kubernetes.io/credential-id, sourceIPs and the authorization decision. No bodies at Metadata, a minted password can't land there even by accident. Attribution of the call is covered by that log.

What it doesn't give is addressing by credential, a tenant read path into it, or evidence that the engine applied anything, the event ends at the api response. Where a backend makes the call for a person the record names that backend, which is about who calls rather than about the log. Outbox, exporter, credentialevents and the fail closed threshold can go for this class. Keep a small per credential record of issuance with initiating identity that a tenant can read, plus the engine outcome.

TokenRequest shape transfers, its semantics don't. Tokens are additive, many valid at once, and that is what makes minting again free, while a password mint is destructive replace, so after the second mint the first password is dead and a crash between verifier write and response leaves a live account whose password nobody holds. Two concurrent mints also race on one secret key, so a conditional write with expected version stays.

The other one is a stable id for the account. Tenant deletes postgres orders and creates it again with the same name and the same user app, nothing tells you whether that is the same account, and the old password was disclosed while the new one wasn't so the mark has to land on the right one. Application reports the backing helmrelease uid as its own, a subresource has nothing to key that on, and an annotation on the secret doesn't carry it because the chart recreates that object. One small per account record covers both. That is details.md:427 with your objection applied to it, not five kinds.

On phase 2 the split by source of bytes is right, tenant supplied secrets are a different problem from minted ones. #82 and write-only TenantSecret need putting side by side before one gets dropped. #82 makes external-secrets-operator and a central openbao required components and moves the tenant write path into openbao own api and ui, TenantSecret stays inside our api and doesn't depend on any store. Nothing on the platform is required like that today, keycloak is not even installed unless authentication.oidc.enabled is on and that is off by default. Compare them before recording the withdrawal as done. Whichever way it goes, until that lands this proposal has to say what a strict installation does with a field that needs a supplied value, because with phase 2 gone and #82 behind unmerged work (cozystack/cozystack#4177 and #4168 open, #1942 and #2787 open issues) those fields have no path at all.

Discussion first, revision after. Once the line is agreed the next revision carries the verifier mechanism, the coverage column with a per class answer, and the lost pre publication check written down as a trade. First wave I'll repick by capability instead of by what we could land in one cycle. Your item 5 goes in as an acceptance item.

What I need from you and the other owners is the line: which classes we call verifier only, what the platform is allowed to hold for its own service accounts, how much per credential record is worth keeping.

[Timofei Larkin (lllamnyp)]

Thanks for checking the verifier claims on main, and for the corrections. The per-class list, the service-account passwords sharing the Secret, the audit pipeline that does exist, and the two ways TokenRequest semantics fail to transfer are all right. Here is the line you asked for, then the #82 comparison.

The line

1. Which classes are verifier-only. Your list, taken as the coverage column: PostgreSQL, MariaDB users, ClickHouse and OpenSearch are verifier-only in the first cut. NATS, RabbitMQ, Redis and Valkey are verifier-capable engines behind plaintext operator paths, and each conversion decides whether to change the operator path or accept plaintext; they are not first wave. MongoDB, COSI keys, Outline, Qdrant and Harbor's admin are plaintext by engine necessity, and for them "once" is a policy on the tenant-facing interface, which the row should say. Kafka has no user surface. Kubernetes and Talos stay at the CA. VM cloudInit is a real gap and belongs in the table as untyped user data with no schema check, out of scope for this proposal but named.

2. What the platform may hold for its own service accounts. Plaintext where the consumer needs it, in a Secret the tenant is never granted. MariaDB root, the ClickHouse backup user, the OpenSearch admin and Harbor's redis password move out of <release>-credentials into a separate Secret with no entry in any ApplicationDefinition selector and no name grant in the dashboard resource map. That is the route-three closure for this class, it is chart hygiene per engine, and it is the actual fix for cozystack/cozystack#4164 as far as credentials go. <release>-credentials then holds tenant-facing material only, verifiers where the engine takes them.

3. How much per-credential record to keep. One small record per account: a stable identity that survives delete-and-recreate of the application, issuance time and initiating identity, previously-exposed until rotated, and the engine outcome. That is your subresource-with-a-record shape and I will approve it. The conditional write with expected version stays, for the reason you gave: a password mint is a destructive replace, not an additive token. What does not come back is the outbox, exporter, credentialevents kind and the fail-closed threshold; the apiserver audit you verified covers attribution, and the record covers what it lacks.

On #82 versus a write-only TenantSecret

The comparison you asked for is fair and #82 now carries it in its alternatives section, revised today. The short version of my position:

A write-only TenantSecret is a can of worms I do not want opened. create on tenantsecrets cannot be scoped by name, so a tenant can pre-create a name an operator creates-if-missing and choose its content, or pre-create a chart-rendered name and fail the release. update reaches every labelled Secret, which is exactly the set of application credentials, so a tenant can overwrite a password the database never received. delete removes a Secret a running workload mounts. Your phase 2's write-only shapes, private classification and supplier grants exist to fence those in, and fencing them in is building a secrets engine inside an aggregated API on top of a store with no versioning, no read audit beyond the apiserver's, and optional encryption at rest. Kubernetes Secrets were never a secrets engine.

On the cost objection: #82 was wrong to say "required", and it no longer does. External Secrets Operator and the central OpenBao are default-installed optional packages, the same way KEDA is. An installation that disables them keeps everything else and loses secret references; a chart that receives a reference there fails the render naming the capability, exactly as autoscaling does without KEDA. And the central OpenBao is not on the critical path: the seam is a per-tenant SecretStore, so the first consumers validate against a tenant's managed OpenBao application or an external store, and #82's rollout has been reordered to say so. Nothing in it waits on cozystack/cozystack#4177 or #4168 any more.

Yes, if applications lean on references, the operator becomes mandatory in practice. That is the intended direction and it is stated in #82 as such, the same way Keycloak became the de facto identity provider once managed Kubernetes and Grafana integrated with it.

What a strict installation does in the meantime. Your proposal's strict policy admits the inline form as Legacy for reference-only classes until the reference capability exists. That is the whole interim; no stopgap write grant on tenantsecrets.

So: phase 2 comes out of this proposal, and the next revision carries the verifier mechanism, the per-class column, the separated service-account Secret, the one record, and the lost pre-publication check as a stated trade. The first wave picked by capability is the right call.

[myasnikovdaniil]

Still reworking, please do not review yet

[myasnikovdaniil]

Timofei Larkin (@lllamnyp) rewrote proposal from scratch on the line from your last comment, please take another look. Verifier mechanism, per class coverage column, one record per account and lost pre-publication check as stated trade (§3) are in, phase 2 is gone in favour of #82.

Where it departs from what we agreed:

1. Tenant accounts move out of <release>-credentials into one Secret per account, platform accounts stay where they are. mariadb rootPasswordSecretKeyRef cant change once set, so root has nowhere to move, and cnpg wants Secret per role anyway. <release>-credentials loses selector entries and name grants, and migration rotates every platform password tenants could read.
2. Credential is view with no storage, record sits in account Secret annotations. API writes that Secret and chart never renders it, so my objection about chart recreating the object doesn't apply. Identity is Secret UID, application recreated under same name gets new Secrets with new UIDs.
3. Instead of previously-exposed record carries origin of live password (NotIssued, Migrated, Minted, Revoked) and last ten operations. Only tenant-root can read audit log, so other tenants see who changed password only from the record.
4. OpenSearch is wave 2. Per-user Secrets that chart renders don't reach engine today, and OpensearchUser takes plaintext only.
5. clickhouse operator watches no Secrets, so verifier goes through HelmRelease valuesFrom and every clickhouse mint costs release upgrade (and waits while release is suspended).
6. Plaintext engines stay in this proposal as waves 2 and 3 instead of later one on store. Same mint, plaintext in account Secret that no tenant can read, wave 2 engines move to verifiers afterwards as separate tasks. Separate proposal would repeat most of this one.
7. No strict mode and no Legacy. Conversion is announced ahead per kind, no flag keeps passwords viewable, field that needs tenant supplied value stays inline until reference capability exists.

Item 5 from your review is not run yet. Live checks for cnpg and mariadb-operator are in Testing, and wave 1 converts only after they pass.

[Timofei Larkin (lllamnyp)]

Verdict

NOT LGTM, against head 7dbc5c5, and close. The direction is the one we agreed and the rewrite carries it: verifier-only storage where the engine takes one, a TokenRequest-shaped mint, one record per account on a Secret the API owns, kube-apiserver audit for attribution, and phase 2 gone to #82. All seven departures you listed are right, and the first two are improvements on what I asked for. Three things block, none of them about the shape.

Findings

[MAJOR] README.md:242, the audit-body claim is load-bearing and only tested at one level. "kube-apiserver does not record the body of a request it proxies to an aggregated API at any audit level" is what keeps a minted password out of the audit log on an installation whose policy says RequestResponse for this group. The stand test described in Context ran a Metadata policy. If the claim is wrong at Request or RequestResponse, every mint on such an installation writes the plaintext into VictoriaLogs, and the Security section's "neither server logs or audits it" is false. Run the same stand test with a RequestResponse rule on core.cozystack.io and put the result in Testing next to the CNPG and mariadb-operator checks. If bodies are captured, the fix is a documented policy constraint, not a design change, but the text has to say which.

[MAJOR] README.md:297, :328, :341, a render-time existence check reintroduces lookup. The converted chart "refuses to render while a declared account has no account Secret". For ClickHouse that is valuesFrom failing, which Flux sees. For PostgreSQL and MariaDB the only way a chart can know whether a Secret exists at render time is lookup, which is invisible to the Flux digest and is the mechanism this proposal exists to remove. CNPG already skips every change to a role whose Secret is missing (README.md:191), and mariadb-operator locks a User with no reference (README.md:195), so neither engine needs the chart to check. Say per engine how the refusal works, or drop it where the operator already fails safe.

[MAJOR] README.md:295 and Rollout step 2, the lineage webhook change needs its own treatment. Today the webhook computes tenantresource from the ApplicationDefinition selectors on first admission and copies its result over whatever the chart set, so a chart-rendered "false" on a Secret the definition still selects comes out "true". Honouring a pre-set "false" only narrows visibility and is safe, but it is new semantics in a security-relevant path, shipped as an "independent change" with no test named and no mention in Security. Two things to add. First, a sentence in Security stating that a chart may narrow the verdict and never widen it, and a test that asserts a chart-set "true" is still overwritten. Second, the observation that the webhook's verdict was never authoritative against Flux: any writer that sets all five lineage labels skips the verdict entirely, so the "false" change is a convenience for the migration window and existing Secrets, and the real closure is the selector entries leaving the definitions, which the proposal already does. Saying so keeps the next reader from thinking the webhook is the control.

Minor

• README.md:330, refusing to delete a converted application whose release is suspended is new behaviour on the Application API and belongs in a release note.
• README.md:201, ClickHouse's render failure on a missing verifier is the valuesFrom failure, which is fine; the sentence reads as if the chart checks, and the fix for the second major should make the distinction.
• The item-5 live checks are the gate for wave 1 and the text says so. Add the RequestResponse audit run to that list.

Fix the three majors and this is an approve.

[myasnikovdaniil]

Timofei Larkin (@lllamnyp) thanks, all three are addressed in the last push.

1. Ran audit with RequestResponse and Request rules on kube-apiserver 1.34 and 1.37, with a small test aggregated API behind an APIService that returns a password-like value from a subresource. Audit log has objectRef, user and level for the call but no requestObject or responseObject (control ConfigMap at RequestResponse has both), so password does not reach the log.
2. lookup is dropped. Text now says what engines do themselves when Secret is missing: cnpg leaves the role untouched, mariadb-operator sets User to Ready=False, flux fails clickhouse release with ValuesError. I ran all three on pinned versions, valuesFrom stays only for clickhouse.
3. Webhook is described as convenience now, with both tests. One correction: objectSelector skips an object that has managed-by-cozystack label, one label is enough, not all five (mutatingwebhookconfiguration.yaml:43-46), so for existing Secret the rendered "false" is what closes it.

Minors are done too.

Bigger change is in migration, please look at it again. While checking it I found that rotating root and backup passwords inside pre-upgrade hook lets tenant read the new values, and mariadb root rotation could not recover after interruption. So now hook only writes account Secrets, and a one-shot Job after the charts switch waits for each release to be Ready on the converted chart and then cleans up and rotates. Account Secret names are also mapped to valid DNS name (chart accepts usernames that kubernetes rejects).

[Timofei Larkin (lllamnyp)]

Round 6, against head 1361e34

The three majors from the last round are closed, each with the evidence I asked for, and your correction on the objectSelector is right: one label is enough, and the handler's five-label check is only its own UPDATE short-circuit. The migration rework is sound and better than what it replaced; the additive hook plus a re-runnable post-switch Job is the right split, and the left-out-release table in example.md is exactly what a reader needs.

Two non-blocking notes on the migration, then the questions this round turned up.

Migration, non-blocking

• README.md:318, the post-switch Job is the first post-upgrade actor with cluster-admin and pods/exec that is not a Helm hook and can be rerun by hand. Security says so; the release note should too.
• A release skipped on the Ready timeout keeps unrotated platform passwords and tenant keys in <release>-credentials until someone reruns the Job. The label flip has already taken it off tenantsecrets, so nothing is exposed, but an operator needs one place to see the list of skipped releases rather than one Event per HelmRelease. Name where that summary lands.

Open questions

1. What does the API read to know a kind's accounts? §1 says the view "lists the users the application declares in users" (README.md:112) and seeding is keyed on users (README.md:132 to 136). A converted kind is "an application kind whose chart has moved to this design" (README.md:64), served "once the kind's chart artifact is the converted one" (README.md:290), and Rollout step 3 names "the adapters for PostgreSQL, MariaDB and ClickHouse" (README.md:357) without saying what an adapter is. That holds together for the three wave-1 engines because all three happen to carry spec.users. The coverage table lists kinds that do not: Redis and Valkey with one shared password (README.md:264), Bucket with keys the COSI driver issues (README.md:265), Qdrant with a single API key (README.md:267), Harbor with an admin and a Redis password (README.md:268), Monitoring with a Grafana admin password (README.md:269). For each of those, what is the enumeration rule, where is it declared, and what tells the API that the kind is converted? Each wave-2 and wave-3 kind "gets its own row in §3 at its conversion" defers this to the implementation pull request; I would like the contract stated here, since it is the one thing every later kind has to satisfy and the one thing an out-of-tree catalog has to provide.

2. Does WATCH work on a derived view, and what does its resourceVersion mean? The authorization table grants watch on credentials. The view is computed from an application spec, an account Secret, and an operator object, so a watch has to be transformed from at least three sources or status.engine never changes inside an open watch. TenantSecret only transforms one Secret stream. Say how a watch is served, and whether the resourceVersion the view shows, which is the Secret's, is what a client should pass back as a list-and-watch bookmark.

3. What does a LIST cost? One application read, one Secret read and one operator-object read per account is fine for a namespace. The dashboard will list at tenant-root scope. State the bound, or that the view is namespace-only.

4. Headless provisioning. Minting is imperative by design, which is right, and the dashboard chains it. A pipeline that applies manifests, Flux or kubectl apply in CI, creates the application and gets accounts in NotIssued with no path to a usable password except a call made by something that can hold the response. Non-goals say delivery is out of scope and "the caller stores it" (README.md:76). For a tenant with no human in the loop, what is the intended sequence, and is it in scope for this proposal to name it, even as a later item?

5. Terraform. The earlier revision said payload-bearing state was unsupported. This one is silent. A provider resource that wraps the mint will hold the plaintext in state, as aws_iam_access_key does, and will detect a mint made elsewhere from status.password.at. Say whether that is the supported shape, and what the provider does on import, where no read returns the value.

6. Two subresources. The authorization table never grants mint without revoke. Is there a case for separating them, or is one subresource with the action in the body enough? Either answer is fine; the table should match it.

[myasnikovdaniil]

Timofei Larkin (@lllamnyp) thanks, all in the last push.

1. spec.users is the contract, tenant accounts are its keys (name rules from §1, no password field), any other account is declared in the ApplicationDefinition of the kind. Out-of-tree kind provides the same two. It is in §7.
2. No watch. The view is built from three sources (application, account Secret, engine object), so watch needs merging them and a synthesized resourceVersion, and removing a verb later breaks clients while adding it doesn't. resourceVersion is the account Secret's, mint precondition only.
3. Namespaced only, like tenantsecrets. List reads from caches, the cost is informers for engine objects.
4. Create application, mint from the tenant ServiceAccount, store the response, storing is on the caller. A GitOps helper that mints and writes into the tenant's password store can be built on that, later work. Sequence is in example.md.
5. Terraform keeps plaintext in state like aws_iam_access_key, caller's choice. import returns metadata only, a mint made elsewhere shows in status.password.at. Also in example.md.
6. Two subresources stay. Audit log holds no bodies, so the subresource is the only thing that tells mint from revoke, and a custom role can grant revoke alone. The table says both go to admin and the tenant ServiceAccount by default.

Skipped releases are listed in a ConfigMap in cozy-system, the Job exits non-zero while it is not empty.

[Timofei Larkin (lllamnyp)]

Against head 1fa622a. Five of the six are closed as answered: dropping watch rather than explaining it is the right call, and the ConfigMap plus non-zero exit for skipped releases is what I wanted. Three things are still missing from the contract paragraph at README.md:258, and all three are small.

1. The ApplicationDefinition declaration has no shape. The paragraph says any account that is not a spec.users key is "declared in its ApplicationDefinition", and nothing says what that declaration is: no field, no schema, no example. One worked example would settle it, and the kind to use is one with no users at all, Redis or Qdrant, so the example shows a tenant credential declared statically and how the chart consumes it. If MariaDB's root is declared the same way, a second line for a platform-only account is enough. Bucket needs one sentence of its own, since its keys come from the COSI driver and neither spec.users nor a static declaration says so.

2. The engine side is code, not contract. The verifier function, the account Secret's layout and labels, and the wiring into the operator are "the engine wiring of §3", which Rollout step 3 now places inside the Cozystack API for the three wave-1 engines. An out-of-tree kind can therefore declare its accounts, but the API has no hash function and no Secret layout for an engine it does not know, so "provides the same two things" promises more than the API can serve. Either say that an out-of-tree kind can convert only by reusing an in-tree engine's wiring, or say the declaration carries the engine part as well. Both are fine; the text has to pick one.

3. "The chart artifact is the converted one" is undefined. The chart reference keeps its name across versions, so something marks an artifact or a version as converted, and the paragraph does not say what. This decides when seeding starts and when the migration hook runs for a kind, so it belongs in the text.

Nothing else from me on this revision.

[myasnikovdaniil]

Timofei Larkin (@lllamnyp) thanks, the last push goes further than your three points, so below is the reason for each change and three questions.

1. Declaration is spec.credentials of the ApplicationDefinition (the definition of the kind, not the tenant's application), because tenant admin edits the application spec and must not decide how its passwords are stored. format says how a password is stored, users names the values map with tenant accounts, accounts lists fixed ones like the single Redis password, and secretLabels and valuesPath cover operator quirks. Redis example is in example.md. Platform accounts (MariaDB root, ClickHouse backup) are entries in accounts with owner: platform, which tells API to do nothing for them: no Credential, no account Secret, no mint. They stay in <release>-credentials as today, hidden from tenants, and the migration rotates them once. Their live values stay readable in Helm history for cluster admins and Flux, so whether one rotation is enough is in Open questions. Bucket keys come from the COSI driver and a format cannot describe that, so its row in §3 explains mint there.
2. API knows four password formats (scram-sha-256, mysql-native, sha256, plaintext), each one a function in a Go package, and a kind names one in format. API has no code for CNPG, MariaDB or ClickHouse, only for the hash. Out-of-tree kind uses plaintext (password stays in the account Secret, like waves 2 and 3) or an existing format, new format is a change in the repo.
3. Nothing marks "converted" in API. It serves a kind when its ApplicationDefinition has the block, and seeds on create and for accounts a write adds. Chart version of a HelmRelease is 0.0.0+<digest>, a hash with no order, so API cannot tell old chart from new, and a check for a one-time migration would stay in API forever. The migration tracks it itself: the hook records the chart version of each release in its ConfigMap and the Job waits until it changes.

Two changes you did not ask for.

I removed status.engine from Credential, it showed whether the engine applied a new password. To compute it API has to read CNPG, mariadb-operator and ClickHouse objects, so cozystack-api would depend on three operators, and today that knowledge sits in controllers (the backup strategy ones). Cost is that a tenant does not see whether the new password is applied yet. To bring it back, a controller watching those operators would write the state into an annotation of the account Secret and API would show it. I did not design that controller, it is in Open questions. Do you want it in this proposal?

The previous revision gave mint only to admin. Then a use member, who reads the password today and connects an app with it, loses that working path and the role gets much less useful until a helper puts the password somewhere readable. So use and above can mint and revoke now, use already restarts VMs and opens consoles. Cost is that a use member can replace a password and break running clients, same as admin. Is that fine, or admin-only plus a helper?

Last question is the migration. Applications are not versioned, so all releases of a kind switch to the new chart in one platform upgrade, whenever Flux gets to each of them, and that is why we need a hook and a Job. The new ApplicationDefinition and the new chart also come through separate HelmReleases in no fixed order, so for some minutes a release can run the old chart while API already serves its credentials. A mint or revoke then is accepted but takes effect only at the switch, Upgrade lists each case. We can accept that and tell operators in the release note to hold mints, revokes and new applications until the Job is done, which is what the text says now. Or close it with a lock: the hook marks each HelmRelease as migrating, API refuses mint, revoke and seeding for marked ones, the Job removes the mark. The lock is code in API that exists only for the migration, so I did not add it. Which one do you prefer, and is versioning of applications planned?

[Aleksei Sviridkin (lexfrei)]

myasnikovdaniil most of the claims about current Cozystack hold on main (785cbc391b7c): the tenantsecrets route, the name grants in subjectsForTenantAndAccessLevel, the webhook objectSelector, the audit input in monitoring-agents, MaxHistory 5 and the _-prefixed values filter. Three things block, and the first one comes from the postgres chart moving after the 29932ddbb baseline.

The PostgreSQL wiring in section 3 is written against a chart that has changed. The postgres chart now has per-user createRole and bypassRls, and the init-script tracks the roles it owns through membership in cozystack:managed, granted WITH INHERIT FALSE, SET FALSE (packages/apps/postgres/templates/init-script.yaml:196,232). The delete pass picks a role only if it has the comment and that membership (:298). With managed.roles as section 3 lists it, three things break:

1. CNPG applies inRoles with GRANT and REVOKE, so it revokes cozystack:managed. A user removed from values is then never dropped and keeps LOGIN and its password, the opposite of what example.md step 11 says.
2. createrole and bypassrls default to false in CNPG's role spec, so CNPG strips attributes the tenant set.
3. CNPG adopts an existing role by name, which removes the chart's refusal to adopt a role it did not create once CREATEROLE was granted.

Section 3 needs to say how the marker membership, the role attributes and the adoption refusal carry over, and the walkthrough has to follow it.

The Upgrade section puts the hook "in every platform variant that installs managed applications". The hook renders only with migrations.enabled (packages/core/platform/templates/migration-hook.yaml:1), which is false in values.yaml and true only in the values-isp-* files. The default variant still renders every PackageSource, so applications installed there by hand get the converted chart with no hook and no Job. Please say what default gets, even if the answer is a manual step.

All 31 commits land in main as written, since community merges with rebase. Many of them narrate the review instead of the change: "answer the seventh review round", "close the three round-4 blocking findings", "address review wording", "fix the three migration defects the cross-check found". Others describe details.md and verification.md, which are not in the final tree. Please squash to one commit, or a few, whose messages describe the proposal.

Smaller points:

• The text should stay state-free. "merged on 2026-09-25" (line 20), the paragraph on the withdrawn write-only TenantSecret of the earlier revision (line 18) and "(the earlier revision of this proposal)" (line 371) describe this PR's history. The design reads the same without them.
• Line 253 says Bucket's "row in section 3 says how a mint works there", but section 3 has rows only for the wave-1 engines.
• The PR body still says the record shows "whether engine applied it". status.engine was removed and is now an open question.

On the earlier reviews: scooby87's and IvanHunters' points were about the old shape (details.md, Stage-B, strictOnly, the journal, credential bindings), and the rewrite removed all of it. lllamnyp's later points (the declaration shape, format as a function, what "converted" means) are answered in section 7 and the Redis example.

[Aleksei Sviridkin (lexfrei)]

This list leaves out what the init-script on main also gives an account: createRole and bypassRls (CNPG's createrole and bypassrls default to false and get reverted), and membership in cozystack:managed WITH INHERIT FALSE, SET FALSE. The delete pass selects on that membership (init-script.yaml:298), and CNPG revokes any role missing from inRoles. The chart has to render the first two, and the text has to say how the marker survives.

[Aleksei Sviridkin (lexfrei)]

On main, the removal of undeclared users depends on the cozystack:managed membership, and so does the refusal to adopt an unmarked role once CREATEROLE was granted (packages/apps/postgres/values.yaml:262). With CNPG creating accounts, which component refuses to adopt a squatted role?

[Aleksei Sviridkin (lexfrei)]

The hook is gated by migrations.enabled, which only the values-isp-* variants turn on. default still renders every PackageSource. What happens to applications installed there by hand?

[Aleksei Sviridkin (lexfrei)]

On main the delete pass needs both the comment and membership in cozystack:managed (init-script.yaml:298). If CNPG revoked that membership, web is not dropped and keeps LOGIN.

[Aleksei Sviridkin (lexfrei)]

Section 3 has no Bucket row yet. I'd say here that it comes at conversion, as line 273 does.

[Timofei Larkin (lllamnyp)]

Against head 90d95ba. The three contract items are answered and the declaration block is the right place for them. Three things below: a readability request, an extensibility requirement I want in the text before this lands, and answers to your three questions. lexfrei's findings are separate and stand.

1. Spell the machinery out in one place

To understand how a kind is converted I had to assemble it from §1, §3, §7 and the Redis example, and then work out for myself what a chart author writes, what the API writes, and what the chart must not do. A reader of a design proposal should not have to. Please add a section, in the README or at the top of example.md, that walks one kind through the three parties:

• The definition. The full credentials block for the kind, next to the secrets and release fields it interacts with, with a line on why each field is there.
• The chart. What it renders and what it must not: no lookup, no generation, no password in any rendered object, no grant or selector on account Secrets, platform accounts rendered with tenantresource: "false", and how the engine or the pod gets to <release>.<user>.account at runtime, since the chart cannot read it at render time.
• The API. The Secret it writes on create and on an update that adds or removes an account, with the exact type, labels, annotations and the two data keys; the name mapping; and the mint's conditional write.

Do it once for PostgreSQL and once for a kind outside the tree that is not a database, because the second is where the contract is tested. Here is the shape I mean, for a web application behind nginx auth_basic, assuming an apr1 format exists. This is what I had to reconstruct; if the proposal carried it, I would not have had to.

What the kind's author writes in the definition.

apiVersion: cozystack.io/v1alpha1
kind: ApplicationDefinition
metadata: {name: myapp}
spec:
  application:
    kind: MyApp
    openAPISchema: |-            # users is a map with no password property; the API drops one with a warning
      {"type":"object","properties":{"users":{"type":"object","additionalProperties":{"type":"object"}}}}
  release: {prefix: myapp-, chartRef: {...}}
  credentials:
    format: apr1                 # the account Secret holds an htpasswd hash
    users: users                 # keys of spec.users are the tenant accounts
                                 # no secretLabels: nothing but the pod reads the Secrets
                                 # no accounts: no fixed or platform account in this kind
  secrets: {include: []}         # nothing credential-bearing is tenant-visible

What the chart renders, and what it must not. No lookup, no randAlphaNum, no Secret with a password in it, nothing in the dashboard resource map granting a Secret. The chart cannot read the account Secrets at render time, so the pod reads them at runtime: one Secret volume per declared account, and a sidecar that composes the file nginx reads.

volumes:
  {{- range $user, $_ := .Values.users }}
  - name: account-{{ include "myapp.accountName" $user }}       # same DNS mapping the API uses
    secret:
      secretName: {{ $.Release.Name }}.{{ include "myapp.accountName" $user }}.account
      optional: true            # a Secret written around the API must not block the pod
  {{- end }}
  - {name: auth, emptyDir: {}}
containers:
  - name: nginx                 # auth_basic_user_file /etc/nginx/auth/htpasswd, re-read per request
    volumeMounts: [{name: auth, mountPath: /etc/nginx/auth, readOnly: true}]
  - name: htpasswd-sync         # writes "<username>:<password>" per mounted account, renames over htpasswd, loops on change

What the API writes. On create, before the HelmRelease, one Secret per key of spec.users; on an update that adds a key, one more; on an update that removes one, a delete. For users: {alice: {}, Bob.Smith: {}} the second is:

apiVersion: v1
kind: Secret
type: kubernetes.io/basic-auth
metadata:
  name: myapp-site.bob-smith.account            # lowercased, non-DNS characters replaced
  namespace: tenant-acme
  ownerReferences: [{kind: HelmRelease, name: myapp-site, ...}]   # set right after the HelmRelease write
  annotations:
    credentials.cozystack.io/origin: NotIssued
    credentials.cozystack.io/history: '[{"operation":"Create","at":"...","by":"jane@example.org"}]'
data:
  username: Bob.Smith                           # the original name
  password: $apr1$...                           # hash of a random value the API discarded

Nobody can log in until a mint: POST .../credentials/myapp-site.bob-smith/mint replaces password and the annotations in one conditional write and returns the plaintext once; the mounted file changes on the kubelet's sync period, the sidecar rewrites htpasswd, and the new password works. kubectl get credentials -n tenant-acme lists both accounts with origin and history and nothing else.

That is the whole contract for a kind, on one page, with the three parties separated. The PostgreSQL version is the same page with CNPG's passwordSecret and the reload label in place of the volume and the sidecar.

2. The two declared fields have to be extension points from day one

Not built now, but the API shape must allow building them without breaking the block. Both become structs with a kind, and both get a webhook variant. Field names below are illustrative; the discriminator and the enumeration change at the end are what I want landed.

Format.

credentials:
  format:
    name: apr1
    kind: Webhook                    # BuiltIn | Webhook; BuiltIn when only name is set
    webhook:
      clientConfig:                  # the shape every Kubernetes webhook uses
        service: {name: htpasswd-formats, namespace: cozy-system, port: 443, path: /format}
        caBundle: <base64>
      timeoutSeconds: 5

The mint calls it between generating and writing, with a versioned request carrying the format name, the username and the plaintext, and gets the stored value back. Any failure fails the mint closed with nothing written. Three consequences belong in the text: the plaintext now also crosses to the webhook, so a format webhook is inside the trust boundary and is declared only by a platform administrator, which the definition's ownership already gives; the migration CLI has the built-in functions compiled in and cannot serve a webhook format, so such a kind ships its own migration or converts only new applications; and a kind whose format webhook is unreachable is not served, as a kind with an unknown built-in is not today. A bare string format: scram-sha-256 cannot later become an object on a CRD field, so the struct has to be the shape in the first release.

Account sources. users: users names one values key whose map keys are accounts. That stays the default, since it is what the charts do, but it must not be the only shape. The Kubernetes shape is a list of objects with a name, and a kind must be able to answer "which accounts does this application have" with logic the API does not know.

credentials:
  accounts:
    - kind: ValuesMap                # today: keys of a map under this path
      path: users
    - kind: ValuesList               # the Kubernetes shape: [{name: alice, ...}]
      path: accounts
      nameField: name
    - kind: Static
      name: default
      when: authEnabled
    - kind: Static
      name: root
      owner: platform
    - kind: Expression               # CEL over the application, in-process
      expression: "spec.accounts.map(a, {name: a.name})"
    - kind: Webhook                  # external logic, e.g. asking the COSI driver which keys exist
      webhook: {clientConfig: {...}, timeoutSeconds: 5}

A webhook source takes the application and returns the account list, each entry with name, optional username for the DNS mapping, and owner. A kind may list several sources at once, which is how a chart moves from the map to the list without a flag day.

The enumeration change this forces. §1 builds the Credential view from the application spec on every read. With a webhook source that is a webhook call per GET. Make the account Secrets the record of which accounts exist: label each with its application, and have the view list Secrets by label. The source is then consulted only at write time, on create and update, to compute what to seed and what to delete, which is the loop §1 already describes. Reads become cheap and source-independent, and it costs nothing with only the built-in sources. This one is not optional, because it decides where the view comes from.

3. Your three questions

Engine-applied signal. Open question is fine. When it is designed, the definition is the place to say who to ask: a status source with the same kind discriminator, built-in for the operators the platform knows and a webhook for the rest. No need to pick now; just do not close the door on it in the block.

Who may mint. Admin-only for now. Cozystack has no fine-grained role management of its own, so the tiers are the only knob, and use minting is a one-line change later if the helper takes long. I have no strong feelings beyond starting narrow.

The migration window. Accept it. The release-note instruction to hold mints, revokes and new applications of a converting kind until the Job reports done is enough, and a lock in the API that exists only for the migration is not worth its code. Application versioning is a long way off and should not be assumed here.

[myasnikovdaniil]

Timofei Larkin (@lllamnyp) thanks, all in the last push, squashed to one commit.

1. Contract for a kind is at the top of example.md, for postgres, clickhouse and your nginx kind. I took your example almost as is. apr1 is not one of the four formats, so there it is a fifth function in the repo, with a webhook format the kind would bring its own.
2. format and account sources are structs with kind now, and users is merged into accounts. ValuesMap and Static are built, the other kinds are named and API refuses them until they are built, webhook format consequences are in section 7. Credential is listed by label on account Secrets, sources are asked on write and on mint only. The cost is that an account with no Secret yet (before conversion, release the migration left out) is not listed, a mint by name creates the Secret. Status source for the engine signal is noted in the open question.
3. Mint and revoke are admin and above, migration window is accepted with the release note.

Section 3 for postgres changed too, I ran CNPG 1.30.0 live: the marker goes into inRoles and the refusal to adopt a planted role is a pre-upgrade hook.

[Timofei Larkin (lllamnyp)]

LGTM, against head fc62e36.

The contract for a kind is now the first thing example.md says, with the definition, the chart and the API separated for three kinds; format and the account sources are structs with a kind, with the unbuilt kinds named and refused; the view lists account Secrets by label and consults sources only on write and on mint; mint and revoke sit at admin; the migration window is accepted with the release-note instruction. That is everything I asked for across the rounds.

Two notes for the release note, not for the text: the PostgreSQL pre-upgrade hook fails closed on any role without the marker, so a role created by hand before the conversion blocks that release's upgrades until it is dropped; and during a conversion a declared account without a Secret is absent from the credential view rather than shown as pending.

lexfrei's request-changes is his to lift.

[myasnikovdaniil]

Aleksei Sviridkin (@lexfrei) thanks, all your points are in the last push, squashed to one commit, and the PR body is rewritten. Postgres wiring in section 3 follows what I checked on CNPG 1.30.0 live (marker in inRoles for users and database roles, refusal to adopt a planted role as pre-upgrade hook), and default variant is out of scope since migrations must be enabled. Please take another look.

[scooby87]

LGTM, against head fc62e36.

Both items left open in my earlier review are closed by the rewrite: the API now writes its own account Secrets and takes over no object Helm owns, so no field ownership moves, and those Secrets are owned by the HelmRelease in the same namespace, so no cross-namespace finalizer is needed.