add SCC-related roles caching and other fixes

Author: stlaz

pkg/psalabelsyncer/podsecurity_label_sync_controller.go

@@ -18,6 +18,7 @@ import (
 	"k8s.io/client-go/tools/cache"
 	psapi "k8s.io/pod-security-admission/api"
 
+	securityv1 "github.com/openshift/api/security/v1"
 	securityv1informers "github.com/openshift/client-go/security/informers/externalversions/security/v1"
 	securityv1listers "github.com/openshift/client-go/security/listers/security/v1"
 
@@ -80,18 +81,41 @@ func NewPodSecurityAdmissionLabelSynchronizationController(
 		saToSCCsCache: NewSAToSCCCache(rbacInformers, sccInformer),
 	}
 
-	// FIXME: filter out NSes that don't have the SCC UIDs annotation set yet because that makes the conversion panic
-	// FIXME: also make the conversion not panic but error out instead
 	return factory.New().
 		WithSync(c.sync).
-		WithInformers(
-			namespaceInformer.Informer(),
-			rbacInformers.Roles().Informer(),
+		WithFilteredEventsInformers(
+			func(obj interface{}) bool {
+				return c.saToSCCsCache.IsRoleBindingRelevant(obj)
+			},
 			rbacInformers.RoleBindings().Informer(),
-			rbacInformers.ClusterRoles().Informer(),
 			rbacInformers.ClusterRoleBindings().Informer(),
+		).
+		WithFilteredEventsInformers(
+			func(obj interface{}) bool {
+				return c.saToSCCsCache.IsRoleInvolvesSCCs(obj, true)
+			},
+			rbacInformers.Roles().Informer(),
+			rbacInformers.ClusterRoles().Informer(),
+		).
+		WithFilteredEventsInformers(
+			func(obj interface{}) bool {
+				// TODO: also probably don't react on NSes that are being deleted
+				ns, ok := obj.(*corev1.Namespace)
+				if !ok {
+					return false
+				}
+				// the SCC mapping requires the annotation
+				// FIXME: make the mapping not panic but error out instead
+				if ns.Annotations == nil || len(ns.Annotations[securityv1.UIDRangeAnnotation]) == 0 {
+					return false
+				}
+				return true
+			},
+			namespaceInformer.Informer(),
+		).
+		WithInformers(
 			serviceAccountInformer.Informer(),
-			sccInformer.Informer(),
+			sccInformer.Informer(), // FIXME: we need to resync the cache on an SCC update (in case one is added or removed)
 		).
 		ToController(
 			controllerName,

pkg/psalabelsyncer/sccrolecache.go

@@ -3,10 +3,10 @@ package psalabelsyncer
 import (
 	"fmt"
 	"strings"
+	"sync"
 
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/authentication/serviceaccount"
@@ -15,6 +15,7 @@ import (
 	rbacv1informers "k8s.io/client-go/informers/rbac/v1"
 	rbacv1listers "k8s.io/client-go/listers/rbac/v1"
 	"k8s.io/client-go/tools/cache"
+	"k8s.io/klog/v2"
 	"k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac"
 
 	securityv1 "github.com/openshift/api/security/v1"
@@ -34,6 +35,9 @@ type SAToSCCCache struct {
 
 	rolesSynced        cache.InformerSynced
 	roleBindingsSynced cache.InformerSynced
+
+	usefulRolesLock sync.RWMutex
+	usefulRoles     map[string][]string
 }
 
 // role and clusterrolebinding object for generic handling, assumes one and
@@ -54,7 +58,7 @@ func newRoleBindingObj(obj interface{}) (*roleBindingObj, error) {
 		}, nil
 	}
 
-	return nil, fmt.Errorf("the object is neither a RoleBinding, nor a ClusterRoleBinding: %v", obj)
+	return nil, fmt.Errorf("the object is neither a RoleBinding, nor a ClusterRoleBinding: %T", obj)
 }
 
 func (r *roleBindingObj) RoleRef() rbacv1.RoleRef {
@@ -78,6 +82,57 @@ func (r *roleBindingObj) AppliesToNS(ns string) bool {
 	return ns == r.roleBinding.Namespace
 }
 
+func (r *roleBindingObj) Namespace() string {
+	if r.clusterRoleBinding != nil {
+		return ""
+	}
+	return r.roleBinding.Namespace
+}
+
+// roleObj helps to handle roles and clusterroles in a generic manner
+type roleObj struct {
+	role        *rbacv1.Role
+	clusterRole *rbacv1.ClusterRole
+}
+
+func newRoleObj(obj interface{}) (*roleObj, error) {
+	switch r := obj.(type) {
+	case *rbacv1.ClusterRole:
+		return &roleObj{
+			clusterRole: r,
+		}, nil
+	case *rbacv1.Role:
+		return &roleObj{
+			role: r,
+		}, nil
+	case *roleObj:
+		return r, nil
+	default:
+		return nil, fmt.Errorf("the object is neither a Role, nor a ClusterRole: %T", obj)
+	}
+}
+
+func (r *roleObj) Rules() []rbacv1.PolicyRule {
+	if role := r.clusterRole; role != nil {
+		return role.Rules
+	}
+	return r.role.Rules
+}
+
+func (r *roleObj) Name() string {
+	if role := r.clusterRole; role != nil {
+		return role.Name
+	}
+	return r.role.Name
+}
+
+func (r *roleObj) Namespace() string {
+	if role := r.clusterRole; role != nil {
+		return role.Namespace
+	}
+	return r.role.Namespace
+}
+
 func BySAIndexKeys(obj interface{}) ([]string, error) {
 	roleBinding, err := newRoleBindingObj(obj)
 	if err != nil {
@@ -111,6 +166,8 @@ func NewSAToSCCCache(rbacInformers rbacv1informers.Interface, sccInfomer securit
 		// TODO: do I need these?
 		rolesSynced:        rbacInformers.Roles().Informer().HasSynced,
 		roleBindingsSynced: rbacInformers.RoleBindings().Informer().HasSynced,
+
+		usefulRoles: make(map[string][]string),
 	}
 }
 
@@ -163,41 +220,124 @@ func (c *SAToSCCCache) SCCsFor(serviceAccount *corev1.ServiceAccount) (sets.Stri
 			return nil, err
 		}
 
+		roleCachedKey := fmt.Sprintf("/%s", rb.RoleRef().Name)
+		if rb.RoleRef().Kind == "Role" {
+			roleCachedKey = rb.Namespace() + roleCachedKey
+		}
+
+		c.usefulRolesLock.RLock()
+		// this role does not have SCC-related rules
+		cachedAllowedSCCs := c.usefulRoles[roleCachedKey]
+		if len(cachedAllowedSCCs) == 0 {
+			continue
+		}
+		c.usefulRolesLock.RUnlock()
+
 		// we particularly care only about Roles in the SA NS
 		if roleRef := rb.RoleRef(); rb.AppliesToNS(serviceAccount.Namespace) && roleRef.APIGroup == rbacv1.GroupName {
-			switch roleRef.Kind {
-			case "Role":
-				r, err := c.roleLister.Roles(serviceAccount.Namespace).Get(roleRef.Name)
-				if err != nil {
-					if errors.IsNotFound(err) {
-						continue
-					}
-					// TODO: maybe just ignore and log?
-					return nil, err
-				}
-				allowedSCCs.Insert(SCCsAllowedByPolicyRules(serviceAccount.Namespace, realSAUserInfo, sccs, r.Rules)...)
-
-			case "ClusterRole":
-				r, err := c.clusterRoleLister.Get(roleRef.Name)
-				if err != nil {
-					if errors.IsNotFound(err) {
-						continue
-					}
-					// TODO: maybe just ignore and log?
-					return nil, err
-				}
-				allowedSCCs.Insert(SCCsAllowedByPolicyRules(serviceAccount.Namespace, realSAUserInfo, sccs, r.Rules)...)
-
-			default:
-				// ignore invalid role references
-				continue
-			}
+			allowedSCCs.Insert(cachedAllowedSCCs...)
 		}
 	}
 
 	return allowedSCCs, nil
 }
 
+func (c *SAToSCCCache) GetRoleFromRoleRef(ns string, roleRef rbacv1.RoleRef) (*roleObj, error) {
+	var role interface{}
+	var err error
+	switch kind := roleRef.Kind; kind {
+	case "Role":
+		role, err = c.roleLister.Roles(ns).Get(roleRef.Name)
+	case "ClusterRole":
+		role, err = c.clusterRoleLister.Get(roleRef.Name)
+	default:
+		return nil, fmt.Errorf("unknown kind in roleRef: %s", kind)
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	return newRoleObj(role)
+}
+
+func (c *SAToSCCCache) IsRoleBindingRelevant(obj interface{}) bool {
+	rb, err := newRoleBindingObj(obj)
+	if err != nil {
+		klog.Warningf("unexpected error, this may be a bug: %v", err)
+		return false
+	}
+
+	role, err := c.GetRoleFromRoleRef(rb.Namespace(), rb.RoleRef())
+	if err != nil {
+		klog.Infof("failed to retrieve a role for a rolebinding ref: %v", err)
+		return false
+	}
+
+	// TODO: actually cache the relevant rolebindings and relevant roles
+	// or maybe only the roles and update cached roles on a role update?
+	return c.IsRoleInvolvesSCCs(role, false)
+}
+
+func (c *SAToSCCCache) IsRoleInvolvesSCCs(obj interface{}, isRoleUpdate bool) bool {
+	role, err := newRoleObj(obj)
+	if err != nil {
+		klog.Warningf("unexpected error, this may be a bug: %v", err)
+		return false
+	}
+
+	sccs, err := c.sccLister.List(labels.Everything()) // TODO: this should probably requeue, right?
+	if err != nil {
+		klog.Warning("failed to list SCCs: %v", err)
+		return false
+	}
+
+	if isRoleUpdate {
+		c.SyncRoleCache(role.Namespace(), role.Name(), role.Rules(), sccs)
+	}
+
+	c.usefulRolesLock.RLock()
+	defer c.usefulRolesLock.RUnlock()
+	return len(c.usefulRoles[fmt.Sprintf("%s/%s", role.Namespace(), role.Name())]) != 0
+}
+
+func (c *SAToSCCCache) InitializeRoleCache() error {
+	roles, err := c.roleLister.List(labels.Everything())
+	if err != nil {
+		return fmt.Errorf("failed to initialize role cache: %w", err)
+	}
+
+	clusterRoles, err := c.clusterRoleLister.List(labels.Everything())
+	if err != nil {
+		return fmt.Errorf("failed to initialize role cache: %w", err)
+	}
+
+	sccs, err := c.sccLister.List(labels.Everything())
+	if err != nil {
+		return fmt.Errorf("failed to initialize role cache: %w", err)
+	}
+
+	for _, r := range roles {
+		c.SyncRoleCache(r.Namespace, r.Name, r.Rules, sccs)
+	}
+
+	for _, r := range clusterRoles {
+		c.SyncRoleCache(r.Namespace, r.Name, r.Rules, sccs)
+	}
+
+	return nil
+}
+
+func (c *SAToSCCCache) SyncRoleCache(roleNS, roleName string, rules []rbacv1.PolicyRule, sccs []*securityv1.SecurityContextConstraints) {
+	dummyUserInfo := &user.DefaultInfo{
+		Name: "dummyUser",
+	}
+	if allowedSCCs := SCCsAllowedByPolicyRules("", dummyUserInfo, sccs, rules); len(allowedSCCs) > 0 {
+		c.usefulRolesLock.Lock()
+		c.usefulRoles[fmt.Sprintf("%s/%s", roleNS, roleName)] = allowedSCCs
+		c.usefulRolesLock.Unlock()
+	}
+}
+
 func SCCsAllowedByPolicyRules(nsName string, saUserInfo user.Info, sccs []*securityv1.SecurityContextConstraints, rules []rbacv1.PolicyRule) []string {
 	ar := authorizer.AttributesRecord{
 		User:            saUserInfo,