fix: [nomad-step-2] S2.5 review — gate policies/auth/import on --empty; reject --empty + --import-* (#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

Addresses review #907 blocker: docs/nomad-migration.md claimed
--empty "skips policies/auth/import/deploy" but _disinto_init_nomad
had no $empty gate around those blocks — operators reaching the
"cluster-only escape hatch" would still invoke vault-apply-policies.sh
and vault-nomad-auth.sh, contradicting the runbook.

Changes:
- _disinto_init_nomad: exit 0 immediately after cluster-up when
  --empty is set, in both dry-run and real-run branches. Only the
  cluster-up plan appears; no policies, no auth, no import, no
  deploy. Matches the docs.
- disinto_init: reject --empty combined with any --import-* flag.
  --empty discards the import step, so the combination silently
  does nothing (worse failure mode than a clear error up front).
  Symmetric to the existing --empty vs --with check.
- Pre-flight existence check for policies/auth scripts now runs
  unconditionally on the non-empty path (previously gated on
  --import-*), matching the unconditional invocation. Import-script
  check stays gated on --import-*.

Non-blocking observation also addressed: the pre-flight guard
comment + actual predicate were inconsistent ("unconditionally
invoke policies+auth" but only checked on import). Now the
predicate matches: [ "$empty" != "true" ] gates policies/auth,
and an inner --import-* guard gates the import script.

Tests (+3):
- --empty --dry-run shows no S2.x sections (negative assertions)
- --empty --import-env rejected
- --empty --import-sops --age-key rejected

30/30 nomad tests pass; shellcheck clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude 2026-04-16 19:25:27 +00:00
parent aa3782748d
commit ece5d9b6cc
3 changed files with 67 additions and 4 deletions

View file

@ -684,13 +684,21 @@ _disinto_init_nomad() {
exit 1
fi
# Step 2/3/4 scripts must exist as soon as any --import-* flag is set,
# since we unconditionally invoke policies+auth and optionally import.
# --empty short-circuits after cluster-up: no policies, no auth, no
# import, no deploy. It's the "cluster-only escape hatch" for debugging
# (docs/nomad-migration.md). Caller-side validation already rejects
# --empty combined with --with or any --import-* flag, so reaching
# this branch with those set is a bug in the caller.
#
# On the default (non-empty) path, vault-apply-policies.sh and
# vault-nomad-auth.sh are invoked unconditionally — they are idempotent
# and cheap to re-run, and subsequent --with deployments depend on
# them. vault-import.sh is invoked only when an --import-* flag is set.
local import_any=false
if [ -n "$import_env" ] || [ -n "$import_sops" ]; then
import_any=true
fi
if [ "$import_any" = true ]; then
if [ "$empty" != "true" ]; then
if [ ! -x "$vault_policies_sh" ]; then
echo "Error: ${vault_policies_sh} not found or not executable" >&2
exit 1
@ -699,7 +707,7 @@ _disinto_init_nomad() {
echo "Error: ${vault_auth_sh} not found or not executable" >&2
exit 1
fi
if [ ! -x "$vault_import_sh" ]; then
if [ "$import_any" = true ] && [ ! -x "$vault_import_sh" ]; then
echo "Error: ${vault_import_sh} not found or not executable" >&2
exit 1
fi
@ -722,6 +730,13 @@ _disinto_init_nomad() {
"${cmd[@]}" || true
echo ""
# --empty skips policies/auth/import/deploy — cluster-up only, no
# workloads. The operator-visible dry-run plan must match the real
# run, so short-circuit here too.
if [ "$empty" = "true" ]; then
exit 0
fi
# Vault policies + auth are invoked on every nomad real-run path
# regardless of --import-* flags (they're idempotent; S2.1 + S2.3).
# Mirror that ordering in the dry-run plan so the operator sees the
@ -793,6 +808,12 @@ _disinto_init_nomad() {
sudo -n -- "${cluster_cmd[@]}" || exit $?
fi
# --empty short-circuits here: cluster-up only, no policies/auth/import
# and no deploy. Matches the dry-run plan above and the docs/runbook.
if [ "$empty" = "true" ]; then
exit 0
fi
# Apply Vault policies (S2.1) — idempotent, safe to re-run.
echo ""
echo "── Applying Vault policies ────────────────────────────"
@ -1005,6 +1026,15 @@ disinto_init() {
exit 1
fi
# --empty is the cluster-only escape hatch — it skips policies, auth,
# import, and deploy. Pairing it with --import-* silently does nothing,
# which is a worse failure mode than a clear error. Reject explicitly.
if [ "$empty" = true ] \
&& { [ -n "$import_env" ] || [ -n "$import_sops" ] || [ -n "$age_key" ]; }; then
echo "Error: --empty and --import-env/--import-sops/--age-key are mutually exclusive" >&2
exit 1
fi
# Dispatch on backend — the nomad path runs lib/init/nomad/cluster-up.sh
# (S0.4). The default and --empty variants are identical today; Step 1
# will branch on $empty to add job deployment to the default path.