fix: update key_mode to provide more context #2226

strantalis · 2025-05-16T20:34:28Z

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

Copilot

Pull Request Overview

This PR refines the KeyMode enum to more descriptive values, updates all validation and storage logic to match the new modes, adjusts integration tests, and regenerates Go protobufs.

Introduces four explicit key modes (CONFIG_ROOT_KEY, PROVIDER_ROOT_KEY, REMOTE, PUBLIC_KEY_ONLY) in place of the old LOCAL/REMOTE.
Updates CEL validations in .proto files to enforce wrapping, provider config, and presence/absence rules for each mode.
Modifies DB client logic and integration tests to use the new CONFIG_ROOT_KEY mode and regenerates Go protobuf code.

Reviewed Changes

Copilot reviewed 9 out of 18 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
service/policy/objects.proto	Expanded and renamed `KeyMode` enum values with detailed comments.
service/policy/kasregistry/key_access_server_registry.proto	Updated CEL validation expressions/messages for new `KeyMode`s and formatting tweaks.
service/policy/db/key_access_server_registry.go	Adjusted the Base64 wrapped-key check to include both new local modes.
service/integration/kas_registry_test.go & kas_registry_key_test.go	Replaced tests’ use of `KEY_MODE_LOCAL` with `KEY_MODE_CONFIG_ROOT_KEY`.
protocol/go/policy/objects.pb.go	Regenerated protobuf code to reflect updated enum definitions.
protocol/go/CHANGELOG.md	Entire file removed (likely need to preserve or update this file).

Files not reviewed (9)

docs/grpc/index.html: Language not supported
docs/openapi/policy/actions/actions.swagger.json: Language not supported
docs/openapi/policy/attributes/attributes.swagger.json: Language not supported
docs/openapi/policy/kasregistry/key_access_server_registry.swagger.json: Language not supported
docs/openapi/policy/namespaces/namespaces.swagger.json: Language not supported
docs/openapi/policy/registeredresources/registered_resources.swagger.json: Language not supported
docs/openapi/policy/resourcemapping/resource_mapping.swagger.json: Language not supported
docs/openapi/policy/subjectmapping/subject_mapping.swagger.json: Language not supported
docs/openapi/policy/unsafe/unsafe.swagger.json: Language not supported

Comments suppressed due to low confidence (2)

protocol/go/CHANGELOG.md:1

The entire CHANGELOG has been removed. Consider restoring or updating it to document these breaking enum changes.

-# Changelog

service/integration/kas_registry_key_test.go:135

Tests were updated for CONFIG_ROOT_KEY, but we should add coverage for the new PROVIDER_ROOT_KEY, REMOTE, and PUBLIC_KEY_ONLY modes to ensure proper validation behavior.

KeyMode:       policy.KeyMode_KEY_MODE_CONFIG_ROOT_KEY,

service/policy/kasregistry/key_access_server_registry.proto

Copilot

Pull Request Overview

This PR refines the KeyMode enum with more specific mode names, updates validation rules to cover the new modes, and aligns code and tests to use the updated constants.

Expanded KeyMode enum in the proto to include CONFIG_ROOT_KEY, PROVIDER_ROOT_KEY, and PUBLIC_KEY_ONLY.
Revised CEL validation expressions and removed generic enum checks in .proto definitions.
Updated Go business logic and integration tests to validate wrapped keys for the new modes.

Reviewed Changes

Copilot reviewed 9 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
service/policy/objects.proto	Expanded `KeyMode` enum with detailed mode comments
service/policy/kasregistry/key_access_server_registry.proto	Updated CEL validation blocks and replaced enum-defined_only fields
service/policy/db/key_access_server_registry.go	Adjusted `CreateKey` logic to validate wrapped key in new modes
service/integration/kas_registry_test.go	Switched tests from LOCAL to CONFIG_ROOT_KEY mode
service/integration/kas_registry_key_test.go	Switched tests from LOCAL to CONFIG_ROOT_KEY mode
protocol/go/policy/objects.pb.go	Regenerated Go types for updated `KeyMode` enum
protocol/go/CHANGELOG.md	Removed existing changelog entries

Files not reviewed (9)

docs/grpc/index.html: Language not supported
docs/openapi/policy/actions/actions.swagger.json: Language not supported
docs/openapi/policy/attributes/attributes.swagger.json: Language not supported
docs/openapi/policy/kasregistry/key_access_server_registry.swagger.json: Language not supported
docs/openapi/policy/namespaces/namespaces.swagger.json: Language not supported
docs/openapi/policy/registeredresources/registered_resources.swagger.json: Language not supported
docs/openapi/policy/resourcemapping/resource_mapping.swagger.json: Language not supported
docs/openapi/policy/subjectmapping/subject_mapping.swagger.json: Language not supported
docs/openapi/policy/unsafe/unsafe.swagger.json: Language not supported

Comments suppressed due to low confidence (2)

protocol/go/CHANGELOG.md:1

[nitpick] The changelog was entirely cleared. It would be better to preserve or update previous entries rather than deleting them wholesale, ensuring a continuous release history.

-# Changelog

service/policy/kasregistry/key_access_server_registry.proto:380

The CEL validation block is missing the message: key before the string literals. You need to prefix the combined message strings with message: to form a valid option.

"The wrapped_key is required if key_mode is KEY_MODE_CONFIG_ROOT_KEY or KEY_MODE_PROVIDER_ROOT_KEY. "

Copilot · 2025-05-19T19:16:58Z

service/policy/kasregistry/key_access_server_registry.proto

  // Required
-  KasPrivateKeyCtx private_key_ctx = 6 [(buf.validate.field).required = true]; // Context or additional data specific to the private key, based on the key provider implementation
-  
+  KasPrivateKeyCtx private_key_ctx = 6; // Context or additional data specific to the private key, based on the key provider implementation


Removed the required = true annotation on private_key_ctx but did not add validation to enforce its presence for CONFIG_ROOT_KEY and PROVIDER_ROOT_KEY modes. Consider adding a CEL rule to require private_key_ctx when those modes are selected.

c-r33d

Looks good. Thanks for taking care of this Sean.

Copilot

Pull Request Overview

This PR updates the key_mode definitions and related validations to provide more detailed context about key management. Key changes include updating the enum values and documentation for key_mode in protocol buffers, revising CEL validation expressions accordingly, and changing key_mode usage in the service, database, and test layers.

Reviewed Changes

Copilot reviewed 9 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
service/policy/objects.proto	Updated key_mode enum comments and constant names.
service/policy/kasregistry/key_access_server_registry.proto	Revised CEL validations and messages for key_mode and related fields.
service/policy/db/key_access_server_registry.go	Updated key mode checks in the private key base64 validation.
service/integration/kas_registry_test.go	Updated test cases to use the new key_mode constant.
service/integration/kas_registry_key_test.go	Updated various test cases for key creation and rotation.
protocol/go/policy/objects.pb.go	Updated generated key_mode definitions and enum maps.
protocol/go/CHANGELOG.md	Entire file removal noted.

Files not reviewed (9)

docs/grpc/index.html: Language not supported
docs/openapi/policy/actions/actions.swagger.json: Language not supported
docs/openapi/policy/attributes/attributes.swagger.json: Language not supported
docs/openapi/policy/kasregistry/key_access_server_registry.swagger.json: Language not supported
docs/openapi/policy/namespaces/namespaces.swagger.json: Language not supported
docs/openapi/policy/registeredresources/registered_resources.swagger.json: Language not supported
docs/openapi/policy/resourcemapping/resource_mapping.swagger.json: Language not supported
docs/openapi/policy/subjectmapping/subject_mapping.swagger.json: Language not supported
docs/openapi/policy/unsafe/unsafe.swagger.json: Language not supported

Comments suppressed due to low confidence (1)

protocol/go/CHANGELOG.md:1

The CHANGELOG file has been entirely removed. Please confirm if this removal was intentional or if changelog updates should be maintained elsewhere.

# Changelog

service/policy/kasregistry/key_access_server_registry.proto

🤖 I have created a release *beep* *boop* --- ## [0.3.4](protocol/go/v0.3.3...protocol/go/v0.3.4) (2025-05-20) ### Features * **core:** v2 ERS with proto updates ([#2210](#2210)) ([a161ef8](a161ef8)) * **policy:** Finish resource mapping groups ([#2224](#2224)) ([5ff754e](5ff754e)) ### Bug Fixes * 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>

🤖 I have created a release *beep* *boop* --- ## [0.5.3](service/v0.5.2...service/v0.5.3) (2025-05-22) ### Features * **authz:** authz v2 versioning implementation ([#2173](#2173)) ([557fc21](557fc21)) * **authz:** authz v2, ers v2 protos and gencode for ABAC with actions & registered resource ([#2124](#2124)) ([ea7992a](ea7992a)) * **authz:** export entity id prefix constant from entity instead of authorization service v1 ([#2261](#2261)) ([94079a9](94079a9)) * **authz:** subject mapping plugin support for ABAC with actions ([#2223](#2223)) ([d08b939](d08b939)) * bulk keycloak provisioning ([#2205](#2205)) ([59e4485](59e4485)) * **core:** add otel to opentdf services ([#1858](#1858)) ([53a7aa0](53a7aa0)) * **core:** Adds EC withSalt options ([#2126](#2126)) ([67b6fb8](67b6fb8)) * **core:** enhance db configuration options ([#2285](#2285)) ([ed9ff59](ed9ff59)) * **core:** New Key Index and Manager Plugin SPI ([#2095](#2095)) ([eb446fc](eb446fc)) * **core:** support onConfigUpdate hook when registering services ([#1992](#1992)) ([366d4dc](366d4dc)) * **core:** v2 ERS with proto updates ([#2210](#2210)) ([a161ef8](a161ef8)) * **policy:** actions crud service endpoints and proto validation ([#2037](#2037)) ([e933fa9](e933fa9)) * **policy:** actions service RPCs should actually hit storage layer CRUD ([#2063](#2063)) ([da4faf5](da4faf5)) * **policy:** add enhanced standard/custom actions protos ([#2020](#2020)) ([bbac53f](bbac53f)) * **policy:** Add platform key indexer. ([#2189](#2189)) ([861ef8d](861ef8d)) * **policy:** consume lib/identifier parse function ([#2181](#2181)) ([1cef22b](1cef22b)) * **policy:** DSPX-1018 NDR retrieval by FQN support ([#2131](#2131)) ([0001041](0001041)) * **policy:** DSPX-1057 registered resource action attribute values (DB + Service implementation) ([#2191](#2191)) ([6bf1b2e](6bf1b2e)) * **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-898 NDR database schema ([#2055](#2055)) ([2a10a6a](2a10a6a)) * **policy:** DSPX-901 NDR database crud ([#2071](#2071)) ([20e0a5f](20e0a5f)) * **policy:** DSPX-902 NDR service crud implementation (2/2) ([#2066](#2066)) ([030ad33](030ad33)) * **policy:** DSPX-902 NDR service crud protos only (1/2) ([#2092](#2092)) ([24b6cb5](24b6cb5)) * **policy:** Finish resource mapping groups ([#2224](#2224)) ([5ff754e](5ff754e)) * **policy:** GetMatchedSubjectMappings should provide value FQN ([#2151](#2151)) ([ad80044](ad80044)) * **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:** Restrict KAS deletion when tied to Key ([#2144](#2144)) ([4c4ab13](4c4ab13)) * **policy:** Return KAS Key structure ([#2172](#2172)) ([7f97b99](7f97b99)) * **policy:** rotate keys rpc ([#2180](#2180)) ([0d00743](0d00743)) * **policy:** stored enhanced actions database migration, CRUD queries, SM updates ([#2040](#2040)) ([e6b7c79](e6b7c79)) * **sdk:** Add a KAS allowlist ([#2085](#2085)) ([d7cfdf3](d7cfdf3)) * **sdk:** add nanotdf plaintext policy ([#2182](#2182)) ([e5c56db](e5c56db)) * **sdk:** Use ConnectRPC in the go client ([#2200](#2200)) ([fc34ee6](fc34ee6)) ### Bug Fixes * **core:** access pdp cleanup before actions in ABAC decisioning ([#2123](#2123)) ([9b38a3c](9b38a3c)) * **core:** Autobump service ([#2080](#2080)) ([006c724](006c724)) * **core:** Autobump service ([#2104](#2104)) ([1f72cc7](1f72cc7)) * **core:** Autobump service ([#2108](#2108)) ([be5b7d7](be5b7d7)) * **core:** bump to go 1.24 and bump service proto module dependencies ([#2064](#2064)) ([94891a0](94891a0)) * **core:** Fix DPoP with grpc-gateway ([#2044](#2044)) ([4483ef2](4483ef2)) * **core:** fix service go.mod ([#2141](#2141)) ([3b98f6d](3b98f6d)) * **core:** Improves errors when under heavy load ([#2132](#2132)) ([4490a14](4490a14)) * **core:** Let legacy KAOs use new trust plugins ([#2218](#2218)) ([5aa6916](5aa6916)) * **core:** migrate from mitchellh/mapstructure to go-viper/mapstructure ([#2087](#2087)) ([0a3a82e](0a3a82e)) * **core:** update viper to 1.20.1 ([#2088](#2088)) ([09099e9](09099e9)) * **core:** Updates vulnerable dep go/x/net ([#2072](#2072)) ([11c02cd](11c02cd)) * **deps:** bump github.com/creasty/defaults from 1.7.0 to 1.8.0 in /service ([#2242](#2242)) ([86a9b46](86a9b46)) * **deps:** bump github.com/jackc/pgx/v5 from 5.5.5 to 5.7.5 in /service ([#2249](#2249)) ([d8f3b67](d8f3b67)) * **deps:** bump the internal group across 1 directory with 2 updates ([#2296](#2296)) ([7f92c70](7f92c70)) * **deps:** bump toolchain in /lib/fixtures and /examples to resolve CVE GO-2025-3563 ([#2061](#2061)) ([9c16843](9c16843)) * handle empty private and public key ctx structs ([#2272](#2272)) ([f3fc647](f3fc647)) * **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)) * 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>

