Un-revert "Siem query rule - reduce field_caps usage"#186317
Un-revert "Siem query rule - reduce field_caps usage"#186317lukasolson merged 2 commits intoelastic:mainfrom
Conversation
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
mattkime
left a comment
There was a problem hiding this comment.
Changes lgtm, the additional unit tests are a nice touch. I'm relying on @elastic/security-detection-engine to verify the functionality of their tests.
We should also add a functional test for a lucene query in discover but that's best done as a follow up.
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Page load bundle
To update your PR or re-run it, just comment with: cc @lukasolson |
| let fields = dataView.timeFieldName ? [dataView.timeFieldName] : []; | ||
| if (sort) { | ||
| const sortArr = Array.isArray(sort) ? sort : [sort]; | ||
| fields.push(...sortArr.flatMap((s) => Object.keys(s))); | ||
| } | ||
| for (const query of request.query) { | ||
| if (query.query && query.language === 'kuery') { | ||
| const nodes = fromKueryExpression(query.query); | ||
| const queryFields = getKqlFieldNames(nodes); | ||
| fields = fields.concat(queryFields); | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: if the let usage is only due to the concat call at the end, maybe that can be replaced with push(...queryFields) and make this a const
| let fields = dataView.timeFieldName ? [dataView.timeFieldName] : []; | |
| if (sort) { | |
| const sortArr = Array.isArray(sort) ? sort : [sort]; | |
| fields.push(...sortArr.flatMap((s) => Object.keys(s))); | |
| } | |
| for (const query of request.query) { | |
| if (query.query && query.language === 'kuery') { | |
| const nodes = fromKueryExpression(query.query); | |
| const queryFields = getKqlFieldNames(nodes); | |
| fields = fields.concat(queryFields); | |
| } | |
| } | |
| const fields = dataView.timeFieldName ? [dataView.timeFieldName] : []; | |
| if (sort) { | |
| const sortArr = Array.isArray(sort) ? sort : [sort]; | |
| fields.push(...sortArr.flatMap((s) => Object.keys(s))); | |
| } | |
| for (const query of request.query) { | |
| if (query.query && query.language === 'kuery') { | |
| const nodes = fromKueryExpression(query.query); | |
| const queryFields = getKqlFieldNames(nodes); | |
| fields.push(...queryFields); | |
| } | |
| } |
There was a problem hiding this comment.
Although this nit makes sense, I prefer to leave this PR as close to its original counterpart (#184890) as possible
dej611
left a comment
There was a problem hiding this comment.
Left a minor code review, not a blocker for the PR.
Approve.
dhurley14
left a comment
There was a problem hiding this comment.
Tested locally with one of the original offending pre-built rules Cobalt Strike Command and Control Beacon and that generated an alert. This LGTM! Glad the fix was a simple one-liner.
Summary
#184890 was reverted in #186196 because it contained a bug with alerts created using Lucene queries. The bug was fixed in #186217. This PR un-reverts the original changes and preserves the fix. It also adds unit tests to cover the failed cases.
Checklist