Relax test-rules required field contract for presentation-only fields#220
Conversation
…les required-field contract to match the relaxed rule output schema.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dff4b4fe46
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "issueType", | ||
| "keyExpectedValue", | ||
| "keyActualValue", | ||
| }, |
There was a problem hiding this comment.
When backend rules omit these fields (the schema-relaxation case this change is intended to allow), runtime output is not actually fieldless: a missing issueType is defaulted to IncorrectValue (pkg/engine/inspector.go:55, pkg/engine/vulnerability_builder.go:175-177), and missing expected/actual values become empty strings that reports render directly (for example pkg/report/template/html/report.tmpl:104-105 and pkg/report/model/csv.go:59-64). Relaxing the test-rules contract here lets rules pass CI while producing mislabeled issue types and blank Expected/Found text in scanner outputs, so keep these required for report-backed outputs or add builder/report fallbacks before removing the validation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
we are removing them, and we actually removed them from the rules so it's alright
2 participants
Motivation
Rule results no longer emit
issueType,keyExpectedValue, orkeyActualValueas of a schema relaxation applied to the rule assets (equivalent of the contract change already shipped in the rule-assets repo). Thetest-rulesvalidator still listed them as required fields, causing everytest-rulesCI run to fail for Ansible and other rules that dropped those optional presentation fields.Changes
Rule test validator
Removed
issueType,keyExpectedValue, andkeyActualValuefrom theresultShape.Requiredlist incmd/scanner/test_rules.go. TheallowedIssueTypesguard on theissueTypevalue is kept, so results that do include the field are still validated; only the presence check is relaxed.Author Checklist
QA Instruction
Run
./bin/datadog-iac-scanner test-rules(with a valid API key) and confirm that Ansible AWS rules no longer produceResult missing required fieldfailures. Alternatively, let thetest-rulesCI job on this PR go green.Blast Radius
Only the
test-rulescommand is affected. No change to scan output, findings, SARIF, or any runtime path.Additional Notes
N/A
I submit this contribution under the Apache-2.0 license.