feat(policy): Seed namespaces with standard actions on creation + namespaced actions for obligation triggers#3161
Merged
elizabethhealy merged 8 commits intoMar 24, 2026
Conversation
github-actions
Bot
added
comp:db
Contributor
Summary of ChangesHello, 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 automating the provisioning of essential standard actions for any new namespace. This ensures that every new namespace starts with a foundational set of 'create', 'read', 'update', and 'delete' permissions, streamlining initial setup and maintaining consistency across the system. Highlights
🧠 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. Changelog
Using Gemini Code AssistThe 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
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 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. New namespace, fresh and clean, Actions seeded, a policy scene. Create, read, update, delete, Permissions granted, oh so sweet. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
The pull request successfully implements the feature to seed standard actions ('create', 'read', 'update', 'delete') when a new namespace is created. The changes are well-structured, including the SQL query, the generated Go database function, the integration into the CreateNamespace logic, and a comprehensive integration test. The use of ON CONFLICT DO NOTHING in the SQL query is a good practice for idempotency. The new test case effectively verifies the functionality, ensuring that the standard actions are correctly associated with the newly created namespace. Overall, the code is clean, correct, and maintains high quality.
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
elizabethhealy
changed the title
Member
Author
|
/gemini summary |
Member
Author
|
/gemini review |
Contributor
Summary of ChangesThis pull request streamlines policy management by automating the initial setup of new namespaces and refining how actions are resolved for obligation triggers. It ensures that every new namespace begins with a foundational set of standard actions, reducing manual configuration. Furthermore, it enhances the precision of policy enforcement by prioritizing namespace-scoped actions when defining obligation triggers, making the system more robust and context-aware. Highlights
🧠 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. Changelog
Activity
|
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
elizabethhealy
commented
Mar 16, 2026
Contributor
There was a problem hiding this comment.
Code Review
This pull request correctly implements the seeding of standard actions upon namespace creation and adjusts obligation triggers to prioritize namespace-scoped actions. However, there is a significant atomicity issue in the CreateNamespace function, where the namespace creation and action seeding are not wrapped in a transaction, potentially leading to data inconsistency. I have also provided a suggestion to simplify a new integration test for improved readability.
Contributor
elizabethhealy
marked this pull request as ready for review
March 16, 2026 15:31
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
|
What is the main reason we need the standard actions that cannot be deleted (CRUD) added to a namespace instead of as universal rows? Does it make the queries significantly more compelx? |
Member
Author
@jakedoublev if i understand correctly youre asking why are we putting a copy under eaching namespace instead of just having one copy live without a namespace and adjust the queries to handle that? if so - i just wanted to keep the underlying logic as simple as possible, avoid branching in the code to include un-namespaced standard actions and same for the sql queries. i figured since there are a limited number of standard actions duplicating them across namespaces wouldn't be too much of a burden. so tldr trade some storage overhead for simpler and hopefully safer logic. i think i made this decision when we were assuming policy did not cross namespaces. i think if policy were to be allowed to cross namespaces then the universal rows may make more sense? unless we still want read for one namespace to be distinct from read in another namespace, which i could possibly see being useful lmk your thoughts |
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
📝 WalkthroughWalkthroughNamespace creation now seeds four standard actions into that namespace. Action lookup for obligation triggers was changed to prefer namespace-scoped actions when resolving by name. Integration tests were added to verify seeding and correct namespace preference when listing actions and creating triggers. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant API as Namespace Service
participant DB as Database
User->>API: CreateNamespace(name)
API->>DB: INSERT namespace record
DB-->>API: namespace_id
API->>DB: UPDATE namespace FQN
DB-->>API: OK
API->>DB: seedStandardActionsForNamespace(namespace_id)
DB-->>API: rows affected
API->>DB: GetNamespace(namespace_id)
DB-->>API: Namespace + actions
API-->>User: Namespace with seeded actions
sequenceDiagram
actor User
participant API as Obligation Trigger Service
participant DB as Database
User->>API: CreateObligationTrigger(action_name="read", obligation_id)
API->>DB: SELECT obligation namespace (ov_id)
DB-->>API: namespace_id
API->>DB: Resolve action_id USING CASE:
Note over DB: If action_id provided -> select by id\nIf action_name provided -> prefer a.namespace_id = ov.namespace_id\nElse fallback to a.namespace_id IS NULL
DB-->>API: action_id (namespace-scoped preferred)
API->>DB: INSERT obligation_trigger with action_id
DB-->>API: trigger_id
API-->>User: ObligationTrigger(created)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/integration/namespaces_test.go`:
- Around line 95-114: The test currently checks for CRUD action names across all
returned actions; change the assertion to scope to the newly created namespace
by filtering listed.GetActionsStandard() to only include actions where
action.GetNamespace().GetId() == createdNamespace.GetId(), then assert that the
resulting set of names equals exactly {"create","read","update","delete"} (e.g.,
build a local seen map from the filtered slice and assert all four present and
no extras) so the seeded rows are verified for createdNamespace only; update the
loop that iterates over listed.GetActionsStandard() and the final assertions
accordingly.
In `@service/integration/obligation_triggers_test.go`:
- Around line 258-268: The loop that selects namespaceReadID currently picks the
first non-global "read" action but doesn't ensure it belongs to the test
namespace; update the loop that iterates over listed.GetActionsStandard() to
also require act.GetNamespace().GetId() == s.namespace.GetId() (in addition to
act.GetName() == "read" and act.GetId() != globalRead.GetId()), so
namespaceReadID is pinned to the obligation's namespace for the subsequent
assertion in the test.
In `@service/policy/db/queries/actions.sql`:
- Around line 169-176: The listActions predicate is too broad and will return
standard CRUD rows from other namespaces once seedStandardActionsForNamespace
inserts is_standard = TRUE per-namespace; update the listActions WHERE clause to
prefer actions scoped to the requested namespace (e.g., a.namespace_id = rn.id)
and only include global/legacy standard actions when that namespace does not
have an action with the same name—implement this by replacing the current "OR
a.is_standard = TRUE OR a.namespace_id = rn.id" logic with a conditional that
includes a.is_standard = TRUE only when NOT EXISTS (SELECT 1 FROM actions ax
WHERE ax.name = a.name AND ax.namespace_id = rn.id), so namespace-specific
actions win and global fallbacks are used only when missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1828c2bd-86fa-49a9-a657-e67642579970
📒 Files selected for processing (7)
service/integration/namespaces_test.goservice/integration/obligation_triggers_test.goservice/policy/db/actions.sql.goservice/policy/db/namespaces.goservice/policy/db/obligations.sql.goservice/policy/db/queries/actions.sqlservice/policy/db/queries/obligations.sql
Contributor
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
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/integration/actions_test.go`:
- Around line 203-232: The test
Test_ListActions_WithNamespace_DoesNotLeakStandardActionsFromOtherNamespaces
uses a static namespace name in the CreateNamespace call which can cause
unique-constraint flakiness; change the Name passed to
s.db.PolicyClient.CreateNamespace (the CreateNamespace call that sets
createdNamespace) to include a unique suffix (for example append
time.Now().UnixNano() or use the existing test helper that generates unique
names) so the namespace name is unique across test runs while keeping the rest
of the test logic (list := s.db.PolicyClient.ListActions, iterating
list.GetActionsStandard(), and the expected map checks) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2d1e1146-fdbc-42a6-bb44-a99eba9e998c
📒 Files selected for processing (5)
service/integration/actions_test.goservice/integration/namespaces_test.goservice/integration/obligation_triggers_test.goservice/policy/db/actions.sql.goservice/policy/db/queries/actions.sql
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
alkalescent
reviewed
Mar 24, 2026
alkalescent
approved these changes
Mar 24, 2026
elizabethhealy
deleted the
dspx-2450-seed-namespaces-with-standard-actions
branch
March 24, 2026 19:22
This was referenced Mar 26, 2026
3 tasks
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>
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
3 participants
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.
Proposed Changes
Checklist
Testing Instructions
Summary by CodeRabbit
New Features
Tests