perf(rbac): pre-compile regex patterns at sync time, skip regex for literals

pkg/rbac/compiled.go

@@ -0,0 +1,112 @@
+package rbac
+
+import (
+	"regexp"
+	"strings"
+
+	"github.com/zxh326/kite/pkg/common"
+	"k8s.io/klog/v2"
+)
+
+// compiledPattern holds a pre-compiled representation of a single RBAC pattern.
+// Patterns are compiled once during role sync and reused for every access check,
+// eliminating regexp.Compile from the hot path entirely.
+type compiledPattern struct {
+	raw      string         // original pattern string
+	negate   bool           // true if prefixed with "!"
+	wildcard bool           // true if pattern is "*"
+	literal  string         // for exact string comparison (negation target or plain value)
+	re       *regexp.Regexp // non-nil only when the pattern contains regex metacharacters
+}
+
+// compiledRole wraps a common.Role with pre-compiled patterns for each field.
+type compiledRole struct {
+	common.Role
+	clusters   []compiledPattern
+	namespaces []compiledPattern
+	resources  []compiledPattern
+	verbs      []compiledPattern
+}
+
+// regexpMetaDetector matches any regex metacharacter. If a pattern contains none
+// of these, it is a literal string and regex matching can be skipped entirely (Solution D).
+var regexpMetaDetector = regexp.MustCompile(`[\\.*+?^${}()|[\]]`)
+
+// hasRegexMeta returns true if the pattern contains regex metacharacters.
+func hasRegexMeta(p string) bool {
+	return regexpMetaDetector.MatchString(p)
+}
+
+// compilePatterns converts a slice of raw pattern strings into pre-compiled patterns.
+// Called once per sync cycle (every ~60s), not on the hot path.
+func compilePatterns(patterns []string) []compiledPattern {
+	out := make([]compiledPattern, 0, len(patterns))
+	for _, p := range patterns {
+		cp := compiledPattern{raw: p}
+
+		switch {
+		case len(p) > 1 && strings.HasPrefix(p, "!"):
+			// Negation pattern: "!kube-system"
+			cp.negate = true
+			cp.literal = p[1:]
+
+		case p == "*":
+			// Wildcard: matches everything
+			cp.wildcard = true
+
+		default:
+			// Store literal for exact comparison (always attempted first)
+			cp.literal = p
+
+			// Only compile regex if pattern has metacharacters (Solution D)
+			if hasRegexMeta(p) {
+				re, err := regexp.Compile(p)
+				if err != nil {
+					klog.Errorf("rbac: invalid regex pattern %q: %v", p, err)
+					// Keep as literal-only (will still match via == check)
+				} else {
+					cp.re = re
+				}
+			}
+		}
+
+		out = append(out, cp)
+	}
+	return out
+}
+
+// compileRole converts a common.Role into a compiledRole with pre-compiled patterns.
+func compileRole(r common.Role) compiledRole {
+	return compiledRole{
+		Role:       r,
+		clusters:   compilePatterns(r.Clusters),
+		namespaces: compilePatterns(r.Namespaces),
+		resources:  compilePatterns(r.Resources),
+		verbs:      compilePatterns(r.Verbs),
+	}
+}
+
+// matchCompiled evaluates pre-compiled patterns against a value.
+// Zero allocations, zero regexp.Compile calls on the hot path.
+func matchCompiled(patterns []compiledPattern, val string) bool {
+	for i := range patterns {
+		p := &patterns[i]
+
+		if p.negate {
+			if p.literal == val {
+				return false
+			}
+			continue
+		}
+
+		if p.wildcard || p.literal == val {
+			return true
+		}
+
+		// Only invoke regex if a compiled regexp exists (pattern had metacharacters)
+		if p.re != nil && p.re.MatchString(val) {
+			return true
+		}
+	}
+	return false
+}

pkg/rbac/compiled_test.go

