Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Add alerts-as-data index names as alias to signals indices #119921

Merged
merged 2 commits into from
Dec 1, 2021

Conversation

marshallmain
Copy link
Contributor

@marshallmain marshallmain commented Nov 30, 2021

Summary

Re-adds the index alias to .siem-signals index template and updates the SIGNALS_TEMPLATE_VERSION. Updating the version will cause the create_index_route to be called, applying the alias to existing indices as well as rolling over the index after updating the template.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@marshallmain marshallmain added Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Alerts Security Detection Alerts Area Team v8.0.0 v8.1.0 release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed labels Nov 30, 2021
@marshallmain marshallmain marked this pull request as ready for review November 30, 2021 04:43
@marshallmain marshallmain requested a review from a team as a code owner November 30, 2021 04:43
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@marshallmain marshallmain requested a review from a team November 30, 2021 04:43
@spong
Copy link
Member

spong commented Nov 30, 2021

In testing I noticed Severity/Risk Score were not displaying for pre-8.x alerts within the Alerts Table. At first I thought this was a field aliasing issue, but as discussed the aliases are correct, we just need to 1. be writing to the new fields in addition to the old fields (i.e. both kibana.alert.risk_score & kibana.alert.rule.risk_score), and 2. update the default columns to be use kibana.alert.* fields instead of kibana.alert.rule.*.

Default columns:
Alerts_-_Kibana

Adding other Severity/Risk Score columns:
Pasted_Image_11_30_21__4_44_PM

if (await templateNeedsUpdate({ alias: index, esClient })) {
await esClient.indices.putIndexTemplate({
name: index,
body: getSignalsTemplate(index) as Record<string, unknown>,
body: getSignalsTemplate(index, aadIndexAliasName) as Record<string, unknown>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't been following the latest Rule Preview efforts closely so just curious here, but why are we setting up the preview index with aliases to the AAD index? Looks like it's getting setup with the signals' fieldAliases as well (within getSignalsTemplate). Is this for querying compatibility with existing components?

Copy link
Contributor Author

@marshallmain marshallmain Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need the index alias, I just wanted to avoid having conditional logic in getSignalsTemplate and this route is going away entirely in #116374 (it never shipped in 7.16 so we can remove this route without issue). Since we definitely do need the alias for the "regular" .siem-signals template, I didn't want to make the "alias" parameter optional and then have more confusion about when it is/isn't needed.

After #116374, the preview index/template logic will be handled by the RuleDataService.

The field aliases do matter though, as attempting to write to a field that is defined as an alias in the mapping will result in an error. This means for the new AAD indices since the signal.* fields are still defined (as aliases now) we'll still have conflicts if we try to copy user's source data into those fields.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, perfect -- thanks for explaining! 🙂

Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked out, performed upgrade test locally, and confirmed aliases are now appropriately set post-upgrade. Pre-8.x alerts now show in the Security Alerts table, and querying against the .alerts-security.alerts* Data View within Discover shows documents from the .siem-signals-* index as well.

Thanks for the fix and cleanup around spaceId here @marshallmain! 🙂 LGTM! 👍 🚀

Note: One nit on 'mis-match' of field aliases resulting in pre-8.x alerts not showing Severity/Risk Score. Chatted w/ Marshall and this will be addressed in another PR (write to new kibana.alert.* fields, and update the Alert Table's default columns).

@marshallmain marshallmain merged commit 194d5a9 into elastic:main Dec 1, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 1, 2021
…s indices (elastic#119921)

* Add alerts-as-data index names as alias to signals indices

* Update jest snapshot
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
8.0

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Dec 1, 2021
…s indices (#119921) (#120046)

* Add alerts-as-data index names as alias to signals indices

* Update jest snapshot

Co-authored-by: Marshall Main <[email protected]>
TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021
…s indices (elastic#119921)

* Add alerts-as-data index names as alias to signals indices

* Update jest snapshot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Detection Alerts Security Detection Alerts Area Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants