feat: fix nftables boot ordering — start after tailscaled #81

Merged
forgejo_admin merged 1 commit from 80-feat-fix-nftables-boot-ordering-start-af into main 2026-03-15 05:34:06 +00:00

Summary

  • Adds systemd drop-in via Salt to ensure nftables starts after tailscaled
  • Fixes 10-day boot failure: nftables was failing because tailscale0 didn't exist at service start time
  • Host firewall has been inactive (INPUT ACCEPT) since March 4 due to this ordering race

Changes

  • salt/states/firewall/init.sls — 3 new states:
    • nftables-after-tailscale: deploys /etc/systemd/system/nftables.service.d/after-tailscale.conf with After=tailscaled.service + Wants=tailscaled.service
    • nftables-daemon-reload: cmd.wait runs systemctl daemon-reload when drop-in changes
    • Updated requisites: nftables-service and nftables-reload both require nftables-daemon-reload

Root Cause

nft[644]: /etc/nftables.conf:30:13-24: Error: Interface does not exist
nft[644]:         iif "tailscale0" accept

nftables.service starts at sysinit.target (very early). tailscaled.service starts after NetworkManager.service (later). The interface tailscale0 is created by tailscaled — it doesn't exist when nftables tries to load rules.

Validated

  • salt-call state.apply firewall — 6 succeeded, 0 failures
  • Drop-in file deployed to expected path
  • systemctl daemon-reload executed
  • nftables config unchanged (correct — only boot ordering affected)

Test Plan

  • salt-call state.apply firewall shows 0 failures
  • Drop-in file exists at /etc/systemd/system/nftables.service.d/after-tailscale.conf
  • After reboot: systemctl status nftables shows success
  • After reboot: nft list ruleset shows rules

Note

The actual rule loading + verification (revert timer pattern) is a separate manual step — Phase 8c deliverable 2. This PR only fixes the boot ordering so rules can load successfully.

Closes #80

Review Checklist

  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive
  • Salt state syntax validated via salt-call state.apply firewall
  • plan-pal-e-platform — Phase 8c (Host Firewall Verification)
  • bug-nftables-service-running-oneshot — original bug (service.running fix, resolved)
  • phase-pal-e-platform-network-security — Phase 8 parent
## Summary - Adds systemd drop-in via Salt to ensure nftables starts after tailscaled - Fixes 10-day boot failure: nftables was failing because `tailscale0` didn't exist at service start time - Host firewall has been inactive (`INPUT ACCEPT`) since March 4 due to this ordering race ## Changes - `salt/states/firewall/init.sls` — 3 new states: - `nftables-after-tailscale`: deploys `/etc/systemd/system/nftables.service.d/after-tailscale.conf` with `After=tailscaled.service` + `Wants=tailscaled.service` - `nftables-daemon-reload`: `cmd.wait` runs `systemctl daemon-reload` when drop-in changes - Updated requisites: `nftables-service` and `nftables-reload` both require `nftables-daemon-reload` ## Root Cause ``` nft[644]: /etc/nftables.conf:30:13-24: Error: Interface does not exist nft[644]: iif "tailscale0" accept ``` nftables.service starts at `sysinit.target` (very early). tailscaled.service starts after `NetworkManager.service` (later). The interface `tailscale0` is created by tailscaled — it doesn't exist when nftables tries to load rules. ## Validated - `salt-call state.apply firewall` — 6 succeeded, 0 failures - Drop-in file deployed to expected path - `systemctl daemon-reload` executed - nftables config unchanged (correct — only boot ordering affected) ## Test Plan - [x] `salt-call state.apply firewall` shows 0 failures - [x] Drop-in file exists at `/etc/systemd/system/nftables.service.d/after-tailscale.conf` - [ ] After reboot: `systemctl status nftables` shows success - [ ] After reboot: `nft list ruleset` shows rules ## Note The actual rule loading + verification (revert timer pattern) is a separate manual step — Phase 8c deliverable 2. This PR only fixes the boot ordering so rules can load successfully. Closes #80 ## Review Checklist - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive - [x] Salt state syntax validated via `salt-call state.apply firewall` ## Related - `plan-pal-e-platform` — Phase 8c (Host Firewall Verification) - `bug-nftables-service-running-oneshot` — original bug (service.running fix, resolved) - `phase-pal-e-platform-network-security` — Phase 8 parent
feat: fix nftables boot ordering — start after tailscaled
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
ci/woodpecker/pr/woodpecker Pipeline was successful
ci/woodpecker/pull_request_closed/woodpecker Pipeline was successful
efc4d63b0d
nftables.service was failing at boot because tailscale0 interface
didn't exist yet. The nftables config validates interface names at
load time, causing a hard failure.

Add a systemd drop-in via Salt that ensures nftables starts after
tailscaled.service, so tailscale0 is available when rules load.

Closes #80

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author
Owner

Tofu Plan Output

tailscale_acl.this: Refreshing state... [id=acl]
helm_release.nvidia_device_plugin: Refreshing state... [id=nvidia-device-plugin]
kubernetes_namespace_v1.postgres: Refreshing state... [id=postgres]
kubernetes_namespace_v1.monitoring: Refreshing state... [id=monitoring]
data.kubernetes_namespace_v1.tofu_state: Reading...
kubernetes_namespace_v1.forgejo: Refreshing state... [id=forgejo]
kubernetes_namespace_v1.tailscale: Refreshing state... [id=tailscale]
data.kubernetes_namespace_v1.pal_e_docs: Reading...
kubernetes_namespace_v1.ollama: Refreshing state... [id=ollama]
kubernetes_namespace_v1.minio: Refreshing state... [id=minio]
data.kubernetes_namespace_v1.tofu_state: Read complete after 0s [id=tofu-state]
data.kubernetes_namespace_v1.pal_e_docs: Read complete after 0s [id=pal-e-docs]
kubernetes_namespace_v1.cnpg_system: Refreshing state... [id=cnpg-system]
kubernetes_namespace_v1.woodpecker: Refreshing state... [id=woodpecker]
kubernetes_namespace_v1.keycloak: Refreshing state... [id=keycloak]
kubernetes_namespace_v1.harbor: Refreshing state... [id=harbor]
kubernetes_service_account_v1.tf_backup: Refreshing state... [id=tofu-state/tf-state-backup]
kubernetes_secret_v1.paledocs_db_url: Refreshing state... [id=pal-e-docs/paledocs-db-url]
kubernetes_role_v1.tf_backup: Refreshing state... [id=tofu-state/tf-state-backup]
helm_release.tailscale_operator: Refreshing state... [id=tailscale-operator]
kubernetes_secret_v1.dora_exporter: Refreshing state... [id=monitoring/dora-exporter]
helm_release.kube_prometheus_stack: Refreshing state... [id=kube-prometheus-stack]
kubernetes_config_map_v1.uptime_dashboard: Refreshing state... [id=monitoring/uptime-dashboard]
kubernetes_service_v1.dora_exporter: Refreshing state... [id=monitoring/dora-exporter]
helm_release.forgejo: Refreshing state... [id=forgejo]
helm_release.loki_stack: Refreshing state... [id=loki-stack]
kubernetes_secret_v1.woodpecker_db_credentials: Refreshing state... [id=woodpecker/woodpecker-db-credentials]
kubernetes_manifest.netpol_ollama: Refreshing state...
kubernetes_manifest.netpol_postgres: Refreshing state...
kubernetes_manifest.netpol_forgejo: Refreshing state...
kubernetes_manifest.netpol_minio: Refreshing state...
kubernetes_manifest.netpol_monitoring: Refreshing state...
kubernetes_manifest.netpol_cnpg_system: Refreshing state...
helm_release.cnpg: Refreshing state... [id=cnpg]
kubernetes_persistent_volume_claim_v1.keycloak_data: Refreshing state... [id=keycloak/keycloak-data]
kubernetes_service_v1.keycloak: Refreshing state... [id=keycloak/keycloak]
kubernetes_secret_v1.keycloak_admin: Refreshing state... [id=keycloak/keycloak-admin]
kubernetes_manifest.netpol_woodpecker: Refreshing state...
kubernetes_role_binding_v1.tf_backup: Refreshing state... [id=tofu-state/tf-state-backup]
kubernetes_manifest.netpol_keycloak: Refreshing state...
helm_release.ollama: Refreshing state... [id=ollama]
kubernetes_manifest.netpol_harbor: Refreshing state...
kubernetes_deployment_v1.keycloak: Refreshing state... [id=keycloak/keycloak]
kubernetes_ingress_v1.keycloak_funnel: Refreshing state... [id=keycloak/keycloak-funnel]
kubernetes_ingress_v1.grafana_funnel: Refreshing state... [id=monitoring/grafana-funnel]
kubernetes_ingress_v1.alertmanager_funnel: Refreshing state... [id=monitoring/alertmanager-funnel]
kubernetes_config_map_v1.dora_dashboard: Refreshing state... [id=monitoring/dora-dashboard]
helm_release.blackbox_exporter: Refreshing state... [id=blackbox-exporter]
kubernetes_manifest.blackbox_alerts: Refreshing state...
kubernetes_config_map_v1.pal_e_docs_dashboard: Refreshing state... [id=monitoring/pal-e-docs-dashboard]
helm_release.harbor: Refreshing state... [id=harbor]
kubernetes_deployment_v1.dora_exporter: Refreshing state... [id=monitoring/dora-exporter]
helm_release.minio: Refreshing state... [id=minio]
kubernetes_manifest.dora_exporter_service_monitor: Refreshing state...
kubernetes_ingress_v1.forgejo_funnel: Refreshing state... [id=forgejo/forgejo-funnel]
kubernetes_config_map_v1.grafana_loki_datasource: Refreshing state... [id=monitoring/grafana-loki-datasource]
kubernetes_ingress_v1.harbor_funnel: Refreshing state... [id=harbor/harbor-funnel]
minio_iam_user.tf_backup: Refreshing state... [id=tf-backup]
minio_s3_bucket.postgres_wal: Refreshing state... [id=postgres-wal]
minio_s3_bucket.tf_state_backups: Refreshing state... [id=tf-state-backups]
minio_s3_bucket.assets: Refreshing state... [id=assets]
minio_iam_policy.cnpg_wal: Refreshing state... [id=cnpg-wal]
minio_iam_policy.tf_backup: Refreshing state... [id=tf-backup]
kubernetes_ingress_v1.minio_api_funnel: Refreshing state... [id=minio/minio-api-funnel]
kubernetes_ingress_v1.minio_funnel: Refreshing state... [id=minio/minio-funnel]
minio_iam_user.cnpg: Refreshing state... [id=cnpg]
minio_iam_user_policy_attachment.cnpg: Refreshing state... [id=cnpg-20260302210642491000000001]
minio_iam_user_policy_attachment.tf_backup: Refreshing state... [id=tf-backup-20260314163610110100000001]
kubernetes_secret_v1.tf_backup_s3_creds: Refreshing state... [id=tofu-state/tf-backup-s3-creds]
kubernetes_secret_v1.cnpg_s3_creds: Refreshing state... [id=postgres/cnpg-s3-creds]
kubernetes_secret_v1.woodpecker_cnpg_s3_creds: Refreshing state... [id=woodpecker/cnpg-s3-creds]
kubernetes_cron_job_v1.tf_state_backup: Refreshing state... [id=tofu-state/tf-state-backup]
kubernetes_cron_job_v1.cnpg_backup_verify: Refreshing state... [id=postgres/cnpg-backup-verify]
kubernetes_manifest.woodpecker_postgres: Refreshing state...
helm_release.woodpecker: Refreshing state... [id=woodpecker]
kubernetes_ingress_v1.woodpecker_funnel: Refreshing state... [id=woodpecker/woodpecker-funnel]

No changes. Your infrastructure matches the configuration.

OpenTofu has compared your real infrastructure against your configuration and
found no differences, so no changes are needed.
## Tofu Plan Output ``` tailscale_acl.this: Refreshing state... [id=acl] helm_release.nvidia_device_plugin: Refreshing state... [id=nvidia-device-plugin] kubernetes_namespace_v1.postgres: Refreshing state... [id=postgres] kubernetes_namespace_v1.monitoring: Refreshing state... [id=monitoring] data.kubernetes_namespace_v1.tofu_state: Reading... kubernetes_namespace_v1.forgejo: Refreshing state... [id=forgejo] kubernetes_namespace_v1.tailscale: Refreshing state... [id=tailscale] data.kubernetes_namespace_v1.pal_e_docs: Reading... kubernetes_namespace_v1.ollama: Refreshing state... [id=ollama] kubernetes_namespace_v1.minio: Refreshing state... [id=minio] data.kubernetes_namespace_v1.tofu_state: Read complete after 0s [id=tofu-state] data.kubernetes_namespace_v1.pal_e_docs: Read complete after 0s [id=pal-e-docs] kubernetes_namespace_v1.cnpg_system: Refreshing state... [id=cnpg-system] kubernetes_namespace_v1.woodpecker: Refreshing state... [id=woodpecker] kubernetes_namespace_v1.keycloak: Refreshing state... [id=keycloak] kubernetes_namespace_v1.harbor: Refreshing state... [id=harbor] kubernetes_service_account_v1.tf_backup: Refreshing state... [id=tofu-state/tf-state-backup] kubernetes_secret_v1.paledocs_db_url: Refreshing state... [id=pal-e-docs/paledocs-db-url] kubernetes_role_v1.tf_backup: Refreshing state... [id=tofu-state/tf-state-backup] helm_release.tailscale_operator: Refreshing state... [id=tailscale-operator] kubernetes_secret_v1.dora_exporter: Refreshing state... [id=monitoring/dora-exporter] helm_release.kube_prometheus_stack: Refreshing state... [id=kube-prometheus-stack] kubernetes_config_map_v1.uptime_dashboard: Refreshing state... [id=monitoring/uptime-dashboard] kubernetes_service_v1.dora_exporter: Refreshing state... [id=monitoring/dora-exporter] helm_release.forgejo: Refreshing state... [id=forgejo] helm_release.loki_stack: Refreshing state... [id=loki-stack] kubernetes_secret_v1.woodpecker_db_credentials: Refreshing state... [id=woodpecker/woodpecker-db-credentials] kubernetes_manifest.netpol_ollama: Refreshing state... kubernetes_manifest.netpol_postgres: Refreshing state... kubernetes_manifest.netpol_forgejo: Refreshing state... kubernetes_manifest.netpol_minio: Refreshing state... kubernetes_manifest.netpol_monitoring: Refreshing state... kubernetes_manifest.netpol_cnpg_system: Refreshing state... helm_release.cnpg: Refreshing state... [id=cnpg] kubernetes_persistent_volume_claim_v1.keycloak_data: Refreshing state... [id=keycloak/keycloak-data] kubernetes_service_v1.keycloak: Refreshing state... [id=keycloak/keycloak] kubernetes_secret_v1.keycloak_admin: Refreshing state... [id=keycloak/keycloak-admin] kubernetes_manifest.netpol_woodpecker: Refreshing state... kubernetes_role_binding_v1.tf_backup: Refreshing state... [id=tofu-state/tf-state-backup] kubernetes_manifest.netpol_keycloak: Refreshing state... helm_release.ollama: Refreshing state... [id=ollama] kubernetes_manifest.netpol_harbor: Refreshing state... kubernetes_deployment_v1.keycloak: Refreshing state... [id=keycloak/keycloak] kubernetes_ingress_v1.keycloak_funnel: Refreshing state... [id=keycloak/keycloak-funnel] kubernetes_ingress_v1.grafana_funnel: Refreshing state... [id=monitoring/grafana-funnel] kubernetes_ingress_v1.alertmanager_funnel: Refreshing state... [id=monitoring/alertmanager-funnel] kubernetes_config_map_v1.dora_dashboard: Refreshing state... [id=monitoring/dora-dashboard] helm_release.blackbox_exporter: Refreshing state... [id=blackbox-exporter] kubernetes_manifest.blackbox_alerts: Refreshing state... kubernetes_config_map_v1.pal_e_docs_dashboard: Refreshing state... [id=monitoring/pal-e-docs-dashboard] helm_release.harbor: Refreshing state... [id=harbor] kubernetes_deployment_v1.dora_exporter: Refreshing state... [id=monitoring/dora-exporter] helm_release.minio: Refreshing state... [id=minio] kubernetes_manifest.dora_exporter_service_monitor: Refreshing state... kubernetes_ingress_v1.forgejo_funnel: Refreshing state... [id=forgejo/forgejo-funnel] kubernetes_config_map_v1.grafana_loki_datasource: Refreshing state... [id=monitoring/grafana-loki-datasource] kubernetes_ingress_v1.harbor_funnel: Refreshing state... [id=harbor/harbor-funnel] minio_iam_user.tf_backup: Refreshing state... [id=tf-backup] minio_s3_bucket.postgres_wal: Refreshing state... [id=postgres-wal] minio_s3_bucket.tf_state_backups: Refreshing state... [id=tf-state-backups] minio_s3_bucket.assets: Refreshing state... [id=assets] minio_iam_policy.cnpg_wal: Refreshing state... [id=cnpg-wal] minio_iam_policy.tf_backup: Refreshing state... [id=tf-backup] kubernetes_ingress_v1.minio_api_funnel: Refreshing state... [id=minio/minio-api-funnel] kubernetes_ingress_v1.minio_funnel: Refreshing state... [id=minio/minio-funnel] minio_iam_user.cnpg: Refreshing state... [id=cnpg] minio_iam_user_policy_attachment.cnpg: Refreshing state... [id=cnpg-20260302210642491000000001] minio_iam_user_policy_attachment.tf_backup: Refreshing state... [id=tf-backup-20260314163610110100000001] kubernetes_secret_v1.tf_backup_s3_creds: Refreshing state... [id=tofu-state/tf-backup-s3-creds] kubernetes_secret_v1.cnpg_s3_creds: Refreshing state... [id=postgres/cnpg-s3-creds] kubernetes_secret_v1.woodpecker_cnpg_s3_creds: Refreshing state... [id=woodpecker/cnpg-s3-creds] kubernetes_cron_job_v1.tf_state_backup: Refreshing state... [id=tofu-state/tf-state-backup] kubernetes_cron_job_v1.cnpg_backup_verify: Refreshing state... [id=postgres/cnpg-backup-verify] kubernetes_manifest.woodpecker_postgres: Refreshing state... helm_release.woodpecker: Refreshing state... [id=woodpecker] kubernetes_ingress_v1.woodpecker_funnel: Refreshing state... [id=woodpecker/woodpecker-funnel] No changes. Your infrastructure matches the configuration. OpenTofu has compared your real infrastructure against your configuration and found no differences, so no changes are needed. ```
Author
Owner