@@ -0,0 +1,201 @@
+package rbac
+
+import (
+	"testing"
+
+	"github.com/zxh326/kite/pkg/common"
+	"github.com/zxh326/kite/pkg/model"
+)
+
+// --- Unit tests for compilePatterns / matchCompiled ---
+
+func TestMatchCompiledWildcard(t *testing.T) {
+	patterns := compilePatterns([]string{"*"})
+	if !matchCompiled(patterns, "anything") {
+		t.Error("wildcard should match any value")
+	}
+}
+
+func TestMatchCompiledLiteralExact(t *testing.T) {
+	patterns := compilePatterns([]string{"pods", "services"})
+	if !matchCompiled(patterns, "pods") {
+		t.Error("literal pattern should match exact value")
+	}
+	if !matchCompiled(patterns, "services") {
+		t.Error("literal pattern should match exact value")
+	}
+	if matchCompiled(patterns, "deployments") {
+		t.Error("literal pattern should not match different value")
+	}
+}
+
+func TestMatchCompiledRegex(t *testing.T) {
+	patterns := compilePatterns([]string{"dev.*"})
+	if !matchCompiled(patterns, "dev-cluster") {
+		t.Error("regex pattern should match")
+	}
+	if !matchCompiled(patterns, "dev") {
+		t.Error("regex pattern should match exact prefix")
+	}
+	if matchCompiled(patterns, "prod-cluster") {
+		t.Error("regex pattern should not match non-matching value")
+	}
+}
+
+func TestMatchCompiledNegation(t *testing.T) {
+	patterns := compilePatterns([]string{"!kube-system", "*"})
+	if !matchCompiled(patterns, "default") {
+		t.Error("negation+wildcard should match non-negated value")
+	}
+	if matchCompiled(patterns, "kube-system") {
+		t.Error("negation should block exact match")
+	}
+}
+
+func TestMatchCompiledEmpty(t *testing.T) {
+	patterns := compilePatterns([]string{})
+	if matchCompiled(patterns, "anything") {
+		t.Error("empty patterns should match nothing")
+	}
+}
+
+func TestMatchCompiledInvalidRegex(t *testing.T) {
+	// Invalid regex should be treated as literal-only; should not panic.
+	patterns := compilePatterns([]string{"[invalid"})
+	if matchCompiled(patterns, "anything") {
+		t.Error("invalid regex pattern should not match arbitrary values")
+	}
+	// But should still match the literal string itself
+	if !matchCompiled(patterns, "[invalid") {
+		t.Error("invalid regex should still match as literal")
+	}
+}
+
+func TestMatchCompiledLiteralSkipsRegex(t *testing.T) {
+	// Pattern "pods" has no metacharacters → re should be nil (Solution D)
+	patterns := compilePatterns([]string{"pods"})
+	if len(patterns) != 1 {
+		t.Fatalf("expected 1 pattern, got %d", len(patterns))
+	}
+	if patterns[0].re != nil {
+		t.Error("literal pattern 'pods' should not have a compiled regexp (Solution D)")
+	}
+	if !matchCompiled(patterns, "pods") {
+		t.Error("literal pattern should still match via == comparison")
+	}
+}
+
+func TestMatchCompiledRegexHasMeta(t *testing.T) {
+	patterns := compilePatterns([]string{"dev-.*"})
+	if len(patterns) != 1 {
+		t.Fatalf("expected 1 pattern, got %d", len(patterns))
+	}
+	if patterns[0].re == nil {
+		t.Error("pattern 'dev-.*' has metacharacters and should have compiled regexp")
+	}
+}
+
+// --- Integration: CanAccessCluster / CanAccessNamespace with compiled roles ---
+
+func TestCanAccessClusterCompiled(t *testing.T) {
+	setTestRBACConfig(
+		[]common.Role{{
+			Name:       "dev-access",
+			Clusters:   []string{"dev-.*"},
+			Namespaces: []string{"*"},
+			Resources:  []string{"*"},
+			Verbs:      []string{"*"},
+		}},
+		[]common.RoleMapping{{Name: "dev-access", Users: []string{"alice"}}},
+	)
+
+	if !CanAccessCluster(model.User{Username: "alice"}, "dev-east") {
+		t.Error("should match dev-east via regex")
+	}
+	if CanAccessCluster(model.User{Username: "alice"}, "prod-west") {
+		t.Error("should NOT match prod-west")
+	}
+	if CanAccessCluster(model.User{Username: "bob"}, "dev-east") {
+		t.Error("bob should have no roles")
+	}
+}
+
+func TestCanAccessNamespaceCompiled(t *testing.T) {
+	setTestRBACConfig(
+		[]common.Role{{
+			Name:       "ns-role",
+			Clusters:   []string{"*"},
+			Namespaces: []string{"!kube-system", "team-.*"},
+			Resources:  []string{"*"},
+			Verbs:      []string{"*"},
+		}},
+		[]common.RoleMapping{{Name: "ns-role", Users: []string{"*"}}},
+	)
+
+	if !CanAccessNamespace(model.User{Username: "anyone"}, "any-cluster", "team-alpha") {
+		t.Error("should match team-alpha via regex")
+	}
+	if CanAccessNamespace(model.User{Username: "anyone"}, "any-cluster", "kube-system") {
+		t.Error("kube-system should be negated")
+	}
+	if CanAccessNamespace(model.User{Username: "anyone"}, "any-cluster", "default") {
+		t.Error("default does not match team-.* pattern")
+	}
+}
+
+// --- Benchmarks: old sequential compile vs pre-compiled ---
+
+func BenchmarkMatchCompiled(b *testing.B) {
+	patterns := compilePatterns([]string{"dev-.*", "staging-.*", "!kube-system", "prod"})
+	b.ResetTimer()
+	for b.Loop() {
+		matchCompiled(patterns, "dev-east")
+	}
+}
+
+func BenchmarkMatchCompiledLiteral(b *testing.B) {
+	patterns := compilePatterns([]string{"get", "create", "update", "delete"})
+	b.ResetTimer()
+	for b.Loop() {
+		matchCompiled(patterns, "update")
+	}
+}
+
+func BenchmarkMatchCompiledWildcard(b *testing.B) {
+	patterns := compilePatterns([]string{"*"})
+	b.ResetTimer()
+	for b.Loop() {
+		matchCompiled(patterns, "anything")
+	}
+}
+
+func BenchmarkCanAccessFullCompiled(b *testing.B) {
+	setTestRBACConfig(
+		[]common.Role{
+			{
+				Name:       "dev",
+				Clusters:   []string{"dev-.*"},
+				Namespaces: []string{"!kube-system", "team-.*"},
+				Resources:  []string{"pods", "deployments", "services"},
+				Verbs:      []string{"get", "create", "update"},
+			},
+			{
+				Name:       "admin",
+				Clusters:   []string{"*"},
+				Namespaces: []string{"*"},
+				Resources:  []string{"*"},
+				Verbs:      []string{"*"},
+			},
+		},
+		[]common.RoleMapping{
+			{Name: "dev", Users: []string{"dev-user"}},
+			{Name: "admin", Users: []string{"admin-user"}},
+		},
+	)
+
+	user := model.User{Username: "dev-user"}
+	b.ResetTimer()
+	for b.Loop() {
+		CanAccess(user, "pods", "get", "dev-east", "team-alpha")
+	}
+}

pkg/rbac/manager.go

@@ -11,9 +11,10 @@ import (
 )
 
 var (
-	RBACConfig *common.RolesConfig
-	once       sync.Once
-	rwlock     sync.RWMutex
+	RBACConfig    *common.RolesConfig
+	compiledRoles []compiledRole // pre-compiled regex patterns, rebuilt on every sync
+	once          sync.Once
+	rwlock        sync.RWMutex
 )
 
 func InitRBAC() {
@@ -60,8 +61,16 @@ func loadRolesFromDB() error {
 			cfg.RoleMapping = append(cfg.RoleMapping, rm)
 		}
 	}
+	// Pre-compile all regex patterns once (Solutions A+D).
+	// This runs every ~60s on sync, never on the hot request path.
+	compiled := make([]compiledRole, len(cfg.Roles))
+	for i, r := range cfg.Roles {
+		compiled[i] = compileRole(r)
+	}
+
 	rwlock.Lock()
 	RBACConfig = cfg
+	compiledRoles = compiled
 	rwlock.Unlock()
 	return nil
 }