Add validations for NodeNetworkPolicy

Prevent ACNP appliedTo Node (NodeNetworkPolicy) with "enableLogging: true". Prevent ACNP appliedTo Node (NodeNetworkPolicy) with other selectors. For antrea-io#6525 Signed-off-by: Kumar Atish <[email protected]>
Atish-iaf · Aug 19, 2024 · 87ad7d0 · 87ad7d0
1 parent 7d8d4b1
commit 87ad7d0
Show file tree

Hide file tree

Showing 3 changed files with 149 additions and 1 deletion.
diff --git a/docs/antrea-node-network-policy.md b/docs/antrea-node-network-policy.md
@@ -118,3 +118,4 @@ spec:
 - FQDN is not supported for ACNPs applied to Nodes.
 - Layer 7 NetworkPolicy is not supported yet.
 - For UDP or SCTP, when the `Reject` action is specified in an egress rule, it behaves identical to the `Drop` action.
+- Traffic logging is not supported for ACNPs applied to Nodes.
diff --git a/pkg/controller/networkpolicy/validate.go b/pkg/controller/networkpolicy/validate.go
@@ -599,13 +599,23 @@ func (v *antreaPolicyValidator) validateAppliedTo(ingress, egress []crdv1beta1.R
 		appliedToEgressRule  = 2
 	)
 
+	appliedToNode := false
 	checkAppliedTo := func(appliedTo []crdv1beta1.AppliedTo, appliedToScope int) (string, bool) {
 		appliedToSvcNum := 0
 		for _, eachAppliedTo := range appliedTo {
 			appliedToFieldsNum := numFieldsSetInStruct(eachAppliedTo)
 			if eachAppliedTo.Group != "" && appliedToFieldsNum > 1 {
 				return "group cannot be set with other peers in appliedTo", false
 			}
+			if eachAppliedTo.NodeSelector != nil {
+				if appliedToScope != appliedToPolicy {
+					return "nodeSelector cannot be set in rule-level appliedTo", false
+				}
+				if appliedToFieldsNum > 1 {
+					return "nodeSelector cannot be set with other peers in appliedTo", false
+				}
+				appliedToNode = true
+			}
 			if eachAppliedTo.ServiceAccount != nil && appliedToFieldsNum > 1 {
 				return "serviceAccount cannot be set with other peers in appliedTo", false
 			}
@@ -618,7 +628,7 @@ func (v *antreaPolicyValidator) validateAppliedTo(ingress, egress []crdv1beta1.R
 				}
 				appliedToSvcNum++
 			}
-			if reason, allowed := checkSelectorsLabels(eachAppliedTo.PodSelector, eachAppliedTo.NamespaceSelector, eachAppliedTo.ExternalEntitySelector); !allowed {
+			if reason, allowed := checkSelectorsLabels(eachAppliedTo.PodSelector, eachAppliedTo.NamespaceSelector, eachAppliedTo.ExternalEntitySelector, eachAppliedTo.NodeSelector); !allowed {
 				return reason, allowed
 			}
 		}
@@ -633,18 +643,24 @@ func (v *antreaPolicyValidator) validateAppliedTo(ingress, egress []crdv1beta1.R
 		return reason, allowed
 	}
 
+	enableLogging := false
 	for _, eachIngress := range ingress {
+		enableLogging = enableLogging || eachIngress.EnableLogging
 		reason, allowed = checkAppliedTo(eachIngress.AppliedTo, appliedToIngressRule)
 		if !allowed {
 			return reason, allowed
 		}
 	}
 	for _, eachEgress := range egress {
+		enableLogging = enableLogging || eachEgress.EnableLogging
 		reason, allowed = checkAppliedTo(eachEgress.AppliedTo, appliedToEgressRule)
 		if !allowed {
 			return reason, allowed
 		}
 	}
+	if enableLogging && appliedToNode {
+		return "traffic logging for NodeNetworkPolicy is not supported", false
+	}
 	return "", true
 }
 

