From 4f9cbfda593852016770af00c9e9bf072c09e81c Mon Sep 17 00:00:00 2001 From: Kumar Atish Date: Mon, 19 Aug 2024 10:42:35 +0530 Subject: [PATCH] Add validations for NodeNetworkPolicy Prevent ACNP appliedTo Node (NodeNetworkPolicy) with "enableLogging: true". Prevent ACNP appliedTo Node (NodeNetworkPolicy) with other selectors. For #6525 Signed-off-by: Kumar Atish --- docs/antrea-node-network-policy.md | 1 + pkg/controller/networkpolicy/validate.go | 15 ++- pkg/controller/networkpolicy/validate_test.go | 107 ++++++++++++++++++ 3 files changed, 122 insertions(+), 1 deletion(-) diff --git a/docs/antrea-node-network-policy.md b/docs/antrea-node-network-policy.md index 0a686362b06..8f79d0e6c39 100644 --- 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 yet for ACNPs applied to Nodes. diff --git a/pkg/controller/networkpolicy/validate.go b/pkg/controller/networkpolicy/validate.go index 5639866e452..e0bda05924b 100644 --- a/pkg/controller/networkpolicy/validate.go +++ b/pkg/controller/networkpolicy/validate.go @@ -599,6 +599,7 @@ 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 { @@ -606,6 +607,12 @@ func (v *antreaPolicyValidator) validateAppliedTo(ingress, egress []crdv1beta1.R if eachAppliedTo.Group != "" && appliedToFieldsNum > 1 { return "group cannot be set with other peers in appliedTo", false } + if eachAppliedTo.NodeSelector != nil { + 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 +625,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 +640,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 index 271da1e06c1..94d8bbe8cc3 100644 --- a/pkg/controller/networkpolicy/validate_test.go +++ b/pkg/controller/networkpolicy/validate_test.go @@ -450,6 +450,113 @@ 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-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{