feat: remove jump to swift-postrouting in iptables legacy as rules already exist in iptables nftables (#3958)

* add logic to delete jump to swift postrouting in legacy and fix uts

* revert ut edits

* address linter

* align getting iptables legacy with iptables

* update log

* fail silently should iptables legacy interface fail to create

Author: QxBytes

cns/fakes/iptablesfake.go

@@ -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

cns/restserver/internalapi_linux.go

@@ -3,6 +3,7 @@ package restserver
 import (
 	"fmt"
 	"net"
+	"os/exec"
 	"strconv"
 
 	"github.com/Azure/azure-container-networking/cns"
@@ -22,6 +23,16 @@ 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) {
@@ -32,6 +43,18 @@ func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainer
 	// 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 {
+		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")
+		}
+	} else {
+		logger.Printf("[Azure CNS] Could not create iptables legacy interface, continuing : %v", err)
+	}
+
 	ipt, err := service.iptables.GetIPTables()
 	if err != nil {
 		return types.UnexpectedError, fmt.Sprintf("[Azure CNS] Error. Failed to create iptables interface : %v", err)
@@ -130,7 +153,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 {
@@ -143,6 +166,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

cns/restserver/internalapi_linux_test.go

@@ -15,7 +15,8 @@ import (
 )
 
 type FakeIPTablesProvider struct {
-	iptables *fakes.IPTablesMock
+	iptables       *fakes.IPTablesMock
+	iptablesLegacy *fakes.IPTablesLegacyMock
 }
 
 func (c *FakeIPTablesProvider) GetIPTables() (iptablesClient, error) {
@@ -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
@@ -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
@@ -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)
+			}
 		})
 	}
 }

cns/restserver/internalapi_windows.go

@@ -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, ""

cns/restserver/restserver.go

@@ -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.