feat(#1528): wire DB_BOOTSTRAP_* into jobs-manager to re-parent the mint off edgeuser (S3) - #785
Conversation
…int off edgeuser (S3)
backend#1528 S3: complete the chart half of the bootstrap re-parent. The runtime
seam already merged (client-runtime#310 — _bootstrap_db_config reads
DB_BOOTSTRAP_USER/DB_BOOTSTRAP_PASSWORD, both-or-neither, else falls back to
edgeuser). This injects those into the jobs-manager deployment, gated on a new
S3 flag, so the account-minting DDL (CREATE USER / GRANT for tb_credmgr /
tb_meta / tb_ingest) authenticates as MySQL root instead of the root-equivalent
edgeuser — the prerequisite for REVOKE/DROP of edgeuser.
- values.yaml: bootstrapDbReparent (operator override) + bootstrapDbReparentByEnv
{dev,stg,prod: false} + bootstrapDbPassword. Default-off everywhere; this is
the last, edgeuser-retiring step, flipped dev-first per fleet.
- _helpers.tpl: tracebloc.bootstrapDbReparent, resolving exactly like
tracebloc.serviceDbAccounts (override -> ByEnv -> normalized CLIENT_ENV).
- secrets.yaml: DB_BOOTSTRAP_PASSWORD resolved 2-tier (pin -> existing Secret ->
fail). No generate-random tier: jobs-manager authenticates as root and never
runs ALTER USER root, so a random value would lock the minter out and
CrashLoop it. No alphanumeric constraint (connection param, never DDL);
placeholder-guarded.
- jobs-manager-deployment.yaml: DB_BOOTSTRAP_USER=root + DB_BOOTSTRAP_PASSWORD
(secretKeyRef), gated, both-or-neither.
- values.schema.json: three new keys. Chart.yaml 1.9.51 -> 1.9.52 (+ appVersion).
- tests: +8 helm-unittest cases (default byte-identical, gate-on wiring, ByEnv
resolution, require-pin fail, placeholder reject).
Additive and default-off: nothing renders until the S3 rollout flips the flag.
Verified: helm unittest 499 passed; helm lint clean; helm template confirms
default byte-identical, gate-on wires secret+env, fail-fast on missing pin.
Refs: backend#1528 (D10 close-out, S3), client-runtime#310 (runtime seam).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
left a comment
There was a problem hiding this comment.
Holding the review rather than posting one: this is CONFLICTING / mergeStateStatus: DIRTY, so the diff I'd be reading isn't the diff that will land, and reviewing the wrong three-way merge wastes both our time.
The cause is a stale base, not a single unlucky collision. The Chart.yaml hunk reads:
-version: 1.9.51
+version: 1.9.52
while origin/develop is at 1.9.58 — seven chart versions ahead. The PR was opened today, so the branch was cut from a checkout that had gone stale rather than from the current tip; #764, #777, #779, #780, #781 and #782 have all landed in between, and #779 and #782 both touch files this PR touches.
git fetch && git merge origin/develop, re-bump to whatever is current at that point, then re-request me — I'll pick it up on the next pass. Worth doing before anything else, because two of those intervening PRs changed jobs-manager-deployment.yaml and secrets.yaml, so the merge may well change what this PR needs to say rather than just where its lines sit.
LukasWodka
left a comment
There was a problem hiding this comment.
Picked this up as reviewer. It needs a develop merge before I can review it properly — mergeable=CONFLICTING / mergeStateStatus=DIRTY, confirmed on two consecutive reads so it isn't GitHub's "not yet computed" state.
It isn't only the version bump. Every file this PR touches has also moved on develop since your merge-base:
client/Chart.yaml
client/templates/_helpers.tpl
client/templates/jobs-manager-deployment.yaml
client/values.schema.json
client/values.yaml
Chart.yaml is the loudest — you set 1.9.52, develop is at 1.9.60 — but jobs-manager-deployment.yaml and _helpers.tpl are the ones that matter, because the telemetry work landed in both today. I'm deliberately not doing the correctness pass yet: with those two re-resolved, I'd be reviewing lines that won't be what ships, and a review of a superseded diff is worse than no review because it reads as coverage.
A systemic note, since this is the third one today
chart-version-guard requires a Chart.yaml bump whenever chart content changes — correct, and I verified it's a real required gate. But it means any two concurrent chart PRs conflict by construction, and the one that lands second always has to re-cut. Today that has cost:
- client#765 — closed outright for exactly this (its 1.9.58→1.9.59 became a certain conflict once #784 took 59 and #787 took 60);
- client#785 — this PR, now dirty;
- and #784/#787 themselves had to serialize.
That's not an argument against the guard. It's an argument that the bump wants to be the last thing computed rather than a hand-picked number carried on the branch — a release-time or merge-queue-time bump would remove the whole class. Worth a ticket; it's now demonstrably eating PRs rather than just being untidy.
One question that survives the rebase
The two design decisions you flagged as confirmed on the ticket both look right to me, and neither depends on the merge: require-pin/fail-fast over a baked Edg9@Tr@ce fallback (keeps the literal out of the chart, doesn't fight backend#947), and the ByEnv gate mirroring serviceDbAccounts (the backend#1752 lesson that a plain global bool broke the fleet silently). Default-off everywhere with helm template showing a byte-identical default render is the right shape for a prod-irreversible-adjacent step.
What I'd want to understand is the credential lifetime. As described, DB_BOOTSTRAP_USER=root and DB_BOOTSTRAP_PASSWORD land as env vars on the jobs-manager deployment — so MySQL root is present in the pod environment for the whole process lifetime, not only while the account-minting DDL runs. The mint is a startup-time operation; the credential is permanent.
I'm not saying that's wrong — re-parenting onto root is clearly the right call, and your reasoning for why edgeuser can't revoke itself while the mint authenticates as it is correct. But it does trade "root-equivalent edgeuser in the pod" for "actual root in the pod", and the S3 endgame is DROP USER edgeuser. So: is the intent that jobs-manager holds root permanently from then on, or is a narrower minting identity (global CREATE USER + GRANT OPTION, nothing else) the eventual target? If the latter, worth saying so in the PR so the next reader doesn't take permanent-root as the settled destination.
Happy to do the full pass as soon as it's merged up — no need to re-request, it'll come back to me automatically.
saadqbal
left a comment
There was a problem hiding this comment.
Still holding — the re-request came without the merge. Head is unchanged at 18ba65a1, the same commit I read on 08-21, and the PR is still CONFLICTING / mergeStateStatus: DIRTY. Re-requesting a review doesn't clear a conflict, so there's nothing new here for me to read.
The drift has grown since I flagged it: origin/develop is now at 1.9.60, this branch is at 1.9.52 — eight chart versions, not seven. #784 and #787 landed in the meantime.
Concretely, five of your eight files have also moved on develop:
client/Chart.yaml
client/templates/_helpers.tpl
client/templates/jobs-manager-deployment.yaml
client/values.schema.json
client/values.yaml
#784 ("the chart wires jobs-manager to the Collector's token") edits the same jobs-manager-deployment.yaml this PR is wiring DB_BOOTSTRAP_* into — which is why I'd rather not review the pre-merge diff. The merge is likely to change what this PR needs to say, not just where its lines sit.
Unchanged ask: git fetch && git merge origin/develop, re-bump off whatever the tip is at that point, push, then re-request. I'll review the merged diff on the next pass.
aptracebloc
left a comment
There was a problem hiding this comment.
Reviewed the full chart change — it's clean and I'd approve it as-is:
tracebloc.bootstrapDbReparentresolves byte-for-byte identically totracebloc.serviceDbAccounts(override →…ByEnv→ normalizedCLIENT_ENV) — verified against the existing helper.- Both-or-neither holds:
DB_BOOTSTRAP_USER/DB_BOOTSTRAP_PASSWORDin the deployment and theDB_BOOTSTRAP_PASSWORDsecret key all render under the same gate, so no danglingsecretKeyRefand a default install stays byte-identical. - Secret resolution (pin → existing Secret →
fail, no random tier) is the right call given jobs-manager authenticates as root and never runsALTER USER root; placeholder guard and the "no alphanumeric constraint" reasoning both hold. The b64 vectorRWRnOUBUckBjZQ==correctly decodes toEdg9@Tr@ce. - Schema types (
boolean|null,string, object-of-bool) and the default-off values are correct; the test matrix covers off/on/byEnv/fail-unset/reject-placeholder/non-alphanumeric.
One blocker before I approve: the PR is currently CONFLICTING against develop (mergeStateStatus=DIRTY). Please merge develop and resolve — I'll approve once it's clean and CI is green again.
— drafted with Claude Code
…rap-chart # Conflicts: # client/Chart.yaml # client/values.schema.json
saadqbal
left a comment
There was a problem hiding this comment.
Requesting changes on one thing only: this publishes a live MySQL root password into a public repo. Details on the values.yaml thread — I've deliberately not repeated the value in my comments, since they're public too.
Short version: tracebloc/client is visibility: public, tracebloc/mysql-client is private, and this PR moves the literal from the second into the first — six occurrences, none of them on develop before, including the Helm fail message that operators paste into tickets. It's a baked image default rather than an unknown secret, so this isn't a rotation emergency, but it is a real change in exposure and git history makes it permanent. That's why I'd rather it were fixed before merge than tidied after.
Everything else here is good, and I want to be clear that the blocker is the string, not the design.
The develop merge I asked for is done — 58df48c5, now 0 commits behind and MERGEABLE, where it had been DIRTY since 08-21 with a base seven chart versions stale. It picked up #796, #791, #797, #801, #798, #805 and #804 on the way. That was the whole of my earlier ask.
The gate renders the Secret key and the env var under the same condition. Both are {{- if (include "tracebloc.bootstrapDbReparent" .) }}, so they appear together or not at all — a mismatch here would be a CreateContainerConfigError on a pod that referenced a key the Secret never made.
The three-state override is right, and nil-safe under --reuse-values. (default dict .Values).bootstrapDbReparent with kindIs "invalid" distinguishes unset (fall through to the per-env default) from explicitly false (which wins), and default dict .Values.bootstrapDbReparentByEnv means an upgrade that carries old stored values without the new key resolves to off rather than nil-pointering during templating. That's the failure mode --reuse-values produces on every new top-level key, and it's handled.
No random tier on this password is the correct call, for the reason given: jobs-manager authenticates as root and never runs ALTER USER root, so a generated value would lock the minter out of MySQL and CrashLoop it rather than failing loudly at install. Failing in Helm is much better than failing in a pod. The placeholder regexMatch "^<.*>$" guard and preserving the existing Secret value across upgrades are both right too.
And defaulting the gate off everywhere is the right sequencing for the last, edgeuser-retiring step — flip dev only after serviceDbAccounts and perExperimentDbCreds are verified on the fleet, as the helper's comment says.
Re-request me once the literal is out and I'll approve; nothing else is holding this.
LukasWodka
left a comment
There was a problem hiding this comment.
Not approving — @saadqbal's finding is correct, and it's worse than the thread says in three ways
I verified it independently rather than agreeing on the strength of the argument. Everything below is measured; like Asad, I'm not repeating the value, since this comment is public too.
Confirmed as stated:
tracebloc/client→visibility: public.tracebloc/mysql-client→ 404 for my token, consistent with private.- The literal is on 7 added lines across 4 files —
client/values.yaml(1),client/values.schema.json(1),client/templates/secrets.yaml(2, one being thefailmessage),client/tests/secrets_test.yaml(3). - It is on none of
developtoday. This PR introduces it. quality / gitleaksran and passed, so Asad is right that the automated gate can't catch this class.
And three things the thread doesn't cover.
1. I reproduced the fail-message case, which makes it the worst of the seven — measured, not predicted. Rendering with the gate on and no pin:
$ helm template t . --set bootstrapDbReparent=true …
Error: … <the literal appears in the failure text>
So the credential is printed to stdout by the ordinary misconfiguration path. Operators paste Helm failures into tickets and Slack verbatim; this republishes it every time the gate misfires, in places with no history rewrite available at all.
2. The PR description itself contains it, twice. Asad's list of six enumerates the diff. The description is a public artifact too, and it's already indexable — so any remediation has to include editing the body, not just the files.
3. The exposure has already begun, which changes what "fix" means. sec/1528-s3-bootstrap-chart is a branch on tracebloc/client itself — isCrossRepository: false, not a fork. The commits are therefore already public objects, fetchable by anyone via the PR ref right now. Not merging keeps it out of develop's permanent history, and scrubbing the branch reduces what's indexed, but neither un-publishes what's already fetchable.
So the real remediation is rotation, and the scrub is damage limitation. I'd treat the mysql-client baked root credential as disclosed as of this PR's push and rotate it, independently of whatever happens to this diff. That's an org call rather than something to settle on a PR thread — @LukasWodka, flagging it to you explicitly.
On making it stick. This is the class our own rules say should become a machine check — grep-expressible, so house-rules is the natural home, and this repo already runs quality / house-rules. One caveat that matters: the detector must not embed the literal, or the rule publishes what it's protecting. A partial or hashed pattern, or the rule living in a private repo and running against the public ones, are the two shapes that work. Worth getting right rather than fast.
On the change itself, which is otherwise sound, and worth saying so since the above reads as all-negative:
- Default render is genuinely clean.
helm templatewith no flags → 0DB_BOOTSTRAPreferences. The "additive, nothing renders until the flag flips" claim holds as measured, not just as asserted. - The fail-fast works and the reasoning for having no generate-random tier is right: jobs-manager authenticates as root and never runs
ALTER USER root, so a random value the server was never told to expect would CrashLoop the minter. That's the correct call, and it's the reason the message exists at all — which is precisely why the message needs to name the source indirectly instead of quoting it. - Resolving the gate through
tracebloc.bootstrapDbReparentthe same wayserviceDbAccountsresolves — override →…ByEnv→ normalizedCLIENT_ENV— is the right precedent to follow, and it inherits the backend#1723 alias normalization rather than re-deriving it. - 23/23 checks green and the develop merge cleared the conflict, so the only thing standing between this and mergeable is the exposure.
Holding behind Asad's change-request. Once the literal is out of the diff and the description — and with rotation tracked separately — this is a straightforward approve from me.
…is public repo Review finding (saadqbal change-request, Lukas confirmed): the PR introduced the mysql-client image's baked root password as a literal in 7 lines across 4 files of this PUBLIC repo (values.yaml, values.schema.json, secrets.yaml resolution comment + fail message, and the test fixtures/errorMessage/b64 vector). It was on none of develop before. The org rule forbids committing secrets in code, config, tests or messages, and gitleaks can't catch this class (no provider pattern). Every occurrence now refers to the credential indirectly — "the mysql-client image's baked root password; see backend#947 for the value and its rotated replacement" — keeping all the explanatory intent, none of the value: - values.yaml / values.schema.json / secrets.yaml comment + fail message: point at the mysql-client image + backend#947 instead of quoting the literal. The fail message is the important one — operators paste failures into tickets/Slack. - secrets_test.yaml: dummy fixture "test-root-pw" (still non-alphanumeric, so it still exercises the no-alnum-constraint path) with its own b64 vector; the fail-unset case asserts the stable prefix via errorPattern, not the full string. No behaviour change: helm unittest 562 passed, helm lint clean, default render still byte-identical (0 DB_BOOTSTRAP refs), fail-fast still fires. The literal is in no commit message on this branch, so no history rewrite is needed. Rotation of the baked credential is tracked separately (backend#947) per the review; this commit is the scrub. Refs: backend#1528 (S3), backend#947 (literal removal + rotation). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
left a comment
There was a problem hiding this comment.
The scrub is done properly — two things left, and one of them is a merge-button decision
The tree is clean. e61e1361 removes the literal from all four files, and every replacement is an indirect reference — "the mysql-client image's baked default; see backend#947 for the value and its rotated replacement." Zero occurrences remain in the diff against develop. The fail message got the same treatment, which matters most: that was the one that printed the credential to any operator terminal on a misconfiguration and then into whatever ticket they pasted it in.
Two things still open.
1. The PR description still contains it, twice
gh pr view 785 --json body | grep -c → 2. That's the half Asad's thread didn't enumerate and it hasn't moved. The description is a public artifact and an indexed one, so it needs the same edit as the files.
2. This has to be squash-merged, not merge-committed
Newly relevant, because scrubbing in a follow-up commit doesn't remove anything from the branch:
$ git rev-list origin/develop..e61e1361 → 3 commits
18ba65a contains it (7 lines)
e61e136 contains it (7 lines) ← a removal diff still shows the removed lines
And this repo allows both:
$ gh api repos/tracebloc/client → squash=true merge_commit=true rebase=false
So the merge method decides whether the literal enters develop's permanent history:
- merge commit →
18ba65aande61e136become develop's history, thenstaging's, thenmain's, and every clone anyone ever makes. Irreversible in practice. - squash → only the final tree lands; the literal never enters develop.
Worth deciding before the button is pressed rather than after, since there's no undo on the first option.
To be explicit about what squashing does and doesn't buy: it stops the exposure compounding into develop and every future clone. It does not un-publish anything — the branch commits are already public objects, reachable via the PR ref, and GitHub retains them. So the standing remediation is still to treat the credential as disclosed and rotate the mysql-client baked root password, independently of this PR. @LukasWodka, that's the one that needs an org decision.
On the change itself, unchanged from my earlier read and still sound: default render is byte-clean (0 DB_BOOTSTRAP refs), the no-generate-random reasoning is right (jobs-manager authenticates as root and never runs ALTER USER root, so a random value would CrashLoop the minter), and the gate follows the serviceDbAccounts resolution precedent rather than re-deriving it.
Not approving: @saadqbal's change-request stands and his thread is still open — his call to clear, not mine — and four checks are still IN_PROGRESS.
saadqbal
left a comment
There was a problem hiding this comment.
Approving — my change-request is addressed and my thread is closed.
I verified the scrub independently rather than reading the summary: the tokens removed from all four
files between 58df48c5 and e61e1361 are exactly the credential and the word unrotated, neither
reappears at the new head, and the base64 fixture is properly handled — the vector that decoded to
the literal is gone, not relabelled. PR description, branch commit messages and the conversation are
clean too. Detail on the thread, without the value.
The replacements keep the explanatory intent, which was the point: every reference names the source
indirectly, the dummy still carries a non-alphanumeric so the no-alnum path stays exercised, and the
fail-unset test asserts the prefix via errorPattern — so the assertion can't be tightened back into
re-quoting a value. The guard itself is unchanged, which is what I wanted: two-tier resolution,
placeholder regexMatch, and the fail on the no-random tier.
Gate is clean — no failing or pending checks, Bugbot green, no unresolved threads.
One thing worth keeping visible rather than closing out with this PR: merging does not undo the
disclosure. The literal is reachable in this branch's history on a public repo, so backend#947 is the
real remediation and this was damage limitation. Right call to treat it as disclosed and raise
rotation separately.
LukasWodka
left a comment
There was a problem hiding this comment.
Approving — and one thing that has to be right at the merge button
Squash-merge this, don't merge-commit it. The reason is below; it's the only irreversible decision left.
Gate: 23/23 checks pass, mergeStateStatus=CLEAN, zero unresolved threads, @saadqbal's change-request cleared.
Both remaining items are done. The literal is gone from the diff (0 occurrences against develop) and now from the PR description too (0). Every replacement is an indirect reference — "the mysql-client image's baked default; see backend#947 for the value and its rotated replacement" — including the fail message, which mattered most because that one printed the credential to an operator's terminal on any misconfiguration and then into whatever ticket they pasted it into.
Why the merge method still matters. Scrubbing in a follow-up commit doesn't remove anything from the branch:
$ git rev-list origin/develop..e61e1361 → 3 commits
18ba65a contains it
e61e136 contains it (a removal diff still shows the removed lines)
$ gh api repos/tracebloc/client → squash=true merge_commit=true
A merge commit makes those two commits part of develop's history, then staging's, then main's, and every clone anyone ever takes. A squash lands only the final tree. There's no undo on the first option, which is the whole reason I'm putting it at the top rather than in a footnote.
To be precise about what squashing buys: it stops the exposure compounding. It doesn't un-publish — the branch commits are already public objects reachable via the PR ref, and GitHub retains them. So rotating the mysql-client baked root password remains the real remediation, independent of this PR. @LukasWodka has that one.
On the change itself, verified rather than read:
- Default render is genuinely untouched —
helm templatewith no flags gives 0DB_BOOTSTRAPreferences, so the "additive, nothing renders until the flag flips" claim holds as measured. - The no-generate-random decision is right, and the reasoning is the good kind: jobs-manager authenticates as root and never runs
ALTER USER root, so a random value the server was never told to expect would lock the minter out and CrashLoop it. That's also exactly why thefailmessage has to exist — and therefore why it had to stop quoting the value. - The gate resolves through
tracebloc.bootstrapDbReparentthe same wayserviceDbAccountsdoes — override →…ByEnv→ normalizedCLIENT_ENV— so it inherits the backend#1723 alias normalization instead of re-deriving it, and the fleet's posture reads out of one place. - Defaulting every environment off, with the flip staged per fleet, is the backend#1752 lesson applied rather than restated.
Good change, and the exposure was caught and closed properly.
aptracebloc
left a comment
There was a problem hiding this comment.
Full journey closed — approving.
- The resolver logic I verified earlier is unchanged:
tracebloc.bootstrapDbReparentstill resolves identically toserviceDbAccounts, both-or-neither holds via the shared gate, and secret resolution stays pin → existing Secret →failwith no random tier. - Asad's security catch is correctly addressed: the baked root literal
Edg9@Tr@ceno longer appears anywhere in this public repo (values.yaml comment and the test now reference backend#947 for the value / use the dummytest-root-pwinstead), so the change no longer publishes the secret it's wiring. - Conflict resolved (
MERGEABLE/CLEAN), CI green, no open threads, Asad approved.
LGTM.
— drafted with Claude Code
What
backend#1528 S3 — the chart half of the bootstrap re-parent. The runtime seam already merged (client-runtime#310:
_bootstrap_db_configresolves the minting identity fromDB_BOOTSTRAP_USER/DB_BOOTSTRAP_PASSWORD, both-or-neither, falling back toedgeuserwhen unset). This PR injects those into the jobs-manager deployment, gated on a new S3 flag, so the account-minting DDL (CREATE USER/GRANTfortb_credmgr/tb_meta/tb_ingest) authenticates as MySQLrootinstead of the root-equivalentedgeuser.While the mint authenticates as
edgeuser,edgeusercannot beREVOKEd /DROPped. Re-parenting it ontoroot(which already holds the globalCREATE USER+GRANT OPTIONthe mint needs) is the prerequisite that unblocks the rest of S3.Changes
values.yaml—bootstrapDbReparent(operator override) +bootstrapDbReparentByEnv{dev,stg,prod: false}+bootstrapDbPassword. Default-off everywhere; this is the last,edgeuser-retiring step, flipped dev-first per fleet only onceserviceDbAccounts+perExperimentDbCredsare verified and the S0SHOW GRANTSsnapshot is captured._helpers.tpl—tracebloc.bootstrapDbReparent, resolving exactly liketracebloc.serviceDbAccounts(override →…ByEnv→ normalizedCLIENT_ENV), so the fleet's S3 posture reads out of one place and shares the backend#1723 alias normalization.secrets.yaml—DB_BOOTSTRAP_PASSWORDresolved 2-tier: pin → existing Secret →fail. Deliberately no generate-random tier: jobs-manager authenticates asrootand never runsALTER USER root, so a random value the server was never told to expect would lock the minter out and CrashLoop it. No alphanumeric constraint (unlike the credmgr/tb_* passwords it is never interpolated into DDL — only a mysql-connector connection parameter; the baked root password isn't alphanumeric anyway). Placeholder-guarded.jobs-manager-deployment.yaml—DB_BOOTSTRAP_USER=root+DB_BOOTSTRAP_PASSWORD(secretKeyRef), gated, both-or-neither.values.schema.json— three new keys.Chart.yamlbumped (now 1.9.67,appVersionin lockstep).secrets_test.yaml/jobs_manager_test.yaml.Design notes (both confirmed with the reviewer route on the ticket)
serviceDbAccounts— the backend#1752 lesson that a plain global bool caused a silent fleet break.Credential lifetime (answering @LukasWodka)
DB_BOOTSTRAP_USER=root+DB_BOOTSTRAP_PASSWORDland as env on the jobs-manager deployment, so once the gate is on,rootis present in the pod for the whole process lifetime, not only during the startup mint — this trades "root-equivalentedgeuserin the pod" for "actualrootin the pod". That is deliberate, not the settled destination: the immediate S3 goal is only to stop the mint authenticating asedgeusersoedgeusercan beDROPped. A narrower dedicated minting identity (globalCREATE USER+GRANT OPTION, nothing else) is a reasonable eventual target, but it would itself have to be minted asrootfirst — the same bootstrap dependency — so it belongs in a later step, not this one. Flagging here so permanent-root isn't read as the end state.Security remediation (review finding)
An earlier revision of this PR (diff and this description) quoted the
mysql-clientimage's baked root password as a literal. This public repo must not carry it (org rule; gitleaks can't catch this class). Scrubbed ine61e136— every reference now points at themysql-clientimage + backend#947 instead of quoting the value, keeping all the explanatory intent. Rotation of that baked credential is tracked separately in backend#947 per @saadqbal / @LukasWodka.Verification
helm unittestfull suite: 562 passed (34 suites).helm template(ci/eks base): default render = 0DB_BOOTSTRAPrefs (byte-identical); gate-on+pin wires the Secret (DB_BOOTSTRAP_PASSWORD) and both env vars; gate-on without pin fails fast with the guidance message;bootstrapDbReparentByEnv.dev=truewires it whileprod(default) doesn't; placeholder pin rejected.helm lintclean;chart-version-guard.shsatisfied.Additive and default-off — nothing renders until the S3 rollout flips the flag per fleet.
Not in this PR (remaining S3)
client-runtime/mysql-client-initdb/10-edgeuser-bridge-grants.sqlonce the mint no longer runs as edgeuser.REVOKEedgeuser →DROP USER, fleet-staged dev→stg→prod, gated on the S0 snapshot.Refs: backend#1528 (D10 close-out, S3) · client-runtime#310 (runtime seam) · backend#1752 (ByEnv rollout) · backend#947 (literal removal).
🤖 Generated with Claude Code
Note
High Risk
Touches MySQL bootstrap identity and stores the cluster root password in the chart Secret. A mis-set pin or premature flag flip can lock jobs-manager out of minting or expand who authenticates as root.
Overview
Wires backend#1528 S3 so the account-minting DDL can authenticate as MySQL
rootinstead ofedgeuser, unblocking later REVOKE/DROP of edgeuser. Default installs stay byte-identical:bootstrapDbReparent/bootstrapDbReparentByEnvare off everywhere.When the gate is on, jobs-manager gets
DB_BOOTSTRAP_USER=rootplusDB_BOOTSTRAP_PASSWORDfrom the Secret. Password resolution is pin → existing Secret → fail (no random default, because the minter neverALTER USER root). Placeholders are rejected; there is no alphanumeric constraint.Chart 1.9.67. Helm-unittest covers default-off, ByEnv, missing pin, and placeholder rejection.
Reviewed by Cursor Bugbot for commit e61e136. Bugbot is set up for automated code reviews on this repo. Configure here.