feat(policy)!: disable kas grants in favor of key mappings #2220

strantalis · 2025-05-15T22:21:56Z

Proposed Changes

Checklist

I have added or updated unit tests
I have added or updated integration tests (if appropriate)
I have added or updated documentation

Testing Instructions

github-actions · 2025-05-15T22:22:15Z

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

strantalis · 2025-06-16T15:16:34Z

/gemini-code-assist review

gemini-code-assist · 2025-06-16T15:17:08Z

Thank you for submitting this pull request and for requesting a review. I've reviewed the changes aimed at deprecating KAS grants in favor of key mappings.

First, I'd like to acknowledge the comment from github-actions[bot] regarding the size of this PR. It is indeed a substantial change, reflecting a significant refactoring of the policy service's approach to key associations. While large PRs can sometimes be harder to review, the scope of replacing the KAS grant system with key mappings seems to necessitate a broad set of changes across multiple files, including protobuf definitions, service logic, database interactions, examples, and tests. The changes appear consistent with the stated goal.

Summary of Changes:

The core of this pull request is the deprecation of the older KeyAccessServer based grant system and its replacement with a new system based on Key mappings. This involves:

Marking old RPCs (AssignKeyAccessServerToAttribute, RemoveKeyAccessServerFromAttribute, AssignKeyAccessServerToValue, RemoveKeyAccessServerFromValue, AssignKeyAccessServerToNamespace, RemoveKeyAccessServerFromNamespace) as deprecated in the protobuf definitions and implementing them to return connect.CodeUnimplemented errors.
Introducing new RPCs for key assignments (AssignPublicKeyToAttribute, RemovePublicKeyFromAttribute, AssignPublicKeyToValue, RemovePublicKeyFromValue, AssignPublicKeyToNamespace, RemovePublicKeyFromNamespace).
Updating the database schema and queries to store and retrieve Key associations instead of KeyAccessServer grants.
Refactoring integration tests to use the new key mapping RPCs and verify the updated behavior.
Updating documentation comments in .proto files and the generated docs/grpc/index.html to reflect the deprecation and point to the new key mapping fields (kas_keys).
Removing deprecated commands and variables from the example CLI (examples/cmd/attributes.go).

Feedback:

Correctness: The changes appear logically sound for the intended deprecation and replacement. The integration tests have been updated to reflect the new API and data model, which is crucial for verifying correctness.
Maintainability: Deprecating the old system is a positive step for long-term maintainability, reducing confusion and technical debt. The code changes seem well-contained within the relevant service layers and database interactions.
Testing: It's good to see that the integration tests have been updated to cover the new key mapping functionality and that tests for the deprecated grant functionality have been removed where appropriate. This helps ensure the new system works as expected.
Documentation: The updates to the .proto files and docs/grpc/index.html are a good start for documenting the deprecation. However, given the significant nature of this change, it would be beneficial to review other documentation files (e.g., Markdown files in docs/ and examples/docs/) to ensure they are updated to reflect the new key mapping system and the deprecation of KAS grants.

Breaking Change and Migration:

As indicated by the ! in the commit title (feat(policy)!), this is a breaking change. Users currently relying on the old KAS grant RPCs will need to update their code to use the new key mapping RPCs. It would be helpful to have a clear migration guide or notes for users upgrading to this version.

Overall, this is a significant and necessary change. The approach to deprecation seems appropriate. Please consider reviewing the broader documentation to ensure users have a smooth transition.

Let me know if you have any questions about this feedback.

service/integration/attributes_test.go

service/policy/db/attribute_values.go

service/policy/db/query.sql

test/tdf-roundtrips.bats

service/policy/attributes/attributes.go

service/policy/attributes/attributes.proto

service/policy/namespaces/namespaces.proto

service/policy/db/query.sql

test/policy-service.bats

test/tdf-roundtrips.bats

🤖 I have created a release *beep* *boop* --- ## [0.5.0](protocol/go/v0.4.0...protocol/go/v0.5.0) (2025-06-23) ### ⚠ BREAKING CHANGES * **policy:** disable kas grants in favor of key mappings ([#2220](#2220)) ### Features * **policy:** disable kas grants in favor of key mappings ([#2220](#2220)) ([30f8cf5](30f8cf5)) --- 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>

### Proposed Changes ### Checklist - [ ] I have added or updated unit tests - [ ] I have added or updated integration tests (if appropriate) - [ ] I have added or updated documentation ### Testing Instructions

🤖 I have created a release *beep* *boop* --- ## [0.5.0](protocol/go/v0.4.0...protocol/go/v0.5.0) (2025-06-23) ### ⚠ BREAKING CHANGES * **policy:** disable kas grants in favor of key mappings ([#2220](#2220)) ### Features * **policy:** disable kas grants in favor of key mappings ([#2220](#2220)) ([30f8cf5](30f8cf5)) --- 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.7.0](service/v0.6.0...service/v0.7.0) (2025-06-24) ### ⚠ BREAKING CHANGES * **policy:** disable kas grants in favor of key mappings ([#2220](#2220)) ### Features * **authz:** Add caching to keycloak ERS ([#2466](#2466)) ([f5b0a06](f5b0a06)) * **authz:** auth svc registered resource GetDecision support ([#2392](#2392)) ([5405674](5405674)) * **authz:** authz v2 GetBulkDecision ([#2448](#2448)) ([0da3363](0da3363)) * **authz:** cache entitlement policy within authorization service ([#2457](#2457)) ([c16361c](c16361c)) * **authz:** ensure logging parity between authz v2 and v1 ([#2443](#2443)) ([ef68586](ef68586)) * **core:** add cache manager ([#2449](#2449)) ([2b062c5](2b062c5)) * **core:** consume RPC interceptor request context metadata in logging ([#2442](#2442)) ([2769c48](2769c48)) * **core:** DSPX-609 - add cli-client to keycloak provisioning ([#2396](#2396)) ([48e7489](48e7489)) * **core:** ERS cache setup, fix cache initialization ([#2458](#2458)) ([d0c6938](d0c6938)) * inject logger and cache manager to key managers ([#2461](#2461)) ([9292162](9292162)) * **kas:** expose provider config from key details. ([#2459](#2459)) ([0e7d39a](0e7d39a)) * **main:** Add Close() method to cache manager ([#2465](#2465)) ([32630d6](32630d6)) * **policy:** disable kas grants in favor of key mappings ([#2220](#2220)) ([30f8cf5](30f8cf5)) * **policy:** Restrict deletion of pc with used key. ([#2414](#2414)) ([3b40a46](3b40a46)) * **sdk:** allow Connect-Protocol-Version RPC header for cors ([#2437](#2437)) ([4bf241e](4bf241e)) ### Bug Fixes * **core:** remove generics on new platform cache manager and client ([#2456](#2456)) ([98c3c16](98c3c16)) * **core:** replace opentdf-public client with cli-client ([#2422](#2422)) ([fb18525](fb18525)) * **deps:** bump github.com/casbin/casbin/v2 from 2.106.0 to 2.107.0 in /service in the external group ([#2416](#2416)) ([43afd48](43afd48)) * **deps:** bump github.com/opentdf/platform/protocol/go from 0.4.0 to 0.5.0 in /service ([#2470](#2470)) ([3a73fc9](3a73fc9)) * **deps:** bump github.com/opentdf/platform/sdk from 0.4.7 to 0.5.0 in /service ([#2473](#2473)) ([ad37476](ad37476)) * **deps:** bump the external group across 1 directory with 2 updates ([#2450](#2450)) ([9d8d1f1](9d8d1f1)) * **deps:** bump the external group across 1 directory with 2 updates ([#2472](#2472)) ([d45b3c8](d45b3c8)) * only request a token when near expiration ([#2370](#2370)) ([556d95e](556d95e)) * **policy:** fix casing bug and get provider config on update. ([#2403](#2403)) ([a52b8f9](a52b8f9)) * **policy:** properly formatted pem in test fixtures ([#2409](#2409)) ([54ffd23](54ffd23)) --- 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.7.0](protocol/go/v0.6.2...protocol/go/v0.7.0) (2025-08-08) ### ⚠ BREAKING CHANGES * **policy:** disable kas grants in favor of key mappings ([#2220](#2220)) * **core:** Require go 1.23+ ([#1979](#1979)) ### Features * add ability to retrieve policy resources by id or name ([#1901](#1901)) ([deb4455](deb4455)) * **authz:** authz v2, ers v2 protos and gencode for ABAC with actions & registered resource ([#2124](#2124)) ([ea7992a](ea7992a)) * **authz:** improve v2 request proto validation ([#2357](#2357)) ([f927b99](f927b99)) * **authz:** sensible request limit upper bounds ([#2526](#2526)) ([b3093cc](b3093cc)) * **core:** adds bulk rewrap to sdk and service ([#1835](#1835)) ([11698ae](11698ae)) * **core:** EXPERIMENTAL: EC-wrapped key support ([#1902](#1902)) ([652266f](652266f)) * **core:** Require go 1.23+ ([#1979](#1979)) ([164c922](164c922)) * **core:** v2 ERS with proto updates ([#2210](#2210)) ([a161ef8](a161ef8)) * **policy:** add enhanced standard/custom actions protos ([#2020](#2020)) ([bbac53f](bbac53f)) * **policy:** Add legacy keys. ([#2613](#2613)) ([57370b0](57370b0)) * **policy:** Add list key mappings rpc. ([#2533](#2533)) ([fbc2724](fbc2724)) * **policy:** add obligation protos ([#2579](#2579)) ([50882e1](50882e1)) * **policy:** Add validation to delete keys ([#2576](#2576)) ([cc169d9](cc169d9)) * **policy:** add values to CreateObligationRequest ([#2614](#2614)) ([94535cc](94535cc)) * **policy:** adds new public keys table ([#1836](#1836)) ([cad5048](cad5048)) * **policy:** Allow the deletion of a key. ([#2575](#2575)) ([82b96f0](82b96f0)) * **policy:** cache SubjectConditionSet selectors in dedicated column maintained via trigger ([#2320](#2320)) ([215791f](215791f)) * **policy:** Change return type for delete key proto. ([#2566](#2566)) ([c1ae924](c1ae924)) * **policy:** Default Platform Keys ([#2254](#2254)) ([d7447fe](d7447fe)) * **policy:** disable kas grants in favor of key mappings ([#2220](#2220)) ([30f8cf5](30f8cf5)) * **policy:** DSPX-1018 NDR retrieval by FQN support ([#2131](#2131)) ([0001041](0001041)) * **policy:** DSPX-1057 registered resource action attribute values (protos only) ([#2217](#2217)) ([6375596](6375596)) * **policy:** DSPX-893 NDR define crud protos ([#2056](#2056)) ([55a5c27](55a5c27)) * **policy:** DSPX-902 NDR service crud protos only (1/2) ([#2092](#2092)) ([24b6cb5](24b6cb5)) * **policy:** Finish resource mapping groups ([#2224](#2224)) ([5ff754e](5ff754e)) * **policy:** key management crud ([#2110](#2110)) ([4c3d53d](4c3d53d)) * **policy:** Key management proto ([#2115](#2115)) ([561f853](561f853)) * **policy:** Modify get request to search for keys by kasid with keyid. ([#2147](#2147)) ([780d2e4](780d2e4)) * **policy:** Return KAS Key structure ([#2172](#2172)) ([7f97b99](7f97b99)) * **policy:** Return Simple Kas Keys from non-Key RPCs ([#2387](#2387)) ([5113e0e](5113e0e)) * **policy:** rotate keys rpc ([#2180](#2180)) ([0d00743](0d00743)) * **policy:** Update key status's and UpdateKey rpc. ([#2315](#2315)) ([7908db9](7908db9)) * **policy:** Update simple kas key ([#2378](#2378)) ([09d8239](09d8239)) ### Bug Fixes * add pagination to list public key mappings response ([#1889](#1889)) ([9898fbd](9898fbd)) * **core:** Allow 521 curve to be used ([#2485](#2485)) ([aaf43dc](aaf43dc)) * **core:** Fixes protoJSON parse bug on ec rewrap ([#1943](#1943)) ([9bebfd0](9bebfd0)) * **core:** Update fixtures and flattening in sdk and service ([#1827](#1827)) ([d6d6a7a](d6d6a7a)) * **deps:** bump toolchain in /lib/fixtures and /examples to resolve CVE GO-2025-3563 ([#2061](#2061)) ([9c16843](9c16843)) * **policy:** protovalidate deprecated action types and removal of gRPC gateway in subject mappings svc ([#2377](#2377)) ([54a6de0](54a6de0)) * **policy:** remove gRPC gateway in policy except where needed ([#2382](#2382)) ([1937acb](1937acb)) * **policy:** remove new public keys rpc's ([#1962](#1962)) ([5049bab](5049bab)) * **policy:** remove predefined rules in actions protos ([#2069](#2069)) ([060f059](060f059)) * **policy:** return kas uri on keys for definition, namespace and values ([#2186](#2186)) ([6c55fb8](6c55fb8)) * **sdk:** Fix compatibility between bulk and non-bulk rewrap ([#1914](#1914)) ([74abbb6](74abbb6)) * update key_mode to provide more context ([#2226](#2226)) ([44d0805](44d0805)) --- 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> Co-authored-by: Krish Suchak <42231639+alkalescent@users.noreply.github.com>

github-actions bot added comp:db DB component comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry) labels May 15, 2025

strantalis force-pushed the dspx-501/disable-kas-grants branch from 677a289 to 3ba7aea Compare May 15, 2025 22:23

strantalis force-pushed the dspx-501/disable-kas-grants branch 3 times, most recently from 420b253 to b26498b Compare June 12, 2025 18:28

strantalis closed this Jun 13, 2025

strantalis reopened this Jun 13, 2025

github-actions bot added comp:ci Github Actions Work docs Documentation comp:examples size/xl labels Jun 13, 2025

strantalis force-pushed the dspx-501/disable-kas-grants branch from 7390d03 to 40ea435 Compare June 16, 2025 14:39

strantalis changed the title ~~feat(policy): disable kas grants in favor of key mappings~~ feat(policy)!: disable kas grants in favor of key mappings Jun 16, 2025

strantalis marked this pull request as ready for review June 16, 2025 15:16

strantalis requested review from a team as code owners June 16, 2025 15:16

c-r33d reviewed Jun 16, 2025

View reviewed changes

service/integration/attributes_test.go Show resolved Hide resolved

c-r33d reviewed Jun 16, 2025

View reviewed changes

service/policy/db/attribute_values.go Show resolved Hide resolved

c-r33d reviewed Jun 16, 2025

View reviewed changes

service/policy/db/query.sql Show resolved Hide resolved

strantalis added 5 commits June 17, 2025 12:56

fix(policy): disable kas grants in favor of key mappings

5f66102

save changes

8032c8c

save

6188e99

rebase on main

2adf65a

mod tidy

4d570e7

resolve conflict

3b22403

strantalis force-pushed the dspx-501/disable-kas-grants branch from 40ea435 to 3b22403 Compare June 17, 2025 16:57

strantalis enabled auto-merge June 17, 2025 18:23

c-r33d previously approved these changes Jun 17, 2025

View reviewed changes

test/tdf-roundtrips.bats Show resolved Hide resolved

elizabethhealy reviewed Jun 18, 2025

View reviewed changes

add more deprecation warnings

71d915d

strantalis dismissed c-r33d’s stale review via 71d915d June 18, 2025 19:56

update assign grant rpc language

8107e9d

elizabethhealy previously approved these changes Jun 18, 2025

View reviewed changes

strantalis requested review from c-r33d and jrschumacher June 19, 2025 00:42

jrschumacher previously approved these changes Jun 19, 2025

View reviewed changes

fix lint issues

e3bd0ec

strantalis dismissed stale reviews from jrschumacher and elizabethhealy via e3bd0ec June 20, 2025 15:50

strantalis requested a review from jrschumacher June 20, 2025 15:56

jrschumacher approved these changes Jun 20, 2025

View reviewed changes

c-r33d approved these changes Jun 23, 2025

View reviewed changes

strantalis added this pull request to the merge queue Jun 23, 2025

Merged via the queue into opentdf:main with commit 30f8cf5 Jun 23, 2025
29 checks passed

strantalis deleted the dspx-501/disable-kas-grants branch June 23, 2025 14:39

This was referenced Jun 23, 2025

chore(main): release service 0.7.0 #2406

Merged

chore(main): release protocol/go 0.5.0 #2464

Merged

opentdf-automation bot mentioned this pull request Jul 25, 2025

chore(main): release protocol/go 0.7.0 #2567

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(policy)!: disable kas grants in favor of key mappings #2220

feat(policy)!: disable kas grants in favor of key mappings #2220

Uh oh!

strantalis commented May 15, 2025

Uh oh!

github-actions bot commented May 15, 2025

Uh oh!

strantalis commented Jun 16, 2025

Uh oh!

gemini-code-assist bot commented Jun 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

feat(policy)!: disable kas grants in favor of key mappings #2220

feat(policy)!: disable kas grants in favor of key mappings #2220

Uh oh!

Conversation

strantalis commented May 15, 2025

Proposed Changes

Checklist

Testing Instructions

Uh oh!

github-actions bot commented May 15, 2025

Uh oh!

strantalis commented Jun 16, 2025

Uh oh!

gemini-code-assist bot commented Jun 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!