feat: precomputed scopes by adityathebe · Pull Request #2719 · flanksource/mission-control

adityathebe · 2026-01-14T10:06:04Z

resolves: #2395
dependsOn: flanksource/duty#1728

Summary by CodeRabbit

New Features
- Background jobs added to materialize scopes and purge deleted scopes on a 24h schedule
- New materialization workflow and event handlers for scope and permission materialization
Bug Fixes / Improvements
- Improved role selection for RLS-related tokens and more deterministic RLS payload ordering
Tests
- Tests updated to materialize scopes/permissions during setup for more reliable assertions

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-14T10:06:11Z

Walkthrough

Adds scope/permission materialization: new event constants, background jobs to materialize/cleanup scopes, an RLS payload refactor to collect scope UUIDs, permission-driven materialization engine, test updates, and a minor dependency/go.mod and server import change.

Changes

Cohort / File(s)	Summary
API Event Constants `api/event.go`	Added `EventScopeMaterialize` and `EventPermissionMaterialize` constants.
RLS Payload Refactor `auth/rls.go`	Rewrote payload construction to collect scope UUIDs via a `map[uuid.UUID]struct{}`; introduced `processScopeRefs(ctx, scopeRefs, scopeIDs)` and `setToSortedUUIDSlice`; changed RLS bypass to SET LOCAL ROLE when configured.
Background Jobs `jobs/jobs.go`, `jobs/rls_scope_jobs.go`	Scheduled two new jobs (`materializeAllScopesJob`, `cleanupDeletedScopesJob`) and added job implementations and 24h schedule constants.
Permission Events & Processor `permission/events.go`, `permission/rls_scopes.go`	New event handlers for materialize events and a full ScopeProcessor materialization engine (GetProcessScopeJob, ScopeMaterializationStats, apply/remove/rebuild flows, per-table batch updates).
Server Import Registration `cmd/server.go`	Added side-effect import of the `permission` package to register handlers.
Tests & Fixtures `tests/permissions/*`, `tests/permissions/testdata/scopes/...`	Added test helper `materializeScopesAndPermissions`, updated test assertions to check payload.Scopes, and updated scope test UID.
Permission / DB model tweak `api/v1/permission_types.go`, `db/permissions.go`	`PermissionObject.GlobalObject()` now returns `("", false)` unconditionally; db code simplified to always set `ObjectSelector`.
Auth token / RLS usage `auth/tokens.go`	Retrieve RLS payload earlier and compute DB role (supports DBRoleBypass when RLS disabled).
Dependency `go.mod`	Updated `github.com/flanksource/duty` to a pseudo-version (temporary).

Sequence Diagram

sequenceDiagram
    participant Scheduler as Background Scheduler
    participant Jobs as Job System
    participant Events as Event Handler
    participant Permission as Permission Processor
    participant DB as Database

    Scheduler->>Jobs: schedule materializeAllScopesJob
    Jobs->>DB: query non-deleted scopes
    DB-->>Jobs: scopes list

    loop per scope
        Jobs->>Events: enqueue EventScopeMaterialize(scope_id, rebuild)
        Events->>Permission: GetProcessScopeJob(scope_id, "rebuild")
        Permission->>DB: fetch scope/permission details
        DB-->>Permission: scope targets & selectors
        Permission->>DB: apply __scope updates (batch per table)
        DB-->>Permission: update results
        Permission-->>Events: job result (success/errors)
    end

    Scheduler->>Jobs: schedule cleanupDeletedScopesJob
    Jobs->>DB: query deleted scopes
    DB-->>Jobs: deleted scope list

    loop per deleted scope
        Jobs->>Events: enqueue EventScopeMaterialize(scope_id, remove)
        Events->>Permission: GetProcessScopeJob(scope_id, "remove")
        Permission->>DB: remove scope from tables
        DB-->>Permission: removal confirmation
    end

Possibly related PRs

