fix: NPM Changed Allow All Policy for Ingress and Egress (#2409)

Changed allow all policy for ingress and egress to allow all even with other rules in netpol and added tests.
Azure · Nov 30, 2023 · a69bd6c · a69bd6c
1 parent 7e90960
commit a69bd6c
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 6 deletions.
diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go
@@ -491,9 +491,13 @@ func allowAllPolicy(npmNetPol *policies.NPMNetworkPolicy, direction policies.Dir
 // isAllowAllToIngress returns true if this network policy allows all traffic from internal (i.e,. K8s cluster) and external (i.e., internet)
 // Otherwise, it returns false.
 func isAllowAllToIngress(ingress []networkingv1.NetworkPolicyIngressRule) bool {
-	return len(ingress) == 1 &&
-		len(ingress[0].Ports) == 0 &&
-		len(ingress[0].From) == 0
+	for _, ingressRule := range ingress {
+		// Allow all if any of the ingress rules are allow all.
+		if len(ingressRule.Ports) == 0 && len(ingressRule.From) == 0 {
+			return true
+		}
+	}
+	return false
 }
 
 // ingressPolicy traslates NetworkPolicyIngressRule in NetworkPolicy object
@@ -530,9 +534,13 @@ func ingressPolicy(npmNetPol *policies.NPMNetworkPolicy, netPolName string, ingr
 // isAllowAllToEgress returns true if this network policy allows all traffic to internal (i.e,. K8s cluster) and external (i.e., internet)
 // Otherwise, it returns false.
 func isAllowAllToEgress(egress []networkingv1.NetworkPolicyEgressRule) bool {
-	return len(egress) == 1 &&
-		len(egress[0].Ports) == 0 &&
-		len(egress[0].To) == 0
+	for _, egressRule := range egress {
+		// Allow all if any of the egress rules are allow all.
+		if len(egressRule.Ports) == 0 && len(egressRule.To) == 0 {
+			return true
+		}
+	}
+	return false
 }
 
 // egressPolicy traslates NetworkPolicyEgressRule in networkpolicy object

diff --git a/npm/pkg/controlplane/translation/translatePolicy_test.go b/npm/pkg/controlplane/translation/translatePolicy_test.go
@@ -1462,6 +1462,7 @@ func TestIngressPolicy(t *testing.T) {
 	targetPodMatchType := policies.EitherMatch
 	peerMatchType := policies.SrcMatch
 	emptyString := intstr.FromString("")
+	port443 := intstr.FromInt(443)
 	// TODO(jungukcho): add test cases with more complex rules
 	tests := []struct {
 		name                  string
@@ -1810,6 +1811,45 @@ func TestIngressPolicy(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "allow all ingress rules works with ports rules",
+			targetSelector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"label": "src",
+				},
+			},
+			rules: []networkingv1.NetworkPolicyIngressRule{
+				{
+					Ports: []networkingv1.NetworkPolicyPort{
+						{
+							Protocol: &tcp,
+							Port:     &port443,
+						},
+					},
+				},
+				{},
+			},
+			npmNetPol: &policies.NPMNetworkPolicy{
+				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),
+				},
+				ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{},
+				PodSelectorList: []policies.SetInfo{
+					policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType),
+					policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType),
+				},
+				ACLs: []*policies.ACLPolicy{
+					{
+						Target:    policies.Allowed,
+						Direction: policies.Ingress,
+					},
+				},
+			},
+		},
 		{
 			name: "deny all in ingress rules",
 			targetSelector: &metav1.LabelSelector{
@@ -2152,6 +2192,7 @@ func TestIngressPolicy(t *testing.T) {
 func TestEgressPolicy(t *testing.T) {
 	tcp := v1.ProtocolTCP
 	emptyString := intstr.FromString("")
+	port443 := intstr.FromInt(443)
 	targetPodMatchType := policies.EitherMatch
 	peerMatchType := policies.DstMatch
 	tests := []struct {
@@ -2410,6 +2451,45 @@ func TestEgressPolicy(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "allow all egress rules works with ports rules",
+			targetSelector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"label": "dst",
+				},
+			},
+			rules: []networkingv1.NetworkPolicyEgressRule{
+				{
+					Ports: []networkingv1.NetworkPolicyPort{
+						{
+							Protocol: &tcp,
+							Port:     &port443,
+						},
+					},
+				},
+				{},
+			},
+			npmNetPol: &policies.NPMNetworkPolicy{
+				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),
+				},
+				ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{},
+				PodSelectorList: []policies.SetInfo{
+					policies.NewSetInfo("label:dst", ipsets.KeyValueLabelOfPod, included, targetPodMatchType),
+					policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType),
+				},
+				ACLs: []*policies.ACLPolicy{
+					{
+						Target:    policies.Allowed,
+						Direction: policies.Egress,
+					},
+				},
+			},
+		},
 		{
 			name: "peer nameSpaceSelector and ipblock in egress rules",
 			targetSelector: &metav1.LabelSelector{