feat(policy): DSPX-2541 add GetEntitleableAttributesByFqns API#3633
feat(policy): DSPX-2541 add GetEntitleableAttributesByFqns API#3633alkalescent wants to merge 3 commits into
Conversation
Add a narrow server-side decisioning read API to the attributes service: proto, regenerated protocol/go, handler, DB method, lean SQL, and tests in one PR. Per requested attribute value FQN it returns the attribute rule, value id, the definition's ordered value FQNs (for hierarchy rule logic), and value-level subject mappings, fetched via a new getSubjectMappingsByValueFqns query in a single roundtrip instead of the full-policy load. Stacked on the GetKeyMappingsByFqns PR. Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
|
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:db
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 new read-only API endpoint, GetEntitleableAttributesByFqns, designed to streamline server-side decisioning. By optimizing the retrieval of subject mappings through a targeted SQL query, the change significantly reduces data transfer overhead compared to previous methods, improving the efficiency of the entitlement resolution path. 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 new API for us to call, / To make the policy stand tall. / With mappings fetched in one quick trip, / We've optimized the entitlement ship. Footnotes
|
![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 a new GetEntitleableAttributesByFqns endpoint to the AttributesService and database layer to fetch only entitlement-relevant information for requested attribute value FQNs, optimizing server-side decisioning. Feedback focuses on improving performance and type safety by explicitly casting COALESCE aggregate expressions to ::jsonb in the SQL query. This allows sqlc to generate these fields as []byte instead of interface{}, eliminating the CPU and memory overhead of double-serialization in the Go hydration logic.
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.
| COALESCE( | ||
| JSONB_AGG(JSONB_BUILD_OBJECT('id', a.id, 'name', a.name, | ||
| 'namespace', CASE WHEN a.namespace_id IS NULL THEN NULL | ||
| ELSE JSONB_BUILD_OBJECT('id', ans.id, 'name', ans.name, 'fqn', ans_fqns.fqn) | ||
| END | ||
| )) FILTER (WHERE a.is_standard = TRUE), | ||
| '[]'::JSONB | ||
| ) AS standard_actions, | ||
| COALESCE( | ||
| JSONB_AGG(JSONB_BUILD_OBJECT('id', a.id, 'name', a.name, | ||
| 'namespace', CASE WHEN a.namespace_id IS NULL THEN NULL | ||
| ELSE JSONB_BUILD_OBJECT('id', ans.id, 'name', ans.name, 'fqn', ans_fqns.fqn) | ||
| END | ||
| )) FILTER (WHERE a.is_standard = FALSE), | ||
| '[]'::JSONB | ||
| ) AS custom_actions |
There was a problem hiding this comment.
By default, sqlc infers the type of COALESCE on aggregate functions inside a CTE as interface{}, which forces pgx to parse the JSONB data into Go maps/slices. Explicitly casting these expressions to ::jsonb ensures sqlc generates them as []byte (or json.RawMessage). This avoids the overhead of parsing and re-marshalling the JSON in Go, allowing you to pass the raw bytes directly to unmarshalAllActionsProto in hydrateSubjectMappingForEntitlement.
COALESCE(
JSONB_AGG(JSONB_BUILD_OBJECT('id', a.id, 'name', a.name,
'namespace', CASE WHEN a.namespace_id IS NULL THEN NULL
ELSE JSONB_BUILD_OBJECT('id', ans.id, 'name', ans.name, 'fqn', ans_fqns.fqn)
END
)) FILTER (WHERE a.is_standard = TRUE),
'[]'::JSONB
)::jsonb AS standard_actions,
COALESCE(
JSONB_AGG(JSONB_BUILD_OBJECT('id', a.id, 'name', a.name,
'namespace', CASE WHEN a.namespace_id IS NULL THEN NULL
ELSE JSONB_BUILD_OBJECT('id', ans.id, 'name', ans.name, 'fqn', ans_fqns.fqn)
END
)) FILTER (WHERE a.is_standard = FALSE),
'[]'::JSONB
)::jsonb AS custom_actions| stdActionsBytes, err := json.Marshal(row.StandardActions) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to marshal standard actions: %w", err) | ||
| } | ||
| customActionsBytes, err := json.Marshal(row.CustomActions) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to marshal custom actions: %w", err) | ||
| } |
There was a problem hiding this comment.
The StandardActions and CustomActions fields are currently marshalled back to JSON bytes using json.Marshal because they are typed as interface{} in the generated sqlc struct. This introduces unnecessary CPU and memory overhead due to double-serialization (parsing JSON in pgx -> marshalling back to JSON in Go -> unmarshalling to Proto).\n\nBy explicitly casting these fields to ::jsonb in the SQL query (see the suggestion in subject_mappings.sql), sqlc will generate them as []byte (or json.RawMessage), allowing you to pass them directly to unmarshalAllActionsProto without the json.Marshal step.
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
…attributes-api-v2
…ableAttributesByFqns Adds the entitleable RPC wrapper method so CI's generated-code freshness check (make connect-wrapper-generate) passes. 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:
|
1 participant
Proposed Changes
GetEntitleableAttributesByFqns, a narrow server-side decisioning read API on the attributes service: proto, regeneratedprotocol/go, handler, DB method, lean SQL, and tests, all in one PR (per repo convention for adding an RPC).rule, the value id, the definition's ordered value FQNs (for hierarchy rule logic), and the value-level subject mappings. Subject mappings are fetched via a newgetSubjectMappingsByValueFqnsquery in a single roundtrip, instead of the fullListAllSubjectMappingsload the entitlement path uses today.Stacked on #3632 (base
DSPX-2541-key-mappings-api). Migrating the v1/v2 decisioning + PDP consumers onto this API and adding namespace-scoped fetch is a separate PR (it depends on the still-open tenancy model under epic DSPX-2184).Checklist
Testing Instructions
go test ./service/integration/ -run TestAttributesSuite/Test_GetEntitleableAttributesByFqnsgolangci-lint run ./policy/db/... ./policy/attributes/...— 0 issues.Related