fix(kas): Allow admin to set registered kas uri #2624

c-r33d · 2025-08-08T20:19:20Z

Proposed Changes

1.) Allow admin to set the registered KAS URI, instead of inferring it if present.

Note

If the registered_kas_uri is empty or not present we fallback to
inferring.

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

gemini-code-assist

Summary of Changes

Hello @c-r33d, I'm Gemini Code Assist¹! 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!

I've updated the KAS (Key Access Server) configuration to allow administrators to explicitly define the registered KAS URI. Previously, the system would infer this URI based on network settings and hostname, which could lead to ambiguity. This change provides more direct control and predictability over how the KAS identifies itself, simplifying deployments and ensuring consistent behavior.

Highlights

KAS URI Configuration: Introduced a new configuration option (registered_kas_uri) under kas.preview.key_management in opentdf-dev.yaml and opentdf-kas-mode.yaml, allowing the KAS URI to be explicitly set.
Removal of Inferred URI Logic: Eliminated the code responsible for inferring the KAS URI from the server's public hostname and listening address, streamlining the KAS initialization process.
Configuration Schema Update: Refactored the Preview and KeyManagement structs in service/kas/access/provider.go to properly support the new nested enabled and registered_kas_uri fields for key management.
Cleanup of public_hostname: Removed the server.public_hostname configuration entry from opentdf-dev.yaml and opentdf-kas-mode.yaml as it is no longer used for KAS URI determination.
Test and Fixture Alignment: Updated service/internal/fixtures/policy_fixtures.yaml to reflect the new explicit URI setting and modified service/kas/access/publicKey_test.go to remove outdated URI inference test logic.

Using Gemini Code Assist

The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize 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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

URI now defined,No more guessing, clear the path,Config's true control.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a configuration option to explicitly set the registered KAS URI, removing the previous logic that inferred it from the server's public hostname and port. The changes are well-implemented across configuration files, service logic, and tests. I've provided a couple of suggestions to improve robustness and code consistency. One suggestion is to add a validation check to ensure the KAS URI is not empty when key management is enabled. The other is a minor naming convention fix for better code consistency.

service/kas/kas.go

service/kas/access/provider.go

github-actions · 2025-08-08T20:24:17Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	186.793516ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	92.794483ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	348.194723ms
Throughput	287.20 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	36.865278402s
Average Latency	366.942993ms
Throughput	135.63 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	25.691576705s
Average Latency	255.963672ms
Throughput	194.62 requests/second

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

c-r33d · 2025-08-08T20:42:44Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors the Key Access Service (KAS) configuration to allow administrators to explicitly set the registered KAS URI, removing the previous logic that inferred it. This is a good improvement for configuration clarity and robustness. However, the implementation introduces a breaking change in the configuration format for key_management, which could impact existing deployments. I've also included a minor suggestion to improve code idiom.

service/kas/access/provider.go

service/kas/kas.go

github-actions · 2025-08-08T20:45:44Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	169.943945ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	99.564331ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	347.373481ms
Throughput	287.87 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	36.232506242s
Average Latency	360.135883ms
Throughput	138.00 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	25.423997741s
Average Latency	253.414504ms
Throughput	196.66 requests/second

github-actions · 2025-08-08T20:47:23Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	169.065657ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	84.376953ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	349.428399ms
Throughput	286.18 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	36.817216166s
Average Latency	366.585665ms
Throughput	135.81 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	25.530471666s
Average Latency	254.061317ms
Throughput	195.84 requests/second

github-actions · 2025-08-11T13:14:59Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	177.754493ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	95.007832ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	343.997955ms
Throughput	290.70 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	37.016907306s
Average Latency	368.106401ms
Throughput	135.07 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	25.506735321s
Average Latency	254.455908ms
Throughput	196.03 requests/second

c-r33d · 2025-08-11T16:04:07Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a feature allowing administrators to explicitly set the registered KAS URI via a new configuration option, registered_kas_uri. When this option is not provided, the system falls back to the previous behavior of inferring the URI from the server's configuration. The implementation correctly refactors the URI determination logic into a new function, determineKASURL, and updates the necessary configuration files and tests. My review identifies one area for improvement in the new function regarding inconsistent error handling, where panic is used instead of returning an error as per the function's signature.

