feat(policy): deprecate GetAttributeValuesByFqns in favor of narrow read APIs - #3745
Conversation
…ead APIs Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
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 a deprecation notice for the GetAttributeValuesByFqns RPC to encourage the adoption of more specific read APIs. The change is non-breaking and serves as a signal for future migration. Additionally, it adjusts linting rules to accommodate generated code that references this deprecated method. 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 old RPC must fade away, / New methods take the light of day. / With deprecation tags in place, / We move at a much faster pace. Footnotes
|
github-actions
Bot
added
comp:policy
📝 WalkthroughWalkthrough
ChangesAttributes RPC deprecation
Dynamic policy documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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 deprecates the GetAttributeValuesByFqns RPC in the attributes service protobuf definition, updating the generated documentation and linter exclusions accordingly. The review feedback suggests formatting the deprecation comment using the standard // Deprecated: ... prefix to ensure that IDEs and code generators correctly parse and display the deprecation notice.
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.
![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 @.golangci.yaml:
- Line 213: Add sdk/sdkconnect/ to the formatters exclusions.paths section,
mirroring its existing entry under linters exclusions.paths and preserving the
generated: lax configuration.
🪄 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: 4df700e4-918b-458a-bf0f-9ee9b91c19b7
⛔ Files ignored due to path filters (2)
protocol/go/policy/attributes/attributes.pb.gois excluded by!**/*.pb.goprotocol/go/policy/attributes/attributes_grpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (5)
.golangci.yamldocs/grpc/index.htmldocs/openapi/policy/attributes/attributes.openapi.yamlprotocol/go/policy/attributes/attributesconnect/attributes.connect.goservice/policy/attributes/attributes.proto
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
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:
|
Add proto doc comments (surfaced in generated SDK/OpenAPI/gRPC docs) for the dynamic value mapping contract: - DynamicValueResolver.operator: IN_CONTAINS is substring-based and over-matches by design; prefer IN unless substring matching is intended. - DynamicValueMapping: multiple mappings on a definition are OR-ed, a mapping's subject sets are AND-ed, and coexistence rules (no value-level subject mappings on the same definition; values under such a definition cannot be added to a registered resource's action attribute values). Comment-only; buf breaking passes. Regenerated protocol/go and policy docs. 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:
|
|
🤖 I have created a release *beep* *boop* --- ## [0.39.0](opentdf/platform@protocol/go/v0.38.0...protocol/go/v0.39.0) (2026-07-21) ### Features * **policy:** Add name and manager lookup ([opentdf#3753](opentdf#3753)) ([bfbc65d](opentdf@bfbc65d)) * **policy:** Add UnsafeUpdateKey rpc. ([opentdf#3728](opentdf#3728)) ([c89b193](opentdf@c89b193)) * **policy:** create subject mappings with attribute values ([opentdf#3741](opentdf#3741)) ([718320b](opentdf@718320b)) * **policy:** deprecate GetAttributeValuesByFqns in favor of narrow read APIs ([opentdf#3745](opentdf#3745)) ([baba709](opentdf@baba709)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [0.21.0](opentdf/platform@service/v0.20.0...service/v0.21.0) (2026-07-22) ### ⚠ BREAKING CHANGES * **policy:** GetPC does not respect manager field. ([opentdf#3752](opentdf#3752)) ### Features * **core:** Update fixtures ([opentdf#3766](opentdf#3766)) ([9ac1640](opentdf@9ac1640)) * **policy:** add entitleable namespace field and narrow-read-API doc corrections ([opentdf#3727](opentdf#3727)) ([0fc2f01](opentdf@0fc2f01)) * **policy:** Add name and manager lookup ([opentdf#3753](opentdf#3753)) ([bfbc65d](opentdf@bfbc65d)) * **policy:** Add UnsafeUpdateKey implementation ([opentdf#3731](opentdf#3731)) ([cd47518](opentdf@cd47518)) * **policy:** Add UnsafeUpdateKey rpc. ([opentdf#3728](opentdf#3728)) ([c89b193](opentdf@c89b193)) * **policy:** create subject mappings with attribute values ([opentdf#3741](opentdf#3741)) ([718320b](opentdf@718320b)) * **policy:** deprecate GetAttributeValuesByFqns in favor of narrow read APIs ([opentdf#3745](opentdf#3745)) ([baba709](opentdf@baba709)) * **policy:** dynamic attribute value entitlement mappings ([opentdf#3568](opentdf#3568)) ([21e95e0](opentdf@21e95e0)) * **policy:** populate entitleable definition namespace ([opentdf#3737](opentdf#3737)) ([3f32839](opentdf@3f32839)) ### Bug Fixes * **authz:** use standard Keycloak token exchange ([opentdf#3754](opentdf#3754)) ([de9ae0d](opentdf@de9ae0d)) * **core:** Pass the platform logger to the authz role provider factory ([opentdf#3732](opentdf#3732)) ([9c32554](opentdf@9c32554)) * **core:** prevent OpenTelemetry resource schema conflicts ([opentdf#3769](opentdf#3769)) ([4a0c41c](opentdf@4a0c41c)) * **deps:** bump github.com/opentdf/platform/protocol/go from 0.38.0 to 0.39.0 in /service ([opentdf#3764](opentdf#3764)) ([8d430f5](opentdf@8d430f5)) * **ers:** coerce attempted_strategies []string to []interface{} for structpb ([opentdf#3645](opentdf#3645)) ([71d6e90](opentdf@71d6e90)) * **policy:** GetPC does not respect manager field. ([opentdf#3752](opentdf#3752)) ([0d3c0a0](opentdf@0d3c0a0)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
This PR updates the platform.branch property in all pom.xml files to the new tag or branch: v0.39.0. See the release: https://github.com/opentdf/platform/releases/tag/protocol%2Fgo%2Fv0.39.0 Release Notes: ## [0.39.0](opentdf/platform@protocol/go/v0.38.0...protocol/go/v0.39.0) (2026-07-21) ### Features * **policy:** Add name and manager lookup ([#3753](opentdf/platform#3753)) ([bfbc65d](opentdf/platform@bfbc65d)) * **policy:** Add UnsafeUpdateKey rpc. ([#3728](opentdf/platform#3728)) ([c89b193](opentdf/platform@c89b193)) * **policy:** create subject mappings with attribute values ([#3741](opentdf/platform#3741)) ([718320b](opentdf/platform@718320b)) * **policy:** deprecate GetAttributeValuesByFqns in favor of narrow read APIs ([#3745](opentdf/platform#3745)) ([baba709](opentdf/platform@baba709)) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Morgan Kleene <mkleene@virtru.com>
2 participants
Summary
Deprecates the
GetAttributeValuesByFqnsRPC (option deprecated = true), steering new use to the narrow read APIs:GetKeyMappingsByFqnsfor client-side key splitsGetEntitleableAttributesByFqnsfor server-side entitlement resolutionNon-breaking: the RPC still functions; this is an annotation. release-please will cut a
protocol/gominor.Also excludes the generated
sdk/sdkconnectconnect wrappers from lint. Those wrappers forward every RPC, including the now-deprecated one, so once the deprecation is visible in the workspace they would tripstaticcheckSA1019; the wrappers are generated, so a path exclusion (like.pb.go) is the appropriate handling.Dynamic Value Mapping Proto Docs
Also adds contract-level doc comments to the DynamicValueMapping protos so the semantics surface in the generated SDK / OpenAPI / gRPC docs (kept out of the service PR #3568 to avoid coupling it to a
protocol/gorelease):DynamicValueResolver.operator:IN_CONTAINSis substring-based and over-matches by design (e.g.adminmatchessuperadmin); preferINunless substring matching is intended.DynamicValueMapping: multiple mappings on a definition are OR-ed; a mapping's subject sets are AND-ed; and coexistence rules (no value-level subject mappings on the same definition; values under such a definition cannot be added to a registered resource's action attribute values).Comment-only, so non-breaking; regenerated
protocol/goand policy docs.Notes
GetAttributeValuesByFqnscalls are affected; the...Response_AttributeAndValueresponse types used elsewhere are not.//nolint:staticcheckat the remaining call sites (sdk/granter.go,sdk/discovery.go,service/authorization,tests-bdd) and bump theirprotocol/gopins. Those must land after this releases, since the nolint directives are only valid once the deprecation exists.Testing
buf lint servicepasses;protocol/gobuilds.// Deprecated:marker forGetAttributeValuesByFqns.Summary by CodeRabbit
Documentation
GetAttributeValuesByFqnsas deprecated across API documentation and client interfaces.GetKeyMappingsByFqnsorGetEntitleableAttributesByFqnsinstead.Chores