Skip to content

Commit 7225b14

Browse files
QxBytesQxBytes
authored
forwardport: fix: add iptables rules for dns in vnet scale cilium case (#3421)
* cover adding iptables rules for dns in vnet scale cilium case * replace existing iptables rules * modify imds iptables rule * mock iptables in cns and add ut * address linter issues * address linter issue
1 parent 0d069df commit 7225b14

File tree

10 files changed

+327
-34
lines changed

10 files changed

+327
-34
lines changed

‎cns/client/client_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ func TestMain(m *testing.M) {
152152
config := common.ServiceConfig{}
153153

154154
httpRestService, err := restserver.NewHTTPRestService(&config, &fakes.WireserverClientFake{},
155-
&fakes.WireserverProxyFake{}, &fakes.NMAgentClientFake{}, nil, nil, nil,
155+
&fakes.WireserverProxyFake{}, &restserver.IPtablesProvider{}, &fakes.NMAgentClientFake{}, nil, nil, nil,
156156
fakes.NewMockIMDSClient())
157157
svc = httpRestService
158158
httpRestService.Name = "cns-test-server"

‎cns/fakes/iptablesfake.go‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
package fakes
2+
3+
import (
4+
"errors"
5+
"strings"
6+
7+
"github.com/Azure/azure-container-networking/iptables"
8+
)
9+
10+
var (
11+
errChainExists = errors.New("chain already exists")
12+
errChainNotFound = errors.New("chain not found")
13+
errRuleExists = errors.New("rule already exists")
14+
)
15+
16+
type IPTablesMock struct {
17+
state map[string]map[string][]string
18+
}
19+
20+
func NewIPTablesMock() *IPTablesMock {
21+
return &IPTablesMock{
22+
state: make(map[string]map[string][]string),
23+
}
24+
}
25+
26+
func (c *IPTablesMock) ensureTableExists(table string) {
27+
_, exists := c.state[table]
28+
if !exists {
29+
c.state[table] = make(map[string][]string)
30+
}
31+
}
32+
33+
func (c *IPTablesMock) ChainExists(table, chain string) (bool, error) {
34+
c.ensureTableExists(table)
35+
36+
builtins := []string{iptables.Input, iptables.Output, iptables.Prerouting, iptables.Postrouting, iptables.Forward}
37+
38+
_, exists := c.state[table][chain]
39+
40+
// these chains always exist
41+
for _, val := range builtins {
42+
if chain == val && !exists {
43+
c.state[table][chain] = []string{}
44+
return true, nil
45+
}
46+
}
47+
48+
return exists, nil
49+
}
50+
51+
func (c *IPTablesMock) NewChain(table, chain string) error {
52+
c.ensureTableExists(table)
53+
54+
exists, _ := c.ChainExists(table, chain)
55+
56+
if exists {
57+
return errChainExists
58+
}
59+
60+
c.state[table][chain] = []string{}
61+
return nil
62+
}
63+
64+
func (c *IPTablesMock) Exists(table, chain string, rulespec ...string) (bool, error) {
65+
c.ensureTableExists(table)
66+
67+
chainExists, _ := c.ChainExists(table, chain)
68+
if !chainExists {
69+
return false, nil
70+
}
71+
72+
targetRule := strings.Join(rulespec, " ")
73+
chainRules := c.state[table][chain]
74+
75+
for _, chainRule := range chainRules {
76+
if targetRule == chainRule {
77+
return true, nil
78+
}
79+
}
80+
return false, nil
81+
}
82+
83+
func (c *IPTablesMock) Append(table, chain string, rulespec ...string) error {
84+
c.ensureTableExists(table)
85+
86+
chainExists, _ := c.ChainExists(table, chain)
87+
if !chainExists {
88+
return errChainNotFound
89+
}
90+
91+
ruleExists, _ := c.Exists(table, chain, rulespec...)
92+
if ruleExists {
93+
return errRuleExists
94+
}
95+
96+
targetRule := strings.Join(rulespec, " ")
97+
c.state[table][chain] = append(c.state[table][chain], targetRule)
98+
return nil
99+
}
100+
101+
func (c *IPTablesMock) Insert(table, chain string, _ int, rulespec ...string) error {
102+
return c.Append(table, chain, rulespec...)
103+
}

‎cns/restserver/api_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1735,7 +1735,7 @@ func startService(serviceConfig common.ServiceConfig, _ configuration.CNSConfig)
17351735
config.Store = fileStore
17361736

17371737
nmagentClient := &fakes.NMAgentClientFake{}
1738-
service, err = NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.WireserverProxyFake{},
1738+
service, err = NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.WireserverProxyFake{}, &IPtablesProvider{},
17391739
nmagentClient, nil, nil, nil, fakes.NewMockIMDSClient())
17401740
if err != nil {
17411741
return err

‎cns/restserver/internalapi_linux.go‎

Lines changed: 48 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,28 @@ import (
1111
"github.com/Azure/azure-container-networking/iptables"
1212
"github.com/Azure/azure-container-networking/network/networkutils"
1313
goiptables "github.com/coreos/go-iptables/iptables"
14+
"github.com/pkg/errors"
1415
)
1516

1617
const SWIFT = "SWIFT-POSTROUTING"
1718

19+
type IPtablesProvider struct{}
20+
21+
func (c *IPtablesProvider) GetIPTables() (iptablesClient, error) {
22+
client, err := goiptables.New()
23+
return client, errors.Wrap(err, "failed to get iptables client")
24+
}
25+
1826
// nolint
1927
func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainerRequest) (types.ResponseCode, string) {
2028
service.Lock()
2129
defer service.Unlock()
2230

2331
// Parse primary ip and ipnet from nnc
24-
ncPrimaryIP, ncIPNet, _ := net.ParseCIDR(req.IPConfiguration.IPSubnet.IPAddress + "/" + fmt.Sprintf("%d", req.IPConfiguration.IPSubnet.PrefixLength))
25-
ipt, err := goiptables.New()
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))
35+
ipt, err := service.iptables.GetIPTables()
2636
if err != nil {
2737
return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to create iptables interface : %v", err)
2838
}
@@ -56,41 +66,51 @@ func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainer
5666
}
5767
}
5868

