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

[Response Ops][Alerting] Using alertsClient for legacy siem notification rule types to write default alerts-as-data docs #174553

Merged
merged 8 commits into from
Jan 15, 2024

Conversation

ymao1
Copy link
Contributor

@ymao1 ymao1 commented Jan 9, 2024

Towards https://github.com/elastic/response-ops-team/issues/164
Resolves #171795

Summary

  • Switches this rule type to use alertsClient from alerting framework in favor of the deprecated alertFactory
  • Defines the default alert config for these rule types so framework level fields will be written out into the .alerts-default.alerts-default index with no rule type specific fields.
  • Updated some terminology from alert to rule

To Verify

  • Follow the instructions in this PR to add a legacy notification to a detection rule.
  • Verify the notification fires as expected
  • Verify an alert document is written to .alerts-default.alerts-default that looks like:
{
    "kibana.alert.rule.category": "Security Solution notification (Legacy)",
    "kibana.alert.rule.consumer": "siem",
    "kibana.alert.rule.execution.uuid": "cbad59ec-2a6e-4791-81c3-ae0fefd3d48a",
    "kibana.alert.rule.name": "Legacy notification with one action",
    "kibana.alert.rule.parameters": {
        "ruleAlertId": "9c07db42-b5fa-4ef9-8d7e-48d5688fd88e"
    },
    "kibana.alert.rule.producer": "siem",
    "kibana.alert.rule.rule_type_id": "siem.notifications",
    "kibana.alert.rule.tags": [],
    "kibana.alert.rule.uuid": "1869763e-c6e7-47fd-8275-0c9568127d84",
    "kibana.space_ids": [
        "default"
    ],
    "@timestamp": "2024-01-10T18:12:02.433Z",
    "event.action": "close",
    "event.kind": "signal",
    "kibana.alert.action_group": "recovered",
    "kibana.alert.flapping_history": [
        true,
        true,
        false,
        false
    ],
    "kibana.alert.instance.id": "1869763e-c6e7-47fd-8275-0c9568127d84",
    "kibana.alert.maintenance_window_ids": [],
    "kibana.alert.status": "recovered",
    "kibana.alert.uuid": "119269e0-a767-43c9-b383-a8840b4dddd5",
    "kibana.alert.workflow_status": "open",
    "kibana.alert.start": "2024-01-10T18:08:53.373Z",
    "kibana.alert.time_range": {
        "gte": "2024-01-10T18:08:53.373Z",
        "lte": "2024-01-10T18:09:56.367Z"
    },
    "kibana.version": "8.13.0",
    "tags": [],
    "kibana.alert.duration.us": 62994000,
    "kibana.alert.end": "2024-01-10T18:09:56.367Z",
    "kibana.alert.rule.revision": 0,
    "kibana.alert.flapping": false
}

@ymao1 ymao1 self-assigned this Jan 9, 2024
@ymao1 ymao1 added Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Feature:Alerting/Alerts-as-Data Issues related to Alerts-as-data and RuleRegistry v8.13.0 Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Jan 9, 2024
@@ -46,7 +46,7 @@ import {
} from '../../../../../common/api/detection_engine/signals_migration/mocks';

// eslint-disable-next-line no-restricted-imports
import type { LegacyRuleNotificationAlertType } from '../../rule_actions_legacy';
import type { LegacyRuleNotificationRuleType } from '../../rule_actions_legacy';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

did some alert to rule renaming

@ymao1 ymao1 force-pushed the aad-legacy-siem-notifications branch from 3e9a3ab to 94271d8 Compare January 10, 2024 15:56
@ymao1
Copy link
Contributor Author

ymao1 commented Jan 10, 2024

/ci

@ymao1 ymao1 marked this pull request as ready for review January 10, 2024 20:42
@ymao1 ymao1 requested review from a team as code owners January 10, 2024 20:42
@ymao1 ymao1 requested a review from WafaaNasr January 10, 2024 20:42
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@vitaliidm vitaliidm self-requested a review January 11, 2024 08:44
Copy link
Contributor

@ersin-erdal ersin-erdal left a comment

Choose a reason for hiding this comment

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

Tested locally and alerts are written in .internal.alerts-security.alerts-default not .alerts-default.alerts-default.

LGTM If above is expected.

Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

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

I did some testing and discovered the following behaviour:

Generated alert is displayed in alert table now and when clicking on the rule title it says rule is deleted

Screenshot 2024-01-12 at 16 32 57 Screenshot 2024-01-12 at 16 33 18

Which is not deleted, but displayed in that way because it's not proper Security rule.
We do not show this legacy notification rile in UI at all. And now when alert is generated and exposed in UI, similarly legacy rule is exposed as well.

Is there a way for us to not show generated alert in alerts table?

results_link:
'/app/security/detections/rules/id/rule-id?timerange=(global:(linkTo:!(timeline),timerange:(from:1576255233400,kind:absolute,to:1576341633400)),timeline:(linkTo:!(global),timerange:(from:1576255233400,kind:absolute,to:1576341633400)))',
rule: {
alert_suppression: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like almost all properties of this large rule object are identical in 4 tests cases. Is it possible to retrieve them in one variable and then just spread in different properties.

for example:

rule: {
...ruleMock
  meta: {
              kibana_siem_app_url: 'http://localhost',
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved in this commit: c90430a

@ymao1 ymao1 requested a review from a team as a code owner January 12, 2024 19:07
@ymao1
Copy link
Contributor Author

ymao1 commented Jan 12, 2024

Is there a way for us to not show generated alert in alerts table?

@vitaliidm Thanks for your review! I pushed a commit to exclude the legacy rule type from being displayed.

@ymao1 ymao1 requested a review from vitaliidm January 12, 2024 19:08
Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

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

Thanks for implementing the feedback

No legacy alerts displayed in alerts table anymore

@ymao1
Copy link
Contributor Author

ymao1 commented Jan 15, 2024

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #7 / useGetCaseMetrics calls getSingleCaseMetrics with correct arguments

Metrics [docs]

Unknown metric groups

References to deprecated APIs

id before after diff
securitySolution 520 519 -1

History

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

cc @ymao1

@ymao1 ymao1 merged commit 803298a into elastic:main Jan 15, 2024
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jan 15, 2024
@ymao1 ymao1 deleted the aad-legacy-siem-notifications branch January 15, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Alerting/Alerts-as-Data Issues related to Alerts-as-data and RuleRegistry Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.13.0
Projects
No open projects
6 participants