Skip to content

Commit

Permalink
Remove ServiceAccount Informer and Index
Browse files Browse the repository at this point in the history
Without label selector support we don't need to handle ServiceAccount events anymore.

Signed-off-by: wgrayson <wgrayson@vmware.com>
  • Loading branch information
GraysonWu committed Feb 10, 2022
1 parent fc5f5fb commit be55e6a
Show file tree
Hide file tree
Showing 12 changed files with 10 additions and 142 deletions.
1 change: 0 additions & 1 deletion build/yamls/antrea-aks.yml
Expand Up @@ -3689,7 +3689,6 @@ rules:
- pods
- namespaces
- configmaps
- serviceaccounts
verbs:
- get
- watch
Expand Down
1 change: 0 additions & 1 deletion build/yamls/antrea-eks.yml
Expand Up @@ -3689,7 +3689,6 @@ rules:
- pods
- namespaces
- configmaps
- serviceaccounts
verbs:
- get
- watch
Expand Down
1 change: 0 additions & 1 deletion build/yamls/antrea-gke.yml
Expand Up @@ -3689,7 +3689,6 @@ rules:
- pods
- namespaces
- configmaps
- serviceaccounts
verbs:
- get
- watch
Expand Down
1 change: 0 additions & 1 deletion build/yamls/antrea-ipsec.yml
Expand Up @@ -3689,7 +3689,6 @@ rules:
- pods
- namespaces
- configmaps
- serviceaccounts
verbs:
- get
- watch
Expand Down
1 change: 0 additions & 1 deletion build/yamls/antrea-kind.yml
Expand Up @@ -3689,7 +3689,6 @@ rules:
- pods
- namespaces
- configmaps
- serviceaccounts
verbs:
- get
- watch
Expand Down
1 change: 0 additions & 1 deletion build/yamls/antrea.yml
Expand Up @@ -3689,7 +3689,6 @@ rules:
- pods
- namespaces
- configmaps
- serviceaccounts
verbs:
- get
- watch
Expand Down
1 change: 0 additions & 1 deletion build/yamls/base/controller-rbac.yml
Expand Up @@ -16,7 +16,6 @@ rules:
- pods
- namespaces
- configmaps
- serviceaccounts
verbs:
- get
- watch
Expand Down
2 changes: 0 additions & 2 deletions cmd/antrea-controller/controller.go
Expand Up @@ -116,7 +116,6 @@ func run(o *Options) error {
podInformer := informerFactory.Core().V1().Pods()
namespaceInformer := informerFactory.Core().V1().Namespaces()
serviceInformer := informerFactory.Core().V1().Services()
serviceAccountInformer := informerFactory.Core().V1().ServiceAccounts()
networkPolicyInformer := informerFactory.Networking().V1().NetworkPolicies()
nodeInformer := informerFactory.Core().V1().Nodes()
cnpInformer := crdInformerFactory.Crd().V1alpha1().ClusterNetworkPolicies()
Expand Down Expand Up @@ -162,7 +161,6 @@ func run(o *Options) error {
groupEntityIndex,
namespaceInformer,
serviceInformer,
serviceAccountInformer,
networkPolicyInformer,
cnpInformer,
anpInformer,
Expand Down
94 changes: 7 additions & 87 deletions pkg/controller/networkpolicy/clusternetworkpolicy.go
Expand Up @@ -15,8 +15,6 @@
package networkpolicy

import (
"reflect"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
Expand All @@ -29,7 +27,6 @@ import (
"antrea.io/antrea/pkg/controller/grouping"
"antrea.io/antrea/pkg/controller/networkpolicy/store"
antreatypes "antrea.io/antrea/pkg/controller/types"
"antrea.io/antrea/pkg/util/k8s"
utilsets "antrea.io/antrea/pkg/util/sets"
)

Expand Down Expand Up @@ -258,82 +255,6 @@ func (n *NetworkPolicyController) deleteNamespace(old interface{}) {
}
}

// filterACNPsByServiceAccount gets all ClusterNetworkPolicies that will need to be
// 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(ServiceAccountIndex, k8s.NamespacedName(sa.Namespace, sa.Name))
if err != nil {
klog.ErrorS(err, "Error fetching ClusterNetworkPolicies that could be affected by the ServiceAccount ADD/UPDATE/DELETE event", "ServiceAccountName", sa.Name, "ServiceAccountNamespace", sa.Namespace)
return nil
}
for _, acnpObj := range acnpObjs {
acnps = append(acnps, acnpObj.(*crdv1alpha1.ClusterNetworkPolicy))
}
return acnps
}

// addServiceAccount receives ServiceAccount ADD events and triggers all
// ClusterNetworkPolicies that refer to this ServiceAccount in `serviceAccounts`
// field to be re-processed.
func (n *NetworkPolicyController) addServiceAccount(obj interface{}) {
defer n.heartbeat("addServiceAccount")
serviceAccount := obj.(*v1.ServiceAccount)
klog.V(2).InfoS("Processing ServiceAccount ADD event", "ServiceAccountNS", serviceAccount.Namespace, "ServiceAccountName", serviceAccount.Name, "ServiceAccountLabels", serviceAccount.Labels)
affectedACNPs := n.filterACNPsByServiceAccount(serviceAccount)
for _, acnp := range affectedACNPs {
n.reprocessCNP(acnp, true)
}
}

// updateServiceAccount receives ServiceAccount UPDATE events and triggers all
// ClusterNetworkPolicies that refer to this ServiceAccount in `serviceAccounts`
// field to be re-processed.
func (n *NetworkPolicyController) updateServiceAccount(oldObj, curObj interface{}) {
defer n.heartbeat("updateServiceAccount")
oldServiceAccount, curServiceAccount := oldObj.(*v1.ServiceAccount), curObj.(*v1.ServiceAccount)
if reflect.DeepEqual(oldServiceAccount.Labels, curServiceAccount.Labels) {
klog.V(2).InfoS("Labels of ServiceAccount didn't change, so skip ACNP reprocess", "ServiceAccountNS", curServiceAccount.Namespace, "ServiceAccountName", curServiceAccount.Name, "ServiceAccountLabels", curServiceAccount.Labels)
return
}
klog.V(2).InfoS("Processing ServiceAccount UPDATE event", "ServiceAccountNS", curServiceAccount.Namespace, "ServiceAccountName", curServiceAccount.Name, "OldServiceAccountLabels", oldServiceAccount.Labels, "CurServiceAccountLabels", curServiceAccount.Labels)
affectedACNPsByOldServiceAccount := n.filterACNPsByServiceAccount(oldServiceAccount)
affectedACNPsByCurServiceAccount := n.filterACNPsByServiceAccount(curServiceAccount)
getACNPNamesSet := func(acnps []*crdv1alpha1.ClusterNetworkPolicy) sets.String {
nameSet := sets.String{}
for _, acnp := range acnps {
nameSet.Insert(acnp.Name)
}
return nameSet
}
oldACNPNameSet := getACNPNamesSet(affectedACNPsByOldServiceAccount)
curACNPNameSet := getACNPNamesSet(affectedACNPsByCurServiceAccount)
affectedACNPNameSet := utilsets.SymmetricDifferenceString(oldACNPNameSet, curACNPNameSet)
for _, acnp := range affectedACNPsByOldServiceAccount {
if affectedACNPNameSet.Has(acnp.Name) {
n.reprocessCNP(acnp, true)
}
}
for _, acnp := range affectedACNPsByCurServiceAccount {
if affectedACNPNameSet.Has(acnp.Name) {
n.reprocessCNP(acnp, true)
}
}
}

// deleteNamespace receives ServiceAccount DELETE events and triggers all
// ClusterNetworkPolicies that refer to this ServiceAccount in `serviceAccounts`
// field to be re-processed.
func (n *NetworkPolicyController) deleteServiceAccount(old interface{}) {
serviceAccount := old.(*v1.ServiceAccount)
defer n.heartbeat("deleteServiceAccount")
klog.V(2).InfoS("Processing ServiceAccount DELETE event", "ServiceAccountNS", serviceAccount.Namespace, "ServiceAccountName", serviceAccount.Name, "ServiceAccountLabels", serviceAccount.Labels)
affectedACNPs := n.filterACNPsByServiceAccount(serviceAccount)
for _, acnp := range affectedACNPs {
n.reprocessCNP(acnp, true)
}
}

// processClusterNetworkPolicy creates an internal NetworkPolicy instance
// corresponding to the crdv1alpha1.ClusterNetworkPolicy object. This method
// does not commit the internal NetworkPolicy in store, instead returns an
Expand Down Expand Up @@ -505,18 +426,17 @@ func hasPerNamespaceRule(cnp *crdv1alpha1.ClusterNetworkPolicy) bool {
// at cluster level (appliedTo groups which will not need to be split by Namespaces).
func (n *NetworkPolicyController) processClusterAppliedTo(appliedTo []crdv1alpha1.NetworkPolicyPeer, appliedToGroupNamesSet sets.String) []string {
var appliedToGroupNames []string
insertATG := func(atg string) {
if atg != "" {
appliedToGroupNames = append(appliedToGroupNames, atg)
appliedToGroupNamesSet.Insert(atg)
}
}
for _, at := range appliedTo {
var atg string
if at.Group != "" {
insertATG(n.processAppliedToGroupForCG(at.Group))
atg = n.processAppliedToGroupForCG(at.Group)
} else {
translatedAT := n.translateServiceAccountInPeer(at)
insertATG(n.createAppliedToGroup("", translatedAT.PodSelector, translatedAT.NamespaceSelector, translatedAT.ExternalEntitySelector))
atg = n.createAppliedToGroup("", translatedAT.PodSelector, translatedAT.NamespaceSelector, translatedAT.ExternalEntitySelector)
}
if atg != "" {
appliedToGroupNames = append(appliedToGroupNames, atg)
appliedToGroupNamesSet.Insert(atg)
}
}
return appliedToGroupNames
Expand Down
42 changes: 0 additions & 42 deletions pkg/controller/networkpolicy/networkpolicy_controller.go
Expand Up @@ -82,8 +82,6 @@ const (
PriorityIndex = "priority"
// ClusterGroupIndex is used to index ClusterNetworkPolicies by ClusterGroup names.
ClusterGroupIndex = "clustergroup"
// ServiceAccountIndex is used to index ClusterNetworkPolicies by ServiceAccount namespacedName.
ServiceAccountIndex = "ServiceAccount"

appliedToGroupType grouping.GroupType = "appliedToGroup"
addressGroupType grouping.GroupType = "addressGroup"
Expand Down Expand Up @@ -141,10 +139,6 @@ type NetworkPolicyController struct {
// serviceListerSynced is a function which returns true if the Service shared informer has been synced at least once.
serviceListerSynced cache.InformerSynced

serviceAccountInformer coreinformers.ServiceAccountInformer
serviceAccountLister corelisters.ServiceAccountLister
serviceAccountSynced cache.InformerSynced

networkPolicyInformer networkinginformers.NetworkPolicyInformer
// networkPolicyLister is able to list/get Network Policies and is populated by the shared informer passed to
// NewNetworkPolicyController.
Expand Down Expand Up @@ -278,30 +272,6 @@ var cnpIndexers = cache.Indexers{
}
return groupNames.List(), nil
},
ServiceAccountIndex: func(obj interface{}) ([]string, error) {
cnp, ok := obj.(*secv1alpha1.ClusterNetworkPolicy)
if !ok {
return []string{}, nil
}
saNamespacedNames := sets.String{}
addSaNamespacedNames := func(peers []secv1alpha1.NetworkPolicyPeer) {
for _, peer := range peers {
if peer.ServiceAccount != nil {
saNamespacedNames.Insert(k8s.NamespacedName(peer.ServiceAccount.Namespace, peer.ServiceAccount.Name))
}
}
}
addSaNamespacedNames(cnp.Spec.AppliedTo)
for _, ingressRule := range cnp.Spec.Ingress {
addSaNamespacedNames(ingressRule.From)
addSaNamespacedNames(ingressRule.AppliedTo)
}
for _, egressRule := range cnp.Spec.Egress {
addSaNamespacedNames(egressRule.To)
addSaNamespacedNames(egressRule.AppliedTo)
}
return saNamespacedNames.List(), nil
},
}

var anpIndexers = cache.Indexers{
Expand All @@ -320,7 +290,6 @@ func NewNetworkPolicyController(kubeClient clientset.Interface,
groupingInterface grouping.Interface,
namespaceInformer coreinformers.NamespaceInformer,
serviceInformer coreinformers.ServiceInformer,
serviceAccountInformer coreinformers.ServiceAccountInformer,
networkPolicyInformer networkinginformers.NetworkPolicyInformer,
cnpInformer secinformers.ClusterNetworkPolicyInformer,
anpInformer secinformers.NetworkPolicyInformer,
Expand Down Expand Up @@ -367,9 +336,6 @@ func NewNetworkPolicyController(kubeClient clientset.Interface,
n.serviceInformer = serviceInformer
n.serviceLister = serviceInformer.Lister()
n.serviceListerSynced = serviceInformer.Informer().HasSynced
n.serviceAccountInformer = serviceAccountInformer
n.serviceAccountLister = serviceAccountInformer.Lister()
n.serviceAccountSynced = serviceInformer.Informer().HasSynced
n.cnpInformer = cnpInformer
n.cnpLister = cnpInformer.Lister()
n.cnpListerSynced = cnpInformer.Informer().HasSynced
Expand Down Expand Up @@ -399,14 +365,6 @@ func NewNetworkPolicyController(kubeClient clientset.Interface,
},
resyncPeriod,
)
n.serviceAccountInformer.Informer().AddEventHandlerWithResyncPeriod(
cache.ResourceEventHandlerFuncs{
AddFunc: n.addServiceAccount,
UpdateFunc: n.updateServiceAccount,
DeleteFunc: n.deleteServiceAccount,
},
resyncPeriod,
)
tierInformer.Informer().AddIndexers(tierIndexers)
cnpInformer.Informer().AddIndexers(cnpIndexers)
cnpInformer.Informer().AddEventHandlerWithResyncPeriod(
Expand Down
Expand Up @@ -105,7 +105,6 @@ func newController(objects ...runtime.Object) (*fake.Clientset, *networkPolicyCo
groupEntityIndex,
informerFactory.Core().V1().Namespaces(),
informerFactory.Core().V1().Services(),
informerFactory.Core().V1().ServiceAccounts(),
informerFactory.Networking().V1().NetworkPolicies(),
crdInformerFactory.Crd().V1alpha1().ClusterNetworkPolicies(),
crdInformerFactory.Crd().V1alpha1().NetworkPolicies(),
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/antreapolicy_test.go
Expand Up @@ -2538,7 +2538,7 @@ func testToServices(t *testing.T) {
}
}

func testServiceAccountsSelector(t *testing.T, data *TestData) {
func testServiceAccountSelector(t *testing.T, data *TestData) {
k8sUtils.CreateOrUpdateServiceAccount(k8sUtils.BuildServiceAccount("test-sa", "x", nil))
defer k8sUtils.DeleteServiceAccount("x", "test-sa")

Expand Down Expand Up @@ -2907,7 +2907,7 @@ func TestAntreaPolicy(t *testing.T) {
t.Run("Case=ACNPNoTierSetDefaultTier", func(t *testing.T) { testMutateACNPNoTier(t) })
t.Run("Case=ANPNoTierSetDefaultTier", func(t *testing.T) { testMutateANPNoTier(t) })
t.Run("Case=ANPNoRuleNameSetRuleName", func(t *testing.T) { testMutateANPNoRuleName(t) })
t.Run("Case=ACNPNoRuleNameStRuleName", func(t *testing.T) { testMutateACNPNoRuleName(t) })
t.Run("Case=ACNPNoRuleNameSetRuleName", func(t *testing.T) { testMutateACNPNoRuleName(t) })
})

t.Run("TestGroupDefaultDENY", func(t *testing.T) {
Expand Down Expand Up @@ -2961,7 +2961,7 @@ func TestAntreaPolicy(t *testing.T) {
t.Run("Case=ACNPFQDNPolicy", func(t *testing.T) { testFQDNPolicy(t) })
t.Run("Case=FQDNPolicyInCluster", func(t *testing.T) { testFQDNPolicyInClusterService(t) })
t.Run("Case=ACNPToServices", func(t *testing.T) { testToServices(t) })
t.Run("Case=ACNPServiceAccountsSelector", func(t *testing.T) { testServiceAccountsSelector(t, data) })
t.Run("Case=ACNPServiceAccountSelector", func(t *testing.T) { testServiceAccountSelector(t, data) })
})
// print results for reachability tests
printResults()
Expand Down

0 comments on commit be55e6a

Please sign in to comment.