diff --git a/docs/changelog-20260713-docfix194-opentofu-module-validation.md b/docs/changelog-20260713-docfix194-opentofu-module-validation.md new file mode 100644 index 0000000..b6330dd --- /dev/null +++ b/docs/changelog-20260713-docfix194-opentofu-module-validation.md @@ -0,0 +1,103 @@ +# DOCFIX-194 (2026-07-13) -- the OpenTofu gate was green over two broken modules + +`scripts/opentofu-validate.sh` printed **`PASS`** on 2026-07-13 while **two modules were flatly +broken** -- neither could have applied, and one could not even `init`. Both are modules that +Stage 3 and Stage 4 are built on. This fixes both defects and closes the blind spot that hid +them. + +## Root cause of the miss: root-only validation + +`tofu validate` against the ROOT module only parses modules the root actually **instantiates**. +A module that is not called -- or is called only from a **commented-out** block -- is **never +parsed at all**. Root's `main.tf` calls `dc-planes`, `dc-storage-pool`, `mesh-link`, +`office1-network` and `opnsense-edge`. It does NOT call: + + base-image cloudinit-vm maas-vm-host netem-link node-vm + +which is precisely the set Stage 2 (`cloudinit-vm`, `base-image`) and Stage 3 +(`node-vm`, `maas-vm-host`, `netem-link`) depend on. **They had never been parsed by any tool.** +This was only findable because a `tofu` binary now exists on the jumphost (OpenTofu v1.12.3) -- +the tree was authored in sessions with no binary to self-check, exactly as +`opentofu/README.md` warned. + +## Defect 1 -- `modules/node-vm`: `libvirt_volume` had a `format` attribute that does not exist + + format = { # WRONG -- rejected: "An argument named format is not expected here" + type = "qcow2" + } + +Verified against the provider's own schema (`tofu providers schema -json`, dmacvicar/libvirt +v0.9.8): `libvirt_volume` has **no top-level `format`**. It nests under `target`: + + target = { # CORRECT + format = { + type = "qcow2" + } + } + +`modules/base-image` had the correct nesting all along -- which is why it passed and node-vm did +not. **`node-vm` builds EVERY DC node** (PXE-boot blanks). Stage 3/4 would have failed on first +use. The module's own header even said "VERIFY with `tofu providers schema -json` before the +first real apply" -- nobody could, until now. + +## Defect 2 -- `modules/netem-link`: a destroy provisioner referencing `var.*` (could not `init`) + + provisioner "local-exec" { + when = destroy + command = "ssh ${var.vcloud_host_ssh_target} 'sudo tc qdisc del dev ${var.bridge_name} root' || true" + } + +OpenTofu rejects this **at init time**: + + Error: Invalid reference from destroy provisioner + Destroy-time provisioners and their connection configurations may only reference + attributes of the related resource, via 'self', 'count.index', or 'each.key'. + +So the module could not even initialize, let alone apply. Fixed with the canonical pattern -- +stash the values in `input`, read them back as `self.input.*`: + + input = { + ssh_target = var.vcloud_host_ssh_target + bridge_name = var.bridge_name + } + provisioner "local-exec" { + when = destroy + command = "ssh ${self.input.ssh_target} 'sudo tc qdisc del dev ${self.input.bridge_name} root' || true" + } + +`netem-link` is the DR/latency mechanism for Stage 3 and the **Stage 6 failover drill**. + +## The durable fix -- S3: validate EVERY module standalone + +The two bugs are one-line fixes. The real defect was the gate. `opentofu-validate.sh` now runs a +new **S3** stage that validates every directory under `opentofu/modules/` **standalone**, in a +TEMP COPY (so the gate never writes `.terraform/` or a module-level `.terraform.lock.hcl` into +the repo -- only the ROOT lock file is tracked, deliberately). + +**A gate that reports green over unparsed code is worse than no gate: it manufactures +confidence.** That is what happened here, and it is what S3 prevents. + +## Verification + +`tests/opentofu-validate/run-tests.sh`: **9 PASS** (T2 skipped -- can't exercise the +missing-binary guard on a box that has the binary). Three new cases: + +- **T8 (the load-bearing one):** a fixture whose ROOT IS VALID and does NOT call a BROKEN module. + Root-only validation reports PASS on it. S3 must FAIL. **If T8 ever goes green, the blind spot + is back.** +- **T9:** the same fixture shape with a CORRECT module passes -- proving T8 fails on the defect, + not on the fixture's shape. +- **T10:** all 10 real modules validate standalone. + +Fixtures use `terraform_data` (a builtin), so they need no provider download -- the cases stay +fast and need no registry access. + +Gate against the real tree: S1 PASS, S2 PASS, fmt clean, root validate clean, **all 10 modules +PASS standalone**. + +## Revert + + git revert + +Restores the two broken modules and the root-only gate. Nothing was instantiated; no live system +is affected by either the bug or the fix. diff --git a/docs/session-ledger.md b/docs/session-ledger.md index 697cfb4..92cdd18 100644 --- a/docs/session-ledger.md +++ b/docs/session-ledger.md @@ -2011,6 +2011,33 @@ scp/install/reboot path against this edge. Template reduction to a minimal bootstrap (sshd + key + console) is the next step and is NOT yet done. +## DOCFIX-194 2026-07-13: the OpenTofu gate was GREEN over two BROKEN modules + +- **`scripts/opentofu-validate.sh` printed PASS while `node-vm` and `netem-link` were flatly + broken.** Root cause: `tofu validate` on the ROOT module never parses a module the root does + not instantiate. Root calls dc-planes/dc-storage-pool/mesh-link/office1-network/opnsense-edge. + It does NOT call **base-image, cloudinit-vm, maas-vm-host, netem-link, node-vm** -- exactly the + set Stage 2 and Stage 3 are built on. Never parsed by any tool, ever. +- **node-vm:** `libvirt_volume` had a top-level `format = {...}`. Schema-verified + (`tofu providers schema -json`, dmacvicar/libvirt v0.9.8): **there is no top-level `format`** -- + it nests under `target`. base-image had it right all along. **node-vm builds EVERY DC node**; + Stage 3/4 would have failed on first use. +- **netem-link:** a destroy provisioner referenced `var.*`. OpenTofu rejects that at INIT -- the + module could not even initialize. It is the DR/latency mechanism for Stage 3 + the Stage 6 + failover drill. Fixed with the canonical `input` + `self.input.*` pattern. +- **Durable fix -- S3:** the gate now validates EVERY module standalone (in a temp copy, so it + never pollutes the repo). Harness 9 PASS; **T8 is the load-bearing case** (root VALID, uncalled + module BROKEN -> gate must FAIL). If T8 ever goes green, the blind spot is back. +- Only findable because a **tofu binary now exists** on the jumphost (v1.12.3). The tree was + authored with no binary to self-check -- precisely the risk `opentofu/README.md` flagged. +- **A gate that reports green over unparsed code is worse than no gate.** +- `docs/changelog-20260713-docfix194-opentofu-module-validation.md`. + +**STALE DOC NOTE (not yet fixed):** `docs/dc-dc-deployment-workflow.md`'s status table still says +gap #2 is open ("no tofu binary"), the tree is UNVALIDATED, and gap #17 (no ISP-uplink network) +is open. All three are now FALSE: tofu v1.12.3 is installed, the tree validates (10/10 modules), +and `office1-wan` exists. Fixing that table is queued, NOT done. + ## SAFETY SWEEP 2026-07-13 (done after the checkpoint, operator away, repo-only) - Marked every instruction that still said "render config.xml and push it to the edge" as diff --git a/opentofu/modules/netem-link/main.tf b/opentofu/modules/netem-link/main.tf index b0a64db..3ce5608 100644 --- a/opentofu/modules/netem-link/main.tf +++ b/opentofu/modules/netem-link/main.tf @@ -23,12 +23,30 @@ resource "terraform_data" "netem" { triggers_replace = [var.bridge_name, var.netem_args] + # `input` exists ONLY to make the values reachable from the destroy-time + # provisioner below. A destroy provisioner may reference ONLY `self`, + # `count.index`, or `each.key` -- never `var.*`. The original code + # interpolated var.vcloud_host_ssh_target and var.bridge_name directly into + # the destroy command, which OpenTofu rejects at INIT time: + # + # Error: Invalid reference from destroy provisioner + # Destroy-time provisioners and their connection configurations may only + # reference attributes of the related resource, via 'self', ... + # + # So this module could not even initialize, let alone apply. Stashing the + # values in `input` and reading them back as `self.input.*` is the canonical + # fix (OpenTofu's own guidance for exactly this case). (DOCFIX-194, 2026-07-13.) + input = { + ssh_target = var.vcloud_host_ssh_target + bridge_name = var.bridge_name + } + provisioner "local-exec" { command = "ssh ${var.vcloud_host_ssh_target} 'sudo tc qdisc replace dev ${var.bridge_name} root netem ${var.netem_args}'" } provisioner "local-exec" { when = destroy - command = "ssh ${var.vcloud_host_ssh_target} 'sudo tc qdisc del dev ${var.bridge_name} root' || true" + command = "ssh ${self.input.ssh_target} 'sudo tc qdisc del dev ${self.input.bridge_name} root' || true" } } diff --git a/opentofu/modules/node-vm/main.tf b/opentofu/modules/node-vm/main.tf index 986acba..71baaaf 100644 --- a/opentofu/modules/node-vm/main.tf +++ b/opentofu/modules/node-vm/main.tf @@ -27,8 +27,19 @@ name = "${var.vm_name}-disk.qcow2" pool = var.pool_name capacity = var.disk_size_bytes - format = { - type = "qcow2" + + # `format` is NOT a top-level attribute of libvirt_volume -- it nests under + # `target`. Verified against the provider's own schema (dmacvicar/libvirt + # v0.9.8, `tofu providers schema -json`): libvirt_volume's top-level + # attributes are name/pool/capacity/allocation/physical/type/target/create/ + # backing_store/... and there is NO `format` among them. The old top-level + # form was rejected outright -- "An argument named format is not expected + # here" -- so this module could never have applied. modules/base-image had + # the correct nesting all along; this one did not. (DOCFIX-194, 2026-07-13.) + target = { + format = { + type = "qcow2" + } } } diff --git a/scripts/opentofu-validate.sh b/scripts/opentofu-validate.sh index 5e6af67..ecbee79 100644 --- a/scripts/opentofu-validate.sh +++ b/scripts/opentofu-validate.sh @@ -3,7 +3,9 @@ # # Syntax/schema/semantic gate for opentofu/ (VR1 IaC, D-103). Read-only: runs the # static semantic guards (S1, S2) + `tofu fmt -check` + `tofu init -backend=false` -# + `tofu validate` against the root module. Does NOT plan or apply -- no provider +# + `tofu validate` against the root module + S3, which validates EVERY module +# STANDALONE (root-only validation never parses uncalled modules -- that blind spot +# hid two broken modules until 2026-07-13; see S3's comment). Does NOT plan or apply -- no provider # connection, no libvirt reach required. This is the harness opentofu/README.md # tells you to run before trusting anything under opentofu/, since it was # authored in a session with no tofu binary available to self-check. @@ -136,12 +138,53 @@ exit 1 fi -echo "== tofu validate ==" +echo "== tofu validate (root module) ==" if ! ( cd "$TOFU_DIR" && tofu validate ); then echo " [FAIL] validate failed" fail=1 fi +# S3: validate EVERY module standalone, not just the ones root happens to call. +# +# THE BLIND SPOT THIS CLOSES (DOCFIX-194, 2026-07-13): `tofu validate` on the root +# module only reaches modules the root actually INSTANTIATES. Modules that are not +# called -- or are called only from a commented-out block -- are NEVER PARSED. This +# gate printed a green "PASS" on 2026-07-13 while TWO modules were flat-out broken: +# +# node-vm libvirt_volume had a top-level `format = {...}` -- not a real +# attribute (it nests under `target`). Could never have applied. +# node-vm builds EVERY DC node. +# netem-link a destroy-time provisioner referenced var.* -- OpenTofu rejects +# that at INIT. The module could not even initialize. netem-link is +# the DR/latency mechanism for Stage 3 and the Stage 6 failover drill. +# +# Both would have detonated at first use, mid-deploy, in a stage where the runbook +# says "fix-forward is usually correct". A gate that reports green over unparsed code +# is worse than no gate: it manufactures false confidence. Hence this loop. +# +# Each module is validated in a TEMP COPY so the gate never writes .terraform/ or +# .terraform.lock.hcl into the repo tree (module-level lock files are noise; only the +# ROOT lock file is tracked, deliberately -- see .gitignore). +echo "== tofu validate (every module standalone -- the root-only blind spot) ==" +if [ -d "$TOFU_DIR/modules" ]; then + for moddir in "$TOFU_DIR"/modules/*/; do + mod="$(basename "$moddir")" + tmp="$(mktemp -d)" + cp -R "$moddir." "$tmp/" 2>/dev/null + rm -rf "$tmp/.terraform" "$tmp/.terraform.lock.hcl" + if out="$( cd "$tmp" && tofu init -backend=false -input=false -no-color 2>&1 && tofu validate -no-color 2>&1 )"; then + echo " [PASS] modules/$mod" + else + echo " [FAIL] modules/$mod" + printf '%s\n' "$out" | grep -E "^(Error| on | [0-9]+:)" | head -8 | sed 's/^/ /' + fail=1 + fi + rm -rf "$tmp" + done +else + echo " [WARN] no modules/ dir under $TOFU_DIR -- nothing to validate standalone" +fi + if [ "$fail" -eq 0 ]; then echo "PASS: opentofu-validate ($TOFU_DIR)" exit 0 diff --git a/tests/opentofu-validate/fixtures/s3-bad/main.tf b/tests/opentofu-validate/fixtures/s3-bad/main.tf new file mode 100644 index 0000000..6dac935 --- /dev/null +++ b/tests/opentofu-validate/fixtures/s3-bad/main.tf @@ -0,0 +1,7 @@ +# Root is deliberately VALID and deliberately does NOT call modules/broken. +# That is the whole point of this fixture: root-only validation reports PASS, +# because an uncalled module is never parsed. S3 must still catch it. +variable "unused" { + type = string + default = "root is fine" +} diff --git a/tests/opentofu-validate/fixtures/s3-bad/modules/broken/main.tf b/tests/opentofu-validate/fixtures/s3-bad/modules/broken/main.tf new file mode 100644 index 0000000..7bd4be8 --- /dev/null +++ b/tests/opentofu-validate/fixtures/s3-bad/modules/broken/main.tf @@ -0,0 +1,15 @@ +# Reproduces the real netem-link defect (DOCFIX-194): a destroy-time provisioner +# may reference ONLY self/count.index/each.key -- never var.*. OpenTofu rejects +# this at INIT. Uses terraform_data (a builtin) so the fixture needs NO provider +# download: the harness stays fast and offline. +variable "target" { + type = string + default = "host" +} + +resource "terraform_data" "broken" { + provisioner "local-exec" { + when = destroy + command = "echo ${var.target}" + } +} diff --git a/tests/opentofu-validate/fixtures/s3-good/main.tf b/tests/opentofu-validate/fixtures/s3-good/main.tf new file mode 100644 index 0000000..4c4e88f --- /dev/null +++ b/tests/opentofu-validate/fixtures/s3-good/main.tf @@ -0,0 +1,4 @@ +variable "unused" { + type = string + default = "root is fine" +} diff --git a/tests/opentofu-validate/fixtures/s3-good/modules/fine/main.tf b/tests/opentofu-validate/fixtures/s3-good/modules/fine/main.tf new file mode 100644 index 0000000..da8ffe5 --- /dev/null +++ b/tests/opentofu-validate/fixtures/s3-good/modules/fine/main.tf @@ -0,0 +1,17 @@ +# The correct form: stash the value in `input`, read it back via self.input.* +# in the destroy provisioner. +variable "target" { + type = string + default = "host" +} + +resource "terraform_data" "fine" { + input = { + target = var.target + } + + provisioner "local-exec" { + when = destroy + command = "echo ${self.input.target}" + } +} diff --git a/tests/opentofu-validate/run-tests.sh b/tests/opentofu-validate/run-tests.sh index 804d19b..222be17 100644 --- a/tests/opentofu-validate/run-tests.sh +++ b/tests/opentofu-validate/run-tests.sh @@ -1,11 +1,12 @@ #!/usr/bin/env bash # tests/opentofu-validate/run-tests.sh -- offline harness for -# scripts/opentofu-validate.sh. Only tests the guard clauses this environment -# can actually exercise (no tofu binary is available anywhere this repo has -# been worked in so far -- see opentofu/README.md). The fmt/init/validate path -# itself is UNTESTED here; that is real, logged residual risk, not an -# oversight -- run the script for real on a machine with the tofu binary -# before trusting opentofu/. +# scripts/opentofu-validate.sh. +# +# UPDATED 2026-07-13: a tofu binary IS now present on the jumphost (OpenTofu +# v1.12.3), so the fmt/init/validate path is no longer untestable. T8-T10 exercise +# S3 (module-standalone validation) for real; they SKIP if no tofu is on PATH. +# The old header claimed "no tofu binary is available anywhere this repo has been +# worked in" -- that was true when written and is now false. # Exit: 0 all pass | 1 any case failed. ASCII + LF. set -uo pipefail HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -55,5 +56,36 @@ "T7 back-compat: the old --check-memory-unit flag still works" \ --check-memory-unit "$HERE/../../opentofu" +# --- S3: the root-only blind spot (DOCFIX-194) ------------------------------- +# +# `tofu validate` on the root module NEVER PARSES a module the root does not +# instantiate. On 2026-07-13 this gate printed PASS while node-vm (a bogus +# top-level `format` on libvirt_volume) and netem-link (a destroy provisioner +# referencing var.*) were both broken -- neither is called from root's main.tf. +# Both would have detonated at first use, mid-deploy. +# +# T8's fixture is the load-bearing one: its ROOT IS VALID and does NOT call the +# broken module. A gate that only validates root reports PASS on it. If T8 ever +# goes green-when-it-should-be-red, the blind spot is back. +# +# The fixtures use `terraform_data` (a builtin), so no provider download is +# needed -- these cases stay fast and need no registry access. + +if command -v tofu >/dev/null 2>&1; then + run 1 '\[FAIL\] modules/broken' \ + "T8 S3 REJECTS a broken module that root does NOT call (rc 1) -- THE blind-spot regression test" \ + "$HERE/fixtures/s3-bad" + + run 0 '\[PASS\] modules/fine' \ + "T9 S3 ACCEPTS a correct standalone module (rc 0) -- proves T8 fails on the defect, not on the fixture shape" \ + "$HERE/fixtures/s3-good" + + run 0 '\[PASS\] modules/node-vm' \ + "T10 S3 PASSES against the real opentofu/ tree (all 10 modules validate standalone)" \ + "$HERE/../../opentofu" +else + echo " SKIP T8-T10 S3 module-standalone cases (no tofu binary on PATH)" +fi + echo; echo "RESULT: PASS=$PASS FAIL=$FAIL" [[ "$FAIL" -eq 0 ]] && { echo "ALL PASS"; exit 0; } || exit 1