feat(policy)!: DSPX-2998 add namespace fields to resource mapping protos - #3565
Merged
alkalescent merged 5 commits intoJun 10, 2026
Merged
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughAdds optional namespace ownership to ResourceMapping, extends resourcemapping requests with ChangesNamespace Ownership for Resource Mappings
🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
comp:policy
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 introduces optional namespace support for Resource Mappings and Resource Mapping Groups. By adding namespace fields to the proto definitions, the system gains the ability to associate resources with specific namespaces, facilitating better organization and filtering. This change is wire-compatible and serves as the foundational proto contract for upcoming service-level implementations. Highlights
New Features🧠 You can now enable Memory (public preview) 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
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 the 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 counterproductive. 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. The namespace fields now take their place, / To bring some order to the space. / With validation strict and tight, / The resource mappings look just right. Footnotes
|
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
![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
This pull request introduces optional namespace fields (namespace_id and namespace_fqn) to several resource mapping messages and requests, along with updating the generated documentation and adding unit tests. The review feedback highlights a validation issue where the regular expressions used for UUID validation on namespace_id lack start (^) and end ($) anchors, which allows partial substring matches. Adding these anchors is recommended to ensure strict UUID validation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Contributor
This was referenced Jun 4, 2026
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
alkalescent
marked this pull request as ready for review
June 5, 2026 17:52
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@service/policy/resourcemapping/resource_mapping_test.go`:
- Around line 170-183: In Test_ListResourceMappingGroupsRequest_NamespaceFilters
add the missing negative-case for NamespaceId by creating a request with an
invalid NamespaceId (e.g., not-a-uuid) while leaving NamespaceFqn valid, call
v.Validate(req) and assert an error contains the UUID validation message
(errMessageOptionalUUID); likewise, update the complementary test in this file
that currently asserts invalid NamespaceId to also assert the invalid
NamespaceFqn path by creating a request with NamespaceFqn set to a non-URI and
asserting v.Validate(req) returns an error containing errMessageOptionalURI —
use the same validator call (v.Validate) and error assertion pattern as the
existing NamespaceFqn negative test to keep consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c4c063d1-0d91-4792-86c0-31c127626604
⛔ Files ignored due to path filters (2)
protocol/go/policy/objects.pb.gois excluded by!**/*.pb.goprotocol/go/policy/resourcemapping/resource_mapping.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (13)
docs/grpc/index.htmldocs/openapi/authorization/authorization.openapi.yamldocs/openapi/policy/actions/actions.openapi.yamldocs/openapi/policy/attributes/attributes.openapi.yamldocs/openapi/policy/objects.openapi.yamldocs/openapi/policy/obligations/obligations.openapi.yamldocs/openapi/policy/registeredresources/registered_resources.openapi.yamldocs/openapi/policy/resourcemapping/resource_mapping.openapi.yamldocs/openapi/policy/subjectmapping/subject_mapping.openapi.yamldocs/openapi/policy/unsafe/unsafe.openapi.yamlservice/policy/objects.protoservice/policy/resourcemapping/resource_mapping.protoservice/policy/resourcemapping/resource_mapping_test.go
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
c-r33d
reviewed
Jun 8, 2026
c-r33d
reviewed
Jun 8, 2026
alkalescent
added a commit
that referenced
this pull request
Jun 8, 2026
Implement the service side of optional namespacing for resource mappings: - Add a migration adding a nullable namespace_id (FK to attribute_namespaces, ON DELETE CASCADE) plus an index to resource_mappings. - Resolve a mapping's owning namespace from namespace_id/namespace_fqn or its group; a grouped mapping must share the group's namespace. - Remove the constraint forcing the mapped attribute value into the group's namespace, allowing mappings to cross namespaces to the values they map. - Hydrate the namespace on get/list responses and add namespace_id/namespace_fqn filters to ListResourceMappings, plus namespace_fqn to ListResourceMappingGroups. - Enforce a namespace on create when namespaced_policy is enabled (namespace_id, namespace_fqn, or group_id satisfies it). - Update/extend integration tests for the new behavior. Stacked on the proto PR (#3565). Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
elizabethhealy
previously approved these changes
Jun 9, 2026
alkalescent
enabled auto-merge
June 9, 2026 16:10
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jun 9, 2026
Contributor
Add an optional owning namespace to resource mappings, mirroring the namespacing already present on registered resources: - ResourceMapping gains a hydrated policy.Namespace field. - Create/UpdateResourceMappingRequest gain optional namespace_id and namespace_fqn. - ListResourceMappingsRequest gains optional namespace_id/namespace_fqn filters; ListResourceMappingGroupsRequest gains namespace_fqn parity. Regenerated protocol/go, OpenAPI/gRPC docs, and SDK wrappers. Adds proto validation unit tests for the new fields. Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
Address review feedback on the resource mapping protos: - Switch namespace_id/namespace_fqn validation to the house buf.validate.message.oneof + string.uuid/string.uri pattern (matching subject_mapping/registered_resources), making the two mutually exclusive. - Add namespace_fqn to CreateResourceMappingGroupRequest (oneof required) and UpdateResourceMappingGroupRequest (oneof optional). - Regenerate protocol/go + docs; update/extend proto validation unit tests. Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
Cover the invalid-UUID path for namespace_id on CreateResourceMappingGroup validation, and invalid namespace_id/namespace_fqn on UpdateResourceMappingGroup, per review feedback. Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
alkalescent
force-pushed
the
DSPX-2998-optional-namespace-resource-mappings
branch
from
June 10, 2026 16:09
a8fa30f to
b8879a8
Compare
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
|
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Contributor
c-r33d
reviewed
Jun 10, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jun 10, 2026
This was referenced Jun 10, 2026
alkalescent
added a commit
that referenced
this pull request
Jun 15, 2026
Implement the service side of optional namespacing for resource mappings: - Add a migration adding a nullable namespace_id (FK to attribute_namespaces, ON DELETE CASCADE) plus an index to resource_mappings. - Resolve a mapping's owning namespace from namespace_id/namespace_fqn or its group; a grouped mapping must share the group's namespace. - Remove the constraint forcing the mapped attribute value into the group's namespace, allowing mappings to cross namespaces to the values they map. - Hydrate the namespace on get/list responses and add namespace_id/namespace_fqn filters to ListResourceMappings, plus namespace_fqn to ListResourceMappingGroups. - Enforce a namespace on create when namespaced_policy is enabled (namespace_id, namespace_fqn, or group_id satisfies it). - Update/extend integration tests for the new behavior. Stacked on the proto PR (#3565). Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
4 tasks
Contributor
|
/backport |
Contributor
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-3565-to-release/service/v0.11
git worktree add --checkout .worktree/backport-3565-to-release/service/v0.11 backport-3565-to-release/service/v0.11
cd .worktree/backport-3565-to-release/service/v0.11
git reset --hard HEAD^
git cherry-pick -x e4a04ed6ee04a346a00a49d3c1f381295f9e9443
git push --force-with-lease |
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
4 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
First PR in a stacked series for DSPX-2998 (Resource Mappings & Resource Mapping Groups should be optionally namespaced). This PR adds the proto contract only; the service implementation and otdfctl/migration support follow in stacked PRs.
policy.Namespace namespacefield to theResourceMappingmessage.namespace_id/namespace_fqntoCreateResourceMappingRequestandUpdateResourceMappingRequest.namespace_id/namespace_fqnfilters toListResourceMappingsRequest, andnamespace_fqnparity toListResourceMappingGroupsRequest(which already hadnamespace_id).protocol/go, OpenAPI/gRPC docs, and SDK connect wrappers.This mirrors the namespacing pattern established for Registered Resources (#3110/#3111/#3165). Adding optional fields is wire-compatible; marked
!per the RR proto-PR convention.Checklist
Testing Instructions
cd service && go test ./policy/resourcemapping/...Related
Summary by CodeRabbit
New Features
Documentation
Tests