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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions cns/fakes/iptablesfake.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,19 @@ var (
errIndexBounds = errors.New("index out of bounds")
)

type IPTablesLegacyMock struct {
deleteCallCount int
}

func (c *IPTablesLegacyMock) Delete(_, _ string, _ ...string) error {
c.deleteCallCount++
return nil
}

func (c *IPTablesLegacyMock) DeleteCallCount() int {
return c.deleteCallCount
}

type IPTablesMock struct {
state map[string]map[string][]string
clearChainCallCount int
Expand Down
32 changes: 25 additions & 7 deletions cns/restserver/internalapi_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package restserver
import (
"fmt"
"net"
"os/exec"
"strconv"

"github.com/Azure/azure-container-networking/cns"
Expand All @@ -22,16 +23,32 @@ func (c *IPtablesProvider) GetIPTables() (iptablesClient, error) {
client, err := goiptables.New()
return client, errors.Wrap(err, "failed to get iptables client")
}
func (c *IPtablesProvider) GetIPTablesLegacy() (iptablesLegacyClient, error) {
return &iptablesLegacy{}, nil
}

type iptablesLegacy struct{}

func (c *iptablesLegacy) Delete(table, chain string, rulespec ...string) error {
cmd := append([]string{"-t", table, "-D", chain}, rulespec...)
return errors.Wrap(exec.Command("iptables-legacy", cmd...).Run(), "iptables legacy failed delete")
}

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

// Parse primary ip and ipnet from nnc
// 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))
iptl, err := service.iptables.GetIPTablesLegacy()
if err != nil {
return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to create iptables legacy interface : %v", err)
}
err = iptl.Delete(iptables.Nat, iptables.Postrouting, "-j", SWIFTPOSTROUTING)
// ignore if command fails
if err == nil {
logger.Printf("[Azure CNS] Deleted legacy jump to SWIFT-POSTROUTING Chain")
}

ipt, err := service.iptables.GetIPTables()
if err != nil {
return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to create iptables interface : %v", err)
Expand Down Expand Up @@ -103,8 +120,8 @@ func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainer

// define all rules we want in the chain
rules := [][]string{
{"-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()},
{"-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()},
{"-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},
{"-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},
{"-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},
}

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

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

// we only need to run this code once as the iptable rule applies to all secondary ip configs in the same subnet
Expand Down
36 changes: 26 additions & 10 deletions cns/restserver/internalapi_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import (
)

type FakeIPTablesProvider struct {
iptables *fakes.IPTablesMock
iptables *fakes.IPTablesMock
iptablesLegacy *fakes.IPTablesLegacyMock
}

func (c *FakeIPTablesProvider) GetIPTables() (iptablesClient, error) {
Expand All @@ -26,6 +27,13 @@ func (c *FakeIPTablesProvider) GetIPTables() (iptablesClient, error) {
return c.iptables, nil
}

func (c *FakeIPTablesProvider) GetIPTablesLegacy() (iptablesLegacyClient, error) {
if c.iptablesLegacy == nil {
c.iptablesLegacy = &fakes.IPTablesLegacyMock{}
}
return c.iptablesLegacy, nil
}

func TestAddSNATRules(t *testing.T) {
type chainExpectation struct {
table string
Expand Down Expand Up @@ -70,8 +78,8 @@ func TestAddSNATRules(t *testing.T) {
chain: SWIFTPOSTROUTING,
expected: []string{
"-N SWIFT-POSTROUTING",
"-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",
"-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",
"-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",
"-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",
"-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",
},
},
Expand Down Expand Up @@ -140,8 +148,8 @@ func TestAddSNATRules(t *testing.T) {
chain: SWIFTPOSTROUTING,
expected: []string{
"-N SWIFT-POSTROUTING",
"-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",
"-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",
"-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",
"-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",
"-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",
},
},
Expand Down Expand Up @@ -201,15 +209,15 @@ func TestAddSNATRules(t *testing.T) {
chain: SWIFTPOSTROUTING,
rule: []string{
"-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",
"-p", "udp", "--dport", strconv.Itoa(iptables.DNSPort), "-j", "SNAT", "--to", "10.0.0.4",
},
},
{
table: iptables.Nat,
chain: SWIFTPOSTROUTING,
rule: []string{
"-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",
"-p", "tcp", "--dport", strconv.Itoa(iptables.DNSPort), "-j", "SNAT", "--to", "10.0.0.4",
},
},
{
Expand All @@ -235,8 +243,8 @@ func TestAddSNATRules(t *testing.T) {
chain: SWIFTPOSTROUTING,
expected: []string{
"-N SWIFT-POSTROUTING",
"-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",
"-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",
"-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",
"-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",
"-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",
},
},
Expand Down Expand Up @@ -307,8 +315,10 @@ func TestAddSNATRules(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
service := getTestService(cns.KubernetesCRD)
ipt := fakes.NewIPTablesMock()
iptl := &fakes.IPTablesLegacyMock{}
service.iptables = &FakeIPTablesProvider{
iptables: ipt,
iptables: ipt,
iptablesLegacy: iptl,
}

// setup pre-existing rules
Expand Down Expand Up @@ -360,6 +370,12 @@ func TestAddSNATRules(t *testing.T) {
if actualClearChainCalls != tt.expectedClearChainCalls {
t.Fatalf("ClearChain call count mismatch: got %d, expected %d", actualClearChainCalls, tt.expectedClearChainCalls)
}

// verify we delete legacy swift postrouting jump
actualLegacyDeleteCalls := iptl.DeleteCallCount()
if actualLegacyDeleteCalls != 1 {
t.Fatalf("Delete call count mismatch: got %d, expected 1", actualLegacyDeleteCalls)
}
})
}
}
4 changes: 4 additions & 0 deletions cns/restserver/internalapi_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ func (*IPtablesProvider) GetIPTables() (iptablesClient, error) {
return nil, errUnsupportedAPI
}

func (*IPtablesProvider) GetIPTablesLegacy() (iptablesLegacyClient, error) {
return nil, errUnsupportedAPI
}

// nolint
func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainerRequest) (types.ResponseCode, string) {
return types.Success, ""
Expand Down
4 changes: 4 additions & 0 deletions cns/restserver/restserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,13 @@ type iptablesClient interface {
ClearChain(table string, chain string) error
Delete(table, chain string, rulespec ...string) error
}
type iptablesLegacyClient interface {
Delete(table, chain string, rulespec ...string) error
}

type iptablesGetter interface {
GetIPTables() (iptablesClient, error)
GetIPTablesLegacy() (iptablesLegacyClient, error)
}

// HTTPRestService represents http listener for CNS - Container Networking Service.
Expand Down
Loading