feat/view rls #2538 — Modifies auth/rls.go to include scope UUIDs in RLS payload and related scope-processing logic.
chore: bump duty to v1.0.1153 #2703 — Another change to go.mod updating github.com/flanksource/duty (related dependency change).
chore: bump duty to v1.0.1157 #2712 — Also updates go.mod for github.com/flanksource/duty to a v1.0.1157 series version (related dependency change).

Suggested labels

ready

Suggested reviewers

moshloop
yashmehrotra

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: precomputed scopes' is concise and directly related to the main objective of introducing precomputed scope materialization for row-level security.
Linked Issues check	✅ Passed	The PR implements core requirements from issue `#2395`: introduces Scope resource processing, materializes scope assignments to resources via RLS scope tables, and enables row-level security based on scope membership.
Out of Scope Changes check	✅ Passed	All changes directly support scope materialization: new event constants, RLS payload refactoring for scope-centric approach, background jobs for materialization, permission event handling, and test updates.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feat/precomputed-scopes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

auth/rls.go (1)

97-104: Build failure: SetPostgresSessionRLSWithRole is undefined on rls.Payload.

The pipeline failures show that rlsPayload.SetPostgresSessionRLSWithRole does not exist on *rls.Payload. This method must be added to the duty package.

Both this and the DBRoleBypass issue indicate the duty dependency PR needs to be merged first, and this PR's go.mod updated to reference the merged version.

🤖 Fix all issues with AI agents

In `@auth/rls.go`:
- Around line 79-94: Add the missing DBRoleBypass field to the PostgrestConfig
in the duty package so the reference
dutyAPI.DefaultConfig.Postgrest.DBRoleBypass compiles: add a DBRoleBypass string
field to the PostgrestConfig struct, ensure DefaultConfig initializes it (and
any config parsing/loading populates it, e.g., from env/YAML), and update any
relevant tests or config docs; this will allow the rls.go code that reads
dutyAPI.DefaultConfig.Postgrest.DBRoleBypass and falls back to DBRole to build
successfully.

In `@auth/tokens.go`:
- Around line 74-82: The code references the non-existent field
config.Postgrest.DBRoleBypass causing build failures; update the role selection
in the block around GetRLSPayload and the local variable role so it does not
reference DBRoleBypass — keep role := config.Postgrest.DBRole and, for now,
remove the rlsPayload.Disable && config.Postgrest.DBRoleBypass != "" branch (or
replace it with a no-op/fallback that leaves role as DBRole), and add a TODO
noting to reintroduce DBRoleBypass once the duty dependency adds that field; use
the symbols GetRLSPayload, rlsPayload, and the local role to locate the change.

In `@permission/rls_scopes.go`:
- Around line 275-276: permission.Action tokens aren't trimmed before matching,
so MatchItems(policy.ActionRead, strings.Split(permission.Action, ",")...) can
miss "read" when permission.Action includes spaces; update the check in the
materialization path to normalize tokens by splitting and trimming each token
(e.g., use strings.Split and strings.TrimSpace or strings.FieldsFunc) before
calling collections.MatchItems with the cleaned slice, keeping the same
semantics for policy.ActionRead and permission.Action.
- Around line 429-433: The update can append duplicate scopeIDs when concurrent
workers race; modify the UpdateColumn call that uses
ctx.DB().Table(table).Where("id IN ?", ids).UpdateColumn("__scope",
gorm.Expr("array_append(COALESCE(__scope, '{}'::uuid[]), ?)", scopeID)) so the
WHERE clause also excludes rows that already contain scopeID (e.g. add a NOT
condition like NOT (? = ANY(__scope)) or equivalent SQL to the Where) so the
update is idempotent; keep the same symbols (ctx.DB(), table, ids, scopeID,
UpdateColumn, gorm.Expr) and ensure the combined WHERE prevents appending if
__scope already includes scopeID.

🧹 Nitpick comments (2)

jobs/rls_scope_jobs.go (1)

