Skip to content

Commit 8709ec0

Browse files
QxBytesQxBytes
committed
add logic to delete jump to swift postrouting in legacy and fix uts
1 parent bcd6fb7 commit 8709ec0

File tree

5 files changed

+65
-10
lines changed

5 files changed

+65
-10
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(table, chain string, rulespec ...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: 18 additions & 0 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,6 +23,16 @@ 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 {
27+
return &iptablesLegacy{}
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 exec.Command("iptables-legacy", cmd...).Run()
35+
}
2536

2637
// nolint
2738
func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainerRequest) (types.ResponseCode, string) {
@@ -32,6 +43,13 @@ func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainer
3243
// in podsubnet case, ncPrimaryIP is the pod subnet's primary ip
3344
// in vnet scale case, ncPrimaryIP is the node's ip
3445
ncPrimaryIP, _, _ := net.ParseCIDR(req.IPConfiguration.IPSubnet.IPAddress + "/" + fmt.Sprintf("%d", req.IPConfiguration.IPSubnet.PrefixLength))
46+
iptl := service.iptables.GetIPTablesLegacy()
47+
err := iptl.Delete(iptables.Nat, iptables.Postrouting, "-j", SWIFTPOSTROUTING)
48+
// ignore if command fails
49+
if err == nil {
50+
logger.Printf("[Azure CNS] Deleted legacy jump to SWIFT-POSTROUTING Chain")
51+
}
52+
3553
ipt, err := service.iptables.GetIPTables()
3654
if err != nil {
3755
return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to create iptables interface : %v", err)

‎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 {
31+
if c.iptablesLegacy == nil {
32+
c.iptablesLegacy = &fakes.IPTablesLegacyMock{}
33+
}
34+
return c.iptablesLegacy
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
7074
}
7175

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

0 commit comments

Comments
 (0)