[Security Solution] [Detections] iterate over default timestamp and optional override in DE loop#83481
[Security Solution] [Detections] iterate over default timestamp and optional override in DE loop#83481dhurley14 wants to merge 32 commits intoelastic:masterfrom
Conversation
d711c91 to
9d8555c
Compare
...ck/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts
Outdated
Show resolved
Hide resolved
01aae86 to
9d30d52
Compare
58c797b to
e15fbcc
Compare
...ck/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts
Outdated
Show resolved
Hide resolved
4101fa7 to
eeba7a6
Compare
|
@elasticmachine merge upstream |
fa3828e to
46ffc5c
Compare
|
@elasticmachine merge upstream |
8f84d48 to
46426b4
Compare
… no override is provided
…index data structure.
…o ensure we do not blow up the UI by trying to display too many index names in the partial error banner
…ivileges which are needed for the api getFieldMapping, adds error handling for when user is missing monitor privileges and monitor cluster privileges for checking cat.indices api
4b97a81 to
ebcff6a
Compare
💚 Build SucceededMetrics [docs]Distributable file count
History
To update your PR or re-run it, just comment with: |
marshallmain
left a comment
There was a problem hiding this comment.
Converting the list of index patterns to a list of concrete indices will cause rules to fail on clusters that have rolled indices over enough times, which is likely on large or long running clusters.

The maximum size of 4096 bytes is exceeded with ~130 rollovers of the .siem-signals-default index. With 7 default index patterns, we could only support ~20 concrete indices per pattern before a rule using the default patterns would break, depending on the length of each index name.
I think we should explore options to do the multiple timestamp querying without resorting to expanding the index patterns out into concrete indices. We should be able to build elasticsearch DSL queries that don't break if the index patterns include indices that are missing the expected timestamp field, and then we can build a fallback query that looks for documents that use @timestamp instead of the timestamp override if provided.
| }; | ||
| } | ||
| return toReturn; | ||
| } catch (exc) { |
There was a problem hiding this comment.
If any of the index patterns don't have associated indices then the request 404s and prevents the rest of the mappings from being checked. We can solve this by catching the error inside the loop. Probably want to use Promise.all instead of the loop here though, similar to in getIndexesMatchingIndexPatterns
| // timestampsAndIndices can be empty if the rule author doesn't have | ||
| // view_index_metadata privileges and we can't access field mappings api | ||
| const indexPatternsToSearch = !isEmpty(timestampsAndIndices) | ||
| ? Object.keys(timestampsAndIndices[timestamp]) |
There was a problem hiding this comment.
The @timestamp key is removed from timestampsAndIndices if all indices have the timestamp override but we still iterate over the whole timestampsToSort array in the top level loop. The result is timestampsAndIndices[timestamp] here can be undefined, which causes Object.keys to throw an error and fail the rule.
|
The issue of expanding too many concrete indices into the query requires a large refactor (#83481 (review)). Closing this PR until I come up with a better solution. |
Summary
Resolves #75382
Partially dependent on #84293[Merged ✅]Lots of edge cases to cover in this one so please take your time reviewing.
We now allow for index patterns that match indices with either the optionally provided timestamp override field or the default
@timestampto be used simultaneously when searching for events against our rules. Because some indices in a given index pattern may not have the timestamp override and/or@timestampfield, we now display a "partial failure" status for a rule to show when some indices were missing this and could not be searched against.For this partial failure to be its most useful, the rule author should have at a minimum
view_index_metadatafor each index the rule is searching against. To provide more detailed messages it is also suggested for the rule author to havemonitorprivileges for all indices andmonitorfor the cluster as well. Without themonitorprivilege we cannot provide detailed messages on which specific indices matching the given index patterns are missing the timestamp override field /@timestamp.Partial failure example setup
Using the following indices
Indices
and a rule that searched
*:*against the index patternmyfa*which matches all three indices above, we are able to search and produce signals againstmyfakeindex-2andmyfakeindex-3thus the partial failure where we cannot search againstmyfakeindex-1because it is missing both the timestamp override and the default@timestampfield from its mapping.Screenshot partial failure
Full failure example setup
I deleted the indices
myfakeindex-2andmyfakeindex-3so that onlymyfakeindex-1was left matching themyfa*index pattern, which did not have the timestamp override field nor the@timestampfield. A situation like this would generate the following error:Screenshot full failure
Full failure no view_index_metadata screenshot
The following is the screenshot a user will see if the rule author did not have
view_index_metadataprivileges for the indices the rule searched againstScreenshot
Checklist
Delete any items that are not applicable to this PR.
For maintainers