feat(policy)!: DSPX-2998 add namespace fields to resource mapping protos#3565
feat(policy)!: DSPX-2998 add namespace fields to resource mapping protos#3565alkalescent wants to merge 3 commits into
Conversation
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>
|
Warning Review limit reached
More reviews will be available in 7 minutes and 27 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR extends resource mapping functionality with namespace ownership and filtering capabilities. The core proto message gains a 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 unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
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
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
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.
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
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
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
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