diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 167a6f8ba6..3354ba4a93 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -29,8 +29,6 @@ var ( IPSetMode: ipsets.ApplyAllIPSets, NetworkName: AzureNetworkName, } - // ErrResetDataPlane error while resetting dataplane - ErrResetDataPlane = fmt.Errorf("Failed to reset dataplane") ) type DataPlane struct { @@ -89,16 +87,26 @@ func (dp *DataPlane) InitializeDataPlane() error { // Create Kube-All-NS IPSet kubeAllSet := ipsets.NewIPSetMetadata(util.KubeAllNamespacesFlag, ipsets.KeyLabelOfNamespace) dp.CreateIPSets([]*ipsets.IPSetMetadata{kubeAllSet}) - return dp.initializeDataPlane() + if err := dp.initializeDataPlane(); err != nil { + return npmerrors.ErrorWrapper(npmerrors.InitializeDataPlane, false, "failed to initialize overall dataplane", err) + } + // TODO update when piped error is fixed in fexec + // if err := dp.policyMgr.Initialize(); err != nil { + // return npmerrors.ErrorWrapper(npmerrors.InitializeDataPlane, false, "failed to initialize policy dataplane", err) + // } + return nil } // ResetDataPlane helps in cleaning up dataplane sets and policies programmed // by NPM, retunring a clean slate func (dp *DataPlane) ResetDataPlane() error { - err := dp.ipsetMgr.ResetIPSets() - if err != nil { - return ErrResetDataPlane + if err := dp.ipsetMgr.ResetIPSets(); err != nil { + return npmerrors.ErrorWrapper(npmerrors.ResetDataPlane, false, "failed to reset ipsets dataplane", err) } + // TODO update when piped error is fixed in fexec + // if err := dp.policyMgr.Reset(); err != nil { + // return npmerrors.ErrorWrapper(npmerrors.ResetDataPlane, false, "failed to reset policy dataplane", err) + // } return dp.resetDataPlane() } @@ -280,12 +288,12 @@ func (dp *DataPlane) UpdatePolicy(policy *policies.NPMNetworkPolicy) error { // and remove/apply only the delta of IPSets and policies // Taking the easy route here, delete existing policy - err := dp.policyMgr.RemovePolicy(policy.Name, nil) + err := dp.RemovePolicy(policy.Name) if err != nil { return fmt.Errorf("[DataPlane] error while updating policy: %w", err) } // and add the new updated policy - err = dp.policyMgr.AddPolicy(policy, nil) + err = dp.AddPolicy(policy) if err != nil { return fmt.Errorf("[DataPlane] error while updating policy: %w", err) } diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index 0e2862c959..5650a7f700 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -1,6 +1,7 @@ package dataplane import ( + "fmt" "testing" "github.com/Azure/azure-container-networking/common" @@ -20,12 +21,10 @@ var ( ExitCode: 0, } - emptyMockIOShim = common.NewMockIOShim([]testutils.TestCmd{}) - setPodKey1 = &ipsets.TranslatedIPSet{ Metadata: ipsets.NewIPSetMetadata("setpodkey1", ipsets.KeyLabelOfPod), } - testPolicyobj = &policies.NPMNetworkPolicy{ + testPolicyobj = policies.NPMNetworkPolicy{ Name: "ns1/testpolicy", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ { @@ -68,7 +67,9 @@ var ( func TestNewDataPlane(t *testing.T) { metrics.InitializeAll() - dp, err := NewDataPlane("testnode", emptyMockIOShim) + + calls := getNewDataplaneTestCalls() + dp, err := NewDataPlane("testnode", common.NewMockIOShim(calls)) require.NoError(t, err) if dp == nil { @@ -81,7 +82,9 @@ func TestNewDataPlane(t *testing.T) { func TestInitializeDataPlane(t *testing.T) { metrics.InitializeAll() - dp, err := NewDataPlane("testnode", emptyMockIOShim) + + calls := append(getNewDataplaneTestCalls(), policies.GetInitializeTestCalls()...) + dp, err := NewDataPlane("testnode", common.NewMockIOShim(calls)) require.NoError(t, err) assert.NotNil(t, dp) @@ -91,7 +94,10 @@ func TestInitializeDataPlane(t *testing.T) { func TestResetDataPlane(t *testing.T) { metrics.InitializeAll() - dp, err := NewDataPlane("testnode", emptyMockIOShim) + + calls := append(getNewDataplaneTestCalls(), getInitializeTestCalls()...) + calls = append(calls, getResetTestCalls()...) + dp, err := NewDataPlane("testnode", common.NewMockIOShim(calls)) require.NoError(t, err) assert.NotNil(t, dp) @@ -103,7 +109,9 @@ func TestResetDataPlane(t *testing.T) { func TestCreateAndDeleteIpSets(t *testing.T) { metrics.InitializeAll() - dp, err := NewDataPlane("testnode", emptyMockIOShim) + + calls := getNewDataplaneTestCalls() + dp, err := NewDataPlane("testnode", common.NewMockIOShim(calls)) require.NoError(t, err) assert.NotNil(t, dp) setsTocreate := []*ipsets.IPSetMetadata{ @@ -141,7 +149,9 @@ func TestCreateAndDeleteIpSets(t *testing.T) { func TestAddToSet(t *testing.T) { metrics.InitializeAll() - dp, err := NewDataPlane("testnode", emptyMockIOShim) + + calls := getNewDataplaneTestCalls() + dp, err := NewDataPlane("testnode", common.NewMockIOShim(calls)) require.NoError(t, err) setsTocreate := []*ipsets.IPSetMetadata{ @@ -201,23 +211,26 @@ func TestAddToSet(t *testing.T) { func TestApplyPolicy(t *testing.T) { metrics.InitializeAll() - calls := []testutils.TestCmd{fakeIPSetRestoreSuccess} + + calls := append(getNewDataplaneTestCalls(), getAddPolicyTestCallsForDP(&testPolicyobj)...) ioShim := common.NewMockIOShim(calls) dp, err := NewDataPlane("testnode", ioShim) require.NoError(t, err) - err = dp.AddPolicy(testPolicyobj) + err = dp.AddPolicy(&testPolicyobj) require.NoError(t, err) } func TestRemovePolicy(t *testing.T) { metrics.InitializeAll() - calls := []testutils.TestCmd{fakeIPSetRestoreSuccess, fakeIPSetRestoreSuccess} + + calls := append(getNewDataplaneTestCalls(), getAddPolicyTestCallsForDP(&testPolicyobj)...) + calls = append(calls, getRemovePolicyTestCallsForDP(&testPolicyobj)...) ioShim := common.NewMockIOShim(calls) dp, err := NewDataPlane("testnode", ioShim) require.NoError(t, err) - err = dp.AddPolicy(testPolicyobj) + err = dp.AddPolicy(&testPolicyobj) require.NoError(t, err) err = dp.RemovePolicy(testPolicyobj.Name) @@ -226,15 +239,9 @@ func TestRemovePolicy(t *testing.T) { func TestUpdatePolicy(t *testing.T) { metrics.InitializeAll() - calls := []testutils.TestCmd{fakeIPSetRestoreSuccess, fakeIPSetRestoreSuccess} - ioShim := common.NewMockIOShim(calls) - dp, err := NewDataPlane("testnode", ioShim) - require.NoError(t, err) - - err = dp.AddPolicy(testPolicyobj) - require.NoError(t, err) - testPolicyobj.ACLs = []*policies.ACLPolicy{ + updatedTestPolicyobj := testPolicyobj + updatedTestPolicyobj.ACLs = []*policies.ACLPolicy{ { PolicyID: "testpol1", Target: policies.Dropped, @@ -242,6 +249,61 @@ func TestUpdatePolicy(t *testing.T) { }, } - err = dp.UpdatePolicy(testPolicyobj) + calls := append(getNewDataplaneTestCalls(), getAddPolicyTestCallsForDP(&testPolicyobj)...) + calls = append(calls, getRemovePolicyTestCallsForDP(&testPolicyobj)...) + calls = append(calls, getAddPolicyTestCallsForDP(&updatedTestPolicyobj)...) + for _, call := range calls { + fmt.Println(call) + } + ioShim := common.NewMockIOShim(calls) + dp, err := NewDataPlane("testnode", ioShim) + require.NoError(t, err) + + err = dp.AddPolicy(&testPolicyobj) + require.NoError(t, err) + + err = dp.UpdatePolicy(&updatedTestPolicyobj) require.NoError(t, err) } + +func getNewDataplaneTestCalls() []testutils.TestCmd { + return append(getResetTestCalls(), getInitializeTestCalls()...) +} + +func getInitializeTestCalls() []testutils.TestCmd { + return []testutils.TestCmd{} + // TODO update when piped error is fixed in fexec + // return policies.GetInitializeTestCalls() +} + +func getResetTestCalls() []testutils.TestCmd { + return ipsets.GetResetTestCalls() + // TODO update when piped error is fixed in fexec + // return append(ipsets.GetResetTestCalls(), policies.GetResetTestCalls()...) +} + +func getAddPolicyTestCallsForDP(networkPolicy *policies.NPMNetworkPolicy) []testutils.TestCmd { + toAddOrUpdateSets := getAffectedIPSets(networkPolicy) + calls := ipsets.GetApplyIPSetsTestCalls(toAddOrUpdateSets, nil) + calls = append(calls, policies.GetAddPolicyTestCalls(networkPolicy)...) + return calls +} + +func getRemovePolicyTestCallsForDP(networkPolicy *policies.NPMNetworkPolicy) []testutils.TestCmd { + // NOTE toDeleteSets is only correct if these ipsets are referenced by no other policy in iMgr + toDeleteSets := getAffectedIPSets(networkPolicy) + calls := policies.GetRemovePolicyTestCalls(networkPolicy) + calls = append(calls, ipsets.GetApplyIPSetsTestCalls(nil, toDeleteSets)...) + return calls +} + +func getAffectedIPSets(networkPolicy *policies.NPMNetworkPolicy) []*ipsets.IPSetMetadata { + sets := make([]*ipsets.IPSetMetadata, 0) + for _, translatedIPSet := range networkPolicy.PodSelectorIPSets { + sets = append(sets, translatedIPSet.Metadata) + } + for _, translatedIPSet := range networkPolicy.RuleIPSets { + sets = append(sets, translatedIPSet.Metadata) + } + return sets +} diff --git a/npm/pkg/dataplane/ioutil/file-creator.go b/npm/pkg/dataplane/ioutil/file-creator.go index b47d1d80c3..0ef78c9ddc 100644 --- a/npm/pkg/dataplane/ioutil/file-creator.go +++ b/npm/pkg/dataplane/ioutil/file-creator.go @@ -29,9 +29,9 @@ type FileCreator struct { ioShim *common.IOShim } -// TODO for iptables: -// lineFailurePattern := "line (\\d+) failed" -// AND "Error occurred at line: (\\d+)" +// TODO ideas: +// - section to error handler(s) map for addLine +// - error handlers have the kind of line error pattern as a requirement // Line defines the content, section, and error handlers for a line type Line struct { diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index b0c70f6816..6a42f656af 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -3,6 +3,7 @@ package ipsets import ( "errors" "fmt" + "reflect" "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/npm/util" @@ -387,3 +388,7 @@ func (set *IPSet) canSetBeSelectorIPSet() bool { set.Type == Namespace || set.Type == NestedLabelOfPod) } + +func (ipset *TranslatedIPSet) Equals(otherIPSet *TranslatedIPSet) bool { + return reflect.DeepEqual(ipset, otherIPSet) +} diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go index fbe64f9e14..dcdb92778e 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go @@ -8,7 +8,7 @@ import ( "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ioutil" - "github.com/Azure/azure-container-networking/npm/util" + dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" testutils "github.com/Azure/azure-container-networking/test/utils" "github.com/stretchr/testify/require" ) @@ -19,12 +19,7 @@ var ( NetworkName: "", } - ipsetRestoreStringSlice = []string{util.Ipset, util.IpsetRestoreFlag} - fakeRestoreSuccessCommand = testutils.TestCmd{ - Cmd: ipsetRestoreStringSlice, - Stdout: "success", - ExitCode: 0, - } + ipsetRestoreStringSlice = []string{"ipset", "restore"} ) func TestDestroyNPMIPSets(t *testing.T) { @@ -109,8 +104,8 @@ func TestApplyCreationsAndAdds(t *testing.T) { creator := iMgr.getFileCreator(1, nil, toAddOrUpdateSetNames) actualFileString := getSortedFileString(creator) - assertEqualFileStrings(t, expectedFileString, actualFileString) - wasFileAltered, err := creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag) + dptestutils.AssertEqualMultilineStrings(t, expectedFileString, actualFileString) + wasFileAltered, err := creator.RunCommandOnceWithFile("ipset", "restore") require.NoError(t, err) require.False(t, wasFileAltered) } @@ -154,8 +149,8 @@ func TestApplyDeletions(t *testing.T) { lines = append(lines, getSortedLines(TestKeyNSList, TestNSSet.HashedName)...) expectedFileString := strings.Join(lines, "\n") + "\n" - assertEqualFileStrings(t, expectedFileString, actualFileString) - wasFileAltered, err := creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag) + dptestutils.AssertEqualMultilineStrings(t, expectedFileString, actualFileString) + wasFileAltered, err := creator.RunCommandOnceWithFile("ipset", "restore") require.NoError(t, err) require.False(t, wasFileAltered) } @@ -183,7 +178,7 @@ func TestFailureOnCreation(t *testing.T) { toDeleteSetNames := []string{TestCIDRSet.PrefixName} assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) creator := iMgr.getFileCreator(2, toDeleteSetNames, toAddOrUpdateSetNames) - wasFileAltered, err := creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag) + wasFileAltered, err := creator.RunCommandOnceWithFile("ipset", "restore") require.Error(t, err) require.True(t, wasFileAltered) @@ -196,8 +191,8 @@ func TestFailureOnCreation(t *testing.T) { expectedFileString := strings.Join(lines, "\n") + "\n" actualFileString := getSortedFileString(creator) - assertEqualFileStrings(t, expectedFileString, actualFileString) - wasFileAltered, err = creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag) + dptestutils.AssertEqualMultilineStrings(t, expectedFileString, actualFileString) + wasFileAltered, err = creator.RunCommandOnceWithFile("ipset", "restore") require.NoError(t, err) require.False(t, wasFileAltered) } @@ -234,7 +229,7 @@ func TestFailureOnAddToList(t *testing.T) { assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) creator := iMgr.getFileCreator(2, toDeleteSetNames, toAddOrUpdateSetNames) originalFileString := creator.ToString() - wasFileAltered, err := creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag) + wasFileAltered, err := creator.RunCommandOnceWithFile("ipset", "restore") require.Error(t, err) require.True(t, wasFileAltered) @@ -260,8 +255,8 @@ func TestFailureOnAddToList(t *testing.T) { expectedFileString = strings.ReplaceAll(expectedFileString, badLine+"\n", "") actualFileString := getSortedFileString(creator) - assertEqualFileStrings(t, expectedFileString, actualFileString) - wasFileAltered, err = creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag) + dptestutils.AssertEqualMultilineStrings(t, expectedFileString, actualFileString) + wasFileAltered, err = creator.RunCommandOnceWithFile("ipset", "restore") require.NoError(t, err) require.False(t, wasFileAltered) } @@ -289,7 +284,7 @@ func TestFailureOnFlush(t *testing.T) { toDeleteSetNames := []string{TestKVPodSet.PrefixName, TestCIDRSet.PrefixName} assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) creator := iMgr.getFileCreator(2, toDeleteSetNames, toAddOrUpdateSetNames) - wasFileAltered, err := creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag) + wasFileAltered, err := creator.RunCommandOnceWithFile("ipset", "restore") require.Error(t, err) require.True(t, wasFileAltered) @@ -302,8 +297,8 @@ func TestFailureOnFlush(t *testing.T) { expectedFileString := strings.Join(lines, "\n") + "\n" actualFileString := getSortedFileString(creator) - assertEqualFileStrings(t, expectedFileString, actualFileString) - wasFileAltered, err = creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag) + dptestutils.AssertEqualMultilineStrings(t, expectedFileString, actualFileString) + wasFileAltered, err = creator.RunCommandOnceWithFile("ipset", "restore") require.NoError(t, err) require.False(t, wasFileAltered) } @@ -330,7 +325,7 @@ func TestFailureOnDeletion(t *testing.T) { toDeleteSetNames := []string{TestKVPodSet.PrefixName, TestCIDRSet.PrefixName} assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) creator := iMgr.getFileCreator(2, toDeleteSetNames, toAddOrUpdateSetNames) - wasFileAltered, err := creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag) + wasFileAltered, err := creator.RunCommandOnceWithFile("ipset", "restore") require.Error(t, err) require.True(t, wasFileAltered) @@ -344,8 +339,8 @@ func TestFailureOnDeletion(t *testing.T) { expectedFileString := strings.Join(lines, "\n") + "\n" actualFileString := getSortedFileString(creator) - assertEqualFileStrings(t, expectedFileString, actualFileString) - wasFileAltered, err = creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag) + dptestutils.AssertEqualMultilineStrings(t, expectedFileString, actualFileString) + wasFileAltered, err = creator.RunCommandOnceWithFile("ipset", "restore") require.NoError(t, err) require.False(t, wasFileAltered) } @@ -402,18 +397,3 @@ func getSortedFileString(creator *ioutil.FileCreator) string { func isAddLine(line string) bool { return len(line) >= 2 && line[:2] == "-A" } - -func assertEqualFileStrings(t *testing.T, expectedFileString, actualFileString string) { - if expectedFileString == actualFileString { - return - } - fmt.Println("EXPECTED FILE STRING:") - for _, line := range strings.Split(expectedFileString, "\n") { - fmt.Println(line) - } - fmt.Println("ACTUAL FILE STRING") - for _, line := range strings.Split(actualFileString, "\n") { - fmt.Println(line) - } - require.FailNow(t, "got unexpected file string (see print contents above)") -} diff --git a/npm/pkg/dataplane/ipsets/testutils_test.go b/npm/pkg/dataplane/ipsets/testutils.go similarity index 100% rename from npm/pkg/dataplane/ipsets/testutils_test.go rename to npm/pkg/dataplane/ipsets/testutils.go diff --git a/npm/pkg/dataplane/ipsets/testutils_linux.go b/npm/pkg/dataplane/ipsets/testutils_linux.go new file mode 100644 index 0000000000..f1bb187c33 --- /dev/null +++ b/npm/pkg/dataplane/ipsets/testutils_linux.go @@ -0,0 +1,18 @@ +package ipsets + +import testutils "github.com/Azure/azure-container-networking/test/utils" + +var fakeRestoreSuccessCommand = testutils.TestCmd{ + Cmd: []string{"ipset", "restore"}, + Stdout: "success", + ExitCode: 0, +} + +func GetApplyIPSetsTestCalls(toAddOrUpdateIPSets, toDeleteIPSets []*IPSetMetadata) []testutils.TestCmd { + // TODO eventually call ipset save if there are toAddOrUpdateIPSets + return []testutils.TestCmd{fakeRestoreSuccessCommand} +} + +func GetResetTestCalls() []testutils.TestCmd { + return []testutils.TestCmd{} +} diff --git a/npm/pkg/dataplane/ipsets/testutils_windows.go b/npm/pkg/dataplane/ipsets/testutils_windows.go new file mode 100644 index 0000000000..2a97b83d2f --- /dev/null +++ b/npm/pkg/dataplane/ipsets/testutils_windows.go @@ -0,0 +1,11 @@ +package ipsets + +import testutils "github.com/Azure/azure-container-networking/test/utils" + +func GetApplyIPSetsTestCalls(_, _ []*IPSetMetadata) []testutils.TestCmd { + return []testutils.TestCmd{} +} + +func GetResetTestCalls() []testutils.TestCmd { + return []testutils.TestCmd{} +} diff --git a/npm/pkg/dataplane/parse/parser.go b/npm/pkg/dataplane/parse/parser.go index c4a9b83d5d..8bdc54ccdc 100644 --- a/npm/pkg/dataplane/parse/parser.go +++ b/npm/pkg/dataplane/parse/parser.go @@ -145,8 +145,8 @@ func Line(readIndex int, iptableBuffer []byte) ([]byte, int) { return iptableBuffer[leftLineIndex : lastNonWhiteSpaceIndex+1], curReadIndex } -// parseChainNameFromRuleLine gets the chain name from given rule line. -func parseChainNameFromRuleLine(ruleLine []byte) (string, int) { +// parseChainNameFromRuleLine gets the chain name from given rule line. +func parseChainNameFromRuleLine(ruleLine []byte) (chainName string, ruleReadIndex int) { spaceIndex := bytes.Index(ruleLine, SpaceBytes) if spaceIndex == -1 { panic(fmt.Sprintf("Unexpected chain line in iptables-save output: %v", string(ruleLine))) @@ -157,7 +157,10 @@ func parseChainNameFromRuleLine(ruleLine []byte) (string, int) { panic(fmt.Sprintf("Unexpected chain line in iptables-save output: %v", string(ruleLine))) } chainNameEnd := chainNameStart + spaceIndex - return string(ruleLine[chainNameStart:chainNameEnd]), chainNameEnd + 1 + + chainName = string(ruleLine[chainNameStart:chainNameEnd]) + ruleReadIndex = chainNameEnd + 1 + return } // parseRuleFromLine creates an iptable rule object from rule line with chain name excluded from the byte array. @@ -237,10 +240,9 @@ func parseTarget(nextIndex int, target *NPMIPtable.Target, ruleLine []byte) int return parseTargetOptionAndValue(nextIndex+spaceIndex+1, target, "", ruleLine) } -func parseTargetOptionAndValue(nextIndex int, target *NPMIPtable.Target, curOption string, ruleLine []byte) int { +func parseTargetOptionAndValue(nextIndex int, target *NPMIPtable.Target, currentOption string, ruleLine []byte) int { spaceIndex := bytes.Index(ruleLine[nextIndex:], SpaceBytes) - currentOption := curOption - if spaceIndex == -1 { + if spaceIndex == -1 { // no more spaces if currentOption == "" { panic(fmt.Sprintf("Rule's value have no preceded option: %v", string(ruleLine))) } @@ -261,7 +263,7 @@ func parseTargetOptionAndValue(nextIndex int, target *NPMIPtable.Target, curOpti // recursively parsing options and their value until a new flag is encounter return parseTargetOptionAndValue(nextIndex, target, currentOption, ruleLine) } - // this is a new flag + // this is a new flag, so stop adding option-values to the target return nextIndex } } diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go new file mode 100644 index 0000000000..ee4f80a411 --- /dev/null +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -0,0 +1,397 @@ +package policies + +import ( + "errors" + "fmt" + "strconv" + "strings" + "time" + + "github.com/Azure/azure-container-networking/npm/metrics" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ioutil" + "github.com/Azure/azure-container-networking/npm/util" + npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" + "k8s.io/klog" + utilexec "k8s.io/utils/exec" +) + +const ( + defaultlockWaitTimeInSeconds string = "60" + reconcileChainTimeInMinutes int = 5 + + doesNotExistErrorCode int = 1 // Bad rule (does a matching rule exist in that chain?) + couldntLoadTargetErrorCode int = 2 // Couldn't load target `AZURE-NPM-EGRESS':No such file or directory + + minLineNumberStringLength int = 3 + minChainStringLength int = 7 +) + +var ( + iptablesAzureChains = []string{ + util.IptablesAzureChain, + util.IptablesAzureIngressChain, + util.IptablesAzureIngressAllowMarkChain, + util.IptablesAzureEgressChain, + util.IptablesAzureAcceptChain, + } + iptablesAzureDeprecatedChains = []string{ + // NPM v1 + util.IptablesAzureIngressFromChain, + util.IptablesAzureIngressPortChain, + util.IptablesAzureIngressDropsChain, + util.IptablesAzureEgressToChain, + util.IptablesAzureEgressPortChain, + util.IptablesAzureEgressDropsChain, + // older + util.IptablesAzureTargetSetsChain, + util.IptablesAzureIngressWrongDropsChain, + } + iptablesOldAndNewChains = append(iptablesAzureChains, iptablesAzureDeprecatedChains...) + + jumpToAzureChainArgs = []string{util.IptablesJumpFlag, util.IptablesAzureChain, util.IptablesModuleFlag, util.IptablesCtstateModuleFlag, util.IptablesCtstateFlag, util.IptablesNewState} + jumpFromForwardToAzureChainArgs = append([]string{util.IptablesForwardChain}, jumpToAzureChainArgs...) + + ingressOrEgressPolicyChainPattern = fmt.Sprintf("'Chain %s-\\|Chain %s-'", util.IptablesAzureIngressPolicyChainPrefix, util.IptablesAzureEgressPolicyChainPrefix) +) + +func (pMgr *PolicyManager) initialize() error { + if err := pMgr.initializeNPMChains(); err != nil { + return npmerrors.SimpleErrorWrapper("failed to initialize NPM chains", err) + } + return nil +} + +func (pMgr *PolicyManager) reset() error { + if err := pMgr.removeNPMChains(); err != nil { + return npmerrors.SimpleErrorWrapper("failed to remove NPM chains", err) + } + return nil +} + +// initializeNPMChains creates all chains/rules and makes sure the jump from FORWARD chain to +// AZURE-NPM chain is after the jumps to KUBE-FORWARD & KUBE-SERVICES chains (if they exist). +func (pMgr *PolicyManager) initializeNPMChains() error { + klog.Infof("Initializing AZURE-NPM chains.") + creator := pMgr.getCreatorForInitChains() + err := restore(creator) + if err != nil { + return npmerrors.SimpleErrorWrapper("failed to create chains and rules", err) + } + + // add the jump rule from FORWARD chain to AZURE-NPM chain + if err := pMgr.positionAzureChainJumpRule(); err != nil { + baseErrString := "failed to add/reposition jump from FORWARD chain to AZURE-NPM chain" + metrics.SendErrorLogAndMetric(util.IptmID, "Error: %s with error: %s", baseErrString, err.Error()) + return npmerrors.SimpleErrorWrapper(baseErrString, err) // we used to ignore this error in v1 + } + return nil +} + +// removeNPMChains removes the jump rule from FORWARD chain to AZURE-NPM chain +// and flushes and deletes all NPM Chains. +func (pMgr *PolicyManager) removeNPMChains() error { + deleteErrCode, deleteErr := pMgr.runIPTablesCommand(util.IptablesDeletionFlag, jumpFromForwardToAzureChainArgs...) + hadDeleteError := deleteErr != nil && deleteErrCode != couldntLoadTargetErrorCode + if hadDeleteError { + baseErrString := "failed to delete jump from FORWARD chain to AZURE-NPM chain" + metrics.SendErrorLogAndMetric(util.IptmID, "Error: %s with exit code %d and error: %s", baseErrString, deleteErrCode, deleteErr.Error()) + // FIXME update ID + return npmerrors.SimpleErrorWrapper(baseErrString, deleteErr) + } + + // flush all chains (will create any chain, including deprecated ones, if they don't exist) + creatorToFlush, chainsToDelete := pMgr.getCreatorAndChainsForReset() + restoreError := restore(creatorToFlush) + if restoreError != nil { + return npmerrors.SimpleErrorWrapper("failed to flush chains", restoreError) + } + + // TODO aggregate an error for each chain that failed to delete + var anyDeleteErr error + for _, chainName := range chainsToDelete { + errCode, err := pMgr.runIPTablesCommand(util.IptablesDestroyFlag, chainName) + if err != nil { + klog.Infof("couldn't delete chain %s with error [%v] and exit code [%d]", chainName, err, errCode) + anyDeleteErr = err + } + } + + if anyDeleteErr != nil { + return npmerrors.SimpleErrorWrapper("couldn't delete all chains", anyDeleteErr) + } + return nil +} + +// ReconcileChains periodically creates the jump rule from FORWARD chain to AZURE-NPM chain (if it d.n.e) +// and makes sure it's after the jumps to KUBE-FORWARD & KUBE-SERVICES chains (if they exist). +func (pMgr *PolicyManager) ReconcileChains(stopChannel <-chan struct{}) { + go pMgr.reconcileChains(stopChannel) +} + +func (pMgr *PolicyManager) reconcileChains(stopChannel <-chan struct{}) { + ticker := time.NewTicker(time.Minute * time.Duration(reconcileChainTimeInMinutes)) + defer ticker.Stop() + + for { + select { + case <-stopChannel: + return + case <-ticker.C: + if err := pMgr.positionAzureChainJumpRule(); err != nil { + metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to reconcile jump rule to Azure-NPM due to %s", err.Error()) + } + } + } +} + +// this function has a direct comparison in NPM v1 iptables manager (iptm.go) +func (pMgr *PolicyManager) runIPTablesCommand(operationFlag string, args ...string) (int, error) { + allArgs := []string{util.IptablesWaitFlag, defaultlockWaitTimeInSeconds, operationFlag} + allArgs = append(allArgs, args...) + + if operationFlag != util.IptablesCheckFlag { + klog.Infof("Executing iptables command with args %v", allArgs) + } + + command := pMgr.ioShim.Exec.Command(util.Iptables, allArgs...) + output, err := command.CombinedOutput() + + var exitError utilexec.ExitError + if ok := errors.As(err, &exitError); ok { + errCode := exitError.ExitStatus() + allArgsString := strings.Join(allArgs, " ") + msgStr := strings.TrimSuffix(string(output), "\n") + if errCode > 0 && operationFlag != util.IptablesCheckFlag { + metrics.SendErrorLogAndMetric(util.IptmID, "Error: There was an error running command: [%s %s] Stderr: [%v, %s]", util.Iptables, allArgsString, exitError, msgStr) + } + return errCode, npmerrors.SimpleErrorWrapper(fmt.Sprintf("failed to run iptables command [%s %s] Stderr: [%s]", util.Iptables, allArgsString, msgStr), exitError) + } + return 0, nil +} + +func (pMgr *PolicyManager) getCreatorForInitChains() *ioutil.FileCreator { + creator := pMgr.getNewCreatorWithChains(iptablesAzureChains) + + // add AZURE-NPM chain rules + creator.AddLine("", nil, util.IptablesAppendFlag, util.IptablesAzureChain, util.IptablesJumpFlag, util.IptablesAzureIngressChain) + + creator.AddLine("", nil, util.IptablesAppendFlag, util.IptablesAzureChain, util.IptablesJumpFlag, util.IptablesAzureEgressChain) + + creator.AddLine("", nil, util.IptablesAppendFlag, util.IptablesAzureChain, util.IptablesJumpFlag, util.IptablesAzureAcceptChain) + + // add AZURE-NPM-INGRESS chain rules + ingressDropSpecs := []string{util.IptablesAppendFlag, util.IptablesAzureIngressChain, util.IptablesJumpFlag, util.IptablesDrop} + ingressDropSpecs = append(ingressDropSpecs, getOnMarkSpecs(util.IptablesAzureIngressDropMarkHex)...) + ingressDropSpecs = append(ingressDropSpecs, getCommentSpecs(fmt.Sprintf("DROP-ON-INGRESS-DROP-MARK-%s", util.IptablesAzureIngressDropMarkHex))...) + creator.AddLine("", nil, ingressDropSpecs...) + + // add AZURE-NPM-INGRESS-ALLOW-MARK chain + markIngressAllowSpecs := []string{util.IptablesAppendFlag, util.IptablesAzureIngressAllowMarkChain} + markIngressAllowSpecs = append(markIngressAllowSpecs, getSetMarkSpecs(util.IptablesAzureIngressAllowMarkHex)...) + markIngressAllowSpecs = append(markIngressAllowSpecs, getCommentSpecs(fmt.Sprintf("SET-INGRESS-ALLOW-MARK-%s", util.IptablesAzureIngressAllowMarkHex))...) + creator.AddLine("", nil, markIngressAllowSpecs...) + + creator.AddLine("", nil, util.IptablesAppendFlag, util.IptablesAzureIngressAllowMarkChain, util.IptablesJumpFlag, util.IptablesAzureEgressChain) + + // add AZURE-NPM-EGRESS chain rules + egressDropSpecs := []string{util.IptablesAppendFlag, util.IptablesAzureEgressChain, util.IptablesJumpFlag, util.IptablesDrop} + egressDropSpecs = append(egressDropSpecs, getOnMarkSpecs(util.IptablesAzureEgressDropMarkHex)...) + egressDropSpecs = append(egressDropSpecs, getCommentSpecs(fmt.Sprintf("DROP-ON-EGRESS-DROP-MARK-%s", util.IptablesAzureEgressDropMarkHex))...) + creator.AddLine("", nil, egressDropSpecs...) + + jumpOnIngressMatchSpecs := []string{util.IptablesAppendFlag, util.IptablesAzureEgressChain, util.IptablesJumpFlag, util.IptablesAzureAcceptChain} + jumpOnIngressMatchSpecs = append(jumpOnIngressMatchSpecs, getOnMarkSpecs(util.IptablesAzureIngressAllowMarkHex)...) + jumpOnIngressMatchSpecs = append(jumpOnIngressMatchSpecs, getCommentSpecs(fmt.Sprintf("ACCEPT-ON-INGRESS-ALLOW-MARK-%s", util.IptablesAzureIngressAllowMarkHex))...) + creator.AddLine("", nil, jumpOnIngressMatchSpecs...) + + // add AZURE-NPM-ACCEPT chain rules + clearSpecs := []string{util.IptablesAppendFlag, util.IptablesAzureAcceptChain} + clearSpecs = append(clearSpecs, getSetMarkSpecs(util.IptablesAzureClearMarkHex)...) + clearSpecs = append(clearSpecs, getCommentSpecs("Clear-AZURE-NPM-MARKS")...) + creator.AddLine("", nil, clearSpecs...) + + creator.AddLine("", nil, util.IptablesAppendFlag, util.IptablesAzureAcceptChain, util.IptablesJumpFlag, util.IptablesAccept) + + creator.AddLine("", nil, util.IptablesRestoreCommit) + return creator +} + +// add/reposition AZURE-NPM chain after KUBE-FORWARD and KUBE-SERVICE chains if they exist +// this function has a direct comparison in NPM v1 iptables manager (iptm.go) +func (pMgr *PolicyManager) positionAzureChainJumpRule() error { + kubeServicesLine, kubeServicesLineNumErr := pMgr.getChainLineNumber(util.IptablesKubeServicesChain) + if kubeServicesLineNumErr != nil { + // not possible to cover this branch currently because of testing limitations for pipeCommandToGrep() + baseErrString := "failed to get index of jump from KUBE-SERVICES chain to FORWARD chain with error" + metrics.SendErrorLogAndMetric(util.IptmID, "Error: %s: %s", baseErrString, kubeServicesLineNumErr.Error()) + return npmerrors.SimpleErrorWrapper(baseErrString, kubeServicesLineNumErr) + } + + index := kubeServicesLine + 1 + + // TODO could call getChainLineNumber instead, and say it doesn't exist for lineNum == 0 + jumpRuleErrCode, checkErr := pMgr.runIPTablesCommand(util.IptablesCheckFlag, jumpFromForwardToAzureChainArgs...) + hadCheckError := checkErr != nil && jumpRuleErrCode != doesNotExistErrorCode + if hadCheckError { + baseErrString := "failed to check if jump from FORWARD chain to AZURE-NPM chain exists" + metrics.SendErrorLogAndMetric(util.IptmID, "Error: %s: %s", baseErrString, checkErr.Error()) + return npmerrors.SimpleErrorWrapper(baseErrString, checkErr) + } + jumpRuleExists := jumpRuleErrCode != doesNotExistErrorCode + + if !jumpRuleExists { + klog.Infof("Inserting jump from FORWARD chain to AZURE-NPM chain") + jumpRuleInsertionArgs := append([]string{util.IptablesForwardChain, strconv.Itoa(index)}, jumpToAzureChainArgs...) + if insertErrCode, insertErr := pMgr.runIPTablesCommand(util.IptablesInsertionFlag, jumpRuleInsertionArgs...); insertErr != nil { + baseErrString := "failed to insert jump from FORWARD chain to AZURE-NPM chain" + metrics.SendErrorLogAndMetric(util.IptmID, "Error: %s with error code %d and error %s", baseErrString, insertErrCode, insertErr.Error()) + // FIXME update ID + return npmerrors.SimpleErrorWrapper(baseErrString, insertErr) + } + return nil + } + + if kubeServicesLine <= 1 { + // jump to KUBE-SERVICES chain doesn't exist or is the first rule + return nil + } + + npmChainLine, npmLineNumErr := pMgr.getChainLineNumber(util.IptablesAzureChain) + if npmLineNumErr != nil { + // not possible to cover this branch currently because of testing limitations for pipeCommandToGrep() + baseErrString := "failed to get index of jump from FORWARD chain to AZURE-NPM chain" + metrics.SendErrorLogAndMetric(util.IptmID, "Error: %s: %s", baseErrString, npmLineNumErr.Error()) + // FIXME update ID + return npmerrors.SimpleErrorWrapper(baseErrString, npmLineNumErr) + } + + // Kube-services line number is less than npm chain line number then all good + if kubeServicesLine < npmChainLine { + return nil + } + + // AZURE-NPM chain is before KUBE-SERVICES then + // delete existing jump rule and add it in the right order + metrics.SendErrorLogAndMetric(util.IptmID, "Info: Reconciler deleting and re-adding jump from FORWARD chain to AZURE-NPM chain table.") + if deleteErrCode, deleteErr := pMgr.runIPTablesCommand(util.IptablesDeletionFlag, jumpFromForwardToAzureChainArgs...); deleteErr != nil { + baseErrString := "failed to delete jump from FORWARD chain to AZURE-NPM chain" + metrics.SendErrorLogAndMetric(util.IptmID, "Error: %s with error code %d and error %s", baseErrString, deleteErrCode, deleteErr.Error()) + // FIXME update ID + return npmerrors.SimpleErrorWrapper(baseErrString, deleteErr) + } + + // Reduce index for deleted AZURE-NPM chain + if index > 1 { + index-- + } + jumpRuleInsertionArgs := append([]string{util.IptablesForwardChain, strconv.Itoa(index)}, jumpToAzureChainArgs...) + if insertErrCode, insertErr := pMgr.runIPTablesCommand(util.IptablesInsertionFlag, jumpRuleInsertionArgs...); insertErr != nil { + baseErrString := "after deleting, failed to insert jump from FORWARD chain to AZURE-NPM chain" + // FIXME update ID + metrics.SendErrorLogAndMetric(util.IptmID, "Error: %s with error code %d and error %s", baseErrString, insertErrCode, insertErr.Error()) + return npmerrors.SimpleErrorWrapper(baseErrString, insertErr) + } + + return nil +} + +// returns 0 if the chain d.n.e. +// this function has a direct comparison in NPM v1 iptables manager (iptm.go) +func (pMgr *PolicyManager) getChainLineNumber(chain string) (int, error) { + // TODO could call this once and use regex instead of grep to cut down on OS calls + listForwardEntriesCommand := pMgr.ioShim.Exec.Command(util.Iptables, + util.IptablesWaitFlag, defaultlockWaitTimeInSeconds, util.IptablesTableFlag, util.IptablesFilterTable, + util.IptablesNumericFlag, util.IptablesListFlag, util.IptablesForwardChain, util.IptablesLineNumbersFlag, + ) + grepCommand := pMgr.ioShim.Exec.Command("grep", chain) + searchResults, gotMatches, err := pipeCommandToGrep(listForwardEntriesCommand, grepCommand) + if err != nil { + // not possible to cover this branch currently because of testing limitations for pipeCommandToGrep() + return 0, npmerrors.SimpleErrorWrapper(fmt.Sprintf("failed to determine line number for jump from FORWARD chain to %s chain", chain), err) + } + if !gotMatches { + return 0, nil + } + if len(searchResults) >= minLineNumberStringLength { + lineNum, _ := strconv.Atoi(string(searchResults[0])) + return lineNum, nil + } + return 0, nil +} + +func pipeCommandToGrep(command, grepCommand utilexec.Cmd) (searchResults []byte, gotMatches bool, commandError error) { + pipe, commandError := command.StdoutPipe() + if commandError != nil { + return + } + + closePipe := func() { _ = pipe.Close() } // appease go lint + defer closePipe() + + commandError = command.Start() + if commandError != nil { + return + } + + // Without this wait, defunct iptable child process are created + wait := func() { _ = command.Wait() } // appease go lint + defer wait() + + output, err := grepCommand.CombinedOutput() + if err != nil { + // grep returns err status 1 if nothing is found + return + } + searchResults = output + gotMatches = true + return +} + +// make this a function for easier testing +func (pMgr *PolicyManager) getCreatorAndChainsForReset() (creator *ioutil.FileCreator, chainsToFlush []string) { + oldPolicyChains, err := pMgr.getPolicyChainNames() + if err != nil { + // not possible to cover this branch currently because of testing limitations for pipeCommandToGrep() + metrics.SendErrorLogAndMetric(util.IptmID, "Error: failed to determine NPM ingress/egress policy chains to delete") + } + chainsToFlush = iptablesOldAndNewChains + chainsToFlush = append(chainsToFlush, oldPolicyChains...) // will work even if oldPolicyChains is nil + creator = pMgr.getNewCreatorWithChains(chainsToFlush) + creator.AddLine("", nil, util.IptablesRestoreCommit) + return +} + +func (pMgr *PolicyManager) getPolicyChainNames() ([]string, error) { + iptablesListCommand := pMgr.ioShim.Exec.Command(util.Iptables, + util.IptablesWaitFlag, defaultlockWaitTimeInSeconds, util.IptablesTableFlag, util.IptablesFilterTable, + util.IptablesNumericFlag, util.IptablesListFlag, + ) + grepCommand := pMgr.ioShim.Exec.Command("grep", ingressOrEgressPolicyChainPattern) + searchResults, gotMatches, err := pipeCommandToGrep(iptablesListCommand, grepCommand) + if err != nil { + // not possible to cover this branch currently because of testing limitations for pipeCommandToGrep() + return nil, npmerrors.SimpleErrorWrapper("failed to get policy chain names", err) + } + if !gotMatches { + return nil, nil + } + lines := strings.Split(string(searchResults), "\n") + chainNames := make([]string, 0, len(lines)) // don't want to preallocate size in case of have malformed lines + for _, line := range lines { + if len(line) < minChainStringLength { + klog.Errorf("got unexpected grep output for ingress/egress policy chains") + } else { + chainNames = append(chainNames, line[minChainStringLength-1:]) + } + } + return chainNames, nil +} + +func getOnMarkSpecs(mark string) []string { + return []string{ + util.IptablesModuleFlag, + util.IptablesMarkVerb, + util.IptablesMarkFlag, + mark, + } +} diff --git a/npm/pkg/dataplane/policies/chain-management_linux_test.go b/npm/pkg/dataplane/policies/chain-management_linux_test.go new file mode 100644 index 0000000000..e49147e741 --- /dev/null +++ b/npm/pkg/dataplane/policies/chain-management_linux_test.go @@ -0,0 +1,421 @@ +package policies + +import ( + "fmt" + "strings" + "testing" + + "github.com/Azure/azure-container-networking/common" + dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" + testutils "github.com/Azure/azure-container-networking/test/utils" + "github.com/stretchr/testify/require" +) + +func TestInitChainsCreator(t *testing.T) { + pMgr := NewPolicyManager(common.NewMockIOShim(nil)) + creator := pMgr.getCreatorForInitChains() // doesn't make any exec calls + actualFileString := creator.ToString() + expectedLines := []string{"*filter"} + for _, chain := range iptablesAzureChains { + expectedLines = append(expectedLines, fmt.Sprintf(":%s - -", chain)) + } + + expectedLines = append(expectedLines, []string{ + "-A AZURE-NPM -j AZURE-NPM-INGRESS", + "-A AZURE-NPM -j AZURE-NPM-EGRESS", + "-A AZURE-NPM -j AZURE-NPM-ACCEPT", + "-A AZURE-NPM-INGRESS -j DROP -m mark --mark 0x4000 -m comment --comment DROP-ON-INGRESS-DROP-MARK-0x4000", + "-A AZURE-NPM-INGRESS-ALLOW-MARK -j MARK --set-mark 0x2000 -m comment --comment SET-INGRESS-ALLOW-MARK-0x2000", + "-A AZURE-NPM-INGRESS-ALLOW-MARK -j AZURE-NPM-EGRESS", + "-A AZURE-NPM-EGRESS -j DROP -m mark --mark 0x5000 -m comment --comment DROP-ON-EGRESS-DROP-MARK-0x5000", + "-A AZURE-NPM-EGRESS -j AZURE-NPM-ACCEPT -m mark --mark 0x2000 -m comment --comment ACCEPT-ON-INGRESS-ALLOW-MARK-0x2000", + "-A AZURE-NPM-ACCEPT -j MARK --set-mark 0x0 -m comment --comment Clear-AZURE-NPM-MARKS", + "-A AZURE-NPM-ACCEPT -j ACCEPT", + "COMMIT\n", + }...) + expectedFileString := strings.Join(expectedLines, "\n") + dptestutils.AssertEqualMultilineStrings(t, expectedFileString, actualFileString) +} + +func TestInitChainsSuccess(t *testing.T) { + calls := GetInitializeTestCalls() + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.NoError(t, pMgr.initializeNPMChains()) +} + +func TestInitChainsFailureOnRestore(t *testing.T) { + calls := []testutils.TestCmd{fakeIPTablesRestoreFailureCommand} + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.Error(t, pMgr.initializeNPMChains()) +} + +func TestInitChainsFailureOnPosition(t *testing.T) { + calls := []testutils.TestCmd{ + fakeIPTablesRestoreCommand, // gives correct exit code + { + Cmd: listLineNumbersCommandStrings, + ExitCode: 1, // grep call gets this exit code (exit code 1 means grep found nothing) + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + { + Cmd: []string{"grep", "KUBE-SERVICES"}, + Stdout: "iptables: No chain/target/match by that name.", // this Stdout and ExitCode are for the iptables check command below + ExitCode: 2, // Check failed for unknown reason + }, + {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.Error(t, pMgr.initializeNPMChains()) +} + +func TestRemoveChainsCreator(t *testing.T) { + creatorCalls := []testutils.TestCmd{ + { + Cmd: listPolicyChainNamesCommandStrings, + Stdout: "Chain AZURE-NPM-INGRESS-123456\nChain AZURE-NPM-EGRESS-123456", + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + {Cmd: []string{"grep", ingressOrEgressPolicyChainPattern}}, + } + + pMgr := NewPolicyManager(common.NewMockIOShim(creatorCalls)) + creator, chainsToFlush := pMgr.getCreatorAndChainsForReset() + expectedChainsToFlush := []string{ + "AZURE-NPM", + "AZURE-NPM-INGRESS", + "AZURE-NPM-INGRESS-ALLOW-MARK", + "AZURE-NPM-EGRESS", + "AZURE-NPM-ACCEPT", + // deprecated + "AZURE-NPM-INGRESS-FROM", + "AZURE-NPM-INGRESS-PORT", + "AZURE-NPM-INGRESS-DROPS", + "AZURE-NPM-EGRESS-TO", + "AZURE-NPM-EGRESS-PORT", + "AZURE-NPM-EGRESS-DROPS", + "AZURE-NPM-TARGET-SETS", + "AZURE-NPM-INRGESS-DROPS", + // policy chains + "AZURE-NPM-INGRESS-123456", + "AZURE-NPM-EGRESS-123456", + } + require.Equal(t, expectedChainsToFlush, chainsToFlush) + actualFileString := creator.ToString() + expectedLines := []string{"*filter"} + for _, chain := range expectedChainsToFlush { + expectedLines = append(expectedLines, fmt.Sprintf(":%s - -", chain)) + } + expectedLines = append(expectedLines, "COMMIT\n") + expectedFileString := strings.Join(expectedLines, "\n") + dptestutils.AssertEqualMultilineStrings(t, expectedFileString, actualFileString) +} + +func TestRemoveChainsSuccess(t *testing.T) { + calls := GetResetTestCalls() + for _, chain := range iptablesOldAndNewChains { + calls = append(calls, getFakeDestroyCommand(chain)) + } + calls = append( + calls, + getFakeDestroyCommand("AZURE-NPM-INGRESS-123456"), + getFakeDestroyCommand("AZURE-NPM-EGRESS-123456"), + ) + + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.NoError(t, pMgr.removeNPMChains()) +} + +func TestRemoveChainsFailureOnDelete(t *testing.T) { + calls := []testutils.TestCmd{ + { + Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + ExitCode: 1, // delete failure + }, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.Error(t, pMgr.removeNPMChains()) +} + +func TestRemoveChainsFailureOnRestore(t *testing.T) { + calls := []testutils.TestCmd{ + {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + { + Cmd: listPolicyChainNamesCommandStrings, + Stdout: "Chain AZURE-NPM-INGRESS-123456\nChain AZURE-NPM-EGRESS-123456", + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + { + Cmd: []string{"grep", ingressOrEgressPolicyChainPattern}, + ExitCode: 1, // ExitCode 1 for the iptables restore command + }, + fakeIPTablesRestoreFailureCommand, // the exit code doesn't matter for this command since it receives the exit code of the command above + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.Error(t, pMgr.removeNPMChains()) +} + +func TestRemoveChainsFailureOnDestroy(t *testing.T) { + calls := []testutils.TestCmd{ + {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + { + Cmd: listPolicyChainNamesCommandStrings, + Stdout: "Chain AZURE-NPM-INGRESS-123456\nChain AZURE-NPM-EGRESS-123456", + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + {Cmd: []string{"grep", ingressOrEgressPolicyChainPattern}}, // ExitCode 0 for the iptables restore command + fakeIPTablesRestoreCommand, + } + calls = append(calls, getFakeDestroyFailureCommand(iptablesOldAndNewChains[0])) // this ExitCode here will actually impact the next below + for _, chain := range iptablesOldAndNewChains[1:] { + calls = append(calls, getFakeDestroyCommand(chain)) + } + calls = append( + calls, + getFakeDestroyCommand("AZURE-NPM-INGRESS-123456"), + getFakeDestroyCommand("AZURE-NPM-EGRESS-123456"), + ) + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.Error(t, pMgr.removeNPMChains()) +} + +func TestPositionJumpWhenNoChainsExist(t *testing.T) { + calls := []testutils.TestCmd{ + { + Cmd: listLineNumbersCommandStrings, + ExitCode: 1, // grep call gets this exit code (exit code 1 means grep found nothing) + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + { + Cmd: []string{"grep", "KUBE-SERVICES"}, + Stdout: "iptables: No chain/target/match by that name.", // this Stdout and ExitCode are for the iptables check command below + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "1", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.NoError(t, pMgr.positionAzureChainJumpRule()) +} + +func TestPositionJumpWhenOnlyAzureExists(t *testing.T) { + calls := []testutils.TestCmd{ + { + Cmd: listLineNumbersCommandStrings, + ExitCode: 1, // grep call gets this exit code (exit code 1 means grep found nothing) + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + {Cmd: []string{"grep", "KUBE-SERVICES"}}, // ExitCode 0 for the iptables check command below + {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.NoError(t, pMgr.positionAzureChainJumpRule()) +} + +func TestPositionJumpWhenOnlyKubeServicesExists(t *testing.T) { + calls := []testutils.TestCmd{ + { + Cmd: listLineNumbersCommandStrings, + Stdout: "3 KUBE-SERVICES all -- 0.0.0.0/0 0.0.0.0/0 ", // grep call gets this Stdout + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + { + Cmd: []string{"grep", "KUBE-SERVICES"}, + Stdout: "iptables: No chain/target/match by that name.", // this Stdout and ExitCode are for the iptables check command below + ExitCode: 1, + }, + {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"}}, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.NoError(t, pMgr.positionAzureChainJumpRule()) +} + +func TestPositionJumpWhenOnlyKubeServicesExistsAndInsertFails(t *testing.T) { + calls := []testutils.TestCmd{ + { + Cmd: listLineNumbersCommandStrings, + Stdout: "3 KUBE-SERVICES all -- 0.0.0.0/0 0.0.0.0/0 ", // grep call gets this Stdout + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + { + Cmd: []string{"grep", "KUBE-SERVICES"}, + Stdout: "iptables: No chain/target/match by that name.", // this Stdout and ExitCode are for the iptables check command below + ExitCode: 1, + }, + { + Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + ExitCode: 1, // ExitCode 1 for insert below + }, + {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "4", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.Error(t, pMgr.positionAzureChainJumpRule()) +} + +func TestPositionJumpWhenAzureAfterKubeServices(t *testing.T) { + // don't move the rule for AZURE-NPM + calls := []testutils.TestCmd{ + { + Cmd: listLineNumbersCommandStrings, + Stdout: "3 KUBE-SERVICES all -- 0.0.0.0/0 0.0.0.0/0 ", // grep call gets this Stdout + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + {Cmd: []string{"grep", "KUBE-SERVICES"}}, // ExitCode 0 for the iptables check command below + { + Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + Stdout: "4 AZURE-NPM all -- 0.0.0.0/0 0.0.0.0/0 ", // grep call below gets this Stdout + }, + {Cmd: listLineNumbersCommandStrings}, + {Cmd: []string{"grep", "AZURE-NPM"}}, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.NoError(t, pMgr.positionAzureChainJumpRule()) +} + +func TestPositionJumpWhenAzureBeforeKubeServices(t *testing.T) { + // move the rule for AZURE-NPM + calls := []testutils.TestCmd{ + { + Cmd: listLineNumbersCommandStrings, + Stdout: "3 KUBE-SERVICES all -- 0.0.0.0/0 0.0.0.0/0 ", // grep call gets this Stdout + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + {Cmd: []string{"grep", "KUBE-SERVICES"}}, // ExitCode 0 for the iptables check command below + { + Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + Stdout: "2 AZURE-NPM all -- 0.0.0.0/0 0.0.0.0/0 ", // grep call below gets this Stdout + }, + {Cmd: listLineNumbersCommandStrings}, + {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"}}, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.NoError(t, pMgr.positionAzureChainJumpRule()) +} + +func TestPositionJumpWhenAzureBeforeKubeServicesAndDeleteFails(t *testing.T) { + calls := []testutils.TestCmd{ + { + Cmd: listLineNumbersCommandStrings, + Stdout: "3 KUBE-SERVICES all -- 0.0.0.0/0 0.0.0.0/0 ", // grep call gets this Stdout + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + {Cmd: []string{"grep", "KUBE-SERVICES"}}, // ExitCode 0 for the iptables check command below + { + Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + Stdout: "2 AZURE-NPM all -- 0.0.0.0/0 0.0.0.0/0 ", // grep call below gets this Stdout + }, + { + Cmd: listLineNumbersCommandStrings, + ExitCode: 1, + // NOTE: now MockIOShim is off by 2 for ExitCodes and Stdout + // ExitCode 1 for delete command below + }, + {Cmd: []string{"grep", "AZURE-NPM"}}, + {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.Error(t, pMgr.positionAzureChainJumpRule()) +} + +func TestPositionJumpWhenAzureBeforeKubeServicesAndInsertFails(t *testing.T) { + calls := []testutils.TestCmd{ + { + Cmd: listLineNumbersCommandStrings, + Stdout: "3 KUBE-SERVICES all -- 0.0.0.0/0 0.0.0.0/0 ", // grep call gets this Stdout + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + {Cmd: []string{"grep", "KUBE-SERVICES"}}, // ExitCode 0 for the iptables check command below + { + Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}, + Stdout: "2 AZURE-NPM all -- 0.0.0.0/0 0.0.0.0/0 ", // grep call below gets this Stdout + }, + {Cmd: listLineNumbersCommandStrings}, // NOTE: now MockIOShim is off by 2 for ExitCodes and Stdout + // ExitCode 0 for delete command below + { + Cmd: []string{"grep", "AZURE-NPM"}, + ExitCode: 1, // ExitCode 1 for insert command below + }, + {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"}}, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + require.Error(t, pMgr.positionAzureChainJumpRule()) +} + +func TestGetChainLineNumber(t *testing.T) { + testChainName := "TEST-CHAIN-NAME" + grepCommand := testutils.TestCmd{Cmd: []string{"grep", testChainName}} + + // chain exists at line 3 + calls := []testutils.TestCmd{ + { + Cmd: listLineNumbersCommandStrings, + Stdout: fmt.Sprintf("3 %s all -- 0.0.0.0/0 0.0.0.0/0 ", testChainName), + ExitCode: 0, + }, + grepCommand, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + lineNum, err := pMgr.getChainLineNumber(testChainName) + require.Equal(t, 3, lineNum) + require.NoError(t, err) + + // chain doesn't exist + calls = []testutils.TestCmd{ + { + Cmd: listLineNumbersCommandStrings, + ExitCode: 1, // grep found nothing + }, + grepCommand, + } + pMgr = NewPolicyManager(common.NewMockIOShim(calls)) + lineNum, err = pMgr.getChainLineNumber(testChainName) + require.Equal(t, 0, lineNum) + require.NoError(t, err) +} + +func TestGetPolicyChainNames(t *testing.T) { + // grep that finds results + grepCommand := testutils.TestCmd{Cmd: []string{"grep", ingressOrEgressPolicyChainPattern}} + calls := []testutils.TestCmd{ + { + Cmd: listPolicyChainNamesCommandStrings, + Stdout: "Chain AZURE-NPM-INGRESS-123456\nChain AZURE-NPM-EGRESS-123456", + }, + grepCommand, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + chainNames, err := pMgr.getPolicyChainNames() + expectedChainNames := []string{ + "AZURE-NPM-INGRESS-123456", + "AZURE-NPM-EGRESS-123456", + } + require.Equal(t, expectedChainNames, chainNames) + require.NoError(t, err) + + // grep with no results + calls = []testutils.TestCmd{ + { + Cmd: listPolicyChainNamesCommandStrings, + ExitCode: 1, // grep found nothing + }, + grepCommand, + } + pMgr = NewPolicyManager(common.NewMockIOShim(calls)) + chainNames, err = pMgr.getPolicyChainNames() + expectedChainNames = nil + require.Equal(t, expectedChainNames, chainNames) + require.NoError(t, err) +} + +func getFakeDestroyCommand(chain string) testutils.TestCmd { + return testutils.TestCmd{Cmd: []string{"iptables", "-w", "60", "-X", chain}} +} + +func getFakeDestroyFailureCommand(chain string) testutils.TestCmd { + command := getFakeDestroyCommand(chain) + command.ExitCode = 1 + return command +} diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index dc969ba044..3d8d29dba5 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -1,7 +1,10 @@ package policies import ( + "strconv" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + "github.com/Azure/azure-container-networking/npm/util" networkingv1 "k8s.io/api/networking/v1" ) @@ -47,6 +50,37 @@ type ACLPolicy struct { Protocol Protocol } +func (aclPolicy *ACLPolicy) hasKnownDirection() bool { + return aclPolicy.Direction == Ingress || + aclPolicy.Direction == Egress || + aclPolicy.Direction == Both +} + +func (aclPolicy *ACLPolicy) hasIngress() bool { + return aclPolicy.Direction == Ingress || aclPolicy.Direction == Both +} + +func (aclPolicy *ACLPolicy) hasEgress() bool { + return aclPolicy.Direction == Egress || aclPolicy.Direction == Both +} + +func (aclPolicy *ACLPolicy) hasKnownProtocol() bool { + return aclPolicy.Protocol != "" && (aclPolicy.Protocol == TCP || + aclPolicy.Protocol == UDP || + aclPolicy.Protocol == SCTP || + aclPolicy.Protocol == ICMP || + aclPolicy.Protocol == AnyProtocol) +} + +func (aclPolicy *ACLPolicy) hasKnownTarget() bool { + return aclPolicy.Target == Allowed || aclPolicy.Target == Dropped +} + +func (aclPolicy *ACLPolicy) satisifiesPortAndProtocolConstraints() bool { + return aclPolicy.Protocol != AnyProtocol || + (len(aclPolicy.SrcPorts) == 0 && len(aclPolicy.DstPorts) == 0) +} + // SetInfo helps capture additional details in a matchSet // example match set in linux: // ! azure-npm-123 src,src @@ -56,20 +90,39 @@ type ACLPolicy struct { type SetInfo struct { IPSet *ipsets.IPSetMetadata Included bool - MatchType string // match type can be “src”, “src,dst” or “dst,dst” etc + MatchType MatchType } +// Ports represents a range of ports. +// To specify one port, set Port and EndPort to the same value. +// uint16 is used since there are 2^16 - 1 TCP/UDP ports (0 is invalid) +// and 2^16 SCTP ports. ICMP is connectionless and doesn't use ports. type Ports struct { Port int32 EndPort int32 } +func (portRange *Ports) isValidRange() bool { + return portRange.Port <= portRange.EndPort +} + +func (portRange *Ports) toIPTablesString() string { + start := strconv.Itoa(int(portRange.Port)) + if portRange.Port == portRange.EndPort { + return start + } + end := strconv.Itoa(int(portRange.EndPort)) + return start + ":" + end +} + type Verdict string type Direction string type Protocol string +type MatchType int8 + const ( // Ingress when packet is entering a container Ingress Direction = "IN" @@ -92,5 +145,29 @@ const ( // ICMP Protocol ICMP Protocol = "icmp" // AnyProtocol can be used for all other protocols - AnyProtocol Protocol = "any" + AnyProtocol Protocol = "all" +) + +// Possible MatchTypes. +// MatchTypes with 2 locations (e.g. DstDst) are for ip and port respectively. +const ( + SrcMatch MatchType = 0 + DstMatch MatchType = 1 + DstDstMatch MatchType = 3 ) + +var matchTypeStrings = map[MatchType]string{ + SrcMatch: util.IptablesSrcFlag, + DstMatch: util.IptablesDstFlag, + DstDstMatch: util.IptablesDstFlag + "," + util.IptablesDstFlag, +} + +// match type is only used in Linux +func (setInfo *SetInfo) hasKnownMatchType() bool { + _, exists := matchTypeStrings[setInfo.MatchType] + return exists +} + +func (matchType MatchType) toIPTablesString() string { + return matchTypeStrings[matchType] +} diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index e03838b774..26b75653cc 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -1,7 +1,10 @@ package policies import ( + "fmt" + "github.com/Azure/azure-container-networking/common" + npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" "k8s.io/klog" ) @@ -23,8 +26,18 @@ func NewPolicyManager(ioShim *common.IOShim) *PolicyManager { } } +func (pMgr *PolicyManager) Initialize() error { + if err := pMgr.initialize(); err != nil { + return npmerrors.ErrorWrapper(npmerrors.InitializePolicyMgr, false, "failed to initialize policy manager", err) + } + return nil +} + func (pMgr *PolicyManager) Reset() error { - return pMgr.reset() + if err := pMgr.reset(); err != nil { + return npmerrors.ErrorWrapper(npmerrors.ResetPolicyMgr, false, "failed to reset policy manager", err) + } + return nil } func (pMgr *PolicyManager) PolicyExists(name string) bool { @@ -42,10 +55,15 @@ func (pMgr *PolicyManager) AddPolicy(policy *NPMNetworkPolicy, endpointList map[ klog.Infof("[DataPlane] No ACLs in policy %s to apply", policy.Name) return nil } + normalizePolicy(policy) + if err := checkForErrors(policy); err != nil { + return npmerrors.Errorf(npmerrors.AddPolicy, false, fmt.Sprintf("couldn't add malformed policy: %s", err.Error())) + } + // Call actual dataplane function to apply changes err := pMgr.addPolicy(policy, endpointList) if err != nil { - return err + return npmerrors.Errorf(npmerrors.AddPolicy, false, fmt.Sprintf("failed to add policy: %v", err)) } pMgr.policyMap.cache[policy.Name] = policy @@ -65,10 +83,71 @@ func (pMgr *PolicyManager) RemovePolicy(name string, endpointList map[string]str // Call actual dataplane function to apply changes err := pMgr.removePolicy(policy, endpointList) if err != nil { - return err + return npmerrors.Errorf(npmerrors.RemovePolicy, false, fmt.Sprintf("failed to remove policy: %v", err)) } delete(pMgr.policyMap.cache, name) return nil } + +func normalizePolicy(networkPolicy *NPMNetworkPolicy) { + for _, aclPolicy := range networkPolicy.ACLs { + if aclPolicy.Protocol == "" { + aclPolicy.Protocol = AnyProtocol + } + for _, portRange := range aclPolicy.SrcPorts { + if portRange.EndPort == 0 { + portRange.EndPort = portRange.Port + } + } + for _, portRange := range aclPolicy.DstPorts { + if portRange.EndPort == 0 { + portRange.EndPort = portRange.Port + } + } + } +} + +// TODO do verification in controller? +func checkForErrors(networkPolicy *NPMNetworkPolicy) error { + for _, aclPolicy := range networkPolicy.ACLs { + if !aclPolicy.hasKnownTarget() { + return npmerrors.SimpleError(fmt.Sprintf("ACL policy %s has unknown target", aclPolicy.PolicyID)) + } + if !aclPolicy.hasKnownDirection() { + return npmerrors.SimpleError(fmt.Sprintf("ACL policy %s has unknown direction", aclPolicy.PolicyID)) + } + if !aclPolicy.hasKnownProtocol() { + return npmerrors.SimpleError(fmt.Sprintf("ACL policy %s has unknown protocol (set to All if desired)", aclPolicy.PolicyID)) + } + if !aclPolicy.satisifiesPortAndProtocolConstraints() { + return npmerrors.SimpleError(fmt.Sprintf( + "ACL policy %s has multiple src or dst ports, so must have protocol tcp, udp, udplite, sctp, or dccp but has protocol %s", + aclPolicy.PolicyID, + string(aclPolicy.Protocol), + )) + } + for _, portRange := range aclPolicy.DstPorts { + if !portRange.isValidRange() { + return npmerrors.SimpleError(fmt.Sprintf("ACL policy %s has invalid port range in DstPorts (start: %d, end: %d)", aclPolicy.PolicyID, portRange.Port, portRange.EndPort)) + } + } + for _, portRange := range aclPolicy.DstPorts { + if !portRange.isValidRange() { + return npmerrors.SimpleError(fmt.Sprintf("ACL policy %s has invalid port range in SrcPorts (start: %d, end: %d)", aclPolicy.PolicyID, portRange.Port, portRange.EndPort)) + } + } + for _, setInfo := range aclPolicy.SrcList { + if !setInfo.hasKnownMatchType() { + return npmerrors.SimpleError(fmt.Sprintf("ACL policy %s has set %s in SrcList with unknown Match Type", aclPolicy.PolicyID, setInfo.IPSet.Name)) + } + } + for _, setInfo := range aclPolicy.DstList { + if !setInfo.hasKnownMatchType() { + return npmerrors.SimpleError(fmt.Sprintf("ACL policy %s has set %s in DstList with unknown Match Type", aclPolicy.PolicyID, setInfo.IPSet.Name)) + } + } + } + return nil +} diff --git a/npm/pkg/dataplane/policies/policymanager_linux.go b/npm/pkg/dataplane/policies/policymanager_linux.go index 7c6f03a7d4..67d02a7501 100644 --- a/npm/pkg/dataplane/policies/policymanager_linux.go +++ b/npm/pkg/dataplane/policies/policymanager_linux.go @@ -1,13 +1,312 @@ package policies -func (pMgr *PolicyManager) addPolicy(policy *NPMNetworkPolicy, _ map[string]string) error { +import ( + "fmt" + "strings" + + "github.com/Azure/azure-container-networking/log" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ioutil" + "github.com/Azure/azure-container-networking/npm/util" + npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" +) + +const ( + maxRetryCount = 1 + unknownLineErrorPattern = "line (\\d+) failed" // TODO this could happen if syntax is off or AZURE-NPM-INGRESS doesn't exist for -A AZURE-NPM-INGRESS -j hash(NP1) ... + knownLineErrorPattern = "Error occurred at line: (\\d+)" + + chainSectionPrefix = "chain" // TODO are sections necessary for error handling? + maxLengthForMatchSetSpecs = 6 // 5-6 elements depending on Included boolean +) + +// shouldn't call this if the np has no ACLs (check in generic) +func (pMgr *PolicyManager) addPolicy(networkPolicy *NPMNetworkPolicy, _ map[string]string) error { + // TODO check for newPolicy errors + creator := pMgr.getCreatorForNewNetworkPolicies(networkPolicy) + err := restore(creator) + if err != nil { + return npmerrors.SimpleErrorWrapper("failed to restore iptables with updated policies", err) + } + return nil +} + +func (pMgr *PolicyManager) removePolicy(networkPolicy *NPMNetworkPolicy, _ map[string]string) error { + deleteErr := pMgr.deleteOldJumpRulesOnRemove(networkPolicy) + if deleteErr != nil { + return npmerrors.SimpleErrorWrapper("failed to delete jumps to policy chains", deleteErr) + } + creator := pMgr.getCreatorForRemovingPolicies(networkPolicy) + restoreErr := restore(creator) + if restoreErr != nil { + return npmerrors.SimpleErrorWrapper("failed to flush policies", restoreErr) + } return nil } -func (pMgr *PolicyManager) removePolicy(policy *NPMNetworkPolicy, _ map[string]string) error { +func restore(creator *ioutil.FileCreator) error { + err := creator.RunCommandWithFile(util.IptablesRestore, util.IptablesRestoreTableFlag, util.IptablesFilterTable, util.IptablesRestoreNoFlushFlag) + if err != nil { + return npmerrors.SimpleErrorWrapper("failed to restore iptables file", err) + } + return nil +} + +func (pMgr *PolicyManager) getCreatorForRemovingPolicies(networkPolicies ...*NPMNetworkPolicy) *ioutil.FileCreator { + allChainNames := getAllChainNames(networkPolicies) + creator := pMgr.getNewCreatorWithChains(allChainNames) + creator.AddLine("", nil, util.IptablesRestoreCommit) + return creator +} + +// returns all chain names (ingress and egress policy chain names) +func getAllChainNames(networkPolicies []*NPMNetworkPolicy) []string { + chainNames := make([]string, 0) + for _, networkPolicy := range networkPolicies { + hasIngress, hasEgress := networkPolicy.hasIngressAndEgress() + + if hasIngress { + chainNames = append(chainNames, networkPolicy.getIngressChainName()) + } + if hasEgress { + chainNames = append(chainNames, networkPolicy.getEgressChainName()) + } + } + return chainNames +} + +// returns two booleans indicating whether the network policy has ingress and egress respectively +func (networkPolicy *NPMNetworkPolicy) hasIngressAndEgress() (hasIngress, hasEgress bool) { + hasIngress = false + hasEgress = false + for _, aclPolicy := range networkPolicy.ACLs { + hasIngress = hasIngress || aclPolicy.hasIngress() + hasEgress = hasEgress || aclPolicy.hasEgress() + } + return +} + +func (networkPolicy *NPMNetworkPolicy) getEgressChainName() string { + return networkPolicy.getChainName(util.IptablesAzureEgressPolicyChainPrefix) +} + +func (networkPolicy *NPMNetworkPolicy) getIngressChainName() string { + return networkPolicy.getChainName(util.IptablesAzureIngressPolicyChainPrefix) +} + +func (networkPolicy *NPMNetworkPolicy) getChainName(prefix string) string { + policyHash := util.Hash(networkPolicy.Name) // assuming the name is unique + return joinWithDash(prefix, policyHash) +} + +func (pMgr *PolicyManager) getNewCreatorWithChains(chainNames []string) *ioutil.FileCreator { + creator := ioutil.NewFileCreator(pMgr.ioShim, maxRetryCount, knownLineErrorPattern, unknownLineErrorPattern) // TODO pass an array instead of this ... thing + + creator.AddLine("", nil, "*"+util.IptablesFilterTable) // specify the table + for _, chainName := range chainNames { + // add chain headers + sectionID := joinWithDash(chainSectionPrefix, chainName) + counters := "-" // TODO specify counters eventually? would need iptables-save file + creator.AddLine(sectionID, nil, ":"+chainName, "-", counters) + // TODO remove sections?? + } + return creator +} + +// will make a similar func for on update eventually +func (pMgr *PolicyManager) deleteOldJumpRulesOnRemove(policy *NPMNetworkPolicy) error { + fmt.Println(policy.ACLs[0]) + + shouldDeleteIngress, shouldDeleteEgress := policy.hasIngressAndEgress() + fmt.Println(shouldDeleteIngress, shouldDeleteEgress) + if shouldDeleteIngress { + if err := pMgr.deleteJumpRule(policy, true); err != nil { + return err + } + } + if shouldDeleteEgress { + if err := pMgr.deleteJumpRule(policy, false); err != nil { + return err + } + } return nil } -func (pMgr *PolicyManager) reset() error { +func (pMgr *PolicyManager) deleteJumpRule(policy *NPMNetworkPolicy, isIngress bool) error { + var specs []string + var baseChainName string + var chainName string + if isIngress { + specs = getIngressJumpSpecs(policy) + baseChainName = util.IptablesAzureIngressChain + chainName = policy.getIngressChainName() + } else { + specs = getEgressJumpSpecs(policy) + baseChainName = util.IptablesAzureEgressChain + chainName = policy.getEgressChainName() + } + + specs = append([]string{baseChainName}, specs...) + errCode, err := pMgr.runIPTablesCommand(util.IptablesDeletionFlag, specs...) + if err != nil && errCode != couldntLoadTargetErrorCode { + errorString := fmt.Sprintf("failed to delete jump from %s chain to %s chain for policy %s with exit code %d", baseChainName, chainName, policy.Name, errCode) + log.Errorf(errorString+": %w", err) + return npmerrors.SimpleErrorWrapper(errorString, err) + } return nil } + +func getIngressJumpSpecs(networkPolicy *NPMNetworkPolicy) []string { + chainName := networkPolicy.getIngressChainName() + specs := []string{util.IptablesJumpFlag, chainName} + return append(specs, getMatchSetSpecsForNetworkPolicy(networkPolicy, DstMatch)...) +} + +func getEgressJumpSpecs(networkPolicy *NPMNetworkPolicy) []string { + chainName := networkPolicy.getEgressChainName() + specs := []string{util.IptablesJumpFlag, chainName} + return append(specs, getMatchSetSpecsForNetworkPolicy(networkPolicy, SrcMatch)...) +} + +// noflush add to chains impacted +func (pMgr *PolicyManager) getCreatorForNewNetworkPolicies(networkPolicies ...*NPMNetworkPolicy) *ioutil.FileCreator { + allChainNames := getAllChainNames(networkPolicies) + creator := pMgr.getNewCreatorWithChains(allChainNames) + + ingressJumpLineNumber := 1 + egressJumpLineNumber := 1 + for _, networkPolicy := range networkPolicies { + writeNetworkPolicyRules(creator, networkPolicy) + + // add jump rule(s) to policy chain(s) + hasIngress, hasEgress := networkPolicy.hasIngressAndEgress() + if hasIngress { + ingressJumpSpecs := getInsertSpecs(util.IptablesAzureIngressChain, ingressJumpLineNumber, getIngressJumpSpecs(networkPolicy)) + creator.AddLine("", nil, ingressJumpSpecs...) // TODO error handler + ingressJumpLineNumber++ + } + if hasEgress { + egressJumpSpecs := getInsertSpecs(util.IptablesAzureEgressChain, egressJumpLineNumber, getEgressJumpSpecs(networkPolicy)) + creator.AddLine("", nil, egressJumpSpecs...) // TODO error handler + egressJumpLineNumber++ + } + } + creator.AddLine("", nil, util.IptablesRestoreCommit) + return creator +} + +// write rules for the policy chain(s) +func writeNetworkPolicyRules(creator *ioutil.FileCreator, networkPolicy *NPMNetworkPolicy) { + for _, aclPolicy := range networkPolicy.ACLs { + var chainName string + var actionSpecs []string + if aclPolicy.hasIngress() { + chainName = networkPolicy.getIngressChainName() + if aclPolicy.Target == Allowed { + actionSpecs = []string{util.IptablesJumpFlag, util.IptablesAzureEgressChain} + } else { + actionSpecs = getSetMarkSpecs(util.IptablesAzureIngressDropMarkHex) + } + } else { + chainName = networkPolicy.getEgressChainName() + if aclPolicy.Target == Allowed { + actionSpecs = []string{util.IptablesJumpFlag, util.IptablesAzureAcceptChain} + } else { + actionSpecs = getSetMarkSpecs(util.IptablesAzureEgressDropMarkHex) + } + } + line := []string{"-A", chainName} + line = append(line, actionSpecs...) + line = append(line, getIPTablesRuleSpecs(aclPolicy)...) + creator.AddLine("", nil, line...) // TODO add error handler + } +} + +func getIPTablesRuleSpecs(aclPolicy *ACLPolicy) []string { + specs := make([]string, 0) + specs = append(specs, util.IptablesProtFlag, string(aclPolicy.Protocol)) // NOTE: protocol must be ALL instead of nil + specs = append(specs, getPortSpecs(aclPolicy.SrcPorts, false)...) + specs = append(specs, getPortSpecs(aclPolicy.DstPorts, true)...) + specs = append(specs, getMatchSetSpecsFromSetInfo(aclPolicy.SrcList)...) + specs = append(specs, getMatchSetSpecsFromSetInfo(aclPolicy.DstList)...) + if aclPolicy.Comment != "" { + specs = append(specs, getCommentSpecs(aclPolicy.Comment)...) + } + return specs +} + +func getPortSpecs(portRanges []Ports, isDst bool) []string { + if len(portRanges) == 0 { + return []string{} + } + if len(portRanges) == 1 { + portFlag := util.IptablesSrcPortFlag + if isDst { + portFlag = util.IptablesDstPortFlag + } + return []string{portFlag, portRanges[0].toIPTablesString()} + } + + portRangeStrings := make([]string, 0) + for _, portRange := range portRanges { + portRangeStrings = append(portRangeStrings, portRange.toIPTablesString()) + } + portFlag := util.IptablesMultiSrcPortFlag + if isDst { + portFlag = util.IptablesMultiDstPortFlag + } + specs := []string{util.IptablesModuleFlag, util.IptablesMultiportFlag, portFlag} + return append(specs, strings.Join(portRangeStrings, ",")) +} + +func getMatchSetSpecsForNetworkPolicy(networkPolicy *NPMNetworkPolicy, matchType MatchType) []string { + // TODO update to use included boolean/new data structure from Junguk's PR + specs := make([]string, 0, maxLengthForMatchSetSpecs*len(networkPolicy.PodSelectorIPSets)) + for _, translatedIPSet := range networkPolicy.PodSelectorIPSets { + matchString := matchType.toIPTablesString() + hashedSetName := util.GetHashedName(translatedIPSet.Metadata.GetPrefixName()) + specs = append(specs, util.IptablesModuleFlag, util.IptablesSetModuleFlag, util.IptablesMatchSetFlag, hashedSetName, matchString) + } + return specs +} + +func getMatchSetSpecsFromSetInfo(setInfoList []SetInfo) []string { + specs := make([]string, 0, maxLengthForMatchSetSpecs*len(setInfoList)) + for _, setInfo := range setInfoList { + matchString := setInfo.MatchType.toIPTablesString() + specs = append(specs, util.IptablesModuleFlag, util.IptablesSetModuleFlag) + if !setInfo.Included { + specs = append(specs, util.IptablesNotFlag) + } + hashedSetName := util.GetHashedName(setInfo.IPSet.GetPrefixName()) + specs = append(specs, util.IptablesMatchSetFlag, hashedSetName, matchString) + } + return specs +} + +func getSetMarkSpecs(mark string) []string { + return []string{ + util.IptablesJumpFlag, + util.IptablesMark, + util.IptablesSetMarkFlag, + mark, + } +} + +func getCommentSpecs(comment string) []string { + return []string{ + util.IptablesModuleFlag, + util.IptablesCommentModuleFlag, + util.IptablesCommentFlag, + comment, + } +} + +func getInsertSpecs(chainName string, index int, specs []string) []string { + indexString := fmt.Sprint(index) + insertSpecs := []string{util.IptablesInsertionFlag, chainName, indexString} + return append(insertSpecs, specs...) +} + +func joinWithDash(prefix, item string) string { + return fmt.Sprintf("%s-%s", prefix, item) +} diff --git a/npm/pkg/dataplane/policies/policymanager_linux_test.go b/npm/pkg/dataplane/policies/policymanager_linux_test.go new file mode 100644 index 0000000000..1a6b95debd --- /dev/null +++ b/npm/pkg/dataplane/policies/policymanager_linux_test.go @@ -0,0 +1,141 @@ +package policies + +import ( + "fmt" + "strings" + "testing" + + "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" + testutils "github.com/Azure/azure-container-networking/test/utils" + "github.com/stretchr/testify/require" +) + +var ( + testPolicy1IngressChain = TestNetworkPolicies[0].getIngressChainName() + testPolicy1EgressChain = TestNetworkPolicies[0].getEgressChainName() + testPolicy2IngressChain = TestNetworkPolicies[1].getIngressChainName() + testPolicy3EgressChain = TestNetworkPolicies[2].getEgressChainName() + + testPolicy1IngressJump = fmt.Sprintf("-j %s -m set --match-set %s dst", testPolicy1IngressChain, ipsets.TestKVNSList.HashedName) + testPolicy1EgressJump = fmt.Sprintf("-j %s -m set --match-set %s src", testPolicy1EgressChain, ipsets.TestKVNSList.HashedName) + testPolicy2IngressJump = fmt.Sprintf("-j %s -m set --match-set %s dst -m set --match-set %s dst", testPolicy2IngressChain, ipsets.TestKVNSList.HashedName, ipsets.TestKeyPodSet.HashedName) + testPolicy3EgressJump = fmt.Sprintf("-j %s", testPolicy3EgressChain) + + testACLRule1 = fmt.Sprintf( + "-j MARK --set-mark 0x4000 -p tcp --sport 144:255 -m multiport --dports 222:333,456 -m set --match-set %s src -m set ! --match-set %s dst -m comment --comment comment1", + ipsets.TestCIDRSet.HashedName, + ipsets.TestKeyPodSet.HashedName, + ) + testACLRule2 = fmt.Sprintf("-j AZURE-NPM-EGRESS -p udp --sport 144 -m set --match-set %s src -m comment --comment comment2", ipsets.TestCIDRSet.HashedName) + testACLRule3 = fmt.Sprintf("-j MARK --set-mark 0x5000 -p udp --dport 144 -m set --match-set %s src -m comment --comment comment3", ipsets.TestCIDRSet.HashedName) + testACLRule4 = fmt.Sprintf("-j AZURE-NPM-ACCEPT -p all -m set --match-set %s src -m comment --comment comment4", ipsets.TestCIDRSet.HashedName) +) + +func TestAddPolicies(t *testing.T) { + calls := []testutils.TestCmd{fakeIPTablesRestoreCommand} + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + creator := pMgr.getCreatorForNewNetworkPolicies(TestNetworkPolicies...) + fileString := creator.ToString() + expectedLines := []string{ + "*filter", + // all chains + fmt.Sprintf(":%s - -", testPolicy1IngressChain), + fmt.Sprintf(":%s - -", testPolicy1EgressChain), + fmt.Sprintf(":%s - -", testPolicy2IngressChain), + fmt.Sprintf(":%s - -", testPolicy3EgressChain), + // policy 1 + fmt.Sprintf("-A %s %s", testPolicy1IngressChain, testACLRule1), + fmt.Sprintf("-A %s %s", testPolicy1IngressChain, testACLRule2), + fmt.Sprintf("-A %s %s", testPolicy1EgressChain, testACLRule3), + fmt.Sprintf("-A %s %s", testPolicy1EgressChain, testACLRule4), + fmt.Sprintf("-I AZURE-NPM-INGRESS 1 %s", testPolicy1IngressJump), + fmt.Sprintf("-I AZURE-NPM-EGRESS 1 %s", testPolicy1EgressJump), + // policy 2 + fmt.Sprintf("-A %s %s", testPolicy2IngressChain, testACLRule1), + fmt.Sprintf("-I AZURE-NPM-INGRESS 2 %s", testPolicy2IngressJump), + // policy 3 + fmt.Sprintf("-A %s %s", testPolicy3EgressChain, testACLRule4), + fmt.Sprintf("-I AZURE-NPM-EGRESS 2 %s", testPolicy3EgressJump), + "COMMIT\n", + } + expectedFileString := strings.Join(expectedLines, "\n") + dptestutils.AssertEqualMultilineStrings(t, expectedFileString, fileString) + + err := pMgr.addPolicy(TestNetworkPolicies[0], nil) + require.NoError(t, err) +} + +func TestAddPoliciesError(t *testing.T) { + calls := []testutils.TestCmd{fakeIPTablesRestoreFailureCommand} + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + err := pMgr.addPolicy(TestNetworkPolicies[0], nil) + require.Error(t, err) +} + +func TestRemovePolicies(t *testing.T) { + calls := []testutils.TestCmd{ + fakeIPTablesRestoreCommand, + getFakeDeleteJumpCommand("AZURE-NPM-INGRESS", testPolicy1IngressJump), + getFakeDeleteJumpCommandWithCode("AZURE-NPM-EGRESS", testPolicy1EgressJump, 2), // if the policy chain doesn't exist, we shouldn't error + fakeIPTablesRestoreCommand, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + creator := pMgr.getCreatorForRemovingPolicies(TestNetworkPolicies...) + fileString := creator.ToString() + expectedLines := []string{ + "*filter", + fmt.Sprintf(":%s - -", testPolicy1IngressChain), + fmt.Sprintf(":%s - -", testPolicy1EgressChain), + fmt.Sprintf(":%s - -", testPolicy2IngressChain), + fmt.Sprintf(":%s - -", testPolicy3EgressChain), + "COMMIT\n", + } + expectedFileString := strings.Join(expectedLines, "\n") + dptestutils.AssertEqualMultilineStrings(t, expectedFileString, fileString) + + err := pMgr.AddPolicy(TestNetworkPolicies[0], nil) // need the policy in the cache + require.NoError(t, err) + err = pMgr.RemovePolicy(TestNetworkPolicies[0].Name, nil) + require.NoError(t, err) +} + +func TestRemovePoliciesErrorOnRestore(t *testing.T) { + calls := []testutils.TestCmd{ + fakeIPTablesRestoreCommand, + getFakeDeleteJumpCommand("AZURE-NPM-INGRESS", testPolicy1IngressJump), + getFakeDeleteJumpCommand("AZURE-NPM-EGRESS", testPolicy1EgressJump), + fakeIPTablesRestoreFailureCommand, + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + err := pMgr.AddPolicy(TestNetworkPolicies[0], nil) + require.NoError(t, err) + err = pMgr.RemovePolicy(TestNetworkPolicies[0].Name, nil) + require.Error(t, err) +} + +func TestRemovePoliciesErrorOnIngressRule(t *testing.T) { + calls := []testutils.TestCmd{ + fakeIPTablesRestoreCommand, + getFakeDeleteJumpCommandWithCode("AZURE-NPM-INGRESS", testPolicy1IngressJump, 1), // anything but 0 or 2 + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + err := pMgr.AddPolicy(TestNetworkPolicies[0], nil) + require.NoError(t, err) + err = pMgr.RemovePolicy(TestNetworkPolicies[0].Name, nil) + require.Error(t, err) +} + +func TestRemovePoliciesErrorOnEgressRule(t *testing.T) { + calls := []testutils.TestCmd{ + fakeIPTablesRestoreCommand, + getFakeDeleteJumpCommand("AZURE-NPM-INGRESS", testPolicy1IngressJump), + getFakeDeleteJumpCommandWithCode("AZURE-NPM-EGRESS", testPolicy1EgressJump, 1), // anything but 0 or 2 + } + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) + err := pMgr.AddPolicy(TestNetworkPolicies[0], nil) + require.NoError(t, err) + err = pMgr.RemovePolicy(TestNetworkPolicies[0].Name, nil) + require.Error(t, err) +} diff --git a/npm/pkg/dataplane/policies/policymanager_test.go b/npm/pkg/dataplane/policies/policymanager_test.go index d42b66eb82..038e717f61 100644 --- a/npm/pkg/dataplane/policies/policymanager_test.go +++ b/npm/pkg/dataplane/policies/policymanager_test.go @@ -1,11 +1,12 @@ package policies import ( + "fmt" "testing" "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" - testutils "github.com/Azure/azure-container-networking/test/utils" + "github.com/stretchr/testify/require" ) var ( @@ -13,7 +14,7 @@ var ( epList = map[string]string{"10.0.0.1": "test123", "10.0.0.2": "test456"} testNSSet = ipsets.NewIPSetMetadata("test-ns-set", ipsets.Namespace) testKeyPodSet = ipsets.NewIPSetMetadata("test-keyPod-set", ipsets.KeyLabelOfPod) - testNetPol = NPMNetworkPolicy{ + testNetPol = &NPMNetworkPolicy{ Name: "test/test-netpol", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ { @@ -45,12 +46,12 @@ var ( { IPSet: testNSSet, Included: true, - MatchType: "src", + MatchType: SrcMatch, }, { IPSet: testKeyPodSet, Included: true, - MatchType: "src", + MatchType: SrcMatch, }, }, }, @@ -62,24 +63,18 @@ var ( ) func TestAddPolicy(t *testing.T) { - pMgr := NewPolicyManager(common.NewMockIOShim([]testutils.TestCmd{})) + netpol := &NPMNetworkPolicy{} - netpol := NPMNetworkPolicy{} + calls := GetAddPolicyTestCalls(netpol) + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) - err := pMgr.AddPolicy(&netpol, epList) - if err != nil { - t.Errorf("AddPolicy() returned error %s", err.Error()) - } + require.NoError(t, pMgr.AddPolicy(netpol, epList)) - err = pMgr.AddPolicy(&testNetPol, epList) - if err != nil { - t.Errorf("AddPolicy() returned error %s", err.Error()) - } + require.NoError(t, pMgr.AddPolicy(testNetPol, epList)) } func TestGetPolicy(t *testing.T) { - pMgr := NewPolicyManager(common.NewMockIOShim([]testutils.TestCmd{})) - netpol := NPMNetworkPolicy{ + netpol := &NPMNetworkPolicy{ Name: "test", ACLs: []*ACLPolicy{ { @@ -90,39 +85,26 @@ func TestGetPolicy(t *testing.T) { }, } - err := pMgr.AddPolicy(&netpol, epList) - if err != nil { - t.Errorf("AddPolicy() returned error %s", err.Error()) - } + calls := GetAddPolicyTestCalls(netpol) + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) - ok := pMgr.PolicyExists("test") - if !ok { - t.Error("PolicyExists() returned false") - } + require.NoError(t, pMgr.AddPolicy(netpol, epList)) - policy, ok := pMgr.GetPolicy("test") - if !ok { - t.Error("GetPolicy() returned false") - } else if policy.Name != "test" { - t.Errorf("GetPolicy() returned wrong policy %s", policy.Name) - } + require.True(t, pMgr.PolicyExists("test")) + policy, ok := pMgr.GetPolicy("test") + require.True(t, ok) + require.Equal(t, "test", policy.Name) } func TestRemovePolicy(t *testing.T) { - pMgr := NewPolicyManager(common.NewMockIOShim([]testutils.TestCmd{})) + calls := append(GetAddPolicyTestCalls(testNetPol), GetRemovePolicyTestCalls(testNetPol)...) + fmt.Println(calls) + pMgr := NewPolicyManager(common.NewMockIOShim(calls)) - err := pMgr.AddPolicy(&testNetPol, epList) - if err != nil { - t.Errorf("AddPolicy() returned error %s", err.Error()) - } + require.NoError(t, pMgr.AddPolicy(testNetPol, epList)) - err = pMgr.RemovePolicy("test", epList) - if err != nil { - t.Errorf("RemovePolicy() returned error %s", err.Error()) - } - err = pMgr.RemovePolicy("test/test-netpol", nil) - if err != nil { - t.Errorf("RemovePolicy() returned error %s", err.Error()) - } + require.NoError(t, pMgr.RemovePolicy("test", epList)) + + require.NoError(t, pMgr.RemovePolicy("test/test-netpol", nil)) } diff --git a/npm/pkg/dataplane/policies/policymanager_windows.go b/npm/pkg/dataplane/policies/policymanager_windows.go index ee7bc600ad..6a56e2d6e3 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows.go +++ b/npm/pkg/dataplane/policies/policymanager_windows.go @@ -20,6 +20,16 @@ type endpointPolicyBuilder struct { otherPolicies []hcn.EndpointPolicy } +func (pMgr *PolicyManager) initialize() error { + // TODO + return nil +} + +func (pMgr *PolicyManager) reset() error { + // TODO + return nil +} + func (pMgr *PolicyManager) addPolicy(policy *NPMNetworkPolicy, endpointList map[string]string) error { klog.Infof("[DataPlane Windows] adding policy %s on %+v", policy.Name, endpointList) if endpointList == nil { @@ -135,11 +145,6 @@ func (pMgr *PolicyManager) removePolicy(policy *NPMNetworkPolicy, endpointList m return aggregateErr } -func (pMgr *PolicyManager) reset() error { - // TODO - return nil -} - // addEPPolicyWithEpID given an EP ID and a list of policies, add the policies to the endpoint func (pMgr *PolicyManager) applyPoliciesToEndpointID(epID string, policies hcn.PolicyEndpointRequest) error { epObj, err := pMgr.getEndpointByID(epID) diff --git a/npm/pkg/dataplane/policies/testutils.go b/npm/pkg/dataplane/policies/testutils.go new file mode 100644 index 0000000000..6519df1273 --- /dev/null +++ b/npm/pkg/dataplane/policies/testutils.go @@ -0,0 +1,111 @@ +package policies + +import "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + +var ( + // TestNetworkPolicies for testing + TestNetworkPolicies = []*NPMNetworkPolicy{ + { + Name: "test1", + PodSelectorIPSets: []*ipsets.TranslatedIPSet{ + {Metadata: ipsets.TestKVNSList.Metadata}, + }, + ACLs: testACLs, + }, + { + Name: "test2", + PodSelectorIPSets: []*ipsets.TranslatedIPSet{ + {Metadata: ipsets.TestKVNSList.Metadata}, + {Metadata: ipsets.TestKeyPodSet.Metadata}, + }, + ACLs: []*ACLPolicy{ + testACLs[0], + }, + }, + { + Name: "test3", + ACLs: []*ACLPolicy{ + testACLs[3], + }, + }, + } + + testACLs = []*ACLPolicy{ + { + PolicyID: "test1", + Comment: "comment1", + SrcList: []SetInfo{ + { + ipsets.TestCIDRSet.Metadata, + true, + SrcMatch, + }, + }, + DstList: []SetInfo{ + { + ipsets.TestKeyPodSet.Metadata, + false, + DstMatch, + }, + }, + Target: Dropped, + Direction: Ingress, + SrcPorts: []Ports{ + {144, 255}, + }, + DstPorts: []Ports{ + {222, 333}, + {456, 456}, + }, + Protocol: TCP, + }, + { + PolicyID: "test2", + Comment: "comment2", + SrcList: []SetInfo{ + { + ipsets.TestCIDRSet.Metadata, + true, + SrcMatch, + }, + }, + Target: Allowed, + Direction: Ingress, + SrcPorts: []Ports{ + {144, 144}, + }, + Protocol: UDP, + }, + { + PolicyID: "test3", + Comment: "comment3", + SrcList: []SetInfo{ + { + ipsets.TestCIDRSet.Metadata, + true, + SrcMatch, + }, + }, + Target: Dropped, + Direction: Egress, + DstPorts: []Ports{ + {144, 144}, + }, + Protocol: UDP, + }, + { + PolicyID: "test4", + Comment: "comment4", + SrcList: []SetInfo{ + { + ipsets.TestCIDRSet.Metadata, + true, + SrcMatch, + }, + }, + Target: Allowed, + Direction: Egress, + Protocol: AnyProtocol, + }, + } +) diff --git a/npm/pkg/dataplane/policies/testutils_linux.go b/npm/pkg/dataplane/policies/testutils_linux.go new file mode 100644 index 0000000000..0710a74d39 --- /dev/null +++ b/npm/pkg/dataplane/policies/testutils_linux.go @@ -0,0 +1,81 @@ +package policies + +import ( + "strings" + + "github.com/Azure/azure-container-networking/npm/util" + testutils "github.com/Azure/azure-container-networking/test/utils" +) + +var ( + fakeIPTablesRestoreCommand = testutils.TestCmd{Cmd: []string{"iptables-restore", "-T", "filter", "--noflush"}} + fakeIPTablesRestoreFailureCommand = testutils.TestCmd{Cmd: []string{"iptables-restore", "-T", "filter", "--noflush"}, ExitCode: 1} + + listLineNumbersCommandStrings = []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L", "FORWARD", "--line-numbers"} + listPolicyChainNamesCommandStrings = []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"} +) + +func GetAddPolicyTestCalls(_ *NPMNetworkPolicy) []testutils.TestCmd { + return []testutils.TestCmd{fakeIPTablesRestoreCommand} +} + +func GetRemovePolicyTestCalls(policy *NPMNetworkPolicy) []testutils.TestCmd { + calls := []testutils.TestCmd{} + hasIngress, hasEgress := policy.hasIngressAndEgress() + if hasIngress { + deleteIngressJumpSpecs := []string{"iptables", "-w", "60", "-D", util.IptablesAzureIngressChain} + deleteIngressJumpSpecs = append(deleteIngressJumpSpecs, getIngressJumpSpecs(policy)...) + calls = append(calls, testutils.TestCmd{Cmd: deleteIngressJumpSpecs}) + } + if hasEgress { + deleteEgressJumpSpecs := []string{"iptables", "-w", "60", "-D", util.IptablesAzureEgressChain} + deleteEgressJumpSpecs = append(deleteEgressJumpSpecs, getEgressJumpSpecs(policy)...) + calls = append(calls, testutils.TestCmd{Cmd: deleteEgressJumpSpecs}) + } + + calls = append(calls, fakeIPTablesRestoreCommand) + return calls +} + +func GetInitializeTestCalls() []testutils.TestCmd { + return []testutils.TestCmd{ + fakeIPTablesRestoreCommand, // gives correct exit code + { + Cmd: listLineNumbersCommandStrings, + ExitCode: 1, // grep call gets this exit code (exit code 1 means grep found nothing) + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + { + Cmd: []string{"grep", "KUBE-SERVICES"}, + Stdout: "iptables: No chain/target/match by that name.", // this Stdout and ExitCode are for the iptables check command below + ExitCode: 1, + }, + {Cmd: []string{"iptables", "-w", "60", "-C", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + {Cmd: []string{"iptables", "-w", "60", "-I", "FORWARD", "1", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + } +} + +func GetResetTestCalls() []testutils.TestCmd { + return []testutils.TestCmd{ + {Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"}}, + { + Cmd: listPolicyChainNamesCommandStrings, + Stdout: "Chain AZURE-NPM-INGRESS-123456\nChain AZURE-NPM-EGRESS-123456", + }, + // NOTE: after the StdOut pipe used for grep, MockIOShim gets confused and each command's ExitCode and Stdout are applied to the ensuing command + {Cmd: []string{"grep", ingressOrEgressPolicyChainPattern}}, // ExitCode 0 for the iptables restore command + fakeIPTablesRestoreCommand, + } +} + +func getFakeDeleteJumpCommand(chainName, jumpRule string) testutils.TestCmd { + args := []string{"iptables", "-w", "60", "-D", chainName} + args = append(args, strings.Split(jumpRule, " ")...) + return testutils.TestCmd{Cmd: args} +} + +func getFakeDeleteJumpCommandWithCode(chainName, jumpRule string, exitCode int) testutils.TestCmd { + command := getFakeDeleteJumpCommand(chainName, jumpRule) + command.ExitCode = exitCode + return command +} diff --git a/npm/pkg/dataplane/policies/testutils_windows.go b/npm/pkg/dataplane/policies/testutils_windows.go new file mode 100644 index 0000000000..4c23aea334 --- /dev/null +++ b/npm/pkg/dataplane/policies/testutils_windows.go @@ -0,0 +1,19 @@ +package policies + +import testutils "github.com/Azure/azure-container-networking/test/utils" + +func GetAddPolicyTestCalls(_ *NPMNetworkPolicy) []testutils.TestCmd { + return []testutils.TestCmd{} +} + +func GetRemovePolicyTestCalls(_ *NPMNetworkPolicy) []testutils.TestCmd { + return []testutils.TestCmd{} +} + +func GetInitializeTestCalls() []testutils.TestCmd { + return []testutils.TestCmd{} +} + +func GetResetTestCalls() []testutils.TestCmd { + return []testutils.TestCmd{} +} diff --git a/npm/pkg/dataplane/testutils/file-comparison.go b/npm/pkg/dataplane/testutils/file-comparison.go new file mode 100644 index 0000000000..bd7d1ca76a --- /dev/null +++ b/npm/pkg/dataplane/testutils/file-comparison.go @@ -0,0 +1,36 @@ +package dptestutils + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func AssertEqualMultilineStrings(t *testing.T, expectedMultilineString, actualMultilineString string) { + if expectedMultilineString == actualMultilineString { + return + } + fmt.Println("EXPECTED FILE STRING:") + expectedLines := strings.Split(expectedMultilineString, "\n") + for _, line := range expectedLines { + fmt.Println(line) + } + fmt.Println("ACTUAL FILE STRING") + actualLines := strings.Split(actualMultilineString, "\n") + for _, line := range actualLines { + fmt.Println(line) + } + if len(expectedLines) != len(actualLines) { + fmt.Printf("expected %d lines, got %d\n", len(expectedLines), len(actualLines)) + } + for k, expectedLine := range expectedLines { + line := actualLines[k] + if expectedLine != line { + fmt.Printf("expected the next line, but got the one below it:\n%s\n%s\n", expectedLine, line) + break + } + } + require.FailNow(t, "got unexpected file string (see print contents above)") +} diff --git a/npm/util/const.go b/npm/util/const.go index ebb2453a94..fd37e1f8da 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -21,64 +21,86 @@ const ( // iptables related constants. const ( - Iptables string = "iptables" - Ip6tables string = "ip6tables" - IptablesSave string = "iptables-save" - IptablesRestore string = "iptables-restore" - IptablesConfigFile string = "/var/log/iptables.conf" - IptablesTestConfigFile string = "/var/log/iptables-test.conf" - IptablesLockFile string = "/run/xtables.lock" - IptablesChainCreationFlag string = "-N" - IptablesInsertionFlag string = "-I" - IptablesAppendFlag string = "-A" - IptablesDeletionFlag string = "-D" - IptablesFlushFlag string = "-F" - IptablesCheckFlag string = "-C" - IptablesDestroyFlag string = "-X" - IptablesJumpFlag string = "-j" - IptablesWaitFlag string = "-w" - IptablesAccept string = "ACCEPT" - IptablesReject string = "REJECT" - IptablesDrop string = "DROP" - IptablesReturn string = "RETURN" - IptablesMark string = "MARK" - IptablesSrcFlag string = "src" - IptablesDstFlag string = "dst" - IptablesNotFlag string = "!" - IptablesProtFlag string = "-p" - IptablesSFlag string = "-s" - IptablesDFlag string = "-d" - IptablesDstPortFlag string = "--dport" - IptablesModuleFlag string = "-m" - IptablesSetModuleFlag string = "set" - IptablesMatchSetFlag string = "--match-set" - IptablesSetMarkFlag string = "--set-mark" - IptablesMarkFlag string = "--mark" - IptablesMarkVerb string = "mark" - IptablesStateModuleFlag string = "state" - IptablesStateFlag string = "--state" - IptablesMultiportFlag string = "multiport" - IptablesMultiDestportFlag string = "--dports" - IptablesRelatedState string = "RELATED" - IptablesEstablishedState string = "ESTABLISHED" - IptablesFilterTable string = "filter" - IptablesCommentModuleFlag string = "comment" - IptablesCommentFlag string = "--comment" + Iptables string = "iptables" + Ip6tables string = "ip6tables" //nolint (avoid warning to capitalize this p) + IptablesSave string = "iptables-save" + IptablesRestore string = "iptables-restore" + IptablesRestoreNoFlushFlag string = "--noflush" + IptablesRestoreTableFlag string = "-T" + IptablesRestoreCommit string = "COMMIT" + IptablesConfigFile string = "/var/log/iptables.conf" + IptablesTestConfigFile string = "/var/log/iptables-test.conf" + IptablesLockFile string = "/run/xtables.lock" + IptablesChainCreationFlag string = "-N" + IptablesInsertionFlag string = "-I" + IptablesAppendFlag string = "-A" + IptablesDeletionFlag string = "-D" + IptablesFlushFlag string = "-F" + IptablesCheckFlag string = "-C" + IptablesDestroyFlag string = "-X" + IptablesJumpFlag string = "-j" + IptablesWaitFlag string = "-w" + IptablesAccept string = "ACCEPT" + IptablesReject string = "REJECT" + IptablesDrop string = "DROP" + IptablesReturn string = "RETURN" + IptablesMark string = "MARK" + IptablesSrcFlag string = "src" + IptablesDstFlag string = "dst" + IptablesNotFlag string = "!" + IptablesProtFlag string = "-p" + IptablesSFlag string = "-s" + IptablesDFlag string = "-d" + IptablesDstPortFlag string = "--dport" + IptablesSrcPortFlag string = "--sport" + IptablesModuleFlag string = "-m" + IptablesSetModuleFlag string = "set" + IptablesMatchSetFlag string = "--match-set" + IptablesSetMarkFlag string = "--set-mark" + IptablesMarkFlag string = "--mark" + IptablesMarkVerb string = "mark" + IptablesStateModuleFlag string = "state" + IptablesStateFlag string = "--state" + IptablesCtstateModuleFlag string = "conntrack" // state module is obsolete: https://unix.stackexchange.com/questions/108169/what-is-the-difference-between-m-conntrack-ctstate-and-m-state-state + IptablesCtstateFlag string = "--ctstate" + IptablesMultiportFlag string = "multiport" + IptablesMultiDstPortFlag string = "--dports" + IptablesMultiSrcPortFlag string = "--sports" + IptablesRelatedState string = "RELATED" + IptablesEstablishedState string = "ESTABLISHED" + IptablesNewState string = "NEW" + IptablesFilterTable string = "filter" + IptablesCommentModuleFlag string = "comment" + IptablesCommentFlag string = "--comment" IptablesAddCommentFlag - IptablesAzureChain string = "AZURE-NPM" - IptablesAzureAcceptChain string = "AZURE-NPM-ACCEPT" - IptablesAzureKubeSystemChain string = "AZURE-NPM-KUBE-SYSTEM" - IptablesAzureIngressChain string = "AZURE-NPM-INGRESS" + + IptablesTableFlag string = "-t" + IptablesListFlag string = "-L" + IptablesNumericFlag string = "-n" + IptablesLineNumbersFlag string = "--line-numbers" + + IptablesKubeServicesChain string = "KUBE-SERVICES" + IptablesForwardChain string = "FORWARD" + IptablesInputChain string = "INPUT" + IptablesAzureChain string = "AZURE-NPM" + IptablesAzureAcceptChain string = "AZURE-NPM-ACCEPT" + IptablesAzureKubeSystemChain string = "AZURE-NPM-KUBE-SYSTEM" + IptablesAzureIngressChain string = "AZURE-NPM-INGRESS" + IptablesAzureIngressAllowMarkChain string = "AZURE-NPM-INGRESS-ALLOW-MARK" + IptablesAzureEgressChain string = "AZURE-NPM-EGRESS" + + // Chains used in NPM v1 IptablesAzureIngressPortChain string = "AZURE-NPM-INGRESS-PORT" IptablesAzureIngressFromChain string = "AZURE-NPM-INGRESS-FROM" - IptablesAzureEgressChain string = "AZURE-NPM-EGRESS" IptablesAzureEgressPortChain string = "AZURE-NPM-EGRESS-PORT" IptablesAzureEgressToChain string = "AZURE-NPM-EGRESS-TO" - IptablesKubeServicesChain string = "KUBE-SERVICES" - IptablesForwardChain string = "FORWARD" - IptablesInputChain string = "INPUT" IptablesAzureIngressDropsChain string = "AZURE-NPM-INGRESS-DROPS" IptablesAzureEgressDropsChain string = "AZURE-NPM-EGRESS-DROPS" + + // NPM v2 Chains + IptablesAzureIngressPolicyChainPrefix string = "AZURE-NPM-INGRESS" + IptablesAzureEgressPolicyChainPrefix string = "AZURE-NPM-EGRESS" + // Below chain exists only in NPM before v1.2.6 // TODO delete this below set while cleaning up IptablesAzureTargetSetsChain string = "AZURE-NPM-TARGET-SETS" @@ -90,7 +112,16 @@ const ( IptablesAzureIngressFromPodChain string = "AZURE-NPM-INGRESS-FROM-POD" IptablesAzureEgressToNsChain string = "AZURE-NPM-EGRESS-TO-NS" IptablesAzureEgressToPodChain string = "AZURE-NPM-EGRESS-TO-POD" + // Below are the skb->mark NPM will use for different criteria + IptablesAzureClearMarkHex string = "0x0" + + // marks in NPM v2 + IptablesAzureIngressAllowMarkHex string = "0x2000" // same as old IptablesAzureIngressMarkHex + IptablesAzureIngressDropMarkHex string = "0x4000" + IptablesAzureEgressDropMarkHex string = "0x5000" + + // marks in NPM v1 IptablesAzureIngressMarkHex string = "0x2000" // IptablesAzureEgressXMarkHex is used for us to not override but append to the existing MARK // https://unix.stackexchange.com/a/283455 comment contains the explanation on @@ -99,8 +130,6 @@ const ( // IptablesAzureEgressMarkHex is for checking the absolute value of the mark IptablesAzureEgressMarkHex string = "0x1000" IptablesAzureAcceptMarkHex string = "0x3000" - IptablesAzureClearMarkHex string = "0x0" - IptablesTableFlag string = "-t" ) // ipset related constants. diff --git a/npm/util/errors/errors.go b/npm/util/errors/errors.go index c667d3d6ae..175c67f887 100644 --- a/npm/util/errors/errors.go +++ b/npm/util/errors/errors.go @@ -42,6 +42,11 @@ var ( // Error labels for ipsetmanager const ( + InitializeDataPlane = "InitializeDataPlane" + ResetDataPlane = "ResetDataPlane" + InitializePolicyMgr = "InitializePolicyManager" + ResetPolicyMgr = "ResetPolicyManager" + ResetIPSets = "ResetIPSets" CreateIPSet = "CreateIPSet" AppendIPSet = "AppendIPSet" DeleteIPSet = "DeleteIPSet" @@ -49,6 +54,7 @@ const ( TestIPSet = "TestIPSet" IPSetIntersection = "IPSetIntersection" AddPolicy = "AddNetworkPolicy" + RemovePolicy = "RemovePolicy" GetSelectorReference = "GetSelectorReference" AddSelectorReference = "AddSelectorReference" DeleteSelectorReference = "DeleteSelectorReference" @@ -125,6 +131,16 @@ func Errorf(operation string, isRetriable bool, errstring string) *NPMError { } } +func ErrorWrapper(operation string, isRetriable bool, errstring string, err error) *NPMError { + return &NPMError{ + OperationAction: operation, + IsRetriable: false, + FullCmd: []string{}, + ErrID: Unknown, + Err: fmt.Errorf("%s: %w", errstring, err), + } +} + func Error(operation string, isRetriable bool, err error) *NPMError { return &NPMError{ OperationAction: operation, @@ -169,3 +185,20 @@ type npmErrorRetrySettings struct { func (n *NPMError) Error() string { return fmt.Sprintf("Operation [%s] failed with error code [%v], full cmd %v, full error %v", n.OperationAction, n.ErrID, n.FullCmd, n.Err) } + +// NPMSimpleError and its methods are used to appease go lint +type NPMSimpleError struct { + Err error +} + +func SimpleError(errstring string) *NPMSimpleError { + return nil +} + +func SimpleErrorWrapper(errstring string, err error) *NPMSimpleError { + return &NPMSimpleError{fmt.Errorf("%s: %w", errstring, err)} +} + +func (n *NPMSimpleError) Error() string { + return n.Err.Error() +} diff --git a/test/integration/npm/main.go b/test/integration/npm/main.go index 930e7053e9..0f0320ac97 100644 --- a/test/integration/npm/main.go +++ b/test/integration/npm/main.go @@ -8,53 +8,28 @@ import ( "github.com/Azure/azure-container-networking/npm/pkg/dataplane" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" - "github.com/Azure/azure-container-networking/npm/util" ) const MaxSleepTime = 15 -type testSet struct { - metadata *ipsets.IPSetMetadata - hashedName string -} - -func createTestSet(name string, setType ipsets.SetType) *testSet { - set := &testSet{ - metadata: &ipsets.IPSetMetadata{ - Name: name, - Type: setType, - }, - } - set.hashedName = util.GetHashedName(set.metadata.GetPrefixName()) - return set -} - var ( - nodeName = "testNode" - testNSSet = createTestSet("test-ns-set", ipsets.Namespace) - testKeyPodSet = createTestSet("test-keyPod-set", ipsets.KeyLabelOfPod) - testKVPodSet = createTestSet("test-kvPod-set", ipsets.KeyValueLabelOfPod) - testNamedportSet = createTestSet("test-namedport-set", ipsets.NamedPorts) - testCIDRSet = createTestSet("test-cidr-set", ipsets.CIDRBlocks) - testKeyNSList = createTestSet("test-keyNS-list", ipsets.KeyLabelOfNamespace) - testKVNSList = createTestSet("test-kvNS-list", ipsets.KeyValueLabelOfNamespace) - testNestedLabelList = createTestSet("test-nestedlabel-list", ipsets.NestedLabelOfPod) - testNetPol = &policies.NPMNetworkPolicy{ + nodeName = "testNode" + testNetPol = &policies.NPMNetworkPolicy{ Name: "test/test-netpol", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ { - Metadata: testNSSet.metadata, + Metadata: ipsets.TestNSSet.Metadata, }, { - Metadata: testKeyPodSet.metadata, + Metadata: ipsets.TestKeyPodSet.Metadata, }, }, RuleIPSets: []*ipsets.TranslatedIPSet{ { - Metadata: testNSSet.metadata, + Metadata: ipsets.TestNSSet.Metadata, }, { - Metadata: testKeyPodSet.metadata, + Metadata: ipsets.TestKeyPodSet.Metadata, }, }, ACLs: []*policies.ACLPolicy{ @@ -69,29 +44,24 @@ var ( Direction: policies.Ingress, SrcList: []policies.SetInfo{ { - IPSet: testNSSet.metadata, + IPSet: ipsets.TestNSSet.Metadata, Included: true, - MatchType: "src", + MatchType: policies.SrcMatch, }, { - IPSet: testKeyPodSet.metadata, + IPSet: ipsets.TestKeyPodSet.Metadata, Included: true, - MatchType: "src", + MatchType: policies.SrcMatch, }, }, }, }, } - // testKeyNSList = createTestSet("test-keyNS-list", ipsets.KeyLabelOfNameSpace) - // testKVNSList = createTestSet("test-kvNS-list", ipsets.KeyValueLabelOfNameSpace) - // testNestedLabelList = createTestSet("test-nestedlabel-list", ipsets.NestedLabelOfPod) ) func main() { dp, err := dataplane.NewDataPlane(nodeName, common.NewIOShim()) - if err != nil { - panic(err) - } + panicOnError(err) printAndWait() podMetadata := &dataplane.PodMetadata{ @@ -101,63 +71,73 @@ func main() { } // add all types of ipsets, some with members added - if err := dp.AddToSets([]*ipsets.IPSetMetadata{testNSSet.metadata}, podMetadata); err != nil { - panic(err) - } + panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestNSSet.Metadata}, podMetadata)) podMetadataB := &dataplane.PodMetadata{ PodKey: "b", PodIP: "10.0.0.1", NodeName: "", } - if err := dp.AddToSets([]*ipsets.IPSetMetadata{testNSSet.metadata}, podMetadataB); err != nil { - panic(err) - } + panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestNSSet.Metadata}, podMetadataB)) podMetadataC := &dataplane.PodMetadata{ PodKey: "c", PodIP: "10.240.0.24", NodeName: nodeName, } - if err := dp.AddToSets([]*ipsets.IPSetMetadata{testKeyPodSet.metadata, testNSSet.metadata}, podMetadataC); err != nil { - panic(err) - } - dp.CreateIPSets([]*ipsets.IPSetMetadata{testKVPodSet.metadata, testNamedportSet.metadata, testCIDRSet.metadata}) + panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestKeyPodSet.Metadata, ipsets.TestNSSet.Metadata}, podMetadataC)) + dp.CreateIPSets([]*ipsets.IPSetMetadata{ipsets.TestKVPodSet.Metadata, ipsets.TestNamedportSet.Metadata, ipsets.TestCIDRSet.Metadata}) // can't do lists on my computer - if err := dp.ApplyDataPlane(); err != nil { - panic(err) - } + panicOnError(dp.ApplyDataPlane()) printAndWait() - if err := dp.AddToLists([]*ipsets.IPSetMetadata{testKeyNSList.metadata, testKVNSList.metadata}, []*ipsets.IPSetMetadata{testNSSet.metadata}); err != nil { - panic(err) - } + panicOnError(dp.AddToLists([]*ipsets.IPSetMetadata{ipsets.TestKeyNSList.Metadata, ipsets.TestKVNSList.Metadata}, []*ipsets.IPSetMetadata{ipsets.TestNSSet.Metadata})) - if err := dp.AddToLists([]*ipsets.IPSetMetadata{testNestedLabelList.metadata}, []*ipsets.IPSetMetadata{testKVPodSet.metadata, testKeyPodSet.metadata}); err != nil { - panic(err) - } + panicOnError(dp.AddToLists([]*ipsets.IPSetMetadata{ipsets.TestNestedLabelList.Metadata}, []*ipsets.IPSetMetadata{ipsets.TestKVPodSet.Metadata, ipsets.TestKeyPodSet.Metadata})) // remove members from some sets and delete some sets - if err := dp.RemoveFromSets([]*ipsets.IPSetMetadata{testNSSet.metadata}, podMetadataB); err != nil { - panic(err) - } - dp.DeleteIPSet(testKVPodSet.metadata) - if err := dp.ApplyDataPlane(); err != nil { - panic(err) - } + panicOnError(dp.RemoveFromSets([]*ipsets.IPSetMetadata{ipsets.TestNSSet.Metadata}, podMetadataB)) + dp.DeleteIPSet(ipsets.TestKVPodSet.Metadata) + panicOnError(dp.ApplyDataPlane()) printAndWait() - if err := dp.RemoveFromSets([]*ipsets.IPSetMetadata{testNSSet.metadata}, podMetadata); err != nil { - panic(err) - } - dp.DeleteIPSet(testNSSet.metadata) - if err := dp.ApplyDataPlane(); err != nil { - panic(err) - } + panicOnError(dp.RemoveFromSets([]*ipsets.IPSetMetadata{ipsets.TestNSSet.Metadata}, podMetadata)) + + dp.DeleteIPSet(ipsets.TestNSSet.Metadata) + panicOnError(dp.ApplyDataPlane()) + printAndWait() + + panicOnError(dp.AddPolicy(testNetPol)) + + testPolicyManager() +} + +func testPolicyManager() { + pMgr := policies.NewPolicyManager(common.NewIOShim()) + + panicOnError(pMgr.Reset()) printAndWait() - if err := dp.AddPolicy(testNetPol); err != nil { + panicOnError(pMgr.AddPolicy(policies.TestNetworkPolicies[0], nil)) + printAndWait() + + panicOnError(pMgr.AddPolicy(policies.TestNetworkPolicies[1], nil)) + printAndWait() + + // remove something that doesn't exist + panicOnError(pMgr.RemovePolicy(policies.TestNetworkPolicies[2].Name, nil)) + printAndWait() + + panicOnError(pMgr.AddPolicy(policies.TestNetworkPolicies[2], nil)) + printAndWait() + + // remove something that exists + panicOnError(pMgr.RemovePolicy(policies.TestNetworkPolicies[1].Name, nil)) +} + +func panicOnError(err error) { + if err != nil { panic(err) } } @@ -169,49 +149,3 @@ func printAndWait() { time.Sleep(time.Second) } } - -// NOTE for Linux -/* - ipset test SETNAME ENTRYNAME: - Warning: 10.0.0.5 is in set azure-npm-2031808719. - 10.0.0.4 is NOT in set azure-npm-2031808719. - - ipset list (references are from setlist or iptables): - Name: azure-npm-3382169694 - Type: hash:net - Revision: 6 - Header: family inet hashsize 1024 maxelem 65536 - Size in memory: 512 - References: 0 - Number of entries: 1 - Members: - 10.0.0.0 - - Name: azure-npm-2031808719 - Type: hash:net - Revision: 6 - Header: family inet hashsize 1024 maxelem 65536 - Size in memory: 512 - References: 0 - Number of entries: 1 - Members: - 10.0.0.5 - - Name: azure-npm-164288419 - Type: hash:ip,port - Revision: 5 - Header: family inet hashsize 1024 maxelem 65536 - Size in memory: 192 - References: 0 - Number of entries: 0 - Members: - - Name: azure-npm-3216600258 - Type: hash:net - Revision: 6 - Header: family inet hashsize 1024 maxelem 4294967295 - Size in memory: 448 - References: 0 - Number of entries: 0 - Members: -*/