Skip to content

feat(policy): Modify KAS indexer to support legacy keys. #2616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Aug 8, 2025

Conversation

c-r33d
Copy link
Contributor

@c-r33d c-r33d commented Aug 7, 2025

Proposed Changes

1.) Create a new function for key_index called ListKeysWith allowing developers to pass in common ListKeyOptions
2.) Modify ListKeys/FindKeyByAlgorithm within key_indexer to support listing legacy keys.

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 August 7, 2025 14:48
@github-actions github-actions bot added comp:kas Key Access Server size/s labels Aug 7, 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.

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 enhances the Key Access Server (KAS) indexer by introducing the capability to filter keys based on their 'legacy' status. The core change involves modifying key listing and finding functions across the key management stack to accept a legacyOnly or includeLegacy parameter, allowing consumers to retrieve only legacy keys or exclude them as needed. This provides more granular control over key discovery and management within the system.

Highlights

  • Key Indexing Interface Update: The KeyIndex interface and its various implementations (DelegatingKeyService, InProcessProvider, KeyIndexer) have been updated to include a legacyOnly boolean parameter in their ListKeys method. This new parameter enables explicit filtering of keys based on their legacy status.
  • Legacy Key Filtering Logic: The ListKeys implementations now incorporate logic to filter the returned keys, ensuring that only those marked as legacy are returned when the legacyOnly parameter is set to true. This includes propagating the legacyOnly flag down to the kasregistry.ListKeysRequest.
  • KeyAdapter.IsLegacy() Implementation: The KeyAdapter's IsLegacy() method has been updated to correctly retrieve the legacy status from the underlying policy.AsymmetricKey protobuf message, rather than always returning false. This allows the system to accurately identify legacy keys.
  • FindKeyByAlgorithm Enhancement: The KeyIndexer's FindKeyByAlgorithm method has been extended to support filtering keys based on their legacy status via an includeLegacy parameter, providing more granular control over key discovery.
  • Test Coverage: New unit tests have been added to verify the correct behavior of the KeyAdapter.IsLegacy() method and the ListKeys filtering functionality in DelegatingKeyService and MockSecurityProvider, ensuring the new features work as expected.
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.


Old keys, new keys, Filtered now with careful hand, Legacy finds peace.

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

This pull request introduces support for filtering legacy keys in the KAS indexer by modifying the ListKeys and FindKeyByAlgorithm methods. The changes correctly propagate the new legacyOnly and includeLegacy parameters through the call chain, and the tests have been updated accordingly.

However, I've identified two critical issues in the implementation within service/kas/key_indexer.go where the new filtering logic is not correctly implemented, leading to incorrect behavior. The provided suggestions should resolve these issues. Please review the comments for details.

Copy link
Contributor

github-actions bot commented Aug 7, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 343.254059ms
Throughput 291.33 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.754324209s
Average Latency 365.842345ms
Throughput 136.04 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.730088666s
Average Latency 256.466999ms
Throughput 194.33 requests/second

Copy link
Contributor

github-actions bot commented Aug 7, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 343.159791ms
Throughput 291.41 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.626326006s
Average Latency 364.228708ms
Throughput 136.51 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.404518524s
Average Latency 252.988895ms
Throughput 196.82 requests/second

Copy link
Contributor

github-actions bot commented Aug 8, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 345.42763ms
Throughput 289.50 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.391438941s
Average Latency 362.28607ms
Throughput 137.39 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.826708223s
Average Latency 257.310551ms
Throughput 193.60 requests/second

Copy link
Contributor

github-actions bot commented Aug 8, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 349.431391ms
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.692950856s
Average Latency 365.399834ms
Throughput 136.27 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.571347896s
Average Latency 255.008345ms
Throughput 195.53 requests/second

Copy link
Contributor

github-actions bot commented Aug 8, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 354.679355ms
Throughput 281.94 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.579972259s
Average Latency 374.454311ms
Throughput 133.05 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.205956913s
Average Latency 261.234185ms
Throughput 190.80 requests/second

Copy link
Contributor

github-actions bot commented Aug 8, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 356.353158ms
Throughput 280.62 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.682767119s
Average Latency 365.444912ms
Throughput 136.30 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.482406875s
Average Latency 253.577734ms
Throughput 196.21 requests/second

Copy link
Contributor

github-actions bot commented Aug 8, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 337.275249ms
Throughput 296.49 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.797274752s
Average Latency 366.464792ms
Throughput 135.88 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.539132802s
Average Latency 254.482208ms
Throughput 195.78 requests/second

@c-r33d c-r33d requested a review from dmihalcik-virtru August 8, 2025 16:55
Copy link
Contributor

github-actions bot commented Aug 8, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 346.535937ms
Throughput 288.57 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.020133641s
Average Latency 358.578312ms
Throughput 138.81 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.179534631s
Average Latency 251.068197ms
Throughput 198.57 requests/second

Copy link
Contributor

github-actions bot commented Aug 8, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 348.811531ms
Throughput 286.69 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.497722857s
Average Latency 362.974031ms
Throughput 136.99 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.596705874s
Average Latency 254.427695ms
Throughput 195.34 requests/second

@c-r33d c-r33d added this pull request to the merge queue Aug 8, 2025
Merged via the queue into main with commit ba96c18 Aug 8, 2025
29 checks passed
@c-r33d c-r33d deleted the feat/DSPX-1535-legacy-key-indexer branch August 8, 2025 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:kas Key Access Server size/s
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants