Skip to content

feat: snat azure dns traffic to node ip and remove jump to swift postrouting in iptables legacy #3930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

QxBytes
Copy link
Contributor

@QxBytes QxBytes commented Aug 13, 2025

Reason for Change:

Changes the ip CNS-added IPTables rules SNAT to from the primary ip to node ip for linux podsubnet scenarios (both azure and cilium cases). CNI-added iptables rules are not modified and windows behavior remains the same (will be modified in a future PR).

Removes the jump to SWIFT-POSTROUTING in iptables-legacy if it is present as CNS is now built with iptables nftables (so we avoid having iptables-legacy and iptables-nftables conflicting with each other). If both rules exist the order of evaluation is nondeterministic.

This change will likely roll out with the change that enables CNS to programSNAT rules in azure cases (previously CNS would only program iptables rules in cilium cases)

Issue Fixed:

Requirements:

Notes:
Tested upgrade and downgrade gets back to original state (minus the jump to SWIFT-POSTROUTING in iptables-legacy):
cilium podsubnet case (this case potentially has the jump to SWIFT-POSTROUTING in iptables-legacy as older cns versions programmed iptables-legacy rules)
azure podsubnet case (this scenario should only be using iptables-nftables)
Old SWIFT-POSTROUTING rule is successfully deleted in iptables legacy if it exists

@QxBytes QxBytes self-assigned this Aug 13, 2025
@QxBytes QxBytes added cns Related to CNS. cilium Related to Cilium. labels Aug 13, 2025
@QxBytes QxBytes requested a review from Copilot August 13, 2025 23:30
Copilot

This comment was marked as outdated.

@QxBytes QxBytes requested a review from Copilot August 13, 2025 23:52
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modifies SNAT behavior for Azure DNS traffic in Linux podsubnet scenarios by changing the source IP from the primary subnet IP to the node IP, and removes conflicting iptables-legacy rules to prevent conflicts with iptables-nftables.

  • Changes SNAT target from subnet primary IP to node IP for Azure DNS traffic
  • Removes jump to SWIFT-POSTROUTING in iptables-legacy to avoid rule conflicts
  • Adds support for iptables-legacy client interface to handle cleanup operations

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
cns/restserver/restserver.go Adds iptablesLegacyClient interface and getter method
cns/restserver/internalapi_windows.go Implements unsupported legacy iptables for Windows
cns/restserver/internalapi_linux_test.go Updates tests to verify node IP usage and legacy rule deletion
cns/restserver/internalapi_linux.go Implements legacy iptables deletion and changes SNAT target to node IP
cns/fakes/iptablesfake.go Adds mock implementation for legacy iptables testing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

…to node ip

todo: snat windows podsubnet azure scenario to node ip
vnetscale scenarios (cilium and azure) already snat to node ip
roll out after cns iptables reconciliation goes in
cni still writes snat to primary ip but it is superseded by cns' rules
@QxBytes QxBytes force-pushed the alew/snat-podsubnet-azure-dns-node-ip-081325 branch from 9578ca5 to 8524b50 Compare August 14, 2025 15:39
@QxBytes QxBytes marked this pull request as ready for review August 14, 2025 16:50
@QxBytes QxBytes requested a review from a team as a code owner August 14, 2025 16:50
@QxBytes QxBytes force-pushed the alew/snat-podsubnet-azure-dns-node-ip-081325 branch from 6adfe24 to b1a7451 Compare August 18, 2025 21:27
@QxBytes QxBytes force-pushed the alew/snat-podsubnet-azure-dns-node-ip-081325 branch from b1a7451 to b8e0df6 Compare August 18, 2025 21:40
@QxBytes
Copy link
Contributor Author

QxBytes commented Aug 18, 2025

/azp run Azure Container Networking PR

@Azure Azure deleted a comment from azure-pipelines bot Aug 18, 2025
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@QxBytes QxBytes enabled auto-merge August 18, 2025 23:04
@QxBytes QxBytes added this pull request to the merge queue Aug 18, 2025
@QxBytes QxBytes removed this pull request from the merge queue due to a manual request Aug 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cilium Related to Cilium. cns Related to CNS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants