vikunja/guest: add the OpenTofu LXC guest stack #1

Merged
pit merged 6 commits from hermes/guest-stack into main 2026-10-08 15:17:27 +00:00
Owner

Builds the OpenTofu guest stack for infra-tracker: one unprivileged Debian 13 LXC on burns (vmid 142, 10.12.0.142/24, vmbr1, fast-storage, 2c/2G/16GB), created with the bpg/proxmox provider.

What is in it:

  • vikunja/guest/ -- versions.tf, providers.tf, variables.tf, keys.tf, container.tf. The guest object variable carries the guest identity; the operator SSH key is injected at creation so Ansible can reach it as root on first boot.
  • Root Makefile -- the guest targets only: guest-plan, guest, guest-import, fmt, validate. The apply gate is the plan-to-artifact / render / type-the-word / apply-the-artifact shape inherited from infra-forge.
  • Root .env.example plus a .gitignore that covers .env and plan.out.
  • README.md -- the layout, the guest values, and how to run it.

Two things are deliberate and worth a reviewer's attention:

  1. nesting = true and it is not optional. Modern systemd requires it; Debian 13 with nesting disabled comes up degraded (services failing 243/CREDENTIALS, empty console).
  2. The base image is declared, then imported once. proxmox_download_file.debian_13_base pins the same URL and sha256 another stack already manages on the synology datastore. The file already exists on the node, so make guest-import must run before the first apply; overwrite_unmanaged = false makes the provider refuse to clobber it, which is exactly what happens if the import is forgotten. Do not set it to true.

Verified:

  • tofu fmt -check -recursive clean; tofu validate passes.
  • tofu plan against the live cluster: Plan: 2 to add, 0 to change, 0 to destroy.
  • No secrets committed; the root .env is gitignored and holds the Proxmox credentials.

Out of scope, not in this PR: the Ansible service role, the npm/ Edge stack, a PRE guest, CI, ADRs, backups (Proxmox owns backups).

Note: the provider resolved to bpg/proxmox 0.116.0 under the ~> 0.77 constraint and the lockfile is committed pinning that version.

Builds the OpenTofu guest stack for infra-tracker: one unprivileged Debian 13 LXC on burns (vmid 142, 10.12.0.142/24, vmbr1, fast-storage, 2c/2G/16GB), created with the bpg/proxmox provider. What is in it: - vikunja/guest/ -- versions.tf, providers.tf, variables.tf, keys.tf, container.tf. The guest object variable carries the guest identity; the operator SSH key is injected at creation so Ansible can reach it as root on first boot. - Root Makefile -- the guest targets only: guest-plan, guest, guest-import, fmt, validate. The apply gate is the plan-to-artifact / render / type-the-word / apply-the-artifact shape inherited from infra-forge. - Root .env.example plus a .gitignore that covers .env and plan.out. - README.md -- the layout, the guest values, and how to run it. Two things are deliberate and worth a reviewer's attention: 1. nesting = true and it is not optional. Modern systemd requires it; Debian 13 with nesting disabled comes up degraded (services failing 243/CREDENTIALS, empty console). 2. The base image is declared, then imported once. proxmox_download_file.debian_13_base pins the same URL and sha256 another stack already manages on the synology datastore. The file already exists on the node, so make guest-import must run before the first apply; overwrite_unmanaged = false makes the provider refuse to clobber it, which is exactly what happens if the import is forgotten. Do not set it to true. Verified: - tofu fmt -check -recursive clean; tofu validate passes. - tofu plan against the live cluster: Plan: 2 to add, 0 to change, 0 to destroy. - No secrets committed; the root .env is gitignored and holds the Proxmox credentials. Out of scope, not in this PR: the Ansible service role, the npm/ Edge stack, a PRE guest, CI, ADRs, backups (Proxmox owns backups). Note: the provider resolved to bpg/proxmox 0.116.0 under the ~> 0.77 constraint and the lockfile is committed pinning that version.
One unprivileged Debian 13 container on node burns (vmid 142,
10.12.0.142/24). Nesting is enabled — modern systemd requires it and
Debian 13 comes up degraded without it.

The Debian 13 base image is declared as a proxmox_download_file with
overwrite_unmanaged = false, so it is imported once (make guest-import)
rather than clobbered.
guest-plan / guest / guest-import / fmt / validate, sourcing the single
root .env. guest plans to plan.out, renders it, confirms by typing the
word, then applies that artifact (copied from infra-forge). guest-import
runs the one-time tofu import of the existing base template.

Gitignore now excludes .env and plan.out.
Replace the placeholder with the layout, the guest values, the
base-image import step, the targets, and the never-commit rules.
Author
Owner

Code review — two axes

Reviewed the diff c1155c4...09253ca (3 commits), merge-base main @ c1155c4.

Standards

Documented-standard checks (README ## Conventions)

  • Branch/PR — satisfied; work is on a branch.
  • "Never commit our addresses: the DDNS hostname and the external IP stay out of the repo." — No hard violation. The diff commits node burns, hostname vikunja, 10.12.0.142/24, 10.12.0.1 (variables.tf defaults; README table). The rule enumerates the DDNS hostname and external IP; RFC1918 internals aren't literally those. Judgement call: the header says "Never commit our addresses," and internal addresses arguably fall under that intent — worth an author ack, not a block. Upstream mirror download.proxmox.com is explicitly allowed.
  • README accuracy — the README claims this stack "declares the same proxmox_download_file resource another stack already manages," but no other stack exists (npm/, vikunja/service/ are "not built yet"). Doc/reality mismatch, not a baseline smell.

Baseline smells (all judgement calls)

  1. Speculative Generality — variables.tf: ssh_keys = list(string) in the guest object is declared, defaulted [], and never read; container.tf uses local.operator_pubkey instead: user_account { keys = [local.operator_pubkey] }. Dead field — delete it.
  2. Duplicated Code — Makefile: guest-plan re-implements the preamble the comment says every Stack target shares (guest_stack_init) rather than calling it ($(call require_secret,.env) / set -a; source .env; set +a / cd $(GUEST_DIR) / tofu init -input=false). Call the macro.
  3. Duplicated Code / Data Clumps → Shotgun Surgery — the datastore + template filename live in three places: GUEST_IMPORT_ID := synology:vztmpl/debian-13-standard_13.6-1_amd64.tar.zst (Makefile), datastore_id = "synology" + the URL filename in container.tf, and the README. Changing the template edits three files; the pair wants to be one value.
  4. Speculative Generality — GUEST_TARGETS lists both resources, i.e. equals "the whole stack"; every future resource must be appended. -target earns its keep only if the set is a real subset.
  5. Mysterious Name — GUEST_WORD := vikunja: the name doesn't convey "apply-gate confirmation word"; the adjacent comment carries the meaning. Rename (e.g. APPLY_CONFIRM_WORD).
  6. Primitive Obsession — ip / gateway as bare strings. Light; HCL offers no better type — acceptable.

No Feature Envy, Message Chains, Middle Man, Refused Bequest, or Repeated Switches found. .terraform.lock.hcl commit is tooling-required, not a smell.

Spec

(c) Implemented but wrong

  1. Import ID is malformed; make guest-import will fail. Spec: "the file already exists on the node, so make guest-import must run before the first apply." Makefile: tofu import proxmox_download_file.debian_13_base synology:vztmpl/debian-13-standard_13.6-1_amd64.tar.zst (README repeats it). The provider parses node_name/datastore_id:content_type/file_name; on this ID the first / yields node_name="synology:vztmpl" and the remainder has no :, so ImportState errors. Correct ID: burns/synology:vztmpl/debian-13-standard_13.6-1_amd64.tar.zst. This breaks the stated pre-apply procedure. (Confirmed against the registry's documented import syntax.)
  2. Dead ssh_keys field. Spec: "the operator SSH key is injected at creation" and "The guest object variable carries the guest identity." The guest object declares ssh_keys = list(string) (default []), but container.tf injects local.operator_pubkey; var.guest.ssh_keys is never referenced. The identity field is unreachable.

(b) Not asked for (scope creep)

  • .gitignore adds .worktrees/. Spec only: "a .gitignore that covers .env and plan.out."
  • Makefile adds help / .DEFAULT_GOAL := help and a default-goal target. Spec: "the guest targets only: guest-plan, guest, guest-import, fmt, validate."

(a) Missing/partial — none material beyond the above.

Value cross-check (all match): vmid 142, ip 10.12.0.142/24, gw 10.12.0.1, bridge vmbr1, datastore fast-storage, cores/memory/swap/disk 2/2048/512/16, unprivileged = true, features { nesting = true }, provider bpg/proxmox ~> 0.77, lockfile pins 0.116.0 (within ~> 0.77), content_type = "vztmpl", overwrite_unmanaged = false. download_file.url / checksum_algorithm = sha256 match the mirror and the pinned checksum matches the real tarball. Makefile targets guest-plan/guest/guest-import/fmt/validate all present; apply-gate shape (plan→artifact→show→word→apply artifact) matches. tofu fmt -check -recursive clean, tofu validate "Success!". No secrets committed; .env and plan.out are gitignored.


Summary — Standards: 7 findings, 0 hard violations (worst: README asserts another stack already manages the base template, but no other stack exists). Spec: 4 findings (worst: the malformed import ID, which breaks the documented make guest-import pre-apply step).

## Code review — two axes Reviewed the diff `c1155c4...09253ca` (3 commits), merge-base `main` @ `c1155c4`. ### Standards Documented-standard checks (README `## Conventions`) - Branch/PR — satisfied; work is on a branch. - "Never commit our addresses: the DDNS hostname and the external IP stay out of the repo." — No hard violation. The diff commits node `burns`, hostname `vikunja`, `10.12.0.142/24`, `10.12.0.1` (`variables.tf` defaults; README table). The rule enumerates the DDNS hostname and external IP; RFC1918 internals aren't literally those. Judgement call: the header says "Never commit our addresses," and internal addresses arguably fall under that intent — worth an author ack, not a block. Upstream mirror `download.proxmox.com` is explicitly allowed. - README accuracy — the README claims this stack "declares the same `proxmox_download_file` resource another stack already manages," but no other stack exists (`npm/`, `vikunja/service/` are "not built yet"). Doc/reality mismatch, not a baseline smell. Baseline smells (all judgement calls) 1. **Speculative Generality** — `variables.tf`: `ssh_keys = list(string)` in the `guest` object is declared, defaulted `[]`, and never read; `container.tf` uses `local.operator_pubkey` instead: `user_account { keys = [local.operator_pubkey] }`. Dead field — delete it. 2. **Duplicated Code** — `Makefile`: `guest-plan` re-implements the preamble the comment says every Stack target shares (`guest_stack_init`) rather than calling it (`$(call require_secret,.env)` / `set -a; source .env; set +a` / `cd $(GUEST_DIR)` / `tofu init -input=false`). Call the macro. 3. **Duplicated Code / Data Clumps → Shotgun Surgery** — the datastore + template filename live in three places: `GUEST_IMPORT_ID := synology:vztmpl/debian-13-standard_13.6-1_amd64.tar.zst` (Makefile), `datastore_id = "synology"` + the URL filename in `container.tf`, and the README. Changing the template edits three files; the pair wants to be one value. 4. **Speculative Generality** — `GUEST_TARGETS` lists both resources, i.e. equals "the whole stack"; every future resource must be appended. `-target` earns its keep only if the set is a real subset. 5. **Mysterious Name** — `GUEST_WORD := vikunja`: the name doesn't convey "apply-gate confirmation word"; the adjacent comment carries the meaning. Rename (e.g. `APPLY_CONFIRM_WORD`). 6. **Primitive Obsession** — `ip` / `gateway` as bare strings. Light; HCL offers no better type — acceptable. No Feature Envy, Message Chains, Middle Man, Refused Bequest, or Repeated Switches found. `.terraform.lock.hcl` commit is tooling-required, not a smell. ### Spec (c) Implemented but wrong 1. **Import ID is malformed; `make guest-import` will fail.** Spec: "the file already exists on the node, so make guest-import must run before the first apply." Makefile: `tofu import proxmox_download_file.debian_13_base synology:vztmpl/debian-13-standard_13.6-1_amd64.tar.zst` (README repeats it). The provider parses `node_name/datastore_id:content_type/file_name`; on this ID the first `/` yields `node_name="synology:vztmpl"` and the remainder has no `:`, so ImportState errors. Correct ID: `burns/synology:vztmpl/debian-13-standard_13.6-1_amd64.tar.zst`. This breaks the stated pre-apply procedure. (Confirmed against the registry's documented import syntax.) 2. **Dead `ssh_keys` field.** Spec: "the operator SSH key is injected at creation" and "The guest object variable carries the guest identity." The `guest` object declares `ssh_keys = list(string)` (default `[]`), but `container.tf` injects `local.operator_pubkey`; `var.guest.ssh_keys` is never referenced. The identity field is unreachable. (b) Not asked for (scope creep) - `.gitignore` adds `.worktrees/`. Spec only: "a .gitignore that covers .env and plan.out." - Makefile adds `help` / `.DEFAULT_GOAL := help` and a default-goal target. Spec: "the guest targets only: guest-plan, guest, guest-import, fmt, validate." (a) Missing/partial — none material beyond the above. Value cross-check (all match): vmid 142, ip 10.12.0.142/24, gw 10.12.0.1, bridge vmbr1, datastore fast-storage, cores/memory/swap/disk 2/2048/512/16, `unprivileged = true`, `features { nesting = true }`, provider `bpg/proxmox ~> 0.77`, lockfile pins 0.116.0 (within `~> 0.77`), `content_type = "vztmpl"`, `overwrite_unmanaged = false`. `download_file.url` / `checksum_algorithm = sha256` match the mirror and the pinned checksum matches the real tarball. Makefile targets guest-plan/guest/guest-import/fmt/validate all present; apply-gate shape (plan→artifact→show→word→apply artifact) matches. `tofu fmt -check -recursive` clean, `tofu validate` "Success!". No secrets committed; `.env` and `plan.out` are gitignored. --- **Summary** — Standards: 7 findings, 0 hard violations (worst: README asserts another stack already manages the base template, but no other stack exists). Spec: 4 findings (worst: the malformed import ID, which breaks the documented `make guest-import` pre-apply step).
Spec:
- Correct the base-image import id to the provider's grammar
  (<node>/<datastore>:<content_type>/<file>). The node prefix was missing, so
  `make guest-import` failed outright ("Unexpected Import Identifier").
  Verified by running both forms: the old id fails, the corrected one imports.
- Add `lifecycle { ignore_changes = [url, checksum, checksum_algorithm] }` to
  the download_file: Proxmox stores no source url/checksum, so after the import
  the provider reads them back as null and the plan wanted to destroy and
  recreate a template another repository's guest is built from. Verified: the
  plan no longer replaces it.
- Drop the dead `ssh_keys` field from the `guest` object; the key is injected
  from `local.operator_pubkey`, so the field was unreachable.

Standards:
- `guest-plan` now calls the shared `guest_stack_init` macro instead of
  re-implementing it.
- Rename `GUEST_WORD` to `APPLY_CONFIRM_WORD` (a name that says what it is).
- Lift the template datastore and file name into a `template` variable so the
  base image is one value, not three copies of the same string.
- Replace `GUEST_TARGETS` (which listed the whole stack) with a `GUEST_EXCLUDE`
  of nothing; the stack is now planned in full.
- Correct the README, which claimed another stack "already manages" the
  template; name infra-forge and show the corrected import command.
- Drop the `.worktrees/` line from .gitignore (no worktree support here).
GUEST_TARGETS listed both resources, i.e. the whole stack — every future
resource would have to be appended to it. A bare plan is the same set and
cannot go stale.
pit force-pushed hermes/guest-stack from 0e597cd7cc to 01dd8b3be4 2026-10-08 15:12:04 +00:00 Compare
Author
Owner

Guest stack applied and verified on the cluster.

Evidence

  • vmid 142, node burns, status running — 2 vCPU, 2048 MB, 15 GB rootfs on fast-storage.
  • Guest: hostname vikunja, Debian 13 (trixie), kernel 7.0.14-19-pve.
  • systemctl is-system-running → running. Nesting is doing its job; the container is not degraded.
  • Network: 10.12.0.142/24, gateway 10.12.0.1 on vmbr1.
  • SSH as root with the operator key works.

Plan is clean after the apply

No changes. Your infrastructure matches the configuration.

So ignore_changes = [url, checksum, checksum_algorithm] on the download_file holds in practice: the provider did not want to replace the template after importing it. The one-time tofu import was run before the apply, per the README.

Guest stack applied and verified on the cluster. **Evidence** - vmid 142, node `burns`, status `running` — 2 vCPU, 2048 MB, 15 GB rootfs on `fast-storage`. - Guest: hostname `vikunja`, Debian 13 (trixie), kernel `7.0.14-19-pve`. - `systemctl is-system-running` → `running`. Nesting is doing its job; the container is not degraded. - Network: `10.12.0.142/24`, gateway `10.12.0.1` on `vmbr1`. - SSH as `root` with the operator key works. **Plan is clean after the apply** ``` No changes. Your infrastructure matches the configuration. ``` So `ignore_changes = [url, checksum, checksum_algorithm]` on the `download_file` holds in practice: the provider did not want to replace the template after importing it. The one-time `tofu import` was run before the apply, per the README.
pit merged commit cf685d9d8d into main 2026-10-08 15:17:27 +00:00
pit deleted branch hermes/guest-stack 2026-10-08 15:17:27 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
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
olympus/infra-tracker!1
No description provided.