PR #81 Review

DOMAIN REVIEW

Tech stack: Salt (SaltStack state files), systemd, nftables

This PR adds a systemd drop-in override via Salt to fix an nftables boot ordering race condition. The tailscale0 interface is referenced in nftables.conf.j2 (line 32-33, via pillar allowed_interfaces) but is created by tailscaled.service at startup. Without the drop-in, nft validates interface names at load time and fails because tailscale0 does not exist yet.

Salt state analysis:

  1. nftables-after-tailscale (file.managed) -- Correctly deploys a systemd drop-in at /etc/systemd/system/nftables.service.d/after-tailscale.conf with After=tailscaled.service and Wants=tailscaled.service. The makedirs: true is correct since the .d directory likely does not exist yet. Permissions (root:root, 0644) are appropriate for systemd unit drop-ins.

  2. nftables-daemon-reload (cmd.wait) -- Uses cmd.wait with a watch on the drop-in file. This is the correct Salt pattern: cmd.wait only fires when the watched state reports changes, so systemctl daemon-reload runs only when the drop-in is created or modified. Idempotent on subsequent runs.

  3. Requisite wiring -- Both nftables-service and nftables-reload now require nftables-daemon-reload. This ensures the daemon-reload has completed (or been skipped as no-op) before the service is enabled or rules are reloaded. The ordering chain is: nftables-package -> nftables-after-tailscale -> nftables-daemon-reload -> nftables-service -> nftables-reload. This is correct.

