export RoleInterface, prevent non-controlled NSes from appearing in the queue

Author: stlaz

pkg/psalabelsyncer/podsecurity_label_sync_controller.go

@@ -181,7 +181,6 @@ func (c *PodSecurityAdmissionLabelSynchronizationController) syncNamespace(ctx c
 	for _, sccName := range nsSCCs.UnsortedList() {
 		scc, err := c.sccLister.Get(sccName)
 		if err != nil {
-			// TODO: the SCC was removed in the meantime and synced in the cache?
 			return err
 		}
 		sccPSaLevel, err := convertSCCToPSALevel(ns, scc)
@@ -239,6 +238,16 @@ func (c *PodSecurityAdmissionLabelSynchronizationController) queueKeysForObj(obj
 	}
 
 	if ns := metaObj.GetObjectMeta().GetNamespace(); len(ns) > 0 { // it is a namespaced object
+		controlled, err := c.checkNSControlled(ns)
+		if err != nil {
+			klog.Errorf("failed to determine whether namespace %q should be enqueued: %v", ns, err)
+			return nil
+		}
+
+		if !controlled {
+			return nil
+		}
+
 		return []string{ns}
 	}
 
@@ -260,6 +269,8 @@ func (c *PodSecurityAdmissionLabelSynchronizationController) allWatchedNamespace
 	return qKeys
 }
 
+// saToSCCCAcheEnqueueFunc is a function that allows the SAToSCCCache enqueue keys
+// in the queue of the `PodSecurityAdmissionLabelSynchronizationController` struct
 func (c *PodSecurityAdmissionLabelSynchronizationController) saToSCCCAcheEnqueueFunc(obj interface{}) {
 	enqeueAllNS := func() {
 		// NS was zero-len, we need to enqueue all controlled namespaces
@@ -273,16 +284,16 @@ func (c *PodSecurityAdmissionLabelSynchronizationController) saToSCCCAcheEnqueue
 		}
 	}
 
-	// TODO: maybe just allow setting 2 separate enqueuers for roles and SCCs?
 	switch t := obj.(type) {
-	case roleInterface: // TODO: should be exported
+	case RoleInterface:
 		if nsName := t.Namespace(); len(nsName) > 0 {
-			ns, err := c.namespaceLister.Get(nsName)
+			controlled, err := c.checkNSControlled(nsName)
 			if err != nil {
-				klog.Errorf("failed to enqueue namespace %q: %v", nsName, err)
+				klog.Errorf("failed to determine whether namespace %q should be enqueued: %v", nsName, err)
 				return
 			}
-			if ns.Labels[labelSyncControlLabel] != "false" {
+
+			if controlled {
 				c.workQueue.Add(nsName)
 			}
 			return
@@ -293,6 +304,19 @@ func (c *PodSecurityAdmissionLabelSynchronizationController) saToSCCCAcheEnqueue
 	}
 }
 
+func (c *PodSecurityAdmissionLabelSynchronizationController) checkNSControlled(ns string) (bool, error) {
+	nsObj, err := c.namespaceLister.Get(ns)
+	if err != nil {
+		return false, err
+	}
+
+	if nsObj.Labels[labelSyncControlLabel] != "false" {
+		return true, nil
+	}
+
+	return false, nil
+}
+
 // controlledNamespacesLabelSelector returns label selector to be used with the
 // PodSecurityAdmissionLabelSynchronizationController.
 func controlledNamespacesLabelSelector() (labels.Selector, error) {

pkg/psalabelsyncer/sccrolecache.go

@@ -52,13 +52,15 @@ type roleBindingInterface interface {
 	Subjects() []rbacv1.Subject
 }
 
-type roleInterface interface {
+// RoleInterface is an interface for generic access to role-like object, such
+// as rbac.Role and rbac.ClusterRole
+type RoleInterface interface {
 	Name() string
 	Namespace() string
 	Rules() []rbacv1.PolicyRule
 }
 
-func usefulRolesKey(r roleInterface) string {
+func usefulRolesKey(r RoleInterface) string {
 	return fmt.Sprintf("%s/%s", r.Namespace(), r.Name())
 }
 
@@ -110,7 +112,9 @@ type roleObj struct {
 	clusterRole *rbacv1.ClusterRole
 }
 
-func newRoleObj(obj interface{}) (roleInterface, error) {
+// NewRoleObj expects either a Role or a ClusterRole as its `obj` input argument,
+// it returns an object that allows generic access to the role-like object
+func NewRoleObj(obj interface{}) (RoleInterface, error) {
 	switch r := obj.(type) {
 	case *rbacv1.ClusterRole:
 		return &roleObj{
@@ -237,7 +241,7 @@ func (c *SAToSCCCache) handleRoleModified(obj interface{}) {
 		return
 	}
 
-	role, err := newRoleObj(obj)
+	role, err := NewRoleObj(obj)
 	if err != nil {
 		klog.Warningf("unexpected error, this may be a bug: %v", err)
 		return
@@ -256,7 +260,7 @@ func (c *SAToSCCCache) handleRoleModified(obj interface{}) {
 }
 
 func (c *SAToSCCCache) handleRoleRemoved(obj interface{}) {
-	role, err := newRoleObj(obj)
+	role, err := NewRoleObj(obj)
 	if err != nil {
 		klog.Warningf("unexpected error, this may be a bug: %v", err)
 		return
@@ -374,7 +378,7 @@ func (c *SAToSCCCache) SCCsFor(serviceAccount *corev1.ServiceAccount) (sets.Stri
 }
 
 // getRoleFromRoleRef tries to retrieve the role or clusterrole from roleRef.
-func (c *SAToSCCCache) getRoleFromRoleRef(ns string, roleRef rbacv1.RoleRef) (roleInterface, error) {
+func (c *SAToSCCCache) getRoleFromRoleRef(ns string, roleRef rbacv1.RoleRef) (RoleInterface, error) {
 	var err error
 	var role interface{}
 	switch kind := roleRef.Kind; kind {
@@ -394,7 +398,7 @@ func (c *SAToSCCCache) getRoleFromRoleRef(ns string, roleRef rbacv1.RoleRef) (ro
 		return nil, fmt.Errorf("unknown kind in roleRef: %s", kind)
 	}
 
-	return newRoleObj(role)
+	return NewRoleObj(role)
 }
 
 // IsRoleBindingRelevant returns true if the cluster/rolebinding supplied binds
@@ -426,7 +430,7 @@ func (c *SAToSCCCache) IsRoleInvolvesSCCs(obj interface{}, isRoleUpdate bool) bo
 	c.usefulRolesLock.Lock()
 	defer c.usefulRolesLock.Unlock()
 
-	role, err := newRoleObj(obj)
+	role, err := NewRoleObj(obj)
 	if err != nil {
 		klog.Warningf("unexpected error, this may be a bug: %v", err)
 		return false
@@ -473,15 +477,15 @@ func (c *SAToSCCCache) ReinitializeRoleCache() error {
 	}
 
 	for _, r := range roles {
-		role, err := newRoleObj(r)
+		role, err := NewRoleObj(r)
 		if err != nil {
 			panic(err)
 		}
 		c.syncRoleCache(role, r.Rules, sccs)
 	}
 
 	for _, r := range clusterRoles {
-		role, err := newRoleObj(r)
+		role, err := NewRoleObj(r)
 		if err != nil {
 			panic(err)
 		}
@@ -494,7 +498,7 @@ func (c *SAToSCCCache) ReinitializeRoleCache() error {
 // syncRoleCache will write the current mapping of "role->SCCs it allows" to the cache.
 // It expects the c.usefulRolesLock to be already locked as even the wrapping context
 // handling roles and SCCs likely requires synchronization.
-func (c *SAToSCCCache) syncRoleCache(role roleInterface, rules []rbacv1.PolicyRule, sccs []*securityv1.SecurityContextConstraints) {
+func (c *SAToSCCCache) syncRoleCache(role RoleInterface, rules []rbacv1.PolicyRule, sccs []*securityv1.SecurityContextConstraints) {
 	if c.usefulRolesLock.TryLock() {
 		c.usefulRolesLock.Unlock()
 		panic("syncRoleCache() requires the usefulRolesLock to be always locked before entering the function")