feat(policy): add narrow attribute read API protos and generated code#3634
Conversation
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 two purpose-built read RPCs to the attributes service, replacing the broader 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. Two new paths for data to flow, Narrow reads to make it go. Keys and rules in tidy sets, Optimized for what it gets. Footnotes
|
github-actions
Bot
added
comp:db
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two new narrow read APIs to the AttributesService: GetKeyMappingsByFqns (for client-side key split construction) and GetEntitleableAttributesByFqns (for server-side entitlement resolution). These APIs allow fetching targeted policy data without loading the entire policy. The changes include new Protobuf definitions, database query implementations, and comprehensive tests. The review feedback suggests optimizing the SQL query getSubjectMappingsByValueFqns by casting standard_actions and custom_actions to ::jsonb. This enables sqlc to generate these fields as []byte instead of interface{}, allowing the Go code to pass the raw bytes directly to unmarshalAllActionsProto and completely avoid the overhead of json.Marshal.
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.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds two new narrow read RPCs on ChangesFQN-based Narrow Read RPCs
Sequence Diagram(s)sequenceDiagram
participant Client
participant AttributesService
participant PolicyDBClient
participant getAttributesByValueFqns
participant getSubjectMappingsByValueFqns
participant resolveEffectiveKasKeys
Client->>AttributesService: GetKeyMappingsByFqns / GetEntitleableAttributesByFqns
AttributesService->>PolicyDBClient: DB lookup by FQNs
PolicyDBClient->>getAttributesByValueFqns: resolve attribute/value pairs
getAttributesByValueFqns-->>PolicyDBClient: resolved pairs
alt GetKeyMappingsByFqns
PolicyDBClient->>resolveEffectiveKasKeys: choose effective KAS keys
resolveEffectiveKasKeys-->>PolicyDBClient: keys per value FQN
else GetEntitleableAttributesByFqns
PolicyDBClient->>getSubjectMappingsByValueFqns: fetch value-level subject mappings
getSubjectMappingsByValueFqns-->>PolicyDBClient: rows per value FQN
end
PolicyDBClient-->>AttributesService: response data
AttributesService-->>Client: RPC response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
@dmihalcik-virtru Would you mind taking a look as the author of the |
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: 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/integration/attributes_test.go`:
- Around line 1672-1688: The test currently validates that lowFqn exists in
resp.GetFqnEntitleableAttributes() with correct properties, but does not assert
that the map contains only the requested value and no other FQNs. Add an
assertion to verify the length of resp.GetFqnEntitleableAttributes() equals 1 or
add explicit checks that highFqn and midFqn are not present in the map, ensuring
the response does not inadvertently over-return entitleable attributes beyond
what was requested.
🪄 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: e9a97052-0f55-4a9a-9338-b3656bc84e4c
⛔ Files ignored due to path filters (1)
protocol/go/policy/attributes/attributes.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (6)
docs/grpc/index.htmldocs/openapi/policy/attributes/attributes.openapi.yamlservice/integration/attributes_test.goservice/policy/attributes/attributes.goservice/policy/attributes/attributes.protoservice/policy/db/attribute_fqn.go
alkalescent
force-pushed
the
DSPX-2541-attribute-read-apis
branch
from
f16cd0d to
6adaa3e
Compare
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
alkalescent
force-pushed
the
DSPX-2541-attribute-read-apis
branch
from
fe9860f to
10d625d
Compare
alkalescent
changed the title
alkalescent
force-pushed
the
DSPX-2541-attribute-read-apis
branch
from
10d625d to
a40a66d
Compare
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Add GetKeyMappingsByFqns and GetEntitleableAttributesByFqns RPCs to the attributes service proto, with regenerated protocol/go, gRPC/OpenAPI docs, and the generated sdkconnect client wrapper. The connect-wrapper and the AttributesServiceClient test mocks ship here because CI regenerates them from the proto and requires a clean tree. Server methods are Unimplemented stubs; the DB-backed implementation and the SDK granter adoption land in follow-up PRs. Signed-off-by: Krish Suchak <[email protected]>
alkalescent
force-pushed
the
DSPX-2541-attribute-read-apis
branch
from
a40a66d to
3239ed4
Compare
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
5 participants
Proposed Changes
Proto and generated-code portion of the narrow attribute read APIs, split out so the heavy service implementation lands separately and
protocol/goreleases cleanly.Adds two RPCs to
AttributesService:GetKeyMappingsByFqns— per value FQN: the attribute rule and effective KAS keys (value > definition > namespace precedence), for client-side key splits.GetEntitleableAttributesByFqns— per value FQN: rule, value id, the definition's ordered value FQNs (hierarchy), and value-level subject mappings, for server-side decisioning.Includes the
.proto, regeneratedprotocol/go, gRPC/OpenAPI docs, and the generatedsdkconnectclient wrapper. The wrapper and theAttributesServiceClienttest mocks ship here because CI regenerates them from the proto (make connect-wrapper-generate) and requires a clean tree. Server methods areUnimplementedstubs.PR stack
Replaces the previous combined PR per review (separate proto/generated from service logic). Merge order:
Follow-up feature branch: switch the SDK granter to consume
GetKeyMappingsByFqnsand add an xtest proving key-split parity with the previous SDK.