feat(policy): Add resource mapping group FQNs#3447
Merged
jakedoublev merged 12 commits intomainfrom May 8, 2026
Merged
Conversation
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 a fully qualified name (FQN) field to resource mapping groups. The change ensures that these groups can be uniquely identified by a URL-based FQN, which is now calculated dynamically in the database layer and exposed through the API. This improves resource traceability and consistency across the platform's policy management services. 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. A group of resources, a name to define, With FQN added, the path is now fine. From database rows to the API call, Consistency reigns, standing proud and tall. Footnotes
|
github-actions
Bot
added
comp:db
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds a computed fully-qualified name (FQN) for ResourceMappingGroup and propagates it through proto, DB queries, generated sqlc code, service transactional writes, API docs, and tests. ChangesResource Mapping Group FQN Addition
🎯 3 (Moderate) | ⏱️ ~22 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Service
participant DBLayer
participant Database
Client->>Service: Create/Update/Get ResourceMappingGroup
Service->>DBLayer: call DB CRUD (inside RunInTx for writes)
DBLayer->>Database: SQL selects/joins compute fqn
Database-->>DBLayer: rows/json with fqn
DBLayer-->>Service: fully populated object (includes fqn)
Service-->>Client: API response (fqn present)
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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
jakedoublev
changed the title
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
jakedoublev
force-pushed
the
DSPX-3195-resource-mapping-group-fqn
branch
from
14c4352 to
c997bb5
Compare
Contributor
X-Test Failure Report |
![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 a fully qualified name (FQN) for resource mapping groups across the platform. Key changes include updating the Protobuf definition, modifying SQL queries to calculate the FQN by joining with the attribute namespaces table, and updating the database client to populate this new field. Furthermore, CRUD operations for resource mapping groups now return the complete object instead of partial data, and integration tests have been updated to verify the FQN generation and retrieval. I have no feedback to provide as there were no review comments.
jakedoublev
force-pushed
the
DSPX-3195-resource-mapping-group-fqn
branch
from
c997bb5 to
9e88341
Compare
Contributor
X-Test Failure Report |
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
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:
|
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@service/policy/db/resource_mapping.go`:
- Around line 109-110: The post-write read in CreateResourceMappingGroup (where
the code calls c.GetResourceMappingGroup(ctx, createdID)) and the similar read
after update is creating ambiguous mutation outcomes if the follow-up SELECT
fails; change the implementation to return the final object directly from the
write (use SQL RETURNING to select all computed/derived fields in the
INSERT/UPDATE) or perform the write+read inside a single transaction and return
the row fetched by RETURNING to callers instead of calling
GetResourceMappingGroup separately, ensuring CreateResourceMappingGroup and
UpdateResourceMappingGroup return the final resource mapping without a separate
follow-up query.
In `@service/policy/resourcemapping/resource_mapping.go`:
- Around line 112-125: The success audit (s.logger.Audit.PolicyCRUDSuccess) is
being emitted inside the RunInTx callback (e.g., around the
CreateResourceMappingGroup call), which can produce false positives if the
transaction commit fails; move the success-audit emission to after RunInTx
returns nil: perform txClient.CreateResourceMappingGroup within the callback and
only set rmGroup and return nil there, then after RunInTx returns check err ==
nil, populate auditParams.ObjectID and auditParams.Original from rmGroup and
call s.logger.Audit.PolicyCRUDSuccess (and set rsp.ResourceMappingGroup) — apply
the same change pattern for the other occurrences (the blocks using RunInTx,
CreateResourceMappingGroup/CreateResourceMapping/UpdateResourceMapping/DeleteResourceMapping
and calling PolicyCRUDSuccess inside the callback).
- Around line 311-313: The UpdateResourceMapping handler currently sets
rsp.ResourceMapping to only include Id (resourceMappingID) even though the fully
updated object updatedRM is available; replace the single-field assignment with
populating rsp.ResourceMapping from updatedRM (or copying/transforming updatedRM
into policy.ResourceMapping) so the response returns the full updated object
instead of only the ID, keeping fields like Name, Rules, Labels, etc.,
consistent with the updatedRM structure used earlier in UpdateResourceMapping.
🪄 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: e4a64c9c-a898-4e0f-84fc-bdf1d8f18ae4
⛔ Files ignored due to path filters (1)
protocol/go/policy/objects.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (16)
docs/grpc/index.htmldocs/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.yamlotdfctl/e2e/resource-mapping-groups.batsservice/integration/resource_mappings_test.goservice/policy/db/queries/resource_mapping.sqlservice/policy/db/resource_mapping.goservice/policy/db/resource_mapping.sql.goservice/policy/objects.protoservice/policy/resourcemapping/resource_mapping.go
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
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
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
May 7, 2026
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
elizabethhealy
approved these changes
May 8, 2026
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
|
Contributor
This was referenced May 8, 2026
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
2 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.
Summary
Summary by CodeRabbit
New Features
Documentation
Tests