Handle missing rules from escalation errors

Also sort final missing rules by namespace

Signed-off-by: Tayler Geiger <[email protected]>

Author: trgeiger

Diff for: internal/operator-controller/authorization/rbac.go

@@ -6,7 +6,10 @@ import (
 	"fmt"
 	"io"
 	"maps"
+	"reflect"
+	"regexp"
 	"sort"
+	"strings"
 
 	rbacv1 "k8s.io/api/rbac/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -91,8 +94,14 @@ func (a *rbacPreAuthorizer) PreAuthorize(ctx context.Context, manifestManager us
 
 	for _, obj := range dm.rbacObjects() {
 		if err := ec.checkEscalation(ctx, manifestManager, obj); err != nil {
-			// In Kubernetes 1.32.2 the specialized PrivilegeEscalationError is gone.
-			// Instead, we simply collect the error.
+			missingEscalationRules, namespace := parseEscalationErrorForMissingRules(err)
+			// Check if we already have these escalation PolicyRules, if so don't append
+			for i, rule := range missingEscalationRules {
+				previousRule := missingRules[namespace][len(missingRules[namespace])-len(missingEscalationRules)+i]
+				if !arePolicyRulesEqual(previousRule, rule) {
+					missingRules[namespace] = append(missingRules[namespace], missingEscalationRules...)
+				}
+			}
 			preAuthEvaluationErrors = append(preAuthEvaluationErrors, err)
 		}
 	}
@@ -110,6 +119,11 @@ func (a *rbacPreAuthorizer) PreAuthorize(ctx context.Context, manifestManager us
 		allMissingPolicyRules = append(allMissingPolicyRules, ScopedPolicyRules{Namespace: ns, MissingRules: sortableRules})
 	}
 
+	// sort allMissingPolicyRules alphabetically by namespace
+	sort.Slice(allMissingPolicyRules, func(i, j int) bool {
+		return allMissingPolicyRules[i].Namespace < allMissingPolicyRules[j].Namespace
+	})
+
 	if len(preAuthEvaluationErrors) > 0 {
 		return allMissingPolicyRules, fmt.Errorf("authorization evaluation errors: %w", errors.Join(preAuthEvaluationErrors...))
 	}
@@ -484,6 +498,42 @@ var fullAuthority = []rbacv1.PolicyRule{
 	{Verbs: []string{"*"}, NonResourceURLs: []string{"*"}},
 }
 
+func parseEscalationErrorForMissingRules(ecError error) ([]rbacv1.PolicyRule, string) {
+	// Regex to capture namespace and serviceaccount
+	userRegex := regexp.MustCompile(`system:serviceaccount:(?P<Namespace>[^:]+):(?P<ServiceAccount>[^"]+)`)
+	// Regex to capture missing permissions
+	permRegex := regexp.MustCompile(`{APIGroups:\[("[^"]*")\], Resources:\[("[^"]*")\], Verbs:\[("[^"]*")\]}`)
+
+	userMatches := userRegex.FindStringSubmatch(ecError.Error())
+	namespace := userMatches[1]
+
+	// Extract permissions
+	var permissions []rbacv1.PolicyRule
+	permMatches := permRegex.FindAllStringSubmatch(ecError.Error(), -1)
+	for _, match := range permMatches {
+		permissions = append(permissions, rbacv1.PolicyRule{
+			APIGroups: []string{strings.Trim(match[1], `"`)},
+			Resources: []string{strings.Trim(match[2], `"`)},
+			Verbs:     []string{strings.Trim(match[3], `"`)},
+		})
+	}
+
+	return permissions, namespace
+}
+
+func arePolicyRulesEqual(rule1 rbacv1.PolicyRule, rule2 rbacv1.PolicyRule) bool {
+	sort.Strings(rule1.Verbs)
+	sort.Strings(rule2.Verbs)
+	sort.Strings(rule1.APIGroups)
+	sort.Strings(rule2.APIGroups)
+	sort.Strings(rule1.Resources)
+	sort.Strings(rule2.Resources)
+	sort.Strings(rule1.ResourceNames)
+	sort.Strings(rule2.ResourceNames)
+
+	return reflect.DeepEqual(rule1, rule2)
+}
+
 func hasAggregationRule(clusterRole *rbacv1.ClusterRole) bool {
 	// Currently, an aggregation rule is considered present only if it has one or more selectors.
 	// An empty slice of ClusterRoleSelectors means no selectors were provided,

Diff for: internal/operator-controller/authorization/rbac_test.go

@@ -55,6 +55,76 @@ subjects:
 - kind: ServiceAccount
   name: test-serviceaccount
   namespace: test-namespace
+  `
+
+	testManifestMultiNamespace = `apiVersion: v1
+kind: Service
+metadata:
+  name: test-service
+  namespace: test-namespace
+spec:
+  clusterIP: None
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: test-extension-role
+  namespace: test-namespace
+rules:
+- apiGroups: ["*"]
+  resources: [serviceaccounts]
+  verbs: [watch]
+- apiGroups: ["*"]
+  resources: [certificates]
+  verbs: [create]
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: test-extension-binding
+  namespace: test-namespace
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: test-extension-role
+subjects:
+- kind: ServiceAccount
+  name: test-serviceaccount
+  namespace: test-namespace
+---
+kind: Service
+metadata:
+  name: test-service
+  namespace: a-test-namespace
+spec:
+  clusterIP: None
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: test-extension-role
+  namespace: a-test-namespace
+rules:
+- apiGroups: ["*"]
+  resources: [serviceaccounts]
+  verbs: [watch]
+- apiGroups: ["*"]
+  resources: [certificates]
+  verbs: [create]
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: test-extension-binding
+  namespace: a-test-namespace
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: test-extension-role
+subjects:
+- kind: ServiceAccount
+  name: test-serviceaccount
+  namespace: a-test-namespace
   `
 
 	saName             = "test-serviceaccount"
@@ -158,7 +228,7 @@ func TestPreAuthorize_Success(t *testing.T) {
 }
 
 func TestPreAuthorize_Failure(t *testing.T) {
-	t.Run("preauthorize failes with missing rbac rules", func(t *testing.T) {
+	t.Run("preauthorize fails with missing rbac rules", func(t *testing.T) {
 		featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.PreflightPermissions, true)
 		fakeClient := setupFakeClient(limitedClusterRole)
 		preAuth := NewRBACPreAuthorizer(fakeClient)
@@ -168,6 +238,17 @@ func TestPreAuthorize_Failure(t *testing.T) {
 	})
 }
 
+func TestPreAuthorizeMultiNamespace_Failure(t *testing.T) {
+	t.Run("preauthorize fails with missing rbac rules in multiple namespaces", func(t *testing.T) {
+		featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.PreflightPermissions, true)
+		fakeClient := setupFakeClient(limitedClusterRole)
+		preAuth := NewRBACPreAuthorizer(fakeClient)
+		missingRules, err := preAuth.PreAuthorize(context.TODO(), &testServiceAccount, strings.NewReader(testManifestMultiNamespace))
+		require.Error(t, err)
+		require.NotEqual(t, []ScopedPolicyRules{}, missingRules)
+	})
+}
+
 func TestPreAuthorize_CheckEscalation(t *testing.T) {
 	t.Run("preauthorize succeeds with no missing rbac rules", func(t *testing.T) {
 		featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.PreflightPermissions, true)