Uh oh!
There was an error while loading. Please reload this page.
fix(installer): give bash the same hostPath dir modes as the chart and the Windows prep (#673) - #700
Conversation
…d the Windows prep (#673) _ensure_release_dirs applied a flat, recursive `chmod -R 777` to data and logs, while Get-ReleaseDirsPrepCommand (#654) and the chart's init-writable-data (#667) both apply a per-dir 2777/3777 split without recursing. Three implementations of one intent, two agreeing and one not — and the odd one out was the copy #667 said should be diffable by eye. Nothing was user-visibly broken: 777 is other-writable and carries no sticky bit, so cross-uid `data delete` worked on the bash path, and on Linux the chart's init container rewrote both dirs at pod start anyway. That absence of a symptom is why the divergence survived two PRs, and why this lands with a test rather than just a fix. - data -> 2777 (setgid, NO sticky: `data delete` unlinks as another uid, #667) - logs -> 3777 (setgid + sticky: nothing has to delete another writer's logs) - drop -R: the dir's own mode governs creation and unlink; recursing stamped setgid/sticky onto every data FILE and walked the whole dataset tree to do it - mysql keeps its recursive 777 — one writer, its own init container, datadir permissions are the database's business (out of scope in #654 for the same reason) - split the pairs on the LAST colon, so a HOST_DATA_DIR containing one can't silently chmod a path that does not exist Tests: hostpath-prep.bats now extracts path:mode pairs from all three sources (bash _release_dirs_spec, the ps1's Get-ReleaseDirsSpec rows + $TB_*_DIR_MODE constants, the chart's init-writable-data loop) and fails if any pair disagrees; cluster.bats asserts the applied modes, that a pre-existing file under data/logs keeps its mode, that mysql stays recursive, and the colon case. Each guard was mutation-checked in all three sources. Closes#673 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
shujaatTracebloc
commented
Aug 13, 2026
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 212a854. Configure here.
saadqbal
left a comment
There was a problem hiding this comment.
Clean PR. Verified the three specs land the same modes — data 2777 (setgid, no sticky), logs 3777 (setgid+sticky) — and confirmed by direct observation that data/logs get exactly those bits while a pre-existing data file keeps its mode (the -R is genuinely gone). Nice touch splitting on the last colon, and the runtime parity test against both the ps1 and the chart is the right guard against this drifting a fourth time. Mutation-checked the parity test myself (2777->777 fails it). 👍
Uh oh!
There was an error while loading. Please reload this page.
Closes#673.
The divergence
_ensure_release_dirs(scripts/lib/cluster.sh) applied a flat, recursivechmod -R 777to the release'sdataandlogsdirs, while the two other implementations of the same intent —Get-ReleaseDirsPrepCommandinscripts/install-k8s.ps1(#654) and the chart'sinit-writable-data(#667) — apply a per-dirpath:modesplit and deliberately do not recurse. Three copies, two agreeing, and the odd one out was the copy #667 said should be "diffed by eye rather than drifting".Nothing was user-visibly broken:
777is other-writable and carries no sticky bit, so cross-uiddata deleteworked on the bash path, and on Linuxinit-writable-datarewrote both dirs at pod start regardless. That absence of a symptom is precisely why the divergence survived two PRs — so this lands with a test, not just a fix.Change
scripts/lib/cluster.sh:TB_SHARED_DIR_MODE=2777/TB_LOGS_DIR_MODE=3777, named to match the PowerShell constants_release_dirs_specemitspath:modepairs (data followsHOST_DATASET_DIR, logs stay local) — same shape asGet-ReleaseDirsSpecdata→2777(setgid, no sticky: the teardown pod unlinks as a different uid than the ingest wrote as, fix(chart): drop the sticky bit on /data/shared so a dataset can be deleted #667).logs→3777(setgid + sticky)-Rdropped: the directory's own mode governs creation and unlink inside it; recursing stamped setgid/sticky onto every data file and walked the whole dataset tree to do itmysqldeliberately unchanged (still recursive777) — one writer, its own init container, datadir permissions are the database's business; out of scope in fix(installer): pre-create the hostPath PV dirs on Windows so the first ingest works #654 for the same reasonHOST_DATA_DIRcontaining one can't silently chmod a path that doesn't existscripts/manifest.sha256regenerated for the changed lib.Tests
scripts/tests/hostpath-prep.bats— the parity guard the issue asked for. Three extractors each read their own source of truth and reduce it to<dir>:<mode>pairs, compared for equality: bash_release_dirs_spec, the ps1'sGet-ReleaseDirsSpecrows with the$TB_*_DIR_MODEconstants resolved (parsed, not executed, so it still runs with no pwsh), and the chart'sfor e in /data/shared:2777 /data/logs:3777loop. Paths legitimately differ between the three, so the comparison is on the shared meaning. Plus a static check that exactly one recursive chmod remains and it is mysql's.scripts/tests/cluster.bats— behavioral: the applied modes (setgid/other-write/sticky read via POSIXls -ldn, not GNU-onlystat -c); a pre-existing file under data/logs keeps mode600, which is what proves the recursion is gone; mysql still recursive; the colon-path case. The mode assertions skip on a filesystem/user that cannot hold setgid+sticky, since the chmod is best-effort by design.Verification
Each guard was mutation-checked rather than merely run green:
TB_SHARED_DIR_MODE2777→777TB_LOGS_DIR_MODE3777→2777/data/logs:3777→1777${entry##*:}→${entry#*:}bash -nandshellcheck --severity=errorclean (the twoSC2034warnings oncluster.share pre-existing),check-style.shclean,gen-manifest.sh --checkpasses,bats-hygiene.bats18/18.Full bats suite: 976 ok, one failure —
install-bootstrap.bats"early bailout: healthy tracebloc doctor" — which reproduces identically on a cleandevelopcheckout (verified by stashing), so it is pre-existing and unrelated to this change.No chart or values change, so no
Chart.yamlbump is required bychart-version-guard.🤖 Generated with Claude Code
Note
Low Risk
Install-time directory permissions on the host only; behavior is best-effort chmod with chart init as a Linux fallback, and tests lock parity with existing Windows/chart semantics.
Overview
Aligns the bash installer's per-release hostPath prep with the Windows installer (#654) and the chart's
init-writable-data(#667), which had already adopted a 2777 (data) / 3777 (logs) split while bash still used recursive 777.scripts/lib/cluster.sh: AddsTB_SHARED_DIR_MODE/TB_LOGS_DIR_MODEand_release_dirs_specforpath:modepairs (data followsHOST_DATASET_DIR; logs stay local)._ensure_release_dirsapplies non-recursivechmodon those dirs only—2777 on data avoids a sticky bit that would block cross-uiddata delete(#667); 3777 on logs keeps setgid + sticky. mysql stays on recursive 777 (out of scope). Paths split on the last colon soHOST_DATA_DIRmay contain:.Tests:
cluster.batscovers modes, non-recursive behavior, mysql, colon paths, and_release_dirs_spec.hostpath-prep.batsadds three-way parity (bash vs ps1 vs chart) and a static check that only mysql useschmod -R.scripts/manifest.sha256updated forcluster.sh.Reviewed by Cursor Bugbot for commit 212a854. Bugbot is set up for automated code reviews on this repo. Configure here.