Skip to content

feat: add fib daddr type local return to nft prerouting and output chains#82

Merged
florentianayuwono merged 8 commits into
mainfrom
copilot/improve-exclude-localhost-traffic
Apr 28, 2026
Merged

feat: add fib daddr type local return to nft prerouting and output chains#82
florentianayuwono merged 8 commits into
mainfrom
copilot/improve-exclude-localhost-traffic

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 27, 2026

What this PR does

Adds fib daddr type local return as the first rule in both the nftables prerouting and output chains, dynamically bypassing DNAT for any traffic destined to a local interface address.

chain prerouting {
    type nat hook prerouting priority dstnat; policy accept;
    fib daddr type local return          # skip DNAT for local-destined traffic
    ip daddr @excluded_nets return
    tcp dport { 80, 443 } counter dnat to 10.0.0.1:8443
}

chain output {
    type nat hook output priority -150; policy accept;
    fib daddr type local return          # skip DNAT for local-destined traffic
    ip daddr @excluded_nets return
    tcp dport { 80, 443 } counter dnat to 10.0.0.1:8443
}

Also extends the integration test matrix to include Ubuntu 20.04, confirming compatibility (fib daddr type local is supported since nftables 0.7.0 / Linux kernel 4.10, which predates Ubuntu 20.04's nftables 0.9.3 and kernel 5.4).

Why we need it

The FIB check is kernel-driven and interface-aware — it covers all local IPs automatically without the operator needing to enumerate them. Applying it to both chains eliminates a footgun when deploying aproxy on hosts that also serve inbound traffic, and ensures locally-originated traffic to local addresses is also handled correctly.

Checklist

  • I followed the contributing guide
  • I added or updated the documentation (if applicable)
  • I updated docs/changelog.md with user-relevant changes
  • I used AI to assist with preparing this PR
  • I added or updated tests as needed (unit and integration)
  • If integration test modules are used: I updated the workflow configuration
    (e.g., in .github/workflows/integration_tests.yaml, ensure the modules list is correct)
  • If this is a Grafana dashboard: I added a screenshot of the dashboard
  • If this is Terraform: terraform fmt passes and tflint reports no errors
  • If this is Rockcraft: I updated the version

Test plan:

  • Integration test (test_local_service_accessible_when_port_intercepted): Behavioral end-to-end test that configures aproxy to intercept port 8081, starts an HTTP server on that port inside the ubuntu unit, then curls the unit's own non-loopback IP and asserts a 200 response. Without fib daddr type local return, inbound traffic on the intercepted port would be DNAT'd to the proxy and the local service would be unreachable.

  • Ubuntu 20.04 has been added to the integration test matrix for end-to-end verification across all supported bases. Because any-charm (used to deploy the upstream tinyproxy) does not publish a ubuntu@20.04 build on latest/edge, the tinyproxy deployment falls back to ubuntu@22.04 for that base — the proxy server's host OS is irrelevant to what is being tested; aproxy and the ubuntu principal still run on 20.04.

Review focus: The rule is inserted as the first rule in both the prerouting chain (inbound interception) and the output chain (locally-originated traffic), ensuring local-destined traffic bypasses DNAT in both directions.

Agent-Logs-Url: https://github.com/canonical/aproxy-operator/sessions/e2adeac0-1971-4dd0-a7f4-fcf6045f2a2f

Co-authored-by: florentianayuwono <76247368+florentianayuwono@users.noreply.github.com>
Copilot AI changed the title [WIP] Enhance automatic exclusion of local host traffic from prerouting DNAT Add fib daddr type local return to nft prerouting chain Apr 27, 2026
Copilot AI requested a review from florentianayuwono April 27, 2026 12:37
Copy link
Copy Markdown
Collaborator

@florentianayuwono florentianayuwono left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address the review and make sure to fix the failing linting workflow. can you please also test if this will work with Ubuntu 20.04?

Comment thread tests/unit/test_aproxy_charm_unit.py Outdated
…ration matrix

Agent-Logs-Url: https://github.com/canonical/aproxy-operator/sessions/1b816434-1389-4807-ba7b-81d50c29451d

Co-authored-by: florentianayuwono <76247368+florentianayuwono@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 27, 2026

@copilot please address the review and make sure to fix the failing linting workflow. can you please also test if this will work with Ubuntu 20.04?

Both addressed in cb4a288:

  • Lint fixed: The ruff import-organization error was caused by the added import unittest.mock — the test has been rewritten to not need it.
  • Ubuntu 20.04: Added "20.04" to the integration test matrix in .github/workflows/integration_test.yaml so CI will verify it. For reference, fib daddr type local has been supported since nftables 0.7.0 / Linux kernel 4.10 — Ubuntu 20.04 ships nftables 0.9.3 and kernel 5.4, so the rule is compatible.

…l traffic exclusion

Agent-Logs-Url: https://github.com/canonical/aproxy-operator/sessions/cb7beb88-ccba-4b3f-9f44-7f24a66a3668

Co-authored-by: florentianayuwono <76247368+florentianayuwono@users.noreply.github.com>
…rted base for any-charm

Agent-Logs-Url: https://github.com/canonical/aproxy-operator/sessions/f4afe957-b5ef-4f28-9b77-c6033a7189a1

Co-authored-by: florentianayuwono <76247368+florentianayuwono@users.noreply.github.com>
@florentianayuwono
Copy link
Copy Markdown
Collaborator

@copilot can you fix the lint

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 28, 2026

@copilot can you fix the lint

Fixed in 42a6e4f. The ruff N806 error (Variable in function should be lowercase) was caused by _ANYCHARM_SUPPORTED_BASES using a constant-style name inside a function — renamed to anycharm_supported_bases.

@florentianayuwono florentianayuwono changed the title Add fib daddr type local return to nft prerouting chain feat: add fib daddr type local return to nft prerouting chain Apr 28, 2026
@florentianayuwono florentianayuwono marked this pull request as ready for review April 28, 2026 05:44
@florentianayuwono florentianayuwono requested a review from a team as a code owner April 28, 2026 05:44
@weiiwang01
Copy link
Copy Markdown
Contributor

@copilot can you add fib daddr type local return to the output chain as well?

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 28, 2026

@copilot can you add fib daddr type local return to the output chain as well?

Done in 3f99ed9. fib daddr type local return is now the first rule in both the prerouting and output chains.

Copilot AI changed the title feat: add fib daddr type local return to nft prerouting chain feat: add fib daddr type local return to nft prerouting and output chains Apr 28, 2026
@florentianayuwono
Copy link
Copy Markdown
Collaborator

@copilot fix the failing lint test please

Agent-Logs-Url: https://github.com/canonical/aproxy-operator/sessions/27920951-7c9e-4358-800f-781ded56aef8

Co-authored-by: florentianayuwono <76247368+florentianayuwono@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 28, 2026

@copilot fix the failing lint test please

Fixed in 74ac08a. Two ruff format failures:

  1. tests/unit/test_aproxy_charm_unit.py — trailing blank line at end of file removed.
  2. tests/unit/conftest.py — unnecessary extra parentheses around generator expressions in two lambdas removed.

Copilot AI requested a review from florentianayuwono April 28, 2026 06:22
@florentianayuwono florentianayuwono enabled auto-merge (squash) April 28, 2026 06:23
@florentianayuwono florentianayuwono merged commit 8a4d014 into main Apr 28, 2026
46 of 48 checks passed
@florentianayuwono florentianayuwono deleted the copilot/improve-exclude-localhost-traffic branch April 28, 2026 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improvement: Automatically exclude local host traffic from prerouting DNAT

3 participants