59-
snatUDPRuleexist, err := ipt.Exists(iptables.Nat, SWIFT, "-m", "addrtype", "!", "--dst-type", "local", "-s", ncIPNet.String(), "-d", networkutils.AzureDNS, "-p", iptables.UDP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", ncPrimaryIP.String())
60-
if err != nil {
61-
return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to check for existence of SNAT UDP rule : %v", err)
62-
}
63-
if !snatUDPRuleexist {
64-
logger.Printf("[Azure CNS] Inserting SNAT UDP rule ...")
65-
err = ipt.Insert(iptables.Nat, SWIFT, 1, "-m", "addrtype", "!", "--dst-type", "local", "-s", ncIPNet.String(), "-d", networkutils.AzureDNS, "-p", iptables.UDP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", ncPrimaryIP.String())
69+
// use any secondary ip + the nnc prefix length to get an iptables rule to allow dns and imds traffic from the pods
70+
for _, v := range req.SecondaryIPConfigs {
71+
// put the ip address in standard cidr form (where we zero out the parts that are not relevant)
72+
_, podSubnet, _ := net.ParseCIDR(v.IPAddress + "/" + fmt.Sprintf("%d", req.IPConfiguration.IPSubnet.PrefixLength))
73+
74+
snatUDPRuleExists, err := ipt.Exists(iptables.Nat, SWIFT, "-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())
6675
if err != nil {
67-
return types.FailedToRunIPTableCmd, "[Azure CNS] failed to inset SNAT UDP rule : " + err.Error()
76+
return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to check for existence of pod SNAT UDP rule : %v", err)
77+
}
78+
if !snatUDPRuleExists {
79+
logger.Printf("[Azure CNS] Inserting pod SNAT UDP rule ...")
80+
err = ipt.Insert(iptables.Nat, SWIFT, 1, "-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())
81+
if err != nil {
82+
return types.FailedToRunIPTableCmd, "[Azure CNS] failed to insert pod SNAT UDP rule : " + err.Error()
83+
}
6884
}
69-
}
7085

71-
snatTCPRuleexist, err := ipt.Exists(iptables.Nat, SWIFT, "-m", "addrtype", "!", "--dst-type", "local", "-s", ncIPNet.String(), "-d", networkutils.AzureDNS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", ncPrimaryIP.String())
72-
if err != nil {
73-
return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to check for existence of SNAT TCP rule : %v", err)
74-
}
75-
if !snatTCPRuleexist {
76-
logger.Printf("[Azure CNS] Inserting SNAT TCP rule ...")
77-
err = ipt.Insert(iptables.Nat, SWIFT, 1, "-m", "addrtype", "!", "--dst-type", "local", "-s", ncIPNet.String(), "-d", networkutils.AzureDNS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", ncPrimaryIP.String())
86+
snatPodTCPRuleExists, err := ipt.Exists(iptables.Nat, SWIFT, "-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())
7887
if err != nil {
79-
return types.FailedToRunIPTableCmd, "[Azure CNS] failed to insert SNAT TCP rule : " + err.Error()
88+
return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to check for existence of pod SNAT TCP rule : %v", err)
89+
}
90+
if !snatPodTCPRuleExists {
91+
logger.Printf("[Azure CNS] Inserting pod SNAT TCP rule ...")
92+
err = ipt.Insert(iptables.Nat, SWIFT, 1, "-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())
93+
if err != nil {
94+
return types.FailedToRunIPTableCmd, "[Azure CNS] failed to insert pod SNAT TCP rule : " + err.Error()
95+
}
8096
}
81-
}
8297

83-
snatIMDSRuleexist, err := ipt.Exists(iptables.Nat, SWIFT, "-m", "addrtype", "!", "--dst-type", "local", "-s", ncIPNet.String(), "-d", networkutils.AzureIMDS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.HTTPPort), "-j", iptables.Snat, "--to", req.HostPrimaryIP)
84-
if err != nil {
85-
return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to check for existence of SNAT IMDS rule : %v", err)
86-
}
87-
if !snatIMDSRuleexist {
88-
logger.Printf("[Azure CNS] Inserting SNAT IMDS rule ...")
89-
err = ipt.Insert(iptables.Nat, SWIFT, 1, "-m", "addrtype", "!", "--dst-type", "local", "-s", ncIPNet.String(), "-d", networkutils.AzureIMDS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.HTTPPort), "-j", iptables.Snat, "--to", req.HostPrimaryIP)
98+
snatIMDSRuleexist, err := ipt.Exists(iptables.Nat, SWIFT, "-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)
9099
if err != nil {
91-
return types.FailedToRunIPTableCmd, "[Azure CNS] failed to insert SNAT IMDS rule : " + err.Error()
100+
return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to check for existence of pod SNAT IMDS rule : %v", err)
101+
}
102+
if !snatIMDSRuleexist {
103+
logger.Printf("[Azure CNS] Inserting pod SNAT IMDS rule ...")
104+
err = ipt.Insert(iptables.Nat, SWIFT, 1, "-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)
105+
if err != nil {
106+
return types.FailedToRunIPTableCmd, "[Azure CNS] failed to insert pod SNAT IMDS rule : " + err.Error()
107+
}
92108
}
109+
110+
// we only need to run this code once as the iptable rule applies to all secondary ip configs in the same subnet
111+
break
93112
}
113+
94114
return types.Success, ""
95115
}
96116

‎cns/restserver/internalapi_linux_test.go‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
// Copyright 2020 Microsoft. All rights reserved.
2+
// MIT License
3+
4+
package restserver
5+
6+
import (
7+
"strconv"
8+
"testing"
9+
10+
"github.com/Azure/azure-container-networking/cns"
11+
"github.com/Azure/azure-container-networking/cns/fakes"
12+
"github.com/Azure/azure-container-networking/cns/types"
13+
"github.com/Azure/azure-container-networking/iptables"
14+
"github.com/Azure/azure-container-networking/network/networkutils"
15+
)
16+
17+
type FakeIPTablesProvider struct {
18+
iptables *fakes.IPTablesMock
19+
}
20+
21+
func (c *FakeIPTablesProvider) GetIPTables() (iptablesClient, error) {
22+
// persist iptables in testing
23+
if c.iptables == nil {
24+
c.iptables = fakes.NewIPTablesMock()
25+
}
26+
return c.iptables, nil
27+
}
28+
29+
func TestAddSNATRules(t *testing.T) {
30+
type expectedScenario struct {
31+
table string
32+
chain string
33+
rule []string
34+
}
35+
36+
tests := []struct {
37+
name string
38+
input *cns.CreateNetworkContainerRequest
39+
expected []expectedScenario
40+
}{
41+
{
42+
// in pod subnet, the primary nic ip is in the same address space as the pod subnet
43+
name: "podsubnet",
44+
input: &cns.CreateNetworkContainerRequest{
45+
NetworkContainerid: ncID,
46+
IPConfiguration: cns.IPConfiguration{
47+
IPSubnet: cns.IPSubnet{
48+
IPAddress: "240.1.2.1",
49+
PrefixLength: 24,
50+
},
51+
},
52+
SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{
53+
"abc": {
54+
IPAddress: "240.1.2.7",
55+
},
56+
},
57+
HostPrimaryIP: "10.0.0.4",
58+
},
59+
expected: []expectedScenario{
60+
{
61+
table: iptables.Nat,
62+
chain: SWIFT,
63+
rule: []string{
64+
"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/24", "-d",
65+
networkutils.AzureDNS, "-p", iptables.UDP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", "240.1.2.1",
66+
},
67+
},
68+
{
69+
table: iptables.Nat,
70+
chain: SWIFT,
71+
rule: []string{
72+
"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/24", "-d",
73+
networkutils.AzureDNS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", "240.1.2.1",
74+
},
75+
},
76+
{
77+
table: iptables.Nat,
78+
chain: SWIFT,
79+
rule: []string{
80+
"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/24", "-d",
81+
networkutils.AzureIMDS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.HTTPPort), "-j", iptables.Snat, "--to", "10.0.0.4",
82+
},
83+
},
84+
},
85+
},
86+
{
87+
// in vnet scale, the primary nic ip becomes the node ip (diff address space from pod subnet)
88+
name: "vnet scale",
89+
input: &cns.CreateNetworkContainerRequest{
90+
NetworkContainerid: ncID,
91+
IPConfiguration: cns.IPConfiguration{
92+
IPSubnet: cns.IPSubnet{
93+
IPAddress: "10.0.0.4",
94+
PrefixLength: 28,
95+
},
96+
},
97+
SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{
98+
"abc": {
99+
IPAddress: "240.1.2.15",
100+
},
101+
},
102+
HostPrimaryIP: "10.0.0.4",
103+
},
104+
expected: []expectedScenario{
105+
{
106+
table: iptables.Nat,
107+
chain: SWIFT,
108+
rule: []string{
109+
"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/28", "-d",
110+
networkutils.AzureDNS, "-p", iptables.UDP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", "10.0.0.4",
111+
},
112+
},
113+
{
114+
table: iptables.Nat,
115+
chain: SWIFT,
116+
rule: []string{
117+
"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/28", "-d",
118+
networkutils.AzureDNS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", "10.0.0.4",
119+
},
120+
},
121+
{
122+
table: iptables.Nat,
123+
chain: SWIFT,
124+
rule: []string{
125+
"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/28", "-d",
126+
networkutils.AzureIMDS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.HTTPPort), "-j", iptables.Snat, "--to", "10.0.0.4",
127+
},
128+
},
129+
},
130+
},
131+
}
132+
133+
for _, tt := range tests {
134+
service := getTestService(cns.KubernetesCRD)
135+
service.iptables = &FakeIPTablesProvider{}
136+
resp, msg := service.programSNATRules(tt.input)
137+
if resp != types.Success {
138+
t.Fatal("failed to program snat rules", msg, " case: ", tt.name)
139+
}
140+
finalState, _ := service.iptables.GetIPTables()
141+
for _, ex := range tt.expected {
142+
exists, err := finalState.Exists(ex.table, ex.chain, ex.rule...)
143+
if err != nil || !exists {
144+
t.Fatal("rule not found", ex.rule, " case: ", tt.name)
145+
}
146+
}
147+
}
148+
}

‎cns/restserver/internalapi_windows.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ const (
1616
pwshTimeout = 120 * time.Second
1717
)
1818

19+
var errUnsupportedAPI = errors.New("unsupported api")
20+
21+
type IPtablesProvider struct{}
22+
23+
func (*IPtablesProvider) GetIPTables() (iptablesClient, error) {
24+
return nil, errUnsupportedAPI
25+
}
26+
1927
// nolint
2028
func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainerRequest) (types.ResponseCode, string) {
2129
return types.Success, ""

0 commit comments

Comments
 (0)