-
Notifications
You must be signed in to change notification settings - Fork 65
feat/add filter v2 support #560
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
base: main
Are you sure you want to change the base?
Conversation
abhisek
commented
Aug 11, 2025
- feat: Add support for filter v2
- Add filter v2 support
- Add test for filter v2 evaluator
- fix: CEL v2 query engine
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #560 +/- ##
========================================
+ Coverage 8.23% 8.38% +0.14%
========================================
Files 284 290 +6
Lines 47501 48103 +602
========================================
+ Hits 3914 4033 +119
- Misses 43314 43784 +470
- Partials 273 286 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for filter v2 capabilities by introducing new policy-based filtering with Insights v2 data model while maintaining backward compatibility with existing filters.
- Adds new
--filter-v2
,--filter-v2-suite
,--policy
, and--policy-suite
command-line flags - Implements policy evaluation system using CEL expressions with Insights v2 data model
- Creates new analyzer components for v2 filtering with comprehensive test coverage
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
scan.go | Adds new CLI flags for filter v2 and policy options with validation |
query.go | Adds corresponding query command flags and renames variables for consistency |
pkg/analyzer/filterv2/result.go | Defines filter evaluation result structure |
pkg/analyzer/filterv2/program.go | Implements filter program wrapper for CEL evaluation |
pkg/analyzer/filterv2/eval_test.go | Provides test coverage for the filter evaluator |
pkg/analyzer/filterv2/eval.go | Core CEL evaluator implementation with policy input processing |
pkg/analyzer/cel_filter_v2.go | Single filter v2 analyzer implementation |
pkg/analyzer/cel_filter_suite_v2.go | Filter suite v2 analyzer implementation |
pkg/analyzer/cel_filter_suite.go | Minor formatting improvements to existing filter suite |
pkg/analyzer/filterv2/eval.go
Outdated
filterEvalMaxRules = 150 | ||
) | ||
|
||
var errMaxFilter = errors.New("max filter limit has reached") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message should be grammatically correct: "max filter limit has been reached" instead of "max filter limit has reached".
var errMaxFilter = errors.New("max filter limit has reached") | |
var errMaxFilter = errors.New("max filter limit has been reached") |
Copilot uses AI. Check for mistakes.
break | ||
} | ||
|
||
if iter.HasNext().Value() == false { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Use !iter.HasNext().Value()
instead of == false
for better readability and Go idioms.
if iter.HasNext().Value() == false { | |
if !iter.HasNext().Value() { |
Copilot uses AI. Check for mistakes.
|
||
return ret, nil | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This TODO comment indicates a known inefficiency with JSON serialization round-trip. Consider creating a GitHub issue to track this technical debt or implement the direct Protobuf configuration.
// Protobuf messages are now passed directly to CEL; JSON round-trip removed. |
Copilot uses AI. Check for mistakes.
252a529
to
ff01901
Compare