Expose deploy-flake VM facts #3

Merged

vnprc merged 1 commit from agent/vm-facts-profiles into master

2026-07-14 22:45:12 +01:00

allod-agent commented

2026-07-14 22:24:14 +01:00

Member

Refs allod/nexus#6

Risk

R2 medium. This adds a top-level vmFacts output and a reusable lib.mkVmFacts builder, then bumps the secrets input to the merged rev that exposes lib.machineHostKeys and lib.vmHostKeySecretFiles. It does not change any nixosConfigurations behavior.

Validation

nix flake check
nix eval --json .#vmFacts --apply builtins.attrNames
nix eval --json '.#vmFacts."allod-dev"' | jq -e '.ip and .username and .hostKeys.active and .hostKeySecretFile'
test -r "$(nix eval --raw .#vmFacts.allod-dev.hostKeySecretFile)"
! nix eval .#vmFacts.nexus
! nix eval .#vmFacts.no-such-vm

Refs allod/nexus#6 ## Risk R2 medium. This adds a top-level `vmFacts` output and a reusable `lib.mkVmFacts` builder, then bumps the `secrets` input to the merged rev that exposes `lib.machineHostKeys` and `lib.vmHostKeySecretFiles`. It does not change any `nixosConfigurations` behavior. ## Validation - `nix flake check` - `nix eval --json .#vmFacts --apply builtins.attrNames` - `nix eval --json '.#vmFacts."allod-dev"' | jq -e '.ip and .username and .hostKeys.active and .hostKeySecretFile'` - `test -r "$(nix eval --raw .#vmFacts.allod-dev.hostKeySecretFile)"` - `! nix eval .#vmFacts.nexus` - `! nix eval .#vmFacts.no-such-vm`

allod-agent added 1 commit

2026-07-14 22:24:14 +01:00

Expose VM facts from deploy flake 2765cfd93b

allod-agent commented

2026-07-14 22:40:37 +01:00

Author

Member

Review: Expose deploy-flake VM facts

Verdict: approve, ship-ready. Correctness is sound and verified end-to-end, with strong alignment to the fail-loud, one-source-of-truth, and negative-path-testing principles. The findings below are cosmetic/minor and none block.

Verified (built and evaluated, not just source-read)

vmFacts evaluates to ["allod-dev","privacy-1"]; nexus is correctly excluded despite having host-key, username, and age-file entries, because the filter keys off the inventory type (the right source of truth) rather than off the secrets data.
All six stated validation claims reproduce here (attr names, the jq gate, readable hostKeySecretFile, and the nexus / no-such-vm eval failures).
Both new checks build green, and nix flake check --no-build passes with all four nixosConfigurations still evaluating, confirming the "no nixosConfigurations behavior change" claim.
vm-facts-negative is genuinely self-validating: it asserts that each of the five mutations throws, so the negative guards are proven to fire (validation is itself validated).
The coherence check's final while loop runs downstream of a pipe, i.e. in a subshell, so its exit 1 only terminates the subshell. I reproduced the exact pattern in a runCommand and confirmed it still fails the build: Nix builders run with set -e and the while is the pipeline's last stage, so the non-zero status propagates. Not a bug.

Findings (minor, non-blocking)

Cosmetic: nix flake check now prints warning: unknown flake output 'vmFacts' on every run (exit code still 0). lib is a recognized output and does not warn, whereas a bare top-level vmFacts does. It can be silenced by exposing the data as lib.vmFacts instead of (or in addition to) the top-level output, once the nexus consumption path is settled, or accepted consciously. Tracked as a follow-up.
Consistency: requireIp rejects a null ip, but username, hostKeySecretFile, and hostKeys.{active,staged} are only checked for key presence, not for null values (toString null would silently yield ""). This is fine in practice because the secrets credential-inventory check validates those shapes upstream, but the asymmetry is worth a note.
Very minor: the coherence names-diff relies on Nix attrNames and jq keys producing identically ordered arrays (jq -S . sorts object keys, not array elements). This holds for lowercase, hyphenated ASCII names but would be fragile under exotic names.

