From 00b385d1e2fdf2a775de7863a7d39934f5c1df44 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Mon, 28 Oct 2024 17:56:28 -0700 Subject: [PATCH 01/14] fix: improve iptables version detection Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- go.mod | 1 + go.sum | 2 + npm/pkg/dataplane/dataplane_linux.go | 96 +++++++++++++++++++++++++++- npm/util/const.go | 76 ---------------------- 4 files changed, 98 insertions(+), 77 deletions(-) diff --git a/go.mod b/go.mod index f233d61a1e..ea9b2c10cb 100644 --- a/go.mod +++ b/go.mod @@ -130,6 +130,7 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/monitor/armmonitor v0.11.0 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v5 v5.2.0 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.2.0 + github.com/zcalusic/sysinfo v1.1.2 golang.org/x/sync v0.8.0 gotest.tools/v3 v3.5.1 k8s.io/kubectl v0.28.5 diff --git a/go.sum b/go.sum index b1c97ce8ec..c9efaf9f24 100644 --- a/go.sum +++ b/go.sum @@ -294,6 +294,8 @@ github.com/vishvananda/netns v0.0.4/go.mod h1:SpkAiCQRtJ6TvvxPnOSyH3BMl6unz3xZla github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= +github.com/zcalusic/sysinfo v1.1.2 h1:38KUgZQmCxlN9vUTt4miis4rU5ISJXGXOJ2rY7bMC8g= +github.com/zcalusic/sysinfo v1.1.2/go.mod h1:NX+qYnWGtJVPV0yWldff9uppNKU4h40hJIRPf/pGLv4= go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0= go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= diff --git a/npm/pkg/dataplane/dataplane_linux.go b/npm/pkg/dataplane/dataplane_linux.go index 368e499d24..6d7565e570 100644 --- a/npm/pkg/dataplane/dataplane_linux.go +++ b/npm/pkg/dataplane/dataplane_linux.go @@ -1,11 +1,25 @@ package dataplane import ( + "errors" + "fmt" + "strconv" + "strings" + + "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" "github.com/Azure/azure-container-networking/npm/util" npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" + + "github.com/zcalusic/sysinfo" + "k8s.io/klog" ) +const detectingErrMsg = "failed to detect iptables version. failed to find KUBE chains in iptables-legacy-save and iptables-nft-save and failed to get kernel version. NPM will crash to retry" + +var errDetectingIptablesVersion = errors.New(detectingErrMsg) + func (dp *DataPlane) getEndpointsToApplyPolicies(_ []*policies.NPMNetworkPolicy) (map[string]string, error) { // NOOP in Linux return nil, nil @@ -21,7 +35,9 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { } func (dp *DataPlane) bootupDataPlane() error { - util.DetectIptablesVersion(dp.ioShim) + if err := detectIptablesVersion(dp.ioShim); err != nil { + return npmerrors.ErrorWrapper(npmerrors.BootupDataplane, false, "failed to detect iptables version", err) + } // It is important to keep order to clean-up ACLs before ipsets. Otherwise we won't be able to delete ipsets referenced by ACLs if err := dp.policyMgr.Bootup(nil); err != nil { @@ -37,3 +53,81 @@ func (dp *DataPlane) refreshPodEndpoints() error { // NOOP in Linux return nil } + +// detectIptablesVersion sets the global iptables variable to nft if detected or legacy if detected. +// NPM will crash if it fails to detect either. +// This global variable is referenced in all iptables related functions. +func detectIptablesVersion(ioShim *common.IOShim) error { + klog.Info("first attempt detecting iptables version. running: iptables-nft-save -t mangle") + cmd := ioShim.Exec.Command(util.IptablesSaveNft, "-t", "mangle") + output, err := cmd.CombinedOutput() + if err == nil && strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") { + msg := "detected iptables version on first attempt. found KUBE chains in nft tables. NPM will use iptables-nft" + klog.Info(msg) + metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint) + util.Iptables = util.IptablesNft + util.IptablesSave = util.IptablesSaveNft + util.IptablesRestore = util.IptablesRestoreNft + return nil + } + + if err != nil { + msg := fmt.Sprintf("failed to detect iptables version on first attempt. error running iptables-nft-save. will try detecting using iptables-legacy-save. err: %w", err) + klog.Info(msg) + metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg) + } + + klog.Info("second attempt detecting iptables version. running: iptables-legacy-save -t mangle") + lCmd := ioShim.Exec.Command(util.IptablesSaveLegacy, "-t", "mangle") + loutput, err := lCmd.CombinedOutput() + if err == nil && strings.Contains(string(loutput), "KUBE-IPTABLES-HINT") || strings.Contains(string(loutput), "KUBE-KUBELET-CANARY") { + msg := "detected iptables version on second attempt. found KUBE chains in legacy tables. NPM will use iptables-legacy" + klog.Info(msg) + metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint) + util.Iptables = util.IptablesLegacy + util.IptablesSave = util.IptablesSaveLegacy + util.IptablesRestore = util.IptablesRestoreLegacy + return nil + } + + if err != nil { + msg := fmt.Sprintf("failed to detect iptables version on second attempt. error running iptables-legacy-save. will try detecting using kernel version. err: %w", err) + klog.Info(msg) + metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg) + } + + klog.Info("third attempt detecting iptables version. getting kernel version") + var si sysinfo.SysInfo + si.GetSysInfo() + kernelVersion := strings.Split(si.Kernel.Release, ".") + if kernelVersion[0] == "" { + msg := fmt.Sprintf("failed to detect iptables version on third attempt. error getting kernel version. err: %w", err) + klog.Info(msg) + metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg) + return errDetectingIptablesVersion + } + + majorVersion, err := strconv.Atoi(kernelVersion[0]) + if err != nil { + msg := fmt.Sprintf("failed to detect iptables version on third attempt. error converting kernel version to int. err: %w", err) + klog.Info(msg) + metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg) + return errDetectingIptablesVersion + } + + if majorVersion >= 5 { + msg := "detected iptables version on third attempt. found kernel version >= 5. NPM will use iptables-nft" + klog.Info(msg) + metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint) + util.Iptables = util.IptablesNft + util.IptablesSave = util.IptablesSaveNft + util.IptablesRestore = util.IptablesRestoreNft + return nil + } + + msg := "detected iptables version on third attempt. found kernel version < 5. NPM will use iptables-legacy" + klog.Info(msg) + metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint) + + return nil +} diff --git a/npm/util/const.go b/npm/util/const.go index 38885935be..fb50692065 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -2,14 +2,6 @@ // MIT License package util -import ( - "bytes" - "fmt" - "strings" - - "github.com/Azure/azure-container-networking/common" -) - // kubernetes related constants. const ( KubeSystemFlag string = "kube-system" @@ -271,71 +263,3 @@ const ( DaemonDataplaneID // for v2 FanOutServerID // for v2 ) - -func DetectIptablesVersion(ioShim *common.IOShim) { - cmd := ioShim.Exec.Command(IptablesSaveNft, "-t", "mangle") - - output, err := cmd.CombinedOutput() - if err != nil { - fmt.Printf("Error running iptables-nft-save: %s", err) - return - } - - if strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") { - Iptables = IptablesNft - IptablesSave = IptablesSaveNft - IptablesRestore = IptablesRestoreNft - } else { - lCmd := ioShim.Exec.Command(IptablesSaveLegacy, "-t", "mangle") - - loutput, err := lCmd.CombinedOutput() - if err != nil { - fmt.Printf("Error running iptables-legacy-save: %s", err) - return - } - - if strings.Contains(string(loutput), "KUBE-IPTABLES-HINT") || strings.Contains(string(loutput), "KUBE-KUBELET-CANARY") { - Iptables = IptablesLegacy - IptablesSave = IptablesSaveLegacy - IptablesRestore = IptablesRestoreLegacy - } else { - lsavecmd := ioShim.Exec.Command(IptablesSaveNft) - lsaveoutput, err := lsavecmd.CombinedOutput() - if err != nil { - fmt.Printf("Error running iptables-nft-save: %s", err) - return - } - - lcount := countLines(lsaveoutput) - - savecmd := ioShim.Exec.Command(IptablesSaveLegacy) - saveoutput, err := savecmd.CombinedOutput() - if err != nil { - fmt.Printf("Error running iptables-legacy-save: %s", err) - return - } - - count := countLines(saveoutput) - - if lcount > count { - Iptables = IptablesLegacy - IptablesSave = IptablesSaveLegacy - IptablesRestore = IptablesRestoreLegacy - } else { - Iptables = IptablesNft - IptablesSave = IptablesSaveNft - IptablesRestore = IptablesRestoreNft - } - } - } -} - -func countLines(output []byte) int { - count := 0 - for _, x := range bytes.Split(output, []byte("\n")) { - if len(x) >= 1 && x[0] == '-' { - count++ - } - } - return count -} From 5f3fa4bc8bc47a71e248383263ed4c5d92ea9b50 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Wed, 30 Oct 2024 18:14:23 -0700 Subject: [PATCH 02/14] fix: redo everything and add tests Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- go.mod | 1 - go.sum | 2 - npm/pkg/dataplane/dataplane_linux.go | 97 --- npm/pkg/dataplane/dataplane_linux_test.go | 16 +- npm/pkg/dataplane/dataplane_test.go | 6 +- .../policies/chain-management_linux.go | 324 +++++++-- .../policies/chain-management_linux_test.go | 625 ++++++++++++++++-- npm/pkg/dataplane/policies/policymanager.go | 4 + .../policies/policymanager_linux_test.go | 6 + .../dataplane/policies/policymanager_test.go | 11 +- npm/pkg/dataplane/policies/testutils_linux.go | 72 +- .../dataplane/policies/testutils_windows.go | 2 +- npm/util/const.go | 12 + npm/util/sysinfo_linux.go | 22 + 14 files changed, 912 insertions(+), 288 deletions(-) create mode 100644 npm/util/sysinfo_linux.go diff --git a/go.mod b/go.mod index ea9b2c10cb..f233d61a1e 100644 --- a/go.mod +++ b/go.mod @@ -130,7 +130,6 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/monitor/armmonitor v0.11.0 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v5 v5.2.0 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.2.0 - github.com/zcalusic/sysinfo v1.1.2 golang.org/x/sync v0.8.0 gotest.tools/v3 v3.5.1 k8s.io/kubectl v0.28.5 diff --git a/go.sum b/go.sum index c9efaf9f24..b1c97ce8ec 100644 --- a/go.sum +++ b/go.sum @@ -294,8 +294,6 @@ github.com/vishvananda/netns v0.0.4/go.mod h1:SpkAiCQRtJ6TvvxPnOSyH3BMl6unz3xZla github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= -github.com/zcalusic/sysinfo v1.1.2 h1:38KUgZQmCxlN9vUTt4miis4rU5ISJXGXOJ2rY7bMC8g= -github.com/zcalusic/sysinfo v1.1.2/go.mod h1:NX+qYnWGtJVPV0yWldff9uppNKU4h40hJIRPf/pGLv4= go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0= go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= diff --git a/npm/pkg/dataplane/dataplane_linux.go b/npm/pkg/dataplane/dataplane_linux.go index 6d7565e570..99769bf0ae 100644 --- a/npm/pkg/dataplane/dataplane_linux.go +++ b/npm/pkg/dataplane/dataplane_linux.go @@ -1,25 +1,10 @@ package dataplane import ( - "errors" - "fmt" - "strconv" - "strings" - - "github.com/Azure/azure-container-networking/common" - "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" - "github.com/Azure/azure-container-networking/npm/util" npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" - - "github.com/zcalusic/sysinfo" - "k8s.io/klog" ) -const detectingErrMsg = "failed to detect iptables version. failed to find KUBE chains in iptables-legacy-save and iptables-nft-save and failed to get kernel version. NPM will crash to retry" - -var errDetectingIptablesVersion = errors.New(detectingErrMsg) - func (dp *DataPlane) getEndpointsToApplyPolicies(_ []*policies.NPMNetworkPolicy) (map[string]string, error) { // NOOP in Linux return nil, nil @@ -35,10 +20,6 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { } func (dp *DataPlane) bootupDataPlane() error { - if err := detectIptablesVersion(dp.ioShim); err != nil { - return npmerrors.ErrorWrapper(npmerrors.BootupDataplane, false, "failed to detect iptables version", err) - } - // It is important to keep order to clean-up ACLs before ipsets. Otherwise we won't be able to delete ipsets referenced by ACLs if err := dp.policyMgr.Bootup(nil); err != nil { return npmerrors.ErrorWrapper(npmerrors.BootupDataplane, false, "failed to reset policy dataplane", err) @@ -53,81 +34,3 @@ func (dp *DataPlane) refreshPodEndpoints() error { // NOOP in Linux return nil } - -// detectIptablesVersion sets the global iptables variable to nft if detected or legacy if detected. -// NPM will crash if it fails to detect either. -// This global variable is referenced in all iptables related functions. -func detectIptablesVersion(ioShim *common.IOShim) error { - klog.Info("first attempt detecting iptables version. running: iptables-nft-save -t mangle") - cmd := ioShim.Exec.Command(util.IptablesSaveNft, "-t", "mangle") - output, err := cmd.CombinedOutput() - if err == nil && strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") { - msg := "detected iptables version on first attempt. found KUBE chains in nft tables. NPM will use iptables-nft" - klog.Info(msg) - metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint) - util.Iptables = util.IptablesNft - util.IptablesSave = util.IptablesSaveNft - util.IptablesRestore = util.IptablesRestoreNft - return nil - } - - if err != nil { - msg := fmt.Sprintf("failed to detect iptables version on first attempt. error running iptables-nft-save. will try detecting using iptables-legacy-save. err: %w", err) - klog.Info(msg) - metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg) - } - - klog.Info("second attempt detecting iptables version. running: iptables-legacy-save -t mangle") - lCmd := ioShim.Exec.Command(util.IptablesSaveLegacy, "-t", "mangle") - loutput, err := lCmd.CombinedOutput() - if err == nil && strings.Contains(string(loutput), "KUBE-IPTABLES-HINT") || strings.Contains(string(loutput), "KUBE-KUBELET-CANARY") { - msg := "detected iptables version on second attempt. found KUBE chains in legacy tables. NPM will use iptables-legacy" - klog.Info(msg) - metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint) - util.Iptables = util.IptablesLegacy - util.IptablesSave = util.IptablesSaveLegacy - util.IptablesRestore = util.IptablesRestoreLegacy - return nil - } - - if err != nil { - msg := fmt.Sprintf("failed to detect iptables version on second attempt. error running iptables-legacy-save. will try detecting using kernel version. err: %w", err) - klog.Info(msg) - metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg) - } - - klog.Info("third attempt detecting iptables version. getting kernel version") - var si sysinfo.SysInfo - si.GetSysInfo() - kernelVersion := strings.Split(si.Kernel.Release, ".") - if kernelVersion[0] == "" { - msg := fmt.Sprintf("failed to detect iptables version on third attempt. error getting kernel version. err: %w", err) - klog.Info(msg) - metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg) - return errDetectingIptablesVersion - } - - majorVersion, err := strconv.Atoi(kernelVersion[0]) - if err != nil { - msg := fmt.Sprintf("failed to detect iptables version on third attempt. error converting kernel version to int. err: %w", err) - klog.Info(msg) - metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg) - return errDetectingIptablesVersion - } - - if majorVersion >= 5 { - msg := "detected iptables version on third attempt. found kernel version >= 5. NPM will use iptables-nft" - klog.Info(msg) - metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint) - util.Iptables = util.IptablesNft - util.IptablesSave = util.IptablesSaveNft - util.IptablesRestore = util.IptablesRestoreNft - return nil - } - - msg := "detected iptables version on third attempt. found kernel version < 5. NPM will use iptables-legacy" - klog.Info(msg) - metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint) - - return nil -} diff --git a/npm/pkg/dataplane/dataplane_linux_test.go b/npm/pkg/dataplane/dataplane_linux_test.go index 7c48ac1d3b..837a33445d 100644 --- a/npm/pkg/dataplane/dataplane_linux_test.go +++ b/npm/pkg/dataplane/dataplane_linux_test.go @@ -1,7 +1,6 @@ package dataplane import ( - "fmt" "testing" "time" @@ -74,9 +73,6 @@ func TestNetPolInBackgroundUpdatePolicy(t *testing.T) { calls := append(getBootupTestCalls(), getAddPolicyTestCallsForDP(&testPolicyobj)...) calls = append(calls, getRemovePolicyTestCallsForDP(&testPolicyobj)...) calls = append(calls, getAddPolicyTestCallsForDP(&updatedTestPolicyobj)...) - for _, call := range calls { - fmt.Println(call) - } ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) dp, err := NewDataPlane("testnode", ioshim, netpolInBackgroundCfg, nil) @@ -133,31 +129,31 @@ func TestNetPolInBackgroundFailureToAddFirstTime(t *testing.T) { }, // restore will try twice per pMgr.AddPolicies() call testutils.TestCmd{ - Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}, ExitCode: 1, }, testutils.TestCmd{ - Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}, ExitCode: 1, }, // first policy succeeds testutils.TestCmd{ - Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}, ExitCode: 0, }, // second policy succeeds testutils.TestCmd{ - Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}, ExitCode: 0, }, // third policy fails // restore will try twice per pMgr.AddPolicies() call testutils.TestCmd{ - Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}, ExitCode: 1, }, testutils.TestCmd{ - Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}, ExitCode: 1, }, ) diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index fd32c547f3..00e40aaf29 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -1,7 +1,6 @@ package dataplane import ( - "fmt" "testing" "github.com/Azure/azure-container-networking/common" @@ -262,9 +261,6 @@ func TestUpdatePolicy(t *testing.T) { calls := append(getBootupTestCalls(), getAddPolicyTestCallsForDP(&testPolicyobj)...) calls = append(calls, getRemovePolicyTestCallsForDP(&testPolicyobj)...) calls = append(calls, getAddPolicyTestCallsForDP(&updatedTestPolicyobj)...) - for _, call := range calls { - fmt.Println(call) - } ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) dp, err := NewDataPlane("testnode", ioshim, dpCfg, nil) @@ -420,7 +416,7 @@ func TestUpdatePodCache(t *testing.T) { } func getBootupTestCalls() []testutils.TestCmd { - return append(policies.GetBootupTestCalls(true), ipsets.GetResetTestCalls()...) + return append(policies.GetBootupTestCalls(), ipsets.GetResetTestCalls()...) } func getAddPolicyTestCallsForDP(networkPolicy *policies.NPMNetworkPolicy) []testutils.TestCmd { diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index 0ad7c0ae7d..7b2c22c82d 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -13,6 +13,7 @@ import ( "github.com/Azure/azure-container-networking/npm/util" npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" "github.com/Azure/azure-container-networking/npm/util/ioutil" + "k8s.io/klog" utilexec "k8s.io/utils/exec" ) @@ -23,9 +24,13 @@ const ( // transferred from iptm.go and not sure why this length is important minLineNumberStringLength int = 3 + + detectingErrMsg = "failed to detect iptables version. failed to find KUBE chains in iptables-legacy-save and iptables-nft-save and failed to get kernel version. NPM will crash to retry" ) var ( + errDetectingIptablesVersion = errors.New(detectingErrMsg) + // Must loop through a slice because we need a deterministic order for fexec commands for UTs. iptablesAzureChains = []string{ util.IptablesAzureChain, @@ -162,6 +167,8 @@ func isBaseChain(chain string) bool { Called once at startup. Like the rest of PolicyManager, minimizes the number of OS calls by consolidating all possible actions into one iptables-restore call. +0.1. Detect iptables version. +0.2. Clean up legacy tables if using nft and vice versa. 1. Delete the deprecated jump from FORWARD to AZURE-NPM chain (if it exists). 2. Cleanup old NPM chains, and configure base chains and their rules. 1. Do the following via iptables-restore --noflush: @@ -181,87 +188,29 @@ TODO: could use one grep call instead of separate calls for getting jump line nu func (pMgr *PolicyManager) bootup(_ []string) error { klog.Infof("booting up iptables Azure chains") + // 0.1. Detect iptables version + if err := pMgr.detectIptablesVersion(); err != nil { + return npmerrors.SimpleErrorWrapper("failed to detect iptables version", err) + } + // Stop reconciling so we don't contend for iptables, and so we don't update the staleChains at the same time as reconcile() // Reconciling would only be happening if this function were called to reset iptables well into the azure-npm pod lifecycle. pMgr.reconcileManager.forceLock() defer pMgr.reconcileManager.forceUnlock() - if strings.Contains(util.Iptables, "nft") { - klog.Info("detected nft iptables. cleaning up legacy iptables") - util.Iptables = util.IptablesLegacy - util.IptablesSave = util.IptablesSaveLegacy - util.IptablesRestore = util.IptablesRestoreLegacy - - // 0. delete the deprecated jump to deprecated AZURE-NPM in legacy iptables - deprecatedErrCode, deprecatedErr := pMgr.ignoreErrorsAndRunIPTablesCommand(removeDeprecatedJumpIgnoredErrors, util.IptablesDeletionFlag, deprecatedJumpFromForwardToAzureChainArgs...) - if deprecatedErrCode == 0 { - klog.Infof("deleted deprecated jump rule from FORWARD chain to AZURE-NPM chain") - } else if deprecatedErr != nil { - metrics.SendErrorLogAndMetric(util.IptmID, - "failed to delete deprecated jump rule from FORWARD chain to AZURE-NPM chain for unexpected reason with exit code %d and error: %s", - deprecatedErrCode, deprecatedErr.Error()) - } - - // 0. delete the deprecated jump to current AZURE-NPM in legacy iptables - deprecatedErrCode, deprecatedErr = pMgr.ignoreErrorsAndRunIPTablesCommand(removeDeprecatedJumpIgnoredErrors, util.IptablesDeletionFlag, jumpFromForwardToAzureChainArgs...) - if deprecatedErrCode == 0 { - klog.Infof("deleted deprecated jump rule from FORWARD chain to AZURE-NPM chain") - } else if deprecatedErr != nil { - metrics.SendErrorLogAndMetric(util.IptmID, - "failed to delete deprecated jump rule from FORWARD chain to AZURE-NPM chain for unexpected reason with exit code %d and error: %s", - deprecatedErrCode, deprecatedErr.Error()) - } - - // clean up current chains in legacy iptables - currentChains, err := ioutil.AllCurrentAzureChains(pMgr.ioShim.Exec, util.IptablesDefaultWaitTime) - if err != nil { - return npmerrors.SimpleErrorWrapper("failed to get current chains for bootup", err) - } - - // We have only one chance to clean existing legacy iptables chains. - // So flush all the chains and then destroy them - var aggregateError error - for chain := range currentChains { - errCode, err := pMgr.runIPTablesCommand(util.IptablesFlushFlag, chain) - if err != nil && errCode != doesNotExistErrorCode { - // add to staleChains if it's not one of the iptablesAzureChains - pMgr.staleChains.add(chain) - currentErrString := fmt.Sprintf("failed to flush chain %s with err [%v]", chain, err) - if aggregateError == nil { - aggregateError = npmerrors.SimpleError(currentErrString) - } else { - aggregateError = npmerrors.SimpleErrorWrapper(fmt.Sprintf("%s and had previous error", currentErrString), aggregateError) - } - } - } - - for chain := range currentChains { - errCode, err := pMgr.runIPTablesCommand(util.IptablesDestroyFlag, chain) - if err != nil && errCode != doesNotExistErrorCode { - // add to staleChains if it's not one of the iptablesAzureChains - pMgr.staleChains.add(chain) - currentErrString := fmt.Sprintf("failed to delete chain %s with err [%v]", chain, err) - if aggregateError == nil { - aggregateError = npmerrors.SimpleError(currentErrString) - } else { - aggregateError = npmerrors.SimpleErrorWrapper(fmt.Sprintf("%s and had previous error", currentErrString), aggregateError) - } - } - } - - if aggregateError != nil { - metrics.SendErrorLogAndMetric(util.IptmID, - "failed to flush and delete stale chain in legacy iptables with error: %s", - aggregateError.Error()) - } + // 0.2. cleanup + if err := pMgr.cleanupOtherIptables(); err != nil { + return npmerrors.SimpleErrorWrapper("failed to cleanup other iptables chains", err) + } - util.Iptables = util.IptablesNft - util.IptablesSave = util.IptablesSaveNft - util.IptablesRestore = util.IptablesRestoreNft + if err := pMgr.bootupAfterDetectAndCleanup(); err != nil { + return err } - klog.Info("cleaning up default iptables") + return nil +} +func (pMgr *PolicyManager) bootupAfterDetectAndCleanup() error { // 1. delete the deprecated jump to AZURE-NPM deprecatedErrCode, deprecatedErr := pMgr.ignoreErrorsAndRunIPTablesCommand(removeDeprecatedJumpIgnoredErrors, util.IptablesDeletionFlag, deprecatedJumpFromForwardToAzureChainArgs...) if deprecatedErrCode == 0 { @@ -294,6 +243,235 @@ func (pMgr *PolicyManager) bootup(_ []string) error { return nil } +// detectIptablesVersion sets the global iptables variable to nft if detected or legacy if detected. +// NPM will crash if it fails to detect either. +// This global variable is referenced in all iptables related functions. +func (pMgr *PolicyManager) detectIptablesVersion() error { + klog.Info("first attempt detecting iptables version. running: iptables-nft-save -t mangle") + cmd := pMgr.ioShim.Exec.Command(util.IptablesSaveNft, "-t", "mangle") + output, err := cmd.CombinedOutput() + if err == nil && strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") { + msg := "detected iptables version on first attempt. found KUBE chains in nft iptables. NPM will use iptables-nft" + klog.Info(msg) + metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) + util.SetIptablesToNft() + return nil + } + + if err != nil { + msg := "failed to detect iptables version on first attempt. error running iptables-nft-save. will try detecting using iptables-legacy-save. err: %s" + metrics.SendErrorLogAndMetric(util.IptmID, msg, err.Error()) + } + + klog.Info("second attempt detecting iptables version. running: iptables-legacy-save -t mangle") + lCmd := pMgr.ioShim.Exec.Command(util.IptablesSaveLegacy, "-t", "mangle") + loutput, err := lCmd.CombinedOutput() + if err == nil && strings.Contains(string(loutput), "KUBE-IPTABLES-HINT") || strings.Contains(string(loutput), "KUBE-KUBELET-CANARY") { + msg := "detected iptables version on second attempt. found KUBE chains in legacy tables. NPM will use iptables-legacy" + klog.Info(msg) + metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) + util.SetIptablesToLegacy() + return nil + } + + if err != nil { + msg := "failed to detect iptables version on second attempt. error running iptables-legacy-save. will try detecting using kernel version. err: %s" + metrics.SendErrorLogAndMetric(util.IptmID, msg, err.Error()) + } + + klog.Info("third attempt detecting iptables version. getting kernel version") + kernelRelease := "" + if pMgr.debug { + // for testing purposes + kernelRelease = pMgr.debugKernelVersion + } else { + kernelRelease = util.KernelRelease() + } + kernelVersion := strings.Split(kernelRelease, ".")[0] + if kernelVersion == "" { + metrics.SendErrorLogAndMetric(util.IptmID, "failed to detect iptables version on third attempt. error getting kernel version") + return errDetectingIptablesVersion + } + + majorVersion, err := strconv.Atoi(kernelVersion) + if err != nil { + metrics.SendErrorLogAndMetric(util.IptmID, "failed to detect iptables version on third attempt. error converting kernel version to int. err: %s", err.Error()) + return errDetectingIptablesVersion + } + + if majorVersion >= 5 { + msg := "detected iptables version on third attempt. found kernel version >= 5. NPM will use iptables-nft" + klog.Info(msg) + metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) + util.SetIptablesToNft() + return nil + } + + msg := "detected iptables version on third attempt. found kernel version < 5. NPM will use iptables-legacy" + klog.Info(msg) + metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) + util.SetIptablesToLegacy() + return nil +} + +// clenaupOtherIptablesChains cleans up legacy tables if using nft and vice versa. +// It will only return an error if it fails to delete a jump rule and flush the AZURE-NPM chain (see comment about #3088 below). +// Cleanup logic: +// 1. delete jump rules to AZURE-NPM +// 2. flush all chains +// 3. delete all chains +func (pMgr *PolicyManager) cleanupOtherIptables() error { + hadNFT := util.Iptables == util.IptablesNft + if hadNFT { + klog.Info("detected nft iptables. cleaning up legacy iptables") + util.SetIptablesToLegacy() + } else { + klog.Info("detected legacy iptables. cleaning up legacy iptables") + util.SetIptablesToNft() + } + + defer func() { + if hadNFT { + klog.Info("cleaned up legacy iptables") + util.SetIptablesToNft() + } else { + klog.Info("cleaned up nft tables") + util.SetIptablesToLegacy() + } + }() + + deletedJumpRule := false + + // 1.1. delete the deprecated jump to AZURE-NPM + deprecatedErrCode, deprecatedErr := pMgr.ignoreErrorsAndRunIPTablesCommand(removeDeprecatedJumpIgnoredErrors, util.IptablesDeletionFlag, deprecatedJumpFromForwardToAzureChainArgs...) + if deprecatedErrCode == 0 { + klog.Infof("deleted deprecated jump rule from FORWARD chain to AZURE-NPM chain") + deletedJumpRule = true + } else if deprecatedErr != nil { + metrics.SendErrorLogAndMetric(util.IptmID, + "[cleanup] failed to delete deprecated jump rule from FORWARD chain to AZURE-NPM chain for unexpected reason with exit code %d and error: %s", + deprecatedErrCode, deprecatedErr.Error()) + } + + // 1.2. delete the jump to AZURE-NPM + deprecatedErrCode, deprecatedErr = pMgr.ignoreErrorsAndRunIPTablesCommand(removeDeprecatedJumpIgnoredErrors, util.IptablesDeletionFlag, jumpFromForwardToAzureChainArgs...) + if deprecatedErrCode == 0 { + deletedJumpRule = true + klog.Infof("deleted deprecated jump rule from FORWARD chain to AZURE-NPM chain") + } else if deprecatedErr != nil { + metrics.SendErrorLogAndMetric(util.IptmID, + "[cleanup] failed to delete jump rule from FORWARD chain to AZURE-NPM chain for unexpected reason with exit code %d and error: %s", + deprecatedErrCode, deprecatedErr.Error()) + } + + // 2. get current chains + currentChains, err := ioutil.AllCurrentAzureChains(pMgr.ioShim.Exec, util.IptablesDefaultWaitTime) + if err != nil { + return npmerrors.SimpleErrorWrapper("[cleanup] failed to get current chains for bootup", err) + } + + if len(currentChains) == 0 { + klog.Info("no chains to cleanup") + return nil + } + + klog.Infof("[cleanup] %d chains to clean up", len(currentChains)) + + // 3.1. try to flush all chains at once + chains := make([]string, 0, len(currentChains)) + _, hasAzureChain := currentChains[util.IptablesAzureChain] + if hasAzureChain { + // putting AZURE-NPM chain first is required for proper unit testing (for determinancy in destroying chains) + chains = append(chains, util.IptablesAzureChain) + } + for chain := range currentChains { + if chain == util.IptablesAzureChain { + // putting AZURE-NPM chain first is required for proper unit testing (for determinancy in destroying chains) + continue + } + chains = append(chains, chain) + } + + creator := pMgr.creatorForCleanup(chains) + if err := restore(creator); err != nil { + msg := fmt.Sprintf("[cleanup] failed to flush all chains with error: %s", err.Error()) + klog.Info(msg) + metrics.SendErrorLogAndMetric(util.IptmID, msg) + + // 3.2. if we failed to flush all chains, then try to flush and delete them one by one + var aggregateError error + if _, ok := currentChains[util.IptablesAzureChain]; ok { + _, err := pMgr.runIPTablesCommand(util.IptablesFlushFlag, util.IptablesAzureChain) + aggregateError = err + if err != nil && !deletedJumpRule { + // fixes #3088 + // if we failed to delete a jump rule to AZURE-NPM and we failed to flush AZURE-NPM chain, + // then there is risk that there is a jump rule to AZURE-NPM, which in turn has rules which could lead to allowing or dropping a packet. + // We have failed to cleanup the other iptables rules, and there is no guarantee that packets will be processed correctly now. + // So we must crash and retry. + return npmerrors.SimpleErrorWrapper("[cleanup] must crash and retry. failed to delete jump rule and flush chain %s with error", err) + } + } + + for chain := range currentChains { + if chain == util.IptablesAzureChain { + // already flushed above + continue + } + + errCode, err := pMgr.runIPTablesCommand(util.IptablesFlushFlag, chain) + if err != nil && errCode != doesNotExistErrorCode { + currentErrString := fmt.Sprintf("failed to flush chain %s with err [%v]", chain, err) + if aggregateError == nil { + aggregateError = npmerrors.SimpleError(currentErrString) + } else { + aggregateError = npmerrors.SimpleErrorWrapper(fmt.Sprintf("%s and had previous error", currentErrString), aggregateError) + } + } + } + + if aggregateError != nil { + metrics.SendErrorLogAndMetric(util.IptmID, + "[cleanup] benign failure to flush chains with error: %s", + aggregateError.Error()) + } + } + + // 4. delete all chains + var aggregateError error + for _, chain := range chains { + errCode, err := pMgr.runIPTablesCommand(util.IptablesDestroyFlag, chain) + if err != nil && errCode != doesNotExistErrorCode { + // add to staleChains if it's not one of the iptablesAzureChains + pMgr.staleChains.add(chain) + currentErrString := fmt.Sprintf("failed to delete chain %s with err [%v]", chain, err) + if aggregateError == nil { + aggregateError = npmerrors.SimpleError(currentErrString) + } else { + aggregateError = npmerrors.SimpleErrorWrapper(fmt.Sprintf("%s and had previous error", currentErrString), aggregateError) + } + } + } + + if aggregateError != nil { + metrics.SendErrorLogAndMetric(util.IptmID, + "[cleanup] benign failure to delete chains with error: %s", + aggregateError.Error()) + } + + return nil +} + +func (pMgr *PolicyManager) creatorForCleanup(chains []string) *ioutil.FileCreator { + // pass nil because we don't need to add any lines like ":CHAIN-NAME - -" because that is for creating chains + creator := pMgr.newCreatorWithChains(nil) + for _, chain := range chains { + creator.AddLine("", nil, fmt.Sprintf("-F %s", chain)) + } + creator.AddLine("", nil, util.IptablesRestoreCommit) + return creator +} + // reconcile does the following: // - creates the jump rule from FORWARD chain to AZURE-NPM chain (if it does not exist) and makes sure it's after the jumps to KUBE-FORWARD & KUBE-SERVICES chains (if they exist). // - cleans up stale policy chains. It can be forced to stop this process if reconcileManager.forceLock() is called. @@ -363,7 +541,7 @@ func (pMgr *PolicyManager) ignoreErrorsAndRunIPTablesCommand(ignored []*exitErro allArgs := []string{util.IptablesWaitFlag, util.IptablesDefaultWaitTime, operationFlag} allArgs = append(allArgs, args...) - klog.Infof("Executing iptables command with args %v", allArgs) + klog.Infof("executing iptables command [%s] with args %v", util.Iptables, allArgs) command := pMgr.ioShim.Exec.Command(util.Iptables, allArgs...) output, err := command.CombinedOutput() diff --git a/npm/pkg/dataplane/policies/chain-management_linux_test.go b/npm/pkg/dataplane/policies/chain-management_linux_test.go index 75fa6994b5..62aa4df2a9 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux_test.go +++ b/npm/pkg/dataplane/policies/chain-management_linux_test.go @@ -26,8 +26,7 @@ Chain AZURE-NPM-INGRESS (1 references) Chain AZURE-NPM-INGRESS-ALLOW-MARK (1 references) ` - grepOutputAzureV1Chains = `Chain AZURE-NPM -Chain AZURE-NPM (1 references) + grepOutputAzureV1Chains = `Chain AZURE-NPM (1 references) Chain AZURE-NPM-INGRESS (1 references) Chain AZURE-NPM-INGRESS-DROPS (1 references) Chain AZURE-NPM-INGRESS-TO (1 references) @@ -38,28 +37,12 @@ Chain AZURE-NPM-EGRESS-FROM (1 references) Chain AZURE-NPM-EGRESS-PORTS (1 references) Chain AZURE-NPM-ACCEPT (1 references) ` -) -// similar to TestBootup in policymanager.go except an error occurs -func TestBootupFailure(t *testing.T) { - metrics.ReinitializeAll() - calls := []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, //nolint // AZURE-NPM chain didn't exist - {Cmd: listAllCommandStrings, PipedToCommand: true, HasStartError: true, ExitCode: 1}, - {Cmd: []string{"grep", "Chain AZURE-NPM"}}, - } - ioshim := common.NewMockIOShim(calls) - defer ioshim.VerifyCalls(t, calls) - pMgr := NewPolicyManager(ioshim, ipsetConfig) - - metrics.IncNumACLRules() - metrics.IncNumACLRules() - - require.Error(t, pMgr.Bootup(nil)) - - // make sure that the metrics were reset - promVals{0, 0}.testPrometheusMetrics(t) -} + // pMgr.cleanupOtherIptables() can't be tested deterministically for more than two chains + grepOutputTwoAzureChains = `Chain AZURE-NPM (1 references) +Chain AZURE-NPM-INGRESS (1 references) +` +) func TestStaleChainsForceLock(t *testing.T) { testChains := []string{} @@ -73,6 +56,7 @@ func TestStaleChainsForceLock(t *testing.T) { ioshim := common.NewMockIOShim(calls) // don't verify calls because there shouldn't be as many commands as we create if forceLock works properly pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() start := make(chan struct{}, 1) done := make(chan struct{}, 1) @@ -159,6 +143,7 @@ func TestCleanupChainsSuccess(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() pMgr.staleChains.add(testChain1) pMgr.staleChains.add(testChain2) @@ -177,6 +162,7 @@ func TestCleanupChainsFailure(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() pMgr.staleChains.add(testChain1) pMgr.staleChains.add(testChain2) @@ -364,25 +350,36 @@ func TestBootupLinux(t *testing.T) { // all tests with "no NPM prior" work for any situation (with v1 or v2 prior), // but the fake command exit codes and stdouts are in line with having no NPM prior { - name: "success (no NPM prior)", - calls: GetBootupTestCalls(false), + name: "success (no NPM prior)", + calls: []testutils.TestCmd{ + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, //nolint // AZURE-NPM chain didn't exist + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + { + Cmd: []string{"grep", "Chain AZURE-NPM"}, + ExitCode: 1, + }, + fakeIPTablesRestoreCommand, + {Cmd: listLineNumbersCommandStrings, PipedToCommand: true}, + {Cmd: []string{"grep", "AZURE-NPM"}, ExitCode: 1}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + }, wantErr: false, }, { name: "success after restore failure (no NPM prior)", calls: []testutils.TestCmd{ { - Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, + Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2, Stdout: "iptables v1.8.4 (legacy): Couldn't load target `AZURE-NPM':No such file or directory", }, // AZURE-NPM chain didn't exist - {Cmd: listAllCommandStrings, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, {Cmd: []string{"grep", "Chain AZURE-NPM"}, ExitCode: 1}, fakeIPTablesRestoreFailureCommand, // e.g. xtables lock held by another app. Currently the stdout doesn't matter for retrying fakeIPTablesRestoreCommand, {Cmd: listLineNumbersCommandStrings, PipedToCommand: true}, {Cmd: []string{"grep", "AZURE-NPM"}, ExitCode: 1}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, wantErr: false, }, @@ -390,11 +387,11 @@ func TestBootupLinux(t *testing.T) { name: "success: v2 existed prior", calls: []testutils.TestCmd{ { - Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, + Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 1, Stdout: "No chain/target/match by that name", }, // deprecated rule did not exist - {Cmd: listAllCommandStrings, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, { Cmd: []string{"grep", "Chain AZURE-NPM"}, Stdout: grepOutputAzureChainsWithoutPolicies, @@ -402,15 +399,15 @@ func TestBootupLinux(t *testing.T) { fakeIPTablesRestoreCommand, {Cmd: listLineNumbersCommandStrings, PipedToCommand: true}, {Cmd: []string{"grep", "AZURE-NPM"}, ExitCode: 1}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, wantErr: false, }, { name: "v1 existed prior: successfully delete deprecated jump", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}}, // deprecated rule existed - {Cmd: listAllCommandStrings, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}}, // deprecated rule existed + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, { Cmd: []string{"grep", "Chain AZURE-NPM"}, Stdout: grepOutputAzureV1Chains, @@ -418,15 +415,15 @@ func TestBootupLinux(t *testing.T) { fakeIPTablesRestoreCommand, {Cmd: listLineNumbersCommandStrings, PipedToCommand: true}, {Cmd: []string{"grep", "AZURE-NPM"}, ExitCode: 1}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, wantErr: false, }, { name: "v1 existed prior: unknown error while deleting deprecated jump", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 3}, // unknown error - {Cmd: listAllCommandStrings, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 3}, // unknown error + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, { Cmd: []string{"grep", "Chain AZURE-NPM"}, Stdout: grepOutputAzureV1Chains, @@ -434,15 +431,15 @@ func TestBootupLinux(t *testing.T) { fakeIPTablesRestoreCommand, {Cmd: listLineNumbersCommandStrings, PipedToCommand: true}, {Cmd: []string{"grep", "AZURE-NPM"}, ExitCode: 1}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, wantErr: false, }, { name: "failure while finding current chains (no NPM prior)", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, // AZURE-NPM chain didn't exist - {Cmd: listAllCommandStrings, PipedToCommand: true, HasStartError: true, ExitCode: 1}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, // AZURE-NPM chain didn't exist + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true, HasStartError: true, ExitCode: 1}, {Cmd: []string{"grep", "Chain AZURE-NPM"}}, }, wantErr: true, @@ -450,8 +447,8 @@ func TestBootupLinux(t *testing.T) { { name: "failure twice on restore (no NPM prior)", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, // AZURE-NPM chain didn't exist - {Cmd: listAllCommandStrings, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, // AZURE-NPM chain didn't exist + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, {Cmd: []string{"grep", "Chain AZURE-NPM"}, ExitCode: 1}, fakeIPTablesRestoreFailureCommand, fakeIPTablesRestoreFailureCommand, @@ -461,8 +458,8 @@ func TestBootupLinux(t *testing.T) { { name: "failure on position (no NPM prior)", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, // AZURE-NPM chain didn't exist - {Cmd: listAllCommandStrings, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, // AZURE-NPM chain didn't exist + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, { Cmd: []string{"grep", "Chain AZURE-NPM"}, Stdout: grepOutputAzureChainsWithoutPolicies, @@ -471,7 +468,7 @@ func TestBootupLinux(t *testing.T) { {Cmd: listLineNumbersCommandStrings, PipedToCommand: true}, {Cmd: []string{"grep", "AZURE-NPM"}, ExitCode: 1}, { - Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1, }, }, @@ -484,7 +481,8 @@ func TestBootupLinux(t *testing.T) { ioshim := common.NewMockIOShim(tt.calls) defer ioshim.VerifyCalls(t, tt.calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - err := pMgr.bootup(nil) + util.SetIptablesToNft() + err := pMgr.bootupAfterDetectAndCleanup() if tt.wantErr { require.Error(t, err) } else { @@ -506,7 +504,7 @@ func TestPositionAzureChainJumpRule(t *testing.T) { calls: []testutils.TestCmd{ {Cmd: listLineNumbersCommandStrings, PipedToCommand: true}, {Cmd: []string{"grep", "AZURE-NPM"}, ExitCode: 1}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, placeAzureChainFirst: util.PlaceAzureChainFirst, wantErr: false, @@ -516,7 +514,7 @@ func TestPositionAzureChainJumpRule(t *testing.T) { calls: []testutils.TestCmd{ {Cmd: listLineNumbersCommandStrings, PipedToCommand: true}, {Cmd: []string{"grep", "AZURE-NPM"}, ExitCode: 1}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, }, placeAzureChainFirst: util.PlaceAzureChainFirst, wantErr: true, @@ -550,8 +548,8 @@ func TestPositionAzureChainJumpRule(t *testing.T) { Cmd: []string{"grep", "AZURE-NPM"}, Stdout: "2 AZURE-NPM all -- 0.0.0.0/0 0.0.0.0/0 ...", }, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, placeAzureChainFirst: util.PlaceAzureChainFirst, wantErr: false, @@ -564,7 +562,7 @@ func TestPositionAzureChainJumpRule(t *testing.T) { Cmd: []string{"grep", "AZURE-NPM"}, Stdout: "2 AZURE-NPM all -- 0.0.0.0/0 0.0.0.0/0 ...", }, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, }, placeAzureChainFirst: util.PlaceAzureChainFirst, wantErr: true, @@ -577,8 +575,8 @@ func TestPositionAzureChainJumpRule(t *testing.T) { Cmd: []string{"grep", "AZURE-NPM"}, Stdout: "2 AZURE-NPM all -- 0.0.0.0/0 0.0.0.0/0 ...", }, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, }, placeAzureChainFirst: util.PlaceAzureChainFirst, wantErr: true, @@ -590,7 +588,7 @@ func TestPositionAzureChainJumpRule(t *testing.T) { {Cmd: []string{"grep", "AZURE-NPM"}, ExitCode: 1}, {Cmd: listLineNumbersCommandStrings, PipedToCommand: true}, {Cmd: []string{"grep", "KUBE-SERVICES"}, ExitCode: 1}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, wantErr: false, @@ -605,7 +603,7 @@ func TestPositionAzureChainJumpRule(t *testing.T) { Cmd: []string{"grep", "KUBE-SERVICES"}, Stdout: "3 KUBE-SERVICES all -- 0.0.0.0/0 0.0.0.0/0 ...", }, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "4", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "4", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, wantErr: false, @@ -620,8 +618,8 @@ func TestPositionAzureChainJumpRule(t *testing.T) { }, {Cmd: listLineNumbersCommandStrings, PipedToCommand: true}, {Cmd: []string{"grep", "KUBE-SERVICES"}, ExitCode: 1}, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, wantErr: false, @@ -636,8 +634,8 @@ func TestPositionAzureChainJumpRule(t *testing.T) { }, {Cmd: listLineNumbersCommandStrings, PipedToCommand: true}, {Cmd: []string{"grep", "KUBE-SERVICES"}, ExitCode: 1}, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, wantErr: false, @@ -672,8 +670,8 @@ func TestPositionAzureChainJumpRule(t *testing.T) { Cmd: []string{"grep", "KUBE-SERVICES"}, Stdout: "3 KUBE-SERVICES all -- 0.0.0.0/0 0.0.0.0/0 ...", }, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "4", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "4", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, wantErr: false, @@ -691,8 +689,8 @@ func TestPositionAzureChainJumpRule(t *testing.T) { Cmd: []string{"grep", "KUBE-SERVICES"}, Stdout: "3 KUBE-SERVICES all -- 0.0.0.0/0 0.0.0.0/0 ...", }, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "3", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "3", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, wantErr: false, @@ -719,7 +717,7 @@ func TestPositionAzureChainJumpRule(t *testing.T) { Cmd: []string{"grep", "KUBE-SERVICES"}, Stdout: "3 KUBE-SERVICES all -- 0.0.0.0/0 0.0.0.0/0 ...", }, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "4", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "4", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, }, placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, wantErr: true, @@ -737,7 +735,7 @@ func TestPositionAzureChainJumpRule(t *testing.T) { Cmd: []string{"grep", "KUBE-SERVICES"}, Stdout: "3 KUBE-SERVICES all -- 0.0.0.0/0 0.0.0.0/0 ...", }, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, }, placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, wantErr: true, @@ -755,8 +753,8 @@ func TestPositionAzureChainJumpRule(t *testing.T) { Cmd: []string{"grep", "KUBE-SERVICES"}, Stdout: "3 KUBE-SERVICES all -- 0.0.0.0/0 0.0.0.0/0 ...", }, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "3", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "3", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, }, placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, wantErr: true, @@ -772,6 +770,8 @@ func TestPositionAzureChainJumpRule(t *testing.T) { PlaceAzureChainFirst: tt.placeAzureChainFirst, } pMgr := NewPolicyManager(ioshim, cfg) + util.SetIptablesToNft() + err := pMgr.positionAzureChainJumpRule() if tt.wantErr { require.Error(t, err) @@ -863,6 +863,8 @@ func TestChainLineNumber(t *testing.T) { ioshim := common.NewMockIOShim(tt.calls) defer ioshim.VerifyCalls(t, tt.calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() + lineNum, err := pMgr.chainLineNumber(testChainName) if tt.wantErr { require.Error(t, err) @@ -875,7 +877,7 @@ func TestChainLineNumber(t *testing.T) { } func getFakeDestroyCommand(chain string) testutils.TestCmd { - return testutils.TestCmd{Cmd: []string{"iptables", "-w", "60", "-X", chain}} + return testutils.TestCmd{Cmd: []string{"iptables-nft", "-w", "60", "-X", chain}} } func getFakeDestroyCommandWithExitCode(chain string, exitCode int) testutils.TestCmd { @@ -894,3 +896,492 @@ func stringsToMap(items []string) map[string]struct{} { } return m } + +func TestDetectIptablesVersion(t *testing.T) { + type args struct { + name string + kernelVersion string + calls []testutils.TestCmd + expectedErr bool + expectedIptablesVersion string + } + + tests := []args{ + { + name: "iptables-nft-save returns kube chains", + calls: []testutils.TestCmd{ + { + Cmd: []string{"iptables-nft-save", "-t", "mangle"}, + Stdout: iptablesSaveMangleOutput, + }, + }, + expectedErr: false, + expectedIptablesVersion: util.IptablesNft, + }, + { + name: "iptables-save returns kube chains", + calls: []testutils.TestCmd{ + { + Cmd: []string{"iptables-nft-save", "-t", "mangle"}, + Stdout: "", + }, + { + Cmd: []string{"iptables-save", "-t", "mangle"}, + Stdout: iptablesSaveMangleOutput, + }, + }, + expectedErr: false, + expectedIptablesVersion: util.IptablesLegacy, + }, + { + name: "iptables-nft-save and iptables-save both fail: kernel version >= 5", + kernelVersion: "5.0.0", + calls: []testutils.TestCmd{ + { + Cmd: []string{"iptables-nft-save", "-t", "mangle"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables-save", "-t", "mangle"}, + ExitCode: 1, + }, + }, + expectedErr: false, + expectedIptablesVersion: util.IptablesNft, + }, + { + name: "no kube chains: kernel version >= 5", + kernelVersion: "5.0.0", + calls: []testutils.TestCmd{ + { + Cmd: []string{"iptables-nft-save", "-t", "mangle"}, + Stdout: "", + }, + { + Cmd: []string{"iptables-save", "-t", "mangle"}, + Stdout: "", + }, + }, + expectedErr: false, + expectedIptablesVersion: util.IptablesNft, + }, + { + name: "no kube chains: kernel version < 5", + kernelVersion: "4.5.5", + calls: []testutils.TestCmd{ + { + Cmd: []string{"iptables-nft-save", "-t", "mangle"}, + Stdout: "", + }, + { + Cmd: []string{"iptables-save", "-t", "mangle"}, + Stdout: "", + }, + }, + expectedErr: false, + expectedIptablesVersion: util.IptablesLegacy, + }, + { + name: "no kube chains: kernel version is empty", + kernelVersion: "", + calls: []testutils.TestCmd{ + { + Cmd: []string{"iptables-nft-save", "-t", "mangle"}, + Stdout: "", + }, + { + Cmd: []string{"iptables-save", "-t", "mangle"}, + Stdout: "", + }, + }, + expectedErr: true, + }, + } + + for _, tt := range tests { + tt := tt + if tt.name != "no kube chains: kernel version is empty" { + continue + } + + t.Run(tt.name, func(t *testing.T) { + + metrics.InitializeAll() + + ioshim := common.NewMockIOShim(tt.calls) + defer ioshim.VerifyCalls(t, tt.calls) + cfg := &PolicyManagerCfg{ + debug: true, + debugKernelVersion: tt.kernelVersion, + NodeIP: "6.7.8.9", + PolicyMode: IPSetPolicyMode, + PlaceAzureChainFirst: util.PlaceAzureChainFirst, + } + pMgr := NewPolicyManager(ioshim, cfg) + err := pMgr.detectIptablesVersion() + + if tt.expectedErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tt.expectedIptablesVersion, util.Iptables) + } + }) + } +} + +func TestCleanupOtherChains(t *testing.T) { + type args struct { + name string + startWithNft bool + calls []testutils.TestCmd + expectedErr bool + } + + tests := []args{ + { + name: "cleanup legacy jump no chains", + startWithNft: true, + calls: []testutils.TestCmd{ + {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}}, // deprecated rule existed + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + { + Cmd: []string{"grep", "Chain AZURE-NPM"}, + ExitCode: 1, + }, + }, + expectedErr: false, + }, + { + name: "cleanup legacy jump and chains", + startWithNft: true, + calls: []testutils.TestCmd{ + {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}}, // deprecated rule existed + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + { + Cmd: []string{"grep", "Chain AZURE-NPM"}, + Stdout: grepOutputTwoAzureChains, + }, + {Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}}, + {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM"}}, + {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS"}}, + }, + expectedErr: false, + }, + { + name: "cleanup legacy retry flushes", + startWithNft: true, + calls: []testutils.TestCmd{ + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + { + Cmd: []string{"grep", "Chain AZURE-NPM"}, + Stdout: grepOutputTwoAzureChains, + }, + { + Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM"}}, + {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS"}}, + {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM"}}, + {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS"}}, + }, + expectedErr: false, + }, + { + name: "cleanup legacy error: delete/flush errors", + startWithNft: true, + calls: []testutils.TestCmd{ + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + { + Cmd: []string{"grep", "Chain AZURE-NPM"}, + Stdout: grepOutputTwoAzureChains, + }, + { + Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM"}, + ExitCode: 1, + }, + }, + expectedErr: true, + }, + { + name: "cleanup legacy errors ok if deleted jump (non-deprecated)", + startWithNft: true, + calls: []testutils.TestCmd{ + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + { + Cmd: []string{"grep", "Chain AZURE-NPM"}, + Stdout: grepOutputTwoAzureChains, + }, + { + Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS"}, + ExitCode: 1, + }, + }, + expectedErr: false, + }, + { + name: "cleanup legacy errors ok if deleted jump (deprecated)", + startWithNft: true, + calls: []testutils.TestCmd{ + {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}}, + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + { + Cmd: []string{"grep", "Chain AZURE-NPM"}, + Stdout: grepOutputTwoAzureChains, + }, + { + Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS"}, + ExitCode: 1, + }, + }, + expectedErr: false, + }, + { + name: "cleanup legacy other flush errors ok", + startWithNft: true, + calls: []testutils.TestCmd{ + {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}}, + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true, + ExitCode: 1, + }, + { + Cmd: []string{"grep", "Chain AZURE-NPM"}, + Stdout: grepOutputTwoAzureChains, + }, + { + Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM"}}, + { + Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM"}}, + { + Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS"}, + ExitCode: 1, + }, + }, + expectedErr: false, + }, + { + name: "cleanup legacy error: list error", + startWithNft: true, + calls: []testutils.TestCmd{ + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true, HasStartError: true, + ExitCode: 1, + }, + {Cmd: []string{"grep", "Chain AZURE-NPM"}}, + }, + expectedErr: true, + }, + { + name: "cleanup nft", + startWithNft: false, + calls: []testutils.TestCmd{ + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}}, // deprecated rule existed + { + Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + { + Cmd: []string{"grep", "Chain AZURE-NPM"}, + ExitCode: 1, + }, + }, + expectedErr: false, + }, + { + name: "cleanup nft error", + startWithNft: false, + calls: []testutils.TestCmd{ + { + Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + { + Cmd: []string{"grep", "Chain AZURE-NPM"}, + Stdout: grepOutputTwoAzureChains, + }, + { + Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM"}, ExitCode: 1}, + }, + expectedErr: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + ioshim := common.NewMockIOShim(tt.calls) + defer ioshim.VerifyCalls(t, tt.calls) + pMgr := NewPolicyManager(ioshim, ipsetConfig) + + if tt.startWithNft { + util.SetIptablesToNft() + } else { + util.SetIptablesToLegacy() + } + + err := pMgr.cleanupOtherIptables() + if tt.expectedErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + if tt.startWithNft { + require.Equal(t, util.IptablesNft, util.Iptables) + } else { + require.Equal(t, util.IptablesLegacy, util.Iptables) + } + }) + } +} + +func TestCreatorForCleanup(t *testing.T) { + chains := []string{ + "AZURE-NPM", + "AZURE-NPM-INGRESS", + "AZURE-NPM-EGRESS", + "AZURE-NPM-ACCEPT", + } + + expectedLines := []string{ + "*filter", + "-F AZURE-NPM", + "-F AZURE-NPM-INGRESS", + "-F AZURE-NPM-EGRESS", + "-F AZURE-NPM-ACCEPT", + "COMMIT", + "", + } + + ioshim := common.NewMockIOShim(nil) + defer ioshim.VerifyCalls(t, nil) + pMgr := NewPolicyManager(ioshim, ipsetConfig) + creator := pMgr.creatorForCleanup(chains) + actualLines := strings.Split(creator.ToString(), "\n") + sortedActualLines := sortFlushes(actualLines) + sortedExpectedLines := sortFlushes(expectedLines) + dptestutils.AssertEqualLines(t, sortedExpectedLines, sortedActualLines) + assertStaleChainsContain(t, pMgr.staleChains, []string{}...) +} diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index bf185ac64c..b52acb6f11 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -29,6 +29,10 @@ const ( ) type PolicyManagerCfg struct { + // debug is only used for testing. Currently just indicating whether to use debug kernel version + debug bool + // debugKernelVersion is only used for testing + debugKernelVersion string // NodeIP is only used in Windows NodeIP string // PolicyMode only affects Windows diff --git a/npm/pkg/dataplane/policies/policymanager_linux_test.go b/npm/pkg/dataplane/policies/policymanager_linux_test.go index 37f5ca356b..c6af8f1127 100644 --- a/npm/pkg/dataplane/policies/policymanager_linux_test.go +++ b/npm/pkg/dataplane/policies/policymanager_linux_test.go @@ -212,6 +212,7 @@ func TestAddPolicyFailure(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() require.Error(t, pMgr.AddPolicies([]*NPMNetworkPolicy{testNetPol}, nil)) _, ok := pMgr.GetPolicy(testNetPol.PolicyKey) @@ -224,6 +225,7 @@ func TestCreatorForAddPolicies(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() // 1. test with activation policies := []*NPMNetworkPolicy{allTestNetworkPolicies[0]} @@ -287,6 +289,7 @@ func TestCreatorForRemovePolicies(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() // 1. test without deactivation (i.e. flushing azure chain when removing the last policy) // hack: the cache is empty (and len(cache) != len(allTestNetworkPolicies)), so shouldDeactivate will be false @@ -334,6 +337,7 @@ func TestRemovePoliciesAcceptableError(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() require.NoError(t, pMgr.AddPolicies([]*NPMNetworkPolicy{bothDirectionsNetPol}, epList)) require.NoError(t, pMgr.RemovePolicy(bothDirectionsNetPol.PolicyKey)) _, ok := pMgr.GetPolicy(bothDirectionsNetPol.PolicyKey) @@ -380,6 +384,7 @@ func TestRemovePoliciesError(t *testing.T) { ioshim := common.NewMockIOShim(tt.calls) defer ioshim.VerifyCalls(t, tt.calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() err := pMgr.AddPolicies([]*NPMNetworkPolicy{bothDirectionsNetPol}, nil) require.NoError(t, err) err = pMgr.RemovePolicy(bothDirectionsNetPol.PolicyKey) @@ -402,6 +407,7 @@ func TestUpdatingStaleChains(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() // add so we can remove. no stale chains to start require.NoError(t, pMgr.AddPolicies([]*NPMNetworkPolicy{bothDirectionsNetPol}, nil)) diff --git a/npm/pkg/dataplane/policies/policymanager_test.go b/npm/pkg/dataplane/policies/policymanager_test.go index 793dcda3f0..b7c1ff99f6 100644 --- a/npm/pkg/dataplane/policies/policymanager_test.go +++ b/npm/pkg/dataplane/policies/policymanager_test.go @@ -101,10 +101,12 @@ func (p promVals) testPrometheusMetrics(t *testing.T) { // see chain-management_linux_test.go for testing when an error occurs func TestBootup(t *testing.T) { metrics.ReinitializeAll() - calls := GetBootupTestCalls(false) + calls := GetBootupTestCalls() ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) + // do NOT set iptables to nft here + // it should be set because of Bootup metrics.IncNumACLRules() metrics.IncNumACLRules() @@ -126,7 +128,7 @@ func TestAddPolicy(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - + util.SetIptablesToNft() require.NoError(t, pMgr.AddPolicies([]*NPMNetworkPolicy{testNetPol}, epList)) _, ok := pMgr.GetPolicy(testNetPol.PolicyKey) require.True(t, ok) @@ -144,6 +146,7 @@ func TestAddEmptyPolicy(t *testing.T) { testNetPol := testNetworkPolicy() ioshim := common.NewMockIOShim(nil) pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() require.NoError(t, pMgr.AddPolicies([]*NPMNetworkPolicy{ { Namespace: "x", @@ -173,7 +176,7 @@ func TestGetPolicy(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - + util.SetIptablesToNft() require.NoError(t, pMgr.AddPolicies([]*NPMNetworkPolicy{netpol}, epList)) require.True(t, pMgr.PolicyExists("x/test-netpol")) @@ -190,6 +193,7 @@ func TestRemovePolicy(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() require.NoError(t, pMgr.AddPolicies([]*NPMNetworkPolicy{testNetPol}, epList)) require.NoError(t, pMgr.RemovePolicy(testNetPol.PolicyKey)) _, ok := pMgr.GetPolicy(testNetPol.PolicyKey) @@ -202,6 +206,7 @@ func TestRemoveNonexistentPolicy(t *testing.T) { metrics.ReinitializeAll() ioshim := common.NewMockIOShim(nil) pMgr := NewPolicyManager(ioshim, ipsetConfig) + util.SetIptablesToNft() require.NoError(t, pMgr.RemovePolicy("wrong-policy-key")) promVals{0, 0}.testPrometheusMetrics(t) } diff --git a/npm/pkg/dataplane/policies/testutils_linux.go b/npm/pkg/dataplane/policies/testutils_linux.go index caf8f51ddc..0a6e8312f5 100644 --- a/npm/pkg/dataplane/policies/testutils_linux.go +++ b/npm/pkg/dataplane/policies/testutils_linux.go @@ -7,12 +7,26 @@ import ( testutils "github.com/Azure/azure-container-networking/test/utils" ) +const iptablesSaveMangleOutput = `# Generated by iptables-save v1.8.4 on Thu Jan 7 17:00:00 2021 +*mangle +:PREROUTING ACCEPT [0:0] +:INPUT ACCEPT [0:0] +:FORWARD ACCEPT [0:0] +:OUTPUT ACCEPT [0:0] +:POSTROUTING ACCEPT [0:0] +:KUBE-IPTABLES-HINT - [0:0] +:KUBE-KUBELET-CANARY - [0:0] +:KUBE-PROXY-CANARY - [0:0] +-A FORWARD -d 168.63.129.16/32 -p tcp -m tcp --dport 32526 -j DROP +-A FORWARD -d 168.63.129.16/32 -p tcp -m tcp --dport 80 -j DROP +COMMIT +` + var ( - fakeIPTablesRestoreCommand = testutils.TestCmd{Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}} - fakeIPTablesRestoreFailureCommand = testutils.TestCmd{Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, ExitCode: 1} + fakeIPTablesRestoreCommand = testutils.TestCmd{Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}} + fakeIPTablesRestoreFailureCommand = testutils.TestCmd{Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}, ExitCode: 1} - listLineNumbersCommandStrings = []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L", "FORWARD", "--line-numbers"} - listAllCommandStrings = []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"} + listLineNumbersCommandStrings = []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L", "FORWARD", "--line-numbers"} ) func GetAddPolicyTestCalls(_ *NPMNetworkPolicy) []testutils.TestCmd { @@ -27,12 +41,12 @@ func GetRemovePolicyTestCalls(policy *NPMNetworkPolicy) []testutils.TestCmd { calls := []testutils.TestCmd{} hasIngress, hasEgress := policy.hasIngressAndEgress() if hasIngress { - deleteIngressJumpSpecs := []string{"iptables", "-w", "60", "-D", util.IptablesAzureIngressChain} + deleteIngressJumpSpecs := []string{"iptables-nft", "-w", "60", "-D", util.IptablesAzureIngressChain} deleteIngressJumpSpecs = append(deleteIngressJumpSpecs, ingressJumpSpecs(policy)...) calls = append(calls, testutils.TestCmd{Cmd: deleteIngressJumpSpecs}) } if hasEgress { - deleteEgressJumpSpecs := []string{"iptables", "-w", "60", "-D", util.IptablesAzureEgressChain} + deleteEgressJumpSpecs := []string{"iptables-nft", "-w", "60", "-D", util.IptablesAzureEgressChain} deleteEgressJumpSpecs = append(deleteEgressJumpSpecs, egressJumpSpecs(policy)...) calls = append(calls, testutils.TestCmd{Cmd: deleteEgressJumpSpecs}) } @@ -50,24 +64,28 @@ func GetRemovePolicyFailureTestCalls(policy *NPMNetworkPolicy) []testutils.TestC return append(calls, fakeIPTablesRestoreFailureCommand) } -func GetBootupTestCalls(addDetectCalls bool) []testutils.TestCmd { - detectIptable := []testutils.TestCmd{ - {Cmd: []string{"iptables-nft-save", "-t", "mangle"}, Stdout: ""}, //nolint // AZURE-NPM chain didn't exist - {Cmd: []string{"iptables-save", "-t", "mangle"}, Stdout: `# Generated by iptables-save v1.8.7 on Wed May 3 01:35:24 2023 - *mangle - :PREROUTING ACCEPT [0:0] - :INPUT ACCEPT [0:0] - :FORWARD ACCEPT [0:0] - :OUTPUT ACCEPT [0:0] - :POSTROUTING ACCEPT [0:0] - :KUBE-IPTABLES-HINT - [0:0] - :KUBE-KUBELET-CANARY - [0:0] - :KUBE-PROXY-CANARY - [0:0] - COMMIT`}, //nolint // AZURE-NPM chain didn't exist - } +func GetBootupTestCalls() []testutils.TestCmd { bootUp := []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, //nolint // AZURE-NPM chain didn't exist - {Cmd: listAllCommandStrings, PipedToCommand: true}, + // detect iptables version to be nft + { + Cmd: []string{"iptables-nft-save", "-t", "mangle"}, + Stdout: iptablesSaveMangleOutput, + }, + // legacy clean up + {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, //nolint // AZURE-NPM chain didn't exist + {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 2}, //nolint // AZURE-NPM chain didn't exist + {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + { + // 1 AZURE-NPM chain + Cmd: []string{"grep", "Chain AZURE-NPM"}, + Stdout: `Chain AZURE-NPM (0 references) +`, + }, + {Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}}, + {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM"}}, + // nft bootup + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, //nolint // AZURE-NPM chain didn't exist + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, { Cmd: []string{"grep", "Chain AZURE-NPM"}, ExitCode: 1, @@ -75,17 +93,13 @@ func GetBootupTestCalls(addDetectCalls bool) []testutils.TestCmd { fakeIPTablesRestoreCommand, {Cmd: listLineNumbersCommandStrings, PipedToCommand: true}, {Cmd: []string{"grep", "AZURE-NPM"}, ExitCode: 1}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - } - - if addDetectCalls { - return append(detectIptable, bootUp...) + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, } return bootUp } func getFakeDeleteJumpCommand(chainName, jumpRule string) testutils.TestCmd { - args := []string{"iptables", "-w", "60", "-D", chainName} + args := []string{"iptables-nft", "-w", "60", "-D", chainName} args = append(args, strings.Split(jumpRule, " ")...) return testutils.TestCmd{Cmd: args} } diff --git a/npm/pkg/dataplane/policies/testutils_windows.go b/npm/pkg/dataplane/policies/testutils_windows.go index 914ea76986..dfc25dab94 100644 --- a/npm/pkg/dataplane/policies/testutils_windows.go +++ b/npm/pkg/dataplane/policies/testutils_windows.go @@ -10,6 +10,6 @@ func GetRemovePolicyTestCalls(_ *NPMNetworkPolicy) []testutils.TestCmd { return []testutils.TestCmd{} } -func GetBootupTestCalls(_ bool) []testutils.TestCmd { +func GetBootupTestCalls() []testutils.TestCmd { return []testutils.TestCmd{} } diff --git a/npm/util/const.go b/npm/util/const.go index fb50692065..a5653e3fdd 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -263,3 +263,15 @@ const ( DaemonDataplaneID // for v2 FanOutServerID // for v2 ) + +func SetIptablesToNft() { + Iptables = IptablesNft + IptablesSave = IptablesSaveNft + IptablesRestore = IptablesRestoreNft +} + +func SetIptablesToLegacy() { + Iptables = IptablesLegacy + IptablesSave = IptablesSaveLegacy + IptablesRestore = IptablesRestoreLegacy +} diff --git a/npm/util/sysinfo_linux.go b/npm/util/sysinfo_linux.go new file mode 100644 index 0000000000..523819e894 --- /dev/null +++ b/npm/util/sysinfo_linux.go @@ -0,0 +1,22 @@ +package util + +// this file has bits copied from github.com/zcalusic/sysinfo (v1.1.2) + +import ( + "os" + "strings" +) + +func KernelRelease() string { + return slurpFile("/proc/sys/kernel/osrelease") +} + +// Read one-liner text files, strip newline. +func slurpFile(path string) string { + data, err := os.ReadFile(path) + if err != nil { + return "" + } + + return strings.TrimSpace(string(data)) +} From 334d8c0449424ae394b32679a59cfe57227f503e Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:15:22 -0700 Subject: [PATCH 03/14] fix: address comments Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- .../policies/chain-management_linux.go | 63 ++++++++++--------- .../policies/chain-management_linux_test.go | 13 ++-- npm/pkg/dataplane/policies/policymanager.go | 5 +- npm/util/sysinfo_linux.go | 31 +++++++-- 4 files changed, 69 insertions(+), 43 deletions(-) diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index 7b2c22c82d..951d0afe91 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -25,7 +25,7 @@ const ( // transferred from iptm.go and not sure why this length is important minLineNumberStringLength int = 3 - detectingErrMsg = "failed to detect iptables version. failed to find KUBE chains in iptables-legacy-save and iptables-nft-save and failed to get kernel version. NPM will crash to retry" + detectingErrMsg = "failed to detect iptables version. failed to run iptables-legacy-save, run iptables-nft-save, and get kernel version. NPM will crash to retry" ) var ( @@ -249,8 +249,11 @@ func (pMgr *PolicyManager) bootupAfterDetectAndCleanup() error { func (pMgr *PolicyManager) detectIptablesVersion() error { klog.Info("first attempt detecting iptables version. running: iptables-nft-save -t mangle") cmd := pMgr.ioShim.Exec.Command(util.IptablesSaveNft, "-t", "mangle") - output, err := cmd.CombinedOutput() - if err == nil && strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") { + output, nftErr := cmd.CombinedOutput() + if nftErr != nil { + msg := "failed to detect iptables version on first attempt. error running iptables-nft-save. will try detecting using iptables-legacy-save. err: %s" + metrics.SendErrorLogAndMetric(util.IptmID, msg, nftErr.Error()) + } else if strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") { msg := "detected iptables version on first attempt. found KUBE chains in nft iptables. NPM will use iptables-nft" klog.Info(msg) metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) @@ -258,44 +261,42 @@ func (pMgr *PolicyManager) detectIptablesVersion() error { return nil } - if err != nil { - msg := "failed to detect iptables version on first attempt. error running iptables-nft-save. will try detecting using iptables-legacy-save. err: %s" - metrics.SendErrorLogAndMetric(util.IptmID, msg, err.Error()) - } - klog.Info("second attempt detecting iptables version. running: iptables-legacy-save -t mangle") lCmd := pMgr.ioShim.Exec.Command(util.IptablesSaveLegacy, "-t", "mangle") - loutput, err := lCmd.CombinedOutput() - if err == nil && strings.Contains(string(loutput), "KUBE-IPTABLES-HINT") || strings.Contains(string(loutput), "KUBE-KUBELET-CANARY") { - msg := "detected iptables version on second attempt. found KUBE chains in legacy tables. NPM will use iptables-legacy" - klog.Info(msg) - metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) - util.SetIptablesToLegacy() - return nil - } - - if err != nil { + loutput, legacyErr := lCmd.CombinedOutput() + if legacyErr != nil { msg := "failed to detect iptables version on second attempt. error running iptables-legacy-save. will try detecting using kernel version. err: %s" - metrics.SendErrorLogAndMetric(util.IptmID, msg, err.Error()) + metrics.SendErrorLogAndMetric(util.IptmID, msg, legacyErr.Error()) + } else { + if strings.Contains(string(loutput), "KUBE-IPTABLES-HINT") || strings.Contains(string(loutput), "KUBE-KUBELET-CANARY") { + msg := "detected iptables version on second attempt. found KUBE chains in legacy tables. NPM will use iptables-legacy" + klog.Info(msg) + metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) + util.SetIptablesToLegacy() + return nil + } else if nftErr != nil { + msg := "NPM will use iptables-nft. iptables-nft-save failed earlier, but iptables-legacy-save didn't have KUBE chains" + klog.Info(msg) + metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) + util.SetIptablesToNft() + return nil + } } + // we are here if either: + // 1. both nft and legacy save commands failed + // 2. both nft and legacy save commands didn't have KUBE chains klog.Info("third attempt detecting iptables version. getting kernel version") - kernelRelease := "" + var majorVersion int + var versionError error if pMgr.debug { // for testing purposes - kernelRelease = pMgr.debugKernelVersion + majorVersion = pMgr.debugKernelVersion + versionError = pMgr.debugKernelVersionErr } else { - kernelRelease = util.KernelRelease() - } - kernelVersion := strings.Split(kernelRelease, ".")[0] - if kernelVersion == "" { - metrics.SendErrorLogAndMetric(util.IptmID, "failed to detect iptables version on third attempt. error getting kernel version") - return errDetectingIptablesVersion + majorVersion, versionError = util.KernelReleaseMajorVersion() } - - majorVersion, err := strconv.Atoi(kernelVersion) - if err != nil { - metrics.SendErrorLogAndMetric(util.IptmID, "failed to detect iptables version on third attempt. error converting kernel version to int. err: %s", err.Error()) + if versionError != nil { return errDetectingIptablesVersion } diff --git a/npm/pkg/dataplane/policies/chain-management_linux_test.go b/npm/pkg/dataplane/policies/chain-management_linux_test.go index 62aa4df2a9..7fa01655a1 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux_test.go +++ b/npm/pkg/dataplane/policies/chain-management_linux_test.go @@ -900,7 +900,8 @@ func stringsToMap(items []string) map[string]struct{} { func TestDetectIptablesVersion(t *testing.T) { type args struct { name string - kernelVersion string + kernelVersion int + kernelVersionErr error calls []testutils.TestCmd expectedErr bool expectedIptablesVersion string @@ -935,7 +936,7 @@ func TestDetectIptablesVersion(t *testing.T) { }, { name: "iptables-nft-save and iptables-save both fail: kernel version >= 5", - kernelVersion: "5.0.0", + kernelVersion: 5, calls: []testutils.TestCmd{ { Cmd: []string{"iptables-nft-save", "-t", "mangle"}, @@ -951,7 +952,7 @@ func TestDetectIptablesVersion(t *testing.T) { }, { name: "no kube chains: kernel version >= 5", - kernelVersion: "5.0.0", + kernelVersion: 5, calls: []testutils.TestCmd{ { Cmd: []string{"iptables-nft-save", "-t", "mangle"}, @@ -967,7 +968,7 @@ func TestDetectIptablesVersion(t *testing.T) { }, { name: "no kube chains: kernel version < 5", - kernelVersion: "4.5.5", + kernelVersion: 4, calls: []testutils.TestCmd{ { Cmd: []string{"iptables-nft-save", "-t", "mangle"}, @@ -982,8 +983,8 @@ func TestDetectIptablesVersion(t *testing.T) { expectedIptablesVersion: util.IptablesLegacy, }, { - name: "no kube chains: kernel version is empty", - kernelVersion: "", + name: "no kube chains: kernel version error", + kernelVersionErr: fmt.Errorf("kernel version error"), calls: []testutils.TestCmd{ { Cmd: []string{"iptables-nft-save", "-t", "mangle"}, diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index b52acb6f11..1ed48a3ec2 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -32,7 +32,10 @@ type PolicyManagerCfg struct { // debug is only used for testing. Currently just indicating whether to use debug kernel version debug bool // debugKernelVersion is only used for testing - debugKernelVersion string + debugKernelVersion int + // debugKernelVersionErr is only used for testing + debugKernelVersionErr error + // NodeIP is only used in Windows NodeIP string // PolicyMode only affects Windows diff --git a/npm/util/sysinfo_linux.go b/npm/util/sysinfo_linux.go index 523819e894..53869cca30 100644 --- a/npm/util/sysinfo_linux.go +++ b/npm/util/sysinfo_linux.go @@ -3,20 +3,41 @@ package util // this file has bits copied from github.com/zcalusic/sysinfo (v1.1.2) import ( + "fmt" "os" + "strconv" "strings" ) -func KernelRelease() string { - return slurpFile("/proc/sys/kernel/osrelease") +const kernelReleaseFilepath = "/proc/sys/kernel/osrelease" + +var errNoKernelRelease = fmt.Errorf("error finding kernel release") + +func KernelReleaseMajorVersion() (int, error) { + rel, err := slurpFile(kernelReleaseFilepath) + if err != nil { + return 0, err + } + + majorString := strings.Split(rel, ".")[0] + if majorString == "" { + return 0, errNoKernelRelease + } + + majorInt, err := strconv.Atoi(majorString) + if err != nil { + return 0, fmt.Errorf("failed to convert kernel major version to int. version: %s. err: %w", majorString, err) + } + + return majorInt, nil } // Read one-liner text files, strip newline. -func slurpFile(path string) string { +func slurpFile(path string) (string, error) { data, err := os.ReadFile(path) if err != nil { - return "" + return "", fmt.Errorf("failed to read file. file: %s. err: %w", path, err) } - return strings.TrimSpace(string(data)) + return strings.TrimSpace(string(data)), nil } From 85dc587eb75e9b71d542b9ee6a73bc2527d1491a Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Mon, 4 Nov 2024 17:00:05 -0800 Subject: [PATCH 04/14] fix: avoid segfault by only listing one chain Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- .../policies/chain-management_linux.go | 72 +++++++------- .../policies/chain-management_linux_test.go | 95 ++++++++++++------- npm/pkg/dataplane/policies/testutils_linux.go | 19 +--- npm/util/const.go | 1 + 4 files changed, 104 insertions(+), 83 deletions(-) diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index 951d0afe91..bc03f42283 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -91,6 +91,10 @@ var ( util.IptablesJumpFlag, util.IptablesAzureChain, } + + listChainArgs = []string{util.IptablesWaitFlag, util.IptablesDefaultWaitTime, util.IptablesTableFlag, util.IptablesMangleTable, util.IptablesNumericFlag, util.IptablesListFlag} + listHintChainArgs = append(listChainArgs, "KUBE-IPTABLES-HINT") + listCanaryChainArgs = append(listChainArgs, "KUBE-KUBELET-CANARY") ) type exitErrorInfo struct { @@ -246,46 +250,23 @@ func (pMgr *PolicyManager) bootupAfterDetectAndCleanup() error { // detectIptablesVersion sets the global iptables variable to nft if detected or legacy if detected. // NPM will crash if it fails to detect either. // This global variable is referenced in all iptables related functions. +// NPM should use the same iptables version as kube-proxy. +// kube-proxy creates an iptables chain as a hint for which version it uses. +// For more details, see: https://kubernetes.io/blog/2022/09/07/iptables-chains-not-api/#use-case-iptables-mode func (pMgr *PolicyManager) detectIptablesVersion() error { - klog.Info("first attempt detecting iptables version. running: iptables-nft-save -t mangle") - cmd := pMgr.ioShim.Exec.Command(util.IptablesSaveNft, "-t", "mangle") - output, nftErr := cmd.CombinedOutput() - if nftErr != nil { - msg := "failed to detect iptables version on first attempt. error running iptables-nft-save. will try detecting using iptables-legacy-save. err: %s" - metrics.SendErrorLogAndMetric(util.IptmID, msg, nftErr.Error()) - } else if strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") { - msg := "detected iptables version on first attempt. found KUBE chains in nft iptables. NPM will use iptables-nft" - klog.Info(msg) - metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) + klog.Info("first attempt detecting iptables version. looking for hint/canary chain in iptables-nft") + if pMgr.hintOrCanaryChainExist(util.IptablesNft) { util.SetIptablesToNft() return nil } - klog.Info("second attempt detecting iptables version. running: iptables-legacy-save -t mangle") - lCmd := pMgr.ioShim.Exec.Command(util.IptablesSaveLegacy, "-t", "mangle") - loutput, legacyErr := lCmd.CombinedOutput() - if legacyErr != nil { - msg := "failed to detect iptables version on second attempt. error running iptables-legacy-save. will try detecting using kernel version. err: %s" - metrics.SendErrorLogAndMetric(util.IptmID, msg, legacyErr.Error()) - } else { - if strings.Contains(string(loutput), "KUBE-IPTABLES-HINT") || strings.Contains(string(loutput), "KUBE-KUBELET-CANARY") { - msg := "detected iptables version on second attempt. found KUBE chains in legacy tables. NPM will use iptables-legacy" - klog.Info(msg) - metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) - util.SetIptablesToLegacy() - return nil - } else if nftErr != nil { - msg := "NPM will use iptables-nft. iptables-nft-save failed earlier, but iptables-legacy-save didn't have KUBE chains" - klog.Info(msg) - metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) - util.SetIptablesToNft() - return nil - } + klog.Info("second attempt detecting iptables version. looking for hint/canary chain in iptables-legacy") + if pMgr.hintOrCanaryChainExist(util.IptablesLegacy) { + util.SetIptablesToLegacy() + return nil } - // we are here if either: - // 1. both nft and legacy save commands failed - // 2. both nft and legacy save commands didn't have KUBE chains + // at this point, chains do not exist in iptables legacy/nft and/or iptables commands have failed for other reasons klog.Info("third attempt detecting iptables version. getting kernel version") var majorVersion int var versionError error @@ -315,6 +296,31 @@ func (pMgr *PolicyManager) detectIptablesVersion() error { return nil } +func (pMgr *PolicyManager) hintOrCanaryChainExist(iptablesCmd string) bool { + // hint chain should exist since k8s 1.24 (see https://kubernetes.io/blog/2022/09/07/iptables-chains-not-api/#use-case-iptables-mode) + hintCmd := pMgr.ioShim.Exec.Command(iptablesCmd, listHintChainArgs...) + _, hintErr := hintCmd.CombinedOutput() + if hintErr != nil { + klog.Infof("failed to list hint chain. cmd: %s. error: %s", iptablesCmd, hintErr.Error()) + metrics.SendErrorLogAndMetric(util.IptmID, "failed to list hint chain. cmd: %s. error: %s", iptablesCmd, hintErr.Error()) + } else { + metrics.SendLog(util.IptmID, fmt.Sprintf("found hint chain. will use iptables version: %s", iptablesCmd), metrics.DonotPrint) + return true + } + + // check for canary chain + canaryCmd := pMgr.ioShim.Exec.Command(iptablesCmd, listCanaryChainArgs...) + _, canaryErr := canaryCmd.CombinedOutput() + if canaryErr != nil { + klog.Infof("failed to list canary chain. cmd: %s. error: %s", iptablesCmd, canaryErr.Error()) + metrics.SendErrorLogAndMetric(util.IptmID, "failed to list canary chain. cmd: %s. error: %s", iptablesCmd, canaryErr.Error()) + return false + } + + metrics.SendLog(util.IptmID, fmt.Sprintf("found canary chain. will use iptables version: %s", iptablesCmd), metrics.DonotPrint) + return true +} + // clenaupOtherIptablesChains cleans up legacy tables if using nft and vice versa. // It will only return an error if it fails to delete a jump rule and flush the AZURE-NPM chain (see comment about #3088 below). // Cleanup logic: diff --git a/npm/pkg/dataplane/policies/chain-management_linux_test.go b/npm/pkg/dataplane/policies/chain-management_linux_test.go index 7fa01655a1..f772d2697b 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux_test.go +++ b/npm/pkg/dataplane/policies/chain-management_linux_test.go @@ -909,58 +909,69 @@ func TestDetectIptablesVersion(t *testing.T) { tests := []args{ { - name: "iptables-nft-save returns kube chains", + name: "nft has hint chain", calls: []testutils.TestCmd{ { - Cmd: []string{"iptables-nft-save", "-t", "mangle"}, - Stdout: iptablesSaveMangleOutput, + Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + ExitCode: 0, }, }, expectedErr: false, expectedIptablesVersion: util.IptablesNft, }, { - name: "iptables-save returns kube chains", + name: "nft has only canary chain", calls: []testutils.TestCmd{ { - Cmd: []string{"iptables-nft-save", "-t", "mangle"}, - Stdout: "", + Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + ExitCode: 1, }, { - Cmd: []string{"iptables-save", "-t", "mangle"}, - Stdout: iptablesSaveMangleOutput, + Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + ExitCode: 0, }, }, expectedErr: false, - expectedIptablesVersion: util.IptablesLegacy, + expectedIptablesVersion: util.IptablesNft, }, { - name: "iptables-nft-save and iptables-save both fail: kernel version >= 5", - kernelVersion: 5, + name: "legacy has hint chain", calls: []testutils.TestCmd{ { - Cmd: []string{"iptables-nft-save", "-t", "mangle"}, + Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, ExitCode: 1, }, { - Cmd: []string{"iptables-save", "-t", "mangle"}, + Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, ExitCode: 1, }, + { + Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + ExitCode: 0, + }, }, expectedErr: false, - expectedIptablesVersion: util.IptablesNft, + expectedIptablesVersion: util.IptablesLegacy, }, { - name: "no kube chains: kernel version >= 5", + name: "nft and legacy both fail: kernel version >= 5", kernelVersion: 5, calls: []testutils.TestCmd{ { - Cmd: []string{"iptables-nft-save", "-t", "mangle"}, - Stdout: "", + Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + ExitCode: 2, }, { - Cmd: []string{"iptables-save", "-t", "mangle"}, - Stdout: "", + Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + ExitCode: 2, + }, + { + Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + ExitCode: 2, + }, + { + Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + ExitCode: 2, }, }, expectedErr: false, @@ -971,12 +982,20 @@ func TestDetectIptablesVersion(t *testing.T) { kernelVersion: 4, calls: []testutils.TestCmd{ { - Cmd: []string{"iptables-nft-save", "-t", "mangle"}, - Stdout: "", + Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + ExitCode: 1, }, { - Cmd: []string{"iptables-save", "-t", "mangle"}, - Stdout: "", + Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + ExitCode: 1, }, }, expectedErr: false, @@ -987,12 +1006,20 @@ func TestDetectIptablesVersion(t *testing.T) { kernelVersionErr: fmt.Errorf("kernel version error"), calls: []testutils.TestCmd{ { - Cmd: []string{"iptables-nft-save", "-t", "mangle"}, - Stdout: "", + Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + ExitCode: 1, }, { - Cmd: []string{"iptables-save", "-t", "mangle"}, - Stdout: "", + Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + ExitCode: 1, }, }, expectedErr: true, @@ -1001,7 +1028,8 @@ func TestDetectIptablesVersion(t *testing.T) { for _, tt := range tests { tt := tt - if tt.name != "no kube chains: kernel version is empty" { + + if tt.name != "no kube chains: kernel version error" { continue } @@ -1012,11 +1040,12 @@ func TestDetectIptablesVersion(t *testing.T) { ioshim := common.NewMockIOShim(tt.calls) defer ioshim.VerifyCalls(t, tt.calls) cfg := &PolicyManagerCfg{ - debug: true, - debugKernelVersion: tt.kernelVersion, - NodeIP: "6.7.8.9", - PolicyMode: IPSetPolicyMode, - PlaceAzureChainFirst: util.PlaceAzureChainFirst, + debug: true, + debugKernelVersion: tt.kernelVersion, + debugKernelVersionErr: tt.kernelVersionErr, + NodeIP: "6.7.8.9", + PolicyMode: IPSetPolicyMode, + PlaceAzureChainFirst: util.PlaceAzureChainFirst, } pMgr := NewPolicyManager(ioshim, cfg) err := pMgr.detectIptablesVersion() diff --git a/npm/pkg/dataplane/policies/testutils_linux.go b/npm/pkg/dataplane/policies/testutils_linux.go index 0a6e8312f5..f5f5527a3d 100644 --- a/npm/pkg/dataplane/policies/testutils_linux.go +++ b/npm/pkg/dataplane/policies/testutils_linux.go @@ -7,21 +7,6 @@ import ( testutils "github.com/Azure/azure-container-networking/test/utils" ) -const iptablesSaveMangleOutput = `# Generated by iptables-save v1.8.4 on Thu Jan 7 17:00:00 2021 -*mangle -:PREROUTING ACCEPT [0:0] -:INPUT ACCEPT [0:0] -:FORWARD ACCEPT [0:0] -:OUTPUT ACCEPT [0:0] -:POSTROUTING ACCEPT [0:0] -:KUBE-IPTABLES-HINT - [0:0] -:KUBE-KUBELET-CANARY - [0:0] -:KUBE-PROXY-CANARY - [0:0] --A FORWARD -d 168.63.129.16/32 -p tcp -m tcp --dport 32526 -j DROP --A FORWARD -d 168.63.129.16/32 -p tcp -m tcp --dport 80 -j DROP -COMMIT -` - var ( fakeIPTablesRestoreCommand = testutils.TestCmd{Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}} fakeIPTablesRestoreFailureCommand = testutils.TestCmd{Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}, ExitCode: 1} @@ -68,8 +53,8 @@ func GetBootupTestCalls() []testutils.TestCmd { bootUp := []testutils.TestCmd{ // detect iptables version to be nft { - Cmd: []string{"iptables-nft-save", "-t", "mangle"}, - Stdout: iptablesSaveMangleOutput, + Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + ExitCode: 0, }, // legacy clean up {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, //nolint // AZURE-NPM chain didn't exist diff --git a/npm/util/const.go b/npm/util/const.go index a5653e3fdd..c40821b716 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -83,6 +83,7 @@ const ( IptablesEstablishedState string = "ESTABLISHED" IptablesNewState string = "NEW" IptablesFilterTable string = "filter" + IptablesMangleTable string = "mangle" IptablesCommentModuleFlag string = "comment" IptablesCommentFlag string = "--comment" IptablesAddCommentFlag From b399ec0413ed9ebe8bd70e9b927d790955fa3e5f Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Mon, 4 Nov 2024 17:06:49 -0800 Subject: [PATCH 05/14] style: log the kernel version Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- npm/pkg/dataplane/policies/chain-management_linux.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index bc03f42283..7a00107c8a 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -282,16 +282,16 @@ func (pMgr *PolicyManager) detectIptablesVersion() error { } if majorVersion >= 5 { - msg := "detected iptables version on third attempt. found kernel version >= 5. NPM will use iptables-nft" - klog.Info(msg) - metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) + msg := "detected iptables version on third attempt. found kernel version >= 5. NPM will use iptables-nft. kernel version: %d" + klog.Infof(msg, majorVersion) + metrics.SendLog(util.IptmID, fmt.Sprintf(msg, majorVersion), metrics.DonotPrint) util.SetIptablesToNft() return nil } - msg := "detected iptables version on third attempt. found kernel version < 5. NPM will use iptables-legacy" - klog.Info(msg) - metrics.SendLog(util.IptmID, msg, metrics.DonotPrint) + msg := "detected iptables version on third attempt. found kernel version < 5. NPM will use iptables-legacy. kernel version: %d" + klog.Infof(msg, majorVersion) + metrics.SendLog(util.IptmID, fmt.Sprintf(msg, majorVersion), metrics.DonotPrint) util.SetIptablesToLegacy() return nil } From c149095041639753d28350a159c9283c0be8400a Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Tue, 5 Nov 2024 17:01:39 -0800 Subject: [PATCH 06/14] style: fix lints Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- .../policies/chain-management_linux.go | 20 ++++++++++--------- .../policies/chain-management_linux_test.go | 5 ++++- npm/pkg/dataplane/policies/policymanager.go | 6 +++--- npm/util/sysinfo_linux.go | 3 ++- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index 7a00107c8a..e3d36305e4 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -26,6 +26,8 @@ const ( minLineNumberStringLength int = 3 detectingErrMsg = "failed to detect iptables version. failed to run iptables-legacy-save, run iptables-nft-save, and get kernel version. NPM will crash to retry" + + minNftKernelVersion = 5 ) var ( @@ -281,7 +283,7 @@ func (pMgr *PolicyManager) detectIptablesVersion() error { return errDetectingIptablesVersion } - if majorVersion >= 5 { + if majorVersion >= minNftKernelVersion { msg := "detected iptables version on third attempt. found kernel version >= 5. NPM will use iptables-nft. kernel version: %d" klog.Infof(msg, majorVersion) metrics.SendLog(util.IptmID, fmt.Sprintf(msg, majorVersion), metrics.DonotPrint) @@ -304,7 +306,7 @@ func (pMgr *PolicyManager) hintOrCanaryChainExist(iptablesCmd string) bool { klog.Infof("failed to list hint chain. cmd: %s. error: %s", iptablesCmd, hintErr.Error()) metrics.SendErrorLogAndMetric(util.IptmID, "failed to list hint chain. cmd: %s. error: %s", iptablesCmd, hintErr.Error()) } else { - metrics.SendLog(util.IptmID, fmt.Sprintf("found hint chain. will use iptables version: %s", iptablesCmd), metrics.DonotPrint) + metrics.SendLog(util.IptmID, "found hint chain. will use iptables version: %s"+iptablesCmd, metrics.DonotPrint) return true } @@ -317,7 +319,7 @@ func (pMgr *PolicyManager) hintOrCanaryChainExist(iptablesCmd string) bool { return false } - metrics.SendLog(util.IptmID, fmt.Sprintf("found canary chain. will use iptables version: %s", iptablesCmd), metrics.DonotPrint) + metrics.SendLog(util.IptmID, "found canary chain. will use iptables version: "+iptablesCmd, metrics.DonotPrint) return true } @@ -401,9 +403,9 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error { creator := pMgr.creatorForCleanup(chains) if err := restore(creator); err != nil { - msg := fmt.Sprintf("[cleanup] failed to flush all chains with error: %s", err.Error()) - klog.Info(msg) - metrics.SendErrorLogAndMetric(util.IptmID, msg) + msg := "[cleanup] failed to flush all chains with error: %s" + klog.Infof(msg, err.Error()) + metrics.SendErrorLogAndMetric(util.IptmID, msg, err.Error()) // 3.2. if we failed to flush all chains, then try to flush and delete them one by one var aggregateError error @@ -432,7 +434,7 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error { if aggregateError == nil { aggregateError = npmerrors.SimpleError(currentErrString) } else { - aggregateError = npmerrors.SimpleErrorWrapper(fmt.Sprintf("%s and had previous error", currentErrString), aggregateError) + aggregateError = npmerrors.SimpleErrorWrapper(currentErrString+" and had previous error", aggregateError) } } } @@ -455,7 +457,7 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error { if aggregateError == nil { aggregateError = npmerrors.SimpleError(currentErrString) } else { - aggregateError = npmerrors.SimpleErrorWrapper(fmt.Sprintf("%s and had previous error", currentErrString), aggregateError) + aggregateError = npmerrors.SimpleErrorWrapper(currentErrString+" and had previous error", aggregateError) } } } @@ -473,7 +475,7 @@ func (pMgr *PolicyManager) creatorForCleanup(chains []string) *ioutil.FileCreato // pass nil because we don't need to add any lines like ":CHAIN-NAME - -" because that is for creating chains creator := pMgr.newCreatorWithChains(nil) for _, chain := range chains { - creator.AddLine("", nil, fmt.Sprintf("-F %s", chain)) + creator.AddLine("", nil, "-F "+chain) } creator.AddLine("", nil, util.IptablesRestoreCommit) return creator diff --git a/npm/pkg/dataplane/policies/chain-management_linux_test.go b/npm/pkg/dataplane/policies/chain-management_linux_test.go index f772d2697b..d039066037 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux_test.go +++ b/npm/pkg/dataplane/policies/chain-management_linux_test.go @@ -1,6 +1,7 @@ package policies import ( + "errors" "fmt" "sort" "strings" @@ -44,6 +45,8 @@ Chain AZURE-NPM-INGRESS (1 references) ` ) +var errKernelVersion = errors.New("kernel error") + func TestStaleChainsForceLock(t *testing.T) { testChains := []string{} for i := 0; i < 100000; i++ { @@ -1003,7 +1006,7 @@ func TestDetectIptablesVersion(t *testing.T) { }, { name: "no kube chains: kernel version error", - kernelVersionErr: fmt.Errorf("kernel version error"), + kernelVersionErr: errKernelVersion, calls: []testutils.TestCmd{ { Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index 1ed48a3ec2..654b8c87f0 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -30,11 +30,11 @@ const ( type PolicyManagerCfg struct { // debug is only used for testing. Currently just indicating whether to use debug kernel version - debug bool + debug bool //nolint:unused // only used in linux // debugKernelVersion is only used for testing - debugKernelVersion int + debugKernelVersion int //nolint:unused // only used in linux // debugKernelVersionErr is only used for testing - debugKernelVersionErr error + debugKernelVersionErr error //nolint:unused // only used in linux // NodeIP is only used in Windows NodeIP string diff --git a/npm/util/sysinfo_linux.go b/npm/util/sysinfo_linux.go index 53869cca30..d960d85928 100644 --- a/npm/util/sysinfo_linux.go +++ b/npm/util/sysinfo_linux.go @@ -3,6 +3,7 @@ package util // this file has bits copied from github.com/zcalusic/sysinfo (v1.1.2) import ( + "errors" "fmt" "os" "strconv" @@ -11,7 +12,7 @@ import ( const kernelReleaseFilepath = "/proc/sys/kernel/osrelease" -var errNoKernelRelease = fmt.Errorf("error finding kernel release") +var errNoKernelRelease = errors.New("error finding kernel release") func KernelReleaseMajorVersion() (int, error) { rel, err := slurpFile(kernelReleaseFilepath) From 36b33568f5047091bf9e5ee56fe216d2a6848a3f Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Tue, 5 Nov 2024 17:37:13 -0800 Subject: [PATCH 07/14] fix: don't use stale chains. add comments. minor style change Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- .../policies/chain-management_linux.go | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index e3d36305e4..b18ffd83ec 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -335,7 +335,7 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error { klog.Info("detected nft iptables. cleaning up legacy iptables") util.SetIptablesToLegacy() } else { - klog.Info("detected legacy iptables. cleaning up legacy iptables") + klog.Info("detected legacy iptables. cleaning up nft iptables") util.SetIptablesToNft() } @@ -352,25 +352,25 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error { deletedJumpRule := false // 1.1. delete the deprecated jump to AZURE-NPM - deprecatedErrCode, deprecatedErr := pMgr.ignoreErrorsAndRunIPTablesCommand(removeDeprecatedJumpIgnoredErrors, util.IptablesDeletionFlag, deprecatedJumpFromForwardToAzureChainArgs...) - if deprecatedErrCode == 0 { + errCode, err := pMgr.ignoreErrorsAndRunIPTablesCommand(removeDeprecatedJumpIgnoredErrors, util.IptablesDeletionFlag, deprecatedJumpFromForwardToAzureChainArgs...) + if errCode == 0 { klog.Infof("deleted deprecated jump rule from FORWARD chain to AZURE-NPM chain") deletedJumpRule = true - } else if deprecatedErr != nil { + } else if err != nil { metrics.SendErrorLogAndMetric(util.IptmID, "[cleanup] failed to delete deprecated jump rule from FORWARD chain to AZURE-NPM chain for unexpected reason with exit code %d and error: %s", - deprecatedErrCode, deprecatedErr.Error()) + errCode, err.Error()) } // 1.2. delete the jump to AZURE-NPM - deprecatedErrCode, deprecatedErr = pMgr.ignoreErrorsAndRunIPTablesCommand(removeDeprecatedJumpIgnoredErrors, util.IptablesDeletionFlag, jumpFromForwardToAzureChainArgs...) - if deprecatedErrCode == 0 { + errCode, err = pMgr.ignoreErrorsAndRunIPTablesCommand(removeDeprecatedJumpIgnoredErrors, util.IptablesDeletionFlag, jumpFromForwardToAzureChainArgs...) + if errCode == 0 { deletedJumpRule = true klog.Infof("deleted deprecated jump rule from FORWARD chain to AZURE-NPM chain") - } else if deprecatedErr != nil { + } else if err != nil { metrics.SendErrorLogAndMetric(util.IptmID, "[cleanup] failed to delete jump rule from FORWARD chain to AZURE-NPM chain for unexpected reason with exit code %d and error: %s", - deprecatedErrCode, deprecatedErr.Error()) + errCode, err.Error()) } // 2. get current chains @@ -430,6 +430,9 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error { errCode, err := pMgr.runIPTablesCommand(util.IptablesFlushFlag, chain) if err != nil && errCode != doesNotExistErrorCode { + // NOTE: if we fail to flush or delete the chain, then we will never clean it up in the future. + // This is zero-day behavior since NPM supported nft (we used to mark the chain stale, but this would not have worked as expected). + // NPM currently has no mechanism for retrying flush/delete for a chain from the other iptables version (other than the AZURE-NPM chain which is handled above). currentErrString := fmt.Sprintf("failed to flush chain %s with err [%v]", chain, err) if aggregateError == nil { aggregateError = npmerrors.SimpleError(currentErrString) @@ -451,8 +454,9 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error { for _, chain := range chains { errCode, err := pMgr.runIPTablesCommand(util.IptablesDestroyFlag, chain) if err != nil && errCode != doesNotExistErrorCode { - // add to staleChains if it's not one of the iptablesAzureChains - pMgr.staleChains.add(chain) + // NOTE: if we fail to flush or delete the chain, then we will never clean it up in the future. + // This is zero-day behavior since NPM supported nft (we used to mark the chain stale, but this would not have worked as expected). + // NPM currently has no mechanism for retrying flush/delete for a chain from the other iptables version (other than the AZURE-NPM chain which is handled above). currentErrString := fmt.Sprintf("failed to delete chain %s with err [%v]", chain, err) if aggregateError == nil { aggregateError = npmerrors.SimpleError(currentErrString) From 5acf87eb9071389eed47470e1ff013ff10417921 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Tue, 5 Nov 2024 18:57:19 -0800 Subject: [PATCH 08/14] fix: listing kube chain. get stderr too. also add missing ut Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- .../policies/chain-management_linux.go | 24 ++++--- .../policies/chain-management_linux_test.go | 66 ++++++++++++++----- npm/pkg/dataplane/policies/testutils_linux.go | 2 +- 3 files changed, 60 insertions(+), 32 deletions(-) diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index b18ffd83ec..f9a892253c 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -94,9 +94,8 @@ var ( util.IptablesAzureChain, } - listChainArgs = []string{util.IptablesWaitFlag, util.IptablesDefaultWaitTime, util.IptablesTableFlag, util.IptablesMangleTable, util.IptablesNumericFlag, util.IptablesListFlag} - listHintChainArgs = append(listChainArgs, "KUBE-IPTABLES-HINT") - listCanaryChainArgs = append(listChainArgs, "KUBE-KUBELET-CANARY") + listHintChainArgs = []string{"KUBE-IPTABLES-HINT", util.IptablesTableFlag, util.IptablesMangleTable, util.IptablesNumericFlag} + listCanaryChainArgs = []string{"KUBE-KUBELET-CANARY", util.IptablesTableFlag, util.IptablesMangleTable, util.IptablesNumericFlag} ) type exitErrorInfo struct { @@ -300,22 +299,21 @@ func (pMgr *PolicyManager) detectIptablesVersion() error { func (pMgr *PolicyManager) hintOrCanaryChainExist(iptablesCmd string) bool { // hint chain should exist since k8s 1.24 (see https://kubernetes.io/blog/2022/09/07/iptables-chains-not-api/#use-case-iptables-mode) - hintCmd := pMgr.ioShim.Exec.Command(iptablesCmd, listHintChainArgs...) - _, hintErr := hintCmd.CombinedOutput() - if hintErr != nil { - klog.Infof("failed to list hint chain. cmd: %s. error: %s", iptablesCmd, hintErr.Error()) - metrics.SendErrorLogAndMetric(util.IptmID, "failed to list hint chain. cmd: %s. error: %s", iptablesCmd, hintErr.Error()) - } else { + prevIptables := util.Iptables + util.Iptables = iptablesCmd + defer func() { + util.Iptables = prevIptables + }() + + _, hintErr := pMgr.runIPTablesCommand(util.IptablesListFlag, listHintChainArgs...) + if hintErr == nil { metrics.SendLog(util.IptmID, "found hint chain. will use iptables version: %s"+iptablesCmd, metrics.DonotPrint) return true } // check for canary chain - canaryCmd := pMgr.ioShim.Exec.Command(iptablesCmd, listCanaryChainArgs...) - _, canaryErr := canaryCmd.CombinedOutput() + _, canaryErr := pMgr.runIPTablesCommand(util.IptablesListFlag, listCanaryChainArgs...) if canaryErr != nil { - klog.Infof("failed to list canary chain. cmd: %s. error: %s", iptablesCmd, canaryErr.Error()) - metrics.SendErrorLogAndMetric(util.IptmID, "failed to list canary chain. cmd: %s. error: %s", iptablesCmd, canaryErr.Error()) return false } diff --git a/npm/pkg/dataplane/policies/chain-management_linux_test.go b/npm/pkg/dataplane/policies/chain-management_linux_test.go index d039066037..5b7d0f1cbb 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux_test.go +++ b/npm/pkg/dataplane/policies/chain-management_linux_test.go @@ -915,7 +915,7 @@ func TestDetectIptablesVersion(t *testing.T) { name: "nft has hint chain", calls: []testutils.TestCmd{ { - Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, ExitCode: 0, }, }, @@ -926,11 +926,11 @@ func TestDetectIptablesVersion(t *testing.T) { name: "nft has only canary chain", calls: []testutils.TestCmd{ { - Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, ExitCode: 1, }, { - Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"}, ExitCode: 0, }, }, @@ -941,15 +941,15 @@ func TestDetectIptablesVersion(t *testing.T) { name: "legacy has hint chain", calls: []testutils.TestCmd{ { - Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, ExitCode: 1, }, { - Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"}, ExitCode: 1, }, { - Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, ExitCode: 0, }, }, @@ -961,19 +961,19 @@ func TestDetectIptablesVersion(t *testing.T) { kernelVersion: 5, calls: []testutils.TestCmd{ { - Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, ExitCode: 2, }, { - Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"}, ExitCode: 2, }, { - Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, ExitCode: 2, }, { - Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"}, ExitCode: 2, }, }, @@ -985,19 +985,19 @@ func TestDetectIptablesVersion(t *testing.T) { kernelVersion: 4, calls: []testutils.TestCmd{ { - Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, ExitCode: 1, }, { - Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"}, ExitCode: 1, }, { - Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, ExitCode: 1, }, { - Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"}, ExitCode: 1, }, }, @@ -1009,19 +1009,19 @@ func TestDetectIptablesVersion(t *testing.T) { kernelVersionErr: errKernelVersion, calls: []testutils.TestCmd{ { - Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, ExitCode: 1, }, { - Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"}, ExitCode: 1, }, { - Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, ExitCode: 1, }, { - Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"}, + Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"}, ExitCode: 1, }, }, @@ -1173,6 +1173,36 @@ func TestCleanupOtherChains(t *testing.T) { }, expectedErr: true, }, + { + name: "don't flush azure chain if it isn't there", + startWithNft: true, + calls: []testutils.TestCmd{ + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + { + Cmd: []string{"grep", "Chain AZURE-NPM"}, + Stdout: "Chain AZURE-NPM-INGRESS (1 references)\n", + }, + { + Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + { + Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"}, + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS"}}, + {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS"}}, + }, + expectedErr: false, + }, { name: "cleanup legacy errors ok if deleted jump (non-deprecated)", startWithNft: true, diff --git a/npm/pkg/dataplane/policies/testutils_linux.go b/npm/pkg/dataplane/policies/testutils_linux.go index f5f5527a3d..112d020e47 100644 --- a/npm/pkg/dataplane/policies/testutils_linux.go +++ b/npm/pkg/dataplane/policies/testutils_linux.go @@ -53,7 +53,7 @@ func GetBootupTestCalls() []testutils.TestCmd { bootUp := []testutils.TestCmd{ // detect iptables version to be nft { - Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"}, + Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, ExitCode: 0, }, // legacy clean up From 7c7ead65e842aca2231ff629604945f9f05ba9dd Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Tue, 5 Nov 2024 19:59:18 -0800 Subject: [PATCH 09/14] fix: log messages Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- npm/pkg/dataplane/policies/chain-management_linux.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index f9a892253c..a95e64226a 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -352,7 +352,7 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error { // 1.1. delete the deprecated jump to AZURE-NPM errCode, err := pMgr.ignoreErrorsAndRunIPTablesCommand(removeDeprecatedJumpIgnoredErrors, util.IptablesDeletionFlag, deprecatedJumpFromForwardToAzureChainArgs...) if errCode == 0 { - klog.Infof("deleted deprecated jump rule from FORWARD chain to AZURE-NPM chain") + klog.Infof("[cleanup] deleted deprecated jump rule from FORWARD chain to AZURE-NPM chain") deletedJumpRule = true } else if err != nil { metrics.SendErrorLogAndMetric(util.IptmID, @@ -364,7 +364,7 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error { errCode, err = pMgr.ignoreErrorsAndRunIPTablesCommand(removeDeprecatedJumpIgnoredErrors, util.IptablesDeletionFlag, jumpFromForwardToAzureChainArgs...) if errCode == 0 { deletedJumpRule = true - klog.Infof("deleted deprecated jump rule from FORWARD chain to AZURE-NPM chain") + klog.Infof("[cleanup] deleted jump rule from FORWARD chain to AZURE-NPM chain") } else if err != nil { metrics.SendErrorLogAndMetric(util.IptmID, "[cleanup] failed to delete jump rule from FORWARD chain to AZURE-NPM chain for unexpected reason with exit code %d and error: %s", @@ -416,7 +416,7 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error { // then there is risk that there is a jump rule to AZURE-NPM, which in turn has rules which could lead to allowing or dropping a packet. // We have failed to cleanup the other iptables rules, and there is no guarantee that packets will be processed correctly now. // So we must crash and retry. - return npmerrors.SimpleErrorWrapper("[cleanup] must crash and retry. failed to delete jump rule and flush chain %s with error", err) + return npmerrors.SimpleErrorWrapper("[cleanup] must crash and retry. failed to delete jump rule and flush AZURE-NPM chain with error", err) } } From 10f1f142cc10c60db9dd4403b7f2d105afa1ee79 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Wed, 6 Nov 2024 10:55:56 -0800 Subject: [PATCH 10/14] fix: stop checking kernel version. default nft, never crash Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- .../policies/chain-management_linux.go | 47 ++-------- .../policies/chain-management_linux_test.go | 92 +++++-------------- npm/pkg/dataplane/policies/policymanager.go | 7 -- .../policies/policymanager_linux_test.go | 6 -- .../dataplane/policies/policymanager_test.go | 10 +- npm/util/const.go | 10 +- npm/util/sysinfo_linux.go | 44 --------- 7 files changed, 43 insertions(+), 173 deletions(-) delete mode 100644 npm/util/sysinfo_linux.go diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index a95e64226a..39d1afddd2 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -24,15 +24,9 @@ const ( // transferred from iptm.go and not sure why this length is important minLineNumberStringLength int = 3 - - detectingErrMsg = "failed to detect iptables version. failed to run iptables-legacy-save, run iptables-nft-save, and get kernel version. NPM will crash to retry" - - minNftKernelVersion = 5 ) var ( - errDetectingIptablesVersion = errors.New(detectingErrMsg) - // Must loop through a slice because we need a deterministic order for fexec commands for UTs. iptablesAzureChains = []string{ util.IptablesAzureChain, @@ -194,9 +188,7 @@ func (pMgr *PolicyManager) bootup(_ []string) error { klog.Infof("booting up iptables Azure chains") // 0.1. Detect iptables version - if err := pMgr.detectIptablesVersion(); err != nil { - return npmerrors.SimpleErrorWrapper("failed to detect iptables version", err) - } + pMgr.detectIptablesVersion() // Stop reconciling so we don't contend for iptables, and so we don't update the staleChains at the same time as reconcile() // Reconciling would only be happening if this function were called to reset iptables well into the azure-npm pod lifecycle. @@ -254,47 +246,22 @@ func (pMgr *PolicyManager) bootupAfterDetectAndCleanup() error { // NPM should use the same iptables version as kube-proxy. // kube-proxy creates an iptables chain as a hint for which version it uses. // For more details, see: https://kubernetes.io/blog/2022/09/07/iptables-chains-not-api/#use-case-iptables-mode -func (pMgr *PolicyManager) detectIptablesVersion() error { +func (pMgr *PolicyManager) detectIptablesVersion() { klog.Info("first attempt detecting iptables version. looking for hint/canary chain in iptables-nft") if pMgr.hintOrCanaryChainExist(util.IptablesNft) { util.SetIptablesToNft() - return nil + return } klog.Info("second attempt detecting iptables version. looking for hint/canary chain in iptables-legacy") if pMgr.hintOrCanaryChainExist(util.IptablesLegacy) { util.SetIptablesToLegacy() - return nil - } - - // at this point, chains do not exist in iptables legacy/nft and/or iptables commands have failed for other reasons - klog.Info("third attempt detecting iptables version. getting kernel version") - var majorVersion int - var versionError error - if pMgr.debug { - // for testing purposes - majorVersion = pMgr.debugKernelVersion - versionError = pMgr.debugKernelVersionErr - } else { - majorVersion, versionError = util.KernelReleaseMajorVersion() - } - if versionError != nil { - return errDetectingIptablesVersion - } - - if majorVersion >= minNftKernelVersion { - msg := "detected iptables version on third attempt. found kernel version >= 5. NPM will use iptables-nft. kernel version: %d" - klog.Infof(msg, majorVersion) - metrics.SendLog(util.IptmID, fmt.Sprintf(msg, majorVersion), metrics.DonotPrint) - util.SetIptablesToNft() - return nil + return } - msg := "detected iptables version on third attempt. found kernel version < 5. NPM will use iptables-legacy. kernel version: %d" - klog.Infof(msg, majorVersion) - metrics.SendLog(util.IptmID, fmt.Sprintf(msg, majorVersion), metrics.DonotPrint) - util.SetIptablesToLegacy() - return nil + // default to nft if nothing is found + util.SetIptablesToNft() + return } func (pMgr *PolicyManager) hintOrCanaryChainExist(iptablesCmd string) bool { diff --git a/npm/pkg/dataplane/policies/chain-management_linux_test.go b/npm/pkg/dataplane/policies/chain-management_linux_test.go index 5b7d0f1cbb..518798bf66 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux_test.go +++ b/npm/pkg/dataplane/policies/chain-management_linux_test.go @@ -1,7 +1,6 @@ package policies import ( - "errors" "fmt" "sort" "strings" @@ -45,8 +44,6 @@ Chain AZURE-NPM-INGRESS (1 references) ` ) -var errKernelVersion = errors.New("kernel error") - func TestStaleChainsForceLock(t *testing.T) { testChains := []string{} for i := 0; i < 100000; i++ { @@ -59,7 +56,6 @@ func TestStaleChainsForceLock(t *testing.T) { ioshim := common.NewMockIOShim(calls) // don't verify calls because there shouldn't be as many commands as we create if forceLock works properly pMgr := NewPolicyManager(ioshim, ipsetConfig) - util.SetIptablesToNft() start := make(chan struct{}, 1) done := make(chan struct{}, 1) @@ -146,7 +142,6 @@ func TestCleanupChainsSuccess(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - util.SetIptablesToNft() pMgr.staleChains.add(testChain1) pMgr.staleChains.add(testChain2) @@ -165,7 +160,6 @@ func TestCleanupChainsFailure(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - util.SetIptablesToNft() pMgr.staleChains.add(testChain1) pMgr.staleChains.add(testChain2) @@ -484,7 +478,6 @@ func TestBootupLinux(t *testing.T) { ioshim := common.NewMockIOShim(tt.calls) defer ioshim.VerifyCalls(t, tt.calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - util.SetIptablesToNft() err := pMgr.bootupAfterDetectAndCleanup() if tt.wantErr { require.Error(t, err) @@ -773,7 +766,6 @@ func TestPositionAzureChainJumpRule(t *testing.T) { PlaceAzureChainFirst: tt.placeAzureChainFirst, } pMgr := NewPolicyManager(ioshim, cfg) - util.SetIptablesToNft() err := pMgr.positionAzureChainJumpRule() if tt.wantErr { @@ -866,7 +858,6 @@ func TestChainLineNumber(t *testing.T) { ioshim := common.NewMockIOShim(tt.calls) defer ioshim.VerifyCalls(t, tt.calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - util.SetIptablesToNft() lineNum, err := pMgr.chainLineNumber(testChainName) if tt.wantErr { @@ -903,10 +894,7 @@ func stringsToMap(items []string) map[string]struct{} { func TestDetectIptablesVersion(t *testing.T) { type args struct { name string - kernelVersion int - kernelVersionErr error calls []testutils.TestCmd - expectedErr bool expectedIptablesVersion string } @@ -919,7 +907,6 @@ func TestDetectIptablesVersion(t *testing.T) { ExitCode: 0, }, }, - expectedErr: false, expectedIptablesVersion: util.IptablesNft, }, { @@ -934,7 +921,6 @@ func TestDetectIptablesVersion(t *testing.T) { ExitCode: 0, }, }, - expectedErr: false, expectedIptablesVersion: util.IptablesNft, }, { @@ -953,36 +939,10 @@ func TestDetectIptablesVersion(t *testing.T) { ExitCode: 0, }, }, - expectedErr: false, expectedIptablesVersion: util.IptablesLegacy, }, { - name: "nft and legacy both fail: kernel version >= 5", - kernelVersion: 5, - calls: []testutils.TestCmd{ - { - Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, - ExitCode: 2, - }, - { - Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"}, - ExitCode: 2, - }, - { - Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, - ExitCode: 2, - }, - { - Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"}, - ExitCode: 2, - }, - }, - expectedErr: false, - expectedIptablesVersion: util.IptablesNft, - }, - { - name: "no kube chains: kernel version < 5", - kernelVersion: 4, + name: "no kube chains: default nft", calls: []testutils.TestCmd{ { Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, @@ -1001,41 +961,35 @@ func TestDetectIptablesVersion(t *testing.T) { ExitCode: 1, }, }, - expectedErr: false, - expectedIptablesVersion: util.IptablesLegacy, + expectedIptablesVersion: util.IptablesNft, }, { - name: "no kube chains: kernel version error", - kernelVersionErr: errKernelVersion, + name: "nft and legacy both fail: default nft", calls: []testutils.TestCmd{ { Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, - ExitCode: 1, + ExitCode: 2, }, { Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"}, - ExitCode: 1, + ExitCode: 2, }, { Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"}, - ExitCode: 1, + ExitCode: 2, }, { Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"}, - ExitCode: 1, + ExitCode: 2, }, }, - expectedErr: true, + expectedIptablesVersion: util.IptablesNft, }, } for _, tt := range tests { tt := tt - if tt.name != "no kube chains: kernel version error" { - continue - } - t.Run(tt.name, func(t *testing.T) { metrics.InitializeAll() @@ -1043,22 +997,14 @@ func TestDetectIptablesVersion(t *testing.T) { ioshim := common.NewMockIOShim(tt.calls) defer ioshim.VerifyCalls(t, tt.calls) cfg := &PolicyManagerCfg{ - debug: true, - debugKernelVersion: tt.kernelVersion, - debugKernelVersionErr: tt.kernelVersionErr, - NodeIP: "6.7.8.9", - PolicyMode: IPSetPolicyMode, - PlaceAzureChainFirst: util.PlaceAzureChainFirst, + NodeIP: "6.7.8.9", + PolicyMode: IPSetPolicyMode, + PlaceAzureChainFirst: util.PlaceAzureChainFirst, } pMgr := NewPolicyManager(ioshim, cfg) - err := pMgr.detectIptablesVersion() + pMgr.detectIptablesVersion() - if tt.expectedErr { - require.Error(t, err) - } else { - require.NoError(t, err) - require.Equal(t, tt.expectedIptablesVersion, util.Iptables) - } + require.Equal(t, tt.expectedIptablesVersion, util.Iptables) }) } } @@ -1268,19 +1214,19 @@ func TestCleanupOtherChains(t *testing.T) { }, { Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM"}, - ExitCode: 1, + ExitCode: 2, }, { Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS"}, - ExitCode: 1, + ExitCode: 2, }, { Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM"}, - ExitCode: 1, + ExitCode: 2, }, { Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS"}, - ExitCode: 1, + ExitCode: 2, }, }, expectedErr: false, @@ -1402,6 +1348,10 @@ func TestCleanupOtherChains(t *testing.T) { util.SetIptablesToNft() } else { util.SetIptablesToLegacy() + // set back to default + defer func() { + util.SetIptablesToNft() + }() } err := pMgr.cleanupOtherIptables() diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index 654b8c87f0..bf185ac64c 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -29,13 +29,6 @@ const ( ) type PolicyManagerCfg struct { - // debug is only used for testing. Currently just indicating whether to use debug kernel version - debug bool //nolint:unused // only used in linux - // debugKernelVersion is only used for testing - debugKernelVersion int //nolint:unused // only used in linux - // debugKernelVersionErr is only used for testing - debugKernelVersionErr error //nolint:unused // only used in linux - // NodeIP is only used in Windows NodeIP string // PolicyMode only affects Windows diff --git a/npm/pkg/dataplane/policies/policymanager_linux_test.go b/npm/pkg/dataplane/policies/policymanager_linux_test.go index c6af8f1127..37f5ca356b 100644 --- a/npm/pkg/dataplane/policies/policymanager_linux_test.go +++ b/npm/pkg/dataplane/policies/policymanager_linux_test.go @@ -212,7 +212,6 @@ func TestAddPolicyFailure(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - util.SetIptablesToNft() require.Error(t, pMgr.AddPolicies([]*NPMNetworkPolicy{testNetPol}, nil)) _, ok := pMgr.GetPolicy(testNetPol.PolicyKey) @@ -225,7 +224,6 @@ func TestCreatorForAddPolicies(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - util.SetIptablesToNft() // 1. test with activation policies := []*NPMNetworkPolicy{allTestNetworkPolicies[0]} @@ -289,7 +287,6 @@ func TestCreatorForRemovePolicies(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - util.SetIptablesToNft() // 1. test without deactivation (i.e. flushing azure chain when removing the last policy) // hack: the cache is empty (and len(cache) != len(allTestNetworkPolicies)), so shouldDeactivate will be false @@ -337,7 +334,6 @@ func TestRemovePoliciesAcceptableError(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - util.SetIptablesToNft() require.NoError(t, pMgr.AddPolicies([]*NPMNetworkPolicy{bothDirectionsNetPol}, epList)) require.NoError(t, pMgr.RemovePolicy(bothDirectionsNetPol.PolicyKey)) _, ok := pMgr.GetPolicy(bothDirectionsNetPol.PolicyKey) @@ -384,7 +380,6 @@ func TestRemovePoliciesError(t *testing.T) { ioshim := common.NewMockIOShim(tt.calls) defer ioshim.VerifyCalls(t, tt.calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - util.SetIptablesToNft() err := pMgr.AddPolicies([]*NPMNetworkPolicy{bothDirectionsNetPol}, nil) require.NoError(t, err) err = pMgr.RemovePolicy(bothDirectionsNetPol.PolicyKey) @@ -407,7 +402,6 @@ func TestUpdatingStaleChains(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - util.SetIptablesToNft() // add so we can remove. no stale chains to start require.NoError(t, pMgr.AddPolicies([]*NPMNetworkPolicy{bothDirectionsNetPol}, nil)) diff --git a/npm/pkg/dataplane/policies/policymanager_test.go b/npm/pkg/dataplane/policies/policymanager_test.go index b7c1ff99f6..e10be413d9 100644 --- a/npm/pkg/dataplane/policies/policymanager_test.go +++ b/npm/pkg/dataplane/policies/policymanager_test.go @@ -105,13 +105,19 @@ func TestBootup(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim, ipsetConfig) - // do NOT set iptables to nft here - // it should be set because of Bootup + + // verify that the iptables is explicitly set to nft during bootup + util.SetIptablesToLegacy() + // set back to default + defer func() { + util.SetIptablesToNft() + }() metrics.IncNumACLRules() metrics.IncNumACLRules() require.NoError(t, pMgr.Bootup(epIDs)) + require.Equal(t, util.IptablesNft, util.Iptables) expectedNumACLs := 11 if util.IsWindowsDP() { diff --git a/npm/util/const.go b/npm/util/const.go index c40821b716..25a954a354 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -2,6 +2,8 @@ // MIT License package util +import "k8s.io/klog" + // kubernetes related constants. const ( KubeSystemFlag string = "kube-system" @@ -20,10 +22,10 @@ const ( ) var ( - Iptables = IptablesLegacy + Iptables = IptablesNft Ip6tables = Ip6tablesLegacy //nolint (avoid warning to capitalize this p) - IptablesSave = IptablesSaveLegacy - IptablesRestore = IptablesRestoreLegacy + IptablesSave = IptablesSaveNft + IptablesRestore = IptablesRestoreNft ) // iptables related constants. @@ -266,12 +268,14 @@ const ( ) func SetIptablesToNft() { + klog.Info("setting iptables to nft") Iptables = IptablesNft IptablesSave = IptablesSaveNft IptablesRestore = IptablesRestoreNft } func SetIptablesToLegacy() { + klog.Info("setting iptables to legacy") Iptables = IptablesLegacy IptablesSave = IptablesSaveLegacy IptablesRestore = IptablesRestoreLegacy diff --git a/npm/util/sysinfo_linux.go b/npm/util/sysinfo_linux.go deleted file mode 100644 index d960d85928..0000000000 --- a/npm/util/sysinfo_linux.go +++ /dev/null @@ -1,44 +0,0 @@ -package util - -// this file has bits copied from github.com/zcalusic/sysinfo (v1.1.2) - -import ( - "errors" - "fmt" - "os" - "strconv" - "strings" -) - -const kernelReleaseFilepath = "/proc/sys/kernel/osrelease" - -var errNoKernelRelease = errors.New("error finding kernel release") - -func KernelReleaseMajorVersion() (int, error) { - rel, err := slurpFile(kernelReleaseFilepath) - if err != nil { - return 0, err - } - - majorString := strings.Split(rel, ".")[0] - if majorString == "" { - return 0, errNoKernelRelease - } - - majorInt, err := strconv.Atoi(majorString) - if err != nil { - return 0, fmt.Errorf("failed to convert kernel major version to int. version: %s. err: %w", majorString, err) - } - - return majorInt, nil -} - -// Read one-liner text files, strip newline. -func slurpFile(path string) (string, error) { - data, err := os.ReadFile(path) - if err != nil { - return "", fmt.Errorf("failed to read file. file: %s. err: %w", path, err) - } - - return strings.TrimSpace(string(data)), nil -} From f7155c68b2310a1239efc7aabf2db74fc872fe4b Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Wed, 6 Nov 2024 11:19:07 -0800 Subject: [PATCH 11/14] style: fix lint Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- npm/pkg/dataplane/policies/chain-management_linux.go | 1 - npm/pkg/dataplane/policies/chain-management_linux_test.go | 4 +--- npm/pkg/dataplane/policies/policymanager_test.go | 4 +--- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index 39d1afddd2..aebc85eec7 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -261,7 +261,6 @@ func (pMgr *PolicyManager) detectIptablesVersion() { // default to nft if nothing is found util.SetIptablesToNft() - return } func (pMgr *PolicyManager) hintOrCanaryChainExist(iptablesCmd string) bool { diff --git a/npm/pkg/dataplane/policies/chain-management_linux_test.go b/npm/pkg/dataplane/policies/chain-management_linux_test.go index 518798bf66..7a1812552d 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux_test.go +++ b/npm/pkg/dataplane/policies/chain-management_linux_test.go @@ -1349,9 +1349,7 @@ func TestCleanupOtherChains(t *testing.T) { } else { util.SetIptablesToLegacy() // set back to default - defer func() { - util.SetIptablesToNft() - }() + defer util.SetIptablesToNft() } err := pMgr.cleanupOtherIptables() diff --git a/npm/pkg/dataplane/policies/policymanager_test.go b/npm/pkg/dataplane/policies/policymanager_test.go index e10be413d9..2ce9b27285 100644 --- a/npm/pkg/dataplane/policies/policymanager_test.go +++ b/npm/pkg/dataplane/policies/policymanager_test.go @@ -109,9 +109,7 @@ func TestBootup(t *testing.T) { // verify that the iptables is explicitly set to nft during bootup util.SetIptablesToLegacy() // set back to default - defer func() { - util.SetIptablesToNft() - }() + defer util.SetIptablesToNft() metrics.IncNumACLRules() metrics.IncNumACLRules() From 7b3e155341004842a9f01d8e9cb249abf66a0834 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Wed, 6 Nov 2024 11:27:45 -0800 Subject: [PATCH 12/14] style: try fixing gci/gofumpt lint Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- npm/pkg/dataplane/policies/chain-management_linux.go | 1 - npm/pkg/dataplane/policies/chain-management_linux_test.go | 1 - 2 files changed, 2 deletions(-) diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index aebc85eec7..3691e8014f 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -13,7 +13,6 @@ import ( "github.com/Azure/azure-container-networking/npm/util" npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" "github.com/Azure/azure-container-networking/npm/util/ioutil" - "k8s.io/klog" utilexec "k8s.io/utils/exec" ) diff --git a/npm/pkg/dataplane/policies/chain-management_linux_test.go b/npm/pkg/dataplane/policies/chain-management_linux_test.go index 7a1812552d..a211e033a1 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux_test.go +++ b/npm/pkg/dataplane/policies/chain-management_linux_test.go @@ -991,7 +991,6 @@ func TestDetectIptablesVersion(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { - metrics.InitializeAll() ioshim := common.NewMockIOShim(tt.calls) From bebe123889a69fd07849844e619c9f0b67641c50 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Wed, 6 Nov 2024 12:32:36 -0800 Subject: [PATCH 13/14] test: fix unit tests referencing iptables legacy Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- npm/iptm/iptm_test.go | 340 +++++++++---------- npm/pkg/dataplane/parse/parser_test.go | 2 +- npm/util/ioutil/current-azure-chains_test.go | 14 +- 3 files changed, 178 insertions(+), 178 deletions(-) diff --git a/npm/iptm/iptm_test.go b/npm/iptm/iptm_test.go index 912be83495..d927363e03 100644 --- a/npm/iptm/iptm_test.go +++ b/npm/iptm/iptm_test.go @@ -13,95 +13,95 @@ import ( var ( initCalls = []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-ACCEPT"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-INGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-EGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-INGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-INGRESS-FROM"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-EGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-EGRESS-TO"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-INGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-EGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-ACCEPT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-INGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-EGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-INGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-INGRESS-FROM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-EGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-EGRESS-TO"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-INGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-EGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM"}}, // NOTE the following grep call stdouts are misleading. The first grep returns 3, and the second one returns "" (i.e. line 0) // a fix is coming for fakeexec stdout and exit code problems from piping commands (e.g. what we do with grep) - {Cmd: []string{"iptables", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "3 "}, // THIS IS THE GREP CALL + {Cmd: []string{"iptables-nft", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "3 "}, // THIS IS THE GREP CALL {Cmd: []string{"grep", "KUBE-SERVICES"}, Stdout: "4 "}, - {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "3 "}, // THIS IS THE GREP CALL + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "3 "}, // THIS IS THE GREP CALL {Cmd: []string{"grep", "AZURE-NPM"}, Stdout: "4 "}, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "3", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-INGRESS"}}, // broken here - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-EGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-ACCEPT", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "ACCEPT-on-INGRESS-and-EGRESS-mark-0x3000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-ACCEPT", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "ACCEPT-on-INGRESS-mark-0x2000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-ACCEPT", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "ACCEPT-on-EGRESS-mark-0x1000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-m", "state", "--state", "RELATED,ESTABLISHED", "-j", "ACCEPT", "-m", "comment", "--comment", "ACCEPT-on-connection-state"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-ACCEPT", "-j", "MARK", "--set-mark", "0x0", "-m", "comment", "--comment", "Clear-AZURE-NPM-MARKS"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-ACCEPT", "-j", "ACCEPT", "-m", "comment", "--comment", "ACCEPT-All-packets"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-INGRESS", "-j", "AZURE-NPM-INGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-INGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-INGRESS", "-j", "AZURE-NPM-INGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-INGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "3", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-INGRESS"}}, // broken here + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-EGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-ACCEPT", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "ACCEPT-on-INGRESS-and-EGRESS-mark-0x3000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-ACCEPT", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "ACCEPT-on-INGRESS-mark-0x2000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-ACCEPT", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "ACCEPT-on-EGRESS-mark-0x1000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM", "-m", "state", "--state", "RELATED,ESTABLISHED", "-j", "ACCEPT", "-m", "comment", "--comment", "ACCEPT-on-connection-state"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-ACCEPT", "-j", "MARK", "--set-mark", "0x0", "-m", "comment", "--comment", "Clear-AZURE-NPM-MARKS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-ACCEPT", "-j", "ACCEPT", "-m", "comment", "--comment", "ACCEPT-All-packets"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS", "-j", "AZURE-NPM-INGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS", "-j", "AZURE-NPM-INGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, // ///////// - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-INGRESS-PORT", "-j", "AZURE-NPM-INGRESS-FROM", "-m", "comment", "--comment", "ALL-JUMP-TO-AZURE-NPM-INGRESS-FROM"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "AZURE-NPM-EGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "AZURE-NPM-EGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "AZURE-NPM-EGRESS-TO", "-m", "comment", "--comment", "ALL-JUMP-TO-AZURE-NPM-EGRESS-TO"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-INGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS-PORT", "-j", "AZURE-NPM-INGRESS-FROM", "-m", "comment", "--comment", "ALL-JUMP-TO-AZURE-NPM-INGRESS-FROM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "AZURE-NPM-EGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "AZURE-NPM-EGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "AZURE-NPM-EGRESS-TO", "-m", "comment", "--comment", "ALL-JUMP-TO-AZURE-NPM-EGRESS-TO"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, } initWithJumpToAzureAtTopCalls = []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-ACCEPT"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-INGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-EGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-INGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-INGRESS-FROM"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-EGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-EGRESS-TO"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-INGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-EGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM"}}, - - {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-INGRESS"}}, // broken here - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-EGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-ACCEPT", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "ACCEPT-on-INGRESS-and-EGRESS-mark-0x3000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-ACCEPT", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "ACCEPT-on-INGRESS-mark-0x2000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-ACCEPT", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "ACCEPT-on-EGRESS-mark-0x1000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-m", "state", "--state", "RELATED,ESTABLISHED", "-j", "ACCEPT", "-m", "comment", "--comment", "ACCEPT-on-connection-state"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-ACCEPT", "-j", "MARK", "--set-mark", "0x0", "-m", "comment", "--comment", "Clear-AZURE-NPM-MARKS"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-ACCEPT", "-j", "ACCEPT", "-m", "comment", "--comment", "ACCEPT-All-packets"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-INGRESS", "-j", "AZURE-NPM-INGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-INGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-INGRESS", "-j", "AZURE-NPM-INGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-INGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-ACCEPT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-INGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-EGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-INGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-INGRESS-FROM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-EGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-EGRESS-TO"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-INGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM-EGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM"}}, + + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-INGRESS"}}, // broken here + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-EGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-ACCEPT", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "ACCEPT-on-INGRESS-and-EGRESS-mark-0x3000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-ACCEPT", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "ACCEPT-on-INGRESS-mark-0x2000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-ACCEPT", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "ACCEPT-on-EGRESS-mark-0x1000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM", "-m", "state", "--state", "RELATED,ESTABLISHED", "-j", "ACCEPT", "-m", "comment", "--comment", "ACCEPT-on-connection-state"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-ACCEPT", "-j", "MARK", "--set-mark", "0x0", "-m", "comment", "--comment", "Clear-AZURE-NPM-MARKS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-ACCEPT", "-j", "ACCEPT", "-m", "comment", "--comment", "ACCEPT-All-packets"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS", "-j", "AZURE-NPM-INGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS", "-j", "AZURE-NPM-INGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, // ///////// - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-INGRESS-PORT", "-j", "AZURE-NPM-INGRESS-FROM", "-m", "comment", "--comment", "ALL-JUMP-TO-AZURE-NPM-INGRESS-FROM"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "AZURE-NPM-EGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "AZURE-NPM-EGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "AZURE-NPM-EGRESS-TO", "-m", "comment", "--comment", "ALL-JUMP-TO-AZURE-NPM-EGRESS-TO"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-INGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS-PORT", "-j", "AZURE-NPM-INGRESS-FROM", "-m", "comment", "--comment", "ALL-JUMP-TO-AZURE-NPM-INGRESS-FROM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "AZURE-NPM-EGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "AZURE-NPM-EGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "AZURE-NPM-EGRESS-TO", "-m", "comment", "--comment", "ALL-JUMP-TO-AZURE-NPM-EGRESS-TO"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, } ) @@ -124,69 +124,69 @@ func TestUninitNpmChains(t *testing.T) { { name: "no v2 npm chains exist", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, {Cmd: []string{"grep", "Chain AZURE-NPM"}, ExitCode: 1}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-ACCEPT"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-EGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS-FROM"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-EGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-EGRESS-TO"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-EGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-TARGET-SETS"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INRGESS-DROPS"}}, // can we remove this rule now? - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-ACCEPT"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-EGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS-FROM"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-EGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-EGRESS-TO"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-EGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-TARGET-SETS"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INRGESS-DROPS"}}, // can we delete this rule now? + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-ACCEPT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-INGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-EGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-INGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-INGRESS-FROM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-EGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-EGRESS-TO"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-INGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-EGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-TARGET-SETS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-INRGESS-DROPS"}}, // can we remove this rule now? + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-ACCEPT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-INGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-EGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-INGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-INGRESS-FROM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-EGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-EGRESS-TO"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-INGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-EGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-TARGET-SETS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-INRGESS-DROPS"}}, // can we delete this rule now? }, }, { name: " v2 exists chian exists", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, {Cmd: []string{"grep", "Chain AZURE-NPM"}, Stdout: "Chain AZURE-NPM-INGRESS-ALLOW-MARK (1 references)\n"}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-ACCEPT"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-EGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS-FROM"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-EGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-EGRESS-TO"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-EGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-TARGET-SETS"}}, - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INRGESS-DROPS"}}, // can we remove this rule now? - {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS-ALLOW-MARK"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-ACCEPT"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-EGRESS"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS-FROM"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-EGRESS-PORT"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-EGRESS-TO"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-EGRESS-DROPS"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-TARGET-SETS"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INRGESS-DROPS"}}, // can we delete this rule now? - {Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS-ALLOW-MARK"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-ACCEPT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-INGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-EGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-INGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-INGRESS-FROM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-EGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-EGRESS-TO"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-INGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-EGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-TARGET-SETS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-INRGESS-DROPS"}}, // can we remove this rule now? + {Cmd: []string{"iptables-nft", "-w", "60", "-F", "AZURE-NPM-INGRESS-ALLOW-MARK"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-ACCEPT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-INGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-EGRESS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-INGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-INGRESS-FROM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-EGRESS-PORT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-EGRESS-TO"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-INGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-EGRESS-DROPS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-TARGET-SETS"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-INRGESS-DROPS"}}, // can we delete this rule now? + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "AZURE-NPM-INGRESS-ALLOW-MARK"}}, }, }, // currently can't test multiple v2 chains existing because AllCurrentAzureChains() returns a map and fexec needs the exact order of commands @@ -228,9 +228,9 @@ func TestCheckAndAddForwardChain(t *testing.T) { name: "add missing jump to azure at top", args: args{ calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, // "rule does not exist" - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, ExitCode: 1}, // "rule does not exist" + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, placeAzureChainFirst: util.PlaceAzureChainFirst, }, @@ -239,11 +239,11 @@ func TestCheckAndAddForwardChain(t *testing.T) { name: "add missing jump to azure after kube services", args: args{ calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "3 "}, // THIS IS THE GREP CALL STDOUT - {Cmd: []string{"grep", "KUBE-SERVICES"}, ExitCode: 1}, // THIS IS THE EXIT CODE FOR CHECK command below ("rule doesn't exist") - {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "4", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "3 "}, // THIS IS THE GREP CALL STDOUT + {Cmd: []string{"grep", "KUBE-SERVICES"}, ExitCode: 1}, // THIS IS THE EXIT CODE FOR CHECK command below ("rule doesn't exist") + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "4", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, }, @@ -252,9 +252,9 @@ func TestCheckAndAddForwardChain(t *testing.T) { name: "jump to azure already at top", args: args{ calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "1 "}, // THIS IS THE GREP CALL STDOUT + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "1 "}, // THIS IS THE GREP CALL STDOUT {Cmd: []string{"grep", "AZURE-NPM"}}, }, placeAzureChainFirst: util.PlaceAzureChainFirst, @@ -264,11 +264,11 @@ func TestCheckAndAddForwardChain(t *testing.T) { name: "jump to azure already after kube services", args: args{ calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "3 "}, // THIS IS THE GREP CALL STDOUT + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "3 "}, // THIS IS THE GREP CALL STDOUT {Cmd: []string{"grep", "KUBE-SERVICES"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, Stdout: "4 "}, // THIS IS THE GREP CALL STDOUT - {Cmd: []string{"iptables", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, Stdout: "4 "}, // THIS IS THE GREP CALL STDOUT + {Cmd: []string{"iptables-nft", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}}, {Cmd: []string{"grep", "AZURE-NPM"}}, }, placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, @@ -278,12 +278,12 @@ func TestCheckAndAddForwardChain(t *testing.T) { name: "move jump to azure to top", args: args{ calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "5 "}, // THIS IS THE GREP CALL STDOUT + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "5 "}, // THIS IS THE GREP CALL STDOUT {Cmd: []string{"grep", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, placeAzureChainFirst: util.PlaceAzureChainFirst, }, @@ -292,14 +292,14 @@ func TestCheckAndAddForwardChain(t *testing.T) { name: "move jump to azure after kube services", args: args{ calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "3 "}, // THIS IS THE GREP CALL STDOUT + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "AZURE-NPM"}}, + {Cmd: []string{"iptables-nft", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, Stdout: "3 "}, // THIS IS THE GREP CALL STDOUT {Cmd: []string{"grep", "KUBE-SERVICES"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, Stdout: "2 "}, // THIS IS THE GREP CALL STDOUT - {Cmd: []string{"iptables", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, Stdout: "2 "}, // THIS IS THE GREP CALL STDOUT + {Cmd: []string{"iptables-nft", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}}, {Cmd: []string{"grep", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "3", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "3", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, }, placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, }, @@ -319,7 +319,7 @@ func TestCheckAndAddForwardChain(t *testing.T) { func TestExists(t *testing.T) { calls := []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "ACCEPT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "FORWARD", "-j", "ACCEPT"}}, } fexec := testutils.GetFakeExecWithScripts(calls) @@ -341,7 +341,7 @@ func TestExists(t *testing.T) { func TestAddChain(t *testing.T) { calls := []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-N", "TEST-CHAIN"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "TEST-CHAIN"}}, } fexec := testutils.GetFakeExecWithScripts(calls) @@ -355,8 +355,8 @@ func TestAddChain(t *testing.T) { func TestDeleteChain(t *testing.T) { calls := []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-N", "TEST-CHAIN"}}, - {Cmd: []string{"iptables", "-w", "60", "-X", "TEST-CHAIN"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "TEST-CHAIN"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-X", "TEST-CHAIN"}}, } fexec := testutils.GetFakeExecWithScripts(calls) @@ -374,7 +374,7 @@ func TestDeleteChain(t *testing.T) { func TestAdd(t *testing.T) { calls := []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "REJECT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "REJECT"}}, } fexec := testutils.GetFakeExecWithScripts(calls) @@ -420,9 +420,9 @@ func testPrometheusMetrics(t *testing.T, expectedNumACLRules, expectedExecCount func TestDelete(t *testing.T) { calls := []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "-j", "REJECT"}}, - {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "REJECT"}}, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "REJECT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-I", "FORWARD", "-j", "REJECT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-C", "FORWARD", "-j", "REJECT"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-D", "FORWARD", "-j", "REJECT"}}, } fexec := testutils.GetFakeExecWithScripts(calls) @@ -450,7 +450,7 @@ func TestDelete(t *testing.T) { func TestRun(t *testing.T) { calls := []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-N", "TEST-CHAIN"}}, + {Cmd: []string{"iptables-nft", "-w", "60", "-N", "TEST-CHAIN"}}, } fexec := testutils.GetFakeExecWithScripts(calls) @@ -508,7 +508,7 @@ func TestGetChainLineNumber(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { calls := []testutils.TestCmd{ - {Cmd: []string{"iptables", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-t", "filter", "-n", "--list", "FORWARD", "--line-numbers"}, PipedToCommand: true}, {Cmd: []string{"grep", "AZURE-NPM"}, Stdout: tt.stdout, ExitCode: tt.exitCode}, } fexec := testutils.GetFakeExecWithScripts(calls) diff --git a/npm/pkg/dataplane/parse/parser_test.go b/npm/pkg/dataplane/parse/parser_test.go index a5d880e644..daa58c2a42 100644 --- a/npm/pkg/dataplane/parse/parser_test.go +++ b/npm/pkg/dataplane/parse/parser_test.go @@ -30,7 +30,7 @@ func TestParseIptablesObjectFileV2(t *testing.T) { func TestParseIptablesObject(t *testing.T) { calls := []testutils.TestCmd{ - {Cmd: []string{"iptables-save", "-t", "filter"}}, + {Cmd: []string{"iptables-nft-save", "-t", "filter"}}, } parser := IPTablesParser{ diff --git a/npm/util/ioutil/current-azure-chains_test.go b/npm/util/ioutil/current-azure-chains_test.go index f2640d7884..107311394b 100644 --- a/npm/util/ioutil/current-azure-chains_test.go +++ b/npm/util/ioutil/current-azure-chains_test.go @@ -17,7 +17,7 @@ Chain AZURE-NPM-INGRESS-123456 (1 references) Chain AZURE-NPM-INGRESS-ALLOW-MARK (1 references) ` -var listAllCommandStrings = []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"} +var listAllCommandStrings = []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"} func TestAllCurrentAzureChains(t *testing.T) { tests := []struct { @@ -41,7 +41,7 @@ func TestAllCurrentAzureChains(t *testing.T) { { name: "ignore missing newline at end of grep result", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, { Cmd: []string{"grep", "Chain AZURE-NPM"}, Stdout: `Chain AZURE-NPM (1 references) @@ -54,7 +54,7 @@ Chain AZURE-NPM-INGRESS (1 references)`, { name: "ignore unexpected grep line (chain name too short)", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, { Cmd: []string{"grep", "Chain AZURE-NPM"}, Stdout: `Chain AZURE-NPM (1 references) @@ -69,7 +69,7 @@ Chain AZURE-NPM-INGRESS (1 references) { name: "ignore unexpected grep line (no space)", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, { Cmd: []string{"grep", "Chain AZURE-NPM"}, Stdout: `Chain AZURE-NPM (1 references) @@ -83,7 +83,7 @@ Chain AZURE-NPM-INGRESS (1 references) { name: "success with no chains", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, {Cmd: []string{"grep", "Chain AZURE-NPM"}, ExitCode: 1}, }, expectedChains: nil, @@ -92,7 +92,7 @@ Chain AZURE-NPM-INGRESS (1 references) { name: "grep failure", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true, HasStartError: true, ExitCode: 1}, + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true, HasStartError: true, ExitCode: 1}, {Cmd: []string{"grep", "Chain AZURE-NPM"}}, }, expectedChains: nil, @@ -101,7 +101,7 @@ Chain AZURE-NPM-INGRESS (1 references) { name: "invalid grep result", calls: []testutils.TestCmd{ - {Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, + {Cmd: []string{"iptables-nft", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true}, { Cmd: []string{"grep", "Chain AZURE-NPM"}, Stdout: "", From 5934dd569eebfa305ce2022cb34e7323a88068c2 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Wed, 6 Nov 2024 12:46:01 -0800 Subject: [PATCH 14/14] style: fix lint in iptm_test.go Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- npm/iptm/iptm_test.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/npm/iptm/iptm_test.go b/npm/iptm/iptm_test.go index d927363e03..630035f86e 100644 --- a/npm/iptm/iptm_test.go +++ b/npm/iptm/iptm_test.go @@ -54,11 +54,15 @@ var ( {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "AZURE-NPM-EGRESS-DROPS"}}, - {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, + {Cmd: []string{ + "iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000", + }}, {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "AZURE-NPM-EGRESS-TO", "-m", "comment", "--comment", "ALL-JUMP-TO-AZURE-NPM-EGRESS-TO"}}, {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, - {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, + {Cmd: []string{ + "iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000", + }}, {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, } @@ -96,11 +100,15 @@ var ( {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS", "-j", "AZURE-NPM-EGRESS-DROPS"}}, - {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, + {Cmd: []string{ + "iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000", + }}, {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-PORT", "-j", "AZURE-NPM-EGRESS-TO", "-m", "comment", "--comment", "ALL-JUMP-TO-AZURE-NPM-EGRESS-TO"}}, {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-INGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x2000", "-m", "comment", "--comment", "RETURN-on-INGRESS-mark-0x2000"}}, - {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000"}}, + {Cmd: []string{ + "iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x3000", "-m", "comment", "--comment", "RETURN-on-EGRESS-and-INGRESS-mark-0x3000", + }}, {Cmd: []string{"iptables-nft", "-w", "60", "-C", "AZURE-NPM-EGRESS-DROPS", "-j", "RETURN", "-m", "mark", "--mark", "0x1000", "-m", "comment", "--comment", "RETURN-on-EGRESS-mark-0x1000"}}, } )