vikunja/guest: add the OpenTofu LXC guest stack #1
No reviewers
Labels
No labels
needs-info
needs-triage
ready-for-agent
ready-for-human
wontfix
needs-info
needs-triage
ready-for-agent
ready-for-human
review/merge-ready
review/needs-fix
review/needs-human
review/needs-review
wayfinder:grilling
wayfinder:map
wayfinder:prototype
wayfinder:research
wayfinder:task
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
olympus/infra-tracker!1
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "hermes/guest-stack"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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:
Two things are deliberate and worth a reviewer's attention:
Verified:
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.
Code review — two axes
Reviewed the diff
c1155c4...09253ca(3 commits), merge-basemain@c1155c4.Standards
Documented-standard checks (README
## Conventions)burns, hostnamevikunja,10.12.0.142/24,10.12.0.1(variables.tfdefaults; 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 mirrordownload.proxmox.comis explicitly allowed.proxmox_download_fileresource 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)
variables.tf:ssh_keys = list(string)in theguestobject is declared, defaulted[], and never read;container.tfuseslocal.operator_pubkeyinstead:user_account { keys = [local.operator_pubkey] }. Dead field — delete it.Makefile:guest-planre-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.GUEST_IMPORT_ID := synology:vztmpl/debian-13-standard_13.6-1_amd64.tar.zst(Makefile),datastore_id = "synology"+ the URL filename incontainer.tf, and the README. Changing the template edits three files; the pair wants to be one value.GUEST_TARGETSlists both resources, i.e. equals "the whole stack"; every future resource must be appended.-targetearns its keep only if the set is a real subset.GUEST_WORD := vikunja: the name doesn't convey "apply-gate confirmation word"; the adjacent comment carries the meaning. Rename (e.g.APPLY_CONFIRM_WORD).ip/gatewayas bare strings. Light; HCL offers no better type — acceptable.No Feature Envy, Message Chains, Middle Man, Refused Bequest, or Repeated Switches found.
.terraform.lock.hclcommit is tooling-required, not a smell.Spec
(c) Implemented but wrong
make guest-importwill 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 parsesnode_name/datastore_id:content_type/file_name; on this ID the first/yieldsnode_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.)ssh_keysfield. Spec: "the operator SSH key is injected at creation" and "The guest object variable carries the guest identity." Theguestobject declaresssh_keys = list(string)(default[]), butcontainer.tfinjectslocal.operator_pubkey;var.guest.ssh_keysis never referenced. The identity field is unreachable.(b) Not asked for (scope creep)
.gitignoreadds.worktrees/. Spec only: "a .gitignore that covers .env and plan.out."help/.DEFAULT_GOAL := helpand 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 }, providerbpg/proxmox ~> 0.77, lockfile pins 0.116.0 (within~> 0.77),content_type = "vztmpl",overwrite_unmanaged = false.download_file.url/checksum_algorithm = sha256match 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 -recursiveclean,tofu validate"Success!". No secrets committed;.envandplan.outare 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-importpre-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).0e597cd7ccto01dd8b3be4Guest stack applied and verified on the cluster.
Evidence
burns, statusrunning— 2 vCPU, 2048 MB, 15 GB rootfs onfast-storage.vikunja, Debian 13 (trixie), kernel7.0.14-19-pve.systemctl is-system-running→running. Nesting is doing its job; the container is not degraded.10.12.0.142/24, gateway10.12.0.1onvmbr1.rootwith the operator key works.Plan is clean after the apply
So
ignore_changes = [url, checksum, checksum_algorithm]on thedownload_fileholds in practice: the provider did not want to replace the template after importing it. The one-timetofu importwas run before the apply, per the README.