feat(policy): add search support to ListSubjectConditionSets#3556
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:
📝 WalkthroughWalkthroughThis PR adds text search capability to ChangesListSubjectConditionSets Search Filter Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 for the ListSubjectConditionSets RPC. By wiring search terms into the underlying policy database queries, users can now filter condition sets based on selectors and values. The implementation ensures robust matching through case-insensitive SQL operations and includes new utility functions to handle search patterns, while also adding extensive integration tests to verify the new functionality. 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. A search term sent to find the set, / With SQL logic, we are set. / Through JSONB paths the query flows, / To find the match that the user knows. Footnotes
|
github-actions
Bot
added
comp:db
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces search capabilities to the ListSubjectConditionSets endpoint, allowing users to search within the JSONB condition field of subject condition sets. It includes updates to the SQL queries, database client code, utility functions, and comprehensive integration tests. The reviewer's feedback suggests optimizing the SQL query by combining multiple EXISTS clauses into a single one using UNION ALL to reduce duplication and improve query execution. Additionally, the reviewer recommends using the existing pgtypeSubstringSearchPattern function instead of introducing a new NoTrim variant to maintain consistency across search endpoints and avoid redundant helper code.
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>
Signed-off-by: Chris Reed <creed@virtru.com>
2efc296 to
4a4b167
Compare
|
@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:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@service/integration/subject_mappings_test.go`:
- Around line 1493-1529: The created_at ordering assertion can flake because
labelValueMatch and anotherLabelValueMatch are created back-to-back; insert a
short deterministic delay (e.g., time.Sleep(5 * time.Millisecond)) between
creating labelValueMatch and anotherLabelValueMatch (the calls that assign those
variables via s.newSearchTestSubjectConditionSet) so their created_at timestamps
differ, ensuring ListSubjectConditionSets sorted by created_at ASC returns a
stable order; mirror the same approach used in other sort tests in this file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cdd701a1-70f9-4f37-96cf-5b9478271105
📒 Files selected for processing (5)
service/integration/subject_mappings_test.goservice/policy/db/queries/subject_mappings.sqlservice/policy/db/subject_mappings.goservice/policy/db/subject_mappings.sql.goservice/policy/db/utils.go
Signed-off-by: Chris Reed <creed@virtru.com>
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
## Summary - Adds ListSubjectConditionSets RPC search support by wiring request search into the policy DB list query. - Applies escaped, case-insensitive matching in the subject condition set SQL path and adds integration coverage for search behavior, wildcard literals, empty search, and pagination after filtering. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added search functionality for subject condition sets filtered by metadata labels. * Search supports pagination, sorting, and combines with namespace filtering. * Search properly handles whitespace and special character escaping. * **Tests** * Expanded integration test coverage for search behavior with metadata labels. * Updated existing sorting tests with improved cleanup utilities. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Chris Reed <creed@virtru.com>
2 participants
Summary
Summary by CodeRabbit
New Features
Tests