[Alerting] System action types and helpers by cnasikas · Pull Request #167871 · elastic/kibana

cnasikas · 2023-10-03T12:18:39Z

In this PR:

Creation of types for the system actions
Creation of a helper function to detect if it is a system action or not
Use the isSystemAction in the executor to determine if an action is a system action
Pass the isSystemConnector utility function from the actions plugin to the rules factory
Create test utils to help test system actions and connector adapters

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2023-10-03T12:18:42Z

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

elasticmachine · 2023-10-03T12:18:43Z

Pinging @elastic/response-ops-cases (Feature:Cases)

cnasikas · 2023-10-03T12:31:54Z

x-pack/plugins/alerting/server/rules_client_factory.ts

        }
        return { apiKeysEnabled: false };
      },
+      isSystemAction(actionId: string) {


I named it isSystemAction and not isSystemActionConnector because the checks in the alerting plugin will be done upon rule actions.

cnasikas · 2023-10-03T12:33:28Z

x-pack/plugins/alerting/server/task_runner/execution_handler.ts

  isSummaryActionThrottled,
 } from './rule_action_helper';
 import { ConnectorAdapter } from '../connector_adapters/types';
+import { isSystemAction } from '../../common/system_actions/is_system_action';


Should we use actions.isSystemActionConnector (exposed by the rule factory as isSystemAction) instead of using that helper function that checks only the type?

guskovaue · 2023-10-13T12:36:26Z

x-pack/plugins/alerting/server/rules_client/types.ts


-export type NormalizedAlertAction = Omit<RuleAction, 'actionTypeId'>;
+export type NormalizedAlertAction = DistributiveOmit<RuleAction, 'actionTypeId'>;
+export type NormalizedSystemAction = Omit<RuleSystemAction, 'actionTypeId'>;


@cnasikas Why in one case you use DistributiveOmit and in another Omit?

The reason is that the RuleAction is the union RuleDefaultAction | RuleSystemAction. Normal Omit does not work with unions. If you need to remove a property from each item in the union you need to use DistributiveOmit. For RuleSystemAction is not needed because it is not a union.

x-pack/plugins/alerting/common/rule.ts

guskovaue · 2023-10-17T09:22:38Z

x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts

@@ -125,7 +125,7 @@ interface AlertsFilterAttributes {

 export interface RuleActionAttributes {


Why we did not create here two types(system and default) instead of one?

Because this type represents what is being saved in ES. The data in ES should not have the action's type so there is not need to distinguish (I don't think it is possible without the type) the two actions.

Zacqary · 2023-10-18T21:16:08Z

x-pack/plugins/alerting/server/rules_client/lib/get_alert_from_raw.ts

Note that we're deprecating getAlertFromRaw in favor of transformRuleDomainToRuleAttributes, so let's make sure these changes are reflected there as well

I think that analog of old getAlertFromRaw is a new transformRuleAttributesToRuleDomain. I think I've already added the smae type guard there as well: x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.ts line 157.
Isn't what you meant?

Thanks for the feedback! This PR is the first of many. I modified transformRuleDomainToRuleAttributes in another PR https://github.com/elastic/kibana/pull/167884/files#diff-25071c31329de46443821b36fad591c9a13261fb61990cb1b3747aef429b808eR22. For transformRuleAttributesToRuleDomain in the same PR here https://github.com/elastic/kibana/pull/167884/files#diff-d5aa343d106a834626d085c1fd6e4a083b05af194c5c8a886c93ab316d4919f8R152. To keep the size small I choose to not do all the changes. My initial PR tried to accommodate everything and it got too big. I closed it in favor of smaller PRs that deliberately will not pass the CI and will get merged in a feature branch.

@cnasikas I did not recognise your intention from the beginning and started to fix all types :-)
Now I see that you made some of this changes in follow up PRs. So I've deleted my commit for this PR.

kibana-ci · 2023-10-19T13:00:56Z

💔 Build Failed

Failed CI Steps

Test Failures

[job] [logs] x-pack/test/alerting_api_integration/security_and_spaces/group2/config_non_dedicated_task_runner.ts / alerting api integration security and spaces enabled - Group 2 Alerts alerts alerts connector adapters should use connector adapters correctly on system actions
[job] [logs] x-pack/test/alerting_api_integration/security_and_spaces/group2/config.ts / alerting api integration security and spaces enabled - Group 2 Alerts alerts alerts connector adapters should use connector adapters correctly on system actions
[job] [logs] x-pack/test/alerting_api_integration/security_and_spaces/group2/config_non_dedicated_task_runner.ts / alerting api integration security and spaces enabled - Group 2 Alerts alerts alerts connector adapters should use connector adapters correctly on system actions
[job] [logs] x-pack/test/alerting_api_integration/security_and_spaces/group2/config.ts / alerting api integration security and spaces enabled - Group 2 Alerts alerts alerts connector adapters should use connector adapters correctly on system actions
[job] [logs] FTR Configs #11 / dashboard app - group 5 embed mode default URL params renders as expected
[job] [logs] FTR Configs #11 / dashboard app - group 5 embed mode default URL params renders as expected
[job] [logs] Jest Integration Tests #7 / Fleet preconfiguration reset Preconfigured cloud policy With a full preconfigured cloud policy Create correct .fleet-policies

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [b064eba]

History

💔 Build #169004 failed 984b0f8f161b6ae2faf9d07a42f81d3629a19bdc
💔 Build #165927 failed 54fd67a
💔 Build #164843 failed 3bda9b9

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

cc @cnasikas

…#167884) ## Summary This PR enables system actions only to the Create Rule API. Other PRs will follow on a subsequent PR. Depends on: #167871 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Julia <iuliia.guskova@elastic.co>

…API (#168226) Summarize your PR. If it involves visual changes include a screenshot or gif. Depends on: #167871, #167884 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Julia Guskova <iuliia.guskova@elastic.co>

cnasikas added 5 commits October 3, 2023 15:01

Init system action types

6ec30eb

Create isSystemAction helper

40306ce

Use the isSystemAction helper in the executor

0fab8a8

Pass the isSystemAction to the rules factory

5024714

Add test utils for system actions

3bda9b9

cnasikas self-assigned this Oct 3, 2023

cnasikas requested a review from a team as a code owner October 3, 2023 12:18

cnasikas changed the base branch from main to system_actions_mvp October 3, 2023 12:18

cnasikas added the release_note:skip Skip the PR/issue when compiling release notes label Oct 3, 2023

cnasikas commented Oct 3, 2023

View reviewed changes

Merge branch 'system_actions_mvp' into sa_executor_types

54fd67a

This was referenced Oct 6, 2023

[Alerting] Enable creating system actions through the Create Rule API #167884

Merged

[Alerting] Enable updating system actions through the Bulk Edit Rule API #168226

Merged

guskovaue reviewed Oct 13, 2023

View reviewed changes

guskovaue reviewed Oct 17, 2023

View reviewed changes

x-pack/plugins/alerting/common/rule.ts Show resolved Hide resolved

guskovaue reviewed Oct 17, 2023

View reviewed changes

guskovaue approved these changes Oct 18, 2023

View reviewed changes

Zacqary reviewed Oct 18, 2023

View reviewed changes

guskovaue force-pushed the sa_executor_types branch from 984b0f8 to 54fd67a Compare October 19, 2023 12:46

guskovaue merged commit 6923ae2 into elastic:system_actions_mvp Oct 19, 2023

guskovaue mentioned this pull request Oct 30, 2023

Enable disabling system actions through the Bulk Disable Rule API #170097

Closed

2 tasks

		@@ -125,7 +125,7 @@ interface AlertsFilterAttributes {

		export interface RuleActionAttributes {

Conversation

cnasikas commented Oct 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

For maintainers

Uh oh!

elasticmachine commented Oct 3, 2023

Uh oh!

elasticmachine commented Oct 3, 2023

Uh oh!

cnasikas Oct 3, 2023

Choose a reason for hiding this comment

Uh oh!

cnasikas Oct 3, 2023

Choose a reason for hiding this comment

Uh oh!

guskovaue Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

cnasikas Oct 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

guskovaue Oct 17, 2023

Choose a reason for hiding this comment

Uh oh!

cnasikas Oct 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zacqary Oct 18, 2023

Choose a reason for hiding this comment

Uh oh!

guskovaue Oct 19, 2023

Choose a reason for hiding this comment

Uh oh!

cnasikas Oct 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guskovaue Oct 19, 2023

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented Oct 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💔 Build Failed

Failed CI Steps

Test Failures

Metrics [docs]

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cnasikas commented Oct 3, 2023 •

edited

Loading

cnasikas Oct 13, 2023 •

edited

Loading

cnasikas Oct 17, 2023 •

edited

Loading

cnasikas Oct 19, 2023 •

edited

Loading

kibana-ci commented Oct 19, 2023 •

edited

Loading