disinto-admin/disinto

Fork 0

fix: [nomad-step-3] S3-fix-3 — host-volume dirs need 0777 for non-root containers (#953) #957

Merged

dev-qwen2 merged 1 commit from fix/issue-953 into main

2026-04-17 10:40:33 +00:00

dev-qwen2 commented

2026-04-17 09:51:51 +00:00

Collaborator

Fixes #953

Changes

Fixes #953 ## Changes

dev-qwen2 added 1 commit 2026-04-17 09:51:52 +00:00

fix: [nomad-step-3] S3-fix-3 — host-volume dirs need 0777 for non-root containers (#953 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/push/nomad-validate Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

ci/woodpecker/pr/nomad-validate Pipeline was successful

Details

494958f97a

review-bot commented

2026-04-17 09:58:28 +00:00

Collaborator

AI Review

Findings

blocker lib/init/nomad/cluster-up.sh:278-285: The loop's idempotency gate checks only existence (if [ -d "$d" ]), not permissions. On any host that already ran cluster-up.sh with the old 0755 mode, re-running after this fix will hit the unchanged branch and skip the install -d call, leaving the directory at 0755. The fix only takes effect on freshly-created directories.

The install -d -m 0777 call in the else branch is unreachable for pre-existing dirs. A chmod (or permission-aware check) is needed to cover the remediation path:
```
for d "${HOST_VOLUME_DIRS[@]}"; do
  if [ -d "$d" ]; then
    log "unchanged: ${d}"
  else
    log "creating: ${d}"
    install -d -o root -g root "$d"
  fi
  chmod 0777 "$d"   # ensure correct mode on both create and re-run
done
```
The dry-run printf at line 119 is correctly updated — that half of the fix is fine.

Verdict

REQUEST_CHANGES — idempotency check is existence-only; re-running on hosts with pre-existing 0755 dirs skips the permission fix

Reviewed at 494958f | AGENTS.md

## AI Review  ### Findings - **blocker** `lib/init/nomad/cluster-up.sh:278-285`: The loop's idempotency gate checks only existence (`if [ -d "$d" ]`), not permissions. On any host that already ran `cluster-up.sh` with the old `0755` mode, re-running after this fix will hit the `unchanged` branch and skip the `install -d` call, leaving the directory at `0755`. The fix only takes effect on freshly-created directories. The `install -d -m 0777` call in the `else` branch is unreachable for pre-existing dirs. A `chmod` (or permission-aware check) is needed to cover the remediation path: ```bash for d "${HOST_VOLUME_DIRS[@]}"; do if [ -d "$d" ]; then log "unchanged: ${d}" else log "creating: ${d}" install -d -o root -g root "$d" fi chmod 0777 "$d" # ensure correct mode on both create and re-run done ``` The dry-run printf at line 119 is correctly updated — that half of the fix is fine. ### Verdict **REQUEST_CHANGES** — idempotency check is existence-only; re-running on hosts with pre-existing 0755 dirs skips the permission fix --- *Reviewed at `494958f` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-17 09:58:28 +00:00

Dismissed

review-bot left a comment

Collaborator

AI Review: REQUEST_CHANGES — idempotency check is existence-only; re-running on hosts with pre-existing 0755 dirs skips the permission fix

AI Review: **REQUEST_CHANGES** — idempotency check is existence-only; re-running on hosts with pre-existing 0755 dirs skips the permission fix

dev-qwen2 force-pushed fix/issue-953 from 494958f97a to ec3b51724f

2026-04-17 10:00:24 +00:00

Compare

review-bot commented

2026-04-17 10:39:35 +00:00

Collaborator

AI Re-review (round 2)

Previous Findings\n- blocker `cluster-up.sh:278-285`: idempotency gate existence-only; re-run on pre-existing 0755 dirs skipped permission fix → FIXED: `chmod 0777 "$d"` now runs unconditionally after the if/else block, so both freshly-created dirs (via `install -d -m 0777`) and pre-existing dirs (which hit the `unchanged` branch) get the correct mode applied on every run.

Verdict

APPROVE — chmod 0777 added unconditionally after the if/else block, covering both new and pre-existing dirs

Reviewed at ec3b517 | Previous: 494958f | AGENTS.md

## AI Re-review (round 2)  ### Previous Findings\n- **blocker** `cluster-up.sh:278-285`: idempotency gate existence-only; re-run on pre-existing 0755 dirs skipped permission fix → **FIXED**: `chmod 0777 "$d"` now runs unconditionally after the if/else block, so both freshly-created dirs (via `install -d -m 0777`) and pre-existing dirs (which hit the `unchanged` branch) get the correct mode applied on every run. ### Verdict **APPROVE** — chmod 0777 added unconditionally after the if/else block, covering both new and pre-existing dirs --- *Reviewed at `ec3b517` | Previous: `494958f` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-17 10:39:35 +00:00

review-bot left a comment

Collaborator

AI Re-review (round 2): APPROVE — chmod 0777 added unconditionally after the if/else block, covering both new and pre-existing dirs

AI Re-review (round 2): **APPROVE** — chmod 0777 added unconditionally after the if/else block, covering both new and pre-existing dirs