perf(rbac): pre-compile regex patterns at sync time, skip regex for literals#434
Open
DioCrafts wants to merge 2 commits intokite-org:mainfrom
Open
perf(rbac): pre-compile regex patterns at sync time, skip regex for literals#434DioCrafts wants to merge 2 commits intokite-org:mainfrom
DioCrafts wants to merge 2 commits intokite-org:mainfrom
Conversation
…iterals Finding 1.4: match() called regexp.Compile() on every RBAC check, which runs on every protected HTTP request (via RBACMiddleware), plus N times per item in namespace List filtering. This was ~1-5µs per Compile × N roles × M patterns. Solution A — Pre-compiled roles (compiledRole struct): - New compiledPattern type holds pre-compiled *regexp.Regexp alongside wildcard/negation/literal flags - compilePatterns() runs once per sync cycle (~60s) in loadRolesFromDB() - matchCompiled() replaces match(): zero regexp.Compile on the hot path - compiledRoles slice stored alongside RBACConfig, protected by rwlock - CanAccess/CanAccessCluster/CanAccessNamespace now use getCompiledUserRoles() Solution D — Skip regex for literal patterns: - hasRegexMeta() detects patterns without regex metacharacters - Literal patterns (e.g. 'pods', 'get', 'default') have re=nil - matchCompiled() resolves them via == comparison in ~5.6ns vs ~80ns for regex Dead code removed: - Old match() function with per-call regexp.Compile - Old findRole() (replaced by findCompiledRole) - Removed 'regexp' and 'strings' imports from rbac.go - Eliminated redundant double rwlock.RLock in findRole Benchmark results (zero allocations for matching): Wildcard: ~1.8ns/op 0 allocs Literal: ~5.6ns/op 0 allocs Regex: ~80ns/op 0 allocs Full CanAccess: ~620ns/op 8 allocs Before: a single regexp.Compile was ~1000-5000ns. Now the ENTIRE CanAccess with 2 roles + regex patterns is faster than one old Compile call. New tests (10): matchCompiled unit tests for wildcard, literal, regex, negation, empty, invalid regex, literal-skips-regex (Solution D), regex-has-meta, CanAccessCluster, CanAccessNamespace. Benchmarks (4): matchCompiled regex/literal/wildcard + full CanAccess. All 23/23 tests pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2bbd6f4fc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The getCompiledUserRoles() function was recompiling roles on every request when user.Roles was pre-populated by RequireAuth (the common path for all authenticated requests), completely negating the sync-time precompilation optimization. Fix: look up each role by name in the compiledRoles cache via findCompiledRole() instead of calling compileRole() on every request. Fall back to on-the-fly compilation only for roles not found in cache. Also adds proper rwlock.RLock() when reading from the shared compiledRoles slice in the pre-resolved path. Addresses P2 review finding from PR code review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
⚡ perf(rbac): Pre-compile regex patterns at sync time — ~100x faster RBAC checks
TL;DR
Every single HTTP request protected by
RBACMiddlewarewas paying 1–5µs per regex compilation × N roles × M patterns — on every request, for patterns that change once every 60 seconds. This PR eliminatesregexp.Compilefrom the hot path entirely, reducing RBAC check latency from ~24–120µs to ~620ns (measured), with zero heap allocations during matching.🔍 The Problem
The
match()function inpkg/rbac/rbac.gowas the backbone of every RBAC authorization decision:regexp.Compileis expensive: it parses the regex grammar, builds a non-deterministic finite automaton (NFA), and optimizes it into the internal representation. This was happening:RBACMiddleware→CanAccess()CanAccessNamespace()in ListCanAccessCluster()For a typical request with 2 roles and ~3 patterns per field:
regexp.Compilecalls per request → ~24–120µs wastedFor a namespace List with 50 items:
regexp.Compilecalls → 0.6–3ms of pure wasteMeanwhile, RBAC roles are synced from the database every 60 seconds. The patterns are effectively immutable between syncs, yet we were recompiling them thousands of times per minute.
🛠️ The Solution
Two complementary optimizations applied together:
Solution 1 — Pre-compiled roles (
compiledRolestruct)Compile all regex patterns once when roles are loaded from the database, not on every access check.
New type in
pkg/rbac/compiled.go:Compilation happens once in
loadRolesFromDB()(every ~60s):Hot-path matching uses pre-compiled patterns — zero
regexp.Compile:Solution 2 — Skip regex entirely for literal patterns
Most real-world RBAC patterns are plain strings:
"pods","get","default","dev-cluster". These contain no regex metacharacters and don't need regex at all.During compilation, patterns without metacharacters get
re = nil. At match time, they resolve via a simple==comparison in ~5.6ns instead of invoking the regex engine (~80ns). This is the common case in production.📊 Benchmark Results (measured on this codebase)
All matching operations now produce zero heap allocations:
Before vs After comparison
"*"match"pods""dev.*"CanAccess()(2 roles)Real-world impact estimates
📁 Files Changed
pkg/rbac/compiled.go(new)compiledPattern,compiledRoletypes,compilePatterns(),compileRole(),matchCompiled(),hasRegexMeta()pkg/rbac/compiled_test.go(new)pkg/rbac/manager.goloadRolesFromDB()now pre-compiles all patterns intocompiledRolesslice on every syncpkg/rbac/rbac.goCanAccess/CanAccessCluster/CanAccessNamespacerewritten to usematchCompiled(). NewgetCompiledUserRoles()+findCompiledRole(). Oldmatch(),findRole()removedpkg/rbac/rbac_test.gosetTestRBACConfig()helper ensures bothRBACConfigandcompiledRolesare populated for tests🗑️ Dead Code Removed
match()— the old function that calledregexp.Compile()on every invocationfindRole()— replaced byfindCompiledRole()which looks up pre-compiled rolesregexpimport fromrbac.go— no longer needed in the authorization filestringsimport fromrbac.go— negation logic moved to compiled patternsrwlock.RLock()— the oldfindRole()acquired a read lock even though its callerGetUserRoles()already held one. Not a deadlock (Go allows concurrent readers), but wasteful. Eliminated.🧪 Test Coverage
23 tests total — all passing:
Existing tests (13) — unchanged behavior verified
TestCanAccesssubtests continue to pass identically, confirming behavioral equivalenceNew tests (10)
TestMatchCompiledWildcard"*"matches any valueTestMatchCompiledLiteralExactTestMatchCompiledRegexTestMatchCompiledNegation"!kube-system"+"*"blocks kube-systemTestMatchCompiledEmptyTestMatchCompiledInvalidRegexTestMatchCompiledLiteralSkipsRegex"pods"hasre == nil(Solution D proof)TestMatchCompiledRegexHasMeta"dev-.*"correctly gets compiled regexpTestCanAccessClusterCompiledTestCanAccessNamespaceCompiledBenchmarks (4)
BenchmarkMatchCompiledBenchmarkMatchCompiledLiteralBenchmarkMatchCompiledWildcardBenchmarkCanAccessFullCompiled🔒 Safety & Backwards Compatibility
CanAccess(),CanAccessCluster(),CanAccessNamespace(),GetUserRoles()all retain their exact signaturescompiledRolesis protected by the samerwlockasRBACConfig, swapped atomically under write lockregexp.Regexp.MatchStringis goroutine-safe: no mutex needed for concurrent reads💡 Why This Matters
RBAC authorization sits in the absolute hot path of every API request. It's the tax paid before any business logic runs. By moving regex compilation from O(requests × roles × patterns) to O(sync_cycles × roles × patterns), we:
regexp.Regexpobjects per secondThe compile cost is now amortized across the 60-second sync interval, making it effectively free.