## Review: Expose deploy-flake VM facts **Verdict: approve, ship-ready.** Correctness is sound and verified end-to-end, with strong alignment to the fail-loud, one-source-of-truth, and negative-path-testing principles. The findings below are cosmetic/minor and none block. ### Verified (built and evaluated, not just source-read) - `vmFacts` evaluates to `["allod-dev","privacy-1"]`; `nexus` is correctly excluded despite having host-key, username, and age-file entries, because the filter keys off the inventory `type` (the right source of truth) rather than off the secrets data. - All six stated validation claims reproduce here (attr names, the jq gate, readable `hostKeySecretFile`, and the `nexus` / `no-such-vm` eval failures). - Both new checks build green, and `nix flake check --no-build` passes with all four `nixosConfigurations` still evaluating, confirming the "no `nixosConfigurations` behavior change" claim. - `vm-facts-negative` is genuinely self-validating: it asserts that each of the five mutations throws, so the negative guards are proven to fire (validation is itself validated). - The coherence check's final `while` loop runs downstream of a pipe, i.e. in a subshell, so its `exit 1` only terminates the subshell. I reproduced the exact pattern in a `runCommand` and confirmed it still fails the build: Nix builders run with `set -e` and the `while` is the pipeline's last stage, so the non-zero status propagates. Not a bug. ### Findings (minor, non-blocking) 1. Cosmetic: `nix flake check` now prints `warning: unknown flake output 'vmFacts'` on every run (exit code still 0). `lib` is a recognized output and does not warn, whereas a bare top-level `vmFacts` does. It can be silenced by exposing the data as `lib.vmFacts` instead of (or in addition to) the top-level output, once the nexus consumption path is settled, or accepted consciously. Tracked as a follow-up. 2. Consistency: `requireIp` rejects a null `ip`, but `username`, `hostKeySecretFile`, and `hostKeys.{active,staged}` are only checked for key presence, not for null values (`toString null` would silently yield `""`). This is fine in practice because the secrets `credential-inventory` check validates those shapes upstream, but the asymmetry is worth a note. 3. Very minor: the coherence names-diff relies on Nix `attrNames` and jq `keys` producing identically ordered arrays (`jq -S .` sorts object keys, not array elements). This holds for lowercase, hyphenated ASCII names but would be fragile under exotic names.

allod-agent commented

2026-07-14 22:41:54 +01:00

Author

Member

Follow-up on finding 1 (the unknown flake output 'vmFacts' warning): after checking the consumer this is by-design, not something to fix. nexus/scripts/lib/rotation-common.sh reads ${deploy_flake}#vmFacts and ${deploy_flake}#vmFacts."<vm>" directly (attr names, then ip, username, forge_key, host keys, and host-key secret file), provision-vm-from-host.sh does the same, and both have tests pinning that exact .#vmFacts CLI form; the nexus error text even points back here ("compose a profiles rev that provides them, see allod/nexus#6"). Moving the data under lib.vmFacts would break those consumers, and additionally exposing it there would not silence the warning while the top-level output still exists. Recommend keeping vmFacts top-level and accepting the warning consciously — no change needed, and no separate issue since there is nothing actionable. Findings 2 and 3 remain optional polish. Nothing else outstanding.

Follow-up on finding 1 (the `unknown flake output 'vmFacts'` warning): after checking the consumer this is by-design, not something to fix. `nexus/scripts/lib/rotation-common.sh` reads `${deploy_flake}#vmFacts` and `${deploy_flake}#vmFacts."<vm>"` directly (attr names, then ip, username, forge_key, host keys, and host-key secret file), `provision-vm-from-host.sh` does the same, and both have tests pinning that exact `.#vmFacts` CLI form; the nexus error text even points back here ("compose a profiles rev that provides them, see allod/nexus#6"). Moving the data under `lib.vmFacts` would break those consumers, and additionally exposing it there would not silence the warning while the top-level output still exists. Recommend keeping `vmFacts` top-level and accepting the warning consciously — no change needed, and no separate issue since there is nothing actionable. Findings 2 and 3 remain optional polish. Nothing else outstanding.