chore(ansible): remove dead plays and the 26 unreferenced vault keys #627
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: konairius/homelab#627
Reference in New Issue
Block a user
Two dead plays, an orphaned role, and the vault keys nothing references.
Scope note: the original description said vault pruning was out of scope. That is no longer true — it was authorised and is included here. The description now matches the diff; the earlier wording is superseded.
Dead plays
Missed in #625, because I built that removal list from a
hosts:listing truncated withhead -40.The
postgres-exporterrole goes with them —install-postgres-monitoring.ymlwas its only consumer.Vault: 35 keys → 9
Nine are referenced by the surviving tree, all NetBird:
The 26 removed belonged to Proxmox, n8n, owncloud, the grafana and gitea OIDC pairs, woodpecker, SNMP, cloudflared, the UniFi
Polleraccount and both Home Assistant tokens.vault_postgres_exporter_passwordis among them, and only became removable once the dead play above went — which is how the play was found. Cross-referencing every vault key against the tree reported it as "still referenced", true only because a dead play referenced it.How
YAML-aware, not line-based: 47 lines for 35 keys meant at least one multi-line block value (the woodpecker SSH key), which a
seddelete would have cut mid-block. Decryption happened in a0600file outside the repo, shredded afterwards — no plaintext in the working tree.Verification
The edit round-trips through JSON, so the survivors were checked rather than assumed. Normalised hash of just those nine keys:
ansible-inventory --graphresolves andinstall-netbird-server.ymlsyntax-checks against the reduced vault. (--graphdeliberately —--listrenders every value.)This does not revoke anything
Removing a key removes Ansible's copy, nothing more. Credentials still live in their target systems must be revoked there. The two Home Assistant tokens are the clearest case: unreferenced, deleted here, and still valid in HA until revoked in its UI.
Known, not fixed here
common.ymland themonitoring-agentdefaults still point atloki.srv.renner.unoandprometheus.srv.renner.uno, both NXDOMAIN — so edge-1, nas-1 and claude-ws ship logs and metrics nowhere. Real bug, pre-existing, and fixing it means supplying the correct in-cluster endpoints, which is a functional change rather than deletion. Wants its own PR.Missed in the Proxmox removal. I built that list from a `hosts:` listing I had truncated with `head -40`, so anything past the fortieth line was never considered — a self-inflicted blind spot rather than a judgement call. install-postgres-monitoring.yml authentik-db-1.srv.renner.uno and prometheus.srv.renner.uno, both NXDOMAIN configure-audiobookshelf.yml audiobookshelf.srv.renner.uno is NXDOMAIN; audiobookshelf runs in-cluster The postgres-exporter role goes with them — install-postgres-monitoring.yml was its only consumer. Found by cross-referencing every vault key against the surviving tree rather than by looking at plays again: vault_postgres_exporter_password came back as "still referenced", which was only true because a dead play still referenced it. That check now leaves the group_vars/all vault holding nine keys anything uses, all NetBird, and twenty-six nothing does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>🤖 Automated review — model
claude-sonnet-5(complexity: normal), commitc75c7accd5e9110cc36e3253b961dbae0e599fec.Summary
Pure deletion PR: removes two dead Ansible plays (
configure-audiobookshelf.yml,install-postgres-monitoring.yml) and thepostgres-exporterrole, which was only consumed by the removed play. Follow-up to #625, correcting a truncatedhosts:scan. No blocking issues found.🟡 Suggestions
ansible/inventory/group_vars/all/vault.ymlstill containsvault_postgres_exporter_password, now referenced by nothing (the PR body acknowledges this and defers it as a separate change since it requires editing the encrypted vault). Worth a tracking issue/reminder so it isn't forgotten — orphaned vault secrets are easy to lose track of.Tests
No test suite applies here — this is a pure deletion of unused Ansible plays/role. I verified by grepping the full checkout that nothing else references
postgres-exporter,install-postgres-monitoring, orconfigure-audiobookshelf, thatvault_postgres_exporter_passwordhas no remaining consumers, and that neitherauthentik-db-1.srv.renner.unonor the audiobookshelf host appear anywhere inansible/inventory/. Audiobookshelf's actual deployment lives underkubernetes/apps/audiobookshelf/(in-cluster), consistent with the PR's claim that the bare-metal play is obsolete. No.woodpeckerpipeline references the deleted files either.Recommendation
Looks reasonable — no blocking issues; comment above is optional.
group_vars/all/vault.yml held 35 keys. Nine are referenced by the surviving tree, all NetBird: netbird_oidc_client_id vault_netbird_auth_client_secret netbird_oidc_client_secret vault_netbird_datastore_key netbird_oidc_issuer vault_netbird_proxy_token netbird_turn_secret vault_netbird_relay_secret vault_netbird_turn_secret The other 26 belonged to Proxmox, n8n, owncloud, the grafana and gitea OIDC pairs, woodpecker, SNMP, cloudflared, the UniFi Poller account and both Home Assistant tokens — services that now run in-cluster with their own credentials, or no longer exist at all. Edited YAML-aware rather than line-based: at least one value was a multi-line block, which a line delete would have corrupted. Verified the nine survivors are byte-identical, by comparing a normalised hash of just those keys before and after, and again after re-encrypting: e9fbbcef4127b84289ebf32e3b99e59a68e96e275af3c8c17d8ad0414a08e790 Same hash all three times. Decryption happened in a 0600 file outside the repo, shredded afterwards; no plaintext touched the working tree. Removing a key here does NOT revoke the credential. Anything still live in its target system needs revoking there — that is a separate exercise, and the Home Assistant tokens in particular are unreferenced here but valid in HA. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Vault pruned in
97337960— this PR now removes the 26 orphaned keys as well as the two dead plays.35 keys → 9, all NetBird. The other 26 belonged to Proxmox, n8n, owncloud, the grafana and gitea OIDC pairs, woodpecker, SNMP, cloudflared, the UniFi
Polleraccount and both Home Assistant tokens.How it was done
YAML-aware, not line-based — 47 lines for 35 keys meant at least one multi-line block value (the woodpecker SSH key), which a
seddelete would have corrupted mid-block.Decryption happened in a
0600file outside the repo, shredded afterwards. No plaintext touched the working tree at any point.Verification
The round-trip goes through JSON, so I checked the survivors were untouched rather than assuming. Normalised hash of just those nine keys:
Identical all three times.
ansible-inventory --graphstill resolves andinstall-netbird-server.ymlstill syntax-checks against the reduced vault.(
--graph, deliberately —--listrenders every value, which is how I leaked a set of these in the first place.)What this does not do
Removing a key here does not revoke anything. Any credential still live in its target system has to be revoked there. That applies squarely to the two Home Assistant tokens: unreferenced by Ansible, deleted from this vault, and still perfectly valid in Home Assistant until revoked in its UI.
🤖 Automated review — model
CLI default(complexity: complex), commit9733796067f95eec40f21c994d64e80b2c6c2df0.Summary
The PR description says it removes two dead plays (
configure-audiobookshelf.yml,install-postgres-monitoring.yml) and the now-unusedpostgres-exporterrole, and explicitly states that pruning the 26 orphaned vault keys is out of scope ("a separate change ... which I am not doing unattended"). The actual diff and the checked-out HEAD commit contradict that:ansible/inventory/group_vars/all/vault.ymlis rewritten far beyond what a single-key removal would produce, and the commit this PR head points to is titled "chore(ansible): drop the 26 vault keys nothing references" — the very change the PR body says isn't happening here. That mismatch is a blocking issue on its own, independent of whether the underlying key pruning is correct.🔴 Blocking
ansible/inventory/group_vars/all/vault.yml(whole-file diff) — PR body contradicts the actual change. The diff removes 200 lines of ciphertext and adds 41 (diff.patch:6-247). Ansible Vault 1.1 uses AES-256-CTR with no block padding, so ciphertext length tracks plaintext length almost 1:1 — a ~5x shrink corresponds to several kilobytes of removed plaintext, consistent with dropping roughly 26 of 35 vault keys, not the singlevault_postgres_exporter_passwordentry this PR's stated scope would touch. This matches the checked-out HEAD commit message verbatim: "chore(ansible): drop the 26 vault keys nothing references," a change the PR description explicitly disclaims doing here ("Deleting them is a separate change ... which I am not doing unattended"). Whether or not the key pruning itself is correct, a reviewer approving based on the PR description would not know they're also authorizing a mass deletion of encrypted credentials (Proxmox, n8n, owncloud, grafana/gitea OIDC, woodpecker, SNMP, cloudflared, UniFi, Home Assistant tokens per the commit message) — this needs to either be split into its own PR (as the description originally promised) or the description needs to be corrected to describe what's actually in the diff before merge.🟡 Suggestions
prometheus.srv.renner.unolooks inconsistent with live config.ansible/plays/common.yml:20andansible/roles/monitoring-agent/defaults/main.yml:20both still hard-codeprometheus_remote_write_endpoint: "http://prometheus.srv.renner.uno:9090/api/v1/write"as the active remote-write target used by the monitoring-agent role deployed across hosts. If that host truly returns NXDOMAIN as the PR body claims, the monitoring pipeline is already broken independent of this PR, which is worth confirming/flagging rather than silently leaving as-is. This doesn't block removinginstall-postgres-monitoring.yml(that play only installed the exporter binary on the Prometheus host, it isn't what does the remote-write), but it's a discrepancy in the stated reasoning that's easy to double check.Tests
No tests exist for these Ansible plays/roles in this repo (consistent with the rest of the tree), and none are expected here. The repo-wide grep confirms the claim in the PR body: nothing in
ansible/,.woodpecker/pipelines, or elsewhere referencesconfigure-audiobookshelf.yml,install-postgres-monitoring.yml, or thepostgres-exporterrole/vars after this change — that part of the cleanup is verifiably safe.Recommendation
Request changes — 1 blocking issue (PR description does not match the actual diff/commit for the vault.yml change).
chore(ansible): remove two more plays whose hosts are goneto chore(ansible): remove dead plays and the 26 unreferenced vault keysBlocking finding is correct and the description is now fixed, along with the title.
This was a process failure rather than a code one. I opened the PR describing vault pruning as out of scope — "a separate change ... which I am not doing unattended" — then it was authorised, I pushed
97337960to the same branch, and left the description saying the opposite of what the diff does. I added a comment explaining the pruning, which is not the same thing: the body is what a reviewer reads first, and on a change to an encrypted secrets file a description that understates the scope is exactly the wrong failure direction.The reviewer caught it the right way too — not by trusting either the description or the commit title, but by noticing the ciphertext shrank ~5x, far more than the described change could account for.
The title now says what it does:
remove dead plays and the 26 unreferenced vault keys.On the second finding
prometheus.srv.renner.unoincommon.yml/monitoring-agentdefaults is indeed live and indeed dead — as isloki.srv.renner.uno. Both NXDOMAIN, so edge-1, nas-1 and claude-ws are shipping logs and metrics into nothing.Still not fixing it here, and now stated in the description rather than only in a comment. It is pre-existing, and the fix is not a deletion — it needs the correct in-cluster endpoints substituted, which is a functional change I would rather not fold into a removal PR or guess at. It deserves its own change with someone confirming the right targets.