Skip to content

Commit fa90bd2

Browse files
authored
feat: snat azure dns traffic to node ip and remove jump to swift postrouting in iptables legacy (#3930)
* snat azure dns traffic in linux podsubnet azure and cilium scenarios 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 * add logic to delete jump to swift postrouting in legacy and fix uts * address linter * align getting iptables legacy with iptables * update log
1 parent bcd6fb7 commit fa90bd2

File tree

5 files changed

+72
-17
lines changed

5 files changed

+72
-17
lines changed

cns/fakes/iptablesfake.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,19 @@ var (
1616
errIndexBounds = errors.New("index out of bounds")
1717
)
1818

19+
type IPTablesLegacyMock struct {
20+
deleteCallCount int
21+
}
22+
23+
func (c *IPTablesLegacyMock) Delete(_, _ string, _ ...string) error {
24+
c.deleteCallCount++
25+
return nil
26+
}
27+
28+
func (c *IPTablesLegacyMock) DeleteCallCount() int {
29+
return c.deleteCallCount
30+
}
31+
1932
type IPTablesMock struct {
2033
state map[string]map[string][]string
2134
clearChainCallCount int

cns/restserver/internalapi_linux.go

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package restserver
33
import (
44
"fmt"
55
"net"
6+
"os/exec"
67
"strconv"
78

89
"github.com/Azure/azure-container-networking/cns"
@@ -22,16 +23,32 @@ func (c *IPtablesProvider) GetIPTables() (iptablesClient, error) {
2223
client, err := goiptables.New()
2324
return client, errors.Wrap(err, "failed to get iptables client")
2425
}
26+
func (c *IPtablesProvider) GetIPTablesLegacy() (iptablesLegacyClient, error) {
27+
return &iptablesLegacy{}, nil
28+
}
29+
30+
type iptablesLegacy struct{}
31+
32+
func (c *iptablesLegacy) Delete(table, chain string, rulespec ...string) error {
33+
cmd := append([]string{"-t", table, "-D", chain}, rulespec...)
34+
return errors.Wrap(exec.Command("iptables-legacy", cmd...).Run(), "iptables legacy failed delete")
35+
}
2536

2637
// nolint
2738
func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainerRequest) (types.ResponseCode, string) {
2839
service.Lock()
2940
defer service.Unlock()
3041

31-
// Parse primary ip and ipnet from nnc
32-
// in podsubnet case, ncPrimaryIP is the pod subnet's primary ip
33-
// in vnet scale case, ncPrimaryIP is the node's ip
34-
ncPrimaryIP, _, _ := net.ParseCIDR(req.IPConfiguration.IPSubnet.IPAddress + "/" + fmt.Sprintf("%d", req.IPConfiguration.IPSubnet.PrefixLength))
42+
iptl, err := service.iptables.GetIPTablesLegacy()
43+
if err != nil {
44+
return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to create iptables legacy interface : %v", err)
45+
}
46+
err = iptl.Delete(iptables.Nat, iptables.Postrouting, "-j", SWIFTPOSTROUTING)
47+
// ignore if command fails
48+
if err == nil {
49+
logger.Printf("[Azure CNS] Deleted legacy jump to SWIFT-POSTROUTING Chain")
50+
}
51+
3552
ipt, err := service.iptables.GetIPTables()
3653
if err != nil {
3754
return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to create iptables interface : %v", err)
@@ -103,8 +120,8 @@ func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainer
103120

104121
// define all rules we want in the chain
105122
rules := [][]string{
106-
{"-m", "addrtype", "!", "--dst-type", "local", "-s", podSubnet.String(), "-d", networkutils.AzureDNS, "-p", iptables.UDP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", ncPrimaryIP.String()},
107-
{"-m", "addrtype", "!", "--dst-type", "local", "-s", podSubnet.String(), "-d", networkutils.AzureDNS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", ncPrimaryIP.String()},
123+
{"-m", "addrtype", "!", "--dst-type", "local", "-s", podSubnet.String(), "-d", networkutils.AzureDNS, "-p", iptables.UDP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", req.HostPrimaryIP},
124+
{"-m", "addrtype", "!", "--dst-type", "local", "-s", podSubnet.String(), "-d", networkutils.AzureDNS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", req.HostPrimaryIP},
108125
{"-m", "addrtype", "!", "--dst-type", "local", "-s", podSubnet.String(), "-d", networkutils.AzureIMDS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.HTTPPort), "-j", iptables.Snat, "--to", req.HostPrimaryIP},
109126
}
110127

@@ -130,7 +147,7 @@ func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainer
130147
// if rule count doesn't match or not all rules exist, reconcile
131148
// add one because there is always a singular starting rule in the chain, in addition to the ones we add
132149
if len(currentRules) != len(rules)+1 || !allRulesExist {
133-
logger.Printf("[Azure CNS] Reconciling SWIFT-POSTROUTING chain rules")
150+
logger.Printf("[Azure CNS] Reconciling SWIFT-POSTROUTING chain rules to SNAT Azure DNS and IMDS to Host IP")
134151

135152
err = ipt.ClearChain(iptables.Nat, SWIFTPOSTROUTING)
136153
if err != nil {
@@ -143,6 +160,7 @@ func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainer
143160
return types.FailedToRunIPTableCmd, "[Azure CNS] failed to append rule to SWIFT-POSTROUTING chain : " + err.Error()
144161
}
145162
}
163+
logger.Printf("[Azure CNS] Finished reconciling SWIFT-POSTROUTING chain")
146164
}
147165

148166
// we only need to run this code once as the iptable rule applies to all secondary ip configs in the same subnet

cns/restserver/internalapi_linux_test.go

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ import (
1515
)
1616

1717
type FakeIPTablesProvider struct {
18-
iptables *fakes.IPTablesMock
18+
iptables *fakes.IPTablesMock
19+
iptablesLegacy *fakes.IPTablesLegacyMock
1920
}
2021

2122
func (c *FakeIPTablesProvider) GetIPTables() (iptablesClient, error) {
@@ -26,6 +27,13 @@ func (c *FakeIPTablesProvider) GetIPTables() (iptablesClient, error) {
2627
return c.iptables, nil
2728
}
2829

30+
func (c *FakeIPTablesProvider) GetIPTablesLegacy() (iptablesLegacyClient, error) {
31+
if c.iptablesLegacy == nil {
32+
c.iptablesLegacy = &fakes.IPTablesLegacyMock{}
33+
}
34+
return c.iptablesLegacy, nil
35+
}
36+
2937
func TestAddSNATRules(t *testing.T) {
3038
type chainExpectation struct {
3139
table string
@@ -70,8 +78,8 @@ func TestAddSNATRules(t *testing.T) {
7078
chain: SWIFTPOSTROUTING,
7179
expected: []string{
7280
"-N SWIFT-POSTROUTING",
73-
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureDNS + " -p udp --dport " + strconv.Itoa(iptables.DNSPort) + " -j SNAT --to 240.1.2.1",
74-
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureDNS + " -p tcp --dport " + strconv.Itoa(iptables.DNSPort) + " -j SNAT --to 240.1.2.1",
81+
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureDNS + " -p udp --dport " + strconv.Itoa(iptables.DNSPort) + " -j SNAT --to 10.0.0.4",
82+
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureDNS + " -p tcp --dport " + strconv.Itoa(iptables.DNSPort) + " -j SNAT --to 10.0.0.4",
7583
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureIMDS + " -p tcp --dport " + strconv.Itoa(iptables.HTTPPort) + " -j SNAT --to 10.0.0.4",
7684
},
7785
},
@@ -140,8 +148,8 @@ func TestAddSNATRules(t *testing.T) {
140148
chain: SWIFTPOSTROUTING,
141149
expected: []string{
142150
"-N SWIFT-POSTROUTING",
143-
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureDNS + " -p udp --dport " + strconv.Itoa(iptables.DNSPort) + " -j SNAT --to 240.1.2.1",
144-
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureDNS + " -p tcp --dport " + strconv.Itoa(iptables.DNSPort) + " -j SNAT --to 240.1.2.1",
151+
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureDNS + " -p udp --dport " + strconv.Itoa(iptables.DNSPort) + " -j SNAT --to 10.0.0.4",
152+
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureDNS + " -p tcp --dport " + strconv.Itoa(iptables.DNSPort) + " -j SNAT --to 10.0.0.4",
145153
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureIMDS + " -p tcp --dport " + strconv.Itoa(iptables.HTTPPort) + " -j SNAT --to 10.0.0.4",
146154
},
147155
},
@@ -201,15 +209,15 @@ func TestAddSNATRules(t *testing.T) {
201209
chain: SWIFTPOSTROUTING,
202210
rule: []string{
203211
"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/24", "-d", networkutils.AzureDNS,
204-
"-p", "udp", "--dport", strconv.Itoa(iptables.DNSPort), "-j", "SNAT", "--to", "240.1.2.1",
212+
"-p", "udp", "--dport", strconv.Itoa(iptables.DNSPort), "-j", "SNAT", "--to", "10.0.0.4",
205213
},
206214
},
207215
{
208216
table: iptables.Nat,
209217
chain: SWIFTPOSTROUTING,
210218
rule: []string{
211219
"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/24", "-d", networkutils.AzureDNS,
212-
"-p", "tcp", "--dport", strconv.Itoa(iptables.DNSPort), "-j", "SNAT", "--to", "240.1.2.1",
220+
"-p", "tcp", "--dport", strconv.Itoa(iptables.DNSPort), "-j", "SNAT", "--to", "10.0.0.4",
213221
},
214222
},
215223
{
@@ -235,8 +243,8 @@ func TestAddSNATRules(t *testing.T) {
235243
chain: SWIFTPOSTROUTING,
236244
expected: []string{
237245
"-N SWIFT-POSTROUTING",
238-
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureDNS + " -p udp --dport " + strconv.Itoa(iptables.DNSPort) + " -j SNAT --to 240.1.2.1",
239-
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureDNS + " -p tcp --dport " + strconv.Itoa(iptables.DNSPort) + " -j SNAT --to 240.1.2.1",
246+
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureDNS + " -p udp --dport " + strconv.Itoa(iptables.DNSPort) + " -j SNAT --to 10.0.0.4",
247+
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureDNS + " -p tcp --dport " + strconv.Itoa(iptables.DNSPort) + " -j SNAT --to 10.0.0.4",
240248
"-A SWIFT-POSTROUTING -m addrtype ! --dst-type local -s 240.1.2.0/24 -d " + networkutils.AzureIMDS + " -p tcp --dport " + strconv.Itoa(iptables.HTTPPort) + " -j SNAT --to 10.0.0.4",
241249
},
242250
},
@@ -307,8 +315,10 @@ func TestAddSNATRules(t *testing.T) {
307315
t.Run(tt.name, func(t *testing.T) {
308316
service := getTestService(cns.KubernetesCRD)
309317
ipt := fakes.NewIPTablesMock()
318+
iptl := &fakes.IPTablesLegacyMock{}
310319
service.iptables = &FakeIPTablesProvider{
311-
iptables: ipt,
320+
iptables: ipt,
321+
iptablesLegacy: iptl,
312322
}
313323

314324
// setup pre-existing rules
@@ -360,6 +370,12 @@ func TestAddSNATRules(t *testing.T) {
360370
if actualClearChainCalls != tt.expectedClearChainCalls {
361371
t.Fatalf("ClearChain call count mismatch: got %d, expected %d", actualClearChainCalls, tt.expectedClearChainCalls)
362372
}
373+
374+
// verify we delete legacy swift postrouting jump
375+
actualLegacyDeleteCalls := iptl.DeleteCallCount()
376+
if actualLegacyDeleteCalls != 1 {
377+
t.Fatalf("Delete call count mismatch: got %d, expected 1", actualLegacyDeleteCalls)
378+
}
363379
})
364380
}
365381
}

cns/restserver/internalapi_windows.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ func (*IPtablesProvider) GetIPTables() (iptablesClient, error) {
2424
return nil, errUnsupportedAPI
2525
}
2626

27+
func (*IPtablesProvider) GetIPTablesLegacy() (iptablesLegacyClient, error) {
28+
return nil, errUnsupportedAPI
29+
}
30+
2731
// nolint
2832
func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainerRequest) (types.ResponseCode, string) {
2933
return types.Success, ""

cns/restserver/restserver.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,13 @@ type iptablesClient interface {
6464
ClearChain(table string, chain string) error
6565
Delete(table, chain string, rulespec ...string) error
6666
}
67+
type iptablesLegacyClient interface {
68+
Delete(table, chain string, rulespec ...string) error
69+
}
6770

6871
type iptablesGetter interface {
6972
GetIPTables() (iptablesClient, error)
73+
GetIPTablesLegacy() (iptablesLegacyClient, error)
7074
}
7175

7276
// HTTPRestService represents http listener for CNS - Container Networking Service.

0 commit comments

Comments
 (0)