diff --git a/pkg/controller/networkpolicy/validate_test.go b/pkg/controller/networkpolicy/validate_test.go
@@ -450,6 +450,137 @@ func TestValidateAntreaClusterNetworkPolicy(t *testing.T) {
 			operation:      admv1.Create,
 			expectedReason: "",
 		},
+		{
+			name: "acnp-appliedto-node-set-with-psel",
+			policy: &crdv1beta1.ClusterNetworkPolicy{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "acnp-appliedto-node-set-with-psel",
+				},
+				Spec: crdv1beta1.ClusterNetworkPolicySpec{
+					AppliedTo: []crdv1beta1.AppliedTo{
+						{
+							PodSelector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"foo1": "bar1"},
+							},
+							NodeSelector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"foo2": "bar2"},
+							},
+						},
+					},
+				},
+			},
+			operation:      admv1.Create,
+			expectedReason: "nodeSelector cannot be set with other peers in appliedTo",
+		},
+		{
+			name: "acnp-appliedto-node-set-with-nssel",
+			policy: &crdv1beta1.ClusterNetworkPolicy{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "acnp-appliedto-node-set-with-nssel",
+				},
+				Spec: crdv1beta1.ClusterNetworkPolicySpec{
+					AppliedTo: []crdv1beta1.AppliedTo{
+						{
+							NamespaceSelector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"foo1": "bar1"},
+							},
+							NodeSelector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"foo2": "bar2"},
+							},
+						},
+					},
+				},
+			},
+			operation:      admv1.Create,
+			expectedReason: "nodeSelector cannot be set with other peers in appliedTo",
+		},
+		{
+			name: "acnp-rule-appliedto-node-alone",
+			policy: &crdv1beta1.ClusterNetworkPolicy{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "acnp-rule-appliedto-node-alone",
+				},
+				Spec: crdv1beta1.ClusterNetworkPolicySpec{
+					Ingress: []crdv1beta1.Rule{
+						{
+							AppliedTo: []crdv1beta1.AppliedTo{
+								{
+									NodeSelector: &metav1.LabelSelector{
+										MatchLabels: map[string]string{"foo2": "bar2"},
+									},
+								},
+							},
+							Action: &allowAction,
+						},
+					},
+				},
+			},
+			operation:      admv1.Create,
+			expectedReason: "nodeSelector cannot be set in rule-level appliedTo",
+		},
+		{
+			name: "acnp-appliedto-node-alone",
+			policy: &crdv1beta1.ClusterNetworkPolicy{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "acnp-appliedto-node-alone",
+				},
+				Spec: crdv1beta1.ClusterNetworkPolicySpec{
+					AppliedTo: []crdv1beta1.AppliedTo{
+						{
+							NodeSelector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"foo2": "bar2"},
+							},
+						},
+					},
+					Ingress: []crdv1beta1.Rule{
+						{
+							Action: &allowAction,
+							From: []crdv1beta1.NetworkPolicyPeer{
+								{
+									NodeSelector: &metav1.LabelSelector{
+										MatchLabels: map[string]string{"foo1": "bar1"},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			operation:      admv1.Create,
+			expectedReason: "",
+		},
+		{
+			name: "acnp-appliedto-node-with-logging",
+			policy: &crdv1beta1.ClusterNetworkPolicy{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "acnp-appliedto-node-with-logging",
+				},
+				Spec: crdv1beta1.ClusterNetworkPolicySpec{
+					AppliedTo: []crdv1beta1.AppliedTo{
+						{
+							NodeSelector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"foo2": "bar2"},
+							},
+						},
+					},
+					Ingress: []crdv1beta1.Rule{
+						{
+							Action: &allowAction,
+							From: []crdv1beta1.NetworkPolicyPeer{
+								{
+									NodeSelector: &metav1.LabelSelector{
+										MatchLabels: map[string]string{"foo1": "bar1"},
+									},
+								},
+							},
+							EnableLogging: true,
+						},
+					},
+				},
+			},
+			operation:      admv1.Create,
+			expectedReason: "traffic logging for NodeNetworkPolicy is not supported",
+		},
 		{
 			name: "acnp-rule-group-set-with-psel",
 			policy: &crdv1beta1.ClusterNetworkPolicy{