Add omitempty to appliedTo fields and address test failures

build/yamls/antrea-aks.yml

@@ -193,7 +193,6 @@ spec:
               tier:
                 type: string
             required:
-            - appliedTo
             - priority
             type: object
         type: object

build/yamls/antrea-eks.yml

@@ -193,7 +193,6 @@ spec:
               tier:
                 type: string
             required:
-            - appliedTo
             - priority
             type: object
         type: object

build/yamls/antrea-gke.yml

@@ -193,7 +193,6 @@ spec:
               tier:
                 type: string
             required:
-            - appliedTo
             - priority
             type: object
         type: object

build/yamls/antrea-ipsec.yml

@@ -193,7 +193,6 @@ spec:
               tier:
                 type: string
             required:
-            - appliedTo
             - priority
             type: object
         type: object

build/yamls/antrea.yml

@@ -193,7 +193,6 @@ spec:
               tier:
                 type: string
             required:
-            - appliedTo
             - priority
             type: object
         type: object

build/yamls/base/crds.yml

@@ -279,9 +279,8 @@ spec:
           type: object
           properties:
             spec:
-              # Ensure that Spec.AppliedTo and Spec.Priority fields are set
+              # Ensure that Spec.Priority field is set
               required:
-                - appliedTo
                 - priority
               type: object
               properties:

pkg/apis/security/v1alpha1/types.go

