feat(policy): add search support to ListKeyAccessServers - #3557
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, 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 introduces search capabilities to the ListKeyAccessServers RPC, enabling users to filter results by name or URI. The implementation leverages a new SQL filtering layer that ensures case-insensitive matching while correctly handling wildcard characters. Additionally, the changes include robust integration tests to verify the search logic, pagination, and edge cases like empty search terms. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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
Customization To customize the 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 Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. 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. The search is added to the list, / No KAS shall now be missed. / With patterns clear and SQL tight, / The registry is now just right. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request implements search functionality for key access servers by name and URI. It introduces a filtered CTE in the SQL query to perform case-insensitive matching using LIKE with wildcard escaping, and adds comprehensive integration tests to verify this behavior. The feedback suggests removing the redundant LOWER(kas.name) call in the SQL query, as the name is already stored in lowercase, which will optimize query execution.
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Signed-off-by: Chris Reed <creed@virtru.com>
4c15967 to
32999de
Compare
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
## Summary - Adds ListKeyAccessServers RPC search support by wiring request search into the policy DB list query. - Applies escaped, case-insensitive matching in the KAS registry SQL path and adds integration coverage for search behavior, wildcard literals, empty search, and pagination after filtering. --------- Signed-off-by: Chris Reed <creed@virtru.com>
Summary