systemd analysis:

  • After=tailscaled.service -- Correct. Ensures nftables starts after tailscaled, so tailscale0 exists.
  • Wants=tailscaled.service -- Appropriate soft dependency. If tailscaled is not installed (hypothetical), nftables still starts. Requires= would be too strong here since the host could conceivably run without Tailscale in a degraded scenario.
  • The drop-in path /etc/systemd/system/nftables.service.d/after-tailscale.conf follows systemd conventions correctly.

Root cause verification:

Confirmed. salt/pillar/firewall.sls line 17 lists tailscale0 in allowed_interfaces, and the Jinja template at salt/states/firewall/nftables.conf.j2 line 32-33 renders iif "tailscale0" accept. The nft binary validates interface names at load time, so the interface must exist before rules are loaded.

BLOCKERS

None.

  • Test coverage: This is an IaC repo with no automated test framework. Validation is done via salt-call state.apply firewall as documented in the PR body. The Test Plan includes both pre-reboot (Salt apply) and post-reboot (systemctl status + nft list ruleset) verification steps. Two post-reboot items remain unchecked, which is expected -- they require the operator to reboot and verify. This is appropriate for host-level systemd changes.
  • No secrets committed: Confirmed. The diff contains no credentials, keys, or sensitive data.
  • No unvalidated input: Salt pillar data is operator-controlled, not user-supplied.
  • No DRY violations in security paths: The daemon-reload is referenced from two places via require, but this is requisite wiring, not duplicated logic.

NITS

  1. Comment header style -- The existing states use # --- section dividers consistently. The new states follow this convention. No issue.

  2. Drop-in file trailing newline -- The contents: | block produces a trailing newline after Wants=tailscaled.service, which is correct for systemd unit files. No issue.

  3. Consider Requires= vs Wants= -- Wants= is the softer dependency, meaning nftables will still attempt to start even if tailscaled fails. Given that nftables rules reference tailscale0, if tailscaled fails, nftables will also fail (same error as before the fix). A Requires= would make the failure explicit rather than letting nft fail with a confusing interface error. However, Wants= is the conventional choice for drop-ins and avoids creating a hard dependency that could cascade failures. Either is defensible. Non-blocking.

SOP COMPLIANCE

  • Branch named after issue (80-feat-fix-nftables-boot-ordering-start-af references #80)
  • PR body has: Summary, Changes, Test Plan, Related
  • Related references plan slug (plan-pal-e-platform -- Phase 8c)
  • Closes #80 present in PR body
  • No secrets committed
  • No unnecessary file changes (1 file, 25 additions, 0 deletions -- all scoped to the fix)
  • Commit messages are descriptive (PR title: feat: fix nftables boot ordering -- start after tailscaled)
  • Review checklist completed in PR body

PROCESS OBSERVATIONS

  • Change failure risk: LOW. This is a boot-ordering fix with no impact on running state. The drop-in only affects the next boot. The existing nftables rules are unchanged. Salt apply was validated with 0 failures.
  • MTTR impact: POSITIVE. This resolves a 10-day-old host firewall gap where INPUT ACCEPT was the effective policy. Fixing this restores the intended policy drop firewall posture.
  • Deployment frequency: NEUTRAL. Single-file Salt state change. No Terraform or k8s resources affected.
  • Incomplete validation acknowledged. The PR honestly marks post-reboot test items as unchecked. The operator should reboot and verify systemctl status nftables and nft list ruleset before considering Phase 8c fully complete.

VERDICT: APPROVED

## PR #81 Review ### DOMAIN REVIEW **Tech stack: Salt (SaltStack state files), systemd, nftables** This PR adds a systemd drop-in override via Salt to fix an nftables boot ordering race condition. The `tailscale0` interface is referenced in `nftables.conf.j2` (line 32-33, via pillar `allowed_interfaces`) but is created by `tailscaled.service` at startup. Without the drop-in, `nft` validates interface names at load time and fails because `tailscale0` does not exist yet. **Salt state analysis:** 1. **`nftables-after-tailscale` (file.managed)** -- Correctly deploys a systemd drop-in at `/etc/systemd/system/nftables.service.d/after-tailscale.conf` with `After=tailscaled.service` and `Wants=tailscaled.service`. The `makedirs: true` is correct since the `.d` directory likely does not exist yet. Permissions (root:root, 0644) are appropriate for systemd unit drop-ins. 2. **`nftables-daemon-reload` (cmd.wait)** -- Uses `cmd.wait` with a `watch` on the drop-in file. This is the correct Salt pattern: `cmd.wait` only fires when the watched state reports changes, so `systemctl daemon-reload` runs only when the drop-in is created or modified. Idempotent on subsequent runs. 3. **Requisite wiring** -- Both `nftables-service` and `nftables-reload` now `require` `nftables-daemon-reload`. This ensures the daemon-reload has completed (or been skipped as no-op) before the service is enabled or rules are reloaded. The ordering chain is: `nftables-package` -> `nftables-after-tailscale` -> `nftables-daemon-reload` -> `nftables-service` -> `nftables-reload`. This is correct. **systemd analysis:** - `After=tailscaled.service` -- Correct. Ensures nftables starts after tailscaled, so `tailscale0` exists. - `Wants=tailscaled.service` -- Appropriate soft dependency. If tailscaled is not installed (hypothetical), nftables still starts. `Requires=` would be too strong here since the host could conceivably run without Tailscale in a degraded scenario. - The drop-in path `/etc/systemd/system/nftables.service.d/after-tailscale.conf` follows systemd conventions correctly. **Root cause verification:** Confirmed. `salt/pillar/firewall.sls` line 17 lists `tailscale0` in `allowed_interfaces`, and the Jinja template at `salt/states/firewall/nftables.conf.j2` line 32-33 renders `iif "tailscale0" accept`. The nft binary validates interface names at load time, so the interface must exist before rules are loaded. ### BLOCKERS None. - **Test coverage**: This is an IaC repo with no automated test framework. Validation is done via `salt-call state.apply firewall` as documented in the PR body. The Test Plan includes both pre-reboot (Salt apply) and post-reboot (systemctl status + nft list ruleset) verification steps. Two post-reboot items remain unchecked, which is expected -- they require the operator to reboot and verify. This is appropriate for host-level systemd changes. - **No secrets committed**: Confirmed. The diff contains no credentials, keys, or sensitive data. - **No unvalidated input**: Salt pillar data is operator-controlled, not user-supplied. - **No DRY violations in security paths**: The daemon-reload is referenced from two places via `require`, but this is requisite wiring, not duplicated logic. ### NITS 1. **Comment header style** -- The existing states use `# ---` section dividers consistently. The new states follow this convention. No issue. 2. **Drop-in file trailing newline** -- The `contents: |` block produces a trailing newline after `Wants=tailscaled.service`, which is correct for systemd unit files. No issue. 3. **Consider `Requires=` vs `Wants=`** -- `Wants=` is the softer dependency, meaning nftables will still attempt to start even if tailscaled fails. Given that nftables rules reference `tailscale0`, if tailscaled fails, nftables will also fail (same error as before the fix). A `Requires=` would make the failure explicit rather than letting nft fail with a confusing interface error. However, `Wants=` is the conventional choice for drop-ins and avoids creating a hard dependency that could cascade failures. Either is defensible. Non-blocking. ### SOP COMPLIANCE - [x] Branch named after issue (`80-feat-fix-nftables-boot-ordering-start-af` references #80) - [x] PR body has: Summary, Changes, Test Plan, Related - [x] Related references plan slug (`plan-pal-e-platform` -- Phase 8c) - [x] `Closes #80` present in PR body - [x] No secrets committed - [x] No unnecessary file changes (1 file, 25 additions, 0 deletions -- all scoped to the fix) - [x] Commit messages are descriptive (PR title: `feat: fix nftables boot ordering -- start after tailscaled`) - [x] Review checklist completed in PR body ### PROCESS OBSERVATIONS - **Change failure risk: LOW.** This is a boot-ordering fix with no impact on running state. The drop-in only affects the next boot. The existing nftables rules are unchanged. Salt apply was validated with 0 failures. - **MTTR impact: POSITIVE.** This resolves a 10-day-old host firewall gap where `INPUT ACCEPT` was the effective policy. Fixing this restores the intended `policy drop` firewall posture. - **Deployment frequency: NEUTRAL.** Single-file Salt state change. No Terraform or k8s resources affected. - **Incomplete validation acknowledged.** The PR honestly marks post-reboot test items as unchecked. The operator should reboot and verify `systemctl status nftables` and `nft list ruleset` before considering Phase 8c fully complete. ### VERDICT: APPROVED
forgejo_admin deleted branch 80-feat-fix-nftables-boot-ordering-start-af 2026-03-15 05:34:06 +00:00
Sign in to join this conversation.
No description provided.