diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index 85d5b34831..7d38f627a0 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -42,6 +42,7 @@ func (setMetadata *IPSetMetadata) GetHashedName() string { return util.GetHashedName(prefixedName) } +// TODO join with colon instead of dash for easier readability? func (setMetadata *IPSetMetadata) GetPrefixName() string { switch setMetadata.Type { case CIDRBlocks: diff --git a/npm/pkg/dataplane/ipsets/testutils.go b/npm/pkg/dataplane/ipsets/testutils.go index 0224b0c2f2..2f32bf2844 100644 --- a/npm/pkg/dataplane/ipsets/testutils.go +++ b/npm/pkg/dataplane/ipsets/testutils.go @@ -2,6 +2,8 @@ package ipsets import "github.com/Azure/azure-container-networking/npm/util" +// TODO deprecate the TestSet type and replace TestNSSet etc. with just their metadata +// since you can get prefix name and hashed name with metadata methods type TestSet struct { Metadata *IPSetMetadata PrefixName string diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index 030890af84..e306f99314 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -2,7 +2,6 @@ package policies import ( "fmt" - "strconv" "strings" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" @@ -69,6 +68,7 @@ type ACLPolicy struct { const policyIDPrefix = "azure-acl" +// FIXME this impacts windows DP if it isn't equivalent to netPol.PolicyKey // aclPolicyID returns azure-acl-- format // to differentiate ACLs among different network policies, // but aclPolicy in the same network policy has the same aclPolicyID. @@ -76,6 +76,7 @@ func aclPolicyID(policyNS, policyName string) string { return fmt.Sprintf("%s-%s-%s", policyIDPrefix, policyNS, policyName) } +// TODO make this a method of NPMNetworkPolicy, and just use netPol.PolicyKey as the PolicyID func NewACLPolicy(policyNS, policyName string, target Verdict, direction Direction) *ACLPolicy { acl := &ACLPolicy{ PolicyID: aclPolicyID(policyNS, policyName), @@ -132,7 +133,19 @@ func (aclPolicy *ACLPolicy) hasKnownTarget() bool { } func (aclPolicy *ACLPolicy) satisifiesPortAndProtocolConstraints() bool { - return (aclPolicy.Protocol != UnspecifiedProtocol) || (aclPolicy.DstPorts.Port == 0 && aclPolicy.DstPorts.EndPort == 0) + // namedports handle protocol constraints + return (aclPolicy.hasNamedPort() && aclPolicy.Protocol == UnspecifiedProtocol) || + aclPolicy.Protocol != UnspecifiedProtocol || + aclPolicy.DstPorts.isUnspecified() +} + +func (aclPolicy *ACLPolicy) hasNamedPort() bool { + for _, peer := range aclPolicy.DstList { + if peer.IPSet.Type == ipsets.NamedPorts { + return true + } + } + return false } func (netPol *NPMNetworkPolicy) String() string { @@ -221,13 +234,8 @@ 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 +func (portRange *Ports) isUnspecified() bool { + return portRange.Port == 0 } type Direction string diff --git a/npm/pkg/dataplane/policies/policy_linux.go b/npm/pkg/dataplane/policies/policy_linux.go index 5a1902ae81..4e8d271b85 100644 --- a/npm/pkg/dataplane/policies/policy_linux.go +++ b/npm/pkg/dataplane/policies/policy_linux.go @@ -1,6 +1,14 @@ package policies -import "github.com/Azure/azure-container-networking/npm/util" +import ( + "fmt" + "strconv" + "strings" + + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + "github.com/Azure/azure-container-networking/npm/util" + "k8s.io/klog" +) // returns two booleans indicating whether the network policy has ingress and egress respectively func (networkPolicy *NPMNetworkPolicy) hasIngressAndEgress() (hasIngress, hasEgress bool) { @@ -25,3 +33,212 @@ func (networkPolicy *NPMNetworkPolicy) chainName(prefix string) string { policyHash := util.Hash(networkPolicy.PolicyKey) return joinWithDash(prefix, policyHash) } + +type UniqueDirection bool + +const ( + forIngress UniqueDirection = true + forEgress UniqueDirection = false +) + +func (networkPolicy *NPMNetworkPolicy) commentForJumpToIngress() string { + return networkPolicy.commentForJump(forIngress) +} + +func (networkPolicy *NPMNetworkPolicy) commentForJumpToEgress() string { + return networkPolicy.commentForJump(forEgress) +} + +func (networkPolicy *NPMNetworkPolicy) commentForJump(direction UniqueDirection) string { + prefix := "EGRESS" + if direction == forIngress { + prefix = "INGRESS" + } + + toFrom := "FROM" + if direction == forIngress { + toFrom = "TO" + } + + podSelectorComment := "all" + if len(networkPolicy.PodSelectorList) > 0 { + podSelectorComment = commentForInfos(networkPolicy.PodSelectorList) + } + return fmt.Sprintf("%s-POLICY-%s-%s-%s-IN-ns-%s", prefix, networkPolicy.PolicyKey, toFrom, podSelectorComment, networkPolicy.NameSpace) +} + +func commentForInfos(infos []SetInfo) string { + infoComments := make([]string, 0, len(infos)) + for _, info := range infos { + infoComments = append(infoComments, info.comment()) + } + return strings.Join(infoComments, "-AND-") +} + +func (info SetInfo) comment() string { + name := info.IPSet.GetPrefixName() + if info.Included { + return name + } + return "!" + name +} + +func (aclPolicy *ACLPolicy) comment() string { + // cleanPeerList contains peers that aren't namedports + var cleanPeerList []SetInfo + // TODO remove the if check and initialize the list like in egress once we replace SrcList and DstList with PeerList + if aclPolicy.Direction == Ingress { + cleanPeerList = aclPolicy.SrcList + } else { + cleanPeerList = make([]SetInfo, 0, len(aclPolicy.DstList)) + } + + var namedPortPeer SetInfo + foundNamedPortPeer := false + for _, info := range aclPolicy.DstList { + if info.IPSet.Type == ipsets.NamedPorts { + if foundNamedPortPeer { + klog.Errorf("while creating ACL comment, unexpectedly found more than one namedPort peer for ACL:\n%s", aclPolicy.String()) + } + namedPortPeer = info + foundNamedPortPeer = true + } else if aclPolicy.Direction == Egress { + // TODO remove the if check once we replace SrcList and DstList with PeerList + cleanPeerList = append(cleanPeerList, info) + } + } + + builder := strings.Builder{} + if aclPolicy.Target == Allowed { + builder.WriteString("ALLOW") + } else { + builder.WriteString("DROP") + } + + if len(cleanPeerList) == 0 { + builder.WriteString("-ALL") + } else { + if aclPolicy.Direction == Ingress { + builder.WriteString("-FROM-") + } else { + builder.WriteString("-TO-") + } + builder.WriteString(commentForInfos(cleanPeerList)) + } + + builder.WriteString(aclPolicy.Protocol.comment()) + builder.WriteString(aclPolicy.DstPorts.comment()) + if foundNamedPortPeer { + builder.WriteString("-TO-" + namedPortPeer.comment()) + } + return builder.String() +} + +func (proto Protocol) comment() string { + if proto == UnspecifiedProtocol { + return "" + } + return fmt.Sprintf("-ON-%s", string(proto)) +} + +func (portRange *Ports) comment() string { + if portRange.Port == 0 { + return "" + } + if portRange.Port >= portRange.EndPort { + return fmt.Sprintf("-TO-PORT-%d", portRange.Port) + } + return fmt.Sprintf("-TO-PORT-%d:%d", 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 +} + +/* + Notes on commenting + + v1 overall: + "[value]" means include if needed e.g. if a port is specified + - prefix: + - no to/from rules and not allowing external: + - allowed: "ALLOW-ALL" + - denied: "DROP-ALL" + - otherwise: drop the "ALL" + - suffix: + - no to/from rules or port rules and not allowing external: + - ingress: "-FROM-all-namespaces" + - egress: "-TO-all-namespaces" + - otherwise: "" (no suffix) + - append these to each other to form the whole comment: + prefix + [-cidrIPSetName] + [-AND-nsSelectorComment] + [-AND] + [-podSelectorComment] + [-protocolComment] + [-portComment] + -TO (or "-FROM" if egress) + -targetSelectorComment + suffix + + v2 overall for ACLs: + - prefix: + - allowed: + - no IPSets in SrcList/DstList (i.e. allow external): "ALLOW-ALL" + - otherwise: + - ingress: "ALLOW-FROM" + - egress: "ALLOW-TO" + - denied: replace "ALLOW" with "DROP" + - similar idea (think there are at most two non-namedPort ipsets e.g. ns selector and pod selector): + prefix + [-ipset1Name] + [-AND] + [-ipset2Name] + [-ON-protocolComment] + [-TO-namedPortIPSetName] + [-TO-portComment] + NOTE: can have none or only one of namedPort and port (range) + + v2 for jumps to ingress/egress chains: + - prefix: + - ingress: "INGRESS" + - egress: "EGRESS" + - form: + INGRESS (or "EGRESS-" if egress) + -POLICY + -policyKey + -TO (or "-FROM" if egress) + [-podSelectorComment] (or "all" if there are no pod selectors) + -IN-ns + -namespaceName + + strings for protocol, ports, selectors: + protocol: just "name" + + port (range): + - v1: + - for single port: PORT-x + - for namedport: PORT-name + - v2: + - for single port: PORT-x + - with endport: PORT-x:y + - in v2, namedports are specified as IPSets + + selector: + "[!]" means optionally include "!" if the label is not included + - namespace selectors (only for v1): + - if there are no match expressions or labels: + all-namespaces + - otherwise: + ns-[!]label1-AND-ns-[!]label2... + - other w/out ns: + [!]label1-AND-[!]label2... + - other w/ ns: + [!]label1-AND-[!]label2...-AND-ns-[!]labelM-IN-ns-name +*/ diff --git a/npm/pkg/dataplane/policies/policymanager_linux.go b/npm/pkg/dataplane/policies/policymanager_linux.go index 346cca8065..8fff91bc08 100644 --- a/npm/pkg/dataplane/policies/policymanager_linux.go +++ b/npm/pkg/dataplane/policies/policymanager_linux.go @@ -111,11 +111,11 @@ func (pMgr *PolicyManager) deleteOldJumpRulesOnRemove(policy *NPMNetworkPolicy) return nil } -func (pMgr *PolicyManager) deleteJumpRule(policy *NPMNetworkPolicy, isIngress bool) error { +func (pMgr *PolicyManager) deleteJumpRule(policy *NPMNetworkPolicy, direction UniqueDirection) error { var specs []string var baseChainName string var chainName string - if isIngress { + if direction == forIngress { specs = ingressJumpSpecs(policy) baseChainName = util.IptablesAzureIngressChain chainName = policy.ingressChainName() @@ -139,13 +139,17 @@ func (pMgr *PolicyManager) deleteJumpRule(policy *NPMNetworkPolicy, isIngress bo func ingressJumpSpecs(networkPolicy *NPMNetworkPolicy) []string { chainName := networkPolicy.ingressChainName() specs := []string{util.IptablesJumpFlag, chainName} - return append(specs, matchSetSpecsForNetworkPolicy(networkPolicy, DstMatch)...) + specs = append(specs, matchSetSpecsForNetworkPolicy(networkPolicy, DstMatch)...) + specs = append(specs, commentSpecs(networkPolicy.commentForJumpToIngress())...) + return specs } func egressJumpSpecs(networkPolicy *NPMNetworkPolicy) []string { chainName := networkPolicy.egressChainName() specs := []string{util.IptablesJumpFlag, chainName} - return append(specs, matchSetSpecsForNetworkPolicy(networkPolicy, SrcMatch)...) + specs = append(specs, matchSetSpecsForNetworkPolicy(networkPolicy, SrcMatch)...) + specs = append(specs, commentSpecs(networkPolicy.commentForJumpToEgress())...) + return specs } // noflush add to chains impacted @@ -210,9 +214,7 @@ func iptablesRuleSpecs(aclPolicy *ACLPolicy) []string { specs = append(specs, dstPortSpecs(aclPolicy.DstPorts)...) specs = append(specs, matchSetSpecsFromSetInfo(aclPolicy.SrcList)...) specs = append(specs, matchSetSpecsFromSetInfo(aclPolicy.DstList)...) - if aclPolicy.Comment != "" { - specs = append(specs, commentSpecs(aclPolicy.Comment)...) - } + specs = append(specs, commentSpecs(aclPolicy.comment())...) return specs } diff --git a/npm/pkg/dataplane/policies/policymanager_linux_test.go b/npm/pkg/dataplane/policies/policymanager_linux_test.go index f3f3db0c23..558ced1bfe 100644 --- a/npm/pkg/dataplane/policies/policymanager_linux_test.go +++ b/npm/pkg/dataplane/policies/policymanager_linux_test.go @@ -13,32 +13,193 @@ import ( "github.com/stretchr/testify/require" ) +// ACLs +// Don't care about PolicyID for Linux var ( - testPolicy1IngressChain = TestNetworkPolicies[0].ingressChainName() - testPolicy1EgressChain = TestNetworkPolicies[0].egressChainName() - testPolicy2IngressChain = TestNetworkPolicies[1].ingressChainName() - testPolicy3EgressChain = TestNetworkPolicies[2].egressChainName() - - testPolicy1IngressJump = fmt.Sprintf("-j %s -m set --match-set %s dst", testPolicy1IngressChain, ipsets.TestKeyPodSet.HashedName) - testPolicy1EgressJump = fmt.Sprintf("-j %s -m set --match-set %s src", testPolicy1EgressChain, ipsets.TestKeyPodSet.HashedName) - testPolicy2IngressJump = fmt.Sprintf("-j %s -m set --match-set %s dst -m set --match-set %s dst", testPolicy2IngressChain, ipsets.TestKeyPodSet.HashedName, ipsets.TestKVPodSet.HashedName) - testPolicy3EgressJump = fmt.Sprintf("-j %s", testPolicy3EgressChain) - - testACLRule1 = fmt.Sprintf( - "-j MARK --set-mark 0x4000 -p TCP --dport 222:333 -m set --match-set %s src -m set ! --match-set %s dst -m comment --comment comment1", + ingressDeniedACL = &ACLPolicy{ + SrcList: []SetInfo{ + { + ipsets.TestCIDRSet.Metadata, + true, + SrcMatch, + }, + { + ipsets.TestKeyPodSet.Metadata, + false, + DstMatch, + }, + }, + Target: Dropped, + Direction: Ingress, + DstPorts: Ports{ + 222, 333, + }, + Protocol: TCP, + } + ingressAllowedACL = &ACLPolicy{ + SrcList: []SetInfo{ + { + ipsets.TestCIDRSet.Metadata, + true, + SrcMatch, + }, + }, + Target: Allowed, + Direction: Ingress, + Protocol: UnspecifiedProtocol, + } + egressDeniedACL = &ACLPolicy{ + PolicyID: "acl3", + DstList: []SetInfo{ + { + ipsets.TestCIDRSet.Metadata, + true, + DstMatch, + }, + }, + Target: Dropped, + Direction: Egress, + DstPorts: Ports{144, 144}, + Protocol: UDP, + } + egressAllowedACL = &ACLPolicy{ + PolicyID: "acl4", + DstList: []SetInfo{ + { + ipsets.TestNamedportSet.Metadata, + true, + DstMatch, + }, + }, + Target: Allowed, + Direction: Egress, + Protocol: UnspecifiedProtocol, + } +) + +// iptables rule constants for ACLs +const ( + // TODO + ingressDropComment = "DROP-FROM-cidr-test-cidr-set-AND-!podlabel-test-keyPod-set-ON-TCP-TO-PORT-222:333" + ingressAllowComment = "ALLOW-FROM-cidr-test-cidr-set" + egressDropComment = "DROP-TO-cidr-test-cidr-set-ON-UDP-TO-PORT-144" + egressAllowComment = "ALLOW-ALL-TO-namedport:test-namedport-set" +) + +// iptables rule variables for ACLs +var ( + ingressDropRule = fmt.Sprintf( + "-j MARK --set-mark 0x4000 -p TCP --dport 222:333 -m set --match-set %s src -m set ! --match-set %s dst -m comment --comment %s", ipsets.TestCIDRSet.HashedName, ipsets.TestKeyPodSet.HashedName, + ingressDropComment, + ) + ingressAllowRule = fmt.Sprintf("-j AZURE-NPM-INGRESS-ALLOW-MARK -m set --match-set %s src -m comment --comment %s", ipsets.TestCIDRSet.HashedName, ingressAllowComment) + egressDropRule = fmt.Sprintf("-j MARK --set-mark 0x5000 -p UDP --dport 144 -m set --match-set %s dst -m comment --comment %s", ipsets.TestCIDRSet.HashedName, egressDropComment) + egressAllowRule = fmt.Sprintf("-j AZURE-NPM-ACCEPT -m set --match-set %s dst -m comment --comment %s", ipsets.TestNamedportSet.HashedName, egressAllowComment) +) + +// NetworkPolicies +var ( + bothDirectionsNetPol = &NPMNetworkPolicy{ + Name: "test1", + NameSpace: "x", + PolicyKey: "x/test1", + PodSelectorIPSets: []*ipsets.TranslatedIPSet{ + {Metadata: ipsets.TestKeyPodSet.Metadata}, + }, + PodSelectorList: []SetInfo{ + { + IPSet: ipsets.TestKeyPodSet.Metadata, + Included: true, + MatchType: EitherMatch, + }, + }, + ACLs: []*ACLPolicy{ + ingressDeniedACL, + ingressAllowedACL, + egressDeniedACL, + egressAllowedACL, + }, + } + ingressNetPol = &NPMNetworkPolicy{ + Name: "test2", + NameSpace: "y", + PolicyKey: "y/test2", + PodSelectorIPSets: []*ipsets.TranslatedIPSet{ + {Metadata: ipsets.TestKeyPodSet.Metadata}, + {Metadata: ipsets.TestNSSet.Metadata}, + }, + PodSelectorList: []SetInfo{ + { + IPSet: ipsets.TestKeyPodSet.Metadata, + Included: true, + MatchType: EitherMatch, + }, + { + IPSet: ipsets.TestNSSet.Metadata, + Included: true, + MatchType: EitherMatch, + }, + }, + ACLs: []*ACLPolicy{ + ingressDeniedACL, + }, + } + egressNetPol = &NPMNetworkPolicy{ + Name: "test3", + NameSpace: "z", + PolicyKey: "z/test3", + ACLs: []*ACLPolicy{ + egressAllowedACL, + }, + } +) + +// iptables rule constants for NetworkPolicies +const ( + bothDirectionsNetPolIngressJumpComment = "INGRESS-POLICY-x/test1-TO-podlabel-test-keyPod-set-IN-ns-x" + bothDirectionsNetPolEgressJumpComment = "EGRESS-POLICY-x/test1-FROM-podlabel-test-keyPod-set-IN-ns-x" + ingressNetPolJumpComment = "INGRESS-POLICY-y/test2-TO-podlabel-test-keyPod-set-AND-ns-test-ns-set-IN-ns-y" + egressNetPolJumpComment = "EGRESS-POLICY-z/test3-FROM-all-IN-ns-z" +) + +// iptable rule variables for NetworkPolicies +var ( + bothDirectionsNetPolIngressChain = bothDirectionsNetPol.ingressChainName() + bothDirectionsNetPolEgressChain = bothDirectionsNetPol.egressChainName() + ingressNetPolChain = ingressNetPol.ingressChainName() + egressNetPolChain = egressNetPol.egressChainName() + + ingressEgressNetPolIngressJump = fmt.Sprintf( + "-j %s -m set --match-set %s dst -m comment --comment %s", + bothDirectionsNetPolIngressChain, + ipsets.TestKeyPodSet.HashedName, + bothDirectionsNetPolIngressJumpComment, + ) + ingressEgressNetPolEgressJump = fmt.Sprintf( + "-j %s -m set --match-set %s src -m comment --comment %s", + bothDirectionsNetPolEgressChain, + ipsets.TestKeyPodSet.HashedName, + bothDirectionsNetPolEgressJumpComment, ) - testACLRule2 = fmt.Sprintf("-j AZURE-NPM-INGRESS-ALLOW-MARK -p UDP -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 -m set --match-set %s src -m comment --comment comment4", ipsets.TestCIDRSet.HashedName) + ingressNetPolJump = fmt.Sprintf( + "-j %s -m set --match-set %s dst -m set --match-set %s dst -m comment --comment %s", + ingressNetPolChain, + ipsets.TestKeyPodSet.HashedName, + ipsets.TestNSSet.HashedName, + ingressNetPolJumpComment, + ) + egressNetPolJump = fmt.Sprintf("-j %s -m comment --comment %s", egressNetPolChain, egressNetPolJumpComment) ) +var allTestNetworkPolicies = []*NPMNetworkPolicy{bothDirectionsNetPol, ingressNetPol, egressNetPol} + func TestChainNames(t *testing.T) { - expectedName := fmt.Sprintf("AZURE-NPM-INGRESS-%s", util.Hash(TestNetworkPolicies[0].PolicyKey)) - require.Equal(t, expectedName, TestNetworkPolicies[0].ingressChainName()) - expectedName = fmt.Sprintf("AZURE-NPM-EGRESS-%s", util.Hash(TestNetworkPolicies[0].PolicyKey)) - require.Equal(t, expectedName, TestNetworkPolicies[0].egressChainName()) + expectedName := fmt.Sprintf("AZURE-NPM-INGRESS-%s", util.Hash(bothDirectionsNetPol.PolicyKey)) + require.Equal(t, expectedName, bothDirectionsNetPol.ingressChainName()) + expectedName = fmt.Sprintf("AZURE-NPM-EGRESS-%s", util.Hash(bothDirectionsNetPol.PolicyKey)) + require.Equal(t, expectedName, bothDirectionsNetPol.egressChainName()) } func TestAddPolicies(t *testing.T) { @@ -46,34 +207,34 @@ func TestAddPolicies(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim) - creator := pMgr.creatorForNewNetworkPolicies(allChainNames(TestNetworkPolicies), TestNetworkPolicies...) + creator := pMgr.creatorForNewNetworkPolicies(allChainNames(allTestNetworkPolicies), allTestNetworkPolicies...) actualLines := strings.Split(creator.ToString(), "\n") expectedLines := []string{ "*filter", // all chains - fmt.Sprintf(":%s - -", testPolicy1IngressChain), - fmt.Sprintf(":%s - -", testPolicy1EgressChain), - fmt.Sprintf(":%s - -", testPolicy2IngressChain), - fmt.Sprintf(":%s - -", testPolicy3EgressChain), + fmt.Sprintf(":%s - -", bothDirectionsNetPolIngressChain), + fmt.Sprintf(":%s - -", bothDirectionsNetPolEgressChain), + fmt.Sprintf(":%s - -", ingressNetPolChain), + fmt.Sprintf(":%s - -", egressNetPolChain), // 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), + fmt.Sprintf("-A %s %s", bothDirectionsNetPolIngressChain, ingressDropRule), + fmt.Sprintf("-A %s %s", bothDirectionsNetPolIngressChain, ingressAllowRule), + fmt.Sprintf("-A %s %s", bothDirectionsNetPolEgressChain, egressDropRule), + fmt.Sprintf("-A %s %s", bothDirectionsNetPolEgressChain, egressAllowRule), + fmt.Sprintf("-I AZURE-NPM-INGRESS 1 %s", ingressEgressNetPolIngressJump), + fmt.Sprintf("-I AZURE-NPM-EGRESS 1 %s", ingressEgressNetPolEgressJump), // policy 2 - fmt.Sprintf("-A %s %s", testPolicy2IngressChain, testACLRule1), - fmt.Sprintf("-I AZURE-NPM-INGRESS 2 %s", testPolicy2IngressJump), + fmt.Sprintf("-A %s %s", ingressNetPolChain, ingressDropRule), + fmt.Sprintf("-I AZURE-NPM-INGRESS 2 %s", ingressNetPolJump), // policy 3 - fmt.Sprintf("-A %s %s", testPolicy3EgressChain, testACLRule4), - fmt.Sprintf("-I AZURE-NPM-EGRESS 2 %s", testPolicy3EgressJump), + fmt.Sprintf("-A %s %s", egressNetPolChain, egressAllowRule), + fmt.Sprintf("-I AZURE-NPM-EGRESS 2 %s", egressNetPolJump), "COMMIT", "", } dptestutils.AssertEqualLines(t, expectedLines, actualLines) - err := pMgr.addPolicy(TestNetworkPolicies[0], nil) + err := pMgr.AddPolicy(bothDirectionsNetPol, nil) require.NoError(t, err) } @@ -82,126 +243,126 @@ func TestAddPoliciesError(t *testing.T) { ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim) - err := pMgr.addPolicy(TestNetworkPolicies[0], nil) + err := pMgr.addPolicy(bothDirectionsNetPol, 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 + getFakeDeleteJumpCommand("AZURE-NPM-INGRESS", ingressEgressNetPolIngressJump), + getFakeDeleteJumpCommandWithCode("AZURE-NPM-EGRESS", ingressEgressNetPolEgressJump, 2), // if the policy chain doesn't exist, we shouldn't error fakeIPTablesRestoreCommand, } ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim) - creator := pMgr.creatorForRemovingPolicies(allChainNames(TestNetworkPolicies)) + creator := pMgr.creatorForRemovingPolicies(allChainNames(allTestNetworkPolicies)) actualLines := strings.Split(creator.ToString(), "\n") expectedLines := []string{ "*filter", - fmt.Sprintf(":%s - -", testPolicy1IngressChain), - fmt.Sprintf(":%s - -", testPolicy1EgressChain), - fmt.Sprintf(":%s - -", testPolicy2IngressChain), - fmt.Sprintf(":%s - -", testPolicy3EgressChain), + fmt.Sprintf(":%s - -", bothDirectionsNetPolIngressChain), + fmt.Sprintf(":%s - -", bothDirectionsNetPolEgressChain), + fmt.Sprintf(":%s - -", ingressNetPolChain), + fmt.Sprintf(":%s - -", egressNetPolChain), "COMMIT", "", } dptestutils.AssertEqualLines(t, expectedLines, actualLines) - err := pMgr.AddPolicy(TestNetworkPolicies[0], nil) // need the policy in the cache + err := pMgr.AddPolicy(bothDirectionsNetPol, nil) // need the policy in the cache require.NoError(t, err) - err = pMgr.RemovePolicy(TestNetworkPolicies[0].PolicyKey, nil) + err = pMgr.RemovePolicy(bothDirectionsNetPol.PolicyKey, nil) require.NoError(t, err) } func TestRemovePoliciesErrorOnRestore(t *testing.T) { calls := []testutils.TestCmd{ fakeIPTablesRestoreCommand, - getFakeDeleteJumpCommand("AZURE-NPM-INGRESS", testPolicy1IngressJump), - getFakeDeleteJumpCommand("AZURE-NPM-EGRESS", testPolicy1EgressJump), + getFakeDeleteJumpCommand("AZURE-NPM-INGRESS", ingressEgressNetPolIngressJump), + getFakeDeleteJumpCommand("AZURE-NPM-EGRESS", ingressEgressNetPolEgressJump), fakeIPTablesRestoreFailureCommand, } ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim) - err := pMgr.AddPolicy(TestNetworkPolicies[0], nil) + err := pMgr.AddPolicy(bothDirectionsNetPol, nil) require.NoError(t, err) - err = pMgr.RemovePolicy(TestNetworkPolicies[0].PolicyKey, nil) + err = pMgr.RemovePolicy(bothDirectionsNetPol.PolicyKey, nil) require.Error(t, err) } func TestRemovePoliciesErrorOnDeleteForIngress(t *testing.T) { calls := []testutils.TestCmd{ fakeIPTablesRestoreCommand, - getFakeDeleteJumpCommandWithCode("AZURE-NPM-INGRESS", testPolicy1IngressJump, 1), // anything but 0 or 2 + getFakeDeleteJumpCommandWithCode("AZURE-NPM-INGRESS", ingressEgressNetPolIngressJump, 1), // anything but 0 or 2 } ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim) - err := pMgr.AddPolicy(TestNetworkPolicies[0], nil) + err := pMgr.AddPolicy(bothDirectionsNetPol, nil) require.NoError(t, err) - err = pMgr.RemovePolicy(TestNetworkPolicies[0].PolicyKey, nil) + err = pMgr.RemovePolicy(bothDirectionsNetPol.PolicyKey, nil) require.Error(t, err) } func TestRemovePoliciesErrorOnDeleteForEgress(t *testing.T) { calls := []testutils.TestCmd{ fakeIPTablesRestoreCommand, - getFakeDeleteJumpCommand("AZURE-NPM-INGRESS", testPolicy1IngressJump), - getFakeDeleteJumpCommandWithCode("AZURE-NPM-EGRESS", testPolicy1EgressJump, 1), // anything but 0 or 2 + getFakeDeleteJumpCommand("AZURE-NPM-INGRESS", ingressEgressNetPolIngressJump), + getFakeDeleteJumpCommandWithCode("AZURE-NPM-EGRESS", ingressEgressNetPolEgressJump, 1), // anything but 0 or 2 } ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim) - err := pMgr.AddPolicy(TestNetworkPolicies[0], nil) + err := pMgr.AddPolicy(bothDirectionsNetPol, nil) require.NoError(t, err) - err = pMgr.RemovePolicy(TestNetworkPolicies[0].PolicyKey, nil) + err = pMgr.RemovePolicy(bothDirectionsNetPol.PolicyKey, nil) require.Error(t, err) } func TestUpdatingChainsToCleanup(t *testing.T) { - calls := GetAddPolicyTestCalls(TestNetworkPolicies[0]) - calls = append(calls, GetRemovePolicyTestCalls(TestNetworkPolicies[0])...) - calls = append(calls, GetAddPolicyTestCalls(TestNetworkPolicies[1])...) - calls = append(calls, GetRemovePolicyFailureTestCalls(TestNetworkPolicies[1])...) - calls = append(calls, GetAddPolicyTestCalls(TestNetworkPolicies[2])...) - calls = append(calls, GetRemovePolicyTestCalls(TestNetworkPolicies[2])...) - calls = append(calls, GetAddPolicyFailureTestCalls(TestNetworkPolicies[0])...) - calls = append(calls, GetAddPolicyTestCalls(TestNetworkPolicies[0])...) + calls := GetAddPolicyTestCalls(bothDirectionsNetPol) + calls = append(calls, GetRemovePolicyTestCalls(bothDirectionsNetPol)...) + calls = append(calls, GetAddPolicyTestCalls(ingressNetPol)...) + calls = append(calls, GetRemovePolicyFailureTestCalls(ingressNetPol)...) + calls = append(calls, GetAddPolicyTestCalls(egressNetPol)...) + calls = append(calls, GetRemovePolicyTestCalls(egressNetPol)...) + calls = append(calls, GetAddPolicyFailureTestCalls(bothDirectionsNetPol)...) + calls = append(calls, GetAddPolicyTestCalls(bothDirectionsNetPol)...) ioshim := common.NewMockIOShim(calls) defer ioshim.VerifyCalls(t, calls) pMgr := NewPolicyManager(ioshim) // add so we can remove. no stale chains to start - require.NoError(t, pMgr.AddPolicy(TestNetworkPolicies[0], nil)) + require.NoError(t, pMgr.AddPolicy(bothDirectionsNetPol, nil)) assertStaleChainsContain(t, pMgr.staleChains) // successful removal, so mark the policy's chains as stale - require.NoError(t, pMgr.RemovePolicy(TestNetworkPolicies[0].PolicyKey, nil)) - assertStaleChainsContain(t, pMgr.staleChains, testPolicy1IngressChain, testPolicy1EgressChain) + require.NoError(t, pMgr.RemovePolicy(bothDirectionsNetPol.PolicyKey, nil)) + assertStaleChainsContain(t, pMgr.staleChains, bothDirectionsNetPolIngressChain, bothDirectionsNetPolEgressChain) // successful add, so keep the same stale chains - require.NoError(t, pMgr.AddPolicy(TestNetworkPolicies[1], nil)) - assertStaleChainsContain(t, pMgr.staleChains, testPolicy1IngressChain, testPolicy1EgressChain) + require.NoError(t, pMgr.AddPolicy(ingressNetPol, nil)) + assertStaleChainsContain(t, pMgr.staleChains, bothDirectionsNetPolIngressChain, bothDirectionsNetPolEgressChain) // failure to remove, so keep the same stale chains - require.Error(t, pMgr.RemovePolicy(TestNetworkPolicies[1].PolicyKey, nil)) - assertStaleChainsContain(t, pMgr.staleChains, testPolicy1IngressChain, testPolicy1EgressChain) + require.Error(t, pMgr.RemovePolicy(ingressNetPol.PolicyKey, nil)) + assertStaleChainsContain(t, pMgr.staleChains, bothDirectionsNetPolIngressChain, bothDirectionsNetPolEgressChain) // successfully add a new policy. keep the same stale chains - require.NoError(t, pMgr.AddPolicy(TestNetworkPolicies[2], nil)) - assertStaleChainsContain(t, pMgr.staleChains, testPolicy1IngressChain, testPolicy1EgressChain) + require.NoError(t, pMgr.AddPolicy(egressNetPol, nil)) + assertStaleChainsContain(t, pMgr.staleChains, bothDirectionsNetPolIngressChain, bothDirectionsNetPolEgressChain) // successful removal, so mark the policy's chains as stale - require.NoError(t, pMgr.RemovePolicy(TestNetworkPolicies[2].PolicyKey, nil)) - assertStaleChainsContain(t, pMgr.staleChains, testPolicy1IngressChain, testPolicy1EgressChain, testPolicy3EgressChain) + require.NoError(t, pMgr.RemovePolicy(egressNetPol.PolicyKey, nil)) + assertStaleChainsContain(t, pMgr.staleChains, bothDirectionsNetPolIngressChain, bothDirectionsNetPolEgressChain, egressNetPolChain) // failure to add, so keep the same stale chains the same - require.Error(t, pMgr.AddPolicy(TestNetworkPolicies[0], nil)) - assertStaleChainsContain(t, pMgr.staleChains, testPolicy1IngressChain, testPolicy1EgressChain, testPolicy3EgressChain) + require.Error(t, pMgr.AddPolicy(bothDirectionsNetPol, nil)) + assertStaleChainsContain(t, pMgr.staleChains, bothDirectionsNetPolIngressChain, bothDirectionsNetPolEgressChain, egressNetPolChain) // successful add, so remove the policy's chains from the stale chains - require.NoError(t, pMgr.AddPolicy(TestNetworkPolicies[0], nil)) - assertStaleChainsContain(t, pMgr.staleChains, testPolicy3EgressChain) + require.NoError(t, pMgr.AddPolicy(bothDirectionsNetPol, nil)) + assertStaleChainsContain(t, pMgr.staleChains, egressNetPolChain) } diff --git a/npm/pkg/dataplane/policies/testutils.go b/npm/pkg/dataplane/policies/testutils.go index 69632baea7..8e13578474 100644 --- a/npm/pkg/dataplane/policies/testutils.go +++ b/npm/pkg/dataplane/policies/testutils.go @@ -2,6 +2,7 @@ package policies import "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" +// TODO: deprecate this file. Updating this file impacts multiple tests. var ( // TestNetworkPolicies for testing TestNetworkPolicies = []*NPMNetworkPolicy{