diff --git a/CLAUDE.md b/CLAUDE.md index 685d854..d94875c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -146,7 +146,7 @@ Single-contributor, trunk-based (no merge requests / approval gates): - Run `make deploy` — always run `make check` first and show output - Run `make tf-apply` — always run `make tf-plan` first and show output -- Modify `inventories/production/hosts.yml` directly — regenerate via `make tf-inventory` +- Modify `inventories//hosts.yml` directly — regenerate via `make tf-inventory` - Edit vault-encrypted files directly — decrypt first, re-encrypt after - Push to `main` branch - Add a collection to `requirements.yml` without a specific module need in existing role tasks diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cd5d96b..5f83f64 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -48,7 +48,7 @@ Exception: the control node is added to `hosts.yml` by hand — see ## Testing -Before opening a merge request: +Before committing or merging to main: ```bash make lint diff --git a/README.md b/README.md index 01decc6..09c734a 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ See `Makefile` for the full list of targets. │ ├── base/ # OS baseline applied to all hosts │ └── docker_host/ # Docker runtime setup │ -├── terraform/ # VM provisioning + infra DNS (see ADR-006/009) +├── terraform/ # VM provisioning only — no DNS (see ADR-006/009) │ ├── modules/ # Reusable modules (proxmox_vm) │ └── environments/ # Per-env state: staging/, production/ │ diff --git a/STATUS.md b/STATUS.md index b593059..a8ef7be 100644 --- a/STATUS.md +++ b/STATUS.md @@ -28,7 +28,7 @@ _Last reviewed: 2026-05-30._ |---|---| | `roles/base/` | Empty directory. `site.yml` references it, but it applies nothing. | | `roles/docker_host/` | Empty directory. Same. | -| `inventories/*/hosts.yml` | Placeholder stubs (commented examples); regenerated by `make tf-inventory` once Terraform has hosts | +| `inventories/*/hosts.yml` | Structured stubs with empty host maps (`hosts: {}`); regenerated by `make tf-inventory` once Terraform has hosts | | `inventories/production/group_vars/{docker_hosts,proxmox_hosts}/` | Empty dirs | So `make deploy PLAYBOOK=site` currently does effectively nothing — the roles it diff --git a/docs/decisions/003-toolchain.md b/docs/decisions/003-toolchain.md index 19967dc..ce0e15b 100644 --- a/docs/decisions/003-toolchain.md +++ b/docs/decisions/003-toolchain.md @@ -134,4 +134,4 @@ are removed. Each entry in `requirements.yml` must justify its presence. | Kubernetes/Swarm | Out of scope — Docker Compose is the right complexity level | | NixOS targets | Poor Ansible fit; all hosts standardised on Debian 13 | -Terraform is **adopted** for VM provisioning and infrastructure DNS — see `docs/decisions/006-terraform.md`. +Terraform is **adopted** for VM provisioning only (no DNS) — see `docs/decisions/006-terraform.md`. diff --git a/docs/reviews/2026-05-30-findings.json b/docs/reviews/2026-05-30-findings.json new file mode 100644 index 0000000..f5a5bbe --- /dev/null +++ b/docs/reviews/2026-05-30-findings.json @@ -0,0 +1,34 @@ +{ + "date": "2026-05-30", + "reviewed_commit": "de38d1c", + "mode": "on-demand", + "counts": {"auto_fixed": 7, "open": 17}, + "auto_fixed": [ + {"id": "F-dns-readme", "dimension": "consistency", "severity": "high", "location": "README.md:74", "description": "terraform/ tree comment claimed 'infra DNS'", "fix": "changed to 'no DNS'"}, + {"id": "F-dns-adr003", "dimension": "consistency", "severity": "high", "location": "docs/decisions/003-toolchain.md:137", "description": "closing line claimed Terraform does 'infrastructure DNS'", "fix": "changed to 'only (no DNS)'"}, + {"id": "F-status-hosts", "dimension": "drift", "severity": "medium", "location": "STATUS.md:31", "description": "hosts.yml described as 'commented examples'", "fix": "now 'structured stubs with empty host maps'"}, + {"id": "F-claude-hosts", "dimension": "consistency", "severity": "medium", "location": "CLAUDE.md:149", "description": "don't-modify bullet named only production", "fix": "generalised to inventories/"}, + {"id": "F-newrole-pr", "dimension": "drift", "severity": "low", "location": "docs/runbooks/new-role.md:80", "description": "PR/merge-request comment", "fix": "trunk-based merge note"}, + {"id": "F-contrib-mr", "dimension": "drift", "severity": "low", "location": "CONTRIBUTING.md:51", "description": "'before opening a merge request'", "fix": "'before committing or merging to main'"}, + {"id": "F-scripts-readme", "dimension": "cruft", "severity": "medium", "location": "scripts/README.md", "description": "3 scripts undocumented", "fix": "added entries for vault-pass-client.sh, check-vault-encrypted.sh, repo-scan.py"} + ], + "open": [ + {"id": "R1", "dimension": "consistency", "severity": "high", "location": "docs/decisions/005-bootstrapping.md:64-65", "description": "control-node setup still uses cp .vault_pass", "status": "new", "auto_fixable": false}, + {"id": "R2", "dimension": "drift", "severity": "high", "location": "docs/runbooks/new-host.md:6", "description": "prerequisite references .vault_pass file", "status": "new", "auto_fixable": false}, + {"id": "R3", "dimension": "drift", "severity": "high", "location": "docs/runbooks/new-host.md:129-130", "description": "'place .vault_pass' step", "status": "new", "auto_fixable": false}, + {"id": "R4", "dimension": "drift", "severity": "medium", "location": "CONTRIBUTING.md:32-33", "description": "secrets section describes .vault_pass sharing", "status": "new", "auto_fixable": false}, + {"id": "R5", "dimension": "drift", "severity": "medium", "location": "AGENTS.md:16", "description": "'never commit .vault_pass' names a removed file", "status": "new", "auto_fixable": false}, + {"id": "R6", "dimension": "consistency", "severity": "high", "location": "docs/decisions/003-toolchain.md, docs/decisions/008-testing.md", "description": "CI pipelines assume PR-to-main + approval gates vs trunk-based docs", "status": "new", "auto_fixable": false}, + {"id": "R7", "dimension": "consistency", "severity": "medium", "location": "CLAUDE.md:151", "description": "'push to main' forbidden contradicts trunk-based policy above", "status": "new", "auto_fixable": false}, + {"id": "R8", "dimension": "conformance", "severity": "high", "location": "roles/", "description": "base/docker_host empty dirs not tracked by git; site.yml would error on a clean clone", "status": "new", "auto_fixable": false}, + {"id": "R9", "dimension": "conformance", "severity": "medium", "location": "terraform/environments/*/main.tf", "description": "vlan_tag never passed; VMs land untagged vs ADR-007 srv VLAN", "status": "new", "auto_fixable": false}, + {"id": "R10", "dimension": "conformance", "severity": "low", "location": "backend.tf, Makefile, .scaffold/molecule.yml", "description": "/ placeholders unresolved", "status": "new", "auto_fixable": false}, + {"id": "R11", "dimension": "cruft", "severity": "medium", "location": "Makefile new-role + .scaffold/converge.yml", "description": "ROLE_NAME_PLACEHOLDER never substituted", "status": "new", "auto_fixable": false}, + {"id": "R12", "dimension": "conformance", "severity": "low", "location": "Makefile:154", "description": "echo \\n not expanded in sh; use printf", "status": "new", "auto_fixable": false}, + {"id": "R13", "dimension": "drift", "severity": "low", "location": "CLAUDE.md:93", "description": "group_vars tree lists control/ which does not exist", "status": "new", "auto_fixable": false}, + {"id": "R14", "dimension": "consistency", "severity": "low", "location": "docs/decisions/006-terraform.md:40", "description": "'No Galaxy roles' reused in Terraform section", "status": "new", "auto_fixable": false}, + {"id": "R15", "dimension": "consistency", "severity": "low", "location": "docs/decisions/001-architecture.md:22", "description": "DNS row reads present-tense though dns role unbuilt", "status": "new", "auto_fixable": false}, + {"id": "R16", "dimension": "consistency", "severity": "low", "location": "docs/decisions/001-architecture.md:42", "description": "'monitoring agent' baseline not in ADR-002", "status": "new", "auto_fixable": false}, + {"id": "R17", "dimension": "conformance", "severity": "low", "location": "playbooks/bootstrap.yml:12-16", "description": "raw python install hardcodes changed_when: false", "status": "new", "auto_fixable": false} + ] +} diff --git a/docs/reviews/2026-05-30-review.md b/docs/reviews/2026-05-30-review.md new file mode 100644 index 0000000..1a64fb2 --- /dev/null +++ b/docs/reviews/2026-05-30-review.md @@ -0,0 +1,98 @@ +# Repo review — 2026-05-30 + +- **Reviewed commit:** `de38d1c` +- **Mode:** on-demand (interactive); safe fixes applied, rest reported. +- **Method:** `scripts/repo-scan.py` (Phase 0) + 3 fan-out reviewers (consistency, + drift/cruft, conformance) + cross-cutting synthesis. + +## Summary + +| | high | medium | low | total | +|---|---|---|---|---| +| Auto-fixed | 2 | 3 | 2 | 7 | +| Open (reported) | 4 | 4 | 9 | 17 | + +Dominant theme: **drift created by this session's own rapid changes** — residual +`.vault_pass` references after the Vaultwarden/rbw switch, and leftover +PR/merge-request language after going trunk-based. + +## Auto-fixes applied (safe + obvious) + +| File | Fix | +|---|---| +| `README.md` | `terraform/` tree comment: "infra DNS" → "no DNS" | +| `docs/decisions/003-toolchain.md` | closing line: "and infrastructure DNS" → "only (no DNS)" | +| `STATUS.md` | hosts.yml row: "commented examples" → "structured stubs with empty host maps" | +| `CLAUDE.md` | don't-modify bullet generalised `production` → `` | +| `docs/runbooks/new-role.md` | "open PR / merge request" → "merge to main once make test passes" | +| `CONTRIBUTING.md` | "Before opening a merge request" → "Before committing or merging to main" | +| `scripts/README.md` | documented the 3 undocumented scripts | + +Also hardened `scripts/repo-scan.py` itself: the marker check no longer matches +filenames like `TODO.md`, and path-ref checks only fire inside backticks/links (no +prose false positives). + +## Open findings + +### Secrets — residual `.vault_pass` references (reported, not auto-fixed: secrets-adjacent) +- **[high] R1** `docs/decisions/005-bootstrapping.md:64-65` — control-node setup still + says `cp /secure/location/.vault_pass …`. Replace with the rbw setup (ADR-002, + rotate-secrets runbook). +- **[high] R2** `docs/runbooks/new-host.md:6` — prerequisite "you have the vault + password (`.vault_pass`)". → "rbw installed and unlocked". +- **[high] R3** `docs/runbooks/new-host.md:129-130` — "place `.vault_pass`" step → + rbw unlock. +- **[medium] R4** `CONTRIBUTING.md:32-33` — secrets section describes `.vault_pass` + secure-channel sharing → point to rbw/Vaultwarden. +- **[medium] R5** `AGENTS.md:16` — "Never read/print/commit `.vault_pass`" names a + file that no longer exists → reword to the rbw flow. + +### Workflow model — trunk-based vs PR/approval-gates (judgement) +- **[high] R6** `docs/decisions/003-toolchain.md` (CI/CD) & `docs/decisions/008-testing.md` + (CI pipeline) describe pull-requests-to-main + manual approval gates, contradicting + the trunk-based, no-MR model in CLAUDE.md/CONTRIBUTING. Decide: keep *deploy* + approval gates (reasonable even solo) but drop the PR framing, or fully reconcile. +- **[medium] R7** `CLAUDE.md:151` — "Push to `main` branch" listed as forbidden, + contradicting the trunk-based "commit straight to main" policy a few lines above. + +### Roles not in git (judgement) +- **[high] R8** `roles/` — `base` and `docker_host` are empty dirs on disk and **not + tracked by git** (empty dirs aren't committed). A fresh clone has neither, so + `make deploy PLAYBOOK=site` would **error** (role-not-found), not "apply nothing" + as STATUS.md:29-30 says. Decide: scaffold them (`make new-role`) or correct the docs. + +### Terraform correctness +- **[medium] R9** module `vlan_tag` is never passed by either environment → VMs land + on untagged `vmbr0`, contradicting ADR-007's srv VLAN. Wire `vlan_tag` through. +- **[low] R10** `/` placeholders unresolved in `backend.tf` (×2), + Makefile `MOLECULE_IMAGE`, `.scaffold/molecule.yml` — non-functional as committed. + +### Scaffolding (new-role) +- **[medium] R11** `make new-role` copies `.scaffold/converge.yml` with literal + `ROLE_NAME_PLACEHOLDER`, never substituted → scaffolded molecule references a + non-existent role. Sed-substitute `NAME` or document the manual step. +- **[low] R12** `Makefile:154` writes the role README via `echo "…\n…"`; `\n` isn't + expanded in `sh` → literal `\n`. Use `printf`. + +### Other consistency / drift +- **[low] R13** `CLAUDE.md:93` — group_vars tree lists `control/`, which doesn't exist + on disk. Create it or drop the line. +- **[low] R14** `docs/decisions/006-terraform.md:40` — "No Galaxy roles." reused in the + Terraform section (an Ansible-collections concept). Reword. +- **[low] R15** `docs/decisions/001-architecture.md:22` — DNS row reads as present-tense + reality though the `dns` role is unbuilt (per STATUS.md). Add a "planned" pointer. +- **[low] R16** `docs/decisions/001-architecture.md:42` — "monitoring agent" in the + Proxmox baseline isn't reflected in ADR-002's baseline. Align. +- **[low] R17** `playbooks/bootstrap.yml:12-16` — raw python-install task hardcodes + `changed_when: false` → never reports a change. Consider an honest `changed_when`. + +## Suggested follow-up prompt + +> Remediate the boma 2026-05-30 review (`docs/reviews/2026-05-30-review.md`): +> 1. Purge the residual `.vault_pass` references R1–R5 → the rbw/Vaultwarden flow. +> 2. Decide the workflow model R6–R7 — I lean "keep deploy approval gates, drop the +> PR/merge-request framing"; reconcile ADR-003/008 and CLAUDE.md to match. +> 3. Resolve R8 — scaffold `base`/`docker_host` via `make new-role`, or correct +> STATUS.md/roles/README.md to say the roles don't exist yet. +> 4. Fix the Terraform `vlan_tag` wiring (R9). +> Report on the rest. diff --git a/docs/reviews/latest.md b/docs/reviews/latest.md new file mode 100644 index 0000000..92124a2 --- /dev/null +++ b/docs/reviews/latest.md @@ -0,0 +1,23 @@ +# Latest repo review + +Most recent: **2026-05-30** → full report: `docs/reviews/2026-05-30-review.md` + +| | high | medium | low | total | +|---|---|---|---|---| +| Auto-fixed | 2 | 3 | 2 | 7 | +| Open | 4 | 4 | 9 | 17 | + +Dominant theme: drift from this session's own changes — residual `.vault_pass` +references after the Vaultwarden/rbw switch, and leftover PR/merge-request language +after going trunk-based. + +## Suggested follow-up prompt + +> Remediate the boma 2026-05-30 review (`docs/reviews/2026-05-30-review.md`): +> 1. Purge the residual `.vault_pass` references R1–R5 → the rbw/Vaultwarden flow. +> 2. Decide the workflow model R6–R7 — I lean "keep deploy approval gates, drop the +> PR/merge-request framing"; reconcile ADR-003/008 and CLAUDE.md to match. +> 3. Resolve R8 — scaffold `base`/`docker_host` via `make new-role`, or correct +> STATUS.md/roles/README.md to say the roles don't exist yet. +> 4. Fix the Terraform `vlan_tag` wiring (R9). +> Report on the rest. diff --git a/docs/runbooks/new-role.md b/docs/runbooks/new-role.md index b466454..f0df7e4 100644 --- a/docs/runbooks/new-role.md +++ b/docs/runbooks/new-role.md @@ -77,5 +77,5 @@ to `inventories/staging/hosts.yml` for integration testing. git checkout -b role/ git add roles/ git commit -m "Add role" -# open PR / merge request in Forgejo +# merge to main once make test passes, then delete the branch ``` diff --git a/scripts/README.md b/scripts/README.md index 8b1b484..9e4233b 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -5,3 +5,9 @@ dependencies (keeps them runnable anywhere without a venv). - `tf_to_inventory.py` — reads `terraform output -json` on stdin and writes an Ansible `hosts.yml`. Invoked by `make tf-inventory`. Data contract: **ADR-009**. +- `vault-pass-client.sh` — fetches the master vault password from Vaultwarden via + `rbw`. Wired as `vault_password_file` (ADR-002). +- `check-vault-encrypted.sh` — pre-commit guard: fails if a `vault.yml` holds + plaintext secrets. +- `repo-scan.py` — Phase-0 deterministic scan for `/review-repo` (markers, broken + refs, unencrypted vaults, inventory). diff --git a/scripts/repo-scan.py b/scripts/repo-scan.py index 3ce3f9e..daac708 100644 --- a/scripts/repo-scan.py +++ b/scripts/repo-scan.py @@ -24,7 +24,9 @@ PRUNE = {".git", ".venv", ".collections", ".ansible", ".worktrees", SKIP_PREFIX = os.path.join("docs", "reviews") # don't scan our own reports SOURCE_EXTS = {".yml", ".yaml", ".j2", ".py", ".sh", ".md", ".tf", ".cfg", ".ini"} -MARKER_RE = re.compile(r"(?*${}") @@ -115,13 +117,15 @@ def scan(): if m.group(1) not in adrs: findings.append({"check": "broken-adr-ref", "severity": "medium", "path": rpath, "line": i, "detail": f"references ADR-{m.group(1)} (no such file)"}) - for m in PATH_REF_RE.finditer(line): - # Skip paths that are part of a URL (e.g. a backend API endpoint). - token_prefix = line[max(line.rfind(" ", 0, m.start()), - line.rfind("\t", 0, m.start())) + 1:m.start()] - if "://" in token_prefix: + # Only check path-like references that appear inside backticks or a + # markdown link target — bare prose ("roles/docs") is not a real path. + for cand in re.findall(r"`([^`]+)`", line) + re.findall(r"\]\(([^)]+)\)", line): + if "://" in cand: # skip URLs continue - ref = m.group(0).rstrip(".,);:`'\"") + pm = PATH_REF_RE.search(cand) + if not pm: + continue + ref = pm.group(0).rstrip(".,);:`'\"") if any(c in ref for c in PLACEHOLDER): continue if not os.path.exists(os.path.join(ROOT, ref)):