feat(policy): DSPX-2541 add narrow attribute read APIs#3627
feat(policy): DSPX-2541 add narrow attribute read APIs#3627alkalescent wants to merge 2 commits into
Conversation
Add GetKeyMappingsByFqns (client-side key split path) and GetEntitleableAttributesByFqns (server-side decisioning path) to the attributes service, with regenerated protocol/go and docs. These narrow read APIs return only the fields each caller needs, in FQN-keyed maps, replacing the full attribute+value graph returned by GetAttributeValuesByFqns. Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
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 new narrow-read RPC methods to the attributes service. These changes are part of a broader initiative to optimize performance by moving away from the monolithic 'get everything' API, allowing clients and services to fetch only the specific data required for their respective workflows, such as key split construction or entitlement resolution. 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 flow, Narrow reads to make it go. Less to fetch and less to store, Efficiency at the core. Footnotes
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
github-actions
Bot
added
comp:policy
![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 in attributes.proto: GetKeyMappingsByFqns (for client-side key split construction) and GetEntitleableAttributesByFqns (for server-side entitlement resolution), along with their corresponding request/response messages and updated documentation. The feedback suggests adding item-level validation to the fqns fields in both requests to ensure they are non-empty URIs, and using the fully qualified name policy.AttributeRuleTypeEnum for consistency across package references.
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.
| repeated string fqns = 1 [(buf.validate.field).repeated = { | ||
| min_items: 1 | ||
| max_items: 250 | ||
| }]; |
There was a problem hiding this comment.
The fqns repeated field lacks validation for individual items. Adding items validation ensures that each FQN is a non-empty, valid URI, preventing malformed inputs from propagating further into the service.
repeated string fqns = 1 [(buf.validate.field).repeated = {
min_items: 1
max_items: 250
items: {
string: {
min_len: 1
uri: true
}
}
}];
| // the attribute value FQN this mapping is for | ||
| string fqn = 1; | ||
| // the attribute rule, which governs how splits combine (any_of / all_of / hierarchy) | ||
| AttributeRuleTypeEnum rule = 2; |
There was a problem hiding this comment.
For consistency with other referenced types (like policy.SimpleKasKey and policy.SubjectMapping) and to ensure robust cross-package resolution, use the fully qualified name policy.AttributeRuleTypeEnum.
| AttributeRuleTypeEnum rule = 2; | |
| policy.AttributeRuleTypeEnum rule = 2; |
| repeated string fqns = 1 [(buf.validate.field).repeated = { | ||
| min_items: 1 | ||
| max_items: 250 | ||
| }]; |
There was a problem hiding this comment.
The fqns repeated field lacks validation for individual items. Adding items validation ensures that each FQN is a non-empty, valid URI, preventing malformed inputs from propagating further into the service.
repeated string fqns = 1 [(buf.validate.field).repeated = {
min_items: 1
max_items: 250
items: {
string: {
min_len: 1
uri: true
}
}
}];
| // the parent attribute definition FQN | ||
| string attribute_fqn = 2; | ||
| // the attribute rule, which drives rule logic during decisioning | ||
| AttributeRuleTypeEnum rule = 3; |
There was a problem hiding this comment.
For consistency with other referenced types (like policy.SimpleKasKey and policy.SubjectMapping) and to ensure robust cross-package resolution, use the fully qualified name policy.AttributeRuleTypeEnum.
| AttributeRuleTypeEnum rule = 3; | |
| policy.AttributeRuleTypeEnum rule = 3; |
CI builds the workspace via go.work, so adding GetKeyMappingsByFqns and GetEntitleableAttributesByFqns to the generated handler interface requires AttributesService to satisfy them in this PR. Embed the generated UnimplementedAttributesServiceHandler so the new RPCs return CodeUnimplemented until their handlers land in the stacked follow-up PRs (the embedded defaults are shadowed by the real methods there). Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
|
Superseded by the collapsed per-API PRs #3632 (GetKeyMappingsByFqns) and #3633 (GetEntitleableAttributesByFqns). Restructured so each new RPC ships with its proto, generated code, and handler in one PR, matching repo convention (e.g. 4c3d53d) and removing the need for the Unimplemented-handler embed. |
1 participant
Proposed Changes
GetAttributeValuesByFqns(the current "get everything" API):GetKeyMappingsByFqns— client-side key split path. Returns, per value FQN, the attributeruleand the effectiveSimpleKasKeys (FQN-keyed map). No subject mappings, resource mappings, obligations, sibling values, or metadata.GetEntitleableAttributesByFqns— server-side decisioning path. Returns, per value FQN, therule, value id, ordereddefinition_value_fqns(for hierarchy rule logic), and subject mappings (FQN-keyed map). No KAS keys/grants.protocol/goanddocs/from the protos.This PR is proto + generated
protocol/goonly, soprotocol/gocan be released and consumed by the follow-up service and SDK PRs. Handlers, DB queries, theGetEntitlements/PDP memory optimization, SDK migration, and deprecation ofGetAttributeValuesByFqnsare tracked as later PRs.The
sdk/sdkconnectwrapper regeneration is intentionally excluded; it lands in the SDK PR alongside theprotocol/godependency bump (matching prior proto PRs).Checklist
Testing Instructions
make proto-lint— clean.buf breaking --against origin/main service— additive, no breaking changes.cd protocol/go && go build ./... && go vet ./...— clean.protocol/gorelease (GOWORK=off) passes; the new RPCs are not yet implemented (follow-up PRs).Related