feat(policy): Add the ability to do substring search - #3551
Merged
Conversation
📝 WalkthroughWalkthroughAdds server-side substring search filtering to eight policy list RPCs (attributes, namespaces, KAS registry, KAS keys, obligations, registered resources, subject mappings, subject condition sets). Enforces ChangesPolicy List Search + sqlc v1.31.1
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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 docstrings
🧪 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 |
github-actions
Bot
added
comp:policy
Contributor
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 a standardized substring search mechanism across the policy service API. By adding a reusable 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. Ignored Files
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 added to the list, To find the data that we missed. With LIKE or ILIKE in the base, We find the items in their place. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new Search message containing a term field and integrates it as an optional search parameter across multiple list request protobuf definitions in the policy service. It also includes updated documentation and a unit test for search term validation. The reviewer recommends enforcing a minimum length of 1 character on the search term to prevent inefficient database queries with empty strings, along with adding a corresponding test case to verify this validation.
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
X-Test Failure Report |
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
### Proposed Changes 1.) Add substring searching to ListNamespaces, `fqn` field. 2.) Escape characters used by `LIKE\ILIKE` from incoming input 3.) Specify `\` as the escape character ### 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added search functionality to namespace listing with support for name and FQN matching * Search is case-insensitive and supports prefix matching * Search integrates with namespace state filtering (ACTIVE/INACTIVE) * Special wildcard characters are properly escaped to prevent unexpected matches * **Tests** * Added comprehensive test coverage for search functionality, pagination, and edge cases <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - Adds ListAttributes RPC search support by wiring request search into the policy DB list query. - Applies escaped, case-insensitive matching in the attributes 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 attributes, including filtering by fully qualified name with wildcard escaping and combined namespace/state filters. * **Tests** * Added comprehensive integration tests for attribute search operations, including pagination and filter combinations. * **Chores** * Updated sqlc tool to version 1.31.1. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Chris Reed <creed@virtru.com>
## Summary - Adds ListRegisteredResources RPC search support by wiring request search into the policy DB list query. - Applies escaped, case-insensitive matching in the registered resources SQL path and adds integration coverage for search behavior, wildcard literals, namespace filters, empty/whitespace search, and pagination after filtering. --------- Signed-off-by: Chris Reed <creed@virtru.com>
## 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 - Adds ListKeys RPC search support by wiring request search into the policy DB list query. - Applies escaped, case-insensitive matching in the KAS key SQL path and adds integration coverage for search behavior, wildcard literals, empty search, whitespace handling, and pagination after filtering. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Search capability for KAS registry keys by key ID. * Search input automatically trims leading and trailing whitespace. * Wildcard characters in search queries are properly escaped. * Search results work seamlessly with existing filters and pagination. * **Tests** * Added comprehensive test coverage for search functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Chris Reed <creed@virtru.com>
c-r33d
marked this pull request as ready for review
June 15, 2026 14:08
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
## Summary Adds proto validation coverage for `policy.Search` on each List RPC that supports search. ## Changes - Added validation tests confirming search can be provided with a non-empty term. - Added validation tests confirming search can be omitted. - Added validation tests confirming present-but-empty search fails validation. - Updated existing `ListNamespacesRequest` search validation to include the empty-search failure case. --------- Signed-off-by: Chris Reed <creed@virtru.com> Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Contributor
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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
service/policy/db/queries/key_access_server_registry.sql (1)
97-117:⚠️ Potential issue | 🟡 MinorResolve duplicate
kasalias to avoid SQLFluff AL04 failures.The
kasalias is used twice in the same query: for the outerFROM filtered AS kas(line 97) and for the innerINNER JOIN key_access_servers kas(line 115). While SQL scoping prevents runtime issues, duplicate aliases violate SQLFluff's AL04 rule and reduce code clarity. Renaming the inner alias keeps semantics intact.Suggested fix
- INNER JOIN key_access_servers kas ON kask.key_access_server_id = kas.id + INNER JOIN key_access_servers kas_keys_src ON kask.key_access_server_id = kas_keys_src.id ... - 'kas_uri', kas.uri, - 'kas_id', kas.id, + 'kas_uri', kas_keys_src.uri, + 'kas_id', kas_keys_src.id,🤖 Prompt for 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. In `@service/policy/db/queries/key_access_server_registry.sql` around lines 97 - 117, The query uses the `kas` alias twice: once for the outer FROM clause referencing filtered table and again in the inner subquery for the INNER JOIN with key_access_servers table. This duplicate alias violates SQLFluff's AL04 rule and reduces clarity. Rename the inner `key_access_servers kas` alias (in the subquery starting at line 115) to a different name (e.g., kas_server or ks) and update the corresponding reference in the ON clause condition that references this inner table alias to maintain correct join semantics.Source: Linters/SAST tools
🤖 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 `@protocol/go/CHANGELOG.md`:
- Around line 6-20: The CHANGELOG.md file is missing documentation of
client-facing breaking changes for version 0.33.0. Add two breaking changes to
the BREAKING CHANGES section: first, document that Search.term now enforces a
minimum length of 1 character and will reject empty strings, and second,
document that the search field has been removed from multiple request schemas.
These changes impact the client contract and should be clearly listed in the
breaking changes section alongside the existing policy namespace fields change.
In `@service/integration/namespaces_test.go`:
- Around line 702-720: In the Test_ListNamespaces_SearchEmptyQuery_Succeeds
function, add a third ListNamespaces call with a whitespace-only search term (a
string containing only spaces, such as " ") in addition to the existing
noSearch and emptySearch calls. Then add an assertion that the pagination total
from this whitespace-only search matches the same total as the noSearch and
emptySearch results, validating that whitespace-only search terms are properly
trimmed to empty and behave identically to both no search and empty string
search scenarios.
In `@service/integration/subject_mappings_test.go`:
- Around line 1238-1241: The tests at
service/integration/subject_mappings_test.go lines 1238-1241 and 1725-1727
create multiple rows in rapid succession and then assert strict created_at ASC
ordering. Without spacing between inserts, equal timestamps cause flaky test
ordering failures. Add small delays between the createSearchSubjectMapping calls
in both the loop at lines 1238-1241 and the sequential calls at lines 1725-1727
to ensure each row has a distinct timestamp, making the ordering deterministic
and preventing intermittent test failures.
In `@service/policy/db/key_access_server_registry.sql.go`:
- Around line 647-651: The kas.name search condition uses case-sensitive LIKE
matching while pgtypeSubstringSearchPattern lowercases the search input, causing
mixed-case KAS names to be missed; additionally, kas.uri in the same condition
correctly uses case-insensitive ILIKE, creating inconsistency. Update the source
SQL files (not the generated .sql.go files) to change kas.name LIKE to kas.name
ILIKE for case-insensitive matching, apply the same LIKE to ILIKE change in
registered_resources.sql, then regenerate the sqlc code to update all generated
Go files.
In `@service/policy/db/obligations.sql.go`:
- Around line 1746-1752: The SQL queries use case-sensitive LIKE operators
instead of case-insensitive ILIKE, causing searches to fail when FQN or value
casing differs from the search term. In service/policy/db/obligations.sql.go at
lines 1746-1752, replace LIKE with ILIKE in the expression CONCAT_WS('/',
fqns.fqn, 'obl', od.name) LIKE $3::text to enable case-insensitive matching for
the obligation name/FQN search. In service/policy/db/subject_mappings.sql.go at
lines 539-547, replace LIKE with ILIKE in the expression fqns.fqn LIKE $7::TEXT
to align with the case-insensitive label.value ILIKE in the parallel OR branch
of the same condition.
In `@service/policy/db/queries/attributes.sql`:
- Around line 47-52: The LIKE operator used in the attribute FQN search
condition is case-sensitive, but since stored FQN values may contain mixed case
while search input is lowercased, valid matches will be missed. Replace the LIKE
operator with ILIKE in the search condition on the fqns.fqn column to enable
case-insensitive pattern matching. Keep the ESCAPE clause as-is.
In `@service/policy/db/queries/obligations.sql`:
- Around line 201-204: The search predicate in the obligation query uses the
case-sensitive LIKE operator to match against the concatenated FQN path, but the
search input is already lowercased upstream via pgtypeSubstringSearchPattern().
Since FQNs can contain mixed-case components (e.g., https://Example.com), the
comparison will fail. Replace the LIKE operator with ILIKE (PostgreSQL's
case-insensitive variant) on the line that compares CONCAT_WS('/', fqns.fqn,
'obl', od.name) against the search parameter, or alternatively apply LOWER() to
the CONCAT_WS expression to match the lowercased search term.
In `@service/policy/db/queries/subject_mappings.sql`:
- Around line 133-136: The FQN search condition on line 135 uses the
case-sensitive LIKE operator, while line 139 uses the case-insensitive ILIKE
operator for the same search parameter when matching labels, creating an
inconsistency. Replace the LIKE operator with ILIKE in the condition that checks
fqns.fqn against the search parameter to ensure FQN matching is case-insensitive
and consistent with the label matching behavior below it, allowing searches to
correctly match FQN values regardless of casing differences.
---
Outside diff comments:
In `@service/policy/db/queries/key_access_server_registry.sql`:
- Around line 97-117: The query uses the `kas` alias twice: once for the outer
FROM clause referencing filtered table and again in the inner subquery for the
INNER JOIN with key_access_servers table. This duplicate alias violates
SQLFluff's AL04 rule and reduces clarity. Rename the inner `key_access_servers
kas` alias (in the subquery starting at line 115) to a different name (e.g.,
kas_server or ks) and update the corresponding reference in the ON clause
condition that references this inner table alias to maintain correct join
semantics.
🪄 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: cd7eb0be-5e0a-4bb9-b46d-5fa6cb6f0fab
⛔ Files ignored due to path filters (3)
protocol/go/policy/obligations/obligations.pb.gois excluded by!**/*.pb.goprotocol/go/policy/registeredresources/registered_resources.pb.gois excluded by!**/*.pb.goprotocol/go/policy/selectors.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (59)
.github/release-please/release-please-manifest.json.github/workflows/checks.yamlMakefiledocs/grpc/index.htmldocs/openapi/policy/attributes/attributes.openapi.yamldocs/openapi/policy/kasregistry/key_access_server_registry.openapi.yamldocs/openapi/policy/namespaces/namespaces.openapi.yamldocs/openapi/policy/obligations/obligations.openapi.yamldocs/openapi/policy/registeredresources/registered_resources.openapi.yamldocs/openapi/policy/selectors.openapi.yamldocs/openapi/policy/subjectmapping/subject_mapping.openapi.yamlprotocol/go/CHANGELOG.mdservice/integration/attributes_test.goservice/integration/kas_registry_key_test.goservice/integration/kas_registry_test.goservice/integration/namespaces_test.goservice/integration/obligations_test.goservice/integration/registered_resources_test.goservice/integration/subject_mappings_test.goservice/integration/utils.goservice/policy/attributes/attributes_test.goservice/policy/db/actions.sql.goservice/policy/db/attribute_fqn.sql.goservice/policy/db/attribute_values.sql.goservice/policy/db/attributes.goservice/policy/db/attributes.sql.goservice/policy/db/copyfrom.goservice/policy/db/db.goservice/policy/db/key_access_server_registry.goservice/policy/db/key_access_server_registry.sql.goservice/policy/db/key_management.sql.goservice/policy/db/models.goservice/policy/db/namespaces.goservice/policy/db/namespaces.sql.goservice/policy/db/obligations.goservice/policy/db/obligations.sql.goservice/policy/db/queries/attributes.sqlservice/policy/db/queries/key_access_server_registry.sqlservice/policy/db/queries/namespaces.sqlservice/policy/db/queries/obligations.sqlservice/policy/db/queries/registered_resources.sqlservice/policy/db/queries/subject_mappings.sqlservice/policy/db/registered_resources.goservice/policy/db/registered_resources.sql.goservice/policy/db/resource_mapping.sql.goservice/policy/db/subject_mappings.goservice/policy/db/subject_mappings.sql.goservice/policy/db/utils.goservice/policy/db/utils_test.goservice/policy/kasregistry/key_access_server_registry_keys_test.goservice/policy/kasregistry/key_access_server_registry_test.goservice/policy/namespaces/namespaces_test.goservice/policy/obligations/obligations.protoservice/policy/obligations/obligations_test.goservice/policy/registeredresources/registered_resources.protoservice/policy/registeredresources/registered_resources_test.goservice/policy/selectors.protoservice/policy/subjectmapping/subject_condition_set_test.goservice/policy/subjectmapping/subject_mapping_test.go
💤 Files with no reviewable changes (3)
- service/policy/obligations/obligations.proto
- service/policy/registeredresources/registered_resources.proto
- docs/grpc/index.html
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
|
elizabethhealy
approved these changes
Jun 16, 2026
alkalescent
approved these changes
Jun 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
General goal
Search for policy objects with a
Listreq by specifying a Searchterm. The searchtermis simply that, a word or phrase. We do this by using theLIKEorILIKEcommand depending on the specific RPC.The following sanitization is done for each query:
LIKE\ILIKEsuch as%and_are escaped before being queried.Note
Currently there is no goal to optimize this strategy, this is to serve as a starting point. In the case optimizations are needed
we can look into adding GIN indexes and
pg_trgmfor better fuzziness matching.Implementations
ListNamespacesattribute_fqns.fqn)ListAttributesattribute_fqns.fqn)ListKeyAccessServerskey_access_servers.name), KAS URI (key_access_servers.uri)ListKeyskas_keys.key_id)ListObligations<namespace fqn>/obl/<obligation name>)ListRegisteredResourcesregistered_resources.name)ListSubjectMappingsattribute_fqns.fqn), metadata label values (metadata.labels.*)ListSubjectConditionSetsmetadata.labels.*)Summary by CodeRabbit
New Features
Documentation
Tests