🤖 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 <[email protected]>

fix: update keymode to be less generic

3fad5a3

github-actions bot added the size/l label May 16, 2025

strantalis added 4 commits May 19, 2025 09:17

updated enum names

a16fd74

cleanup proto validation

823615c

update enum descriptions

963e8a0

fix lint issue

9a09a12

strantalis marked this pull request as ready for review May 19, 2025 16:06

strantalis requested review from a team as code owners May 19, 2025 16:06

KEK -> ROOT_KEY

5d7f147

strantalis requested a review from Copilot May 19, 2025 18:43

Copilot AI reviewed May 19, 2025

View reviewed changes

service/policy/kasregistry/key_access_server_registry.proto Outdated Show resolved Hide resolved

service/policy/kasregistry/key_access_server_registry.proto Outdated Show resolved Hide resolved

service/policy/kasregistry/key_access_server_registry.proto Outdated Show resolved Hide resolved

cleanup typos

0abc44a

strantalis requested a review from Copilot May 19, 2025 19:13

Copilot AI reviewed May 19, 2025

View reviewed changes

c-r33d approved these changes May 19, 2025

View reviewed changes

strantalis changed the title ~~fix: update keymode to be less generic~~ fix: update key_mode to provide more context May 20, 2025

strantalis requested a review from Copilot May 20, 2025 13:08

Copilot AI reviewed May 20, 2025

View reviewed changes

service/policy/kasregistry/key_access_server_registry.proto Show resolved Hide resolved

jrschumacher reviewed May 20, 2025

View reviewed changes

service/policy/kasregistry/key_access_server_registry.proto Show resolved Hide resolved

jrschumacher reviewed May 20, 2025

View reviewed changes

service/policy/kasregistry/key_access_server_registry.proto Show resolved Hide resolved

strantalis merged commit 44d0805 into opentdf:main May 20, 2025
26 of 29 checks passed

This was referenced May 20, 2025

chore(main): release protocol/go 0.3.4 #2222

Merged

chore(main): release service 0.5.3 #2046

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!

fix: update key_mode to provide more context #2226

fix: update key_mode to provide more context #2226

Uh oh!

strantalis commented May 16, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 19, 2025

Uh oh!

c-r33d left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fix: update key_mode to provide more context #2226

fix: update key_mode to provide more context #2226

Uh oh!

Conversation

strantalis commented May 16, 2025

Proposed Changes

Checklist

Testing Instructions

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 19, 2025

Choose a reason for hiding this comment

Uh oh!

c-r33d left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!