Skip to content

feat(policy): Namespace subject mappings and condition sets - #3172

Merged
c-r33d merged 13 commits into
mainfrom
scs-sm-service-changes
Mar 25, 2026
Merged

feat(policy): Namespace subject mappings and condition sets#3172
c-r33d merged 13 commits into
mainfrom
scs-sm-service-changes

Conversation

@c-r33d

@c-r33d c-r33d commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

Proposed Changes

1.) Add optional namespacing to subject mappings and condition sets.
2.) Policy constructs that make up subject mappings must be of the same namespace, if a top-level namespace is defined.
3.) Guard required namespacing behind feature flag

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

Summary by CodeRabbit

Release Notes

  • New Features

    • Added namespace scoping support for subject mappings and subject condition sets, enabling organization by namespace.
    • Subject mappings and condition sets can now be filtered by namespace in list operations.
    • Introduced namespace consistency validation across related policy entities.
  • Database

    • Extended schema to support optional namespace associations for subject mappings and condition sets.

@c-r33d
c-r33d requested review from a team as code owners March 18, 2026 20:39
@github-actions github-actions Bot added comp:db DB component comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry) docs Documentation size/xl labels Mar 18, 2026
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the policy service by introducing namespace awareness to subject mappings and subject condition sets. This change allows for better organization and isolation of policy definitions within a multi-tenant or segmented environment. It also includes robust validation to maintain data integrity by ensuring that all related policy components adhere to the same namespace, preventing cross-namespace policy misconfigurations.

Highlights

  • Namespace Scoping: Introduced optional namespacing for subject mappings and subject condition sets, allowing these policy constructs to be associated with a specific namespace.
  • Namespace Consistency Enforcement: Implemented validation to ensure that all policy constructs (attribute values, actions, and subject condition sets) within a namespaced subject mapping belong to the same namespace.
  • Database Schema Updates: Added namespace_id columns to subject_condition_set and subject_mappings tables, along with foreign key constraints and indexes, to support namespace-scoped queries.
  • API and Documentation Updates: Updated gRPC API definitions and documentation to include namespace_id and namespace_fqn fields for creating and listing subject mappings and condition sets.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Ignored Files
  • Ignored by pattern: docs/openapi/**/* (8)
    • docs/openapi/policy/actions/actions.openapi.yaml
    • docs/openapi/policy/attributes/attributes.openapi.yaml
    • docs/openapi/policy/objects.openapi.yaml
    • docs/openapi/policy/obligations/obligations.openapi.yaml
    • docs/openapi/policy/registeredresources/registered_resources.openapi.yaml
    • docs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml
    • docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml
    • docs/openapi/policy/unsafe/unsafe.openapi.yaml
  • Ignored by pattern: protocol/**/* (2)
    • protocol/go/policy/objects.pb.go
    • protocol/go/policy/subjectmapping/subject_mapping.pb.go
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.


A policy defined, In namespaces, neatly aligned, Order in the code.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The pull request introduces namespace scoping for SubjectConditionSet and SubjectMapping entities, involving database schema changes, updated protobuf definitions, and refactored Go service and database logic to support namespace-aware creation, retrieval, and consistency checks. Review feedback suggests improving error messages for namespace consistency validation, optimizing the CreateSubjectMapping function to avoid a redundant GetSubjectMapping call, and adding more test cases for various namespace configurations.

Comment thread service/policy/db/subject_mappings.go Outdated
Comment thread service/policy/subjectmapping/subject_mapping.go
Comment thread service/policy/db/subject_mappings.go
Comment thread service/policy/subjectmapping/subject_mapping_test.go
@c-r33d
c-r33d force-pushed the scs-sm-service-changes branch from e9ec75d to e04d713 Compare March 18, 2026 21:04
@github-actions

Copy link
Copy Markdown
Contributor

X-Test Failure Report

@github-actions

Copy link
Copy Markdown
Contributor

X-Test Failure Report

@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 191.817957ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 96.025674ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 388.828668ms
Throughput 257.18 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 39.807915078s
Average Latency 396.437736ms
Throughput 125.60 requests/second

@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 191.819064ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 107.595456ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 398.489499ms
Throughput 250.95 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 39.933368832s
Average Latency 397.995965ms
Throughput 125.21 requests/second

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 188.718352ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 100.572999ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 390.422141ms
Throughput 256.13 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 39.553450676s
Average Latency 393.216776ms
Throughput 126.41 requests/second

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 200.970712ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 106.459022ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 394.838832ms
Throughput 253.27 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 41.166856433s
Average Latency 410.672418ms
Throughput 121.46 requests/second

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 200.34599ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 102.930489ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 389.82403ms
Throughput 256.53 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 42.29847551s
Average Latency 421.497597ms
Throughput 118.21 requests/second

@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai

coderabbitai Bot commented Mar 19, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR extends subject mappings and subject condition sets with optional namespace scoping. Changes include database migrations adding namespace_id columns, SQL queries for namespace filtering and action creation, service-layer namespace resolution and validation logic, and comprehensive integration tests.

Changes

Cohort / File(s) Summary
Database Schema & Migrations
service/policy/db/migrations/20260318000000_add_namespace_to_subject_mappings.md, service/policy/db/migrations/20260318000000_add_namespace_to_subject_mappings.sql, service/policy/db/schema_erd.md
Added nullable namespace_id columns to subject_condition_set and subject_mappings tables with foreign key references to attribute_namespaces and supporting indexes. Updated ER diagram to reflect namespace relationships and removed unique key constraints on name columns in actions and registered_resources.
Data Models
service/policy/db/models.go
Added NamespaceID pgtype.UUID field with JSON tag namespace_id to SubjectConditionSet and SubjectMapping structs.
SQL Queries - Actions
service/policy/db/queries/actions.sql, service/policy/db/actions.sql.go
Added createOrListActionsByNameInNamespace to resolve and create/list actions within a specific namespace with case-insensitive matching, and getActionsByIDs to bulk-fetch action metadata by ID list.
SQL Queries - Subject Mappings
service/policy/db/queries/subject_mappings.sql, service/policy/db/subject_mappings.sql.go
Extended listSubjectConditionSets, getSubjectConditionSet, listSubjectMappings, getSubjectMapping, and matchSubjectMappings to include namespace filtering, namespace data in results, and per-action namespace JSON. Updated createSubjectConditionSet and createSubjectMapping inserts to populate namespace_id. Converted COUNT logic to window function for pagination.
Service Layer Logic
service/policy/db/subject_mappings.go, service/policy/subjectmapping/subject_mapping.go
Added namespace parameter support to CreateSubjectConditionSet and CreateSubjectMapping signatures. Implemented namespace resolution, action resolution/creation within namespaces, namespace consistency validation across attribute values/actions/SCS, and error handling for namespace mismatches. Added validation requiring namespace when NamespacedPolicy is enabled.
Error Handling
service/pkg/db/errors.go
Added error mapping for ErrNamespaceMismatch to return connect.CodeInvalidArgument status with appropriate error message.
Integration Tests
service/integration/subject_mappings_test.go
Added extensive test coverage for namespace-scoped listing behavior, filtering by NamespaceId and NamespaceFqn, namespace consistency validation during subject mapping creation, and behavior of un-namespaced vs. namespaced resource interactions.

Sequence Diagram

sequenceDiagram
    participant Client
    participant ServiceAPI as Service API Handler
    participant DB as Policy DB Client
    participant DBConn as Database
    
    Client->>ServiceAPI: CreateSubjectMapping(request with NamespaceId/Fqn)
    
    ServiceAPI->>DB: resolveNamespace(namespaceID/fqn)
    DB->>DBConn: Query attribute_namespaces
    DBConn-->>DB: namespace record
    DB-->>ServiceAPI: resolved namespaceID
    
    ServiceAPI->>DB: resolveSubjectMappingActions(action names/ids, namespaceID)
    DB->>DBConn: createOrListActionsByNameInNamespace
    DBConn-->>DB: existing + newly created actions
    DB-->>ServiceAPI: action IDs with namespace info
    
    ServiceAPI->>DB: resolveSubjectConditionSet(condition, namespaceID)
    DB->>DBConn: Query/create subject_condition_set in namespace
    DBConn-->>DB: SCS ID with namespace
    DB-->>ServiceAPI: SCS details
    
    ServiceAPI->>DB: validateSubjectMappingNamespaceConsistency
    Note over DB: Check attribute value, actions,<br/>and SCS all in same namespace
    DB-->>ServiceAPI: validation result
    
    alt Validation passes
        ServiceAPI->>DB: createSubjectMapping(with NamespaceID)
        DB->>DBConn: INSERT subject_mappings with namespace_id
        DBConn-->>DB: created mapping ID
        DB->>DBConn: Query full mapping data
        DBConn-->>DB: mapping with namespace details
        DB-->>ServiceAPI: SubjectMapping object
        ServiceAPI-->>Client: success response
    else Validation fails
        ServiceAPI-->>Client: CodeInvalidArgument (ErrNamespaceMismatch)
    end
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~50 minutes


