diff --git a/npm/config/config.go b/npm/config/config.go index b7266f3235..5e79a74cca 100644 --- a/npm/config/config.go +++ b/npm/config/config.go @@ -18,6 +18,7 @@ var DefaultConfig = Config{ EnablePprof: true, EnableHTTPDebugAPI: true, EnableV2Controllers: false, + PlaceAzureChainFirst: false, }, } @@ -33,4 +34,5 @@ type Toggles struct { EnablePprof bool EnableHTTPDebugAPI bool EnableV2Controllers bool + PlaceAzureChainFirst bool } diff --git a/npm/iptm/iptm.go b/npm/iptm/iptm.go index c0e366e722..91d072b65b 100644 --- a/npm/iptm/iptm.go +++ b/npm/iptm/iptm.go @@ -36,6 +36,14 @@ var IptablesAzureChainList = []string{ util.IptablesAzureEgressDropsChain, } +var deprecatedJumpToAzureEntry = &IptEntry{ + Chain: util.IptablesForwardChain, + Specs: []string{ + util.IptablesJumpFlag, + util.IptablesAzureChain, + }, +} + // IptEntry represents an iptables rule. type IptEntry struct { Command string @@ -48,9 +56,10 @@ type IptEntry struct { // IptablesManager stores iptables entries. type IptablesManager struct { - exec utilexec.Interface - io ioshim - OperationFlag string + exec utilexec.Interface + io ioshim + OperationFlag string + placeAzureChainFirst bool } func isDropsChain(chainName string) bool { @@ -63,11 +72,12 @@ func isDropsChain(chainName string) bool { } // NewIptablesManager creates a new instance for IptablesManager object. -func NewIptablesManager(exec utilexec.Interface, io ioshim) *IptablesManager { +func NewIptablesManager(exec utilexec.Interface, io ioshim, placeAzureChainFirst bool) *IptablesManager { iptMgr := &IptablesManager{ - exec: exec, - io: io, - OperationFlag: "", + exec: exec, + io: io, + OperationFlag: "", + placeAzureChainFirst: placeAzureChainFirst, } return iptMgr @@ -93,15 +103,25 @@ func (iptMgr *IptablesManager) InitNpmChains() error { // UninitNpmChains uninitializes Azure NPM chains in iptables. func (iptMgr *IptablesManager) UninitNpmChains() error { // Remove AZURE-NPM chain from FORWARD chain. + iptMgr.OperationFlag = util.IptablesDeletionFlag + errCode, err := iptMgr.run(deprecatedJumpToAzureEntry) + if errCode != iptablesErrDoesNotExist && err != nil { + metrics.SendErrorLogAndMetric(util.IptmID, "Error: failed to delete deprecated jump from FORWARD chain to AZURE-NPM") + return err + } + entry := &IptEntry{ Chain: util.IptablesForwardChain, Specs: []string{ util.IptablesJumpFlag, util.IptablesAzureChain, + util.IptablesModuleFlag, + util.IptablesCtstateModuleFlag, + util.IptablesCtstateFlag, + util.IptablesNewState, }, } - iptMgr.OperationFlag = util.IptablesDeletionFlag - errCode, err := iptMgr.run(entry) + errCode, err = iptMgr.run(entry) if errCode != iptablesErrDoesNotExist && err != nil { metrics.SendErrorLogAndMetric(util.IptmID, "Error: failed to delete AZURE-NPM from Forward chain") return err @@ -201,18 +221,26 @@ func (iptMgr *IptablesManager) checkAndAddForwardChain() error { Specs: []string{ util.IptablesJumpFlag, util.IptablesAzureChain, + util.IptablesModuleFlag, + util.IptablesCtstateModuleFlag, + util.IptablesCtstateFlag, + util.IptablesNewState, }, } - // retrieve KUBE-SERVICES index - kubeServicesLine, err := iptMgr.getChainLineNumber(util.IptablesKubeServicesChain, util.IptablesForwardChain) - if err != nil { - metrics.SendErrorLogAndMetric(util.IptmID, "Error: failed to get index of KUBE-SERVICES in FORWARD chain with error: %s", err.Error()) - return err + var index int + var kubeServicesLine int + if !iptMgr.placeAzureChainFirst { + // retrieve KUBE-SERVICES index + var err error + kubeServicesLine, err = iptMgr.getChainLineNumber(util.IptablesKubeServicesChain, util.IptablesForwardChain) + if err != nil { + metrics.SendErrorLogAndMetric(util.IptmID, "Error: failed to get index of KUBE-SERVICES in FORWARD chain with error: %s", err.Error()) + return err + } + index = kubeServicesLine + 1 // insert the jump to AZURE-NPM after the jump to KUBE-SERVICES } - index := kubeServicesLine + 1 - exists, err := iptMgr.exists(entry) if err != nil { return err @@ -221,7 +249,9 @@ func (iptMgr *IptablesManager) checkAndAddForwardChain() error { if !exists { // position Azure-NPM chain after Kube-Forward and Kube-Service chains if it exists iptMgr.OperationFlag = util.IptablesInsertionFlag - entry.Specs = append([]string{strconv.Itoa(index)}, entry.Specs...) + if !iptMgr.placeAzureChainFirst { + entry.Specs = append([]string{strconv.Itoa(index)}, entry.Specs...) + } if _, err = iptMgr.run(entry); err != nil { metrics.SendErrorLogAndMetric(util.IptmID, "Error: failed to add AZURE-NPM chain to FORWARD chain.") return err @@ -236,11 +266,15 @@ func (iptMgr *IptablesManager) checkAndAddForwardChain() error { return err } - // Kube-services line number is less than npm chain line number then all good - if kubeServicesLine < npmChainLine { - return nil - } else if kubeServicesLine <= 0 { - return nil + if iptMgr.placeAzureChainFirst { + if npmChainLine == 1 { + return nil + } + } else { + // Kube-services line number is less than npm chain line number then all good + if kubeServicesLine < npmChainLine || kubeServicesLine <= 0 { + return nil + } } errCode := 0 @@ -253,11 +287,11 @@ func (iptMgr *IptablesManager) checkAndAddForwardChain() error { return err } iptMgr.OperationFlag = util.IptablesInsertionFlag - // Reduce index for deleted AZURE-NPM chain - if index > 1 { + if !iptMgr.placeAzureChainFirst { + // Reduce index for deleted AZURE-NPM chain index-- + entry.Specs = append([]string{strconv.Itoa(index)}, entry.Specs...) } - entry.Specs = append([]string{strconv.Itoa(index)}, entry.Specs...) if errCode, err = iptMgr.run(entry); err != nil { metrics.SendErrorLogAndMetric(util.IptmID, "Error: failed to add AZURE-NPM chain to FORWARD chain with error code %d.", errCode) return err diff --git a/npm/iptm/iptm_test.go b/npm/iptm/iptm_test.go index ffe373e144..fbeb95603a 100644 --- a/npm/iptm/iptm_test.go +++ b/npm/iptm/iptm_test.go @@ -25,14 +25,16 @@ var ( {Cmd: []string{"iptables", "-w", "60", "-N", "AZURE-NPM-EGRESS-DROPS"}}, {Cmd: []string{"iptables", "-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{"grep", "KUBE-SERVICES"}, Stdout: "4 "}, - {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "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: "3 "}, // THIS IS THE GREP CALL {Cmd: []string{"grep", "AZURE-NPM"}, Stdout: "4 "}, - {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}}, - {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "3", "-j", "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", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-INGRESS"}}, // broken here {Cmd: []string{"iptables", "-w", "60", "-C", "AZURE-NPM", "-j", "AZURE-NPM-EGRESS"}}, @@ -46,7 +48,7 @@ var ( {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", "-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"}}, @@ -62,6 +64,7 @@ var ( unInitCalls = []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", "-F", "AZURE-NPM"}}, {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-ACCEPT"}}, {Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS"}}, @@ -87,6 +90,48 @@ var ( {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? } + + 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", "-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"}}, + } ) func TestInitNpmChains(t *testing.T) { @@ -94,7 +139,7 @@ func TestInitNpmChains(t *testing.T) { fexec := testutils.GetFakeExecWithScripts(calls) defer testutils.VerifyCalls(t, fexec, calls) - iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim()) + iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim(), util.PlaceAzureChainAfterKubeServices) err := iptMgr.InitNpmChains() require.NoError(t, err) @@ -105,13 +150,126 @@ func TestUninitNpmChains(t *testing.T) { fexec := testutils.GetFakeExecWithScripts(calls) defer testutils.VerifyCalls(t, fexec, calls) - iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim()) + iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim(), util.PlaceAzureChainAfterKubeServices) if err := iptMgr.UninitNpmChains(); err != nil { t.Errorf("TestUninitNpmChains @ iptMgr.UninitNpmChains") } } +func TestInitWithJumpToAzureAtTop(t *testing.T) { + calls := initWithJumpToAzureAtTopCalls + + fexec := testutils.GetFakeExecWithScripts(calls) + defer testutils.VerifyCalls(t, fexec, calls) + iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim(), util.PlaceAzureChainFirst) + + err := iptMgr.InitNpmChains() + require.NoError(t, err) +} + +func TestCheckAndAddForwardChain(t *testing.T) { + type args struct { + calls []testutils.TestCmd + placeAzureChainFirst bool + } + tests := []struct { + name string + args args + }{ + { + 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"}}, + }, + placeAzureChainFirst: util.PlaceAzureChainFirst, + }, + }, + { + 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"}}, + }, + placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, + }, + }, + { + 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{"grep", "AZURE-NPM"}}, + }, + placeAzureChainFirst: util.PlaceAzureChainFirst, + }, + }, + { + 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{"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{"grep", "AZURE-NPM"}}, + }, + placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, + }, + }, + { + 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{"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"}}, + }, + placeAzureChainFirst: util.PlaceAzureChainFirst, + }, + }, + { + 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{"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{"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"}}, + }, + placeAzureChainFirst: util.PlaceAzureChainAfterKubeServices, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + fexec := testutils.GetFakeExecWithScripts(tt.args.calls) + defer testutils.VerifyCalls(t, fexec, tt.args.calls) + iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim(), tt.args.placeAzureChainFirst) + err := iptMgr.checkAndAddForwardChain() + require.NoError(t, err) + }) + } +} + func TestExists(t *testing.T) { calls := []testutils.TestCmd{ {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "ACCEPT"}}, @@ -119,7 +277,7 @@ func TestExists(t *testing.T) { fexec := testutils.GetFakeExecWithScripts(calls) defer testutils.VerifyCalls(t, fexec, calls) - iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim()) + iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim(), util.PlaceAzureChainAfterKubeServices) iptMgr.OperationFlag = util.IptablesCheckFlag entry := &IptEntry{ @@ -141,7 +299,7 @@ func TestAddChain(t *testing.T) { fexec := testutils.GetFakeExecWithScripts(calls) defer testutils.VerifyCalls(t, fexec, calls) - iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim()) + iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim(), util.PlaceAzureChainAfterKubeServices) if err := iptMgr.addChain("TEST-CHAIN"); err != nil { t.Errorf("TestAddChain failed @ iptMgr.addChain") @@ -156,7 +314,7 @@ func TestDeleteChain(t *testing.T) { fexec := testutils.GetFakeExecWithScripts(calls) defer testutils.VerifyCalls(t, fexec, calls) - iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim()) + iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim(), util.PlaceAzureChainAfterKubeServices) if err := iptMgr.addChain("TEST-CHAIN"); err != nil { t.Errorf("TestDeleteChain failed @ iptMgr.addChain") @@ -174,7 +332,7 @@ func TestAdd(t *testing.T) { fexec := testutils.GetFakeExecWithScripts(calls) defer testutils.VerifyCalls(t, fexec, calls) - iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim()) + iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim(), util.PlaceAzureChainAfterKubeServices) execCount := resetPrometheusAndGetExecCount(t) defer testPrometheusMetrics(t, 1, execCount+1) @@ -222,7 +380,7 @@ func TestDelete(t *testing.T) { fexec := testutils.GetFakeExecWithScripts(calls) defer testutils.VerifyCalls(t, fexec, calls) - iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim()) + iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim(), util.PlaceAzureChainAfterKubeServices) execCount := resetPrometheusAndGetExecCount(t) defer testPrometheusMetrics(t, 0, execCount+1) @@ -250,7 +408,7 @@ func TestRun(t *testing.T) { fexec := testutils.GetFakeExecWithScripts(calls) defer testutils.VerifyCalls(t, fexec, calls) - iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim()) + iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim(), util.PlaceAzureChainAfterKubeServices) iptMgr.OperationFlag = util.IptablesChainCreationFlag entry := &IptEntry{ @@ -269,7 +427,7 @@ func TestGetChainLineNumber(t *testing.T) { fexec := testutils.GetFakeExecWithScripts(calls) defer testutils.VerifyCalls(t, fexec, calls) - iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim()) + iptMgr := NewIptablesManager(fexec, NewFakeIptOperationShim(), util.PlaceAzureChainAfterKubeServices) lineNum, err := iptMgr.getChainLineNumber(util.IptablesAzureChain, util.IptablesForwardChain) require.NoError(t, err) diff --git a/npm/npm.go b/npm/npm.go index 7404be5bd5..9230c04657 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -116,7 +116,7 @@ func NewNetworkPolicyManager(config npmconfig.Config, // create NameSpace controller npMgr.namespaceControllerV1 = controllersv1.NewNameSpaceController(npMgr.nsInformer, npMgr.ipsMgr, npMgr.npmNamespaceCacheV1) // create network policy controller - npMgr.netPolControllerV1 = controllersv1.NewNetworkPolicyController(npMgr.npInformer, npMgr.ipsMgr) + npMgr.netPolControllerV1 = controllersv1.NewNetworkPolicyController(npMgr.npInformer, npMgr.ipsMgr, config.Toggles.PlaceAzureChainFirst) return npMgr } diff --git a/npm/npm_v1_test.go b/npm/npm_v1_test.go index 230bfdf8fc..d3b94e5acd 100644 --- a/npm/npm_v1_test.go +++ b/npm/npm_v1_test.go @@ -6,6 +6,7 @@ import ( "reflect" "testing" + npmconfig "github.com/Azure/azure-container-networking/npm/config" "github.com/Azure/azure-container-networking/npm/ipsm" "github.com/Azure/azure-container-networking/npm/iptm" "github.com/Azure/azure-container-networking/npm/metrics" @@ -80,7 +81,7 @@ func TestMarshalUnMarshalJSON(t *testing.T) { func TestMain(m *testing.M) { metrics.InitializeAll() exec := exec.New() - iptMgr := iptm.NewIptablesManager(exec, iptm.NewFakeIptOperationShim()) + iptMgr := iptm.NewIptablesManager(exec, iptm.NewFakeIptOperationShim(), npmconfig.DefaultConfig.Toggles.PlaceAzureChainFirst) iptMgr.UninitNpmChains() ipsMgr := ipsm.NewIpsetManager(exec) diff --git a/npm/pkg/controlplane/controllers/v1/controllers_v1_test.go b/npm/pkg/controlplane/controllers/v1/controllers_v1_test.go index dab79956cb..1f0ed392e7 100644 --- a/npm/pkg/controlplane/controllers/v1/controllers_v1_test.go +++ b/npm/pkg/controlplane/controllers/v1/controllers_v1_test.go @@ -5,6 +5,7 @@ import ( "os" "testing" + npmconfig "github.com/Azure/azure-container-networking/npm/config" "github.com/Azure/azure-container-networking/npm/ipsm" "github.com/Azure/azure-container-networking/npm/iptm" "github.com/Azure/azure-container-networking/npm/metrics" @@ -14,7 +15,7 @@ import ( func TestMain(m *testing.M) { metrics.InitializeAll() realexec := exec.New() - iptMgr := iptm.NewIptablesManager(realexec, iptm.NewFakeIptOperationShim()) + iptMgr := iptm.NewIptablesManager(realexec, iptm.NewFakeIptOperationShim(), npmconfig.DefaultConfig.Toggles.PlaceAzureChainFirst) err := iptMgr.UninitNpmChains() if err != nil { fmt.Println("uninitnpmchains failed with %w", err) diff --git a/npm/pkg/controlplane/controllers/v1/networkPolicyController.go b/npm/pkg/controlplane/controllers/v1/networkPolicyController.go index b42d1f9085..fb8856a7c4 100644 --- a/npm/pkg/controlplane/controllers/v1/networkPolicyController.go +++ b/npm/pkg/controlplane/controllers/v1/networkPolicyController.go @@ -43,7 +43,7 @@ type NetworkPolicyController struct { iptMgr *iptm.IptablesManager } -func NewNetworkPolicyController(npInformer networkinginformers.NetworkPolicyInformer, ipsMgr *ipsm.IpsetManager) *NetworkPolicyController { +func NewNetworkPolicyController(npInformer networkinginformers.NetworkPolicyInformer, ipsMgr *ipsm.IpsetManager, placeAzureChainFirst bool) *NetworkPolicyController { netPolController := &NetworkPolicyController{ netPolLister: npInformer.Lister(), workqueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "NetworkPolicy"), @@ -51,7 +51,7 @@ func NewNetworkPolicyController(npInformer networkinginformers.NetworkPolicyInfo // ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), isAzureNpmChainCreated: false, ipsMgr: ipsMgr, - iptMgr: iptm.NewIptablesManager(exec.New(), iptm.NewIptOperationShim()), + iptMgr: iptm.NewIptablesManager(exec.New(), iptm.NewIptOperationShim(), placeAzureChainFirst), } npInformer.Informer().AddEventHandler( diff --git a/npm/pkg/controlplane/controllers/v1/networkPolicyController_test.go b/npm/pkg/controlplane/controllers/v1/networkPolicyController_test.go index ccf7daf491..683700841c 100644 --- a/npm/pkg/controlplane/controllers/v1/networkPolicyController_test.go +++ b/npm/pkg/controlplane/controllers/v1/networkPolicyController_test.go @@ -10,6 +10,7 @@ import ( "github.com/Azure/azure-container-networking/npm/ipsm" "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/metrics/promutil" + "github.com/Azure/azure-container-networking/npm/util" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -45,7 +46,6 @@ func newNetPolFixture(t *testing.T, utilexec exec.Interface) *netPolFixture { netPolLister: []*networkingv1.NetworkPolicy{}, kubeobjects: []runtime.Object{}, ipsMgr: ipsm.NewIpsetManager(utilexec), - // iptMgr: iptm.NewIptablesManager(utilexec, iptm.NewFakeIptOperationShim()), } return f } @@ -54,7 +54,7 @@ func (f *netPolFixture) newNetPolController(stopCh chan struct{}) { kubeclient := k8sfake.NewSimpleClientset(f.kubeobjects...) f.kubeInformer = kubeinformers.NewSharedInformerFactory(kubeclient, noResyncPeriodFunc()) - f.netPolController = NewNetworkPolicyController(f.kubeInformer.Networking().V1().NetworkPolicies(), f.ipsMgr) + f.netPolController = NewNetworkPolicyController(f.kubeInformer.Networking().V1().NetworkPolicies(), f.ipsMgr, util.PlaceAzureChainAfterKubeServices) for _, netPol := range f.netPolLister { f.kubeInformer.Networking().V1().NetworkPolicies().Informer().GetIndexer().Add(netPol) diff --git a/npm/util/const.go b/npm/util/const.go index 2ac8af8e90..1f76af59d5 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -21,6 +21,9 @@ const ( // iptables related constants. const ( + PlaceAzureChainAfterKubeServices = false + PlaceAzureChainFirst = true + Iptables string = "iptables" Ip6tables string = "ip6tables" //nolint (avoid warning to capitalize this p) IptablesSave string = "iptables-save"