Address comments

Signed-off-by: wgrayson <[email protected]>

pkg/apis/crd/v1alpha2/types.go

@@ -186,6 +186,10 @@ type AppliedTo struct {
 	// Groups is the set of ClusterGroup names.
 	// +optional
 	Groups []string `json:"groups,omitempty"`
+	// Select all Pods with the ServiceAccount matched by this field.
+	// Cannot be set with any other selector.
+	// +optional
+	ServiceAccounts []v1alpha1.ServiceAccount `json:"serviceAccounts,omitempty"`
 }
 
 // +genclient

pkg/controller/grouping/custom_label.go

@@ -21,7 +21,7 @@ import (
 )
 
 const (
-	CustomLabelKeyPrefix   = "antrea.io/reserved-label-"
+	CustomLabelKeyPrefix   = "internal.antrea.io/"
 	ServiceAccountLabelKey = "service-account"
 )
 

pkg/controller/networkpolicy/clusternetworkpolicy.go

@@ -16,6 +16,7 @@ package networkpolicy
 
 import (
 	"reflect"
+	"strconv"
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -261,7 +262,7 @@ func (n *NetworkPolicyController) deleteNamespace(old interface{}) {
 // re-processed based on if they are affected by an added/updated/deleted
 // ServiceAccount.
 func (n *NetworkPolicyController) filterACNPsByServiceAccount(sa *v1.ServiceAccount) (acnps []*crdv1alpha1.ClusterNetworkPolicy) {
-	acnpObjs, err := n.cnpInformer.Informer().GetIndexer().ByIndex(ServiceAccountsPeerIndex, HasServiceAccountsPeer)
+	acnpObjs, err := n.cnpInformer.Informer().GetIndexer().ByIndex(HasServiceAccountsPeerIndex, strconv.FormatBool(true))
 	if err != nil {
 		klog.Errorf("Error fetching ClusterNetworkPolicies that have ServiceAccounts in either AppliedTo or Rules: %v", err)
 		return nil
@@ -431,7 +432,7 @@ func (n *NetworkPolicyController) processClusterNetworkPolicy(cnp *crdv1alpha1.C
 		for _, at := range cnp.Spec.AppliedTo {
 			var newATs []crdv1alpha1.NetworkPolicyPeer
 			if len(at.ServiceAccounts) > 0 {
-				newATs = n.transServiceAccounts(at.ServiceAccounts)
+				newATs = n.translateServiceAccounts(at.ServiceAccounts)
 			} else {
 				newATs = append(newATs, at)
 			}
@@ -498,7 +499,7 @@ func (n *NetworkPolicyController) processClusterNetworkPolicy(cnp *crdv1alpha1.C
 					for _, at := range cnpRule.AppliedTo {
 						var newATs []crdv1alpha1.NetworkPolicyPeer
 						if len(at.ServiceAccounts) > 0 {
-							newATs = n.transServiceAccounts(at.ServiceAccounts)
+							newATs = n.translateServiceAccounts(at.ServiceAccounts)
 						} else {
 							newATs = append(newATs, at)
 						}
@@ -553,7 +554,7 @@ func (n *NetworkPolicyController) validateAppliedToWithSA(acnp *crdv1alpha1.Clus
 			if len(peer.ServiceAccounts) == 0 {
 				return true
 			}
-			if len(n.transServiceAccounts(peer.ServiceAccounts)) > 0 {
+			if len(n.translateServiceAccounts(peer.ServiceAccounts)) > 0 {
 				return true
 			}
 		}
@@ -574,9 +575,9 @@ func (n *NetworkPolicyController) validateAppliedToWithSA(acnp *crdv1alpha1.Clus
 	return allRulesHaveAT(acnp.Spec.Ingress) && allRulesHaveAT(acnp.Spec.Egress)
 }
 
-// transServiceAccounts translates a list of ServiceAccount to a list of
+// translateServiceAccounts translates a list of ServiceAccount to a list of
 // NetworkPolicyPeer with PodSelector+NamespaceSelector.
-func (n *NetworkPolicyController) transServiceAccounts(serviceAccounts []crdv1alpha1.ServiceAccount) (newPeers []crdv1alpha1.NetworkPolicyPeer) {
+func (n *NetworkPolicyController) translateServiceAccounts(serviceAccounts []crdv1alpha1.ServiceAccount) (newPeers []crdv1alpha1.NetworkPolicyPeer) {
 	// saToPSelNSel returns a crdv1alpha1.NetworkPolicyPeer with PodSelector +
 	// NamespaceSelector based on ServiceAccount Name + Namespace
 	saToPSelNSel := func(saNs, saName string) crdv1alpha1.NetworkPolicyPeer {
@@ -653,7 +654,7 @@ func (n *NetworkPolicyController) processClusterAppliedTo(appliedTo []crdv1alpha
 		if at.Group != "" {
 			insertATG(n.processAppliedToGroupForCG(at.Group))
 		} else if len(at.ServiceAccounts) > 0 {
-			newPeers := n.transServiceAccounts(at.ServiceAccounts)
+			newPeers := n.translateServiceAccounts(at.ServiceAccounts)
 			for _, newPeer := range newPeers {
 				insertATG(n.createAppliedToGroup("", newPeer.PodSelector, newPeer.NamespaceSelector, nil))
 			}

pkg/controller/networkpolicy/crd_utils.go

@@ -115,7 +115,7 @@ func (n *NetworkPolicyController) toAntreaPeerForCRD(peers []v1alpha1.NetworkPol
 		} else if peer.FQDN != "" {
 			fqdns = append(fqdns, peer.FQDN)
 		} else if len(peer.ServiceAccounts) > 0 {
-			newPeers := n.transServiceAccounts(peer.ServiceAccounts)
+			newPeers := n.translateServiceAccounts(peer.ServiceAccounts)
 			for _, newPeer := range newPeers {
 				normalizedUID := n.createAddressGroup(np.GetNamespace(), newPeer.PodSelector, newPeer.NamespaceSelector, nil)
 				addressGroups = append(addressGroups, normalizedUID)

pkg/controller/networkpolicy/networkpolicy_controller.go

@@ -82,9 +82,8 @@ const (
 	PriorityIndex = "priority"
 	// ClusterGroupIndex is used to index ClusterNetworkPolicies by ClusterGroup names.
 	ClusterGroupIndex = "clustergroup"
-	// ServiceAccountsPeerIndex is used to index ClusterNetworkPolicies by ServiceAccount labels.
-	ServiceAccountsPeerIndex = "hasServiceAccountsPeer"
-	HasServiceAccountsPeer   = "true"
+	// HasServiceAccountsPeerIndex is used to index ClusterNetworkPolicies by ServiceAccount labels.
+	HasServiceAccountsPeerIndex = "hasServiceAccountsPeer"
 
 	appliedToGroupType grouping.GroupType = "appliedToGroup"
 	addressGroupType   grouping.GroupType = "addressGroup"
@@ -279,7 +278,7 @@ var cnpIndexers = cache.Indexers{
 		}
 		return groupNames.List(), nil
 	},
-	ServiceAccountsPeerIndex: func(obj interface{}) ([]string, error) {
+	HasServiceAccountsPeerIndex: func(obj interface{}) ([]string, error) {
 		cnp, ok := obj.(*secv1alpha1.ClusterNetworkPolicy)
 		if !ok {
 			return []string{}, nil
@@ -293,25 +292,25 @@ var cnpIndexers = cache.Indexers{
 			return false
 		}
 		if peerHasServiceAccounts(cnp.Spec.AppliedTo) {
-			return []string{HasServiceAccountsPeer}, nil
+			return []string{strconv.FormatBool(true)}, nil
 		}
 		for _, ingressRule := range cnp.Spec.Ingress {
 			if peerHasServiceAccounts(ingressRule.From) {
-				return []string{HasServiceAccountsPeer}, nil
+				return []string{strconv.FormatBool(true)}, nil
 			}
 			if peerHasServiceAccounts(ingressRule.AppliedTo) {
-				return []string{HasServiceAccountsPeer}, nil
+				return []string{strconv.FormatBool(true)}, nil
 			}
 		}
 		for _, egressRule := range cnp.Spec.Egress {
 			if peerHasServiceAccounts(egressRule.To) {
-				return []string{HasServiceAccountsPeer}, nil
+				return []string{strconv.FormatBool(true)}, nil
 			}
 			if peerHasServiceAccounts(egressRule.AppliedTo) {
-				return []string{HasServiceAccountsPeer}, nil
+				return []string{strconv.FormatBool(true)}, nil
 			}
 		}
-		return []string{}, nil
+		return []string{strconv.FormatBool(false)}, nil
 	},
 }