service/kas/kas.go

github-actions · 2025-08-11T16:08:30Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	170.498894ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	98.026513ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	519.647373ms
Throughput	192.44 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	37.022937863s
Average Latency	368.675811ms
Throughput	135.05 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	25.239105442s
Average Latency	251.188908ms
Throughput	198.11 requests/second

c-r33d · 2025-08-11T16:23:57Z

/gemini review

github-actions · 2025-08-11T16:24:22Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	183.563266ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	98.137421ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	346.292999ms
Throughput	288.77 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	36.635196987s
Average Latency	364.285409ms
Throughput	136.48 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	25.531928988s
Average Latency	254.583172ms
Throughput	195.83 requests/second

gemini-code-assist

Code Review

This pull request introduces a new configuration option registered_kas_uri for the KAS service, allowing an administrator to explicitly set the KAS URI. When this option is not set, the system falls back to inferring the URI from the server configuration. The implementation correctly refactors the URI determination logic into a new function and updates the necessary configuration files, tests, and fixtures. The changes are well-structured. I have one suggestion to simplify the URI inference logic for better readability and efficiency.

service/kas/kas.go

github-actions · 2025-08-11T16:28:14Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	214.698725ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	90.556922ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	370.173278ms
Throughput	270.14 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	41.679148809s
Average Latency	414.861086ms
Throughput	119.96 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	29.056620958s
Average Latency	289.38603ms
Throughput	172.08 requests/second

github-actions · 2025-08-11T16:30:05Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	173.066603ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	83.575962ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	350.981662ms
Throughput	284.92 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	37.188952107s
Average Latency	371.0409ms
Throughput	134.45 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	26.136294715s
Average Latency	260.180002ms
Throughput	191.30 requests/second

### Proposed Changes 1.) Allow admin to set the registered KAS URI, instead of inferring it if present. >[!NOTE] >If the registered_kas_uri is empty or not present we fallback to >inferring. ### 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 --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> (cherry picked from commit 6203fba)

opentdf-automation · 2025-08-11T18:24:46Z

Successfully created backport PR for release/service/v0.8:

fix(kas): Allow admin to set registered kas uri [backport to release/service/v0.8] #2628

### Proposed Changes 1.) Allow admin to set the registered KAS URI, instead of inferring it if present. >[!NOTE] >If the registered_kas_uri is empty or not present we fallback to >inferring. ### 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 --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> (cherry picked from commit 6203fba)

