Fix #606: Refactor NewEvaluator to use Option pattern and add defensive nil checks (#607)

* Initial plan

* Refactor NewEvaluator to use Option pattern and add defensive nil checks

- Implemented Option function type for configuring evaluator
- Added WithIgnoreError option function
- Updated NewEvaluator to accept only name parameter and variadic options
- Updated all usages in pkg/analyzer/filterv2/ and pkg/analyzer/ directories
- Added defensive nil checks for enum constants in EvaluatePackage
- Added comprehensive tests for Option pattern
- All tests pass successfully

Co-authored-by: abhisek <31844+abhisek@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: abhisek <31844+abhisek@users.noreply.github.com>
This commit is contained in:
Copilot 2025-10-02 12:28:25 +05:30 committed by GitHub
parent 3eb501c72b
commit 8671c25fb5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 70 additions and 9 deletions

View File

@ -21,7 +21,7 @@ type celFilterSuiteV2Analyzer struct {
}
func NewCelFilterSuiteV2Analyzer(filePath string, failOnMatch bool) (Analyzer, error) {
evaluator, err := filterv2.NewEvaluator("filter-suite-v2", true)
evaluator, err := filterv2.NewEvaluator("filter-suite-v2", filterv2.WithIgnoreError(true))
if err != nil {
return nil, err
}

View File

@ -21,7 +21,7 @@ type celFilterV2Analyzer struct {
}
func NewCelFilterV2Analyzer(fl string, failOnMatch bool) (Analyzer, error) {
evaluator, err := filterv2.NewEvaluator("single-filter-v2", true)
evaluator, err := filterv2.NewEvaluator("single-filter-v2", filterv2.WithIgnoreError(true))
if err != nil {
return nil, fmt.Errorf("failed to create policy evaluator: %w", err)
}

View File

@ -53,8 +53,18 @@ type filterEvaluator struct {
var _ Evaluator = (*filterEvaluator)(nil)
// Option is a function type that can be used to configure the evaluator
type Option func(*filterEvaluator)
// WithIgnoreError configures the evaluator to ignore errors during evaluation
func WithIgnoreError(ignore bool) Option {
return func(f *filterEvaluator) {
f.ignoreError = ignore
}
}
// NewEvaluator creates a new CEL evaluator for the policy system v2
func NewEvaluator(name string, ignoreError bool) (*filterEvaluator, error) {
func NewEvaluator(name string, opts ...Option) (*filterEvaluator, error) {
env, err := cel.NewEnv(
cel.Macros(cel.StandardMacros...),
cel.EnableMacroCallTracking(),
@ -99,12 +109,19 @@ func NewEvaluator(name string, ignoreError bool) (*filterEvaluator, error) {
return nil, fmt.Errorf("failed to create CEL environment: %w", err)
}
return &filterEvaluator{
evaluator := &filterEvaluator{
name: name,
env: env,
programs: []*FilterProgram{},
ignoreError: ignoreError,
}, nil
ignoreError: false, // default value
}
// Apply options
for _, opt := range opts {
opt(evaluator)
}
return evaluator, nil
}
func (f *filterEvaluator) AddRule(policy *policyv1.Policy, rule *policyv1.Rule) error {
@ -162,14 +179,27 @@ func (f *filterEvaluator) EvaluatePackage(pkg *models.Package) (*FilterEvaluatio
// Get enum constants
enumConstants := getEnumConstantsMap()
// Defensive nil checks for enum constants
projectSourceType := enumConstants["ProjectSourceType"]
if projectSourceType == nil {
logger.Warnf("ProjectSourceType enum constants not found, using empty map")
projectSourceType = map[string]int64{}
}
ecosystem := enumConstants["Ecosystem"]
if ecosystem == nil {
logger.Warnf("Ecosystem enum constants not found, using empty map")
ecosystem = map[string]int64{}
}
// Create evaluation input map with protobuf messages directly and enum constants
evalInputMap := map[string]any{
policyInputVarRoot: policyInput,
policyInputVarPackage: policyInput.GetPackage(),
policyInputVarProject: policyInput.GetProject(),
policyInputVarManifest: policyInput.GetManifest(),
"ProjectSourceType": enumConstants["ProjectSourceType"],
"Ecosystem": enumConstants["Ecosystem"],
"ProjectSourceType": projectSourceType,
"Ecosystem": ecosystem,
}
for _, prog := range f.programs {

View File

@ -72,7 +72,7 @@ func TestEvaluator_AddRule(t *testing.T) {
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
evaluator, err := NewEvaluator("test", true)
evaluator, err := NewEvaluator("test", WithIgnoreError(true))
assert.NoError(t, err)
assert.NotNil(t, evaluator)
@ -92,3 +92,34 @@ func TestEvaluator_AddRule(t *testing.T) {
})
}
}
func TestEvaluator_Options(t *testing.T) {
t.Run("Default ignoreError is false", func(t *testing.T) {
evaluator, err := NewEvaluator("test")
assert.NoError(t, err)
assert.NotNil(t, evaluator)
assert.False(t, evaluator.ignoreError)
})
t.Run("WithIgnoreError sets ignoreError to true", func(t *testing.T) {
evaluator, err := NewEvaluator("test", WithIgnoreError(true))
assert.NoError(t, err)
assert.NotNil(t, evaluator)
assert.True(t, evaluator.ignoreError)
})
t.Run("WithIgnoreError sets ignoreError to false", func(t *testing.T) {
evaluator, err := NewEvaluator("test", WithIgnoreError(false))
assert.NoError(t, err)
assert.NotNil(t, evaluator)
assert.False(t, evaluator.ignoreError)
})
t.Run("Multiple options can be applied", func(t *testing.T) {
evaluator, err := NewEvaluator("test", WithIgnoreError(true))
assert.NoError(t, err)
assert.NotNil(t, evaluator)
assert.True(t, evaluator.ignoreError)
assert.Equal(t, "test", evaluator.name)
})
}