🐰 With namespaces in place, mappings now know where they call home,
Actions grouped by realm, condition sets no longer roam,
Consistency checks ensure each piece belongs,
While tests verify the namespace bonds,
A tidy policy garden where resources safely foam! 🌿✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding namespace support to subject mappings and condition sets, which is the primary feature across all modified files.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch scs-sm-service-changes

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (2)
service/integration/subject_mappings_test.go (2)

629-638: ⚠️ Potential issue | 🟠 Major

Test defined on wrong suite receiver.

Test_ListSubjectMappings_Limit_TooLarge_Fails is defined on NamespacesSuite instead of SubjectMappingsSuite. This will cause the test to run in the wrong suite context and may fail or not execute as intended.

🐛 Proposed fix
-func (s *NamespacesSuite) Test_ListSubjectMappings_Limit_TooLarge_Fails() {
+func (s *SubjectMappingsSuite) Test_ListSubjectMappings_Limit_TooLarge_Fails() {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@service/integration/subject_mappings_test.go` around lines 629 - 638, The
test method Test_ListSubjectMappings_Limit_TooLarge_Fails is declared with the
wrong suite receiver (NamespacesSuite) so it will run in the wrong context;
change its receiver to SubjectMappingsSuite by updating the method signature to
use (s *SubjectMappingsSuite) so the test executes under the correct suite and
has access to SubjectMappingsSuite fields and helpers (keep the body unchanged
except for the receiver).

1111-1120: ⚠️ Potential issue | 🟠 Major

Test defined on wrong suite receiver.

Test_ListSubjectConditionSets_Limit_TooLarge_Fails is defined on NamespacesSuite instead of SubjectMappingsSuite. This causes the test to run in a different test suite context.

🐛 Proposed fix
-func (s *NamespacesSuite) Test_ListSubjectConditionSets_Limit_TooLarge_Fails() {
+func (s *SubjectMappingsSuite) Test_ListSubjectConditionSets_Limit_TooLarge_Fails() {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@service/integration/subject_mappings_test.go` around lines 1111 - 1120, The
test method Test_ListSubjectConditionSets_Limit_TooLarge_Fails is declared with
the wrong receiver (NamespacesSuite); change its receiver to
SubjectMappingsSuite so the test runs in the correct suite context (i.e., update
the method signature from func (s *NamespacesSuite)
Test_ListSubjectConditionSets_Limit_TooLarge_Fails() to func (s
*SubjectMappingsSuite) Test_ListSubjectConditionSets_Limit_TooLarge_Fails()),
ensuring SubjectMappingsSuite is the intended suite type in the same file or
package.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@service/policy/db/migrations/20260318000000_add_namespace_to_subject_mappings.md`:
- Around line 24-31: Update the inaccurate bullet about unscoped records: change
the statement that "Existing records with `namespace_id = NULL` are unscoped and
returned in all list queries regardless of namespace filter" to state that
records with `namespace_id = NULL` are returned only when no namespace filter is
provided; when a specific namespace filter is supplied the NULL records are
excluded (since `NULL = <value>` evaluates to false in SQL), and keep the rest
of the documented behaviors (optional namespace at creation, list queries
accepting an optional namespace filter, and cascade delete) unchanged.

In `@service/policy/db/queries/actions.sql`:
- Around line 147-180: The query can silently insert actions with namespace_id =
NULL when resolved_namespace is not found; update the CTE flow so operations
only proceed if a namespace was resolved: add a namespace_id_check CTE that
selects id FROM resolved_namespace WHERE id IS NOT NULL and then replace
references to (SELECT id FROM resolved_namespace) in existing_actions and in the
INSERT in new_actions with (SELECT id FROM namespace_id_check), and also add a
WHERE EXISTS (SELECT 1 FROM namespace_id_check) guard before the INSERT SELECT
so no rows are created when namespace is missing (alternatively make the
function raise an error when namespace_id_check is empty if callers expect
failure); this touches the resolved_namespace, namespace_id_check (new),
existing_actions, new_actions and the createOrListActionsByNameInNamespace
usage.

In `@service/policy/db/queries/subject_mappings.sql`:
- Line 1: Replace the bare divider line
'----------------------------------------------------------------' at the top of
subject_mappings.sql with a proper SQL comment by prefixing it with '--' (e.g.,
change it to a commented header like '--
-------------------------------------------------------------' or include a
descriptive comment such as '-- SUBJECT CONDITION SETS' followed by the dashed
comment); this removes the syntax error by ensuring the divider is treated as a
comment.
- Around line 72-74: The delimiter lines surrounding the SUBJECT MAPPINGS header
are missing SQL comment prefixes; update the two delimiter lines that flank the
"-- SUBJECT MAPPINGS" header in subject_mappings.sql so each starts with "--"
(i.e., change the bare
"----------------------------------------------------------------" lines to
commented delimiter lines) to ensure they are valid SQL comments and don't break
parsing.

---

Outside diff comments:
In `@service/integration/subject_mappings_test.go`:
- Around line 629-638: The test method
Test_ListSubjectMappings_Limit_TooLarge_Fails is declared with the wrong suite
receiver (NamespacesSuite) so it will run in the wrong context; change its
receiver to SubjectMappingsSuite by updating the method signature to use (s
*SubjectMappingsSuite) so the test executes under the correct suite and has
access to SubjectMappingsSuite fields and helpers (keep the body unchanged
except for the receiver).
- Around line 1111-1120: The test method
Test_ListSubjectConditionSets_Limit_TooLarge_Fails is declared with the wrong
receiver (NamespacesSuite); change its receiver to SubjectMappingsSuite so the
test runs in the correct suite context (i.e., update the method signature from
func (s *NamespacesSuite) Test_ListSubjectConditionSets_Limit_TooLarge_Fails()
to func (s *SubjectMappingsSuite)
Test_ListSubjectConditionSets_Limit_TooLarge_Fails()), ensuring
SubjectMappingsSuite is the intended suite type in the same file or package.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1143ca26-68be-46de-884e-f51d04bb76ad

📥 Commits

Reviewing files that changed from the base of the PR and between f4486dd and 044537a.

📒 Files selected for processing (13)
  • service/integration/subject_mappings_test.go
  • service/pkg/db/errors.go
  • service/policy/db/actions.sql.go
  • service/policy/db/migrations/20260318000000_add_namespace_to_subject_mappings.md
  • service/policy/db/migrations/20260318000000_add_namespace_to_subject_mappings.sql
  • service/policy/db/models.go
  • service/policy/db/queries/actions.sql
  • service/policy/db/queries/subject_mappings.sql
  • service/policy/db/schema_erd.md
  • service/policy/db/subject_mappings.go
  • service/policy/db/subject_mappings.sql.go
  • service/policy/db/utils.go
  • service/policy/subjectmapping/subject_mapping.go

Comment thread service/policy/db/queries/actions.sql
Comment thread service/policy/db/queries/subject_mappings.sql
Comment thread service/policy/db/queries/subject_mappings.sql
@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 198.549568ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 100.263572ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 388.302087ms
Throughput 257.53 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 41.866509831s
Average Latency 416.64293ms
Throughput 119.43 requests/second

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@service/policy/db/migrations/20260318000000_add_namespace_to_subject_mappings.md`:
- Around line 26-27: Remove the dangling contradictory sentence fragment on Line
27 in the migration doc so the behavior is clear: keep the sentence that states
"Existing records with `namespace_id = NULL` are unscoped and returned in all
list queries where no namespace filter is given." (the corrected semantics) and
delete the extra orphan fragment that follows it; ensure the final file reflects
that NULL `namespace_id` is intentional for legacy/global records until
`NamespacedPolicy` is enabled.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 31e024d1-832f-4500-bede-a7ec0ab1c541

📥 Commits

Reviewing files that changed from the base of the PR and between 044537a and 244feba.

📒 Files selected for processing (1)
  • service/policy/db/migrations/20260318000000_add_namespace_to_subject_mappings.md

Comment thread service/policy/db/migrations/20260318000000_add_namespace_to_subject_mappings.md Outdated
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 159.077482ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 83.397829ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 396.214297ms
Throughput 252.39 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 42.294443455s
Average Latency 421.353091ms
Throughput 118.22 requests/second

@github-actions

Copy link
Copy Markdown
Contributor

elizabethhealy
elizabethhealy previously approved these changes Mar 19, 2026
Comment thread service/policy/db/queries/subject_mappings.sql Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
service/policy/db/queries/subject_mappings.sql (2)

1-3: ⚠️ Potential issue | 🔴 Critical

Missing SQL comment prefix causes syntax error.

Line 1 starts with ---------------------------------------------------------------- which should be a comment, but it's missing the -- prefix. This will cause a SQL syntax error when the file is parsed by sqlc or executed directly.

🐛 Proposed fix
-----------------------------------------------------------------
+-- ------------------------------------------------------------
 -- SUBJECT CONDITION SETS
-----------------------------------------------------------------
+-- ------------------------------------------------------------
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@service/policy/db/queries/subject_mappings.sql` around lines 1 - 3, The
leading separator lines in subject_mappings.sql are not commented and will cause
SQL parse errors; update each separator line like the ones starting with
"----------------------------------------------------------------" (and similar
header/footer separator lines) to include the SQL comment prefix (--) so they
become proper comments; ensure every human-readable header line (e.g., "SUBJECT
CONDITION SETS") is also prefixed with -- to avoid syntax errors when sqlc or
the SQL engine parses the file.

64-66: ⚠️ Potential issue | 🔴 Critical

Same issue: missing comment prefix.

Line 64 has the same problem with the delimiter missing the -- prefix.

🐛 Proposed fix
-----------------------------------------------------------------
+-- ------------------------------------------------------------
 -- SUBJECT MAPPINGS
-----------------------------------------------------------------
+-- ------------------------------------------------------------
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@service/policy/db/queries/subject_mappings.sql` around lines 64 - 66, The
file contains a header delimiter line missing the SQL comment prefix; update the
delimiter before the "SUBJECT MAPPINGS" header so it starts with "--" (i.e.,
ensure the separator line uses the "--" prefix like the surrounding comment
blocks) to make it a valid SQL comment; locate the delimiter near the "SUBJECT
MAPPINGS" header in subject_mappings.sql and add the missing "--" at the start
of that line.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@service/policy/db/queries/subject_mappings.sql`:
- Around line 1-3: The leading separator lines in subject_mappings.sql are not
commented and will cause SQL parse errors; update each separator line like the
ones starting with
"----------------------------------------------------------------" (and similar
header/footer separator lines) to include the SQL comment prefix (--) so they
become proper comments; ensure every human-readable header line (e.g., "SUBJECT
CONDITION SETS") is also prefixed with -- to avoid syntax errors when sqlc or
the SQL engine parses the file.
- Around line 64-66: The file contains a header delimiter line missing the SQL
comment prefix; update the delimiter before the "SUBJECT MAPPINGS" header so it
starts with "--" (i.e., ensure the separator line uses the "--" prefix like the
surrounding comment blocks) to make it a valid SQL comment; locate the delimiter
near the "SUBJECT MAPPINGS" header in subject_mappings.sql and add the missing
"--" at the start of that line.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ff3845e8-c936-416b-b077-24ad5c26357d

📥 Commits

Reviewing files that changed from the base of the PR and between 2e594f2 and 5b187c9.

📒 Files selected for processing (6)
  • service/integration/subject_mappings_test.go
  • service/policy/db/actions.sql.go
  • service/policy/db/queries/actions.sql
  • service/policy/db/queries/subject_mappings.sql
  • service/policy/db/subject_mappings.go
  • service/policy/db/subject_mappings.sql.go

@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 188.053314ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 97.294932ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 418.820134ms
Throughput 238.77 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 40.35427051s
Average Latency 402.241223ms
Throughput 123.90 requests/second

@github-actions

Copy link
Copy Markdown
Contributor

Comment thread service/policy/subjectmapping/subject_mapping.go

@jakedoublev jakedoublev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good

@c-r33d
c-r33d added this pull request to the merge queue Mar 25, 2026
Merged via the queue into main with commit 6deed50 Mar 25, 2026
38 checks passed
@c-r33d
c-r33d deleted the scs-sm-service-changes branch March 25, 2026 14:15
JBCongdon pushed a commit to JBCongdon/platform that referenced this pull request May 24, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.14.0](opentdf/platform@service/v0.13.0...service/v0.14.0)
(2026-04-21)


### ⚠ BREAKING CHANGES

* **sdk:** reclassify KAS 400 errors — distinguish tamper from
misconfiguration
([opentdf#3166](opentdf#3166))
* **policy:** optional namespace for RRs
([opentdf#3165](opentdf#3165))
* **policy:** Namespace subject mappings and subject condition sets.
([opentdf#3143](opentdf#3143))
* **policy:** Optional namespace on actions protos, NamespacedPolicy
feature flag ([opentdf#3155](opentdf#3155))
* **policy:** add namespaced actions schema and namespace-aware action
queries ([opentdf#3154](opentdf#3154))
* **policy:** only require namespace on GetAction if no id provided
([opentdf#3144](opentdf#3144))
* **policy:** add namespace field to Actions proto
([opentdf#3130](opentdf#3130))
* **policy:** namespace Registered Resources
([opentdf#3111](opentdf#3111))
* **policy:** add namespace field to RegisteredResource proto
([opentdf#3110](opentdf#3110))

### Features

* **authz:** Namespaced policy in decisioning
([opentdf#3226](opentdf#3226))
([0355934](opentdf@0355934))
* **cli:** migrate otdfctl into platform monorepo
([opentdf#3205](opentdf#3205))
([5177bec](opentdf@5177bec))
* fix tracing ([opentdf#3242](opentdf#3242))
([57e5680](opentdf@57e5680))
* **policy:** add GetObligationTrigger RPC
([opentdf#3318](opentdf#3318))
([d68e39d](opentdf@d68e39d))
* **policy:** add namespace field to Actions proto
([opentdf#3130](opentdf#3130))
([bedc9b3](opentdf@bedc9b3))
* **policy:** add namespace field to RegisteredResource proto
([opentdf#3110](opentdf#3110))
([04fd85d](opentdf@04fd85d))
* **policy:** add namespaced actions schema and namespace-aware action
queries ([opentdf#3154](opentdf#3154))
([c0443f1](opentdf@c0443f1))
* **policy:** add sort ListSubjectMappings API
([opentdf#3255](opentdf#3255))
([9d5d757](opentdf@9d5d757))
* **policy:** Add sort support listregisteredresources api
([opentdf#3312](opentdf#3312))
([91a3ff3](opentdf@91a3ff3))
* **policy:** add sort support to ListAttributes API
([opentdf#3223](opentdf#3223))
([ec3312f](opentdf@ec3312f))
* **policy:** add sort support to ListKeyAccessServer
([opentdf#3287](opentdf#3287))
([7fae2d7](opentdf@7fae2d7))
* **policy:** Add sort support to ListNamespaces API
([opentdf#3192](opentdf#3192))
([aac86cd](opentdf@aac86cd))
* **policy:** add sort support to listobligations api
([opentdf#3300](opentdf#3300))
([9221cac](opentdf@9221cac))
* **policy:** add sort support to ListSubjectConditionSets API
([opentdf#3272](opentdf#3272))
([9010f12](opentdf@9010f12))
* **policy:** add SortField proto and update PageRequest for sort
support ([opentdf#3187](opentdf#3187))
([6cf1862](opentdf@6cf1862))
* **policy:** Enforce same namespace when actions referenced downstream
([opentdf#3206](opentdf#3206))
([4b5463a](opentdf@4b5463a))
* **policy:** namespace Registered Resources
([opentdf#3111](opentdf#3111))
([6db1883](opentdf@6db1883))
* **policy:** Namespace subject mappings and condition sets
([opentdf#3172](opentdf#3172))
([6deed50](opentdf@6deed50))
* **policy:** Namespace subject mappings and subject condition sets.
([opentdf#3143](opentdf#3143))
([3006780](opentdf@3006780))
* **policy:** optional namespace for RRs
([opentdf#3165](opentdf#3165))
([8948018](opentdf@8948018))
* **policy:** rollback migration strategy for namespaced actions
([opentdf#3235](opentdf#3235))
([f7e5e01](opentdf@f7e5e01))
* **policy:** Seed existing namespaces with standard actions
([opentdf#3228](opentdf#3228))
([12136b0](opentdf@12136b0))
* **policy:** Seed namespaces with standard actions on creation +
namespaced actions for obligation triggers
([opentdf#3161](opentdf#3161))
([984d76b](opentdf@984d76b))


### Bug Fixes

* **ci:** Upgrade toolchain version to 1.25.8
([opentdf#3116](opentdf#3116))
([e1b7882](opentdf@e1b7882))
* **core:** do not concat slashes directly in url/file paths
([opentdf#3290](opentdf#3290))
([114c2a7](opentdf@114c2a7))
* **deps:** bump github.com/jackc/pgx/v5 from 5.7.5 to 5.9.0 in /service
([opentdf#3316](opentdf#3316))
([017362e](opentdf@017362e))
* **deps:** bump github.com/opentdf/platform/lib/identifier from 0.2.0
to 0.3.0 in /service
([opentdf#3162](opentdf#3162))
([8bc5dcd](opentdf@8bc5dcd))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.16.0 to
0.17.0 in /service
([opentdf#3125](opentdf#3125))
([29fec61](opentdf@29fec61))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.17.0 to
0.21.0 in /service
([opentdf#3220](opentdf#3220))
([e63add2](opentdf@e63add2))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.21.0 to
0.22.0 in /service
([opentdf#3248](opentdf#3248))
([1ebce73](opentdf@1ebce73))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.22.0 to
0.23.0 in /service
([opentdf#3271](opentdf#3271))
([3338b8e](opentdf@3338b8e))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.23.0 to
0.24.0 in /service
([opentdf#3321](opentdf#3321))
([78e6022](opentdf@78e6022))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.24.0 to
0.25.0 in /service
([opentdf#3333](opentdf#3333))
([3940bf8](opentdf@3940bf8))
* **deps:** bump github.com/opentdf/platform/sdk from 0.13.0 to 0.16.0
in /service ([opentdf#3356](opentdf#3356))
([5617077](opentdf@5617077))
* **deps:** bump
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp from
1.42.0 to 1.43.0 in /service
([opentdf#3282](opentdf#3282))
([046374a](opentdf@046374a))
* **deps:** bump go.opentelemetry.io/otel/sdk from 1.42.0 to 1.43.0 in
/service ([opentdf#3281](opentdf#3281))
([56b33f2](opentdf@56b33f2))
* **deps:** bump google.golang.org/grpc from 1.77.0 to 1.79.3 in
/service ([opentdf#3176](opentdf#3176))
([3289502](opentdf@3289502))
* **deps:** remove direct github.com/docker/docker dependency
([opentdf#3229](opentdf#3229))
([2becb27](opentdf@2becb27))
* **deps:** upgrade testcontainers-go to resolve vulns
([opentdf#3299](opentdf#3299))
([72c6f9b](opentdf@72c6f9b))
* **ers:** include standard JWT claims in claims mode entity resolution
([opentdf#3196](opentdf#3196))
([6d50da1](opentdf@6d50da1))
* **ers:** ldap multi-strategy ers
([opentdf#3117](opentdf#3117))
([d3aaf1a](opentdf@d3aaf1a))
* **policy:** deprecate ListAttributeValues in favor of existing
GetAttribute ([opentdf#3108](opentdf#3108))
([7e17c2d](opentdf@7e17c2d))
* **policy:** make obligation trigger uniqueness client-aware
([opentdf#3114](opentdf#3114))
([9265bc3](opentdf@9265bc3))
* **policy:** omit empty attribute values from create responses
([opentdf#3193](opentdf#3193))
([d298378](opentdf@d298378))
* **policy:** only require namespace on GetAction if no id provided
([opentdf#3144](opentdf#3144))
([10d0c0f](opentdf@10d0c0f))
* **policy:** Optional namespace on actions protos, NamespacedPolicy
feature flag ([opentdf#3155](opentdf#3155))
([c20f039](opentdf@c20f039))
* **policy:** order List* results by created_at
([opentdf#3088](opentdf#3088))
([ea90ac2](opentdf@ea90ac2))
* **sdk:** normalize issuer URL before OIDC discovery
([opentdf#3261](opentdf#3261))
([61f98c9](opentdf@61f98c9))
* **sdk:** reclassify KAS 400 errors — distinguish tamper from
misconfiguration
([opentdf#3166](opentdf#3166))
([f04a385](opentdf@f04a385))
* **sdk:** remove testcontainers from consumer dependency graph
([opentdf#3129](opentdf#3129))
([f17dcdd](opentdf@f17dcdd))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:db DB component comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry) docs Documentation size/xl

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants