forwardport: fix: add iptables rules for dns in vnet scale cilium case

cns/client/client_test.go

@@ -152,7 +152,7 @@ func TestMain(m *testing.M) {
 	config := common.ServiceConfig{}
 
 	httpRestService, err := restserver.NewHTTPRestService(&config, &fakes.WireserverClientFake{},
-		&fakes.WireserverProxyFake{}, &fakes.NMAgentClientFake{}, nil, nil, nil,
+		&fakes.WireserverProxyFake{}, &restserver.IPtablesProvider{}, &fakes.NMAgentClientFake{}, nil, nil, nil,
 		fakes.NewMockIMDSClient())
 	svc = httpRestService
 	httpRestService.Name = "cns-test-server"

cns/fakes/iptablesfake.go

@@ -0,0 +1,103 @@
+package fakes
+
+import (
+	"errors"
+	"strings"
+
+	"github.com/Azure/azure-container-networking/iptables"
+)
+
+var (
+	errChainExists   = errors.New("chain already exists")
+	errChainNotFound = errors.New("chain not found")
+	errRuleExists    = errors.New("rule already exists")
+)
+
+type IPTablesMock struct {
+	state map[string]map[string][]string
+}
+
+func NewIPTablesMock() *IPTablesMock {
+	return &IPTablesMock{
+		state: make(map[string]map[string][]string),
+	}
+}
+
+func (c *IPTablesMock) ensureTableExists(table string) {
+	_, exists := c.state[table]
+	if !exists {
+		c.state[table] = make(map[string][]string)
+	}
+}
+
+func (c *IPTablesMock) ChainExists(table, chain string) (bool, error) {
+	c.ensureTableExists(table)
+
+	builtins := []string{iptables.Input, iptables.Output, iptables.Prerouting, iptables.Postrouting, iptables.Forward}
+
+	_, exists := c.state[table][chain]
+
+	// these chains always exist
+	for _, val := range builtins {
+		if chain == val && !exists {
+			c.state[table][chain] = []string{}
+			return true, nil
+		}
+	}
+
+	return exists, nil
+}
+
+func (c *IPTablesMock) NewChain(table, chain string) error {
+	c.ensureTableExists(table)
+
+	exists, _ := c.ChainExists(table, chain)
+
+	if exists {
+		return errChainExists
+	}
+
+	c.state[table][chain] = []string{}
+	return nil
+}
+
+func (c *IPTablesMock) Exists(table, chain string, rulespec ...string) (bool, error) {
+	c.ensureTableExists(table)
+
+	chainExists, _ := c.ChainExists(table, chain)
+	if !chainExists {
+		return false, nil
+	}
+
+	targetRule := strings.Join(rulespec, " ")
+	chainRules := c.state[table][chain]
+
+	for _, chainRule := range chainRules {
+		if targetRule == chainRule {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+func (c *IPTablesMock) Append(table, chain string, rulespec ...string) error {
+	c.ensureTableExists(table)
+
+	chainExists, _ := c.ChainExists(table, chain)
+	if !chainExists {
+		return errChainNotFound
+	}
+
+	ruleExists, _ := c.Exists(table, chain, rulespec...)
+	if ruleExists {
+		return errRuleExists
+	}
+
+	targetRule := strings.Join(rulespec, " ")
+	c.state[table][chain] = append(c.state[table][chain], targetRule)
+	return nil
+}
+
+func (c *IPTablesMock) Insert(table, chain string, _ int, rulespec ...string) error {
+	return c.Append(table, chain, rulespec...)
+}

cns/restserver/api_test.go

@@ -1735,7 +1735,7 @@ func startService(serviceConfig common.ServiceConfig, _ configuration.CNSConfig)
 	config.Store = fileStore
 
 	nmagentClient := &fakes.NMAgentClientFake{}
-	service, err = NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.WireserverProxyFake{},
+	service, err = NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.WireserverProxyFake{}, &IPtablesProvider{},
 		nmagentClient, nil, nil, nil, fakes.NewMockIMDSClient())
 	if err != nil {
 		return err

cns/restserver/internalapi_linux.go

@@ -11,18 +11,28 @@ import (
 	"github.com/Azure/azure-container-networking/iptables"
 	"github.com/Azure/azure-container-networking/network/networkutils"
 	goiptables "github.com/coreos/go-iptables/iptables"
+	"github.com/pkg/errors"
 )
 
 const SWIFT = "SWIFT-POSTROUTING"
 
+type IPtablesProvider struct{}
+
+func (c *IPtablesProvider) GetIPTables() (iptablesClient, error) {
+	client, err := goiptables.New()
+	return client, errors.Wrap(err, "failed to get iptables client")
+}
+
 // nolint
 func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainerRequest) (types.ResponseCode, string) {
 	service.Lock()
 	defer service.Unlock()
 
 	// Parse primary ip and ipnet from nnc
-	ncPrimaryIP, ncIPNet, _ := net.ParseCIDR(req.IPConfiguration.IPSubnet.IPAddress + "/" + fmt.Sprintf("%d", req.IPConfiguration.IPSubnet.PrefixLength))
-	ipt, err := goiptables.New()
+	// in podsubnet case, ncPrimaryIP is the pod subnet's primary ip
+	// in vnet scale case, ncPrimaryIP is the node's ip
+	ncPrimaryIP, _, _ := net.ParseCIDR(req.IPConfiguration.IPSubnet.IPAddress + "/" + fmt.Sprintf("%d", req.IPConfiguration.IPSubnet.PrefixLength))
+	ipt, err := service.iptables.GetIPTables()
 	if err != nil {
 		return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to create iptables interface : %v", err)
 	}
@@ -56,41 +66,51 @@ func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainer
 		}
 	}
 
-	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())
-	if err != nil {
-		return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to check for existence of SNAT UDP rule : %v", err)
-	}
-	if !snatUDPRuleexist {
-		logger.Printf("[Azure CNS] Inserting SNAT UDP rule ...")
-		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())
+	// use any secondary ip + the nnc prefix length to get an iptables rule to allow dns and imds traffic from the pods
+	for _, v := range req.SecondaryIPConfigs {
+		// put the ip address in standard cidr form (where we zero out the parts that are not relevant)
+		_, podSubnet, _ := net.ParseCIDR(v.IPAddress + "/" + fmt.Sprintf("%d", req.IPConfiguration.IPSubnet.PrefixLength))
+
+		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())
 		if err != nil {
-			return types.FailedToRunIPTableCmd, "[Azure CNS] failed to inset SNAT UDP rule : " + err.Error()
+			return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to check for existence of pod SNAT UDP rule : %v", err)
+		}
+		if !snatUDPRuleExists {
+			logger.Printf("[Azure CNS] Inserting pod SNAT UDP rule ...")
+			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())
+			if err != nil {
+				return types.FailedToRunIPTableCmd, "[Azure CNS] failed to insert pod SNAT UDP rule : " + err.Error()
+			}
 		}
-	}
 
-	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())
-	if err != nil {
-		return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to check for existence of SNAT TCP rule : %v", err)
-	}
-	if !snatTCPRuleexist {
-		logger.Printf("[Azure CNS] Inserting SNAT TCP rule ...")
-		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())
+		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())
 		if err != nil {
-			return types.FailedToRunIPTableCmd, "[Azure CNS] failed to insert SNAT TCP rule : " + err.Error()
+			return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to check for existence of pod SNAT TCP rule : %v", err)
+		}
+		if !snatPodTCPRuleExists {
+			logger.Printf("[Azure CNS] Inserting pod SNAT TCP rule ...")
+			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())
+			if err != nil {
+				return types.FailedToRunIPTableCmd, "[Azure CNS] failed to insert pod SNAT TCP rule : " + err.Error()
+			}
 		}
-	}
 
-	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)
-	if err != nil {
-		return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to check for existence of SNAT IMDS rule : %v", err)
-	}
-	if !snatIMDSRuleexist {
-		logger.Printf("[Azure CNS] Inserting SNAT IMDS rule ...")
-		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)
+		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)
 		if err != nil {
-			return types.FailedToRunIPTableCmd, "[Azure CNS] failed to insert SNAT IMDS rule : " + err.Error()
+			return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to check for existence of pod SNAT IMDS rule : %v", err)
+		}
+		if !snatIMDSRuleexist {
+			logger.Printf("[Azure CNS] Inserting pod SNAT IMDS rule ...")
+			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)
+			if err != nil {
+				return types.FailedToRunIPTableCmd, "[Azure CNS] failed to insert pod SNAT IMDS rule : " + err.Error()
+			}
 		}
+
+		// we only need to run this code once as the iptable rule applies to all secondary ip configs in the same subnet
+		break
 	}
+
 	return types.Success, ""
 }
 

cns/restserver/internalapi_linux_test.go

@@ -0,0 +1,148 @@
+// Copyright 2020 Microsoft. All rights reserved.
+// MIT License
+
+package restserver
+
+import (
+	"strconv"
+	"testing"
+
+	"github.com/Azure/azure-container-networking/cns"
+	"github.com/Azure/azure-container-networking/cns/fakes"
+	"github.com/Azure/azure-container-networking/cns/types"
+	"github.com/Azure/azure-container-networking/iptables"
+	"github.com/Azure/azure-container-networking/network/networkutils"
+)
+
+type FakeIPTablesProvider struct {
+	iptables *fakes.IPTablesMock
+}
+
+func (c *FakeIPTablesProvider) GetIPTables() (iptablesClient, error) {
+	// persist iptables in testing
+	if c.iptables == nil {
+		c.iptables = fakes.NewIPTablesMock()
+	}
+	return c.iptables, nil
+}
+
+func TestAddSNATRules(t *testing.T) {
+	type expectedScenario struct {
+		table string
+		chain string
+		rule  []string
+	}
+
+	tests := []struct {
+		name     string
+		input    *cns.CreateNetworkContainerRequest
+		expected []expectedScenario
+	}{
+		{
+			// in pod subnet, the primary nic ip is in the same address space as the pod subnet
+			name: "podsubnet",
+			input: &cns.CreateNetworkContainerRequest{
+				NetworkContainerid: ncID,
+				IPConfiguration: cns.IPConfiguration{
+					IPSubnet: cns.IPSubnet{
+						IPAddress:    "240.1.2.1",
+						PrefixLength: 24,
+					},
+				},
+				SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{
+					"abc": {
+						IPAddress: "240.1.2.7",
+					},
+				},
+				HostPrimaryIP: "10.0.0.4",
+			},
+			expected: []expectedScenario{
+				{
+					table: iptables.Nat,
+					chain: SWIFT,
+					rule: []string{
+						"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/24", "-d",
+						networkutils.AzureDNS, "-p", iptables.UDP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", "240.1.2.1",
+					},
+				},
+				{
+					table: iptables.Nat,
+					chain: SWIFT,
+					rule: []string{
+						"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/24", "-d",
+						networkutils.AzureDNS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", "240.1.2.1",
+					},
+				},
+				{
+					table: iptables.Nat,
+					chain: SWIFT,
+					rule: []string{
+						"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/24", "-d",
+						networkutils.AzureIMDS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.HTTPPort), "-j", iptables.Snat, "--to", "10.0.0.4",
+					},
+				},
+			},
+		},
+		{
+			// in vnet scale, the primary nic ip becomes the node ip (diff address space from pod subnet)
+			name: "vnet scale",
+			input: &cns.CreateNetworkContainerRequest{
+				NetworkContainerid: ncID,
+				IPConfiguration: cns.IPConfiguration{
+					IPSubnet: cns.IPSubnet{
+						IPAddress:    "10.0.0.4",
+						PrefixLength: 28,
+					},
+				},
+				SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{
+					"abc": {
+						IPAddress: "240.1.2.15",
+					},
+				},
+				HostPrimaryIP: "10.0.0.4",
+			},
+			expected: []expectedScenario{
+				{
+					table: iptables.Nat,
+					chain: SWIFT,
+					rule: []string{
+						"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/28", "-d",
+						networkutils.AzureDNS, "-p", iptables.UDP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", "10.0.0.4",
+					},
+				},
+				{
+					table: iptables.Nat,
+					chain: SWIFT,
+					rule: []string{
+						"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/28", "-d",
+						networkutils.AzureDNS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.DNSPort), "-j", iptables.Snat, "--to", "10.0.0.4",
+					},
+				},
+				{
+					table: iptables.Nat,
+					chain: SWIFT,
+					rule: []string{
+						"-m", "addrtype", "!", "--dst-type", "local", "-s", "240.1.2.0/28", "-d",
+						networkutils.AzureIMDS, "-p", iptables.TCP, "--dport", strconv.Itoa(iptables.HTTPPort), "-j", iptables.Snat, "--to", "10.0.0.4",
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		service := getTestService(cns.KubernetesCRD)
+		service.iptables = &FakeIPTablesProvider{}
+		resp, msg := service.programSNATRules(tt.input)
+		if resp != types.Success {
+			t.Fatal("failed to program snat rules", msg, " case: ", tt.name)
+		}
+		finalState, _ := service.iptables.GetIPTables()
+		for _, ex := range tt.expected {
+			exists, err := finalState.Exists(ex.table, ex.chain, ex.rule...)
+			if err != nil || !exists {
+				t.Fatal("rule not found", ex.rule, " case: ", tt.name)
+			}
+		}
+	}
+}

cns/restserver/internalapi_windows.go

@@ -16,6 +16,14 @@ const (
 	pwshTimeout = 120 * time.Second
 )
 
+var errUnsupportedAPI = errors.New("unsupported api")
+
+type IPtablesProvider struct{}
+
+func (*IPtablesProvider) GetIPTables() (iptablesClient, error) {
+	return nil, errUnsupportedAPI
+}
+
 // nolint
 func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainerRequest) (types.ResponseCode, string) {
 	return types.Success, ""