Skip to content

Conversation

alkalescent
Copy link
Contributor

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

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes

Hello @alkalescent, 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 focuses on enhancing the reliability of the GetDecision authorization logic by introducing comprehensive unit tests for its input validation, specifically for registered resources. This ensures that the validateGetDecisionRegisteredResource function behaves as expected under various valid and invalid conditions, improving the overall stability of the authorization service.

Highlights

  • Unit Testing: Added new, comprehensive unit tests for the validateGetDecisionRegisteredResource function to ensure robust input validation for GetDecision calls involving registered resources.
  • Code Maintenance: Removed a TODO comment in validators.go related to the need for unit tests, indicating the completion of this task.
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 is currently in preview and 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 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.


Code flows, logic gleams, Tests ensure the path is clear, Bugs hide, now revealed.

Footnotes

  1. 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.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The code changes introduce a new validation function validateGetDecisionRegisteredResource and adds corresponding unit tests. The validation function checks the validity of the input parameters for GetDecisionRegisteredResource, including the registered resource value FQN, action, and resources. The unit tests cover various scenarios, including valid inputs, invalid FQN, empty action name, empty resources, and nil resource in the list.

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 540.965666ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 333.532965ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 344.009928ms
Throughput 290.69 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 38.99237782s
Average Latency 388.175644ms
Throughput 128.23 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 27.552509264s
Average Latency 274.438383ms
Throughput 181.47 requests/second

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 512.550733ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 313.696051ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 341.278831ms
Throughput 293.02 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.949213022s
Average Latency 367.029396ms
Throughput 135.32 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.770921714s
Average Latency 256.807753ms
Throughput 194.02 requests/second

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 512.95758ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 331.90992ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 373.262922ms
Throughput 267.91 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.079494678s
Average Latency 368.76578ms
Throughput 134.85 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.058488533s
Average Latency 259.629884ms
Throughput 191.88 requests/second

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 522.999353ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 346.220776ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 341.541051ms
Throughput 292.79 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.880106762s
Average Latency 367.207372ms
Throughput 135.57 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.598760521s
Average Latency 265.099662ms
Throughput 187.98 requests/second

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 529.350287ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 335.387811ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 349.806605ms
Throughput 285.87 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.642698465s
Average Latency 364.704066ms
Throughput 136.45 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.541306702s
Average Latency 254.899336ms
Throughput 195.76 requests/second

@alkalescent alkalescent force-pushed the feature/rr-followup branch from e90f7ae to 9304cc8 Compare June 30, 2025 00:26
Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 523.240866ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 336.850424ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 339.468075ms
Throughput 294.58 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.514478384s
Average Latency 363.459346ms
Throughput 136.93 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.757697975s
Average Latency 256.414537ms
Throughput 194.12 requests/second

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 531.658479ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 324.815279ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 347.47872ms
Throughput 287.79 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.664682364s
Average Latency 365.143224ms
Throughput 136.37 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.832039785s
Average Latency 256.94461ms
Throughput 193.56 requests/second

Copy link
Contributor

github-actions bot commented Jul 1, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 533.938914ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 328.242766ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 336.716023ms
Throughput 296.99 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.854148935s
Average Latency 365.702598ms
Throughput 135.67 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.050765951s
Average Latency 259.216228ms
Throughput 191.93 requests/second

@alkalescent alkalescent force-pushed the feature/rr-followup branch from 93cbfbe to 9304cc8 Compare July 1, 2025 15:46
Copy link
Contributor

github-actions bot commented Jul 1, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 523.918341ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 326.0592ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 350.911119ms
Throughput 284.97 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.343611058s
Average Latency 371.388473ms
Throughput 133.89 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.232887183s
Average Latency 261.24935ms
Throughput 190.60 requests/second

Copy link
Contributor

github-actions bot commented Jul 1, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 547.53381ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 333.785753ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 346.029225ms
Throughput 288.99 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 40.104278518s
Average Latency 398.573865ms
Throughput 124.67 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 28.026247577s
Average Latency 279.307065ms
Throughput 178.40 requests/second

@alkalescent alkalescent marked this pull request as ready for review July 1, 2025 18:57
@alkalescent alkalescent requested review from a team as code owners July 1, 2025 18:57
Copy link
Contributor

github-actions bot commented Jul 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 518.976477ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 339.975484ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 348.198396ms
Throughput 287.19 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.372431993s
Average Latency 370.827117ms
Throughput 133.79 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.885938659s
Average Latency 257.554893ms
Throughput 193.16 requests/second

Copy link
Contributor

github-actions bot commented Jul 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 519.051365ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Error invalid_argument: validation error:
  • resources: value must contain no more than 1000 item(s) [repeated.max_items] |
    | Total Time | 40.230557ms |

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 345.85451ms
Throughput 289.14 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.387660207s
Average Latency 372.04806ms
Throughput 133.73 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.948951238s
Average Latency 258.514932ms
Throughput 192.69 requests/second

@alkalescent alkalescent added this pull request to the merge queue Jul 3, 2025
Merged via the queue into main with commit 443cedb Jul 3, 2025
29 checks passed
@alkalescent alkalescent deleted the feature/rr-followup branch July 3, 2025 22:03
github-merge-queue bot pushed a commit that referenced this pull request Jul 31, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.8.0](service/v0.7.0...service/v0.8.0)
(2025-07-29)


### Features

* **authz:** RR GetDecision improvements
([#2479](#2479))
([443cedb](443cedb))
* **authz:** sensible request limit upper bounds
([#2526](#2526))
([b3093cc](b3093cc))
* **core:** Add the ability to configure the http server settings
([#2522](#2522))
([b1472df](b1472df))
* **policy:** Add list key mappings rpc.
([#2533](#2533))
([fbc2724](fbc2724))
* **policy:** add obligation protos
([#2579](#2579))
([50882e1](50882e1))
* **policy:** add obligation tables
([#2532](#2532))
([c7d7aa4](c7d7aa4))
* **policy:** Add validation to delete keys
([#2576](#2576))
([cc169d9](cc169d9))
* **policy:** Allow the deletion of a key.
([#2575](#2575))
([82b96f0](82b96f0))
* **policy:** Change return type for delete key proto.
([#2566](#2566))
([c1ae924](c1ae924))
* **policy:** sqlc queries refactor
([#2541](#2541))
([e34680e](e34680e))


### Bug Fixes

* add back grants to listAttributesByDefOrValueFqns
([#2493](#2493))
([2b47095](2b47095))
* **authz:** access pdp should use proto getter
([#2530](#2530))
([f856212](f856212))
* **core:** Allow 521 curve to be used
([#2485](#2485))
([aaf43dc](aaf43dc))
* **core:** resolve 'built-in' typos
([#2548](#2548))
([ccdfa96](ccdfa96))
* **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.2.0 to
0.3.0 in /service
([#2504](#2504))
([a9cc4dd](a9cc4dd))
* **sdk:** Prefer KID and Algorithm selection from key maps
([#2475](#2475))
([98fd392](98fd392))

---
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>
github-merge-queue bot pushed a commit that referenced this pull request Sep 17, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.1.0](lib/identifier/v0.0.2...lib/identifier/v0.1.0)
(2025-09-17)


### Features

* **authz:** RR GetDecision improvements
([#2479](#2479))
([443cedb](443cedb))
* **policy:** add protovalidate for obligation defs + vals
([#2699](#2699))
([af5c049](af5c049))

---
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants