[Security Solution] Implement normalization of ruleSource for API responses #188631
Conversation
jpdjere
added
release_note:skip
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
| export const normalizeRuleSource = ({ | ||
| immutable, | ||
| ruleSource, | ||
| }: NormalizeRuleSourceParams): RuleSource => { | ||
| if (!ruleSource) { | ||
| const normalizedRuleSource = immutable | ||
| ? { | ||
| type: 'external', | ||
| isCustomized: false, | ||
| } | ||
| : { | ||
| type: 'internal', | ||
| }; | ||
|
|
||
| return convertObjectKeysToSnakeCase(normalizedRuleSource) as RuleSource; | ||
| } | ||
| return convertObjectKeysToSnakeCase(ruleSource) as RuleSource; | ||
| }; |
There was a problem hiding this comment.
@xcrzx I remember you mentioning handling possible data inconsistencies. For example, data in ES being for whatever reason:
{
"immutable": false,
"ruleSource": {
"type": "external",
"isCustomized": true
}
}
It's hard to me to think where these inconsitencies might arise from, but do you think it makes sense to rely always on immutable to calculate rule_source?
In the case above, calculating rule_source to be:
{
"type": "internal"
}
Or if in ES data looks like:
{
"immutable": true,
"ruleSource": {
"type": "internal",
}
}
calculating rule_source to be:
{
"type": "external",
"is_customized": false
}
WDYT?
There was a problem hiding this comment.
I think what you've implemented is correct. Relying on immutable in responses is not always possible because we also need to return isCustomized, which is calculated on writes. If the ruleSource field is available, we can return it as is. There's synchronization logic implemented on write, so ruleSource should always match immutable if present.
| rule_source: normalizeRuleSource({ | ||
| immutable: params.immutable, | ||
| ruleSource: params.ruleSource, | ||
| }), |
There was a problem hiding this comment.
We should not be normalizing data in case converters, as these functions are solely responsible for converting from camel case to snake case and vice versa. I think what you need is a converter from the alerting rule type to the rule response type.
There was a problem hiding this comment.
Right, thanks.
Created a normalization function to all params, applied in internalRuleToAPIResponse.
|
Hi @e40pud. Small change to a file owned by the DE triggered your review request 🙏 Thanks! |
⏳ Build in-progress, with failures
Failed CI StepsTest Failures
History
cc @jpdjere |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
To update your PR or re-run it, just comment with: cc @jpdjere |
Fixes: #180140
Summary
rule_sourcefor API responsesrule_sourcefield in API responses is calculated out of theimmutableandruleSourcefields.For maintainers