docs(plan): carry-over notes from skeleton code review

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
sjat 2026-06-07 08:38:23 +02:00
parent ad2c00f84a
commit 0721ecc34c

View file

@ -922,3 +922,25 @@ Expected: repo populated on `forgejo.makerfloss.eu`.
- **RouterOS version drift:** exact CLI syntax (NTP `servers=` property, `ssh-keys/import` path) is RouterOS-7 specific; verify against the pinned version from Phase 0.3 and adjust.
- **`net_put`/`net_get` over `network_cli`:** depends on SCP being available on the RouterOS SSH service; if it fails, fall back to importing the key by pasting its contents via `/user/ssh-keys/...` or enabling SCP.
- **`changed_when: false`** is used widely because the `command` module can't detect RouterOS state changes; idempotency comes from the `:if [find]` guards. Revisit if you want accurate change reporting (parse command output).
## Carry-over notes from the skeleton code review (Tasks 13, done 2026-06-07)
The no-device tasks (13) are implemented, reviewed, and committed on branch
`feat/initial-scaffolding`. The code-quality review of the role skeleton raised these
points to handle WHEN the device task files (Tasks 59) are written:
- **`switch_ssh_port` (default 22):** the identity task will *set* the SSH port. If the
device was manually moved to a non-standard port before Ansible manages it, the first
run resets it to 22 and the connection drops. Confirm the live port matches before the
identity task runs, or override `switch_ssh_port` in host_vars.
- **`switch_bridge_name` / `switch_admin_group`:** these default to the CRS310 factory
values (`bridge` / `full`) and are NOT overridden in host_vars. Correct for this one
device; if the bridge/group name ever differs, the VLAN and users tasks silently target
the wrong object. Add explicit host_vars overrides if a second device is ever onboarded.
- **Trunk `pvid: 1` (sfp-sfpplus1):** untagged frames on the uplink land in VLAN 1. In a
hardened VLAN design VLAN 1 is usually unused — when writing `vlans.yml`, decide
deliberately whether the trunk should accept untagged traffic at all, and comment intent.
- **host_vars `# EDIT:` placeholders:** `switch_mgmt_address/gateway/dns/ntp` in
`host_vars/crs310-maker.yml` hold plausible `10.0.99.x` placeholders. Replace with the
real values from the field guide (Step 7) and remove the `# EDIT` comments so it's
unambiguous they were updated.