fix: [nomad-step-2] S2.5 — bin/disinto init --import-env / --import-sops / --age-key wire-up (#883) #907

Merged
dev-bot merged 2 commits from fix/issue-883-2 into main 2026-04-16 19:44:45 +00:00
Collaborator

Fixes #883

Changes

Fixes #883 ## Changes
dev-bot added 1 commit 2026-04-16 19:04:38 +00:00
fix: [nomad-step-2] S2.5 — bin/disinto init --import-env / --import-sops / --age-key wire-up (#883)
All checks were successful
ci/woodpecker/push/ci Pipeline was successful
ci/woodpecker/push/nomad-validate Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/nomad-validate Pipeline was successful
ci/woodpecker/pr/smoke-init Pipeline was successful
7081c98a79
Wire the Step-2 building blocks (import, auth, policies) into
`disinto init --backend=nomad` so a single command on a fresh LXC
provisions cluster + policies + auth + imports secrets + deploys
services.

Adds three flags to `disinto init --backend=nomad`:
  --import-env PATH   plaintext .env from old stack
  --import-sops PATH  sops-encrypted .env.vault.enc (requires --age-key)
  --age-key PATH      age keyfile to decrypt --import-sops

Flow: cluster-up.sh → vault-apply-policies.sh → vault-nomad-auth.sh →
(optional) vault-import.sh → deploy.sh. Policies + auth run on every
nomad real-run path (idempotent); import runs only when --import-* is
set; all layers safe to re-run.

Flag validation:
  --import-sops without --age-key → error
  --age-key without --import-sops → error
  --import-env alone (no sops)    → OK
  --backend=docker + any --import-* → error

Dry-run prints a five-section plan (cluster-up + policies + auth +
import + deploy) with every argv that would be executed; touches
nothing, logs no secret values.

Dry-run output prints one line per --import-* flag that is actually
set — not in an if/elif chain — so all three paths appear when all
three flags are passed. Prior attempts regressed this invariant.

Tests:
  tests/disinto-init-nomad.bats +10 cases covering flag validation,
  dry-run plan shape (each flag prints its own path), policies+auth
  always-on (without --import-*), and --flag=value form.

Docs: docs/nomad-migration.md new file — cutover-day runbook with
invocation shape, flag summary, idempotency contract, dry-run, and
secret-hygiene notes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator

AI Review

Blocker

  • bug docs/nomad-migration.md:50 / bin/disinto:796-822: The flag table says --empty "skips policies/auth/import/deploy", but _disinto_init_nomad has no $empty gate around the policies block (line 796), the auth block (line 810), or the import block (line 824). Only a log echo (lines 711-715) branches on $empty. An operator running --backend=nomad --empty to reach the documented "cluster-only escape hatch" will still invoke vault-apply-policies.sh and vault-nomad-auth.sh, which contradicts the runbook.

    Fix: either add [ "$empty" = "true" ] && { ...; exit 0; } after the cluster-up block to skip policies/auth/import when --empty is set, or correct the docs to match the real behavior (policies+auth always run; only deploy is skipped via the with_services guard).

    Signal: confirmed by grep — the only empty branch in the function is the echo at lines 711-715; no gate exists before line 796.

Non-blocking observation

  • minor bin/disinto:687-706: The pre-flight existence comment says "since we unconditionally invoke policies+auth" but the guard (if [ "$import_any" = true ]) only checks vault_policies_sh and vault_auth_sh when an import flag is set. On an import-free run those scripts are still invoked (lines 799, 813) without pre-flight verification — error still surfaces at execution, but as a raw shell "no such file" rather than the clean Error: ... not found or not executable message. Consider expanding the guard to cover all three scripts unconditionally, or at least matching the comment's stated rationale.

Verdict

REQUEST_CHANGES — --empty docs claim (skip policies/auth/import) contradicts code: no gate exists around those sections


Reviewed at 7081c98 | AGENTS.md

## AI Review <!-- reviewed: 7081c98a79e5e0612ba286b565425b110d7d524a --> ### Blocker - **bug** `docs/nomad-migration.md:50` / `bin/disinto:796-822`: The flag table says `--empty` "skips policies/auth/import/deploy", but `_disinto_init_nomad` has no `$empty` gate around the policies block (line 796), the auth block (line 810), or the import block (line 824). Only a log echo (lines 711-715) branches on `$empty`. An operator running `--backend=nomad --empty` to reach the documented "cluster-only escape hatch" will still invoke `vault-apply-policies.sh` and `vault-nomad-auth.sh`, which contradicts the runbook. **Fix:** either add `[ "$empty" = "true" ] && { ...; exit 0; }` after the cluster-up block to skip policies/auth/import when `--empty` is set, or correct the docs to match the real behavior (policies+auth always run; only deploy is skipped via the `with_services` guard). **Signal:** confirmed by grep — the only `empty` branch in the function is the echo at lines 711-715; no gate exists before line 796. ### Non-blocking observation - **minor** `bin/disinto:687-706`: The pre-flight existence comment says "since we unconditionally invoke policies+auth" but the guard (`if [ "$import_any" = true ]`) only checks `vault_policies_sh` and `vault_auth_sh` when an import flag is set. On an import-free run those scripts are still invoked (lines 799, 813) without pre-flight verification — error still surfaces at execution, but as a raw shell "no such file" rather than the clean `Error: ... not found or not executable` message. Consider expanding the guard to cover all three scripts unconditionally, or at least matching the comment's stated rationale. ### Verdict **REQUEST_CHANGES** — --empty docs claim (skip policies/auth/import) contradicts code: no gate exists around those sections --- *Reviewed at `7081c98` | [AGENTS.md](AGENTS.md)*
review-bot requested changes 2026-04-16 19:20:41 +00:00
Dismissed
review-bot left a comment
Collaborator

AI Review: REQUEST_CHANGES — --empty docs claim (skip policies/auth/import) contradicts code: no gate exists around those sections

AI Review: **REQUEST_CHANGES** — --empty docs claim (skip policies/auth/import) contradicts code: no gate exists around those sections
dev-bot force-pushed fix/issue-883-2 from 7081c98a79 to ece5d9b6cc 2026-04-16 19:25:51 +00:00 Compare
Collaborator

AI Re-review (round 2)

Previous Findings\n- blocker bin/disinto no $empty gate around policies/auth/import blocks → FIXED: if [ "$empty" = "true" ]; then exit 0; fi inserted after cluster-up in both the dry-run path (line 736) and the real-run path (line 813). Dry-run plan now matches real execution under --empty.\n- minor pre-flight guard only checked scripts when import_any=true despite unconditional invocation → FIXED: guard widened to if [ "$empty" != "true" ], covering policies+auth on every non-empty path.\n\n### New Issues\nNone. Fixup also adds caller-side rejection of --empty + --import-* (line 1032–1034) and docs/nomad-migration.md entry for that mutual exclusion — both correct. Tests cover the new --empty dry-run short-circuit and the --empty + --import-* error path.

Verdict

APPROVE — Previous blocker closed: empty gates added after cluster-up in both dry-run and real-run paths; non-blocking pre-flight guard also expanded; no collateral damage in fixup hunks


Reviewed at ece5d9b | Previous: 7081c98 | AGENTS.md

## AI Re-review (round 2) <!-- reviewed: ece5d9b6cc640a3c67f8789f7f40a38902440707 --> ### Previous Findings\n- **blocker** `bin/disinto` no `$empty` gate around policies/auth/import blocks → **FIXED**: `if [ "$empty" = "true" ]; then exit 0; fi` inserted after cluster-up in both the dry-run path (line 736) and the real-run path (line 813). Dry-run plan now matches real execution under `--empty`.\n- **minor** pre-flight guard only checked scripts when `import_any=true` despite unconditional invocation → **FIXED**: guard widened to `if [ "$empty" != "true" ]`, covering policies+auth on every non-empty path.\n\n### New Issues\nNone. Fixup also adds caller-side rejection of `--empty` + `--import-*` (line 1032–1034) and docs/nomad-migration.md entry for that mutual exclusion — both correct. Tests cover the new `--empty` dry-run short-circuit and the `--empty` + `--import-*` error path. ### Verdict **APPROVE** — Previous blocker closed: empty gates added after cluster-up in both dry-run and real-run paths; non-blocking pre-flight guard also expanded; no collateral damage in fixup hunks --- *Reviewed at `ece5d9b` | Previous: `7081c98` | [AGENTS.md](AGENTS.md)*
review-bot approved these changes 2026-04-16 19:39:47 +00:00
review-bot left a comment
Collaborator

AI Re-review (round 2): APPROVE — Previous blocker closed: empty gates added after cluster-up in both dry-run and real-run paths; non-blocking pre-flight guard also expanded; no collateral damage in fixup hunks

AI Re-review (round 2): **APPROVE** — Previous blocker closed: empty gates added after cluster-up in both dry-run and real-run paths; non-blocking pre-flight guard also expanded; no collateral damage in fixup hunks
dev-bot merged commit 164851fc9b into main 2026-04-16 19:44:45 +00:00
dev-bot deleted branch fix/issue-883-2 2026-04-16 19:44:46 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: disinto-admin/disinto#907
No description provided.