18-26: Consider gating RunNow after the migration.
With RunNow: true, every startup will trigger a full rebuild. If this was only for the initial migration, a config/feature flag gate in Start would avoid unnecessary load later.
permission/rls_scopes.go (1)
445-452: Wrap DB errors with ctx.Oops for consistent error codes.

Both functions return raw GORM errors without context. Prefer ctx.Oops() wrapping for consistent error codes and diagnostics. As per coding guidelines, use ctx.Oops().
♻️ Proposed refactor
 	result := ctx.DB().Table(table).
 		Where("id IN ?", ids).
 		Where("NOT (COALESCE(__scope, '{}'::uuid[]) @> ARRAY[?]::uuid[])", scopeID).
 		UpdateColumn("__scope", gorm.Expr("array_append(COALESCE(__scope, '{}'::uuid[]), ?)", scopeID))
 	if result.Error != nil {
-		return 0, result.Error
+		return 0, ctx.Oops().Wrapf(result.Error, "failed to update __scope for %s", table)
 	}
 	return result.RowsAffected, nil
 }
@@
 	result := ctx.DB().Table(table).
 		Where("__scope @> ARRAY[?]::uuid[]", scopeID).
 		UpdateColumn("__scope", gorm.Expr("array_remove(__scope, ?)", scopeID))
 	if result.Error != nil {
-		return 0, result.Error
+		return 0, ctx.Oops().Wrapf(result.Error, "failed to remove __scope for %s", table)
 	}
 	return result.RowsAffected, nil
 }
Also applies to: 469-474

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f8347d0 and 0f32be7.

⛔ Files ignored due to path filters (1)

go.sum is excluded by !**/*.sum

📒 Files selected for processing (13)

api/event.go
api/v1/permission_types.go
auth/rls.go
auth/tokens.go
cmd/server.go
db/permissions.go
go.mod
jobs/jobs.go
jobs/rls_scope_jobs.go
permission/events.go
permission/rls_scopes.go
tests/permissions/permissions_test.go
tests/permissions/testdata/scopes/multi-target-scope.yaml

🚧 Files skipped from review as they are similar to previous changes (2)

cmd/server.go
tests/permissions/testdata/scopes/multi-target-scope.yaml

🧰 Additional context used

📓 Path-based instructions (3)

**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Use context from github.com/flanksource/duty/context.Context to access db (ctx.DB()), properties (ctx.Properties()), and logger (ctx.Logger)
If a file needs to use both context and duty's context, alias context to 'gocontext'
Prefer any over interface{} in Go type declarations
Use ctx.Oops() to craft new errors with error codes from github.com/flanksource/duty/api as tags
Use dutyAPI.WriteError(c, err) in HTTP handlers to return errors with proper HTTP status code mapping based on error codes
For validation errors with no underlying error in HTTP handlers, use dutyAPI.Errorf(dutyAPI.EINVALID, "message")
For wrapping database/internal errors in HTTP handlers, use ctx.Oops().Wrap(err) or ctx.Oops().Wrapf(err, "context")
For permission errors in HTTP handlers, use ctx.Oops().Code(dutyAPI.EFORBIDDEN).Errorf("message")
Use duty.Now() instead of time.Now() for database timestamps and soft deletes
Only add comments if really necessary. Do not add comments that simply explain the code. Exception: comments about functions are considered good practice in Go even if they are self-explanatory

Files:

db/permissions.go
api/v1/permission_types.go
api/event.go
auth/tokens.go
jobs/rls_scope_jobs.go
permission/events.go
tests/permissions/permissions_test.go
jobs/jobs.go
auth/rls.go
permission/rls_scopes.go

api/v1/**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

CRD definitions must be located in the api/v1 directory

Files:

api/v1/permission_types.go

**/*_test.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*_test.go: Always use ginkgo to run tests. Never run go test directly
Always use github.com/onsi/gomega package for assertions in test files
When using gomega with native go tests, use g := gomega.NewWithT(t) and g.Expect() for assertions

Files:

tests/permissions/permissions_test.go

🧠 Learnings (8)