fix(juicefs): forward META_ROLE through the cross-node make target (+ JUICEFS_META_PASSWORD fallback) - #2708
Merged
Conversation
…A_PASSWORD fallback #2683 added META_ROLE to juicefs-cross-node-setup.sh but the make target never forwarded it, so the canonical path (make juicefs-cross-node-setup META_ROLE=juicefs_meta) silently defaulted to supabase_admin — the scoped-role cutover was unreachable on remote nodes (the 5090 step-5 mount). Forward META_ROLE (default supabase_admin, back-compat). Also: DB_PASS now falls back to the funnel-delivered JUICEFS_META_PASSWORD (registered in chit_manifest_register.py, #2705), so a node that received the secret via the pipeline runs with just META_ROLE=juicefs_meta. Both pass as sub-process env (not argv) -> handed to JuiceFS via META_PASSWORD, never in ps. Same 'wired end-to-end?' class as the node-local fixes B850 surfaced tonight. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017g8jC7dupS2ubafo6zPQY6
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dda6c45462
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…it targets The META_ROLE forwarding in this PR is the important half and is correct — the script has supported META_ROLE since #2683 but the target never passed it, so the canonical make path silently used supabase_admin, which pg_hba now REJECTS from the tailnet (#2702). Step 5 would have failed with an auth error that reads like a bad secret rather than a rejected role. The password fallback, though, could not fire. `$(JUICEFS_META_PASSWORD)` is a MAKE variable reference, and make populates its variables only from the environment and from Makefiles — nothing includes the generated tier files. The funnel writes this key to env.tier-data and .env.generated, not to env.shared, so it is never exported into an operator's session either. Net effect: the operator hits "DB_PASS or JUICEFS_META_PASSWORD required" on precisely the node the funnel just delivered to, and the documented remedy is to read the value out of the tier file by hand — which the script's own error text tells them not to do. Switched to the recipe-time shell read this file already uses ~line 229: DB_PASS="$(or $(DB_PASS),$$(grep -m1 '^JUICEFS_META_PASSWORD=' env.tier-data ...))" `$$(...)` not `$(...)`: shell at recipe time, which can read generated files. `cut -d= -f2-` not `-f2`, so a value containing '=' is not truncated at the first one (the neighbouring line has that bug; not inheriting it). Dropped the $(error): the script already fails with a better message that names both DB_PASS and the funnel path, and a make-level abort pre-empts it. Verified all three paths: funnel-delivered key in env.tier-data, no DB_PASS -> script receives META_ROLE=juicefs_meta with a non-empty DB_PASS explicit DB_PASS=... on the command line wins neither script's own "no metadata password" error, no make abort and separately that -f2- preserves `abc=def==` where -f2 yields `abc`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkwiW3VY1xWmahTAtVioxz
…nonical loader
The previous commit read env.tier-data with an inline grep. That works, but it
is not the house pattern and it only reads one file.
scripts/with-env.sh is the canonical loader — "env.shared* -> tier env files ->
.env* overlays", mirroring compose layering — so it honours precedence instead
of hard-coding which tier file the value happens to land in today. Same idiom as
mk/yt-cookies.mk:18, and infra.mk:603 already carries a comment recording this
exact lesson as a prior Codex P1 ("uses with-env.sh to load tier files").
Using it also drops the `cut -d= -f2-` parsing entirely, so there is no longer a
quoting or embedded-'=' edge case to get right.
Verified all three paths with a stubbed script:
tier-delivered key in env.tier-data, no DB_PASS -> DB_PASS_len=27
explicit DB_PASS=explicit -> DB_PASS_len=8
neither script's own "no metadata password" error, no make abort
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FkwiW3VY1xWmahTAtVioxz
Owner
Author
|
Two things: the comment is stale, and the underlying point was still right. Stale: it reviewed But the pointer to It also removes the Verified with a stubbed script — tier-delivered: |
POWERFULMOVES
added a commit
that referenced
this pull request
Aug 24, 2026
…tecting
Two Codex P1s, both correct.
1. ROTATION CONTRADICTED ITSELF. Section 2.1 says never paste the value on a
CLI, then 2.2-alt handed over a `psql -c "ALTER ROLE … PASSWORD '<new>'"`
that puts it in shell history AND in the container's argv. `psql -v` is no
better — same argv. Replaced with a stdin form: `read -rs` (no echo, no
history) piped through a shell builtin, so the value never becomes another
process's argv. Validated against the live DB on a throwaway role: ALTER ROLE
applied and the role authenticated with the piped value over a real scram
path. Also noted where it belongs long-term — behind a Make target reading
stdin, next to secrets-rotate.
2. DELIVERY ORDER WAS SILENTLY WRONG FOR RUNNERLESS NODES. `secrets-funnel-from-prod`
calls pull_chit_bundle.sh, which takes the newest ALREADY-successful run:
gh run list --workflow "$WORKFLOW" --status success --limit 1
That run can predate the Prod secret you just created. The pull succeeds, the
funnel reports success, and JUICEFS_META_PASSWORD is absent or stale — on the
5090, which is exactly the runnerless node this slot exists for. The runbook
now requires a producer run FIRST, an explicit wait, and a createdAt
comparison against when the secret was set. That timestamp check is the whole
guarantee.
The third finding is a merge-order dependency, not a doc defect: the mount
snippet's `META_ROLE=juicefs_meta` with no DB_PASS needs the target change in
#2708. Merge #2708 first.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FkwiW3VY1xWmahTAtVioxz
POWERFULMOVES
added a commit
that referenced
this pull request
Aug 24, 2026
…procedure (#2709) * docs(ops): the metadata credential has an owner and a vendor-sourced procedure "Someone creates the GitHub Prod secret" is not an assignment. Provisioning the JuiceFS metadata credential is fleet infrastructure — it gates every node's ability to mount pmoves-media — so it belongs to Z890 (Infrastructure Coordinator), not to whichever node happens to notice it is missing. B850 holds the metadata DB and should not become the ambient owner of a secret it merely consumes. Written against JuiceFS community documentation rather than our own habits, and the comparison found four things: ALIGNED Dedicated user + network-scoped pg_hba is the vendor's own recommended shape. PR #2702 landed it scoped to the tailnet. Worth recording that this was vendor practice and not local invention -- and that the doc pairs the role WITH the hba rule, which is the pairing the original lane handoff was missing. PARTIAL META_PASSWORD is documented and we use it. META_PASSWORD_FILE is also documented, and B850 already bind-mounts the secret as a file before reading it back through a shell. Adopt on next recreate. DIVERGENT sslmode=disable. Unremarkable container-to-container; not unremarkable once :5432 is on the tailnet. WireGuard covers the transport, but the session has no TLS of its own. Flagged as a decision to make AT Step 4 rather than inherit. GAP The vendor asks for restore TESTS, not just backups. B850 dumps metadata hourly to MinIO and has never restored one. An untested backup is a hypothesis. Z890 task, tracked separately. Plus a constraint worth writing down before someone proposes the obvious fix: PostgreSQL metadata must stay single-server per vendor guidance, so B850 is an SPOF by design and availability work belongs in restore, not replication. Also corrects the mount runbook, which still told operators DB_PASS was "the Supabase DB password". That predates the scoped-role cutover and is now actively wrong: pg_hba REJECTS supabase_admin from the tailnet, so following it produces an auth error that reads like a bad secret rather than a rejected role. It now passes META_ROLE explicitly, since the script still defaults to supabase_admin for back-compat. The vendor's pg_hba example is paraphrased rather than quoted -- it carries a private LAN CIDR, and the no-LAN-IPs guard is right to block that in a public repo even when the address is someone else's documentation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkwiW3VY1xWmahTAtVioxz * docs(ops): the runbook told operators to leak the password it was protecting Two Codex P1s, both correct. 1. ROTATION CONTRADICTED ITSELF. Section 2.1 says never paste the value on a CLI, then 2.2-alt handed over a `psql -c "ALTER ROLE … PASSWORD '<new>'"` that puts it in shell history AND in the container's argv. `psql -v` is no better — same argv. Replaced with a stdin form: `read -rs` (no echo, no history) piped through a shell builtin, so the value never becomes another process's argv. Validated against the live DB on a throwaway role: ALTER ROLE applied and the role authenticated with the piped value over a real scram path. Also noted where it belongs long-term — behind a Make target reading stdin, next to secrets-rotate. 2. DELIVERY ORDER WAS SILENTLY WRONG FOR RUNNERLESS NODES. `secrets-funnel-from-prod` calls pull_chit_bundle.sh, which takes the newest ALREADY-successful run: gh run list --workflow "$WORKFLOW" --status success --limit 1 That run can predate the Prod secret you just created. The pull succeeds, the funnel reports success, and JUICEFS_META_PASSWORD is absent or stale — on the 5090, which is exactly the runnerless node this slot exists for. The runbook now requires a producer run FIRST, an explicit wait, and a createdAt comparison against when the secret was set. That timestamp check is the whole guarantee. The third finding is a merge-order dependency, not a doc defect: the mount snippet's `META_ROLE=juicefs_meta` with no DB_PASS needs the target change in #2708. Merge #2708 first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkwiW3VY1xWmahTAtVioxz --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
#2683 added
META_ROLEtojuicefs-cross-node-setup.sh, but the make target (egress.mk:319) never forwarded it. So the canonical step-5 path —make juicefs-cross-node-setup META_ROLE=juicefs_meta— silently fell back tosupabase_admin, defeating the scoped-role cutover on remote nodes (the 5090 mount). Classic 'the fix exists but isn't wired end-to-end' — the same shape B850 surfaced across tonight's findings.Fix
egress.mk: forwardMETA_ROLE(defaultsupabase_admin, back-compat).DB_PASSnow falls back to the funnel-deliveredJUICEFS_META_PASSWORD(registered inchit_manifest_register.py, feat(secrets): juicefs_meta's password had no route to any other node #2705), so a node that received the secret via the pipeline just runsmake juicefs-cross-node-setup META_ROLE=juicefs_meta.juicefs-cross-node-setup.sh: matchingDB_PASS:-JUICEFS_META_PASSWORDfallback for direct invocation; refreshed the error message.Secrecy
Both
DB_PASSandJUICEFS_META_PASSWORDpass as the sub-process environment (not argv) and are handed to JuiceFS viaMETA_PASSWORD— never inps.Why now
Unblocks 5090 step 5 (mount
pmoves-media) once B850 finishes gate 4 (expose:5432). B850-local mount usesjuicefs-mount-local, so this remote-path fix doesn't affect the completed B850 cutover.🤖 Generated with Claude Code