@@ -47,7 +47,7 @@ type NetworkPolicySpec struct {
 	// Select workloads on which the rules will be applied to. Cannot be set in
 	// conjunction with AppliedTo in each rule.
 	// +optional
-	AppliedTo []NetworkPolicyPeer `json:"appliedTo"`
+	AppliedTo []NetworkPolicyPeer `json:"appliedTo,omitempty"`
 	// Set of ingress rules evaluated based on the order in which they are set.
 	// Currently Ingress rule supports setting the `From` field but not the `To`
 	// field within a Rule.
@@ -86,7 +86,7 @@ type Rule struct {
 	// Select workloads on which this rule will be applied to. Cannot be set in
 	// conjunction with NetworkPolicySpec/ClusterNetworkPolicySpec.AppliedTo.
 	// +optional
-	AppliedTo []NetworkPolicyPeer `json:"appliedTo"`
+	AppliedTo []NetworkPolicyPeer `json:"appliedTo,omitempty"`
 }
 
 // NetworkPolicyPeer describes the grouping selector of workloads.
@@ -114,6 +114,7 @@ type NetworkPolicyPeer struct {
 	// ExternalEntities are matched from Namespaces matched by the
 	// NamespaceSelector.
 	// Cannot be set with any other selector except NamespaceSelector.
+	// +optional
 	ExternalEntitySelector *metav1.LabelSelector `json:"externalEntitySelector,omitempty"`
 }
 
@@ -186,7 +187,8 @@ type ClusterNetworkPolicySpec struct {
 	Priority float64 `json:"priority"`
 	// Select workloads on which the rules will be applied to. Cannot be set in
 	// conjunction with AppliedTo in each rule.
-	AppliedTo []NetworkPolicyPeer `json:"appliedTo"`
+	// +optional
+	AppliedTo []NetworkPolicyPeer `json:"appliedTo,omitempty"`
 	// Set of ingress rules evaluated based on the order in which they are set.
 	// Currently Ingress rule supports setting the `From` field but not the `To`
 	// field within a Rule.

pkg/controller/networkpolicy/antreanetworkpolicy.go

@@ -138,11 +138,11 @@ func (n *NetworkPolicyController) processAntreaNetworkPolicy(np *secv1alpha1.Net
 	for idx, ingressRule := range np.Spec.Ingress {
 		// Set default action to ALLOW to allow traffic.
 		services, namedPortExists := toAntreaServicesForCRD(ingressRule.Ports)
-		appliedToGroupNamesForRule := make([]string, len(ingressRule.AppliedTo))
+		var appliedToGroupNamesForRule []string
 		// Create AppliedToGroup for each AppliedTo present in the ingress rule.
-		for i, at := range ingressRule.AppliedTo {
+		for _, at := range ingressRule.AppliedTo {
 			atGroup := n.createAppliedToGroup(np.Namespace, at.PodSelector, at.NamespaceSelector, at.ExternalEntitySelector)
-			appliedToGroupNamesForRule[i] = atGroup
+			appliedToGroupNamesForRule = append(appliedToGroupNamesForRule, atGroup)
 			appliedToGroupNamesSet.Insert(atGroup)
 		}
 		rules = append(rules, controlplane.NetworkPolicyRule{
@@ -158,11 +158,11 @@ func (n *NetworkPolicyController) processAntreaNetworkPolicy(np *secv1alpha1.Net
 	for idx, egressRule := range np.Spec.Egress {
 		// Set default action to ALLOW to allow traffic.
 		services, namedPortExists := toAntreaServicesForCRD(egressRule.Ports)
-		appliedToGroupNamesForRule := make([]string, len(egressRule.AppliedTo))
+		var appliedToGroupNamesForRule []string
 		// Create AppliedToGroup for each AppliedTo present in the ingress rule.
-		for i, at := range egressRule.AppliedTo {
+		for _, at := range egressRule.AppliedTo {
 			atGroup := n.createAppliedToGroup(np.Namespace, at.PodSelector, at.NamespaceSelector, at.ExternalEntitySelector)
-			appliedToGroupNamesForRule[i] = atGroup
+			appliedToGroupNamesForRule = append(appliedToGroupNamesForRule, atGroup)
 			appliedToGroupNamesSet.Insert(atGroup)
 		}
 		rules = append(rules, controlplane.NetworkPolicyRule{

pkg/controller/networkpolicy/clusternetworkpolicy.go

@@ -138,11 +138,11 @@ func (n *NetworkPolicyController) processClusterNetworkPolicy(cnp *secv1alpha1.C
 	for idx, ingressRule := range cnp.Spec.Ingress {
 		// Set default action to ALLOW to allow traffic.
 		services, namedPortExists := toAntreaServicesForCRD(ingressRule.Ports)
-		appliedToGroupNamesForRule := make([]string, len(ingressRule.AppliedTo))
+		var appliedToGroupNamesForRule []string
 		// Create AppliedToGroup for each AppliedTo present in the ingress rule.
-		for i, at := range ingressRule.AppliedTo {
+		for _, at := range ingressRule.AppliedTo {
 			atGroup := n.createAppliedToGroup("", at.PodSelector, at.NamespaceSelector, at.ExternalEntitySelector)
-			appliedToGroupNamesForRule[i] = atGroup
+			appliedToGroupNamesForRule = append(appliedToGroupNamesForRule, atGroup)
 			appliedToGroupNamesSet.Insert(atGroup)
 		}
 		rules = append(rules, controlplane.NetworkPolicyRule{
@@ -158,11 +158,11 @@ func (n *NetworkPolicyController) processClusterNetworkPolicy(cnp *secv1alpha1.C
 	for idx, egressRule := range cnp.Spec.Egress {
 		// Set default action to ALLOW to allow traffic.
 		services, namedPortExists := toAntreaServicesForCRD(egressRule.Ports)
-		appliedToGroupNamesForRule := make([]string, len(egressRule.AppliedTo))
+		var appliedToGroupNamesForRule []string
 		// Create AppliedToGroup for each AppliedTo present in the ingress rule.
-		for i, at := range egressRule.AppliedTo {
+		for _, at := range egressRule.AppliedTo {
 			atGroup := n.createAppliedToGroup("", at.PodSelector, at.NamespaceSelector, at.ExternalEntitySelector)
-			appliedToGroupNamesForRule[i] = atGroup
+			appliedToGroupNamesForRule = append(appliedToGroupNamesForRule, atGroup)
 			appliedToGroupNamesSet.Insert(atGroup)
 		}
 		rules = append(rules, controlplane.NetworkPolicyRule{

pkg/controller/networkpolicy/validate.go

@@ -134,19 +134,19 @@ func (v *NetworkPolicyValidator) validateAntreaPolicy(op admv1.Operation, tier s
 		if tier != "" && !staticTierSet.Has(tier) && !v.tierExists(tier) {
 			allowed, reason = false, fmt.Sprintf("tier %s does not exist", tier)
 		}
-    appliedToInSpec, appliedToInRules := specAppliedTo != nil, false
-    for i, rule := range append(ingress, egress...) {
-    	if i == 0 {
-    		appliedToInRules = rule.AppliedTo != nil
+		appliedToInSpec, appliedToInRules := specAppliedTo != nil, false
+		for i, rule := range append(ingress, egress...) {
+			if i == 0 {
+				appliedToInRules = rule.AppliedTo != nil
+			}
+			if (rule.AppliedTo != nil) != appliedToInRules {
+				allowed, reason = false, fmt.Sprintf("appliedTo field does not appear consistently in all rules")
+				return reason, allowed
+			}
 		}
-    	if (rule.AppliedTo != nil) != appliedToInRules {
-			allowed, reason = false, fmt.Sprintf("appliedTo field does not appear consistently in all rules")
-			return reason, allowed
+		if appliedToInSpec == appliedToInRules {
+			allowed, reason = false, fmt.Sprintf("appliedTo is set in both spec and rules")
 		}
-	}
-    if appliedToInSpec == appliedToInRules {
-		allowed, reason = false, fmt.Sprintf("appliedTo is set in both spec and rules")
-	}
 	case admv1.Delete:
 		// Delete of Antrea Policies have no validation
 		allowed = true

test/e2e/antreapolicy_test.go

@@ -154,7 +154,7 @@ func testCNPAllowXBtoA(t *testing.T) {
 		SetPriority(1.0).
 		SetAppliedToGroup(map[string]string{"pod": "a"}, nil, nil, nil)
 	builder.AddIngress(v1.ProtocolTCP, &p80, nil, nil, map[string]string{"pod": "b"}, map[string]string{"ns": "x"},
-		nil, nil, secv1alpha1.RuleActionAllow)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionAllow)
 
 	reachability := NewReachability(allPods, false)
 	reachability.Expect(Pod("x/b"), Pod("x/a"), true)
@@ -186,7 +186,7 @@ func testCNPAllowXBtoYA(t *testing.T) {
 		SetPriority(2.0).
 		SetAppliedToGroup(map[string]string{"pod": "a"}, map[string]string{"ns": "y"}, nil, nil)
 	builder.AddIngress(v1.ProtocolTCP, nil, &port81Name, nil, map[string]string{"pod": "b"}, map[string]string{"ns": "x"},
-		nil, nil, secv1alpha1.RuleActionAllow)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionAllow)
 
 	reachability := NewReachability(allPods, false)
 	reachability.Expect(Pod("x/b"), Pod("y/a"), true)
@@ -216,14 +216,14 @@ func testCNPPriorityOverrideDefaultDeny(t *testing.T) {
 		SetPriority(2).
 		SetAppliedToGroup(nil, map[string]string{"ns": "x"}, nil, nil)
 	builder1.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionAllow)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionAllow)
 
 	builder2 := &ClusterNetworkPolicySpecBuilder{}
 	builder2 = builder2.SetName("cnp-priority1").
 		SetPriority(1).
 		SetAppliedToGroup(map[string]string{"pod": "a"}, map[string]string{"ns": "x"}, nil, nil)
 	builder2.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionDrop)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionDrop)
 
 	// Ingress from ns:z to x/a will be dropped since cnp-priority1 has higher precedence.
 	reachabilityBothCNP := NewReachability(allPods, false)
@@ -257,9 +257,9 @@ func testCNPAllowNoDefaultIsolation(t *testing.T) {
 		SetPriority(1.1).
 		SetAppliedToGroup(nil, map[string]string{"ns": "x"}, nil, nil)
 	builder.AddIngress(v1.ProtocolTCP, &p81, nil, nil, nil, map[string]string{"ns": "y"},
-		nil, nil, secv1alpha1.RuleActionAllow)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionAllow)
 	builder.AddEgress(v1.ProtocolTCP, &p81, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionAllow)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionAllow)
 
 	reachability := NewReachability(allPods, true)
 	testStep := []*TestStep{
@@ -284,7 +284,7 @@ func testCNPDropEgress(t *testing.T) {
 		SetPriority(1.0).
 		SetAppliedToGroup(map[string]string{"pod": "a"}, nil, nil, nil)
 	builder.AddEgress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionDrop)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionDrop)
 
 	reachability := NewReachability(allPods, true)
 	reachability.Expect(Pod("x/a"), Pod("z/a"), false)
@@ -322,23 +322,23 @@ func testCNPPriorityOverride(t *testing.T) {
 	cidr := podZBIP + "/32"
 	// Highest priority. Drops traffic from z/b to x/a.
 	builder1.AddIngress(v1.ProtocolTCP, &p80, nil, &cidr, nil, nil,
-		nil, nil, secv1alpha1.RuleActionDrop)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionDrop)
 
 	builder2 := &ClusterNetworkPolicySpecBuilder{}
 	builder2 = builder2.SetName("cnp-priority2").
 		SetPriority(1.02).
 		SetAppliedToGroup(map[string]string{"pod": "a"}, map[string]string{"ns": "x"}, nil, nil)
 	// Medium priority. Allows traffic from z to x/a.
 	builder2.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionAllow)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionAllow)
 
 	builder3 := &ClusterNetworkPolicySpecBuilder{}
 	builder3 = builder3.SetName("cnp-priority3").
 		SetPriority(1.03).
 		SetAppliedToGroup(nil, map[string]string{"ns": "x"}, nil, nil)
 	// Lowest priority. Drops traffic from z to x.
 	builder3.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionDrop)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionDrop)
 
 	reachabilityTwoCNPs := NewReachability(allPods, true)
 	reachabilityTwoCNPs.Expect(Pod("z/a"), Pod("x/b"), false)
@@ -395,7 +395,7 @@ func testCNPTierOverride(t *testing.T) {
 	cidr := podZBIP + "/32"
 	// Highest priority tier. Drops traffic from z/b to x/a.
 	builder1.AddIngress(v1.ProtocolTCP, &p80, nil, &cidr, nil, nil,
-		nil, nil, secv1alpha1.RuleActionDrop)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionDrop)
 
 	builder2 := &ClusterNetworkPolicySpecBuilder{}
 	builder2 = builder2.SetName("cnp-tier-securityops").
@@ -404,7 +404,7 @@ func testCNPTierOverride(t *testing.T) {
 		SetAppliedToGroup(map[string]string{"pod": "a"}, map[string]string{"ns": "x"}, nil, nil)
 	// Medium priority tier. Allows traffic from z to x/a.
 	builder2.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionAllow)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionAllow)
 
 	builder3 := &ClusterNetworkPolicySpecBuilder{}
 	builder3 = builder3.SetName("cnp-tier-application").
@@ -413,7 +413,7 @@ func testCNPTierOverride(t *testing.T) {
 		SetAppliedToGroup(nil, map[string]string{"ns": "x"}, nil, nil)
 	// Lowest priority tier. Drops traffic from z to x.
 	builder3.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionDrop)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionDrop)
 
 	reachabilityTwoCNPs := NewReachability(allPods, true)
 	reachabilityTwoCNPs.Expect(Pod("z/a"), Pod("x/b"), false)
@@ -473,7 +473,7 @@ func testCNPCustomTiers(t *testing.T) {
 		SetAppliedToGroup(map[string]string{"pod": "a"}, map[string]string{"ns": "x"}, nil, nil)
 	// Medium priority tier. Allows traffic from z to x/a.
 	builder1.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionAllow)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionAllow)
 
 	builder2 := &ClusterNetworkPolicySpecBuilder{}
 	builder2 = builder2.SetName("cnp-tier-low").
@@ -482,7 +482,7 @@ func testCNPCustomTiers(t *testing.T) {
 		SetAppliedToGroup(nil, map[string]string{"ns": "x"}, nil, nil)
 	// Lowest priority tier. Drops traffic from z to x.
 	builder2.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionDrop)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionDrop)
 
 	reachabilityTwoCNPs := NewReachability(allPods, true)
 	reachabilityTwoCNPs.Expect(Pod("z/a"), Pod("x/b"), false)
@@ -519,7 +519,7 @@ func testCNPPriorityConflictingRule(t *testing.T) {
 		SetPriority(1).
 		SetAppliedToGroup(nil, map[string]string{"ns": "x"}, nil, nil)
 	builder1.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionDrop)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionDrop)
 
 	builder2 := &ClusterNetworkPolicySpecBuilder{}
 	builder2 = builder2.SetName("cnp-allow").
@@ -528,7 +528,7 @@ func testCNPPriorityConflictingRule(t *testing.T) {
 	// The following ingress rule will take no effect as it is exactly the same as ingress rule of cnp-drop,
 	// but cnp-allow has lower priority.
 	builder2.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionAllow)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionAllow)
 
 	reachabilityBothCNP := NewReachability(allPods, true)
 	reachabilityBothCNP.Expect(Pod("z/a"), Pod("x/a"), false)
@@ -565,21 +565,21 @@ func testCNPRulePrioirty(t *testing.T) {
 		SetPriority(5).
 		SetAppliedToGroup(nil, map[string]string{"ns": "x"}, nil, nil)
 	builder1.AddEgress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "y"},
-		nil, nil, secv1alpha1.RuleActionDrop)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionDrop)
 	// This rule should take no effect as it will be overridden by the first rule of cnp-allow
 	builder1.AddEgress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionDrop)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionDrop)
 
 	builder2 := &ClusterNetworkPolicySpecBuilder{}
 	// cnp-allow will also apply to all pods in namespace x
 	builder2 = builder2.SetName("cnp-allow").
 		SetPriority(5).
 		SetAppliedToGroup(nil, map[string]string{"ns": "x"}, nil, nil)
 	builder2.AddEgress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "z"},
-		nil, nil, secv1alpha1.RuleActionAllow)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionAllow)
 	// This rule should take no effect as it will be overridden by the first rule of cnp-drop
 	builder2.AddEgress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"ns": "y"},
-		nil, nil, secv1alpha1.RuleActionAllow)
+		nil, nil, []ACNPRuleAppliedToSpec{}, secv1alpha1.RuleActionAllow)
 
 	// Only egress from pods in namespace x to namespace y should be denied
 	reachabilityBothCNP := NewReachability(allPods, true)
@@ -617,7 +617,7 @@ func testANPBasic(t *testing.T) {
 		SetPriority(1.0).
 		SetAppliedToGroup(map[string]string{"pod": "a"}, nil)
 	builder.AddIngress(v1.ProtocolTCP, &p80, nil, nil, map[string]string{"pod": "b"}, map[string]string{"ns": "x"},
-		nil, nil, secv1alpha1.RuleActionDrop)
+		nil, nil, []ANPRuleAppliedToSpec{}, secv1alpha1.RuleActionDrop)
 
 	reachability := NewReachability(allPods, true)
 	reachability.Expect(Pod("x/b"), Pod("y/a"), false)