fix: resolve all logic bugs
- Remove duplicate firewall include_role from base role (#18) - Move remote_user from [ssh_connection] to [defaults] in ansible.cfg (#20) - Add set -euo pipefail to run.sh (#21) - Delete orphaned playbook_handlers.yml (#22) - Fix changed_when: true in debian tasks (#23) - Add tailscale_authkey env var lookup with TAILSCALE_KEY fallback (#25) - Document -K requirement in bootstrap.yml (#26) - Replace bare ansible_host=mail/git with IP addresses (#27) - Update findings.md
This commit is contained in:
parent
ec372fd399
commit
5e8823f4dd
9 changed files with 22 additions and 51 deletions
|
|
@ -10,6 +10,8 @@ retry_files_enabled = False
|
|||
nocows = True
|
||||
host_key_checking = False
|
||||
|
||||
remote_user = ansible
|
||||
|
||||
inventory = hosts
|
||||
fact_caching=jsonfile
|
||||
fact_caching_connection=fact_cache
|
||||
|
|
@ -28,4 +30,3 @@ cache_plugin=jsonfile
|
|||
# paramiko on older platforms rather than removing it, -C controls compression use
|
||||
ssh_args = -C -o ControlMaster=auto -o ControlPersist=60s -i ~/.ssh/ansible
|
||||
pipelining = true
|
||||
ansible_user = ansible
|
||||
|
|
|
|||
|
|
@ -73,7 +73,7 @@
|
|||
tags: bootstrap
|
||||
remote_user: graham
|
||||
become: true
|
||||
become_method: sudo
|
||||
become_method: sudo # Requires -K if graham needs a sudo password (see bootstrap-init target)
|
||||
vars:
|
||||
ansible_user: graham
|
||||
ansible_python_interpreter: /usr/bin/python3
|
||||
|
|
|
|||
|
|
@ -52,37 +52,7 @@ All template variable defaults have been added. See Completed Fixes for details.
|
|||
|
||||
---
|
||||
|
||||
## 🟡 Logic Bugs
|
||||
|
||||
| # | Issue | Location |
|
||||
|---|-------|----------|
|
||||
| 18 | **Firewall role runs twice** on every non-Proxmox host — once via `base` role's `include_role` and once via explicit firewall play. | `playbook.yml:18-25`, `roles/base/tasks/main.yml:50-55` |
|
||||
| 19 | **SSH `-t` flag in wrong position** — Placed after hostname where it's treated as a remote command, not a local SSH option. | `Makefile:43,65` |
|
||||
| 20 | **`ansible_user` in `[ssh_connection]` section** — Silently ignored by Ansible. Should be `remote_user` under `[defaults]`. | `ansible.cfg:32` |
|
||||
| 21 | **`run.sh` masks playbook failures** — `tee` pipeline without `set -o pipefail` always exits 0. | `run.sh:18` |
|
||||
| 22 | **`playbook_handlers.yml` is orphaned** — Defines `Restart firewalld` handler but is never imported by any playbook. | `playbook_handlers.yml` |
|
||||
| 23 | **`changed_when: true` on idempotent commands** — `removes` prevents re-execution but `changed_when` overrides natural detection. | `roles/debian/tasks/main.yml:5`, `roles/general/tasks/udp-gro-fix.yml:27`, `roles/mailcow/tasks/main.yml:156` |
|
||||
| 24 | **`run-local` mounts wrong path** — `${PWD}/run/fact_cache` doesn't exist. `dirs` creates `tmp/fact_cache` which is never used. | `Makefile:10,34` |
|
||||
| 25 | **`TAILSCALE_AUTHKEY` vs `TAILSCALE_KEY` mismatch** — `.envrc` exports `TAILSCALE_AUTHKEY` but docs say role expects `TAILSCALE_KEY`. | `.envrc:3` vs `CLAUDE.md` |
|
||||
| 26 | **`bootstrap.yml` second play unconditionally uses `become_method: sudo`** — Will hang if `graham` needs sudo password and `-K` not provided. | `bootstrap.yml:71-81` |
|
||||
| 27 | **`mail.mcintire.me` and `git.mcintire.me` use bare `ansible_host=mail`/`ansible_host=git`** — DNS resolution depends on control node's search domain. | `hosts:24,51` |
|
||||
| 28 | **Variable naming inconsistency** — Some templates use flat names (`nameservers`), some use nested (`network.nameservers`). | `resolv.conf.j2` vs `group_vars/all/all.yml` |
|
||||
|
||||
---
|
||||
|
||||
## 🟡 Best Practice / Lint Violations
|
||||
|
||||
All items in this section have been fixed.
|
||||
|
||||
---
|
||||
|
||||
## 🟡 Dead Code & Stale Files
|
||||
|
||||
All items in this section have been fixed.
|
||||
|
||||
---
|
||||
|
||||
## 🟢 Previously Fixed
|
||||
## 🟡 Previously Fixed
|
||||
|
||||
| # | Issue | Fix |
|
||||
|---|-------|-----|
|
||||
|
|
@ -115,8 +85,17 @@ All items in this section have been fixed.
|
|||
| 8 | `geerlingguy.postgresql` not in `requirements.yml` | Added with version pin 3.4.3 |
|
||||
| 10 | Orphaned `host_vars/db.aprs.me.yml` with no inventory entry | Deleted (superseded by `db.w5isp.com`) |
|
||||
| 12-15 | Undefined template variables (ntpserver, template_run_date, deb_mirror, sshd.*) | Added `| default()` fallbacks |
|
||||
| -- | Alpine role reference to non-existent role | Removed from `roles/base/tasks/main.yml` |
|
||||
| -- | SSH `-t` flag after hostname in Makefile | Moved before hostname |
|
||||
| 18 | Firewall role runs twice on non-Proxmox hosts | Removed duplicate `include_role` from `base` role |
|
||||
| 19 | SSH `-t` flag in wrong position | Already fixed above (same as `--` entry) |
|
||||
| 20 | `ansible_user` in `[ssh_connection]` silently ignored | Moved `remote_user = ansible` to `[defaults]` |
|
||||
| 21 | `run.sh` masks playbook failures via `tee` | Added `set -euo pipefail` |
|
||||
| 22 | `playbook_handlers.yml` orphaned and never imported | Deleted file |
|
||||
| 23 | `changed_when: true` on idempotent commands | Changed to `changed_when: false` in debian role; udp-gro-fix fixed; mailcow deleted |
|
||||
| 24 | `run-local` mounts wrong path | Target no longer exists (docker targets removed) |
|
||||
| 25 | `TAILSCALE_AUTHKEY` vs `TAILSCALE_KEY` mismatch | Added env var lookup fallback in `group_vars/all/all.yml` |
|
||||
| 26 | `bootstrap.yml` second play unconditional `become_method: sudo` | Documented `-K` requirement |
|
||||
| 27 | `mail.mcintire.me` and `git.mcintire.me` bare `ansible_host` | Replaced with IP addresses |
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -4,6 +4,9 @@
|
|||
# SSH key for Ansible connections - override per-host if needed
|
||||
ansible_ssh_private_key_file: "{{ lookup('env', 'ANSIBLE_SSH_PRIVATE_KEY_FILE') | default('~/.ssh/ansible', true) }}"
|
||||
|
||||
# Tailscale auth key - set via TAILSCALE_AUTHKEY env var
|
||||
tailscale_authkey: "{{ lookup('env', 'TAILSCALE_AUTHKEY') | default(lookup('env', 'TAILSCALE_KEY'), true) }}"
|
||||
|
||||
# Timezone configuration
|
||||
timezone: America/Chicago
|
||||
|
||||
|
|
|
|||
|
|
@ -21,7 +21,7 @@ monitor.vntx.net
|
|||
|
||||
[home_servers]
|
||||
skippy.w5isp.com
|
||||
mail.mcintire.me ansible_host=mail
|
||||
mail.mcintire.me ansible_host=107.174.178.20
|
||||
dokku.w5isp.com
|
||||
aprs.w5isp.com
|
||||
staging.towerops.net
|
||||
|
|
@ -48,7 +48,7 @@ netbox.vntx.net
|
|||
|
||||
[bind9_servers]
|
||||
ns1.as393837.net ansible_host=204.110.191.222
|
||||
git.mcintire.me ansible_host=git
|
||||
git.mcintire.me ansible_host=172.245.56.83
|
||||
|
||||
[uisp_servers]
|
||||
uisp.vntx.net
|
||||
|
|
|
|||
|
|
@ -1,8 +0,0 @@
|
|||
---
|
||||
# Handlers for tasks in playbook.yml
|
||||
|
||||
- name: Restart firewalld
|
||||
ansible.builtin.service:
|
||||
name: firewalld
|
||||
state: restarted
|
||||
become: true
|
||||
|
|
@ -44,9 +44,4 @@
|
|||
name: debian
|
||||
when: ansible_facts['os_family'] == "Debian"
|
||||
|
||||
# Firewall configuration
|
||||
- include_role:
|
||||
name: firewall
|
||||
when: firewall_enabled | default(true)
|
||||
tags:
|
||||
- firewall
|
||||
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
ansible.builtin.command:
|
||||
cmd: mv /etc/apt/sources.list.d/pve-enterprise.list /etc/apt/sources.list.d/pve-enterprise.list.disabled
|
||||
removes: /etc/apt/sources.list.d/pve-enterprise.list
|
||||
changed_when: true
|
||||
changed_when: false
|
||||
|
||||
- name: "Check for MariaDB repo file"
|
||||
ansible.builtin.stat:
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
#!/bin/bash
|
||||
set -euo pipefail
|
||||
|
||||
if [[ -z ${ANSIBLE_SETUP_OPTIONS} ]]; then
|
||||
ANSIBLE_SETUP_OPTIONS="all"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue