pal-e-docs

Review: nftables reload-after-tailscale

review-400-2026-03-27 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Bug
  • [x] Lineage -- standalone, discovered during Mac agent setup (#174)
  • [x] Repo -- forgejo_admin/pal-e-platform
  • [x] What Broke -- clear description of stale ifindex problem
  • [x] Repro Steps -- concrete 3-step repro
  • [x] Expected Behavior -- well stated
  • [x] Environment -- archbox, Salt-managed, tailscale0
  • [x] Acceptance Criteria -- 3 criteria listed
  • [x] Related -- references feedback_ci_pipeline_lessons and #174
All required bug template sections present.

Traceability

  • [ ] story:X label -- missing. Foundational infrastructure work, acceptable for a bug fix.
  • [x] arch:tailscale-subnet label -- present on board item
  • [x] Forgejo issue -- #181, open

File Targets

  • [x] salt/states/firewall/ -- verified: directory exists with init.sls and nftables.conf.j2
  • [x] salt/states/firewall/init.sls -- verified: already contains nftables-after-tailscale drop-in (lines 31-45) with After=tailscaled.service and Wants=tailscaled.service
  • [x] salt/pillar/firewall.sls -- verified: tailscale0 is in allowed_interfaces (line 17)
  • [x] salt/states/services/init.sls -- verified: tailscaled managed as service.running with enable: True

Critical Finding: Existing Partial Fix

The codebase already has a systemd drop-in at salt/states/firewall/init.sls:35-45 that creates /etc/systemd/system/nftables.service.d/after-tailscale.conf with:
This ensures nftables loads after tailscaled at boot. However, the bug describes stale ifindex after Tailscale restarts (not just boot). After= only controls boot ordering -- it does NOT trigger a reload when tailscaled restarts mid-uptime. The acceptance criterion "systemd dependency or timer ensures nftables reloads after tailscaled starts" is partially met by the existing code but not fully.
The ticket needs to clarify:
  • Is the bug about boot ordering (already fixed) or mid-uptime Tailscale restarts (not fixed)?
  • If mid-uptime: the fix needs either PartOf=tailscaled.service (restart nftables when tailscaled restarts), a tailscaled ExecStartPost hook, or a systemd .path unit watching the tailscale0 interface.
  • Acceptance criterion #3 ("Salt state manages the systemd drop-in") is already done -- the state nftables-after-tailscale exists.

Repo Placement

Correct. The fix is in salt/states/firewall/ within forgejo_admin/pal-e-platform, which is where the Forgejo issue is filed. Single-repo fix.

Dependencies

  • Board item #391 (Mac build agent #174) is in next_up -- this is the parent context where the bug was discovered. No blocking dependency.
  • Board item #394 (Tailscale Connector #175) is done -- related Tailscale subnet work, no conflict.
  • No other items reference scope:firewall or block this ticket.

Acceptance Criteria Assessment

  • [x] Criterion 1: "After reboot, nft list ruleset | grep iif shows tailscale0 (not a number)" -- testable via SSH after reboot. But this may already pass with the existing drop-in.
  • [~] Criterion 2: "Systemd dependency or timer ensures nftables reloads after tailscaled starts" -- ambiguous. The After= dependency exists. If the intent is reload-on-restart, this needs a different systemd mechanism.
  • [x] Criterion 3: "Salt state manages the systemd drop-in" -- already done (nftables-after-tailscale state, line 35).
Criteria need refinement to distinguish "already done" from "new work needed."

Blast Radius

Low. The fix modifies a single systemd drop-in file on archbox only. No k8s resources affected. No downstream consumers. The Mac minion (lucass-macbook-air-1) does not include the firewall state in its top.sls, so it is unaffected.

Decomposition

Single file target (salt/states/firewall/init.sls), single repo, 3 acceptance criteria. Well within the 5-minute rule. No decomposition needed.

Recommendation

Before moving to next_up, the issue needs:
  • Clarify whether the boot-ordering fix already solves the problem -- test on archbox: reboot, check nft list ruleset | grep iif. If it shows "tailscale0", criterion 1 is already met by existing code.
  • Clarify the restart scenario -- if the bug is specifically about mid-uptime systemctl restart tailscaled, add an explicit acceptance criterion: "After systemctl restart tailscaled, nftables rules still reference iif \"tailscale0\" (not stale index)."
  • Update acceptance criteria to remove criterion 3 (already done) and add a criterion for the actual delta (likely adding PartOf=tailscaled.service to the existing drop-in).