fix deadlocking

openshift · Apr 14, 2022 · 63d7b41 · 63d7b41
1 parent 1095346
commit 63d7b41
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 19 deletions.
diff --git a/pkg/psalabelsyncer/podsecurity_label_sync_controller.go b/pkg/psalabelsyncer/podsecurity_label_sync_controller.go
@@ -3,7 +3,6 @@ package psalabelsyncer
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -109,7 +108,7 @@ func NewPodSecurityAdmissionLabelSynchronizationController(
 			rbacInformers.Roles().Informer(),
 			rbacInformers.ClusterRoles().Informer(),
 		).
-		WithFilteredEventsInformersQueueKeyFunc(
+		WithFilteredEventsInformersQueueKeysFunc(
 			nsToQueueKey,
 			func(obj interface{}) bool {
 				ns, ok := obj.(*corev1.Namespace)
@@ -227,21 +226,12 @@ func (c *PodSecurityAdmissionLabelSynchronizationController) syncNamespace(ctx c
 	return nil
 }
 
-const namespaceKeyPrefix = "namespace/"
-
-func getNSFromQueueKey(qKey string) string {
-	if keyParsed := strings.SplitAfterN(qKey, namespaceKeyPrefix, 1); len(keyParsed) == 2 {
-		return keyParsed[1]
-	}
-	return ""
-}
-
-func nsToQueueKey(obj runtime.Object) string {
+func nsToQueueKey(obj runtime.Object) []string {
 	metaObj, ok := obj.(metav1.ObjectMetaAccessor)
 	if !ok {
-		return factory.DefaultQueueKey
+		return factory.DefaultQueueKeysFunc(obj)
 	}
-	return namespaceKeyPrefix + metaObj.GetObjectMeta().GetName()
+	return []string{metaObj.GetObjectMeta().GetName()}
 }
 
 func (c *PodSecurityAdmissionLabelSynchronizationController) queueKeysForObj(obj runtime.Object) []string {

diff --git a/pkg/psalabelsyncer/sccrolecache.go b/pkg/psalabelsyncer/sccrolecache.go
@@ -228,10 +228,10 @@ func (c *SAToSCCCache) SCCsFor(serviceAccount *corev1.ServiceAccount) (sets.Stri
 		c.usefulRolesLock.Lock()
 		// this role does not have SCC-related rules
 		cachedAllowedSCCs := c.usefulRoles[roleCachedKey]
+		c.usefulRolesLock.Unlock()
 		if len(cachedAllowedSCCs) == 0 {
 			continue
 		}
-		c.usefulRolesLock.Unlock()
 
 		// we particularly care only about Roles in the SA NS
 		if roleRef := rb.RoleRef(); rb.AppliesToNS(serviceAccount.Namespace) && roleRef.APIGroup == rbacv1.GroupName {
@@ -295,13 +295,16 @@ func (c *SAToSCCCache) IsRoleInvolvesSCCs(obj interface{}, isRoleUpdate bool) bo
 	}
 
 	if isRoleUpdate {
-		c.SyncRoleCache(role.Namespace(), role.Name(), role.Rules(), sccs)
+		c.syncRoleCache(role.Namespace(), role.Name(), role.Rules(), sccs)
 	}
 
 	return len(c.usefulRoles[fmt.Sprintf("%s/%s", role.Namespace(), role.Name())]) != 0
 }
 
 func (c *SAToSCCCache) ReinitializeRoleCache() error {
+	c.usefulRolesLock.Lock() // TODO: comment this stuff
+	defer c.usefulRolesLock.Unlock()
+
 	roles, err := c.roleLister.List(labels.Everything())
 	if err != nil {
 		return fmt.Errorf("failed to initialize role cache: %w", err)
@@ -318,17 +321,22 @@ func (c *SAToSCCCache) ReinitializeRoleCache() error {
 	}
 
 	for _, r := range roles {
-		c.SyncRoleCache(r.Namespace, r.Name, r.Rules, sccs)
+		c.syncRoleCache(r.Namespace, r.Name, r.Rules, sccs)
 	}
 
 	for _, r := range clusterRoles {
-		c.SyncRoleCache(r.Namespace, r.Name, r.Rules, sccs)
+		c.syncRoleCache(r.Namespace, r.Name, r.Rules, sccs)
 	}
 
 	return nil
 }
 
-func (c *SAToSCCCache) SyncRoleCache(roleNS, roleName string, rules []rbacv1.PolicyRule, sccs []*securityv1.SecurityContextConstraints) {
+func (c *SAToSCCCache) syncRoleCache(roleNS, roleName string, 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")
+	}
+
 	dummyUserInfo := &user.DefaultInfo{
 		Name: "dummyUser",
 	}