Skip to content

Commit f27f6dd

Browse files
committed
refactor(policies): simplify findings/violations handling after review
- Fix behavioral divergence between Rego and WASM engines for empty findings: both now use len>0 check consistently - Gracefully handle missing violations when findings is absent (no default findings array required in policies) - Pre-allocate violations slice in WASM engine - Trim redundant deprecation comments Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
1 parent af3b233 commit f27f6dd

2 files changed

Lines changed: 11 additions & 20 deletions

File tree

pkg/policies/engine/rego/rego.go

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -210,11 +210,9 @@ func parseResultRule(res rego.ResultSet, policy *engine.Policy, rawData *engine.
210210
result.SkipReason = reason
211211
result.Ignore = ignore
212212

213-
// Try "findings" first (new format, structured objects).
214-
// When present it fully replaces "violations", providing backward
215-
// compatibility: old CLIs that only understand string violations
216-
// ignore the unknown "findings" key.
217-
if findingsRaw, ok := ruleResult["findings"].([]any); ok {
213+
// Prefer non-empty "findings" (structured objects) over "violations" for backward compatibility.
214+
findingsRaw, _ := ruleResult["findings"].([]any)
215+
if len(findingsRaw) > 0 {
218216
for _, f := range findingsRaw {
219217
obj, ok := f.(map[string]any)
220218
if !ok {
@@ -226,20 +224,14 @@ func parseResultRule(res rego.ResultSet, policy *engine.Policy, rawData *engine.
226224
}
227225
result.Violations = append(result.Violations, pv)
228226
}
229-
} else {
230-
// Fallback: violations (strings or structured objects).
231-
// Structured objects in violations are deprecated — use findings instead.
232-
violations, ok := ruleResult["violations"].([]any)
233-
if !ok {
234-
return nil, engine.ResultFormatError{Field: "violations"}
235-
}
227+
} else if violations, ok := ruleResult["violations"].([]any); ok {
228+
// Fallback: violations (strings or deprecated structured objects).
229+
// TODO: remove structured object support once policies are fully migrated to findings.
236230
for _, violation := range violations {
237231
switch v := violation.(type) {
238232
case string:
239233
result.Violations = append(result.Violations, &engine.PolicyViolation{Subject: policy.Name, Violation: v})
240234
case map[string]any:
241-
// Deprecated: structured objects in violations will be removed in a future release.
242-
// Migrate to the "findings" field instead.
243235
pv, err := engine.NewStructuredViolation(policy.Name, v)
244236
if err != nil {
245237
return nil, fmt.Errorf("structured violation in policy %q: %w", policy.Name, err)
@@ -250,6 +242,7 @@ func parseResultRule(res rego.ResultSet, policy *engine.Policy, rawData *engine.
250242
}
251243
}
252244
}
245+
// If neither findings nor violations is present, result has zero violations.
253246
}
254247
}
255248

pkg/policies/engine/wasm/engine.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -206,11 +206,11 @@ func (e *Engine) Verify(ctx context.Context, policy *engine.Policy, input []byte
206206
Skipped: result.Skipped,
207207
SkipReason: result.SkipReason,
208208
Ignore: result.Ignore,
209-
Violations: make([]*engine.PolicyViolation, 0),
210209
}
211210

212-
// "findings" (structured objects) takes precedence over "violations"
211+
// Prefer non-empty "findings" (structured objects) over "violations" for backward compatibility.
213212
if len(result.Findings) > 0 {
213+
evalResult.Violations = make([]*engine.PolicyViolation, 0, len(result.Findings))
214214
for _, raw := range result.Findings {
215215
pv, err := parseWasmFinding(policy.Name, raw)
216216
if err != nil {
@@ -219,8 +219,8 @@ func (e *Engine) Verify(ctx context.Context, policy *engine.Policy, input []byte
219219
evalResult.Violations = append(evalResult.Violations, pv)
220220
}
221221
} else {
222-
// Fallback: violations (strings or structured objects).
223-
// Structured objects in violations are deprecated — use findings instead.
222+
// Fallback: violations (strings or deprecated structured objects).
223+
evalResult.Violations = make([]*engine.PolicyViolation, 0, len(result.Violations))
224224
for _, raw := range result.Violations {
225225
pv, err := parseWasmViolation(policy.Name, raw)
226226
if err != nil {
@@ -267,8 +267,6 @@ func parseWasmViolation(policyName string, raw json.RawMessage) (*engine.PolicyV
267267
Violation: typed,
268268
}, nil
269269
case map[string]any:
270-
// Deprecated: structured objects in violations will be removed in a future release.
271-
// Migrate to the "findings" field instead.
272270
return engine.NewStructuredViolation(policyName, typed)
273271
default:
274272
return nil, fmt.Errorf("violation must be a string or object, got %T", v)

0 commit comments

Comments
 (0)