…service/v0.8] (#2628) # Description Backport of #2624 to `release/service/v0.8`. --------- Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Dave Mihalcik <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Chris Reed <[email protected]>

1.) Add `registered_kas_uri` to kas config. [Kas URI bug fix](opentdf/platform#2624) 2.) Deprecate `server.public_hostname` 3.) Change `preview_features` to `preview` --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

🤖 I have created a release *beep* *boop* --- ## [0.9.0](service/v0.8.0...service/v0.9.0) (2025-08-27) ### Features * **core:** add multi-strategy ERS to support ldap and sql ([#2596](#2596)) ([855611d](855611d)) * **policy:** Add legacy keys. ([#2613](#2613)) ([57370b0](57370b0)) * **policy:** add values to CreateObligationRequest ([#2614](#2614)) ([94535cc](94535cc)) * **policy:** Modify KAS indexer to support legacy keys. ([#2616](#2616)) ([ba96c18](ba96c18)) ### Bug Fixes * **deps:** bump github.com/docker/docker from 28.2.2+incompatible to 28.3.3+incompatible in /service ([#2598](#2598)) ([3c392aa](3c392aa)) * **deps:** bump github.com/go-viper/mapstructure/v2 from 2.3.0 to 2.4.0 in /service ([#2649](#2649)) ([b838bbc](b838bbc)) * **deps:** bump github.com/opentdf/platform/sdk from 0.5.0 to 0.7.0 in /service ([#2660](#2660)) ([2c998ac](2c998ac)) * **kas:** Allow admin to set registered kas uri ([#2624](#2624)) ([6203fba](6203fba)) * updated generated sqlc ([#2609](#2609)) ([e44a569](e44a569)) --- 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>

c-r33d added 2 commits August 8, 2025 15:15

fix(kas): Accept kas uri input from admin.

fecad1e

commit yaml files.

42a8bac

c-r33d requested review from a team as code owners August 8, 2025 20:19

github-actions bot added comp:kas Key Access Server size/s labels Aug 8, 2025

gemini-code-assist bot reviewed Aug 8, 2025

View reviewed changes

service/kas/kas.go Outdated Show resolved Hide resolved

service/kas/access/provider.go Outdated Show resolved Hide resolved

c-r33d and others added 2 commits August 8, 2025 15:41

Update service/kas/kas.go

a83b06e

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

gemini.

a2bde58

gemini-code-assist bot reviewed Aug 8, 2025

View reviewed changes

service/kas/access/provider.go Outdated Show resolved Hide resolved

service/kas/kas.go Show resolved Hide resolved

lint.

899dafa

c-r33d changed the title ~~fix(kas): Allow admin to set registered kas uri~~ fix(kas)!: Allow admin to set registered kas uri Aug 11, 2025

strantalis previously approved these changes Aug 11, 2025

View reviewed changes

backwards compatible.

48256e5

c-r33d dismissed strantalis’s stale review via 48256e5 August 11, 2025 16:03

c-r33d changed the title ~~fix(kas)!: Allow admin to set registered kas uri~~ fix(kas): Allow admin to set registered kas uri Aug 11, 2025

gemini-code-assist bot reviewed Aug 11, 2025

View reviewed changes

service/kas/kas.go Show resolved Hide resolved

fix panic.

32c7725

jentfoo previously approved these changes Aug 11, 2025

View reviewed changes

fix panic.

67e5c18

c-r33d dismissed jentfoo’s stale review via 67e5c18 August 11, 2025 16:23

fix fixtures.

5f65b11

gemini-code-assist bot reviewed Aug 11, 2025

View reviewed changes

service/kas/kas.go Show resolved Hide resolved

c-r33d requested a review from strantalis August 11, 2025 16:39

c-r33d added the backport release/service/v0.8 label Aug 11, 2025

strantalis approved these changes Aug 11, 2025

View reviewed changes

c-r33d added this pull request to the merge queue Aug 11, 2025

Merged via the queue into main with commit 6203fba Aug 11, 2025
29 checks passed

c-r33d deleted the fix/DSPX-1401-kas-uri-mismatch branch August 11, 2025 18:24

opentdf-automation bot mentioned this pull request Aug 11, 2025

fix(kas): Allow admin to set registered kas uri [backport to release/service/v0.8] #2628

Merged

opentdf-automation bot mentioned this pull request Aug 9, 2025

chore(main): release service 0.9.0 #2612

Merged

c-r33d mentioned this pull request Aug 12, 2025

feat(kas): Add registered_kas_uri to values opentdf/charts#148

Merged

fix(kas): Allow admin to set registered kas uri #2624

fix(kas): Allow admin to set registered kas uri #2624

Uh oh!

Conversation

c-r33d commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Checklist

Testing Instructions

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 8, 2025

Benchmark authorization.GetDecisions Results:

Benchmark authorization.v2.GetMultiResourceDecision Results:

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Uh oh!

c-r33d commented Aug 8, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 8, 2025

Benchmark authorization.GetDecisions Results:

Benchmark authorization.v2.GetMultiResourceDecision Results:

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Uh oh!

github-actions bot commented Aug 8, 2025

Benchmark authorization.GetDecisions Results:

Benchmark authorization.v2.GetMultiResourceDecision Results:

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Uh oh!

github-actions bot commented Aug 11, 2025

Benchmark authorization.GetDecisions Results:

Benchmark authorization.v2.GetMultiResourceDecision Results:

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Uh oh!

c-r33d commented Aug 11, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

github-actions bot commented Aug 11, 2025

Benchmark authorization.GetDecisions Results:

Benchmark authorization.v2.GetMultiResourceDecision Results:

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Uh oh!

c-r33d commented Aug 11, 2025

Uh oh!

github-actions bot commented Aug 11, 2025

c-r33d commented Aug 8, 2025 •

edited

Loading