Skip to content

Conversation

@c-r33d
Copy link
Contributor

@c-r33d c-r33d commented Oct 16, 2025

Proposed Changes

1.) Fix kas_client to retrieve obligations from each kao metadata
2.) Dedup returned obligations
3.) Fix tests

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

@c-r33d c-r33d requested review from a team as code owners October 16, 2025 15:31
@github-actions github-actions bot added comp:sdk A software development kit, including library, for client applications and inter-service communicati size/m labels Oct 16, 2025
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @c-r33d, 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 significantly refactors the handling of "required obligations" within the SDK's decryption process. Previously, obligations were managed at a policy level; now, they are associated directly with individual Key Access Objects (KAOs) and then deduplicated. This change provides more precise reporting of obligations, particularly when specific KAOs fail during rewrap operations, ensuring that all relevant obligations are captured and presented accurately.

Highlights

  • Refactored Obligation Handling: Required obligations are now stored directly within each kaoResult (Key Access Object result) instead of being aggregated per policy, allowing for more granular reporting.
  • Deduplication of Obligations: A new utility function dedupRequiredObligations has been introduced to ensure that the final list of triggered obligations is unique across all KAOs for a given policy.
  • Updated KAS Client Logic: The kas_client now correctly retrieves and processes obligations from the metadata of individual KAOs, especially for failed rewrap attempts, ensuring accurate obligation reporting.
  • Removed policyResult Struct: The policyResult struct has been removed, simplifying the data structure for handling rewrap responses and aligning with the new per-KAO obligation model.
  • Enhanced Test Coverage: Unit tests for kas_client have been updated and expanded to cover the new obligation retrieval and deduplication logic, including scenarios with failing KAOs.
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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

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.


Data flows, a stream so wide, Obligations, now aligned. From each KAO, truths reside, Deduped, a clearer path we find.

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.

@c-r33d c-r33d changed the title pull metadata from each kao, dedup returned obligations. fix(sdk): Retrieve required obligations from correct metadata Oct 16, 2025
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

This pull request refactors how obligations are handled, moving them from being policy-level to KAO-level. This is a significant and correct change that aligns with how KAS should behave. The changes are consistently applied across bulk.go, kas_client.go, nanotdf.go, and tdf.go. The tests in kas_client_test.go are well-updated to reflect these changes. However, I found an issue in the test mock for TDF3 in tdf_test.go which is not fully updated, potentially leaving some new logic untested.

@github-actions
Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 359.048373ms
Throughput 278.51 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 39.491948086s
Average Latency 393.407024ms
Throughput 126.61 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 27.728868902s
Average Latency 276.333565ms
Throughput 180.32 requests/second

@github-actions
Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 368.009522ms
Throughput 271.73 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.339490746s
Average Latency 371.228854ms
Throughput 133.91 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.013369294s
Average Latency 259.26614ms
Throughput 192.21 requests/second

@github-actions
Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 366.147351ms
Throughput 273.11 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 38.761519628s
Average Latency 385.750829ms
Throughput 128.99 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 27.225693484s
Average Latency 271.347061ms
Throughput 183.65 requests/second

@github-actions
Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 347.113285ms
Throughput 288.09 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 38.772555174s
Average Latency 386.226016ms
Throughput 128.96 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 27.238895653s
Average Latency 271.165098ms
Throughput 183.56 requests/second

@c-r33d c-r33d merged commit f444a46 into DSPX-1357-obligations-decrypt Oct 17, 2025
25 checks passed
@c-r33d c-r33d deleted the test-obl-sdk branch October 17, 2025 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:sdk A software development kit, including library, for client applications and inter-service communicati size/m

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants