From 7ce494f858ae1d3ee6bbe37fd88006687ca1eb77 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Tue, 22 Mar 2022 20:35:36 -0700 Subject: [PATCH 1/7] remove Name field from NPMNetworkPolicy --- .../goalstateprocessor/goalstateprocessor.go | 4 +- .../goalstateprocessor_test.go | 3 +- .../translation/translatePolicy.go | 41 ++-- .../translation/translatePolicy_test.go | 195 +++++++++--------- npm/pkg/dataplane/dataplane_test.go | 3 +- npm/pkg/dataplane/dataplane_windows.go | 32 +-- npm/pkg/dataplane/dpshim/dpshim_test.go | 3 +- npm/pkg/dataplane/ipsets/ipset.go | 6 +- npm/pkg/dataplane/policies/policy.go | 12 +- npm/pkg/dataplane/policies/policy_linux.go | 2 +- .../policies/policymanager_linux_test.go | 9 +- .../dataplane/policies/policymanager_test.go | 9 +- .../policies/policymanager_windows.go | 12 +- npm/pkg/dataplane/policies/testutils.go | 9 +- 14 files changed, 169 insertions(+), 171 deletions(-) diff --git a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go index c55fb0b9eb..7753eccb96 100644 --- a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go +++ b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go @@ -374,12 +374,12 @@ func (gsp *GoalStateProcessor) processPolicyApplyEvent(goalState *protos.GoalSta klog.Warningf("Empty Policy apply event") continue } - klog.Infof("Processing %s Policy ADD event", netpol.Name) + klog.Infof("Processing %s Policy ADD event", netpol.PolicyKey) klog.Infof("Netpol: %v", netpol) err = gsp.dp.UpdatePolicy(netpol) if err != nil { - klog.Errorf("Error applying policy %s to dataplane with error: %s", netpol.Name, err.Error()) + klog.Errorf("Error applying policy %s to dataplane with error: %s", netpol.PolicyKey, err.Error()) return nil, npmerrors.SimpleErrorWrapper("failed update policy event", err) } appendedPolicies[netpol.PolicyKey] = struct{}{} diff --git a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go index 6c7945ab58..b3fe82cbdb 100644 --- a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go +++ b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go @@ -28,8 +28,7 @@ var ( testNestedKeyPodSet = ipsets.NewIPSetMetadata("test-nestedkeyPod-set", ipsets.NestedLabelOfPod) testNestedKeyPodCPSet = controlplane.NewControllerIPSets(testNestedKeyPodSet) testNetPol = &policies.NPMNetworkPolicy{ - Name: "test-netpol", - NameSpace: "x", + Namespace: "x", PolicyKey: "x/test-netpol", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ { diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index ca962a4b69..a5b5ef2061 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -304,7 +304,7 @@ func ruleExists(ports []networkingv1.NetworkPolicyPort, peer []networkingv1.Netw // (e.g., IPBlock, podSelector, namespaceSelector, or both podSelector and namespaceSelector). func peerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Direction, ports []networkingv1.NetworkPolicyPort, setInfo []policies.SetInfo) error { if len(ports) == 0 { - acl := policies.NewACLPolicy(npmNetPol.NameSpace, npmNetPol.Name, policies.Allowed, direction) + acl := policies.NewACLPolicy(npmNetPol.Namespace, npmNetPol.Name, policies.Allowed, direction) acl.AddSetInfo(setInfo) npmNetPol.ACLs = append(npmNetPol.ACLs, acl) return nil @@ -316,7 +316,7 @@ func peerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Di return err } - acl := policies.NewACLPolicy(npmNetPol.NameSpace, npmNetPol.Name, policies.Allowed, direction) + acl := policies.NewACLPolicy(npmNetPol.Namespace, npmNetPol.Name, policies.Allowed, direction) acl.AddSetInfo(setInfo) npmNetPol.RuleIPSets = portRule(npmNetPol.RuleIPSets, acl, &ports[i], portKind) npmNetPol.ACLs = append(npmNetPol.ACLs, acl) @@ -325,7 +325,7 @@ func peerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Di } // translateRule translates ingress or egress rules and update npmNetPol object. -func translateRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Direction, matchType policies.MatchType, ruleIndex int, +func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, direction policies.Direction, matchType policies.MatchType, ruleIndex int, ports []networkingv1.NetworkPolicyPort, peers []networkingv1.NetworkPolicyPeer) error { // TODO(jungukcho): need to clean up it. // Leave allowExternal variable now while the condition is checked before calling this function. @@ -335,7 +335,7 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Dire // The code inside if condition is to handle allowing all internal traffic, but the case is handled in #2.4. // So, this code may not execute. After confirming this, need to delete it. if !portRuleExists && !peerRuleExists && !allowExternal { - acl := policies.NewACLPolicy(npmNetPol.NameSpace, npmNetPol.Name, policies.Allowed, direction) + acl := policies.NewACLPolicy(npmNetPol.Namespace, npmNetPol.Name, policies.Allowed, direction) ruleIPSets, allowAllInternalSetInfo := allowAllInternal(matchType) npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, ruleIPSets) acl.AddSetInfo([]policies.SetInfo{allowAllInternalSetInfo}) @@ -351,7 +351,7 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Dire return err } - portACL := policies.NewACLPolicy(npmNetPol.NameSpace, npmNetPol.Name, policies.Allowed, direction) + portACL := policies.NewACLPolicy(npmNetPol.Namespace, npmNetPol.Name, policies.Allowed, direction) npmNetPol.RuleIPSets = portRule(npmNetPol.RuleIPSets, portACL, &ports[i], portKind) npmNetPol.ACLs = append(npmNetPol.ACLs, portACL) } @@ -362,7 +362,7 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Dire // #2.1 Handle IPBlock and port if exist if peer.IPBlock != nil { if len(peer.IPBlock.CIDR) > 0 { - ipBlockIPSet, ipBlockSetInfo := ipBlockRule(npmNetPol.Name, npmNetPol.NameSpace, direction, matchType, ruleIndex, peerIdx, peer.IPBlock) + ipBlockIPSet, ipBlockSetInfo := ipBlockRule(netPolName, npmNetPol.Namespace, direction, matchType, ruleIndex, peerIdx, peer.IPBlock) npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, ipBlockIPSet) err := peerAndPortRule(npmNetPol, direction, ports, []policies.SetInfo{ipBlockSetInfo}) if err != nil { @@ -397,7 +397,7 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Dire // #2.3 handle podSelector and port if exist if peer.PodSelector != nil && peer.NamespaceSelector == nil { - podSelectorIPSets, podSelectorList, err := podSelectorWithNS(npmNetPol.NameSpace, matchType, peer.PodSelector) + podSelectorIPSets, podSelectorList, err := podSelectorWithNS(npmNetPol.Namespace, matchType, peer.PodSelector) if err != nil { return err } @@ -448,7 +448,7 @@ func defaultDropACL(policyNS, policyName string, direction policies.Direction) * // allowAllPolicy adds acl to allow all traffic including internal (i.e,. K8s cluster) and external (i.e., internet) func allowAllPolicy(npmNetPol *policies.NPMNetworkPolicy, direction policies.Direction) { - allowAllACL := policies.NewACLPolicy(npmNetPol.NameSpace, npmNetPol.Name, policies.Allowed, direction) + allowAllACL := policies.NewACLPolicy(npmNetPol.Namespace, npmNetPol.Name, policies.Allowed, direction) npmNetPol.ACLs = append(npmNetPol.ACLs, allowAllACL) } @@ -462,7 +462,7 @@ func isAllowAllToIngress(ingress []networkingv1.NetworkPolicyIngressRule) bool { // ingressPolicy traslates NetworkPolicyIngressRule in NetworkPolicy object // to NPMNetworkPolicy object. -func ingressPolicy(npmNetPol *policies.NPMNetworkPolicy, ingress []networkingv1.NetworkPolicyIngressRule) error { +func ingressPolicy(npmNetPol *policies.NPMNetworkPolicy, netPolName string, ingress []networkingv1.NetworkPolicyIngressRule) error { // #1. Allow all traffic from both internal and external. // In yaml file, it is specified with '{}'. if isAllowAllToIngress(ingress) { @@ -473,7 +473,7 @@ func ingressPolicy(npmNetPol *policies.NPMNetworkPolicy, ingress []networkingv1. // #2. If ingress is nil (in yaml file, it is specified with '[]'), it means "Deny all" - it does not allow receiving any traffic from others. if ingress == nil { // Except for allow all traffic case in #1, the rest of them should have default drop rules. - dropACL := defaultDropACL(npmNetPol.NameSpace, npmNetPol.Name, policies.Ingress) + dropACL := defaultDropACL(npmNetPol.Namespace, npmNetPol.Name, policies.Ingress) npmNetPol.ACLs = append(npmNetPol.ACLs, dropACL) return nil } @@ -481,12 +481,12 @@ func ingressPolicy(npmNetPol *policies.NPMNetworkPolicy, ingress []networkingv1. // #3. Ingress rule is not AllowAll (including internal and external) and DenyAll policy. // So, start translating ingress policy. for i, rule := range ingress { - if err := translateRule(npmNetPol, policies.Ingress, policies.SrcMatch, i, rule.Ports, rule.From); err != nil { + if err := translateRule(npmNetPol, netPolName, policies.Ingress, policies.SrcMatch, i, rule.Ports, rule.From); err != nil { return err } } // Except for allow all traffic case in #1, the rest of them should have default drop rules. - dropACL := defaultDropACL(npmNetPol.NameSpace, npmNetPol.Name, policies.Ingress) + dropACL := defaultDropACL(npmNetPol.Namespace, npmNetPol.Name, policies.Ingress) npmNetPol.ACLs = append(npmNetPol.ACLs, dropACL) return nil } @@ -501,7 +501,7 @@ func isAllowAllToEgress(egress []networkingv1.NetworkPolicyEgressRule) bool { // egressPolicy traslates NetworkPolicyEgressRule in networkpolicy object // to NPMNetworkPolicy object. -func egressPolicy(npmNetPol *policies.NPMNetworkPolicy, egress []networkingv1.NetworkPolicyEgressRule) error { +func egressPolicy(npmNetPol *policies.NPMNetworkPolicy, netPolName string, egress []networkingv1.NetworkPolicyEgressRule) error { // #1. Allow all traffic to both internal and external. // In yaml file, it is specified with '{}'. if isAllowAllToEgress(egress) { @@ -512,7 +512,7 @@ func egressPolicy(npmNetPol *policies.NPMNetworkPolicy, egress []networkingv1.Ne // #2. If egress is nil (in yaml file, it is specified with '[]'), it means "Deny all" - it does not allow sending traffic to others. if egress == nil { // Except for allow all traffic case in #1, the rest of them should have default drop rules. - dropACL := defaultDropACL(npmNetPol.NameSpace, npmNetPol.Name, policies.Egress) + dropACL := defaultDropACL(npmNetPol.Namespace, npmNetPol.Name, policies.Egress) npmNetPol.ACLs = append(npmNetPol.ACLs, dropACL) return nil } @@ -520,7 +520,7 @@ func egressPolicy(npmNetPol *policies.NPMNetworkPolicy, egress []networkingv1.Ne // #3. Egress rule is not AllowAll (including internal and external) and DenyAll. // So, start translating egress policy. for i, rule := range egress { - err := translateRule(npmNetPol, policies.Egress, policies.DstMatch, i, rule.Ports, rule.To) + err := translateRule(npmNetPol, netPolName, policies.Egress, policies.DstMatch, i, rule.Ports, rule.To) if err != nil { return err } @@ -528,7 +528,7 @@ func egressPolicy(npmNetPol *policies.NPMNetworkPolicy, egress []networkingv1.Ne // #3. Except for allow all traffic case in #1, the rest of them should have default drop rules. // Add drop ACL to drop the rest of traffic which is not specified in Egress Spec. - dropACL := defaultDropACL(npmNetPol.NameSpace, npmNetPol.Name, policies.Egress) + dropACL := defaultDropACL(npmNetPol.Namespace, npmNetPol.Name, policies.Egress) npmNetPol.ACLs = append(npmNetPol.ACLs, dropACL) return nil } @@ -536,12 +536,13 @@ func egressPolicy(npmNetPol *policies.NPMNetworkPolicy, egress []networkingv1.Ne // TranslatePolicy traslates networkpolicy object to NPMNetworkPolicy object // and return the NPMNetworkPolicy object. func TranslatePolicy(npObj *networkingv1.NetworkPolicy) (*policies.NPMNetworkPolicy, error) { - npmNetPol := policies.NewNPMNetworkPolicy(npObj.Name, npObj.Namespace) + netPolName := npObj.Name + npmNetPol := policies.NewNPMNetworkPolicy(netPolName, npObj.Namespace) // podSelector in spec.PodSelector is common for ingress and egress. // Process this podSelector first. var err error - npmNetPol.PodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.NameSpace, policies.EitherMatch, &npObj.Spec.PodSelector) + npmNetPol.PodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.Namespace, policies.EitherMatch, &npObj.Spec.PodSelector) if err != nil { return nil, err } @@ -551,12 +552,12 @@ func TranslatePolicy(npObj *networkingv1.NetworkPolicy) (*policies.NPMNetworkPol // and Egress will be set if the NetworkPolicy has any egress rules. for _, ptype := range npObj.Spec.PolicyTypes { if ptype == networkingv1.PolicyTypeIngress { - err := ingressPolicy(npmNetPol, npObj.Spec.Ingress) + err := ingressPolicy(npmNetPol, netPolName, npObj.Spec.Ingress) if err != nil { return nil, err } } else { - err := egressPolicy(npmNetPol, npObj.Spec.Egress) + err := egressPolicy(npmNetPol, netPolName, npObj.Spec.Egress) if err != nil { return nil, err } diff --git a/npm/pkg/controlplane/translation/translatePolicy_test.go b/npm/pkg/controlplane/translation/translatePolicy_test.go index 3f044e8733..c446e118d6 100644 --- a/npm/pkg/controlplane/translation/translatePolicy_test.go +++ b/npm/pkg/controlplane/translation/translatePolicy_test.go @@ -1,6 +1,8 @@ package translation import ( + "fmt" + "strings" "testing" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" @@ -16,8 +18,11 @@ import ( const ( nonIncluded bool = false namedPortStr string = "serve-tcp" + defaultNS string = "default" ) +var namedPortPolicyKey = fmt.Sprintf("%s/%s", defaultNS, namedPortStr) + func TestPortType(t *testing.T) { tcp := v1.ProtocolTCP port8000 := intstr.FromInt(8000) @@ -314,12 +319,12 @@ func TestIPBlockSetName(t *testing.T) { }{ { name: "default/test (ingress)", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), want: "test-in-ns-default-0-0IN", }, { name: "default/test (ingress)", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 1, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 1, 0), want: "test-in-ns-default-1-0IN", }, { @@ -349,13 +354,13 @@ func TestIPBlockIPSet(t *testing.T) { }{ { name: "empty ipblock rule", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: nil, translatedIPSet: nil, }, { name: "incorrect ipblock rule with only except", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "", Except: []string{"172.17.1.0/24"}, @@ -364,7 +369,7 @@ func TestIPBlockIPSet(t *testing.T) { }, { name: "only cidr", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "172.17.0.0/16", }, @@ -372,7 +377,7 @@ func TestIPBlockIPSet(t *testing.T) { }, { name: "one cidr and one element in except", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "172.17.0.0/16", Except: []string{"172.17.1.0/24"}, @@ -381,7 +386,7 @@ func TestIPBlockIPSet(t *testing.T) { }, { name: "one cidr and multiple elements in except", - ipBlockInfo: createIPBlockInfo("test-network-policy", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test-network-policy", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "172.17.0.0/16", Except: []string{"172.17.1.0/24", "172.17.2.0/24"}, @@ -390,7 +395,7 @@ func TestIPBlockIPSet(t *testing.T) { }, { name: "one cidr and multiple and duplicated elements in except", - ipBlockInfo: createIPBlockInfo("test-network-policy", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test-network-policy", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "172.17.0.0/16", Except: []string{"172.17.1.0/24", "172.17.2.0/24", "172.17.2.0/24"}, @@ -399,7 +404,7 @@ func TestIPBlockIPSet(t *testing.T) { }, { name: "cidr : 0.0.0.0/0", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "0.0.0.0/0", }, @@ -407,7 +412,7 @@ func TestIPBlockIPSet(t *testing.T) { }, { name: "cidr: 0.0.0.0/0 and except: 10.0.0.0/1", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "0.0.0.0/0", Except: []string{"10.0.0.0/1"}, @@ -416,7 +421,7 @@ func TestIPBlockIPSet(t *testing.T) { }, { name: "cidr: 0.0.0.0/0 and except: 0.0.0.0/1", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "0.0.0.0/0", Except: []string{"0.0.0.0/1"}, @@ -425,7 +430,7 @@ func TestIPBlockIPSet(t *testing.T) { }, { name: "cidr: 0.0.0.0/0 and except: 128.0.0.0/1", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "0.0.0.0/0", Except: []string{"128.0.0.0/1"}, @@ -434,7 +439,7 @@ func TestIPBlockIPSet(t *testing.T) { }, { name: "cidr: 0.0.0.0/0 and except: 0.0.0.0/1 and 128.0.0.0/1", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "0.0.0.0/0", Except: []string{"0.0.0.0/1", "128.0.0.0/1"}, @@ -443,7 +448,7 @@ func TestIPBlockIPSet(t *testing.T) { }, { name: "cidr: 0.0.0.0/0 and except: 0.0.0.0/1 and two 128.0.0.0/1", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "0.0.0.0/0", Except: []string{"0.0.0.0/1", "128.0.0.0/1", "128.0.0.0/1"}, @@ -472,14 +477,14 @@ func TestIPBlockRule(t *testing.T) { }{ { name: "empty ipblock rule ", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: nil, translatedIPSet: nil, setInfo: policies.SetInfo{}, }, { name: "incorrect ipblock rule with only except", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "", Except: []string{"172.17.1.0/24"}, @@ -489,7 +494,7 @@ func TestIPBlockRule(t *testing.T) { }, { name: "only cidr", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "172.17.0.0/16", }, @@ -498,7 +503,7 @@ func TestIPBlockRule(t *testing.T) { }, { name: "one cidr and one element in except", - ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "172.17.0.0/16", Except: []string{"172.17.1.0/24"}, @@ -508,7 +513,7 @@ func TestIPBlockRule(t *testing.T) { }, { name: "one cidr and multiple elements in except", - ipBlockInfo: createIPBlockInfo("test-network-policy", "default", policies.Ingress, policies.SrcMatch, 0, 0), + ipBlockInfo: createIPBlockInfo("test-network-policy", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0), ipBlockRule: &networkingv1.IPBlock{ CIDR: "172.17.0.0/16", Except: []string{"172.17.1.0/24", "172.17.2.0/24"}, @@ -541,16 +546,16 @@ func TestPodSelector(t *testing.T) { }{ { name: "all pods selector in default namespace in ingress", - namespace: "default", + namespace: defaultNS, matchType: matchType, labelSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{}, }, podSelectorIPSets: []*ipsets.TranslatedIPSet{ - ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, podSelectorList: []policies.SetInfo{ - policies.NewSetInfo("default", ipsets.Namespace, included, matchType), + policies.NewSetInfo(defaultNS, ipsets.Namespace, included, matchType), }, }, { @@ -926,7 +931,7 @@ func TestDefaultDropACL(t *testing.T) { { name: "Default drop acl for default/test", policyName: "test", - policyNS: "default", + policyNS: defaultNS, direction: direction, dropACL: &policies.ACLPolicy{ PolicyID: "azure-acl-default-test", @@ -1134,8 +1139,8 @@ func TestPeerAndPortRule(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: namedPortStr, - NameSpace: "default", + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, ACLs: []*policies.ACLPolicy{ { PolicyID: "azure-acl-default-serve-tcp", @@ -1160,8 +1165,8 @@ func TestPeerAndPortRule(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: namedPortStr, - NameSpace: "default", + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("serve-tcp", ipsets.NamedPorts), }, @@ -1188,8 +1193,8 @@ func TestPeerAndPortRule(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: namedPortStr, - NameSpace: "default", + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("serve-tcp", ipsets.NamedPorts), }, @@ -1218,8 +1223,8 @@ func TestPeerAndPortRule(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: namedPortStr, - NameSpace: "default", + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("serve-tcp", ipsets.NamedPorts), }, @@ -1246,8 +1251,8 @@ func TestPeerAndPortRule(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: namedPortStr, - NameSpace: "default", + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("serve-tcp", ipsets.NamedPorts), }, @@ -1276,8 +1281,8 @@ func TestPeerAndPortRule(t *testing.T) { acl.SrcList = setInfo } npmNetPol := &policies.NPMNetworkPolicy{ - Name: tt.npmNetPol.Name, - NameSpace: tt.npmNetPol.NameSpace, + Namespace: tt.npmNetPol.Namespace, + PolicyKey: tt.npmNetPol.PolicyKey, } err := peerAndPortRule(npmNetPol, policies.Ingress, tt.ports, setInfo) require.NoError(t, err) @@ -1316,15 +1321,15 @@ func TestIngressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "serve-tcp", - NameSpace: "default", + Namespace: defaultNS, + PolicyKey: fmt.Sprintf("%s/%s", defaultNS, namedPortStr), PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), - ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), - policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), + policies.NewSetInfo(defaultNS, ipsets.Namespace, included, targetPodMatchType), }, ACLs: []*policies.ACLPolicy{ { @@ -1337,7 +1342,7 @@ func TestIngressPolicy(t *testing.T) { }, Protocol: "TCP", }, - defaultDropACL("default", "serve-tcp", policies.Ingress), + defaultDropACL(defaultNS, "serve-tcp", policies.Ingress), }, }, }, @@ -1361,15 +1366,15 @@ func TestIngressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "only-ipblock", - NameSpace: "default", + Namespace: defaultNS, + PolicyKey: fmt.Sprintf("%s/%s", defaultNS, "only-ipblock"), PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), - ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), - policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), + policies.NewSetInfo(defaultNS, ipsets.Namespace, included, targetPodMatchType), }, RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("only-ipblock-in-ns-default-0-0IN", ipsets.CIDRBlocks, []string{"172.17.0.0/16", "172.17.1.0/24 nomatch"}...), @@ -1383,7 +1388,7 @@ func TestIngressPolicy(t *testing.T) { policies.NewSetInfo("only-ipblock-in-ns-default-0-0IN", ipsets.CIDRBlocks, included, peerMatchType), }, }, - defaultDropACL("default", "only-ipblock", policies.Ingress), + defaultDropACL(defaultNS, "only-ipblock", policies.Ingress), }, }, }, @@ -1408,19 +1413,19 @@ func TestIngressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "only-peer-podSelector", - NameSpace: "default", + Namespace: defaultNS, + PolicyKey: fmt.Sprintf("%s/%s", defaultNS, "only-peer-podSelector"), PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), - ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), - policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), + policies.NewSetInfo(defaultNS, ipsets.Namespace, included, targetPodMatchType), }, RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("peer-podselector-kay:peer-podselector-value", ipsets.KeyValueLabelOfPod), - ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, ACLs: []*policies.ACLPolicy{ { @@ -1429,10 +1434,10 @@ func TestIngressPolicy(t *testing.T) { Direction: policies.Ingress, SrcList: []policies.SetInfo{ policies.NewSetInfo("peer-podselector-kay:peer-podselector-value", ipsets.KeyValueLabelOfPod, included, peerMatchType), - policies.NewSetInfo("default", ipsets.Namespace, included, peerMatchType), + policies.NewSetInfo(defaultNS, ipsets.Namespace, included, peerMatchType), }, }, - defaultDropACL("default", "only-peer-podSelector", policies.Ingress), + defaultDropACL(defaultNS, "only-peer-podSelector", policies.Ingress), }, }, }, @@ -1457,15 +1462,15 @@ func TestIngressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "only-peer-nsSelector", - NameSpace: "default", + PolicyKey: fmt.Sprintf("%s/%s", defaultNS, "only-peer-nsSelector"), + Namespace: defaultNS, PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), - ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), - policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), + policies.NewSetInfo(defaultNS, ipsets.Namespace, included, targetPodMatchType), }, RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("peer-nsselector-kay:peer-nsselector-value", ipsets.KeyValueLabelOfNamespace), @@ -1479,7 +1484,7 @@ func TestIngressPolicy(t *testing.T) { policies.NewSetInfo("peer-nsselector-kay:peer-nsselector-value", ipsets.KeyValueLabelOfNamespace, included, peerMatchType), }, }, - defaultDropACL("default", "only-peer-nsSelector", policies.Ingress), + defaultDropACL(defaultNS, "only-peer-nsSelector", policies.Ingress), }, }, }, @@ -1515,15 +1520,15 @@ func TestIngressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "only-peer-nsSelector", - NameSpace: "default", + Namespace: defaultNS, + PolicyKey: fmt.Sprintf("%s/%s", defaultNS, "only-peer-nsSelector"), PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), - ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), - policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), + policies.NewSetInfo(defaultNS, ipsets.Namespace, included, targetPodMatchType), }, RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("peer-nsselector-kay:peer-nsselector-value", ipsets.KeyValueLabelOfNamespace), @@ -1555,7 +1560,7 @@ func TestIngressPolicy(t *testing.T) { policies.NewSetInfo("only-peer-nsSelector-in-ns-default-0-2IN", ipsets.CIDRBlocks, included, peerMatchType), }, }, - defaultDropACL("default", "only-peer-nsSelector", policies.Ingress), + defaultDropACL(defaultNS, "only-peer-nsSelector", policies.Ingress), }, }, }, @@ -1577,8 +1582,8 @@ func TestIngressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "serve-tcp", - NameSpace: "default", + Namespace: "default", + PolicyKey: "default/serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1610,8 +1615,8 @@ func TestIngressPolicy(t *testing.T) { {}, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "serve-tcp", - NameSpace: "default", + Namespace: "default", + PolicyKey: "default/serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1638,8 +1643,8 @@ func TestIngressPolicy(t *testing.T) { }, rules: nil, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "serve-tcp", - NameSpace: "default", + Namespace: "default", + PolicyKey: "default/serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1660,13 +1665,16 @@ func TestIngressPolicy(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() npmNetPol := &policies.NPMNetworkPolicy{ - Name: tt.npmNetPol.Name, - NameSpace: tt.npmNetPol.NameSpace, + Namespace: tt.npmNetPol.Namespace, + PolicyKey: tt.npmNetPol.PolicyKey, + ACLPolicyID: tt.npmNetPol.ACLPolicyID, } var err error - npmNetPol.PodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.NameSpace, policies.EitherMatch, tt.targetSelector) + npmNetPol.PodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.Namespace, policies.EitherMatch, tt.targetSelector) require.NoError(t, err) - err = ingressPolicy(npmNetPol, tt.rules) + splitPolicyKey := strings.Split(npmNetPol.PolicyKey, "/") + require.Len(t, splitPolicyKey, 2, "policy key must include name") + err = ingressPolicy(npmNetPol, splitPolicyKey[1], tt.rules) if tt.wantErr { require.Error(t, err) } else { @@ -1706,8 +1714,8 @@ func TestEgressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "serve-tcp", - NameSpace: "default", + Namespace: "default", + PolicyKey: "default/serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1751,8 +1759,8 @@ func TestEgressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "only-ipblock", - NameSpace: "default", + Namespace: "default", + PolicyKey: "default/only-ipblock", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1798,8 +1806,8 @@ func TestEgressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "only-peer-podSelector", - NameSpace: "default", + Namespace: "default", + PolicyKey: "default/only-peer-podSelector", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1847,8 +1855,8 @@ func TestEgressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "only-peer-nsSelector", - NameSpace: "default", + Namespace: "default", + PolicyKey: "default/only-peer-nsSelector", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1882,8 +1890,8 @@ func TestEgressPolicy(t *testing.T) { }, rules: nil, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "serve-tcp", - NameSpace: "default", + Namespace: "default", + PolicyKey: "default/serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1908,8 +1916,8 @@ func TestEgressPolicy(t *testing.T) { {}, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "serve-tcp", - NameSpace: "default", + Namespace: "default", + PolicyKey: "default/serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1959,8 +1967,8 @@ func TestEgressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "only-peer-nsSelector", - NameSpace: "default", + Namespace: "default", + PolicyKey: "default/only-peer-nsSelector", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -2021,8 +2029,8 @@ func TestEgressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Name: "serve-tcp", - NameSpace: "default", + Namespace: "default", + PolicyKey: "default/serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -2050,13 +2058,16 @@ func TestEgressPolicy(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() npmNetPol := &policies.NPMNetworkPolicy{ - Name: tt.npmNetPol.Name, - NameSpace: tt.npmNetPol.NameSpace, + Namespace: tt.npmNetPol.Namespace, + PolicyKey: tt.npmNetPol.PolicyKey, + ACLPolicyID: tt.npmNetPol.ACLPolicyID, } var err error - npmNetPol.PodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.NameSpace, policies.EitherMatch, tt.targetSelector) + npmNetPol.PodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.Namespace, policies.EitherMatch, tt.targetSelector) require.NoError(t, err) - err = egressPolicy(npmNetPol, tt.rules) + splitPolicyKey := strings.Split(npmNetPol.PolicyKey, "/") + require.Len(t, splitPolicyKey, 2, "policy key must include name") + err = egressPolicy(npmNetPol, splitPolicyKey[1], tt.rules) if tt.wantErr { require.Error(t, err) } else { diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index 008e580594..8c4c0fb63d 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -32,8 +32,7 @@ var ( Metadata: ipsets.NewIPSetMetadata("setpodkey1", ipsets.KeyLabelOfPod), } testPolicyobj = policies.NPMNetworkPolicy{ - Name: "testpolicy", - NameSpace: "ns1", + Namespace: "ns1", PolicyKey: "ns1/testpolicy", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ { diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 8951fde5d6..ff3f2fc50f 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -131,20 +131,20 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { return err } - for policyName := range selectorReference { + for policyKey := range selectorReference { // Now check if any of these network policies are applied on this endpoint. // If yes then proceed to delete the network policy // Remove policy should be deleting this netpol reference - if _, ok := endpoint.NetPolReference[policyName]; ok { + if _, ok := endpoint.NetPolReference[policyKey]; ok { // Delete the network policy endpointList := map[string]string{ endpoint.IP: endpoint.ID, } - err := dp.policyMgr.RemovePolicy(policyName, endpointList) + err := dp.policyMgr.RemovePolicy(policyKey, endpointList) if err != nil { return err } - delete(endpoint.NetPolReference, policyName) + delete(endpoint.NetPolReference, policyKey) } } } @@ -157,19 +157,19 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { return err } - for netpol := range selectorReference { - toAddPolicies[netpol] = struct{}{} + for policyKey := range selectorReference { + toAddPolicies[policyKey] = struct{}{} } } // Now check if any of these network policies are applied on this endpoint. // If not then proceed to apply the network policy - for policyName := range toAddPolicies { - if _, ok := endpoint.NetPolReference[policyName]; ok { + for policyKey := range toAddPolicies { + if _, ok := endpoint.NetPolReference[policyKey]; ok { continue } // TODO Also check if the endpoint reference in policy for this Ip is right - netpolSelectorIPs, err := dp.getSelectorIPsByPolicyName(policyName) + netpolSelectorIPs, err := dp.getSelectorIPsByPolicyName(policyKey) if err != nil { return err } @@ -179,9 +179,9 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { } // Apply the network policy - policy, ok := dp.policyMgr.GetPolicy(policyName) + policy, ok := dp.policyMgr.GetPolicy(policyKey) if !ok { - return fmt.Errorf("policy with name %s does not exist", policyName) + return fmt.Errorf("policy with name %s does not exist", policyKey) } endpointList := map[string]string{ @@ -192,16 +192,16 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { return err } - endpoint.NetPolReference[policyName] = struct{}{} + endpoint.NetPolReference[policyKey] = struct{}{} } return nil } -func (dp *DataPlane) getSelectorIPsByPolicyName(policyName string) (map[string]struct{}, error) { - policy, ok := dp.policyMgr.GetPolicy(policyName) +func (dp *DataPlane) getSelectorIPsByPolicyName(policyKey string) (map[string]struct{}, error) { + policy, ok := dp.policyMgr.GetPolicy(policyKey) if !ok { - return nil, fmt.Errorf("policy with name %s does not exist", policyName) + return nil, fmt.Errorf("policy with name %s does not exist", policyKey) } return dp.getSelectorIPsByPolicy(policy) @@ -239,7 +239,7 @@ func (dp *DataPlane) getEndpointsToApplyPolicy(policy *policies.NPMNetworkPolicy } endpointList[ip] = endpoint.ID // TODO make sure this is netpol key and not name - endpoint.NetPolReference[policy.Name] = struct{}{} + endpoint.NetPolReference[policy.PolicyKey] = struct{}{} } return endpointList, nil } diff --git a/npm/pkg/dataplane/dpshim/dpshim_test.go b/npm/pkg/dataplane/dpshim/dpshim_test.go index 461b449a85..0dca2fe86d 100644 --- a/npm/pkg/dataplane/dpshim/dpshim_test.go +++ b/npm/pkg/dataplane/dpshim/dpshim_test.go @@ -31,8 +31,7 @@ var ( Metadata: ipsets.NewIPSetMetadata("setpodkey1", ipsets.KeyLabelOfPod), } testPolicyobj = &policies.NPMNetworkPolicy{ - Name: "testpolicy", - NameSpace: "ns1", + Namespace: "ns1", PolicyKey: "ns1/testpolicy", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ { diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index 8f6f64ac49..be604e70c9 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -134,7 +134,7 @@ type SetType int8 const ( // Unknown SetType UnknownType SetType = 0 - // NameSpace IPSet is created to hold + // Namespace IPSet is created to hold // ips of pods in a given NameSapce Namespace SetType = 1 // KeyLabelOfNamespace IPSet is a list kind ipset @@ -160,7 +160,7 @@ const ( var ( setTypeName = map[SetType]string{ UnknownType: Unknown, - Namespace: "NameSpace", + Namespace: "Namespace", KeyLabelOfNamespace: "KeyLabelOfNameSpace", KeyValueLabelOfNamespace: "KeyValueLabelOfNameSpace", KeyLabelOfPod: "KeyLabelOfPod", @@ -202,7 +202,7 @@ type IPSet struct { // Using a map to emulate set and value as struct{} for // minimal memory consumption // SelectorReference holds networkpolicy names where this IPSet - // is being used in PodSelector and NameSpace + // is being used in PodSelector and Namespace SelectorReference map[string]struct{} // NetPolReference holds networkpolicy names where this IPSet // is being referred as part of rules diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index 712f0ac776..b436eda7d7 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -11,9 +11,8 @@ import ( ) type NPMNetworkPolicy struct { - Name string - NameSpace string - // TODO remove Name and Namespace field + // Namespace is only used by Linux to construct an iptables comment + Namespace string // PolicyKey is a unique combination of "namespace/name" of network policy PolicyKey string // PodSelectorIPSets holds all the IPSets generated from Pod Selector @@ -32,8 +31,7 @@ type NPMNetworkPolicy struct { func NewNPMNetworkPolicy(netPolName, netPolNamespace string) *NPMNetworkPolicy { return &NPMNetworkPolicy{ - Name: netPolName, - NameSpace: netPolNamespace, + Namespace: netPolNamespace, PolicyKey: fmt.Sprintf("%s/%s", netPolNamespace, netPolName), } } @@ -76,12 +74,12 @@ func (netPol *NPMNetworkPolicy) PrettyString() string { podSelectorIPSetString := translatedIPSetsToString(netPol.PodSelectorIPSets) podSelectorListString := infoArrayToString(netPol.PodSelectorList) - format := `Name:%s Namespace:%s + format := `Namespace/Name: %s PodSelectorIPSets: %s PodSelectorList: %s ACLs: %s` - return fmt.Sprintf(format, netPol.Name, netPol.NameSpace, podSelectorIPSetString, podSelectorListString, aclArrayString) + return fmt.Sprintf(format, netPol.PolicyKey, podSelectorIPSetString, podSelectorListString, aclArrayString) } // ACLPolicy equivalent to a single iptable rule in linux diff --git a/npm/pkg/dataplane/policies/policy_linux.go b/npm/pkg/dataplane/policies/policy_linux.go index aea5a34e19..e26745c6a2 100644 --- a/npm/pkg/dataplane/policies/policy_linux.go +++ b/npm/pkg/dataplane/policies/policy_linux.go @@ -67,7 +67,7 @@ func (networkPolicy *NPMNetworkPolicy) commentForJump(direction UniqueDirection) 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) + return fmt.Sprintf("%s-POLICY-%s-%s-%s-IN-ns-%s", prefix, networkPolicy.PolicyKey, toFrom, podSelectorComment, networkPolicy.Namespace) } func commentForInfos(infos []SetInfo) string { diff --git a/npm/pkg/dataplane/policies/policymanager_linux_test.go b/npm/pkg/dataplane/policies/policymanager_linux_test.go index 9e26fb5589..36aae4d329 100644 --- a/npm/pkg/dataplane/policies/policymanager_linux_test.go +++ b/npm/pkg/dataplane/policies/policymanager_linux_test.go @@ -107,8 +107,7 @@ var ( // NetworkPolicies var ( bothDirectionsNetPol = &NPMNetworkPolicy{ - Name: "test1", - NameSpace: "x", + Namespace: "x", PolicyKey: "x/test1", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ {Metadata: ipsets.TestKeyPodSet.Metadata}, @@ -128,8 +127,7 @@ var ( }, } ingressNetPol = &NPMNetworkPolicy{ - Name: "test2", - NameSpace: "y", + Namespace: "y", PolicyKey: "y/test2", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ {Metadata: ipsets.TestKeyPodSet.Metadata}, @@ -152,8 +150,7 @@ var ( }, } egressNetPol = &NPMNetworkPolicy{ - Name: "test3", - NameSpace: "z", + Namespace: "z", PolicyKey: "z/test3", ACLs: []*ACLPolicy{ egressAllowedACL, diff --git a/npm/pkg/dataplane/policies/policymanager_test.go b/npm/pkg/dataplane/policies/policymanager_test.go index 6c01d9e66b..a9b5027b71 100644 --- a/npm/pkg/dataplane/policies/policymanager_test.go +++ b/npm/pkg/dataplane/policies/policymanager_test.go @@ -24,8 +24,7 @@ var ( testNSSet = ipsets.NewIPSetMetadata("test-ns-set", ipsets.Namespace) testKeyPodSet = ipsets.NewIPSetMetadata("test-keyPod-set", ipsets.KeyLabelOfPod) testNetPol = &NPMNetworkPolicy{ - Name: "test-netpol", - NameSpace: "x", + Namespace: "x", PolicyKey: "x/test-netpol", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ { @@ -138,8 +137,7 @@ func TestAddEmptyPolicy(t *testing.T) { func TestGetPolicy(t *testing.T) { netpol := &NPMNetworkPolicy{ - Name: "test-netpol", - NameSpace: "x", + Namespace: "x", PolicyKey: "x/test-netpol", ACLs: []*ACLPolicy{ { @@ -217,8 +215,7 @@ func TestNormalizeAndValidatePolicy(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { netPol := &NPMNetworkPolicy{ - Name: "test-netpol", - NameSpace: "x", + Namespace: "x", PolicyKey: "x/test-netpol", ACLs: []*ACLPolicy{tt.acl}, } diff --git a/npm/pkg/dataplane/policies/policymanager_windows.go b/npm/pkg/dataplane/policies/policymanager_windows.go index 3ced28450e..4f5943769e 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows.go +++ b/npm/pkg/dataplane/policies/policymanager_windows.go @@ -47,9 +47,9 @@ func (pMgr *PolicyManager) reconcile() { } func (pMgr *PolicyManager) addPolicy(policy *NPMNetworkPolicy, endpointList map[string]string) error { - klog.Infof("[DataPlane Windows] adding policy %s on %+v", policy.Name, endpointList) + klog.Infof("[DataPlane Windows] adding policy %s on %+v", policy.policyKey, endpointList) if endpointList == nil { - klog.Infof("[DataPlane Windows] No Endpoints to apply policy %s on", policy.Name) + klog.Infof("[DataPlane Windows] No Endpoints to apply policy %s on", policy.policyKey) return nil } @@ -67,12 +67,12 @@ func (pMgr *PolicyManager) addPolicy(policy *NPMNetworkPolicy, endpointList map[ // If the expected ID is not same as epID, there is a chance that old pod got deleted // and same IP is used by new pod with new endpoint. // so we should delete the non-existent endpoint from policy reference - klog.Infof("[DataPlane Windows] PolicyName : %s Endpoint IP: %s's ID %s does not match expected %s", policy.Name, epIP, epID, expectedEpID) + klog.Infof("[DataPlane Windows] PolicyName : %s Endpoint IP: %s's ID %s does not match expected %s", policy.policyKey, epIP, epID, expectedEpID) delete(policy.PodEndpoints, epIP) continue } - klog.Infof("[DataPlane Windows] PolicyName : %s Endpoint IP: %s's ID %s is already in cache", policy.Name, epIP, epID) + klog.Infof("[DataPlane Windows] PolicyName : %s Endpoint IP: %s's ID %s is already in cache", policy.policyKey, epIP, epID) // Deleting the endpoint from EPList so that the policy is not added to this endpoint again delete(endpointList, epIP) } @@ -107,7 +107,7 @@ func (pMgr *PolicyManager) removePolicy(policy *NPMNetworkPolicy, endpointList m if endpointList == nil { if policy.PodEndpoints == nil { - klog.Infof("[DataPlane Windows] No Endpoints to remove policy %s on", policy.Name) + klog.Infof("[DataPlane Windows] No Endpoints to remove policy %s on", policy.policyKey) return nil } endpointList = policy.PodEndpoints @@ -117,7 +117,7 @@ func (pMgr *PolicyManager) removePolicy(policy *NPMNetworkPolicy, endpointList m if err != nil { return err } - klog.Infof("[DataPlane Windows] To Remove Policy: %s \n To Delete ACLs: %+v \n To Remove From %+v endpoints", policy.Name, rulesToRemove, endpointList) + klog.Infof("[DataPlane Windows] To Remove Policy: %s \n To Delete ACLs: %+v \n To Remove From %+v endpoints", policy.policyKey, rulesToRemove, endpointList) // If remove bug is solved we can directly remove the exact policy from the endpoint // but if the bug is not solved then get all existing policies and remove relevant policies from list // then apply remaining policies onto the endpoint diff --git a/npm/pkg/dataplane/policies/testutils.go b/npm/pkg/dataplane/policies/testutils.go index f35b83af50..afd47edeb9 100644 --- a/npm/pkg/dataplane/policies/testutils.go +++ b/npm/pkg/dataplane/policies/testutils.go @@ -7,8 +7,7 @@ var ( // TestNetworkPolicies for testing TestNetworkPolicies = []*NPMNetworkPolicy{ { - Name: "test1", - NameSpace: "x", + Namespace: "x", PolicyKey: "x/test1", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ {Metadata: ipsets.TestKeyPodSet.Metadata}, @@ -28,8 +27,7 @@ var ( ACLs: testACLs, }, { - Name: "test2", - NameSpace: "y", + Namespace: "y", PolicyKey: "y/test2", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ {Metadata: ipsets.TestKeyPodSet.Metadata}, @@ -55,8 +53,7 @@ var ( }, }, { - Name: "test3", - NameSpace: "z", + Namespace: "z", PolicyKey: "z/test3", RuleIPSets: []*ipsets.TranslatedIPSet{ {Metadata: ipsets.TestCIDRSet.Metadata, Members: nil}, From 90b2073e0767fbba964d75814fe4814a7bb90c90 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Tue, 29 Mar 2022 10:50:38 -0700 Subject: [PATCH 2/7] wip moving acl policy id --- .../goalstateprocessor_test.go | 7 +- .../translation/translatePolicy.go | 22 +-- .../translation/translatePolicy_test.go | 165 +++++++++--------- npm/pkg/dataplane/dataplane_test.go | 7 +- npm/pkg/dataplane/dpshim/dpshim_test.go | 6 +- npm/pkg/dataplane/policies/policy.go | 41 ++--- npm/pkg/dataplane/policies/policy_linux.go | 5 + npm/pkg/dataplane/policies/policy_windows.go | 12 +- .../policies/policymanager_linux_test.go | 18 +- .../dataplane/policies/policymanager_test.go | 28 +-- .../policies/policymanager_windows.go | 12 +- .../policies/policymanager_windows_test.go | 12 +- npm/pkg/dataplane/policies/testutils.go | 12 +- test/integration/npm/main.go | 6 +- 14 files changed, 173 insertions(+), 180 deletions(-) diff --git a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go index b3fe82cbdb..1c1aeb7718 100644 --- a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go +++ b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go @@ -28,8 +28,9 @@ var ( testNestedKeyPodSet = ipsets.NewIPSetMetadata("test-nestedkeyPod-set", ipsets.NestedLabelOfPod) testNestedKeyPodCPSet = controlplane.NewControllerIPSets(testNestedKeyPodSet) testNetPol = &policies.NPMNetworkPolicy{ - Namespace: "x", - PolicyKey: "x/test-netpol", + Namespace: "x", + PolicyKey: "x/test-netpol", + ACLPolicyID: "azure-acl-x-test-netpol", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ { Metadata: testNSSet, @@ -48,12 +49,10 @@ var ( }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-123", Target: policies.Dropped, Direction: policies.Ingress, }, { - PolicyID: "azure-acl-234", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{ diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index a5b5ef2061..f04f7da58d 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -304,7 +304,7 @@ func ruleExists(ports []networkingv1.NetworkPolicyPort, peer []networkingv1.Netw // (e.g., IPBlock, podSelector, namespaceSelector, or both podSelector and namespaceSelector). func peerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Direction, ports []networkingv1.NetworkPolicyPort, setInfo []policies.SetInfo) error { if len(ports) == 0 { - acl := policies.NewACLPolicy(npmNetPol.Namespace, npmNetPol.Name, policies.Allowed, direction) + acl := policies.NewACLPolicy(policies.Allowed, direction) acl.AddSetInfo(setInfo) npmNetPol.ACLs = append(npmNetPol.ACLs, acl) return nil @@ -316,7 +316,7 @@ func peerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Di return err } - acl := policies.NewACLPolicy(npmNetPol.Namespace, npmNetPol.Name, policies.Allowed, direction) + acl := policies.NewACLPolicy(policies.Allowed, direction) acl.AddSetInfo(setInfo) npmNetPol.RuleIPSets = portRule(npmNetPol.RuleIPSets, acl, &ports[i], portKind) npmNetPol.ACLs = append(npmNetPol.ACLs, acl) @@ -335,7 +335,7 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, dire // The code inside if condition is to handle allowing all internal traffic, but the case is handled in #2.4. // So, this code may not execute. After confirming this, need to delete it. if !portRuleExists && !peerRuleExists && !allowExternal { - acl := policies.NewACLPolicy(npmNetPol.Namespace, npmNetPol.Name, policies.Allowed, direction) + acl := policies.NewACLPolicy(policies.Allowed, direction) ruleIPSets, allowAllInternalSetInfo := allowAllInternal(matchType) npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, ruleIPSets) acl.AddSetInfo([]policies.SetInfo{allowAllInternalSetInfo}) @@ -351,7 +351,7 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, dire return err } - portACL := policies.NewACLPolicy(npmNetPol.Namespace, npmNetPol.Name, policies.Allowed, direction) + portACL := policies.NewACLPolicy(policies.Allowed, direction) npmNetPol.RuleIPSets = portRule(npmNetPol.RuleIPSets, portACL, &ports[i], portKind) npmNetPol.ACLs = append(npmNetPol.ACLs, portACL) } @@ -441,14 +441,14 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, dire } // defaultDropACL returns ACLPolicy to drop traffic which is not allowed. -func defaultDropACL(policyNS, policyName string, direction policies.Direction) *policies.ACLPolicy { - dropACL := policies.NewACLPolicy(policyNS, policyName, policies.Dropped, direction) +func defaultDropACL(direction policies.Direction) *policies.ACLPolicy { + dropACL := policies.NewACLPolicy(policies.Dropped, direction) return dropACL } // allowAllPolicy adds acl to allow all traffic including internal (i.e,. K8s cluster) and external (i.e., internet) func allowAllPolicy(npmNetPol *policies.NPMNetworkPolicy, direction policies.Direction) { - allowAllACL := policies.NewACLPolicy(npmNetPol.Namespace, npmNetPol.Name, policies.Allowed, direction) + allowAllACL := policies.NewACLPolicy(policies.Allowed, direction) npmNetPol.ACLs = append(npmNetPol.ACLs, allowAllACL) } @@ -473,7 +473,7 @@ func ingressPolicy(npmNetPol *policies.NPMNetworkPolicy, netPolName string, ingr // #2. If ingress is nil (in yaml file, it is specified with '[]'), it means "Deny all" - it does not allow receiving any traffic from others. if ingress == nil { // Except for allow all traffic case in #1, the rest of them should have default drop rules. - dropACL := defaultDropACL(npmNetPol.Namespace, npmNetPol.Name, policies.Ingress) + dropACL := defaultDropACL(policies.Ingress) npmNetPol.ACLs = append(npmNetPol.ACLs, dropACL) return nil } @@ -486,7 +486,7 @@ func ingressPolicy(npmNetPol *policies.NPMNetworkPolicy, netPolName string, ingr } } // Except for allow all traffic case in #1, the rest of them should have default drop rules. - dropACL := defaultDropACL(npmNetPol.Namespace, npmNetPol.Name, policies.Ingress) + dropACL := defaultDropACL(policies.Ingress) npmNetPol.ACLs = append(npmNetPol.ACLs, dropACL) return nil } @@ -512,7 +512,7 @@ func egressPolicy(npmNetPol *policies.NPMNetworkPolicy, netPolName string, egres // #2. If egress is nil (in yaml file, it is specified with '[]'), it means "Deny all" - it does not allow sending traffic to others. if egress == nil { // Except for allow all traffic case in #1, the rest of them should have default drop rules. - dropACL := defaultDropACL(npmNetPol.Namespace, npmNetPol.Name, policies.Egress) + dropACL := defaultDropACL(policies.Egress) npmNetPol.ACLs = append(npmNetPol.ACLs, dropACL) return nil } @@ -528,7 +528,7 @@ func egressPolicy(npmNetPol *policies.NPMNetworkPolicy, netPolName string, egres // #3. Except for allow all traffic case in #1, the rest of them should have default drop rules. // Add drop ACL to drop the rest of traffic which is not specified in Egress Spec. - dropACL := defaultDropACL(npmNetPol.Namespace, npmNetPol.Name, policies.Egress) + dropACL := defaultDropACL(policies.Egress) npmNetPol.ACLs = append(npmNetPol.ACLs, dropACL) return nil } diff --git a/npm/pkg/controlplane/translation/translatePolicy_test.go b/npm/pkg/controlplane/translation/translatePolicy_test.go index c446e118d6..21a396c55d 100644 --- a/npm/pkg/controlplane/translation/translatePolicy_test.go +++ b/npm/pkg/controlplane/translation/translatePolicy_test.go @@ -934,7 +934,6 @@ func TestDefaultDropACL(t *testing.T) { policyNS: defaultNS, direction: direction, dropACL: &policies.ACLPolicy{ - PolicyID: "azure-acl-default-test", Target: policies.Dropped, Direction: direction, }, @@ -945,7 +944,6 @@ func TestDefaultDropACL(t *testing.T) { policyNS: "testns", direction: direction, dropACL: &policies.ACLPolicy{ - PolicyID: "azure-acl-testns-test", Target: policies.Dropped, Direction: direction, }, @@ -956,7 +954,7 @@ func TestDefaultDropACL(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - dropACL := defaultDropACL(tt.policyNS, tt.policyName, tt.direction) + dropACL := defaultDropACL(tt.direction) require.Equal(t, tt.dropACL, dropACL) }) } @@ -1139,11 +1137,11 @@ func TestPeerAndPortRule(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: defaultNS, - PolicyKey: namedPortPolicyKey, + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-serve-tcp", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{}, @@ -1165,14 +1163,14 @@ func TestPeerAndPortRule(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: defaultNS, - PolicyKey: namedPortPolicyKey, + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("serve-tcp", ipsets.NamedPorts), }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-serve-tcp", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{}, @@ -1193,14 +1191,14 @@ func TestPeerAndPortRule(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: defaultNS, - PolicyKey: namedPortPolicyKey, + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("serve-tcp", ipsets.NamedPorts), }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-serve-tcp", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{ @@ -1223,14 +1221,14 @@ func TestPeerAndPortRule(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: defaultNS, - PolicyKey: namedPortPolicyKey, + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("serve-tcp", ipsets.NamedPorts), }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-serve-tcp", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{}, @@ -1251,14 +1249,14 @@ func TestPeerAndPortRule(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: defaultNS, - PolicyKey: namedPortPolicyKey, + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("serve-tcp", ipsets.NamedPorts), }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-serve-tcp", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{}, @@ -1281,8 +1279,9 @@ func TestPeerAndPortRule(t *testing.T) { acl.SrcList = setInfo } npmNetPol := &policies.NPMNetworkPolicy{ - Namespace: tt.npmNetPol.Namespace, - PolicyKey: tt.npmNetPol.PolicyKey, + Namespace: tt.npmNetPol.Namespace, + PolicyKey: tt.npmNetPol.PolicyKey, + ACLPolicyID: tt.npmNetPol.ACLPolicyID, } err := peerAndPortRule(npmNetPol, policies.Ingress, tt.ports, setInfo) require.NoError(t, err) @@ -1321,8 +1320,9 @@ func TestIngressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: defaultNS, - PolicyKey: fmt.Sprintf("%s/%s", defaultNS, namedPortStr), + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), @@ -1333,7 +1333,6 @@ func TestIngressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-serve-tcp", Target: policies.Allowed, Direction: policies.Ingress, DstPorts: policies.Ports{ @@ -1342,7 +1341,7 @@ func TestIngressPolicy(t *testing.T) { }, Protocol: "TCP", }, - defaultDropACL(defaultNS, "serve-tcp", policies.Ingress), + defaultDropACL(policies.Ingress), }, }, }, @@ -1366,8 +1365,9 @@ func TestIngressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: defaultNS, - PolicyKey: fmt.Sprintf("%s/%s", defaultNS, "only-ipblock"), + Namespace: defaultNS, + PolicyKey: fmt.Sprintf("%s/%s", defaultNS, "only-ipblock"), + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, "only-ipblock"), PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), @@ -1381,14 +1381,13 @@ func TestIngressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-only-ipblock", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{ policies.NewSetInfo("only-ipblock-in-ns-default-0-0IN", ipsets.CIDRBlocks, included, peerMatchType), }, }, - defaultDropACL(defaultNS, "only-ipblock", policies.Ingress), + defaultDropACL(policies.Ingress), }, }, }, @@ -1413,8 +1412,9 @@ func TestIngressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: defaultNS, - PolicyKey: fmt.Sprintf("%s/%s", defaultNS, "only-peer-podSelector"), + Namespace: defaultNS, + PolicyKey: fmt.Sprintf("%s/%s", defaultNS, "only-peer-podSelector"), + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, "only-peer-podSelector"), PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), @@ -1429,7 +1429,6 @@ func TestIngressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-only-peer-podSelector", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{ @@ -1437,7 +1436,7 @@ func TestIngressPolicy(t *testing.T) { policies.NewSetInfo(defaultNS, ipsets.Namespace, included, peerMatchType), }, }, - defaultDropACL(defaultNS, "only-peer-podSelector", policies.Ingress), + defaultDropACL(policies.Ingress), }, }, }, @@ -1462,8 +1461,9 @@ func TestIngressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - PolicyKey: fmt.Sprintf("%s/%s", defaultNS, "only-peer-nsSelector"), - Namespace: defaultNS, + PolicyKey: fmt.Sprintf("%s/%s", defaultNS, "only-peer-nsSelector"), + Namespace: defaultNS, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, "only-peer-nsSelector"), PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), @@ -1477,14 +1477,13 @@ func TestIngressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-only-peer-nsSelector", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{ policies.NewSetInfo("peer-nsselector-kay:peer-nsselector-value", ipsets.KeyValueLabelOfNamespace, included, peerMatchType), }, }, - defaultDropACL(defaultNS, "only-peer-nsSelector", policies.Ingress), + defaultDropACL(policies.Ingress), }, }, }, @@ -1520,8 +1519,9 @@ func TestIngressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: defaultNS, - PolicyKey: fmt.Sprintf("%s/%s", defaultNS, "only-peer-nsSelector"), + Namespace: defaultNS, + PolicyKey: fmt.Sprintf("%s/%s", defaultNS, "only-peer-nsSelector"), + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, "only-peer-nsSelector"), PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), @@ -1537,7 +1537,6 @@ func TestIngressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-only-peer-nsSelector", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{ @@ -1545,7 +1544,6 @@ func TestIngressPolicy(t *testing.T) { }, }, { - PolicyID: "azure-acl-default-only-peer-nsSelector", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{ @@ -1553,14 +1551,13 @@ func TestIngressPolicy(t *testing.T) { }, }, { - PolicyID: "azure-acl-default-only-peer-nsSelector", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{ policies.NewSetInfo("only-peer-nsSelector-in-ns-default-0-2IN", ipsets.CIDRBlocks, included, peerMatchType), }, }, - defaultDropACL(defaultNS, "only-peer-nsSelector", policies.Ingress), + defaultDropACL(policies.Ingress), }, }, }, @@ -1582,8 +1579,9 @@ func TestIngressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: "default", - PolicyKey: "default/serve-tcp", + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1594,12 +1592,11 @@ func TestIngressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-serve-tcp", Target: policies.Allowed, Direction: policies.Ingress, Protocol: "TCP", }, - defaultDropACL("default", "serve-tcp", policies.Ingress), + defaultDropACL(policies.Ingress), }, }, wantErr: true, @@ -1615,8 +1612,9 @@ func TestIngressPolicy(t *testing.T) { {}, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: "default", - PolicyKey: "default/serve-tcp", + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1627,7 +1625,6 @@ func TestIngressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-serve-tcp", Target: policies.Allowed, Direction: policies.Ingress, }, @@ -1643,8 +1640,9 @@ func TestIngressPolicy(t *testing.T) { }, rules: nil, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: "default", - PolicyKey: "default/serve-tcp", + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1654,7 +1652,7 @@ func TestIngressPolicy(t *testing.T) { policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), }, ACLs: []*policies.ACLPolicy{ - defaultDropACL("default", "serve-tcp", policies.Ingress), + defaultDropACL(policies.Ingress), }, }, }, @@ -1714,8 +1712,9 @@ func TestEgressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: "default", - PolicyKey: "default/serve-tcp", + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1726,7 +1725,6 @@ func TestEgressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-serve-tcp", Target: policies.Allowed, Direction: policies.Egress, DstPorts: policies.Ports{ @@ -1735,7 +1733,7 @@ func TestEgressPolicy(t *testing.T) { }, Protocol: "TCP", }, - defaultDropACL("default", "serve-tcp", policies.Egress), + defaultDropACL(policies.Egress), }, }, }, @@ -1759,8 +1757,9 @@ func TestEgressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: "default", - PolicyKey: "default/only-ipblock", + Namespace: "default", + PolicyKey: "default/only-ipblock", + ACLPolicyID: "azure-acl-default-only-ipblock", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1774,14 +1773,13 @@ func TestEgressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-only-ipblock", Target: policies.Allowed, Direction: policies.Egress, DstList: []policies.SetInfo{ policies.NewSetInfo("only-ipblock-in-ns-default-0-0OUT", ipsets.CIDRBlocks, included, peerMatchType), }, }, - defaultDropACL("default", "only-ipblock", policies.Egress), + defaultDropACL(policies.Egress), }, }, }, @@ -1806,8 +1804,9 @@ func TestEgressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: "default", - PolicyKey: "default/only-peer-podSelector", + Namespace: "default", + PolicyKey: "default/only-peer-podSelector", + ACLPolicyID: "azure-acl-default-only-peer-podSelector", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1822,7 +1821,6 @@ func TestEgressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-only-peer-podSelector", Target: policies.Allowed, Direction: policies.Egress, DstList: []policies.SetInfo{ @@ -1830,7 +1828,7 @@ func TestEgressPolicy(t *testing.T) { policies.NewSetInfo("default", ipsets.Namespace, included, peerMatchType), }, }, - defaultDropACL("default", "only-peer-podSelector", policies.Egress), + defaultDropACL(policies.Egress), }, }, }, @@ -1855,8 +1853,9 @@ func TestEgressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: "default", - PolicyKey: "default/only-peer-nsSelector", + Namespace: "default", + PolicyKey: "default/only-peer-nsSelector", + ACLPolicyID: "azure-acl-default-only-peer-nsSelector", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1870,14 +1869,13 @@ func TestEgressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-only-peer-nsSelector", Target: policies.Allowed, Direction: policies.Egress, DstList: []policies.SetInfo{ policies.NewSetInfo("peer-nsselector-kay:peer-nsselector-value", ipsets.KeyValueLabelOfNamespace, included, peerMatchType), }, }, - defaultDropACL("default", "only-peer-nsSelector", policies.Egress), + defaultDropACL(policies.Egress), }, }, }, @@ -1890,8 +1888,9 @@ func TestEgressPolicy(t *testing.T) { }, rules: nil, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: "default", - PolicyKey: "default/serve-tcp", + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1901,7 +1900,7 @@ func TestEgressPolicy(t *testing.T) { policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), }, ACLs: []*policies.ACLPolicy{ - defaultDropACL("default", "serve-tcp", policies.Egress), + defaultDropACL(policies.Egress), }, }, }, @@ -1916,8 +1915,9 @@ func TestEgressPolicy(t *testing.T) { {}, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: "default", - PolicyKey: "default/serve-tcp", + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1928,7 +1928,6 @@ func TestEgressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-serve-tcp", Target: policies.Allowed, Direction: policies.Egress, }, @@ -1967,8 +1966,9 @@ func TestEgressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: "default", - PolicyKey: "default/only-peer-nsSelector", + Namespace: "default", + PolicyKey: "default/only-peer-nsSelector", + ACLPolicyID: "azure-acl-default-only-peer-nsSelector", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -1984,7 +1984,6 @@ func TestEgressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-only-peer-nsSelector", Target: policies.Allowed, Direction: policies.Egress, DstList: []policies.SetInfo{ @@ -1992,7 +1991,6 @@ func TestEgressPolicy(t *testing.T) { }, }, { - PolicyID: "azure-acl-default-only-peer-nsSelector", Target: policies.Allowed, Direction: policies.Egress, DstList: []policies.SetInfo{ @@ -2000,14 +1998,13 @@ func TestEgressPolicy(t *testing.T) { }, }, { - PolicyID: "azure-acl-default-only-peer-nsSelector", Target: policies.Allowed, Direction: policies.Egress, DstList: []policies.SetInfo{ policies.NewSetInfo("only-peer-nsSelector-in-ns-default-0-2OUT", ipsets.CIDRBlocks, included, peerMatchType), }, }, - defaultDropACL("default", "only-peer-nsSelector", policies.Egress), + defaultDropACL(policies.Egress), }, }, }, @@ -2029,8 +2026,9 @@ func TestEgressPolicy(t *testing.T) { }, }, npmNetPol: &policies.NPMNetworkPolicy{ - Namespace: "default", - PolicyKey: "default/serve-tcp", + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), @@ -2041,12 +2039,11 @@ func TestEgressPolicy(t *testing.T) { }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-default-serve-tcp", Target: policies.Allowed, Direction: policies.Egress, Protocol: "TCP", }, - defaultDropACL("default", "serve-tcp", policies.Egress), + defaultDropACL(policies.Egress), }, }, wantErr: true, diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index 8c4c0fb63d..bc2b5ca1ee 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -32,8 +32,9 @@ var ( Metadata: ipsets.NewIPSetMetadata("setpodkey1", ipsets.KeyLabelOfPod), } testPolicyobj = policies.NPMNetworkPolicy{ - Namespace: "ns1", - PolicyKey: "ns1/testpolicy", + Namespace: "ns1", + PolicyKey: "ns1/testpolicy", + ACLPolicyID: "azure-acl-ns1-testpolicy", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ { Metadata: ipsets.NewIPSetMetadata("setns1", ipsets.Namespace), @@ -65,7 +66,6 @@ var ( }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "testpol1", Target: policies.Dropped, Direction: policies.Egress, }, @@ -226,7 +226,6 @@ func TestUpdatePolicy(t *testing.T) { updatedTestPolicyobj := testPolicyobj updatedTestPolicyobj.ACLs = []*policies.ACLPolicy{ { - PolicyID: "testpol1", Target: policies.Dropped, Direction: policies.Ingress, }, diff --git a/npm/pkg/dataplane/dpshim/dpshim_test.go b/npm/pkg/dataplane/dpshim/dpshim_test.go index 0dca2fe86d..4e2592eeea 100644 --- a/npm/pkg/dataplane/dpshim/dpshim_test.go +++ b/npm/pkg/dataplane/dpshim/dpshim_test.go @@ -31,8 +31,9 @@ var ( Metadata: ipsets.NewIPSetMetadata("setpodkey1", ipsets.KeyLabelOfPod), } testPolicyobj = &policies.NPMNetworkPolicy{ - Namespace: "ns1", - PolicyKey: "ns1/testpolicy", + Namespace: "ns1", + PolicyKey: "ns1/testpolicy", + ACLPolicyID: "azure-acl-ns1-testpolicy", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ { Metadata: ipsets.NewIPSetMetadata("setns1", ipsets.Namespace), @@ -64,7 +65,6 @@ var ( }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "testpol1", Target: policies.Dropped, Direction: policies.Egress, }, diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index b436eda7d7..23898c8531 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -15,6 +15,8 @@ type NPMNetworkPolicy struct { Namespace string // PolicyKey is a unique combination of "namespace/name" of network policy PolicyKey string + // ACLPolicyID is only used in Windows. See aclPolicyID() in policy_windows.go for more info + ACLPolicyID string // PodSelectorIPSets holds all the IPSets generated from Pod Selector PodSelectorIPSets []*ipsets.TranslatedIPSet // TODO change to slice of pointers @@ -31,8 +33,9 @@ type NPMNetworkPolicy struct { func NewNPMNetworkPolicy(netPolName, netPolNamespace string) *NPMNetworkPolicy { return &NPMNetworkPolicy{ - Namespace: netPolNamespace, - PolicyKey: fmt.Sprintf("%s/%s", netPolNamespace, netPolName), + Namespace: netPolNamespace, + PolicyKey: fmt.Sprintf("%s/%s", netPolNamespace, netPolName), + ACLPolicyID: aclPolicyID(netPolName, netPolNamespace), } } @@ -85,10 +88,6 @@ ACLs: // ACLPolicy equivalent to a single iptable rule in linux // or a single HNS rule in windows type ACLPolicy struct { - // PolicyID is the rules name with a given network policy - // PolicyID will be same for all ACLs in a Network Policy - // it will be "azure-acl-NetPolNS-netPolName" - PolicyID string // Comment is the string attached to rule to identity its representation Comment string // TODO(jungukcho): now I think we do not need to manage SrcList and DstList @@ -112,16 +111,6 @@ type ACLPolicy struct { Protocol Protocol } -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. -func aclPolicyID(policyNS, policyName string) string { - return fmt.Sprintf("%s-%s-%s", policyIDPrefix, policyNS, policyName) -} - // NormalizePolicy helps fill in missed fields in aclPolicy func NormalizePolicy(networkPolicy *NPMNetworkPolicy) { for _, aclPolicy := range networkPolicy.ACLs { @@ -139,44 +128,42 @@ func NormalizePolicy(networkPolicy *NPMNetworkPolicy) { func ValidatePolicy(networkPolicy *NPMNetworkPolicy) error { for _, aclPolicy := range networkPolicy.ACLs { if !aclPolicy.hasKnownTarget() { - return npmerrors.SimpleError(fmt.Sprintf("ACL policy %s has unknown target [%s]", aclPolicy.PolicyID, aclPolicy.Target)) + return npmerrors.SimpleError(fmt.Sprintf("ACL policy for NetPol %s has unknown target [%s]", networkPolicy.PolicyKey, aclPolicy.Target)) } if !aclPolicy.hasKnownDirection() { - return npmerrors.SimpleError(fmt.Sprintf("ACL policy %s has unknown direction [%s]", aclPolicy.PolicyID, aclPolicy.Direction)) + return npmerrors.SimpleError(fmt.Sprintf("ACL policy for NetPol %s has unknown direction [%s]", networkPolicy.PolicyKey, aclPolicy.Direction)) } if !aclPolicy.hasKnownProtocol() { - return npmerrors.SimpleError(fmt.Sprintf("ACL policy %s has unknown protocol [%s]", aclPolicy.PolicyID, aclPolicy.Protocol)) + return npmerrors.SimpleError(fmt.Sprintf("ACL policy for NetPol %s has unknown protocol [%s]", networkPolicy.PolicyKey, aclPolicy.Protocol)) } if !aclPolicy.satisifiesPortAndProtocolConstraints() { return npmerrors.SimpleError(fmt.Sprintf( - "ACL policy %s has dst port(s) (Port or Port and EndPort), so must have protocol tcp, udp, udplite, sctp, or dccp but has protocol %s", - aclPolicy.PolicyID, + "ACL policy for NetPol %s has dst port(s) (Port or Port and EndPort), so must have protocol tcp, udp, udplite, sctp, or dccp but has protocol %s", + networkPolicy.PolicyKey, string(aclPolicy.Protocol), )) } if !aclPolicy.DstPorts.isValidRange() { - return npmerrors.SimpleError(fmt.Sprintf("ACL policy %s has invalid port range in DstPorts (start: %d, end: %d)", aclPolicy.PolicyID, aclPolicy.DstPorts.Port, aclPolicy.DstPorts.EndPort)) + return npmerrors.SimpleError(fmt.Sprintf("ACL policy for NetPol %s has invalid port range in DstPorts (start: %d, end: %d)", networkPolicy.PolicyKey, aclPolicy.DstPorts.Port, aclPolicy.DstPorts.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)) + return npmerrors.SimpleError(fmt.Sprintf("ACL policy for NetPol %s has set %s in SrcList with unknown Match Type", networkPolicy.PolicyKey, 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 npmerrors.SimpleError(fmt.Sprintf("ACL policy for NetPol %s has set %s in DstList with unknown Match Type", networkPolicy.PolicyKey, setInfo.IPSet.Name)) } } } return nil } -// 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 { +func NewACLPolicy(target Verdict, direction Direction) *ACLPolicy { acl := &ACLPolicy{ - PolicyID: aclPolicyID(policyNS, policyName), Target: target, Direction: direction, } diff --git a/npm/pkg/dataplane/policies/policy_linux.go b/npm/pkg/dataplane/policies/policy_linux.go index e26745c6a2..e40cfb5735 100644 --- a/npm/pkg/dataplane/policies/policy_linux.go +++ b/npm/pkg/dataplane/policies/policy_linux.go @@ -20,6 +20,11 @@ const ( maxLengthForMatchSetSpecs = 6 ) +// the NPMNetworkPolicy ACLPolicyID field is unnused in Linux +func aclPolicyID(_, _ string) string { + return "" +} + // returns two booleans indicating whether the network policy has ingress and egress respectively func (networkPolicy *NPMNetworkPolicy) hasIngressAndEgress() (hasIngress, hasEgress bool) { hasIngress = false diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 0b418070c5..5f4fbd16ff 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -11,6 +11,7 @@ import ( const ( blockRulePriotity = 3000 allowRulePriotity = 222 + policyIDPrefix = "azure-acl" ) var ( @@ -27,6 +28,13 @@ var ( ErrProtocolNotSupported = errors.New("Protocol mentioned is not supported") ) +// aclPolicyID returns azure-acl-- format +// to differentiate ACLs among different network policies, +// but aclPolicy in the same network policy has the same aclPolicyID. +func aclPolicyID(policyNS, policyName string) string { + return fmt.Sprintf("%s-%s-%s", policyIDPrefix, policyNS, policyName) +} + // NPMACLPolSettings is an adaption over the existing hcn.ACLPolicySettings // default ACL settings does not contain ID field but HNS is happy with taking an ID // this ID will help us woth correctly identifying the ACL policy when reading from HNS @@ -57,7 +65,7 @@ func (orig NPMACLPolSettings) compare(newACL *NPMACLPolSettings) bool { orig.Priority == newACL.Priority } -func (acl *ACLPolicy) convertToAclSettings() (*NPMACLPolSettings, error) { +func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, error) { policySettings := &NPMACLPolSettings{} for _, setInfo := range acl.SrcList { if !setInfo.Included { @@ -70,7 +78,7 @@ func (acl *ACLPolicy) convertToAclSettings() (*NPMACLPolSettings, error) { } policySettings.RuleType = hcn.RuleTypeSwitch - policySettings.Id = acl.PolicyID + policySettings.Id = aclID policySettings.Direction = getHCNDirection(acl.Direction) policySettings.Action = getHCNAction(acl.Target) diff --git a/npm/pkg/dataplane/policies/policymanager_linux_test.go b/npm/pkg/dataplane/policies/policymanager_linux_test.go index 36aae4d329..6d944890fa 100644 --- a/npm/pkg/dataplane/policies/policymanager_linux_test.go +++ b/npm/pkg/dataplane/policies/policymanager_linux_test.go @@ -15,7 +15,6 @@ import ( ) // ACLs -// Don't care about PolicyID for Linux var ( ingressDeniedACL = &ACLPolicy{ SrcList: []SetInfo{ @@ -50,7 +49,6 @@ var ( Protocol: UnspecifiedProtocol, } egressDeniedACL = &ACLPolicy{ - PolicyID: "acl3", DstList: []SetInfo{ { ipsets.TestCIDRSet.Metadata, @@ -64,7 +62,6 @@ var ( Protocol: UDP, } egressAllowedACL = &ACLPolicy{ - PolicyID: "acl4", DstList: []SetInfo{ { ipsets.TestNamedportSet.Metadata, @@ -107,8 +104,9 @@ var ( // NetworkPolicies var ( bothDirectionsNetPol = &NPMNetworkPolicy{ - Namespace: "x", - PolicyKey: "x/test1", + Namespace: "x", + PolicyKey: "x/test1", + ACLPolicyID: "azure-acl-x-test1", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ {Metadata: ipsets.TestKeyPodSet.Metadata}, }, @@ -127,8 +125,9 @@ var ( }, } ingressNetPol = &NPMNetworkPolicy{ - Namespace: "y", - PolicyKey: "y/test2", + Namespace: "y", + PolicyKey: "y/test2", + ACLPolicyID: "azure-acl-y-test2", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ {Metadata: ipsets.TestKeyPodSet.Metadata}, {Metadata: ipsets.TestNSSet.Metadata}, @@ -150,8 +149,9 @@ var ( }, } egressNetPol = &NPMNetworkPolicy{ - Namespace: "z", - PolicyKey: "z/test3", + Namespace: "z", + PolicyKey: "z/test3", + ACLPolicyID: "azure-acl-z-test3", ACLs: []*ACLPolicy{ egressAllowedACL, }, diff --git a/npm/pkg/dataplane/policies/policymanager_test.go b/npm/pkg/dataplane/policies/policymanager_test.go index a9b5027b71..7c59bd2e5c 100644 --- a/npm/pkg/dataplane/policies/policymanager_test.go +++ b/npm/pkg/dataplane/policies/policymanager_test.go @@ -24,8 +24,9 @@ var ( testNSSet = ipsets.NewIPSetMetadata("test-ns-set", ipsets.Namespace) testKeyPodSet = ipsets.NewIPSetMetadata("test-keyPod-set", ipsets.KeyLabelOfPod) testNetPol = &NPMNetworkPolicy{ - Namespace: "x", - PolicyKey: "x/test-netpol", + Namespace: "x", + PolicyKey: "x/test-netpol", + ACLPolicyID: "azure-acl-x-test-netpol", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ { Metadata: testNSSet, @@ -44,12 +45,10 @@ var ( }, ACLs: []*ACLPolicy{ { - PolicyID: "azure-acl-123", Target: Dropped, Direction: Ingress, }, { - PolicyID: "azure-acl-234", Target: Allowed, Direction: Ingress, SrcList: []SetInfo{ @@ -129,7 +128,11 @@ func TestAddEmptyPolicy(t *testing.T) { metrics.ReinitializeAll() ioshim := common.NewMockIOShim(nil) pMgr := NewPolicyManager(ioshim, ipsetConfig) - require.NoError(t, pMgr.AddPolicy(&NPMNetworkPolicy{PolicyKey: "test"}, nil)) + require.NoError(t, pMgr.AddPolicy(&NPMNetworkPolicy{ + Namespace: "x", + PolicyKey: "x/test-netpol", + ACLPolicyID: "azure-acl-x-test-netpol", + }, nil)) _, ok := pMgr.GetPolicy(testNetPol.PolicyKey) require.False(t, ok) promVals{0, 0}.testPrometheusMetrics(t) @@ -137,11 +140,11 @@ func TestAddEmptyPolicy(t *testing.T) { func TestGetPolicy(t *testing.T) { netpol := &NPMNetworkPolicy{ - Namespace: "x", - PolicyKey: "x/test-netpol", + Namespace: "x", + PolicyKey: "x/test-netpol", + ACLPolicyID: "azure-acl-x-test-netpol", ACLs: []*ACLPolicy{ { - PolicyID: "azure-acl-123", Target: Dropped, Direction: Ingress, }, @@ -193,7 +196,6 @@ func TestNormalizeAndValidatePolicy(t *testing.T) { { name: "valid policy", acl: &ACLPolicy{ - PolicyID: "valid-acl", Target: Dropped, Direction: Ingress, }, @@ -202,7 +204,6 @@ func TestNormalizeAndValidatePolicy(t *testing.T) { { name: "invalid protocol", acl: &ACLPolicy{ - PolicyID: "bad-protocol-acl", Target: Dropped, Direction: Ingress, Protocol: "invalid", @@ -215,9 +216,10 @@ func TestNormalizeAndValidatePolicy(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { netPol := &NPMNetworkPolicy{ - Namespace: "x", - PolicyKey: "x/test-netpol", - ACLs: []*ACLPolicy{tt.acl}, + Namespace: "x", + PolicyKey: "x/test-netpol", + ACLPolicyID: "azure-acl-x-test-netpol", + ACLs: []*ACLPolicy{tt.acl}, } NormalizePolicy(netPol) err := ValidatePolicy(netPol) diff --git a/npm/pkg/dataplane/policies/policymanager_windows.go b/npm/pkg/dataplane/policies/policymanager_windows.go index 4f5943769e..c14e96cb13 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows.go +++ b/npm/pkg/dataplane/policies/policymanager_windows.go @@ -77,7 +77,7 @@ func (pMgr *PolicyManager) addPolicy(policy *NPMNetworkPolicy, endpointList map[ delete(endpointList, epIP) } - rulesToAdd, err := getSettingsFromACL(policy.ACLs) + rulesToAdd, err := getSettingsFromACL(policy) if err != nil { return err } @@ -113,7 +113,7 @@ func (pMgr *PolicyManager) removePolicy(policy *NPMNetworkPolicy, endpointList m endpointList = policy.PodEndpoints } - rulesToRemove, err := getSettingsFromACL(policy.ACLs) + rulesToRemove, err := getSettingsFromACL(policy) if err != nil { return err } @@ -236,10 +236,10 @@ func getEPPolicyReqFromACLSettings(settings []*NPMACLPolSettings) (hcn.PolicyEnd return policyToAdd, nil } -func getSettingsFromACL(acls []*ACLPolicy) ([]*NPMACLPolSettings, error) { - hnsRules := make([]*NPMACLPolSettings, len(acls)) - for i, acl := range acls { - rule, err := acl.convertToAclSettings() +func getSettingsFromACL(policy *NPMNetworkPolicy) ([]*NPMACLPolSettings, error) { + hnsRules := make([]*NPMACLPolSettings, len(policy.ACLs)) + for i, acl := range policy.ACLs { + rule, err := acl.convertToAclSettings(policy.ACLPolicyID) if err != nil { // TODO need some retry mechanism to check why the translations failed return hnsRules, err diff --git a/npm/pkg/dataplane/policies/policymanager_windows_test.go b/npm/pkg/dataplane/policies/policymanager_windows_test.go index 7bc52e2142..228dfc269c 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows_test.go +++ b/npm/pkg/dataplane/policies/policymanager_windows_test.go @@ -16,7 +16,7 @@ import ( var ( expectedACLs = []*hnswrapper.FakeEndpointPolicy{ { - ID: TestNetworkPolicies[0].ACLs[0].PolicyID, + ID: TestNetworkPolicies[0].ACLPolicyID, Protocols: "6", Direction: "In", Action: "Block", @@ -27,7 +27,7 @@ var ( Priority: blockRulePriotity, }, { - ID: TestNetworkPolicies[0].ACLs[0].PolicyID, + ID: TestNetworkPolicies[0].ACLPolicyID, Protocols: "17", Direction: "In", Action: "Allow", @@ -38,7 +38,7 @@ var ( Priority: allowRulePriotity, }, { - ID: TestNetworkPolicies[0].ACLs[0].PolicyID, + ID: TestNetworkPolicies[0].ACLPolicyID, Protocols: "17", Direction: "Out", Action: "Block", @@ -49,7 +49,7 @@ var ( Priority: blockRulePriotity, }, { - ID: TestNetworkPolicies[0].ACLs[0].PolicyID, + ID: TestNetworkPolicies[0].ACLPolicyID, Protocols: "256", Direction: "Out", Action: "Allow", @@ -93,7 +93,7 @@ func TestAddPolicies(t *testing.T) { err := pMgr.AddPolicy(TestNetworkPolicies[0], endPointIDList) require.NoError(t, err) - aclID := TestNetworkPolicies[0].ACLs[0].PolicyID + aclID := TestNetworkPolicies[0].ACLPolicyID aclPolicies, err := hns.Cache.ACLPolicies(endPointIDList, aclID) require.NoError(t, err) @@ -111,7 +111,7 @@ func TestRemovePolicies(t *testing.T) { err := pMgr.AddPolicy(TestNetworkPolicies[0], endPointIDList) require.NoError(t, err) - aclID := TestNetworkPolicies[0].ACLs[0].PolicyID + aclID := TestNetworkPolicies[0].ACLPolicyID aclPolicies, err := hns.Cache.ACLPolicies(endPointIDList, aclID) require.NoError(t, err) diff --git a/npm/pkg/dataplane/policies/testutils.go b/npm/pkg/dataplane/policies/testutils.go index afd47edeb9..5bb11b6eb4 100644 --- a/npm/pkg/dataplane/policies/testutils.go +++ b/npm/pkg/dataplane/policies/testutils.go @@ -66,8 +66,7 @@ var ( testACLs = []*ACLPolicy{ { - PolicyID: "test1", - Comment: "comment1", + Comment: "comment1", SrcList: []SetInfo{ { ipsets.TestCIDRSet.Metadata, @@ -90,8 +89,7 @@ var ( Protocol: TCP, }, { - PolicyID: "test1", - Comment: "comment2", + Comment: "comment2", SrcList: []SetInfo{ { ipsets.TestCIDRSet.Metadata, @@ -104,8 +102,7 @@ var ( Protocol: UDP, }, { - PolicyID: "test1", - Comment: "comment3", + Comment: "comment3", SrcList: []SetInfo{ { ipsets.TestCIDRSet.Metadata, @@ -121,8 +118,7 @@ var ( Protocol: UDP, }, { - PolicyID: "test1", - Comment: "comment4", + Comment: "comment4", SrcList: []SetInfo{ { ipsets.TestCIDRSet.Metadata, diff --git a/test/integration/npm/main.go b/test/integration/npm/main.go index f94c2007bb..de3ca6556e 100644 --- a/test/integration/npm/main.go +++ b/test/integration/npm/main.go @@ -33,7 +33,9 @@ var ( nodeName = "testNode" testNetPol = &policies.NPMNetworkPolicy{ - PolicyKey: "test/test-netpol", + PolicyKey: "test/test-netpol", + Namespace: "test", + ACLPolicyID: "azure-acl-test-netpol", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ { Metadata: ipsets.TestNSSet.Metadata, @@ -52,12 +54,10 @@ var ( }, ACLs: []*policies.ACLPolicy{ { - PolicyID: "azure-acl-123", Target: policies.Dropped, Direction: policies.Ingress, }, { - PolicyID: "azure-acl-123", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{ From 8a4301771c4574deb43e793e65486e4e74844a05 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Thu, 12 May 2022 09:52:45 -0700 Subject: [PATCH 3/7] fix policy key typo for removing Name field --- npm/pkg/dataplane/policies/policymanager_windows.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/npm/pkg/dataplane/policies/policymanager_windows.go b/npm/pkg/dataplane/policies/policymanager_windows.go index c14e96cb13..63530ae959 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows.go +++ b/npm/pkg/dataplane/policies/policymanager_windows.go @@ -47,9 +47,9 @@ func (pMgr *PolicyManager) reconcile() { } func (pMgr *PolicyManager) addPolicy(policy *NPMNetworkPolicy, endpointList map[string]string) error { - klog.Infof("[DataPlane Windows] adding policy %s on %+v", policy.policyKey, endpointList) + klog.Infof("[DataPlane Windows] adding policy %s on %+v", policy.PolicyKey, endpointList) if endpointList == nil { - klog.Infof("[DataPlane Windows] No Endpoints to apply policy %s on", policy.policyKey) + klog.Infof("[DataPlane Windows] No Endpoints to apply policy %s on", policy.PolicyKey) return nil } @@ -67,12 +67,12 @@ func (pMgr *PolicyManager) addPolicy(policy *NPMNetworkPolicy, endpointList map[ // If the expected ID is not same as epID, there is a chance that old pod got deleted // and same IP is used by new pod with new endpoint. // so we should delete the non-existent endpoint from policy reference - klog.Infof("[DataPlane Windows] PolicyName : %s Endpoint IP: %s's ID %s does not match expected %s", policy.policyKey, epIP, epID, expectedEpID) + klog.Infof("[DataPlane Windows] PolicyName : %s Endpoint IP: %s's ID %s does not match expected %s", policy.PolicyKey, epIP, epID, expectedEpID) delete(policy.PodEndpoints, epIP) continue } - klog.Infof("[DataPlane Windows] PolicyName : %s Endpoint IP: %s's ID %s is already in cache", policy.policyKey, epIP, epID) + klog.Infof("[DataPlane Windows] PolicyName : %s Endpoint IP: %s's ID %s is already in cache", policy.PolicyKey, epIP, epID) // Deleting the endpoint from EPList so that the policy is not added to this endpoint again delete(endpointList, epIP) } @@ -107,7 +107,7 @@ func (pMgr *PolicyManager) removePolicy(policy *NPMNetworkPolicy, endpointList m if endpointList == nil { if policy.PodEndpoints == nil { - klog.Infof("[DataPlane Windows] No Endpoints to remove policy %s on", policy.policyKey) + klog.Infof("[DataPlane Windows] No Endpoints to remove policy %s on", policy.PolicyKey) return nil } endpointList = policy.PodEndpoints @@ -117,7 +117,7 @@ func (pMgr *PolicyManager) removePolicy(policy *NPMNetworkPolicy, endpointList m if err != nil { return err } - klog.Infof("[DataPlane Windows] To Remove Policy: %s \n To Delete ACLs: %+v \n To Remove From %+v endpoints", policy.policyKey, rulesToRemove, endpointList) + klog.Infof("[DataPlane Windows] To Remove Policy: %s \n To Delete ACLs: %+v \n To Remove From %+v endpoints", policy.PolicyKey, rulesToRemove, endpointList) // If remove bug is solved we can directly remove the exact policy from the endpoint // but if the bug is not solved then get all existing policies and remove relevant policies from list // then apply remaining policies onto the endpoint From 04cb841f7dc27a04c1920256c5c63191afb54e06 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Thu, 12 May 2022 11:57:44 -0700 Subject: [PATCH 4/7] fix lint and log --- npm/pkg/dataplane/policies/policy.go | 3 ++- npm/pkg/dataplane/policies/policymanager_windows.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index 23898c8531..d29ddd1553 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -145,7 +145,8 @@ func ValidatePolicy(networkPolicy *NPMNetworkPolicy) error { } if !aclPolicy.DstPorts.isValidRange() { - return npmerrors.SimpleError(fmt.Sprintf("ACL policy for NetPol %s has invalid port range in DstPorts (start: %d, end: %d)", networkPolicy.PolicyKey, aclPolicy.DstPorts.Port, aclPolicy.DstPorts.EndPort)) + return npmerrors.SimpleError(fmt.Sprintf("ACL policy for NetPol %s has invalid port range in DstPorts (start: %d, end: %d)", + networkPolicy.PolicyKey, aclPolicy.DstPorts.Port, aclPolicy.DstPorts.EndPort)) } for _, setInfo := range aclPolicy.SrcList { diff --git a/npm/pkg/dataplane/policies/policymanager_windows.go b/npm/pkg/dataplane/policies/policymanager_windows.go index 63530ae959..c2c77d69ea 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows.go +++ b/npm/pkg/dataplane/policies/policymanager_windows.go @@ -48,7 +48,7 @@ func (pMgr *PolicyManager) reconcile() { func (pMgr *PolicyManager) addPolicy(policy *NPMNetworkPolicy, endpointList map[string]string) error { klog.Infof("[DataPlane Windows] adding policy %s on %+v", policy.PolicyKey, endpointList) - if endpointList == nil { + if len(endpointList) == 0 { klog.Infof("[DataPlane Windows] No Endpoints to apply policy %s on", policy.PolicyKey) return nil } From 14606b5c7d1c3fb019d98aea6eea6a77bae11847 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Thu, 12 May 2022 11:58:06 -0700 Subject: [PATCH 5/7] temp debug logs --- npm/pkg/dataplane/dataplane_windows.go | 6 +++++- npm/pkg/dataplane/ipsets/ipsetmanager_windows.go | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index ff3f2fc50f..b8df5c400c 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -213,6 +213,8 @@ func (dp *DataPlane) getSelectorIPsByPolicy(policy *policies.NPMNetworkPolicy) ( selectorIpSets[ipset.Metadata.GetPrefixName()] = struct{}{} } + klog.Infof("policy %s has policy selector: %+v", policy.PolicyKey, selectorIpSets) // FIXME remove after debugging + return dp.ipsetMgr.GetIPsFromSelectorIPSets(selectorIpSets) } @@ -238,9 +240,9 @@ func (dp *DataPlane) getEndpointsToApplyPolicy(policy *policies.NPMNetworkPolicy continue } endpointList[ip] = endpoint.ID - // TODO make sure this is netpol key and not name endpoint.NetPolReference[policy.PolicyKey] = struct{}{} } + klog.Infof("[DataPlane] Endpoints to apply policy %s: %+v", policy.PolicyKey, endpointList) // FIXME remove after debugging return endpointList, nil } @@ -278,7 +280,9 @@ func (dp *DataPlane) refreshAllPodEndpoints() error { } dp.endpointCache[ep.IP] = ep + klog.Infof("updating endpoint cache to include %s: %+v", ep.IP, ep) // FIXME remove after debugging } + klog.Infof("endpoint cache after refreshing all pod endpoints: %+v", dp.endpointCache) // FIXME remove after debugging return nil } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index a1a5889dc5..851c02092d 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -56,6 +56,7 @@ func (iMgr *IPSetManager) GetIPsFromSelectorIPSets(setList map[string]struct{}) return nil, err } } + klog.Infof("setintersection for getIPsFromSelectorIPSets %+v", setintersections) // FIXME remove after debugging return setintersections, err } From 48056b937321358544675eeecd70f6575a29a06b Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Thu, 12 May 2022 13:11:05 -0700 Subject: [PATCH 6/7] another debug log --- npm/pkg/dataplane/ipsets/ipsetmanager_windows.go | 1 + 1 file changed, 1 insertion(+) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index 851c02092d..06954b223d 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -51,6 +51,7 @@ func (iMgr *IPSetManager) GetIPsFromSelectorIPSets(setList map[string]struct{}) } firstLoop = false } + klog.Infof("set [%s] has ippodkey: %+v", set.Name, set.IPPodKey) // FIXME remove after debugging setintersections, err = set.getSetIntersection(setintersections) if err != nil { return nil, err From 37531b36a9a829ded022ebb0bba41373d4e12904 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Fri, 13 May 2022 08:49:39 -0700 Subject: [PATCH 7/7] update a couple UTs --- .../policies/policymanager_windows_test.go | 3 ++- npm/pkg/dataplane/policies/testutils.go | 15 +++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/npm/pkg/dataplane/policies/policymanager_windows_test.go b/npm/pkg/dataplane/policies/policymanager_windows_test.go index 228dfc269c..2d32bcabf6 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows_test.go +++ b/npm/pkg/dataplane/policies/policymanager_windows_test.go @@ -14,6 +14,7 @@ import ( ) var ( + // TODO fix these expected ACLs (e.g. local/remote addresses and ports are off) expectedACLs = []*hnswrapper.FakeEndpointPolicy{ { ID: TestNetworkPolicies[0].ACLPolicyID, @@ -123,7 +124,7 @@ func TestRemovePolicies(t *testing.T) { verifyFakeHNSCacheACLs(t, expectedACLs, acls) } - err = pMgr.RemovePolicy(TestNetworkPolicies[0].Name, nil) + err = pMgr.RemovePolicy(TestNetworkPolicies[0].PolicyKey, nil) require.NoError(t, err) verifyACLCacheIsCleaned(t, hns, len(endPointIDList)) } diff --git a/npm/pkg/dataplane/policies/testutils.go b/npm/pkg/dataplane/policies/testutils.go index 5bb11b6eb4..aab0f98140 100644 --- a/npm/pkg/dataplane/policies/testutils.go +++ b/npm/pkg/dataplane/policies/testutils.go @@ -7,8 +7,9 @@ var ( // TestNetworkPolicies for testing TestNetworkPolicies = []*NPMNetworkPolicy{ { - Namespace: "x", - PolicyKey: "x/test1", + Namespace: "x", + PolicyKey: "x/test1", + ACLPolicyID: "azure-acl-x-test1", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ {Metadata: ipsets.TestKeyPodSet.Metadata}, }, @@ -27,8 +28,9 @@ var ( ACLs: testACLs, }, { - Namespace: "y", - PolicyKey: "y/test2", + Namespace: "y", + PolicyKey: "y/test2", + ACLPolicyID: "azure-acl-y-test2", PodSelectorIPSets: []*ipsets.TranslatedIPSet{ {Metadata: ipsets.TestKeyPodSet.Metadata}, {Metadata: ipsets.TestKVPodSet.Metadata}, @@ -53,8 +55,9 @@ var ( }, }, { - Namespace: "z", - PolicyKey: "z/test3", + Namespace: "z", + PolicyKey: "z/test3", + ACLPolicyID: "azure-acl-z-test3", RuleIPSets: []*ipsets.TranslatedIPSet{ {Metadata: ipsets.TestCIDRSet.Metadata, Members: nil}, },