From b310c8aca58c6cd57bdd747b3f30fc17f9282e18 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 3 Oct 2023 14:14:16 +0300 Subject: [PATCH 01/14] Support system actions in the create rule API --- .../routes/rule/apis/create/schemas/v1.ts | 2 +- x-pack/plugins/alerting/common/rule.ts | 24 +- .../rule/methods/create/create_rule.test.ts | 779 +++++++++++------- .../rule/methods/create/create_rule.ts | 22 +- .../create/schemas/create_rule_data_schema.ts | 47 +- .../rule/schemas/action_schemas.ts | 30 +- ...form_raw_actions_to_domain_actions.test.ts | 103 +++ ...transform_raw_actions_to_domain_actions.ts | 64 ++ ...orm_rule_attributes_to_rule_domain.test.ts | 138 ++++ ...ransform_rule_attributes_to_rule_domain.ts | 30 +- .../validate_rule_action_params.test.ts | 92 +++ .../validate_rule_action_params.ts | 58 ++ .../server/data/rule/types/rule_attributes.ts | 25 +- .../lib/validate_system_actions.test.ts | 187 +++++ .../server/lib/validate_system_actions.ts | 59 ++ .../apis/create/create_rule_route.test.ts | 254 +++++- .../rule/apis/create/create_rule_route.ts | 7 +- .../v1.test.ts | 104 +++ .../transform_rule_to_rule_response/v1.ts | 40 +- .../rules_client/common/inject_references.ts | 4 +- .../alerting/server/rules_client/types.ts | 27 +- .../server/rules_client_factory.test.ts | 8 +- .../alerting/server/rules_client_factory.ts | 13 +- x-pack/plugins/alerting/server/types.ts | 11 +- 24 files changed, 1707 insertions(+), 421 deletions(-) create mode 100644 x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.test.ts create mode 100644 x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.ts create mode 100644 x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.test.ts create mode 100644 x-pack/plugins/alerting/server/connector_adapters/validate_rule_action_params.test.ts create mode 100644 x-pack/plugins/alerting/server/connector_adapters/validate_rule_action_params.ts create mode 100644 x-pack/plugins/alerting/server/lib/validate_system_actions.test.ts create mode 100644 x-pack/plugins/alerting/server/lib/validate_system_actions.ts create mode 100644 x-pack/plugins/alerting/server/routes/rule/transforms/transform_rule_to_rule_response/v1.test.ts diff --git a/x-pack/plugins/alerting/common/routes/rule/apis/create/schemas/v1.ts b/x-pack/plugins/alerting/common/routes/rule/apis/create/schemas/v1.ts index 98d82abf62be4f..498e1501ae0338 100644 --- a/x-pack/plugins/alerting/common/routes/rule/apis/create/schemas/v1.ts +++ b/x-pack/plugins/alerting/common/routes/rule/apis/create/schemas/v1.ts @@ -71,7 +71,7 @@ export const actionAlertsFilterSchema = schema.object({ export const actionSchema = schema.object({ uuid: schema.maybe(schema.string()), - group: schema.string(), + group: schema.maybe(schema.string()), id: schema.string(), actionTypeId: schema.maybe(schema.string()), params: schema.recordOf(schema.string(), schema.maybe(schema.any()), { defaultValue: {} }), diff --git a/x-pack/plugins/alerting/common/rule.ts b/x-pack/plugins/alerting/common/rule.ts index 63bd8dd4f8f4a7..065816c321abab 100644 --- a/x-pack/plugins/alerting/common/rule.ts +++ b/x-pack/plugins/alerting/common/rule.ts @@ -82,13 +82,13 @@ export interface RuleExecutionStatus { export type RuleActionParams = SavedObjectAttributes; export type RuleActionParam = SavedObjectAttribute; -export interface RuleActionFrequency extends SavedObjectAttributes { +export interface RuleActionFrequency { summary: boolean; notifyWhen: RuleNotifyWhenType; throttle: string | null; } -export interface AlertsFilterTimeframe extends SavedObjectAttributes { +export interface AlertsFilterTimeframe { days: IsoWeekday[]; timezone: string; hours: { @@ -97,7 +97,7 @@ export interface AlertsFilterTimeframe extends SavedObjectAttributes { }; } -export interface AlertsFilter extends SavedObjectAttributes { +export interface AlertsFilter { query?: { kql: string; filters: Filter[]; @@ -121,7 +121,7 @@ export const RuleActionTypes = { export type RuleActionTypes = typeof RuleActionTypes[keyof typeof RuleActionTypes]; -export interface RuleAction { +export interface RuleDefaultAction { uuid?: string; group: string; id: string; @@ -129,9 +129,19 @@ export interface RuleAction { params: RuleActionParams; frequency?: RuleActionFrequency; alertsFilter?: AlertsFilter; - type?: typeof RuleActionTypes.DEFAULT; + type: typeof RuleActionTypes.DEFAULT; } +export interface RuleSystemAction { + uuid?: string; + id: string; + actionTypeId: string; + params: RuleActionParams; + type: typeof RuleActionTypes.SYSTEM; +} + +export type RuleAction = RuleDefaultAction | RuleSystemAction; + export interface RuleLastRun { outcome: RuleLastRunOutcomes; outcomeOrder?: number; @@ -195,10 +205,12 @@ export interface SanitizedAlertsFilter extends AlertsFilter { timeframe?: AlertsFilterTimeframe; } -export type SanitizedRuleAction = Omit & { +export type SanitizedDefaultRuleAction = Omit & { alertsFilter?: SanitizedAlertsFilter; }; +export type SanitizedRuleAction = SanitizedDefaultRuleAction | RuleSystemAction; + export type SanitizedRule = Omit< Rule, 'apiKey' | 'actions' diff --git a/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.test.ts b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.test.ts index bf6218614206c8..bc3a9c1f39a689 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.test.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.test.ts @@ -24,10 +24,12 @@ import { ruleNotifyWhen } from '../../constants'; import { TaskStatus } from '@kbn/task-manager-plugin/server'; import { auditLoggerMock } from '@kbn/security-plugin/server/audit/mocks'; import { getBeforeSetup, setGlobalDate } from '../../../../rules_client/tests/lib'; -import { RecoveredActionGroup } from '../../../../../common'; +import { RecoveredActionGroup, RuleActionTypes, RuleSystemAction } from '../../../../../common'; import { bulkMarkApiKeysForInvalidation } from '../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation'; import { getRuleExecutionStatusPending, getDefaultMonitoring } from '../../../../lib'; import { ConnectorAdapterRegistry } from '../../../../connector_adapters/connector_adapter_registry'; +import { ConnectorAdapter } from '../../../../connector_adapters/types'; +import { RuleDomain } from '../../types'; jest.mock('../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation', () => ({ bulkMarkApiKeysForInvalidation: jest.fn(), @@ -60,6 +62,7 @@ const authorization = alertingAuthorizationMock.create(); const actionsAuthorization = actionsAuthorizationMock.create(); const auditLogger = auditLoggerMock.create(); const internalSavedObjectsRepository = savedObjectsRepositoryMock.create(); +const connectorAdapterRegistry = new ConnectorAdapterRegistry(); const kibanaVersion = 'v8.0.0'; const rulesClientParams: jest.Mocked = { @@ -83,7 +86,6 @@ const rulesClientParams: jest.Mocked = { minimumScheduleInterval: { value: '1m', enforce: false }, isAuthenticationTypeAPIKey: jest.fn(), getAuthenticationAPIKey: jest.fn(), - connectorAdapterRegistry: new ConnectorAdapterRegistry(), getAlertIndicesAlias: jest.fn(), alertsService: null, }; @@ -119,6 +121,7 @@ function getMockData(overwrites: Record = {}): CreateRuleParams params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, ], ...overwrites, @@ -152,6 +155,9 @@ describe('create()', () => { isSystemAction: false, }, ]); + + actionsClient.isSystemAction.mockImplementation((id: string) => id === 'system_action-id'); + taskManager.schedule.mockResolvedValue({ id: 'task-123', taskType: 'alerting:123', @@ -165,6 +171,7 @@ describe('create()', () => { params: {}, ownerId: null, }); + rulesClientParams.getActionsClient.mockResolvedValue(actionsClient); }); @@ -191,6 +198,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -345,12 +353,14 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, }, ], }; + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', @@ -367,6 +377,7 @@ describe('create()', () => { }, ], }); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', @@ -384,13 +395,16 @@ describe('create()', () => { }, ], }); + const result = await rulesClient.create({ data }); + expect(authorization.ensureAuthorized).toHaveBeenCalledWith({ entity: 'rule', consumer: 'bar', operation: 'create', ruleTypeId: '123', }); + expect(result).toMatchInlineSnapshot(` Object { "actions": Array [ @@ -401,6 +415,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid", }, ], "alertTypeId": "123", @@ -576,6 +592,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -640,6 +657,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -744,6 +762,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, { group: 'default', @@ -751,6 +770,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, { group: 'default', @@ -758,6 +778,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, ], }); @@ -817,6 +838,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -825,6 +847,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_1', actionTypeId: 'test', + uuid: 'test-uuid-1', params: { foo: true, }, @@ -833,6 +856,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_2', actionTypeId: 'test2', + uuid: 'test-uuid-2', params: { foo: true, }, @@ -877,6 +901,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid", }, Object { "actionTypeId": "test", @@ -885,6 +911,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid-1", }, Object { "actionTypeId": "test2", @@ -893,6 +921,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid-2", }, ], "alertTypeId": "123", @@ -925,6 +955,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, { group: 'default', @@ -932,6 +963,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, { group: 'default', @@ -939,6 +971,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, ], }); @@ -1018,6 +1051,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -1026,6 +1060,7 @@ describe('create()', () => { group: 'default', actionRef: 'preconfigured:preconfigured', actionTypeId: 'test', + uuid: 'test-uuid-1', params: { foo: true, }, @@ -1034,6 +1069,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_2', actionTypeId: 'test2', + uuid: 'test-uuid-2', params: { foo: true, }, @@ -1074,6 +1110,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid", }, Object { "actionTypeId": "test", @@ -1082,6 +1120,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid-1", }, Object { "actionTypeId": "test2", @@ -1090,6 +1130,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid-2", }, ], "alertTypeId": "123", @@ -1183,265 +1225,6 @@ describe('create()', () => { expect(actionsClient.isPreconfigured).toHaveBeenCalledTimes(3); }); - test('creates a rule with some actions using system connectors', async () => { - const data = getMockData({ - actions: [ - { - group: 'default', - id: '1', - params: { - foo: true, - }, - }, - { - group: 'default', - id: 'system_action-id', - params: {}, - }, - { - group: 'default', - id: '2', - params: { - foo: true, - }, - }, - ], - }); - - actionsClient.getBulk.mockReset(); - actionsClient.getBulk.mockResolvedValue([ - { - id: '1', - actionTypeId: 'test', - config: { - from: 'me@me.com', - hasAuth: false, - host: 'hello', - port: 22, - secure: null, - service: null, - }, - isMissingSecrets: false, - name: 'email connector', - isPreconfigured: false, - isDeprecated: false, - isSystemAction: false, - }, - { - id: '2', - actionTypeId: 'test2', - config: { - from: 'me@me.com', - hasAuth: false, - host: 'hello', - port: 22, - secure: null, - service: null, - }, - isMissingSecrets: false, - name: 'another email connector', - isPreconfigured: false, - isDeprecated: false, - isSystemAction: false, - }, - { - id: 'system_action-id', - actionTypeId: 'test', - config: {}, - isMissingSecrets: false, - name: 'system action connector', - isPreconfigured: false, - isDeprecated: false, - isSystemAction: true, - }, - ]); - - actionsClient.isSystemAction.mockReset(); - actionsClient.isSystemAction.mockReturnValueOnce(false); - actionsClient.isSystemAction.mockReturnValueOnce(true); - actionsClient.isSystemAction.mockReturnValueOnce(false); - - unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ - id: '1', - type: 'alert', - attributes: { - executionStatus: getRuleExecutionStatusPending('2019-02-12T21:01:22.479Z'), - alertTypeId: '123', - schedule: { interval: '1m' }, - params: { - bar: true, - }, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - notifyWhen: null, - actions: [ - { - group: 'default', - actionRef: 'action_0', - actionTypeId: 'test', - params: { - foo: true, - }, - }, - { - group: 'default', - actionRef: 'system_action:system_action-id', - actionTypeId: 'test', - params: {}, - }, - { - group: 'default', - actionRef: 'action_2', - actionTypeId: 'test2', - params: { - foo: true, - }, - }, - ], - running: false, - }, - references: [ - { - name: 'action_0', - type: 'action', - id: '1', - }, - { - name: 'action_2', - type: 'action', - id: '2', - }, - ], - }); - - unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ - id: '1', - type: 'alert', - attributes: { - actions: [], - scheduledTaskId: 'task-123', - }, - references: [], - }); - - const result = await rulesClient.create({ data }); - - expect(result).toMatchInlineSnapshot(` - Object { - "actions": Array [ - Object { - "actionTypeId": "test", - "group": "default", - "id": "1", - "params": Object { - "foo": true, - }, - }, - Object { - "actionTypeId": "test", - "group": "default", - "id": "system_action-id", - "params": Object {}, - }, - Object { - "actionTypeId": "test2", - "group": "default", - "id": "2", - "params": Object { - "foo": true, - }, - }, - ], - "alertTypeId": "123", - "createdAt": 2019-02-12T21:01:22.479Z, - "executionStatus": Object { - "lastExecutionDate": 2019-02-12T21:01:22.000Z, - "status": "pending", - }, - "id": "1", - "notifyWhen": null, - "params": Object { - "bar": true, - }, - "running": false, - "schedule": Object { - "interval": "1m", - }, - "scheduledTaskId": "task-123", - "updatedAt": 2019-02-12T21:01:22.479Z, - } - `); - - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( - 'alert', - { - actions: [ - { - group: 'default', - actionRef: 'action_0', - actionTypeId: 'test', - params: { - foo: true, - }, - uuid: '111', - }, - { - group: 'default', - actionRef: 'system_action:system_action-id', - actionTypeId: 'test', - params: {}, - uuid: '112', - }, - { - group: 'default', - actionRef: 'action_2', - actionTypeId: 'test2', - params: { - foo: true, - }, - uuid: '113', - }, - ], - alertTypeId: '123', - apiKey: null, - apiKeyOwner: null, - apiKeyCreatedByUser: null, - consumer: 'bar', - createdAt: '2019-02-12T21:01:22.479Z', - createdBy: 'elastic', - enabled: true, - legacyId: null, - executionStatus: { - lastExecutionDate: '2019-02-12T21:01:22.479Z', - status: 'pending', - }, - monitoring: getDefaultMonitoring('2019-02-12T21:01:22.479Z'), - meta: { versionApiKeyLastmodified: kibanaVersion }, - muteAll: false, - snoozeSchedule: [], - mutedInstanceIds: [], - name: 'abc', - notifyWhen: null, - params: { bar: true }, - revision: 0, - running: false, - schedule: { interval: '1m' }, - tags: ['foo'], - throttle: null, - updatedAt: '2019-02-12T21:01:22.479Z', - updatedBy: 'elastic', - }, - { - id: 'mock-saved-object-id', - references: [ - { id: '1', name: 'action_0', type: 'action' }, - { id: '2', name: 'action_2', type: 'action' }, - ], - } - ); - expect(actionsClient.isSystemAction).toHaveBeenCalledTimes(3); - }); - test('creates a disabled alert', async () => { const data = getMockData({ enabled: false }); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ @@ -1464,6 +1247,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -1489,6 +1273,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid", }, ], "alertTypeId": "123", @@ -1547,7 +1333,6 @@ describe('create()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', useSavedObjectReferences: { extractReferences: extractReferencesFn, @@ -1556,7 +1341,6 @@ describe('create()', () => { validate: { params: { validate: (params) => params }, }, - validLegacyConsumers: [], })); const data = getMockData({ params: ruleParams, @@ -1581,6 +1365,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -1621,7 +1406,7 @@ describe('create()', () => { actionTypeId: 'test', group: 'default', params: { foo: true }, - uuid: '115', + uuid: '112', }, ], alertTypeId: '123', @@ -1679,6 +1464,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid", }, ], "alertTypeId": "123", @@ -1736,7 +1523,6 @@ describe('create()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', useSavedObjectReferences: { extractReferences: extractReferencesFn, @@ -1745,7 +1531,6 @@ describe('create()', () => { validate: { params: { validate: (params) => params }, }, - validLegacyConsumers: [], })); const data = getMockData({ params: ruleParams, @@ -1769,6 +1554,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -1810,7 +1596,7 @@ describe('create()', () => { actionTypeId: 'test', group: 'default', params: { foo: true }, - uuid: '116', + uuid: '113', }, ], alertTypeId: '123', @@ -1868,6 +1654,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid", }, ], "alertTypeId": "123", @@ -1914,6 +1702,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -1956,6 +1745,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -1985,7 +1775,7 @@ describe('create()', () => { group: 'default', actionTypeId: 'test', params: { foo: true }, - uuid: '118', + uuid: '115', }, ], alertTypeId: '123', @@ -2040,6 +1830,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid", }, ], "alertTypeId": "123", @@ -2097,6 +1889,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -2126,7 +1919,7 @@ describe('create()', () => { group: 'default', actionTypeId: 'test', params: { foo: true }, - uuid: '119', + uuid: '116', }, ], legacyId: null, @@ -2181,6 +1974,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid", }, ], "alertTypeId": "123", @@ -2238,6 +2033,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -2267,7 +2063,7 @@ describe('create()', () => { group: 'default', actionTypeId: 'test', params: { foo: true }, - uuid: '120', + uuid: '117', }, ], legacyId: null, @@ -2322,6 +2118,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid", }, ], "alertTypeId": "123", @@ -2387,6 +2185,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -2435,7 +2234,7 @@ describe('create()', () => { }, actionRef: 'action_0', actionTypeId: 'test', - uuid: '121', + uuid: '118', }, ], apiKeyOwner: null, @@ -2504,6 +2303,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid", }, ], "alertTypeId": "123", @@ -2564,9 +2365,7 @@ describe('create()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', - validLegacyConsumers: [], }); await expect(rulesClient.create({ data })).rejects.toThrowErrorMatchingInlineSnapshot( `"params invalid: [param1]: expected value of type [string] but got [undefined]"` @@ -2623,6 +2422,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -2672,6 +2472,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -2719,6 +2520,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -2777,6 +2579,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -2818,7 +2621,7 @@ describe('create()', () => { group: 'default', actionTypeId: 'test', params: { foo: true }, - uuid: '129', + uuid: '126', }, ], alertTypeId: '123', @@ -2880,6 +2683,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -2923,7 +2727,7 @@ describe('create()', () => { group: 'default', actionTypeId: 'test', params: { foo: true }, - uuid: '130', + uuid: '127', }, ], legacyId: null, @@ -3033,7 +2837,6 @@ describe('create()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3042,7 +2845,6 @@ describe('create()', () => { validate: { params: { validate: (params) => params }, }, - validLegacyConsumers: [], })); const createdAttributes = { ...data, @@ -3063,6 +2865,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -3107,7 +2910,6 @@ describe('create()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3116,7 +2918,6 @@ describe('create()', () => { validate: { params: { validate: (params) => params }, }, - validLegacyConsumers: [], })); const data = getMockData({ schedule: { interval: '1s' } }); @@ -3132,6 +2933,7 @@ describe('create()', () => { ...rulesClientParams, minimumScheduleInterval: { value: '1m', enforce: true }, }); + ruleTypeRegistry.get.mockImplementation(() => ({ id: '123', name: 'Test', @@ -3146,7 +2948,6 @@ describe('create()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3155,7 +2956,6 @@ describe('create()', () => { validate: { params: { validate: (params) => params }, }, - validLegacyConsumers: [], })); const data = getMockData({ @@ -3172,6 +2972,7 @@ describe('create()', () => { notifyWhen: 'onActionGroupChange', throttle: null, }, + type: RuleActionTypes.DEFAULT, }, { group: 'group2', @@ -3184,6 +2985,7 @@ describe('create()', () => { notifyWhen: 'onActionGroupChange', throttle: null, }, + type: RuleActionTypes.DEFAULT, }, ], }); @@ -3207,6 +3009,7 @@ describe('create()', () => { notifyWhen: 'onActionGroupChange', throttle: null, }, + type: RuleActionTypes.DEFAULT, }, { group: 'group2', @@ -3214,6 +3017,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, ], }); @@ -3240,7 +3044,6 @@ describe('create()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3249,7 +3052,6 @@ describe('create()', () => { validate: { params: { validate: (params) => params }, }, - validLegacyConsumers: [], })); const data = getMockData({ @@ -3262,6 +3064,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, ], }); @@ -3291,7 +3094,6 @@ describe('create()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3300,7 +3102,6 @@ describe('create()', () => { validate: { params: { validate: (params) => params }, }, - validLegacyConsumers: [], })); const data = getMockData({ @@ -3318,6 +3119,7 @@ describe('create()', () => { notifyWhen: 'onActionGroupChange', throttle: null, }, + type: RuleActionTypes.DEFAULT, }, { group: 'group2', @@ -3325,6 +3127,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, ], }); @@ -3355,7 +3158,6 @@ describe('create()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3364,7 +3166,6 @@ describe('create()', () => { validate: { params: { validate: (params) => params }, }, - validLegacyConsumers: [], })); const data = getMockData({ @@ -3383,6 +3184,7 @@ describe('create()', () => { notifyWhen: 'onThrottleInterval', throttle: '1h', }, + type: RuleActionTypes.DEFAULT, }, { group: 'group2', @@ -3395,6 +3197,7 @@ describe('create()', () => { notifyWhen: 'onThrottleInterval', throttle: '3m', }, + type: RuleActionTypes.DEFAULT, }, { group: 'group3', @@ -3407,6 +3210,7 @@ describe('create()', () => { notifyWhen: 'onThrottleInterval', throttle: '240m', }, + type: RuleActionTypes.DEFAULT, }, ], }); @@ -3437,7 +3241,6 @@ describe('create()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3446,7 +3249,6 @@ describe('create()', () => { validate: { params: { validate: (params) => params }, }, - validLegacyConsumers: [], })); const data = getMockData({ @@ -3465,6 +3267,7 @@ describe('create()', () => { notifyWhen: 'onThrottleInterval', throttle: '1h', }, + type: RuleActionTypes.DEFAULT, }, { group: 'group2', @@ -3477,6 +3280,7 @@ describe('create()', () => { notifyWhen: 'onThrottleInterval', throttle: '3m', }, + type: RuleActionTypes.DEFAULT, }, { group: 'group3', @@ -3484,6 +3288,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, ], }); @@ -3504,6 +3309,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, { group: 'default', @@ -3511,6 +3317,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, { group: 'default', @@ -3518,6 +3325,7 @@ describe('create()', () => { params: { foo: true, }, + type: RuleActionTypes.DEFAULT, }, ], }); @@ -3552,6 +3360,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: '.slack', + uuid: 'test-uuid', params: { foo: true, }, @@ -3596,6 +3405,8 @@ describe('create()', () => { "params": Object { "foo": true, }, + "type": "default", + "uuid": "test-uuid", }, ], "alertTypeId": "123", @@ -3638,7 +3449,6 @@ describe('create()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3652,7 +3462,6 @@ describe('create()', () => { mappings: { fieldMap: { field: { type: 'keyword', required: false } } }, shouldWrite: true, }, - validLegacyConsumers: [], })); const data = getMockData({ @@ -3671,11 +3480,12 @@ describe('create()', () => { throttle: '10h', }, alertsFilter: {}, + type: RuleActionTypes.DEFAULT, }, ], }); await expect(rulesClient.create({ data })).rejects.toThrowErrorMatchingInlineSnapshot( - `"Failed to validate actions due to the following error: Action's alertsFilter must have either \\"query\\" or \\"timeframe\\" : 152"` + `"Failed to validate actions due to the following error: Action's alertsFilter must have either \\"query\\" or \\"timeframe\\" : 149"` ); expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); expect(taskManager.schedule).not.toHaveBeenCalled(); @@ -3697,7 +3507,6 @@ describe('create()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3706,7 +3515,6 @@ describe('create()', () => { validate: { params: { validate: (params) => params }, }, - validLegacyConsumers: [], })); const data = getMockData({ @@ -3727,11 +3535,12 @@ describe('create()', () => { alertsFilter: { query: { kql: 'test:1', filters: [] }, }, + type: RuleActionTypes.DEFAULT, }, ], }); await expect(rulesClient.create({ data })).rejects.toThrowErrorMatchingInlineSnapshot( - `"Failed to validate actions due to the following error: This ruleType (Test) can't have an action with Alerts Filter. Actions: [153]"` + `"Failed to validate actions due to the following error: This ruleType (Test) can't have an action with Alerts Filter. Actions: [150]"` ); expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); expect(taskManager.schedule).not.toHaveBeenCalled(); @@ -3760,6 +3569,7 @@ describe('create()', () => { group: 'default', actionRef: 'action_0', actionTypeId: 'test', + uuid: 'test-uuid', params: { foo: true, }, @@ -3802,7 +3612,7 @@ describe('create()', () => { group: 'default', actionTypeId: 'test', params: { foo: true }, - uuid: '154', + uuid: '151', }, ], alertTypeId: '123', @@ -3869,4 +3679,381 @@ describe('create()', () => { expect.any(Object) ); }); + + describe('actions', () => { + const connectorAdapter: ConnectorAdapter = { + connectorTypeId: '.test', + ruleActionParamsSchema: schema.object({ foo: schema.string() }), + buildActionParams: jest.fn(), + }; + + connectorAdapterRegistry.register(connectorAdapter); + + beforeEach(() => { + actionsClient.getBulk.mockReset(); + actionsClient.getBulk.mockResolvedValue([ + { + id: '1', + actionTypeId: 'test', + config: { + from: 'me@me.com', + hasAuth: false, + host: 'hello', + port: 22, + secure: null, + service: null, + }, + isMissingSecrets: false, + name: 'email connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: false, + }, + { + id: 'system_action-id', + actionTypeId: '.test', + config: {}, + isMissingSecrets: false, + name: 'system action connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: true, + }, + ]); + + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'alert', + attributes: { + executionStatus: getRuleExecutionStatusPending('2019-02-12T21:01:22.479Z'), + alertTypeId: '123', + schedule: { interval: '1m' }, + params: { + bar: true, + }, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + notifyWhen: null, + actions: [ + { + group: 'default', + actionRef: 'action_0', + actionTypeId: 'test', + uuid: 'test-uuid', + params: { + foo: true, + }, + }, + { + group: 'default', + actionRef: 'system_action:system_action-id', + actionTypeId: 'test', + uuid: 'test-uuid-1', + params: { foo: 'test' }, + }, + ], + running: false, + }, + references: [ + { + name: 'action_0', + type: 'action', + id: '1', + }, + ], + }); + }); + + test('create a rule with system actions and default actions', async () => { + const data = getMockData({ + actions: [ + { + group: 'default', + id: '1', + params: { + foo: true, + }, + type: RuleActionTypes.DEFAULT, + }, + { + id: 'system_action-id', + params: { + foo: 'test', + }, + type: RuleActionTypes.SYSTEM, + }, + ], + }); + + const result = await rulesClient.create({ data }); + + expect(result).toMatchInlineSnapshot(` + Object { + "actions": Array [ + Object { + "actionTypeId": "test", + "group": "default", + "id": "1", + "params": Object { + "foo": true, + }, + "type": "default", + "uuid": "test-uuid", + }, + Object { + "actionTypeId": "test", + "id": "system_action-id", + "params": Object { + "foo": "test", + }, + "type": "system", + "uuid": "test-uuid-1", + }, + ], + "alertTypeId": "123", + "createdAt": 2019-02-12T21:01:22.479Z, + "executionStatus": Object { + "lastExecutionDate": 2019-02-12T21:01:22.000Z, + "status": "pending", + }, + "id": "1", + "notifyWhen": null, + "params": Object { + "bar": true, + }, + "running": false, + "schedule": Object { + "interval": "1m", + }, + "scheduledTaskId": "task-123", + "updatedAt": 2019-02-12T21:01:22.479Z, + } + `); + + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( + 'alert', + { + actions: [ + { + group: 'default', + actionRef: 'action_0', + actionTypeId: 'test', + params: { + foo: true, + }, + uuid: '153', + }, + { + actionRef: 'system_action:system_action-id', + actionTypeId: '.test', + params: { foo: 'test' }, + uuid: '154', + }, + ], + alertTypeId: '123', + apiKey: null, + apiKeyOwner: null, + apiKeyCreatedByUser: null, + consumer: 'bar', + createdAt: '2019-02-12T21:01:22.479Z', + createdBy: 'elastic', + enabled: true, + legacyId: null, + executionStatus: { + lastExecutionDate: '2019-02-12T21:01:22.479Z', + status: 'pending', + }, + monitoring: getDefaultMonitoring('2019-02-12T21:01:22.479Z'), + meta: { versionApiKeyLastmodified: kibanaVersion }, + muteAll: false, + snoozeSchedule: [], + mutedInstanceIds: [], + name: 'abc', + notifyWhen: null, + params: { bar: true }, + revision: 0, + running: false, + schedule: { interval: '1m' }, + tags: ['foo'], + throttle: null, + updatedAt: '2019-02-12T21:01:22.479Z', + updatedBy: 'elastic', + }, + { + id: 'mock-saved-object-id', + references: [{ id: '1', name: 'action_0', type: 'action' }], + } + ); + }); + + test('should construct the refs correctly and not persist the type of the action', async () => { + const data = getMockData({ + actions: [ + { + group: 'default', + id: '1', + params: { + foo: true, + }, + type: RuleActionTypes.DEFAULT, + }, + { + id: 'system_action-id', + params: { + foo: 'test', + }, + type: RuleActionTypes.SYSTEM, + }, + ], + }); + + await rulesClient.create({ data }); + + const rule = unsecuredSavedObjectsClient.create.mock.calls[0][1] as RuleDomain; + + expect(rule.actions).toEqual([ + { + group: 'default', + actionRef: 'action_0', + actionTypeId: 'test', + params: { + foo: true, + }, + uuid: '155', + }, + { + actionRef: 'system_action:system_action-id', + actionTypeId: '.test', + params: { foo: 'test' }, + uuid: '156', + }, + ]); + }); + + test('should add the actions type to the response correctly', async () => { + const data = getMockData({ + actions: [ + { + group: 'default', + id: '1', + params: { + foo: true, + }, + type: RuleActionTypes.DEFAULT, + }, + { + id: 'system_action-id', + params: { + foo: 'test', + }, + type: RuleActionTypes.SYSTEM, + }, + ], + }); + + const result = await rulesClient.create({ data }); + + expect(result.actions).toMatchInlineSnapshot(` + Array [ + Object { + "actionTypeId": "test", + "group": "default", + "id": "1", + "params": Object { + "foo": true, + }, + "type": "default", + "uuid": "test-uuid", + }, + Object { + "actionTypeId": "test", + "id": "system_action-id", + "params": Object { + "foo": "test", + }, + "type": "system", + "uuid": "test-uuid-1", + }, + ] + `); + }); + + test('should throw an error if the system action does not exist', async () => { + const action: RuleSystemAction = { + id: 'fake-system-action', + uuid: '123', + params: {}, + actionTypeId: '.test', + type: RuleActionTypes.SYSTEM, + }; + + const data = getMockData({ actions: [action] }); + await expect(() => rulesClient.create({ data })).rejects.toMatchInlineSnapshot( + `[Error: Action fake-system-action is not a system action]` + ); + + expect(actionsClient.getBulk).toBeCalledWith({ + ids: ['fake-system-action'], + throwIfSystemAction: false, + }); + }); + + test('should throw an error if the system action contains the frequency', async () => { + const action = { + id: 'system_action-id', + uuid: '123', + params: {}, + actionTypeId: '.test', + frequency: { + summary: false, + notifyWhen: 'onActionGroupChange', + throttle: null, + }, + type: RuleActionTypes.SYSTEM, + }; + + const data = getMockData({ actions: [action] }); + await expect(() => rulesClient.create({ data })).rejects.toMatchInlineSnapshot(` + [Error: Error validating create data - [actions.0]: types that failed validation: + - [actions.0.0.group]: expected value of type [string] but got [undefined] + - [actions.0.1.frequency]: definition for this key is missing] + `); + }); + + test('should throw an error if the system action contains the alertsFilter', async () => { + const action = { + id: 'system_action-id', + uuid: '123', + params: {}, + actionTypeId: '.test', + alertsFilter: { + query: { kql: 'test:1', filters: [] }, + }, + type: RuleActionTypes.SYSTEM, + }; + + const data = getMockData({ actions: [action] }); + await expect(() => rulesClient.create({ data })).rejects.toMatchInlineSnapshot(` + [Error: Error validating create data - [actions.0]: types that failed validation: + - [actions.0.0.group]: expected value of type [string] but got [undefined] + - [actions.0.1.alertsFilter]: definition for this key is missing] + `); + }); + + test('should throw an error if the default action does not contain the group', async () => { + const action = { + id: 'action-id-1', + params: {}, + actionTypeId: '.test', + type: RuleActionTypes.DEFAULT, + }; + + const data = getMockData({ actions: [action] }); + await expect(() => rulesClient.create({ data })).rejects.toMatchInlineSnapshot(` + [Error: Error validating create data - [actions.0]: types that failed validation: + - [actions.0.0.group]: expected value of type [string] but got [undefined] + - [actions.0.1.type]: expected value to equal [system]] + `); + }); + }); }); diff --git a/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts index 616a16a8315ed3..2a1f75d434e197 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts @@ -8,6 +8,7 @@ import Semver from 'semver'; import Boom from '@hapi/boom'; import { SavedObject, SavedObjectsUtils } from '@kbn/core/server'; import { withSpan } from '@kbn/apm-utils'; +import { validateSystemActions } from '../../../../lib/validate_system_actions'; import { parseDuration } from '../../../../../common/parse_duration'; import { WriteOperations, AlertingAuthorizationEntity } from '../../../../authorization'; import { @@ -25,7 +26,7 @@ import { generateAPIKeyName, apiKeyAsRuleDomainProperties } from '../../../../ru import { ruleAuditEvent, RuleAuditAction } from '../../../../rules_client/common/audit_events'; import { RulesClientContext } from '../../../../rules_client/types'; import { RuleDomain, RuleParams } from '../../types'; -import { SanitizedRule } from '../../../../types'; +import { RuleActionTypes, RuleSystemAction, SanitizedRule } from '../../../../types'; import { transformRuleAttributesToRuleDomain, transformRuleDomainToRuleAttributes, @@ -54,8 +55,16 @@ export async function createRule( // TODO (http-versioning): This should be of type Rule, change this when all rule types are fixed ): Promise> { const { data: initialData, options, allowMissingConnectorSecrets } = createParams; + const actionsClient = await context.getActionsClient(); - const data = { ...initialData, actions: addGeneratedActionValues(initialData.actions) }; + const systemActions = initialData.actions.filter( + (action): action is RuleSystemAction => action.type === RuleActionTypes.SYSTEM + ); + + const data = { + ...initialData, + actions: addGeneratedActionValues(initialData.actions), + }; const id = options?.id || SavedObjectsUtils.generateId(); @@ -71,6 +80,12 @@ export async function createRule( throw Boom.badRequest(`Error validating create data - ${error.message}`); } + await validateSystemActions({ + actionsClient, + connectorAdapterRegistry: context.connectorAdapterRegistry, + systemActions, + }); + try { await withSpan({ name: 'authorization.ensureAuthorized', type: 'rules' }, () => context.authorization.ensureAuthorized({ @@ -199,7 +214,8 @@ export async function createRule( logger: context.logger, ruleType: context.ruleTypeRegistry.get(createdRuleSavedObject.attributes.alertTypeId), references, - } + }, + (connectorId: string) => actionsClient.isSystemAction(connectorId) ); // Try to validate created rule, but don't throw. diff --git a/x-pack/plugins/alerting/server/application/rule/methods/create/schemas/create_rule_data_schema.ts b/x-pack/plugins/alerting/server/application/rule/methods/create/schemas/create_rule_data_schema.ts index ce51e88c5ce73c..4cdca61fec89b2 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/create/schemas/create_rule_data_schema.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/create/schemas/create_rule_data_schema.ts @@ -6,9 +6,35 @@ */ import { schema } from '@kbn/config-schema'; +import { RuleActionTypes } from '../../../../../../common'; import { validateDuration } from '../../../validation'; import { notifyWhenSchema, actionAlertsFilterSchema } from '../../../schemas'; +const defaultActionSchema = schema.object({ + group: schema.string(), + id: schema.string(), + actionTypeId: schema.maybe(schema.string()), + params: schema.recordOf(schema.string(), schema.maybe(schema.any()), { defaultValue: {} }), + frequency: schema.maybe( + schema.object({ + summary: schema.boolean(), + notifyWhen: notifyWhenSchema, + throttle: schema.nullable(schema.string({ validate: validateDuration })), + }) + ), + uuid: schema.maybe(schema.string()), + alertsFilter: schema.maybe(actionAlertsFilterSchema), + type: schema.literal(RuleActionTypes.DEFAULT), +}); + +export const systemActionSchema = schema.object({ + id: schema.string(), + actionTypeId: schema.maybe(schema.string()), + params: schema.recordOf(schema.string(), schema.maybe(schema.any()), { defaultValue: {} }), + uuid: schema.maybe(schema.string()), + type: schema.literal(RuleActionTypes.SYSTEM), +}); + export const createRuleDataSchema = schema.object({ name: schema.string(), alertTypeId: schema.string(), @@ -20,23 +46,8 @@ export const createRuleDataSchema = schema.object({ schedule: schema.object({ interval: schema.string({ validate: validateDuration }), }), - actions: schema.arrayOf( - schema.object({ - group: schema.string(), - id: schema.string(), - actionTypeId: schema.maybe(schema.string()), - params: schema.recordOf(schema.string(), schema.maybe(schema.any()), { defaultValue: {} }), - frequency: schema.maybe( - schema.object({ - summary: schema.boolean(), - notifyWhen: notifyWhenSchema, - throttle: schema.nullable(schema.string({ validate: validateDuration })), - }) - ), - uuid: schema.maybe(schema.string()), - alertsFilter: schema.maybe(actionAlertsFilterSchema), - }), - { defaultValue: [] } - ), + actions: schema.arrayOf(schema.oneOf([defaultActionSchema, systemActionSchema]), { + defaultValue: [], + }), notifyWhen: schema.maybe(schema.nullable(notifyWhenSchema)), }); diff --git a/x-pack/plugins/alerting/server/application/rule/schemas/action_schemas.ts b/x-pack/plugins/alerting/server/application/rule/schemas/action_schemas.ts index f123466eca1ab3..38f04823870c79 100644 --- a/x-pack/plugins/alerting/server/application/rule/schemas/action_schemas.ts +++ b/x-pack/plugins/alerting/server/application/rule/schemas/action_schemas.ts @@ -6,6 +6,7 @@ */ import { schema } from '@kbn/config-schema'; +import { RuleActionTypes } from '../../../../common'; import { notifyWhenSchema } from './notify_when_schema'; export const actionParamsSchema = schema.recordOf(schema.string(), schema.maybe(schema.any())); @@ -57,7 +58,7 @@ const actionFrequencySchema = schema.object({ /** * Unsanitized (domain) action schema, used by internal rules clients */ -export const actionDomainSchema = schema.object({ +export const defaultActionDomainSchema = schema.object({ uuid: schema.maybe(schema.string()), group: schema.string(), id: schema.string(), @@ -65,8 +66,22 @@ export const actionDomainSchema = schema.object({ params: actionParamsSchema, frequency: schema.maybe(actionFrequencySchema), alertsFilter: schema.maybe(actionDomainAlertsFilterSchema), + type: schema.literal(RuleActionTypes.DEFAULT), }); +export const systemActionDomainSchema = schema.object({ + id: schema.string(), + actionTypeId: schema.string(), + params: actionParamsSchema, + uuid: schema.maybe(schema.string()), + type: schema.literal(RuleActionTypes.SYSTEM), +}); + +export const actionDomainSchema = schema.oneOf([ + defaultActionDomainSchema, + systemActionDomainSchema, +]); + /** * Sanitized (non-domain) action schema, returned by rules clients for other solutions */ @@ -81,7 +96,7 @@ export const actionAlertsFilterSchema = schema.object({ timeframe: schema.maybe(actionAlertsFilterTimeFrameSchema), }); -export const actionSchema = schema.object({ +export const defaultActionSchema = schema.object({ uuid: schema.maybe(schema.string()), group: schema.string(), id: schema.string(), @@ -89,4 +104,15 @@ export const actionSchema = schema.object({ params: actionParamsSchema, frequency: schema.maybe(actionFrequencySchema), alertsFilter: schema.maybe(actionAlertsFilterSchema), + type: schema.literal(RuleActionTypes.DEFAULT), }); + +export const systemActionSchema = schema.object({ + id: schema.string(), + actionTypeId: schema.string(), + params: actionParamsSchema, + uuid: schema.maybe(schema.string()), + type: schema.literal(RuleActionTypes.SYSTEM), +}); + +export const actionSchema = schema.oneOf([defaultActionSchema, systemActionSchema]); diff --git a/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.test.ts b/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.test.ts new file mode 100644 index 00000000000000..eb9f21979e4fbd --- /dev/null +++ b/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.test.ts @@ -0,0 +1,103 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { RecoveredActionGroup } from '../../../../common'; +import { UntypedNormalizedRuleType } from '../../../rule_type_registry'; +import { RuleActionAttributes } from '../../../data/rule/types'; +import { transformRawActionsToDomainActions } from './transform_raw_actions_to_domain_actions'; + +const ruleType: jest.Mocked = { + id: 'test.rule-type', + name: 'My test rule', + actionGroups: [{ id: 'default', name: 'Default' }, RecoveredActionGroup], + defaultActionGroupId: 'default', + minimumLicenseRequired: 'basic', + isExportable: true, + recoveryActionGroup: RecoveredActionGroup, + executor: jest.fn(), + producer: 'alerts', + cancelAlertsOnRuleTimeout: true, + ruleTaskTimeout: '5m', + autoRecoverAlerts: true, + doesSetRecoveryContext: true, + validate: { + params: { validate: (params) => params }, + }, + alerts: { + context: 'test', + mappings: { fieldMap: { field: { type: 'keyword', required: false } } }, + shouldWrite: true, + }, +}; + +const defaultAction: RuleActionAttributes = { + group: 'default', + uuid: '1', + actionRef: 'default-action-ref', + actionTypeId: '.test', + params: {}, + frequency: { + summary: false, + notifyWhen: 'onThrottleInterval', + throttle: '1m', + }, + alertsFilter: { query: { kql: 'test:1', dsl: '{}', filters: [] } }, +}; + +const systemAction: RuleActionAttributes = { + actionRef: 'system_action:my-system-action-id', + uuid: '123', + actionTypeId: '.test-system-action', + params: {}, +}; + +const isSystemAction = (id: string) => id === 'my-system-action-id'; + +describe('transformRawActionsToDomainActions', () => { + it('transforms the actions correctly', () => { + const res = transformRawActionsToDomainActions({ + actions: [defaultAction, systemAction], + ruleId: 'test-rule', + references: [ + { name: 'system_action:my-system-action-id', id: 'my-system-action-id', type: 'action' }, + { name: 'default-action-ref', id: 'default-action-id', type: 'action' }, + ], + isSystemAction, + }); + + expect(res).toMatchInlineSnapshot(` + Array [ + Object { + "actionTypeId": ".test", + "alertsFilter": Object { + "query": Object { + "filters": Array [], + "kql": "test:1", + }, + }, + "frequency": Object { + "notifyWhen": "onThrottleInterval", + "summary": false, + "throttle": "1m", + }, + "group": "default", + "id": "default-action-id", + "params": Object {}, + "type": "default", + "uuid": "1", + }, + Object { + "actionTypeId": ".test-system-action", + "id": "my-system-action-id", + "params": Object {}, + "type": "system", + "uuid": "123", + }, + ] + `); + }); +}); diff --git a/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.ts b/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.ts new file mode 100644 index 00000000000000..d6c7533befdf10 --- /dev/null +++ b/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.ts @@ -0,0 +1,64 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { omit } from 'lodash'; +import { SavedObjectReference } from '@kbn/core/server'; +import { injectReferencesIntoActions } from '../../../rules_client/common'; +import { RuleAttributes } from '../../../data/rule/types'; +import { RawRule, RuleActionTypes } from '../../../types'; +import { RuleDomain } from '../types'; + +interface Args { + ruleId: string; + actions: RuleAttributes['actions'] | RawRule['actions']; + isSystemAction: (connectorId: string) => boolean; + omitGeneratedValues?: boolean; + references?: SavedObjectReference[]; +} + +export const transformRawActionsToDomainActions = ({ + actions, + ruleId, + references, + omitGeneratedValues = true, + isSystemAction, +}: Args) => { + const actionsWithInjectedRefs = actions + ? injectReferencesIntoActions(ruleId, actions, references || []) + : []; + + const ruleDomainActions: RuleDomain['actions'] = actionsWithInjectedRefs.map((action) => { + if (isSystemAction(action.id)) { + return { + id: action.id, + params: action.params, + actionTypeId: action.actionTypeId, + uuid: action.uuid, + type: RuleActionTypes.SYSTEM, + }; + } + + const defaultAction = { + group: action.group ?? 'default', + id: action.id, + params: action.params, + actionTypeId: action.actionTypeId, + uuid: action.uuid, + ...(action.frequency ? { frequency: action.frequency } : {}), + ...(action.alertsFilter ? { alertsFilter: action.alertsFilter } : {}), + type: RuleActionTypes.DEFAULT, + }; + + if (omitGeneratedValues) { + return omit(defaultAction, 'alertsFilter.query.dsl'); + } + + return defaultAction; + }); + + return ruleDomainActions; +}; diff --git a/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.test.ts b/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.test.ts new file mode 100644 index 00000000000000..a95ff00f45bd38 --- /dev/null +++ b/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.test.ts @@ -0,0 +1,138 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { RecoveredActionGroup } from '../../../../common'; +import { loggingSystemMock } from '@kbn/core-logging-server-mocks'; +import { transformRuleAttributesToRuleDomain } from './transform_rule_attributes_to_rule_domain'; +import { UntypedNormalizedRuleType } from '../../../rule_type_registry'; +import { RuleActionAttributes } from '../../../data/rule/types'; + +const ruleType: jest.Mocked = { + id: 'test.rule-type', + name: 'My test rule', + actionGroups: [{ id: 'default', name: 'Default' }, RecoveredActionGroup], + defaultActionGroupId: 'default', + minimumLicenseRequired: 'basic', + isExportable: true, + recoveryActionGroup: RecoveredActionGroup, + executor: jest.fn(), + producer: 'alerts', + cancelAlertsOnRuleTimeout: true, + ruleTaskTimeout: '5m', + autoRecoverAlerts: true, + doesSetRecoveryContext: true, + validate: { + params: { validate: (params) => params }, + }, + alerts: { + context: 'test', + mappings: { fieldMap: { field: { type: 'keyword', required: false } } }, + shouldWrite: true, + }, +}; + +const defaultAction: RuleActionAttributes = { + group: 'default', + uuid: '1', + actionRef: 'default-action-ref', + actionTypeId: '.test', + params: {}, + frequency: { + summary: false, + notifyWhen: 'onThrottleInterval', + throttle: '1m', + }, + alertsFilter: { query: { kql: 'test:1', dsl: '{}', filters: [] } }, +}; + +const systemAction: RuleActionAttributes = { + actionRef: 'system_action:my-system-action-id', + uuid: '123', + actionTypeId: '.test-system-action', + params: {}, +}; + +const isSystemAction = (id: string) => id === 'my-system-action-id'; + +describe('transformRuleAttributesToRuleDomain', () => { + const MOCK_API_KEY = Buffer.from('123:abc').toString('base64'); + const logger = loggingSystemMock.create().get(); + const references = [{ name: 'default-action-ref', type: 'action', id: 'default-action-id' }]; + + const rule = { + enabled: false, + tags: ['foo'], + createdBy: 'user', + createdAt: '2019-02-12T21:01:22.479Z', + updatedAt: '2019-02-12T21:01:22.479Z', + legacyId: null, + muteAll: false, + mutedInstanceIds: [], + snoozeSchedule: [], + alertTypeId: 'myType', + schedule: { interval: '1m' }, + consumer: 'myApp', + scheduledTaskId: 'task-123', + executionStatus: { + lastExecutionDate: '2019-02-12T21:01:22.479Z', + status: 'pending' as const, + }, + params: {}, + throttle: null, + notifyWhen: null, + actions: [defaultAction, systemAction], + name: 'my rule name', + revision: 0, + updatedBy: 'user', + apiKey: MOCK_API_KEY, + apiKeyOwner: 'user', + }; + + it('transforms the actions correctly', () => { + const res = transformRuleAttributesToRuleDomain( + rule, + { + id: '1', + logger, + ruleType, + references, + }, + isSystemAction + ); + + expect(res.actions).toMatchInlineSnapshot(` + Array [ + Object { + "actionTypeId": ".test", + "alertsFilter": Object { + "query": Object { + "filters": Array [], + "kql": "test:1", + }, + }, + "frequency": Object { + "notifyWhen": "onThrottleInterval", + "summary": false, + "throttle": "1m", + }, + "group": "default", + "id": "default-action-id", + "params": Object {}, + "type": "default", + "uuid": "1", + }, + Object { + "actionTypeId": ".test-system-action", + "id": "my-system-action-id", + "params": Object {}, + "type": "system", + "uuid": "123", + }, + ] + `); + }); +}); diff --git a/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.ts b/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.ts index 26831b9dff81c5..a90d1c900c6443 100644 --- a/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.ts +++ b/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.ts @@ -4,20 +4,18 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import { omit, isEmpty } from 'lodash'; +import { isEmpty } from 'lodash'; import { Logger } from '@kbn/core/server'; import { SavedObjectReference } from '@kbn/core/server'; import { ruleExecutionStatusValues } from '../constants'; import { getRuleSnoozeEndTime } from '../../../lib'; import { RuleDomain, Monitoring, RuleParams } from '../types'; import { RuleAttributes } from '../../../data/rule/types'; -import { RawRule, PartialRule } from '../../../types'; +import { PartialRule } from '../../../types'; import { UntypedNormalizedRuleType } from '../../../rule_type_registry'; -import { - injectReferencesIntoActions, - injectReferencesIntoParams, -} from '../../../rules_client/common'; +import { injectReferencesIntoParams } from '../../../rules_client/common'; import { getActiveScheduledSnoozes } from '../../../lib/is_rule_snoozed'; +import { transformRawActionsToDomainActions } from './transform_raw_actions_to_domain_actions'; const INITIAL_LAST_RUN_METRICS = { duration: 0, @@ -120,7 +118,8 @@ interface TransformEsToRuleParams { export const transformRuleAttributesToRuleDomain = ( esRule: RuleAttributes, - transformParams: TransformEsToRuleParams + transformParams: TransformEsToRuleParams, + isSystemAction: (connectorId: string) => boolean ): RuleDomain => { const { scheduledTaskId, executionStatus, monitoring, snoozeSchedule, lastRun } = esRule; @@ -141,6 +140,7 @@ export const transformRuleAttributesToRuleDomain = omit(ruleAction, 'alertsFilter.query.dsl')); - } + const ruleDomainActions: RuleDomain['actions'] = transformRawActionsToDomainActions({ + ruleId: id, + actions: esRule.actions, + references, + isSystemAction, + omitGeneratedValues, + }); const params = injectReferencesIntoParams( id, @@ -177,7 +177,7 @@ export const transformRuleAttributesToRuleDomain = { + const firstConnectorAdapter: ConnectorAdapter = { + connectorTypeId: '.test', + ruleActionParamsSchema: schema.object({ foo: schema.string() }), + buildActionParams: jest.fn(), + }; + + const secondConnectorAdapter: ConnectorAdapter = { + connectorTypeId: '.test-2', + ruleActionParamsSchema: schema.object({ bar: schema.string() }), + buildActionParams: jest.fn(), + }; + + let registry: ConnectorAdapterRegistry; + + beforeEach(() => { + registry = new ConnectorAdapterRegistry(); + }); + + describe('validateConnectorAdapterActionParams', () => { + it('should validate correctly invalid params', () => { + registry.register(firstConnectorAdapter); + + expect(() => + validateConnectorAdapterActionParams({ + connectorAdapterRegistry: registry, + connectorTypeId: firstConnectorAdapter.connectorTypeId, + params: { foo: 5 }, + }) + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid system action params. System action type: .test - [foo]: expected value of type [string] but got [number]"` + ); + }); + + it('should not throw if the connectorTypeId is not defined', () => { + registry.register(firstConnectorAdapter); + + expect(() => + validateConnectorAdapterActionParams({ + connectorAdapterRegistry: registry, + params: {}, + }) + ).not.toThrow(); + }); + + it('should not throw if the connector adapter is not registered', () => { + expect(() => + validateConnectorAdapterActionParams({ + connectorAdapterRegistry: registry, + connectorTypeId: firstConnectorAdapter.connectorTypeId, + params: {}, + }) + ).not.toThrow(); + }); + }); + + describe('bulkValidateConnectorAdapterActionParams', () => { + it('should validate correctly invalid params with multiple actions', () => { + const actions = [ + { actionTypeId: firstConnectorAdapter.connectorTypeId, params: { foo: 5 } }, + { actionTypeId: secondConnectorAdapter.connectorTypeId, params: { bar: 'test' } }, + ]; + + registry.register(firstConnectorAdapter); + registry.register(secondConnectorAdapter); + + expect(() => + bulkValidateConnectorAdapterActionParams({ + connectorAdapterRegistry: registry, + actions, + }) + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid system action params. System action type: .test - [foo]: expected value of type [string] but got [number]"` + ); + }); + }); +}); diff --git a/x-pack/plugins/alerting/server/connector_adapters/validate_rule_action_params.ts b/x-pack/plugins/alerting/server/connector_adapters/validate_rule_action_params.ts new file mode 100644 index 00000000000000..abfd56f0e90794 --- /dev/null +++ b/x-pack/plugins/alerting/server/connector_adapters/validate_rule_action_params.ts @@ -0,0 +1,58 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import Boom from '@hapi/boom'; +import { ConnectorAdapterRegistry } from './connector_adapter_registry'; + +interface ValidateSchemaArgs { + connectorAdapterRegistry: ConnectorAdapterRegistry; + connectorTypeId?: string; + params: Record; +} + +interface BulkValidateSchemaArgs { + connectorAdapterRegistry: ConnectorAdapterRegistry; + actions: Array<{ actionTypeId: string; params: Record }>; +} + +export const validateConnectorAdapterActionParams = ({ + connectorAdapterRegistry, + connectorTypeId, + params, +}: ValidateSchemaArgs) => { + if (!connectorTypeId) { + return; + } + + if (!connectorAdapterRegistry.has(connectorTypeId)) { + return; + } + + const connectorAdapter = connectorAdapterRegistry.get(connectorTypeId); + const schema = connectorAdapter.ruleActionParamsSchema; + + try { + schema.validate(params); + } catch (error) { + throw Boom.badRequest( + `Invalid system action params. System action type: ${connectorAdapter.connectorTypeId} - ${error.message}` + ); + } +}; + +export const bulkValidateConnectorAdapterActionParams = ({ + connectorAdapterRegistry, + actions, +}: BulkValidateSchemaArgs) => { + for (const action of actions) { + validateConnectorAdapterActionParams({ + connectorAdapterRegistry, + connectorTypeId: action.actionTypeId, + params: action.params, + }); + } +}; diff --git a/x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts b/x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts index aa8adda873cdec..239422d5ad28ae 100644 --- a/x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts +++ b/x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts @@ -7,6 +7,7 @@ import type { SavedObjectAttributes } from '@kbn/core/server'; import { Filter } from '@kbn/es-query'; +import type { WeekdayStr } from '@kbn/rrule'; import { IsoWeekday } from '../../../../common'; import { ruleNotifyWhenAttributes, @@ -15,7 +16,6 @@ import { ruleExecutionStatusErrorReasonAttributes, ruleExecutionStatusWarningReasonAttributes, } from '../constants'; -import { RRuleAttributes } from '../../r_rule/types'; export type RuleNotifyWhenAttributes = typeof ruleNotifyWhenAttributes[keyof typeof ruleNotifyWhenAttributes]; @@ -28,6 +28,27 @@ export type RuleExecutionStatusErrorReasonAttributes = export type RuleExecutionStatusWarningReasonAttributes = typeof ruleExecutionStatusWarningReasonAttributes[keyof typeof ruleExecutionStatusWarningReasonAttributes]; +type RRuleFreq = 0 | 1 | 2 | 3 | 4 | 5 | 6; + +export interface RRuleAttributes { + dtstart: string; + tzid: string; + freq?: RRuleFreq; + until?: string; + count?: number; + interval?: number; + wkst?: WeekdayStr; + byweekday?: Array; + bymonth?: number[]; + bysetpos?: number[]; + bymonthday: number[]; + byyearday: number[]; + byweekno: number[]; + byhour: number[]; + byminute: number[]; + bysecond: number[]; +} + export interface RuleSnoozeScheduleAttributes { duration: number; rRule: RRuleAttributes; @@ -125,7 +146,7 @@ interface AlertsFilterAttributes { export interface RuleActionAttributes { uuid: string; - group: string; + group?: string; actionRef: string; actionTypeId: string; params: SavedObjectAttributes; diff --git a/x-pack/plugins/alerting/server/lib/validate_system_actions.test.ts b/x-pack/plugins/alerting/server/lib/validate_system_actions.test.ts new file mode 100644 index 00000000000000..de3a45d5c09b96 --- /dev/null +++ b/x-pack/plugins/alerting/server/lib/validate_system_actions.test.ts @@ -0,0 +1,187 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { ActionsClient } from '@kbn/actions-plugin/server'; +import { actionsClientMock } from '@kbn/actions-plugin/server/mocks'; +import { schema } from '@kbn/config-schema'; +import { ConnectorAdapterRegistry } from '../connector_adapters/connector_adapter_registry'; +import { ConnectorAdapter } from '../connector_adapters/types'; +import { NormalizedSystemAction } from '../rules_client'; +import { RuleActionTypes, RuleSystemAction } from '../types'; +import { validateSystemActions } from './validate_system_actions'; + +describe('validateSystemActions', () => { + const connectorAdapter: ConnectorAdapter = { + connectorTypeId: '.test', + ruleActionParamsSchema: schema.object({ foo: schema.string() }), + buildActionParams: jest.fn(), + }; + + let registry: ConnectorAdapterRegistry; + let actionsClient: jest.Mocked; + + beforeEach(() => { + registry = new ConnectorAdapterRegistry(); + actionsClient = actionsClientMock.create(); + actionsClient.getBulk.mockResolvedValue([ + { + id: 'system_action-id', + actionTypeId: '.test', + config: {}, + isMissingSecrets: false, + name: 'system action connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: true, + }, + ]); + }); + + it('should not validate with empty system actions', async () => { + const res = await validateSystemActions({ + connectorAdapterRegistry: registry, + systemActions: [], + actionsClient, + }); + + expect(res).toBe(undefined); + expect(actionsClient.getBulk).not.toBeCalled(); + expect(actionsClient.isSystemAction).not.toBeCalled(); + }); + + it('should throw an error if the action is not a system action even if it is declared as one', async () => { + const systemActions: RuleSystemAction[] = [ + { + id: 'not-exist', + uuid: '123', + params: { foo: 'test' }, + actionTypeId: '.test', + type: RuleActionTypes.SYSTEM, + }, + ]; + + registry.register(connectorAdapter); + + actionsClient.isSystemAction.mockReturnValue(false); + + await expect(() => + validateSystemActions({ + connectorAdapterRegistry: registry, + systemActions, + actionsClient, + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Action not-exist is not a system action"`); + }); + + it('should throw an error if the action is system action but is not returned from the actions client (getBulk)', async () => { + const systemActions: RuleSystemAction[] = [ + { + id: 'not-exist', + uuid: '123', + params: { foo: 'test' }, + actionTypeId: '.test', + type: RuleActionTypes.SYSTEM, + }, + ]; + + registry.register(connectorAdapter); + + actionsClient.isSystemAction.mockReturnValue(true); + + await expect(() => + validateSystemActions({ + connectorAdapterRegistry: registry, + systemActions, + actionsClient, + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Action not-exist is not a system action"`); + }); + + it('should throw an error if the params are not valid', async () => { + const systemActions: RuleSystemAction[] = [ + { + id: 'system_action-id', + uuid: '123', + params: { 'not-exist': 'test' }, + actionTypeId: '.test', + type: RuleActionTypes.SYSTEM, + }, + ]; + + registry.register(connectorAdapter); + + actionsClient.isSystemAction.mockReturnValue(true); + + await expect(() => + validateSystemActions({ + connectorAdapterRegistry: registry, + systemActions, + actionsClient, + }) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"Invalid system action params. System action type: .test - [foo]: expected value of type [string] but got [undefined]"` + ); + }); + + it('should call getBulk correctly', async () => { + const systemActions: Array = [ + { + id: 'system_action-id', + uuid: '123', + params: { foo: 'test' }, + type: RuleActionTypes.SYSTEM, + }, + { + id: 'system_action-id-2', + uuid: '123', + params: { foo: 'test' }, + actionTypeId: '.test', + type: RuleActionTypes.SYSTEM, + }, + ]; + + actionsClient.getBulk.mockResolvedValue([ + { + id: 'system_action-id', + actionTypeId: '.test', + config: {}, + isMissingSecrets: false, + name: 'system action connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: true, + }, + { + id: 'system_action-id-2', + actionTypeId: '.test', + config: {}, + isMissingSecrets: false, + name: 'system action connector 2', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: true, + }, + ]); + + registry.register(connectorAdapter); + + actionsClient.isSystemAction.mockReturnValue(true); + + const res = await validateSystemActions({ + connectorAdapterRegistry: registry, + systemActions, + actionsClient, + }); + + expect(res).toBe(undefined); + + expect(actionsClient.getBulk).toBeCalledWith({ + ids: ['system_action-id', 'system_action-id-2'], + throwIfSystemAction: false, + }); + }); +}); diff --git a/x-pack/plugins/alerting/server/lib/validate_system_actions.ts b/x-pack/plugins/alerting/server/lib/validate_system_actions.ts new file mode 100644 index 00000000000000..ebe9426297ca2d --- /dev/null +++ b/x-pack/plugins/alerting/server/lib/validate_system_actions.ts @@ -0,0 +1,59 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import Boom from '@hapi/boom'; +import { ActionsClient } from '@kbn/actions-plugin/server'; +import { ConnectorAdapterRegistry } from '../connector_adapters/connector_adapter_registry'; +import { bulkValidateConnectorAdapterActionParams } from '../connector_adapters/validate_rule_action_params'; +import { NormalizedSystemAction } from '../rules_client'; +import { RuleSystemAction } from '../types'; +interface Params { + actionsClient: ActionsClient; + connectorAdapterRegistry: ConnectorAdapterRegistry; + systemActions: Array; +} + +export const validateSystemActions = async ({ + actionsClient, + connectorAdapterRegistry, + systemActions, +}: Params) => { + if (systemActions.length === 0) { + return; + } + + /** + * When updating or creating a rule the actions may not contain + * the actionTypeId. We need to getBulk using the + * actionsClient to get the actionTypeId of each action. + * The actionTypeId is needed to get the schema of + * the action params using the connector adapter registry + */ + const actionIds: string[] = systemActions.map((action) => action.id); + + const actionResults = await actionsClient.getBulk({ ids: actionIds, throwIfSystemAction: false }); + const systemActionsWithActionTypeId: RuleSystemAction[] = []; + + for (const systemAction of systemActions) { + const isSystemAction = actionsClient.isSystemAction(systemAction.id); + const foundAction = actionResults.find((actionRes) => actionRes.id === systemAction.id); + + if (!isSystemAction || !foundAction) { + throw Boom.badRequest(`Action ${systemAction.id} is not a system action`); + } + + systemActionsWithActionTypeId.push({ + ...systemAction, + actionTypeId: foundAction.actionTypeId, + }); + } + + bulkValidateConnectorAdapterActionParams({ + connectorAdapterRegistry, + actions: systemActionsWithActionTypeId, + }); +}; diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/create/create_rule_route.test.ts b/x-pack/plugins/alerting/server/routes/rule/apis/create/create_rule_route.test.ts index e952a72ec38590..b84c53b9b5409f 100644 --- a/x-pack/plugins/alerting/server/routes/rule/apis/create/create_rule_route.test.ts +++ b/x-pack/plugins/alerting/server/routes/rule/apis/create/create_rule_route.test.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { pick } from 'lodash'; +import { omit, pick } from 'lodash'; import { createRuleRoute } from './create_rule_route'; import { httpServiceMock } from '@kbn/core/server/mocks'; import { licenseStateMock } from '../../../../lib/license_state.mock'; @@ -15,9 +15,16 @@ import type { CreateRuleRequestBodyV1 } from '../../../../../common/routes/rule/ import { rulesClientMock } from '../../../../rules_client.mock'; import { RuleTypeDisabledError } from '../../../../lib'; import { AsApiContract } from '../../../lib'; -import { SanitizedRule } from '../../../../types'; +import { + RuleActionTypes, + RuleDefaultAction, + RuleSystemAction, + SanitizedRule, + SanitizedRuleResponse, +} from '../../../../types'; import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks'; import { usageCountersServiceMock } from '@kbn/usage-collection-plugin/server/usage_counters/usage_counters_service.mock'; +import { actionsClientMock } from '@kbn/actions-plugin/server/mocks'; const rulesClient = rulesClientMock.create(); @@ -32,6 +39,38 @@ beforeEach(() => { describe('createRuleRoute', () => { const createdAt = new Date(); const updatedAt = new Date(); + const action: RuleDefaultAction = { + actionTypeId: 'test', + group: 'default', + id: '2', + params: { + foo: true, + }, + uuid: '123-456', + alertsFilter: { + query: { + kql: 'name:test', + dsl: '{"must": {"term": { "name": "test" }}}', + filters: [], + }, + timeframe: { + days: [1], + hours: { start: '08:00', end: '17:00' }, + timezone: 'UTC', + }, + }, + type: RuleActionTypes.DEFAULT, + }; + + const systemAction: RuleSystemAction = { + actionTypeId: 'test-2', + id: 'system_action-id', + params: { + foo: true, + }, + uuid: '123-456', + type: RuleActionTypes.SYSTEM, + }; const mockedAlert: SanitizedRule<{ bar: boolean }> = { alertTypeId: '1', @@ -43,29 +82,7 @@ describe('createRuleRoute', () => { bar: true, }, throttle: '30s', - actions: [ - { - actionTypeId: 'test', - group: 'default', - id: '2', - params: { - foo: true, - }, - uuid: '123-456', - alertsFilter: { - query: { - kql: 'name:test', - dsl: '{"must": {"term": { "name": "test" }}}', - filters: [], - }, - timeframe: { - days: [1], - hours: { start: '08:00', end: '17:00' }, - timezone: 'UTC', - }, - }, - }, - ], + actions: [action], enabled: true, muteAll: false, createdBy: '', @@ -89,18 +106,21 @@ describe('createRuleRoute', () => { notify_when: mockedAlert.notifyWhen, actions: [ { - group: mockedAlert.actions[0].group, + group: action.group, id: mockedAlert.actions[0].id, params: mockedAlert.actions[0].params, alerts_filter: { - query: { kql: mockedAlert.actions[0].alertsFilter!.query!.kql, filters: [] }, - timeframe: mockedAlert.actions[0].alertsFilter?.timeframe!, + query: { + kql: action.alertsFilter!.query!.kql, + filters: [], + }, + timeframe: action.alertsFilter?.timeframe!, }, }, ], }; - const createResult: AsApiContract> = { + const createResult: AsApiContract> = { ...ruleToCreate, mute_all: mockedAlert.muteAll, created_by: mockedAlert.createdBy, @@ -119,8 +139,8 @@ describe('createRuleRoute', () => { { ...ruleToCreate.actions[0], alerts_filter: { - query: mockedAlert.actions[0].alertsFilter?.query!, - timeframe: mockedAlert.actions[0].alertsFilter!.timeframe!, + query: action.alertsFilter?.query!, + timeframe: action.alertsFilter!.timeframe!, }, connector_type_id: 'test', uuid: '123-456', @@ -198,6 +218,7 @@ describe('createRuleRoute', () => { "params": Object { "foo": true, }, + "type": "default", }, ], "alertTypeId": "1", @@ -314,6 +335,7 @@ describe('createRuleRoute', () => { "params": Object { "foo": true, }, + "type": "default", }, ], "alertTypeId": "1", @@ -431,6 +453,7 @@ describe('createRuleRoute', () => { "params": Object { "foo": true, }, + "type": "default", }, ], "alertTypeId": "1", @@ -548,6 +571,7 @@ describe('createRuleRoute', () => { "params": Object { "foo": true, }, + "type": "default", }, ], "alertTypeId": "1", @@ -646,4 +670,172 @@ describe('createRuleRoute', () => { expect(res.forbidden).toHaveBeenCalledWith({ body: { message: 'Fail' } }); }); + + describe('actions', () => { + it('adds the type of the actions correctly before passing the request to the rules client', async () => { + const licenseState = licenseStateMock.create(); + const router = httpServiceMock.createRouter(); + const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true }); + const mockUsageCountersSetup = usageCountersServiceMock.createSetupContract(); + const mockUsageCounter = mockUsageCountersSetup.createUsageCounter('test'); + const actionsClient = actionsClientMock.create(); + actionsClient.isSystemAction.mockImplementation((id: string) => id === 'system_action-id'); + + createRuleRoute({ + router, + licenseState, + encryptedSavedObjects, + usageCounter: mockUsageCounter, + }); + + const [_, handler] = router.post.mock.calls[0]; + + rulesClient.create.mockResolvedValueOnce({ ...mockedAlert, actions: [action, systemAction] }); + + const [context, req, res] = mockHandlerArguments( + { rulesClient, actionsClient }, + { + body: { ...ruleToCreate, actions: [omit(action, 'type'), omit(systemAction, 'type')] }, + }, + ['ok'] + ); + + await handler(context, req, res); + + expect(rulesClient.create.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "data": Object { + "actions": Array [ + Object { + "actionTypeId": "test", + "group": "default", + "id": "2", + "params": Object { + "foo": true, + }, + "type": "default", + "uuid": "123-456", + }, + Object { + "actionTypeId": "test-2", + "id": "system_action-id", + "params": Object { + "foo": true, + }, + "type": "system", + "uuid": "123-456", + }, + ], + "alertTypeId": "1", + "consumer": "bar", + "enabled": true, + "name": "abc", + "notifyWhen": "onActionGroupChange", + "params": Object { + "bar": true, + }, + "schedule": Object { + "interval": "10s", + }, + "tags": Array [ + "foo", + ], + "throttle": "30s", + }, + "options": Object { + "id": undefined, + }, + }, + ] + `); + }); + + it('removes the type from the actions correctly before sending the response', async () => { + const licenseState = licenseStateMock.create(); + const router = httpServiceMock.createRouter(); + const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true }); + const mockUsageCountersSetup = usageCountersServiceMock.createSetupContract(); + const mockUsageCounter = mockUsageCountersSetup.createUsageCounter('test'); + const actionsClient = actionsClientMock.create(); + actionsClient.isSystemAction.mockImplementation((id: string) => id === 'system_action-id'); + + createRuleRoute({ + router, + licenseState, + encryptedSavedObjects, + usageCounter: mockUsageCounter, + }); + + const [_, handler] = router.post.mock.calls[0]; + + rulesClient.create.mockResolvedValueOnce({ ...mockedAlert, actions: [action, systemAction] }); + + const [context, req, res] = mockHandlerArguments( + { rulesClient, actionsClient }, + { + body: ruleToCreate, + }, + ['ok'] + ); + + const routeRes = await handler(context, req, res); + + // @ts-expect-error: body exists + expect(routeRes.body.actions).toEqual([ + { + alerts_filter: { + query: { dsl: '{"must": {"term": { "name": "test" }}}', filters: [], kql: 'name:test' }, + timeframe: { days: [1], hours: { end: '17:00', start: '08:00' }, timezone: 'UTC' }, + }, + connector_type_id: 'test', + group: 'default', + id: '2', + params: { foo: true }, + uuid: '123-456', + }, + { + connector_type_id: 'test-2', + id: 'system_action-id', + params: { foo: true }, + uuid: '123-456', + }, + ]); + }); + + it('fails if the action contains a type in the request', async () => { + const actionToValidate: RuleDefaultAction = { + actionTypeId: 'test', + group: 'default', + id: '2', + params: { + foo: true, + }, + uuid: '123-456', + type: RuleActionTypes.DEFAULT, + }; + + const licenseState = licenseStateMock.create(); + const router = httpServiceMock.createRouter(); + const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true }); + const mockUsageCountersSetup = usageCountersServiceMock.createSetupContract(); + const mockUsageCounter = mockUsageCountersSetup.createUsageCounter('test'); + + createRuleRoute({ + router, + licenseState, + encryptedSavedObjects, + usageCounter: mockUsageCounter, + }); + + const [config, _] = router.post.mock.calls[0]; + + expect(() => + // @ts-expect-error: body exists + config.validate.body.validate({ ...ruleToCreate, actions: [actionToValidate] }) + ).toThrowErrorMatchingInlineSnapshot( + `"[actions.0.type]: definition for this key is missing"` + ); + }); + }); }); diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/create/create_rule_route.ts b/x-pack/plugins/alerting/server/routes/rule/apis/create/create_rule_route.ts index 6b28b642849041..a6b252497bace5 100644 --- a/x-pack/plugins/alerting/server/routes/rule/apis/create/create_rule_route.ts +++ b/x-pack/plugins/alerting/server/routes/rule/apis/create/create_rule_route.ts @@ -13,7 +13,7 @@ import { } from '../../../lib'; import { BASE_ALERTING_API_PATH } from '../../../../types'; import { RouteOptions } from '../../..'; -import type { +import { CreateRuleRequestBodyV1, CreateRuleRequestParamsV1, CreateRuleResponseV1, @@ -40,6 +40,7 @@ export const createRuleRoute = ({ router, licenseState, usageCounter }: RouteOpt router.handleLegacyErrors( verifyAccessAndContext(licenseState, async function (context, req, res) { const rulesClient = (await context.alerting).getRulesClient(); + const actionsClient = (await context.actions).getActionsClient(); // Assert versioned inputs const createRuleData: CreateRuleRequestBodyV1 = req.body; @@ -55,7 +56,9 @@ export const createRuleRoute = ({ router, licenseState, usageCounter }: RouteOpt // TODO (http-versioning): Remove this cast, this enables us to move forward // without fixing all of other solution types const createdRule: Rule = (await rulesClient.create({ - data: transformCreateBodyV1(createRuleData), + data: transformCreateBodyV1(createRuleData, (connectorId: string) => + actionsClient.isSystemAction(connectorId) + ), options: { id: params?.id }, })) as Rule; diff --git a/x-pack/plugins/alerting/server/routes/rule/transforms/transform_rule_to_rule_response/v1.test.ts b/x-pack/plugins/alerting/server/routes/rule/transforms/transform_rule_to_rule_response/v1.test.ts new file mode 100644 index 00000000000000..b3f041fe26632a --- /dev/null +++ b/x-pack/plugins/alerting/server/routes/rule/transforms/transform_rule_to_rule_response/v1.test.ts @@ -0,0 +1,104 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { RuleActionTypes, RuleDefaultAction, RuleSystemAction } from '../../../../../common'; +import { transformRuleToRuleResponse } from './v1'; + +describe('transformRuleToRuleResponse', () => { + const defaultAction: RuleDefaultAction = { + id: '1', + uuid: '111', + params: { foo: 'bar' }, + group: 'default', + actionTypeId: '.test', + frequency: { notifyWhen: 'onThrottleInterval', summary: true, throttle: '1h' }, + alertsFilter: { + query: { dsl: '{test:1}', kql: 'test:1s', filters: [] }, + timeframe: { + days: [1, 2, 3], + hours: { end: '15:00', start: '00:00' }, + timezone: 'UTC', + }, + }, + type: RuleActionTypes.DEFAULT, + }; + + const systemAction: RuleSystemAction = { + id: '1', + uuid: '111', + params: { foo: 'bar' }, + actionTypeId: '.test', + type: RuleActionTypes.SYSTEM, + }; + + const rule = { + id: '3d534c70-582b-11ec-8995-2b1578a3bc5d', + enabled: true, + name: 'stressing index-threshold 37/200', + tags: [], + alertTypeId: '.index-threshold', + consumer: 'alerts', + schedule: { + interval: '1s', + }, + actions: [], + params: {}, + createdBy: 'elastic', + updatedBy: '2889684073', + createdAt: new Date('2023-08-01T09:16:35.368Z'), + updatedAt: new Date('2023-08-01T09:16:35.368Z'), + notifyWhen: 'onActiveAlert' as const, + throttle: null, + apiKey: null, + apiKeyOwner: '2889684073', + muteAll: false, + mutedInstanceIds: [], + scheduledTaskId: '52125fb0-5895-11ec-ae69-bb65d1a71b72', + executionStatus: { + status: 'ok' as const, + lastExecutionDate: new Date('2023-08-01T09:16:35.368Z'), + lastDuration: 1194, + }, + revision: 0, + }; + + describe('actions', () => { + it('transforms a default action correctly', () => { + const res = transformRuleToRuleResponse({ ...rule, actions: [defaultAction] }); + expect(res.actions).toEqual([ + { + alerts_filter: { + query: { dsl: '{test:1}', filters: [], kql: 'test:1s' }, + timeframe: { + days: [1, 2, 3], + hours: { end: '15:00', start: '00:00' }, + timezone: 'UTC', + }, + }, + connector_type_id: '.test', + frequency: { notify_when: 'onThrottleInterval', summary: true, throttle: '1h' }, + group: 'default', + id: '1', + params: { foo: 'bar' }, + uuid: '111', + }, + ]); + }); + + it('transforms a system action correctly', () => { + const res = transformRuleToRuleResponse({ ...rule, actions: [systemAction] }); + expect(res.actions).toEqual([ + { + id: '1', + uuid: '111', + params: { foo: 'bar' }, + connector_type_id: '.test', + }, + ]); + }); + }); +}); diff --git a/x-pack/plugins/alerting/server/routes/rule/transforms/transform_rule_to_rule_response/v1.ts b/x-pack/plugins/alerting/server/routes/rule/transforms/transform_rule_to_rule_response/v1.ts index e43427fe4470aa..08b915b744bc06 100644 --- a/x-pack/plugins/alerting/server/routes/rule/transforms/transform_rule_to_rule_response/v1.ts +++ b/x-pack/plugins/alerting/server/routes/rule/transforms/transform_rule_to_rule_response/v1.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { RuleActionTypes } from '../../../../../common'; import { RuleResponseV1, RuleParamsV1, @@ -38,18 +39,16 @@ const transformMonitoring = (monitoring: Monitoring): MonitoringV1 => { }; }; -export const transformRuleToRuleResponse = ( - rule: Rule -): RuleResponseV1 => ({ - id: rule.id, - enabled: rule.enabled, - name: rule.name, - tags: rule.tags, - rule_type_id: rule.alertTypeId, - consumer: rule.consumer, - schedule: rule.schedule, - actions: rule.actions.map( - ({ group, id, actionTypeId, params, frequency, uuid, alertsFilter }) => ({ +const transformRuleActions = (actions: Rule['actions']): RuleResponseV1['actions'] => { + return actions.map((action) => { + if (action.type === RuleActionTypes.SYSTEM) { + const { id, actionTypeId, params, uuid } = action; + return { id, params, uuid, connector_type_id: actionTypeId }; + } + + const { group, id, actionTypeId, params, frequency, uuid, alertsFilter } = action; + + return { group, id, params, @@ -65,8 +64,21 @@ export const transformRuleToRuleResponse = ( : {}), ...(uuid && { uuid }), ...(alertsFilter && { alerts_filter: alertsFilter }), - }) - ), + }; + }); +}; + +export const transformRuleToRuleResponse = ( + rule: Rule +): RuleResponseV1 => ({ + id: rule.id, + enabled: rule.enabled, + name: rule.name, + tags: rule.tags, + rule_type_id: rule.alertTypeId, + consumer: rule.consumer, + schedule: rule.schedule, + actions: transformRuleActions(rule.actions), params: rule.params, ...(rule.mapped_params ? { mapped_params: rule.mapped_params } : {}), ...(rule.scheduledTaskId !== undefined ? { scheduled_task_id: rule.scheduledTaskId } : {}), diff --git a/x-pack/plugins/alerting/server/rules_client/common/inject_references.ts b/x-pack/plugins/alerting/server/rules_client/common/inject_references.ts index bbf192e4f1cf44..2938c913723254 100644 --- a/x-pack/plugins/alerting/server/rules_client/common/inject_references.ts +++ b/x-pack/plugins/alerting/server/rules_client/common/inject_references.ts @@ -9,7 +9,7 @@ import Boom from '@hapi/boom'; import { omit } from 'lodash'; import { SavedObjectReference, SavedObjectAttributes } from '@kbn/core/server'; import { UntypedNormalizedRuleType } from '../../rule_type_registry'; -import { Rule, RawRule, RuleTypeParams } from '../../types'; +import { RawRule, RuleTypeParams } from '../../types'; import { RuleActionAttributes } from '../../data/rule/types'; import { preconfiguredConnectorActionRefPrefix, @@ -45,7 +45,7 @@ export function injectReferencesIntoActions( ...omit(action, 'actionRef'), id: reference.id, }; - }) as Rule['actions']; + }); } export function injectReferencesIntoParams< diff --git a/x-pack/plugins/alerting/server/rules_client/types.ts b/x-pack/plugins/alerting/server/rules_client/types.ts index 89dce3ed90451d..6be77417ddce86 100644 --- a/x-pack/plugins/alerting/server/rules_client/types.ts +++ b/x-pack/plugins/alerting/server/rules_client/types.ts @@ -21,6 +21,7 @@ import { EncryptedSavedObjectsClient } from '@kbn/encrypted-saved-objects-plugin import { TaskManagerStartContract } from '@kbn/task-manager-plugin/server'; import { IEventLogClient, IEventLogger } from '@kbn/event-log-plugin/server'; import { AuditLogger } from '@kbn/security-plugin/server'; +import { DistributiveOmit } from '@elastic/eui'; import { RegistryRuleType } from '../rule_type_registry'; import { RuleTypeRegistry, @@ -29,12 +30,12 @@ import { SanitizedRule, RuleSnoozeSchedule, RawRuleAlertsFilter, + RuleSystemAction, + RuleDefaultAction, } from '../types'; import { AlertingAuthorization } from '../authorization'; import { AlertingRulesConfig } from '../config'; import { ConnectorAdapterRegistry } from '../connector_adapters/connector_adapter_registry'; -import { GetAlertIndicesAlias } from '../lib'; -import { AlertsService } from '../alerts_service'; export type { BulkEditOperation, @@ -78,20 +79,29 @@ export interface RulesClientContext { readonly isAuthenticationTypeAPIKey: () => boolean; readonly getAuthenticationAPIKey: (name: string) => CreateAPIKeyResult; readonly connectorAdapterRegistry: ConnectorAdapterRegistry; - readonly getAlertIndicesAlias: GetAlertIndicesAlias; - readonly alertsService: AlertsService | null; + readonly isSystemAction: (actionId: string) => boolean; } -export type NormalizedAlertAction = Omit; +export type NormalizedAlertAction = DistributiveOmit; +export type NormalizedSystemAction = Omit; -export type NormalizedAlertActionWithGeneratedValues = Omit< - NormalizedAlertAction, - 'uuid' | 'alertsFilter' +export type NormalizedAlertDefaultActionWithGeneratedValues = Omit< + RuleDefaultAction, + 'uuid' | 'alertsFilter' | 'actionTypeId' > & { uuid: string; alertsFilter?: RawRuleAlertsFilter; }; +export type NormalizedAlertSystemActionWithGeneratedValues = Omit< + RuleSystemAction, + 'uuid' | 'actionTypeId' +> & { uuid: string }; + +export type NormalizedAlertActionWithGeneratedValues = + | NormalizedAlertDefaultActionWithGeneratedValues + | NormalizedAlertSystemActionWithGeneratedValues; + export interface RegistryAlertTypeWithAuth extends RegistryRuleType { authorizedConsumers: string[]; } @@ -127,7 +137,6 @@ export interface IndexType { [key: string]: unknown; } -// TODO: remove once all mute endpoints have been migrated to RuleMuteAlertOptions export interface MuteOptions extends IndexType { alertId: string; alertInstanceId: string; diff --git a/x-pack/plugins/alerting/server/rules_client_factory.test.ts b/x-pack/plugins/alerting/server/rules_client_factory.test.ts index 31f06344d45f41..79cb83f13da9f5 100644 --- a/x-pack/plugins/alerting/server/rules_client_factory.test.ts +++ b/x-pack/plugins/alerting/server/rules_client_factory.test.ts @@ -47,8 +47,6 @@ const rulesClientFactoryParams: jest.Mocked = { ruleTypeRegistry: ruleTypeRegistryMock.create(), getSpaceId: jest.fn(), spaceIdToNamespace: jest.fn(), - getAlertIndicesAlias: jest.fn(), - alertsService: null, maxScheduledPerMinute: 10000, minimumScheduleInterval: { value: '1m', enforce: false }, internalSavedObjectsRepository, @@ -116,9 +114,8 @@ test('creates a rules client with proper constructor arguments when security is minimumScheduleInterval: { value: '1m', enforce: false }, isAuthenticationTypeAPIKey: expect.any(Function), getAuthenticationAPIKey: expect.any(Function), - getAlertIndicesAlias: expect.any(Function), - alertsService: null, connectorAdapterRegistry: expect.any(ConnectorAdapterRegistry), + isSystemAction: expect.any(Function), }); }); @@ -161,9 +158,8 @@ test('creates a rules client with proper constructor arguments', async () => { minimumScheduleInterval: { value: '1m', enforce: false }, isAuthenticationTypeAPIKey: expect.any(Function), getAuthenticationAPIKey: expect.any(Function), - getAlertIndicesAlias: expect.any(Function), - alertsService: null, connectorAdapterRegistry: expect.any(ConnectorAdapterRegistry), + isSystemAction: expect.any(Function), }); }); diff --git a/x-pack/plugins/alerting/server/rules_client_factory.ts b/x-pack/plugins/alerting/server/rules_client_factory.ts index 337d4daab903be..4e982bae3fa908 100644 --- a/x-pack/plugins/alerting/server/rules_client_factory.ts +++ b/x-pack/plugins/alerting/server/rules_client_factory.ts @@ -27,8 +27,6 @@ import { RulesClient } from './rules_client'; import { AlertingAuthorizationClientFactory } from './alerting_authorization_client_factory'; import { AlertingRulesConfig } from './config'; import { ConnectorAdapterRegistry } from './connector_adapters/connector_adapter_registry'; -import { GetAlertIndicesAlias } from './lib'; -import { AlertsService } from './alerts_service/alerts_service'; export interface RulesClientFactoryOpts { logger: Logger; taskManager: TaskManagerStartContract; @@ -47,8 +45,6 @@ export interface RulesClientFactoryOpts { minimumScheduleInterval: AlertingRulesConfig['minimumScheduleInterval']; maxScheduledPerMinute: AlertingRulesConfig['maxScheduledPerMinute']; connectorAdapterRegistry: ConnectorAdapterRegistry; - getAlertIndicesAlias: GetAlertIndicesAlias; - alertsService: AlertsService | null; } export class RulesClientFactory { @@ -70,8 +66,6 @@ export class RulesClientFactory { private minimumScheduleInterval!: AlertingRulesConfig['minimumScheduleInterval']; private maxScheduledPerMinute!: AlertingRulesConfig['maxScheduledPerMinute']; private connectorAdapterRegistry!: ConnectorAdapterRegistry; - private getAlertIndicesAlias!: GetAlertIndicesAlias; - private alertsService!: AlertsService | null; public initialize(options: RulesClientFactoryOpts) { if (this.isInitialized) { @@ -95,8 +89,6 @@ export class RulesClientFactory { this.minimumScheduleInterval = options.minimumScheduleInterval; this.maxScheduledPerMinute = options.maxScheduledPerMinute; this.connectorAdapterRegistry = options.connectorAdapterRegistry; - this.getAlertIndicesAlias = options.getAlertIndicesAlias; - this.alertsService = options.alertsService; } public create(request: KibanaRequest, savedObjects: SavedObjectsServiceStart): RulesClient { @@ -126,8 +118,6 @@ export class RulesClientFactory { encryptedSavedObjectsClient: this.encryptedSavedObjectsClient, auditLogger: securityPluginSetup?.audit.asScoped(request), connectorAdapterRegistry: this.connectorAdapterRegistry, - getAlertIndicesAlias: this.getAlertIndicesAlias, - alertsService: this.alertsService, async getUserName() { if (!securityPluginStart) { return null; @@ -185,6 +175,9 @@ export class RulesClientFactory { } return { apiKeysEnabled: false }; }, + isSystemAction(actionId: string) { + return actions.isSystemActionConnector(actionId); + }, }); } } diff --git a/x-pack/plugins/alerting/server/types.ts b/x-pack/plugins/alerting/server/types.ts index 66e2c3bfa6069a..1e5e91ad764fa5 100644 --- a/x-pack/plugins/alerting/server/types.ts +++ b/x-pack/plugins/alerting/server/types.ts @@ -25,6 +25,7 @@ import { SharePluginStart } from '@kbn/share-plugin/server'; import type { FieldMap } from '@kbn/alerts-as-data-utils'; import { Alert } from '@kbn/alerts-as-data-utils'; import { Filter } from '@kbn/es-query'; +import { ActionsApiRequestHandlerContext } from '@kbn/actions-plugin/server'; import { RuleTypeRegistry as OrigruleTypeRegistry } from './rule_type_registry'; import { PluginSetupContract, PluginStartContract } from './plugin'; import { RulesClient } from './rules_client'; @@ -56,6 +57,7 @@ import { AlertsFilter, AlertsFilterTimeframe, RuleAlertData, + RuleActionResponse, } from '../common'; import { PublicAlertFactory } from './alert/create_alert_factory'; import { RulesSettingsFlappingProperties } from '../common/rules_settings'; @@ -80,6 +82,7 @@ export interface AlertingApiRequestHandlerContext { */ export type AlertingRequestHandlerContext = CustomRequestHandlerContext<{ alerting: AlertingApiRequestHandlerContext; + actions: ActionsApiRequestHandlerContext; }>; /** @@ -405,9 +408,9 @@ export interface RawRuleAlertsFilter extends AlertsFilter { timeframe?: AlertsFilterTimeframe; } -export interface RawRuleAction extends SavedObjectAttributes { +export interface RawRuleAction { uuid: string; - group: string; + group?: string; actionRef: string; actionTypeId: string; params: RuleActionParams; @@ -422,7 +425,7 @@ export interface RawRuleAction extends SavedObjectAttributes { // note that the `error` property is "null-able", as we're doing a partial // update on the rule when we update this data, but need to ensure we // delete any previous error if the current status has no error -export interface RawRuleExecutionStatus extends SavedObjectAttributes { +export interface RawRuleExecutionStatus { status: RuleExecutionStatuses; lastExecutionDate: string; lastDuration?: number; @@ -436,7 +439,7 @@ export interface RawRuleExecutionStatus extends SavedObjectAttributes { }; } -export interface RawRule extends SavedObjectAttributes { +export interface RawRule { enabled: boolean; name: string; tags: string[]; From 6ec30ebd526c607fde2cf00b67d1990d2265ed32 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 3 Oct 2023 15:01:52 +0300 Subject: [PATCH 02/14] Init system action types --- x-pack/plugins/alerting/common/rule.ts | 24 ++++++++++++++----- x-pack/plugins/alerting/server/index.ts | 1 + .../alerting/server/rules_client/types.ts | 22 +++++++++++++---- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/alerting/common/rule.ts b/x-pack/plugins/alerting/common/rule.ts index 63bd8dd4f8f4a7..065816c321abab 100644 --- a/x-pack/plugins/alerting/common/rule.ts +++ b/x-pack/plugins/alerting/common/rule.ts @@ -82,13 +82,13 @@ export interface RuleExecutionStatus { export type RuleActionParams = SavedObjectAttributes; export type RuleActionParam = SavedObjectAttribute; -export interface RuleActionFrequency extends SavedObjectAttributes { +export interface RuleActionFrequency { summary: boolean; notifyWhen: RuleNotifyWhenType; throttle: string | null; } -export interface AlertsFilterTimeframe extends SavedObjectAttributes { +export interface AlertsFilterTimeframe { days: IsoWeekday[]; timezone: string; hours: { @@ -97,7 +97,7 @@ export interface AlertsFilterTimeframe extends SavedObjectAttributes { }; } -export interface AlertsFilter extends SavedObjectAttributes { +export interface AlertsFilter { query?: { kql: string; filters: Filter[]; @@ -121,7 +121,7 @@ export const RuleActionTypes = { export type RuleActionTypes = typeof RuleActionTypes[keyof typeof RuleActionTypes]; -export interface RuleAction { +export interface RuleDefaultAction { uuid?: string; group: string; id: string; @@ -129,9 +129,19 @@ export interface RuleAction { params: RuleActionParams; frequency?: RuleActionFrequency; alertsFilter?: AlertsFilter; - type?: typeof RuleActionTypes.DEFAULT; + type: typeof RuleActionTypes.DEFAULT; } +export interface RuleSystemAction { + uuid?: string; + id: string; + actionTypeId: string; + params: RuleActionParams; + type: typeof RuleActionTypes.SYSTEM; +} + +export type RuleAction = RuleDefaultAction | RuleSystemAction; + export interface RuleLastRun { outcome: RuleLastRunOutcomes; outcomeOrder?: number; @@ -195,10 +205,12 @@ export interface SanitizedAlertsFilter extends AlertsFilter { timeframe?: AlertsFilterTimeframe; } -export type SanitizedRuleAction = Omit & { +export type SanitizedDefaultRuleAction = Omit & { alertsFilter?: SanitizedAlertsFilter; }; +export type SanitizedRuleAction = SanitizedDefaultRuleAction | RuleSystemAction; + export type SanitizedRule = Omit< Rule, 'apiKey' | 'actions' diff --git a/x-pack/plugins/alerting/server/index.ts b/x-pack/plugins/alerting/server/index.ts index 6aa1c44fe6e81d..66d1f707bb623c 100644 --- a/x-pack/plugins/alerting/server/index.ts +++ b/x-pack/plugins/alerting/server/index.ts @@ -68,6 +68,7 @@ export { isValidAlertIndexName, } from './alerts_service'; export { getDataStreamAdapter } from './alerts_service/lib/data_stream_adapter'; +export type { ConnectorAdapter } from './connector_adapters/types'; export const plugin = (initContext: PluginInitializerContext) => new AlertingPlugin(initContext); diff --git a/x-pack/plugins/alerting/server/rules_client/types.ts b/x-pack/plugins/alerting/server/rules_client/types.ts index 89dce3ed90451d..14f85886bb698d 100644 --- a/x-pack/plugins/alerting/server/rules_client/types.ts +++ b/x-pack/plugins/alerting/server/rules_client/types.ts @@ -21,6 +21,7 @@ import { EncryptedSavedObjectsClient } from '@kbn/encrypted-saved-objects-plugin import { TaskManagerStartContract } from '@kbn/task-manager-plugin/server'; import { IEventLogClient, IEventLogger } from '@kbn/event-log-plugin/server'; import { AuditLogger } from '@kbn/security-plugin/server'; +import { DistributiveOmit } from '@elastic/eui'; import { RegistryRuleType } from '../rule_type_registry'; import { RuleTypeRegistry, @@ -29,6 +30,8 @@ import { SanitizedRule, RuleSnoozeSchedule, RawRuleAlertsFilter, + RuleSystemAction, + RuleDefaultAction, } from '../types'; import { AlertingAuthorization } from '../authorization'; import { AlertingRulesConfig } from '../config'; @@ -80,18 +83,29 @@ export interface RulesClientContext { readonly connectorAdapterRegistry: ConnectorAdapterRegistry; readonly getAlertIndicesAlias: GetAlertIndicesAlias; readonly alertsService: AlertsService | null; + readonly isSystemAction: (actionId: string) => boolean; } -export type NormalizedAlertAction = Omit; +export type NormalizedAlertAction = DistributiveOmit; +export type NormalizedSystemAction = Omit; -export type NormalizedAlertActionWithGeneratedValues = Omit< - NormalizedAlertAction, - 'uuid' | 'alertsFilter' +export type NormalizedAlertDefaultActionWithGeneratedValues = Omit< + RuleDefaultAction, + 'uuid' | 'alertsFilter' | 'actionTypeId' > & { uuid: string; alertsFilter?: RawRuleAlertsFilter; }; +export type NormalizedAlertSystemActionWithGeneratedValues = Omit< + RuleSystemAction, + 'uuid' | 'actionTypeId' +> & { uuid: string }; + +export type NormalizedAlertActionWithGeneratedValues = + | NormalizedAlertDefaultActionWithGeneratedValues + | NormalizedAlertSystemActionWithGeneratedValues; + export interface RegistryAlertTypeWithAuth extends RegistryRuleType { authorizedConsumers: string[]; } From 40306cec867e3c6b585f4539703aca0b5d4cfb63 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 3 Oct 2023 15:02:22 +0300 Subject: [PATCH 03/14] Create isSystemAction helper --- x-pack/plugins/alerting/common/index.ts | 2 ++ .../system_actions/is_system_action.test.ts | 36 +++++++++++++++++++ .../common/system_actions/is_system_action.ts | 17 +++++++++ 3 files changed, 55 insertions(+) create mode 100644 x-pack/plugins/alerting/common/system_actions/is_system_action.test.ts create mode 100644 x-pack/plugins/alerting/common/system_actions/is_system_action.ts diff --git a/x-pack/plugins/alerting/common/index.ts b/x-pack/plugins/alerting/common/index.ts index e2e9e477cc4cc2..e0a23e50ca222b 100644 --- a/x-pack/plugins/alerting/common/index.ts +++ b/x-pack/plugins/alerting/common/index.ts @@ -38,6 +38,8 @@ export * from './rule_tags_aggregation'; export * from './iso_weekdays'; export * from './saved_objects/rules/mappings'; +export { isSystemAction } from './system_actions/is_system_action'; + export type { MaintenanceWindowModificationMetadata, DateRange, diff --git a/x-pack/plugins/alerting/common/system_actions/is_system_action.test.ts b/x-pack/plugins/alerting/common/system_actions/is_system_action.test.ts new file mode 100644 index 00000000000000..ee5c030c2c5dc3 --- /dev/null +++ b/x-pack/plugins/alerting/common/system_actions/is_system_action.test.ts @@ -0,0 +1,36 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { RuleSystemAction, RuleActionTypes, RuleDefaultAction } from '../rule'; +import { isSystemAction } from './is_system_action'; + +describe('isSystemAction', () => { + const defaultAction: RuleDefaultAction = { + actionTypeId: '.test', + uuid: '111', + group: 'default', + id: '1', + params: {}, + type: RuleActionTypes.DEFAULT, + }; + + const systemAction: RuleSystemAction = { + id: '1', + uuid: '123', + params: { 'not-exist': 'test' }, + actionTypeId: '.test', + type: RuleActionTypes.SYSTEM, + }; + + it('returns true if it is a system action', () => { + expect(isSystemAction(systemAction)).toBe(true); + }); + + it('returns false if it is not a system action', () => { + expect(isSystemAction(defaultAction)).toBe(false); + }); +}); diff --git a/x-pack/plugins/alerting/common/system_actions/is_system_action.ts b/x-pack/plugins/alerting/common/system_actions/is_system_action.ts new file mode 100644 index 00000000000000..ae9958b20b6b83 --- /dev/null +++ b/x-pack/plugins/alerting/common/system_actions/is_system_action.ts @@ -0,0 +1,17 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { AsApiContract } from '@kbn/actions-plugin/common'; +import { RuleAction, RuleSystemAction, RuleActionTypes } from '../rule'; + +type GetSystemActionType = T extends RuleAction + ? RuleSystemAction + : AsApiContract; + +export const isSystemAction = ( + action: RuleAction | AsApiContract +): action is GetSystemActionType => action.type === RuleActionTypes.SYSTEM; From 0fab8a8cc34a3448414ae1e82f2d1470b2d0d80e Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 3 Oct 2023 15:05:21 +0300 Subject: [PATCH 04/14] Use the isSystemAction helper in the executor --- .../server/task_runner/execution_handler.ts | 24 ++++++------------- .../alerting/server/task_runner/fixtures.ts | 9 ++++++- .../task_runner/rule_action_helper.test.ts | 22 ++++------------- .../server/task_runner/rule_action_helper.ts | 17 +++---------- .../server/task_runner/rule_loader.ts | 11 ++++++--- .../server/task_runner/task_runner.test.ts | 5 ++++ .../server/task_runner/task_runner.ts | 8 ++++++- .../task_runner/task_runner_cancel.test.ts | 3 ++- 8 files changed, 45 insertions(+), 54 deletions(-) diff --git a/x-pack/plugins/alerting/server/task_runner/execution_handler.ts b/x-pack/plugins/alerting/server/task_runner/execution_handler.ts index d12b3428e62ebb..fde39f906485aa 100644 --- a/x-pack/plugins/alerting/server/task_runner/execution_handler.ts +++ b/x-pack/plugins/alerting/server/task_runner/execution_handler.ts @@ -39,7 +39,8 @@ import { RuleTypeState, SanitizedRule, RuleAlertData, - RuleActionTypes, + RuleDefaultAction, + RuleSystemAction, RuleNotifyWhen, } from '../../common'; import { @@ -52,6 +53,7 @@ import { isSummaryActionThrottled, } from './rule_action_helper'; import { ConnectorAdapter } from '../connector_adapters/types'; +import { isSystemAction } from '../../common/system_actions/is_system_action'; enum Reasons { MUTED = 'muted', @@ -64,7 +66,7 @@ export interface RunResult { } interface RunSummarizedActionArgs { - action: RuleAction; + action: RuleDefaultAction; summarizedAlerts: CombinedSummarizedAlerts; spaceId: string; } @@ -75,14 +77,14 @@ interface RunActionArgs< ActionGroupIds extends string, RecoveryActionGroupId extends string > { - action: RuleAction; + action: RuleDefaultAction; alert: Alert; ruleId: string; spaceId: string; } interface RunSystemActionArgs { - action: RuleAction; + action: RuleSystemAction; connectorAdapter: ConnectorAdapter; summarizedAlerts: CombinedSummarizedAlerts; rule: SanitizedRule; @@ -615,7 +617,7 @@ export class ExecutionHandler< action, }: { alert: Alert; - action: RuleAction; + action: RuleDefaultAction; }) { const alertId = alert.getId(); const { rule, ruleLabel, logger } = this; @@ -943,15 +945,3 @@ export class ExecutionHandler< return bulkActions; } } - -/** - * TODO: Substitute with a function which takes into - * account system actions. - * - * Because RuleAction has the type set as RuleActionTypes.DEFAULT - * TS produce an error as the check below will always return false. - * We need the check to be able to test. - */ - -// @ts-expect-error -const isSystemAction = (action: RuleAction) => action.type === RuleActionTypes.SYSTEM; diff --git a/x-pack/plugins/alerting/server/task_runner/fixtures.ts b/x-pack/plugins/alerting/server/task_runner/fixtures.ts index 163b9415c7e5d0..fb795f4777b344 100644 --- a/x-pack/plugins/alerting/server/task_runner/fixtures.ts +++ b/x-pack/plugins/alerting/server/task_runner/fixtures.ts @@ -15,6 +15,8 @@ import { RuleLastRunOutcomeOrderMap, RuleLastRunOutcomes, SanitizedRule, + SanitizedRuleAction, + RuleActionTypes, } from '../../common'; import { getDefaultMonitoring } from '../lib/monitoring'; import { UntypedNormalizedRuleType } from '../rule_type_registry'; @@ -43,6 +45,7 @@ export const RULE_ACTIONS = [ foo: true, }, uuid: '111-111', + type: RuleActionTypes.DEFAULT, }, { actionTypeId: 'action', @@ -52,6 +55,7 @@ export const RULE_ACTIONS = [ isResolved: true, }, uuid: '222-222', + type: RuleActionTypes.DEFAULT, }, ]; @@ -192,6 +196,7 @@ export const mockedRuleTypeSavedObject: Rule = { foo: true, }, uuid: '111-111', + type: RuleActionTypes.DEFAULT, }, { group: RecoveredActionGroup.id, @@ -201,6 +206,7 @@ export const mockedRuleTypeSavedObject: Rule = { isResolved: true, }, uuid: '222-222', + type: RuleActionTypes.DEFAULT, }, ], executionStatus: { @@ -283,7 +289,8 @@ export const mockedRule: SanitizedRule return { ...action, id: action.uuid, - }; + type: RuleActionTypes.DEFAULT, + } as SanitizedRuleAction; }), isSnoozedUntil: undefined, }; diff --git a/x-pack/plugins/alerting/server/task_runner/rule_action_helper.test.ts b/x-pack/plugins/alerting/server/task_runner/rule_action_helper.test.ts index 84fb89de01cbf8..ca2dceb497841b 100644 --- a/x-pack/plugins/alerting/server/task_runner/rule_action_helper.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/rule_action_helper.test.ts @@ -6,7 +6,7 @@ */ import { Logger } from '@kbn/logging'; -import { RuleAction, RuleActionTypes } from '../types'; +import { RuleAction, RuleActionTypes, RuleSystemAction } from '../types'; import { generateActionHash, getSummaryActionsFromTaskState, @@ -26,6 +26,7 @@ const mockOldAction: RuleAction = { actionTypeId: 'slack', params: {}, uuid: '123-456', + type: RuleActionTypes.DEFAULT, }; const mockAction: RuleAction = { @@ -39,6 +40,7 @@ const mockAction: RuleAction = { throttle: null, }, uuid: '123-456', + type: RuleActionTypes.DEFAULT, }; const mockSummaryAction: RuleAction = { @@ -52,11 +54,11 @@ const mockSummaryAction: RuleAction = { throttle: '1d', }, uuid: '111-111', + type: RuleActionTypes.DEFAULT, }; -const mockSystemAction = { +const mockSystemAction: RuleSystemAction = { id: '1', - group: 'default', actionTypeId: '.test', params: {}, uuid: '123-456', @@ -71,8 +73,6 @@ describe('rule_action_helper', () => { }); test('should return false if the action is a system action', () => { - // TODO: Remove when system actions are introduced in types - // @ts-expect-error: cannot accept system actions at the moment const result = isSummaryAction(mockSystemAction); expect(result).toBe(false); }); @@ -105,8 +105,6 @@ describe('rule_action_helper', () => { }); test('should return false if the action is a system action', () => { - // TODO: Remove when system actions are introduced in types - // @ts-expect-error: cannot accept system actions at the moment const result = isActionOnInterval(mockSystemAction); expect(result).toBe(false); }); @@ -150,8 +148,6 @@ describe('rule_action_helper', () => { }); test('should return a hash for system actions action', () => { - // TODO: Remove when system actions are introduced in types - // @ts-expect-error: cannot accept system actions at the moment const result = generateActionHash(mockSystemAction); expect(result).toBe('system-action:.test:summary'); }); @@ -186,8 +182,6 @@ describe('rule_action_helper', () => { test('should filtered out system actions', () => { const result = getSummaryActionsFromTaskState({ - // TODO: Remove when system actions are introduced in types - // @ts-expect-error: cannot accept system actions at the moment actions: [mockSummaryAction, mockSystemAction], summaryActions: { '111-111': { date: new Date('01.01.2020').toISOString() }, @@ -236,8 +230,6 @@ describe('rule_action_helper', () => { }); test('should return false if the action is a system action', () => { - // TODO: Remove when system actions are introduced in types - // @ts-expect-error: cannot accept system actions at the moment const result = isSummaryActionThrottled({ action: mockSystemAction, logger }); expect(result).toBe(false); }); @@ -347,8 +339,6 @@ describe('rule_action_helper', () => { }); test('should return false if the action is a system action', () => { - // TODO: Remove when system actions are introduced in types - // @ts-expect-error: cannot accept system actions at the moment const result = isSummaryActionOnInterval(mockSystemAction); expect(result).toBe(false); }); @@ -381,8 +371,6 @@ describe('rule_action_helper', () => { }); test('returns undefined start and end action is a system action', () => { - // TODO: Remove when system actions are introduced in types - // @ts-expect-error: cannot accept system actions at the moment expect(getSummaryActionTimeBounds(mockSystemAction, { interval: '1m' }, null)).toEqual({ start: undefined, end: undefined, diff --git a/x-pack/plugins/alerting/server/task_runner/rule_action_helper.ts b/x-pack/plugins/alerting/server/task_runner/rule_action_helper.ts index 158d5bbcbcf37d..94d3486ec96aad 100644 --- a/x-pack/plugins/alerting/server/task_runner/rule_action_helper.ts +++ b/x-pack/plugins/alerting/server/task_runner/rule_action_helper.ts @@ -10,10 +10,11 @@ import { IntervalSchedule, parseDuration, RuleAction, + RuleDefaultAction, RuleNotifyWhenTypeValues, ThrottledActions, - RuleActionTypes, } from '../../common'; +import { isSystemAction } from '../../common/system_actions/is_system_action'; export const isSummaryAction = (action?: RuleAction) => { if (action != null && isSystemAction(action)) { @@ -108,7 +109,7 @@ export const getSummaryActionsFromTaskState = ({ summaryActions?: ThrottledActions; }) => { const actionsWithoutSystemActions = actions.filter( - (action): action is RuleAction => !isSystemAction(action) + (action): action is RuleDefaultAction => !isSystemAction(action) ); return Object.entries(summaryActions).reduce((newObj, [key, val]) => { @@ -154,15 +155,3 @@ export const getSummaryActionTimeBounds = ( return { start: startDate.valueOf(), end: now.valueOf() }; }; - -/** - * TODO: Substitute with a function which takes into - * account system actions. - * - * Because RuleAction has the type set as RuleActionTypes.DEFAULT - * TS produce an error as the check below will always return false. - * We need the check to be able to test. - */ - -// @ts-expect-error -const isSystemAction = (action: RuleAction) => action.type === RuleActionTypes.SYSTEM; diff --git a/x-pack/plugins/alerting/server/task_runner/rule_loader.ts b/x-pack/plugins/alerting/server/task_runner/rule_loader.ts index f6bb71aef74532..0218d246521555 100644 --- a/x-pack/plugins/alerting/server/task_runner/rule_loader.ts +++ b/x-pack/plugins/alerting/server/task_runner/rule_loader.ts @@ -25,8 +25,12 @@ import { import { MONITORING_HISTORY_LIMIT, RuleTypeParams } from '../../common'; import { AlertingEventLogger } from '../lib/alerting_event_logger/alerting_event_logger'; -export interface RuleData extends LoadedIndirectParams { - indirectParams: RawRule; +interface SerializableRawRule extends RawRule { + [key: string]: unknown; +} + +export interface RuleData + extends LoadedIndirectParams { rule: SanitizedRule; version: string | undefined; fakeRequest: CoreKibanaRequest; @@ -121,6 +125,7 @@ export async function getRuleAttributes( const fakeRequest = getFakeKibanaRequest(context, spaceId, rawRule.attributes.apiKey); const rulesClient = context.getRulesClientWithRequest(fakeRequest); + const rule = rulesClient.getAlertFromRaw({ id: ruleId, ruleTypeId: rawRule.attributes.alertTypeId as string, @@ -133,7 +138,7 @@ export async function getRuleAttributes( return { rule, version: rawRule.version, - indirectParams: rawRule.attributes, + indirectParams: rawRule.attributes as SerializableRawRule, fakeRequest, rulesClient, }; diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts b/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts index fe50e6e3632b61..2ccfcd84400b7f 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts @@ -17,6 +17,7 @@ import { Rule, RuleAction, RuleAlertData, + RuleActionTypes, } from '../types'; import { ConcreteTaskInstance, isUnrecoverableError } from '@kbn/task-manager-plugin/server'; import { TaskRunnerContext } from './task_runner_factory'; @@ -1525,6 +1526,7 @@ describe('Task Runner', () => { foo: true, }, uuid: '111-111', + type: RuleActionTypes.DEFAULT, }, { group: recoveryActionGroup.id, @@ -1534,6 +1536,7 @@ describe('Task Runner', () => { isResolved: true, }, uuid: '222-222', + type: RuleActionTypes.DEFAULT, }, ], }); @@ -1622,6 +1625,7 @@ describe('Task Runner', () => { foo: true, }, uuid: '111-111', + type: RuleActionTypes.DEFAULT, }, ], }); @@ -1685,6 +1689,7 @@ describe('Task Runner', () => { foo: true, }, uuid: '111-111', + type: RuleActionTypes.DEFAULT, }, ], }); diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner.ts b/x-pack/plugins/alerting/server/task_runner/task_runner.ts index 939dfe9406f698..142bfc215a3b16 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.ts @@ -50,6 +50,7 @@ import { RuleAlertData, SanitizedRule, RuleNotifyWhen, + RuleActionTypes, } from '../../common'; import { NormalizedRuleType, UntypedNormalizedRuleType } from '../rule_type_registry'; import { getEsErrorMessage } from '../lib/errors'; @@ -560,7 +561,12 @@ export class TaskRunner< flappingSettings, notifyOnActionGroupChange: notifyWhen === RuleNotifyWhen.CHANGE || - some(actions, (action) => action.frequency?.notifyWhen === RuleNotifyWhen.CHANGE), + some( + actions, + (action) => + action.type !== RuleActionTypes.SYSTEM && + action.frequency?.notifyWhen === RuleNotifyWhen.CHANGE + ), maintenanceWindowIds, }); }); diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner_cancel.test.ts b/x-pack/plugins/alerting/server/task_runner/task_runner_cancel.test.ts index 41140b5b25df78..f7528eaffe6547 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner_cancel.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner_cancel.test.ts @@ -111,6 +111,7 @@ describe('Task Runner Cancel', () => { const uiSettingsService = uiSettingsServiceMock.createStartContract(); const dataPlugin = dataPluginMock.createStartContract(); const inMemoryMetrics = inMemoryMetricsMock.create(); + const connectorAdapterRegistry = new ConnectorAdapterRegistry(); type TaskRunnerFactoryInitializerParamsType = jest.Mocked & { actionsPlugin: jest.Mocked; @@ -151,7 +152,7 @@ describe('Task Runner Cancel', () => { getMaintenanceWindowClientWithRequest: jest .fn() .mockReturnValue(maintenanceWindowClientMock.create()), - connectorAdapterRegistry: new ConnectorAdapterRegistry(), + connectorAdapterRegistry, }; beforeEach(() => { From 5024714b6c895fb688010d7a5d406ede0a408f06 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 3 Oct 2023 15:09:57 +0300 Subject: [PATCH 05/14] Pass the isSystemAction to the rules factory --- .../plugins/alerting/server/data/rule/types/rule_attributes.ts | 2 +- .../alerting/server/rules_client_conflict_retries.test.ts | 1 + x-pack/plugins/alerting/server/rules_client_factory.test.ts | 2 ++ x-pack/plugins/alerting/server/rules_client_factory.ts | 3 +++ 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts b/x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts index aa8adda873cdec..b0e2a58a72b21e 100644 --- a/x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts +++ b/x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts @@ -125,7 +125,7 @@ interface AlertsFilterAttributes { export interface RuleActionAttributes { uuid: string; - group: string; + group?: string; actionRef: string; actionTypeId: string; params: SavedObjectAttributes; diff --git a/x-pack/plugins/alerting/server/rules_client_conflict_retries.test.ts b/x-pack/plugins/alerting/server/rules_client_conflict_retries.test.ts index 62b6259afb43c8..8bde9a52b3805d 100644 --- a/x-pack/plugins/alerting/server/rules_client_conflict_retries.test.ts +++ b/x-pack/plugins/alerting/server/rules_client_conflict_retries.test.ts @@ -70,6 +70,7 @@ const rulesClientParams: jest.Mocked = { getAlertIndicesAlias: jest.fn(), alertsService: null, connectorAdapterRegistry: new ConnectorAdapterRegistry(), + isSystemAction: jest.fn(), }; // this suite consists of two suites running tests against mutable RulesClient APIs: diff --git a/x-pack/plugins/alerting/server/rules_client_factory.test.ts b/x-pack/plugins/alerting/server/rules_client_factory.test.ts index 31f06344d45f41..857a3e3de43aa8 100644 --- a/x-pack/plugins/alerting/server/rules_client_factory.test.ts +++ b/x-pack/plugins/alerting/server/rules_client_factory.test.ts @@ -119,6 +119,7 @@ test('creates a rules client with proper constructor arguments when security is getAlertIndicesAlias: expect.any(Function), alertsService: null, connectorAdapterRegistry: expect.any(ConnectorAdapterRegistry), + isSystemAction: expect.any(Function), }); }); @@ -164,6 +165,7 @@ test('creates a rules client with proper constructor arguments', async () => { getAlertIndicesAlias: expect.any(Function), alertsService: null, connectorAdapterRegistry: expect.any(ConnectorAdapterRegistry), + isSystemAction: expect.any(Function), }); }); diff --git a/x-pack/plugins/alerting/server/rules_client_factory.ts b/x-pack/plugins/alerting/server/rules_client_factory.ts index 337d4daab903be..fba3d89f5abf4b 100644 --- a/x-pack/plugins/alerting/server/rules_client_factory.ts +++ b/x-pack/plugins/alerting/server/rules_client_factory.ts @@ -185,6 +185,9 @@ export class RulesClientFactory { } return { apiKeysEnabled: false }; }, + isSystemAction(actionId: string) { + return actions.isSystemActionConnector(actionId); + }, }); } } From 3bda9b94c2327561388f2c61a286e750d8502975 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 3 Oct 2023 15:14:20 +0300 Subject: [PATCH 06/14] Add test utils for system actions --- .../alerting_api_integration/common/config.ts | 1 + .../common/lib/alert_utils.ts | 64 +++++++++- .../plugins/alerts/server/action_types.ts | 65 ++++++++++ .../alerts/server/connector_adapters.ts | 37 ++++++ .../common/plugins/alerts/server/plugin.ts | 2 + .../group2/tests/alerting/alerts.ts | 116 +++++++++++++++++- 6 files changed, 283 insertions(+), 2 deletions(-) create mode 100644 x-pack/test/alerting_api_integration/common/plugins/alerts/server/connector_adapters.ts diff --git a/x-pack/test/alerting_api_integration/common/config.ts b/x-pack/test/alerting_api_integration/common/config.ts index 7fdd7668135a49..2a19e31381fcd3 100644 --- a/x-pack/test/alerting_api_integration/common/config.ts +++ b/x-pack/test/alerting_api_integration/common/config.ts @@ -68,6 +68,7 @@ const enabledActionTypes = [ 'test.capped', 'test.system-action', 'test.system-action-kibana-privileges', + 'test.system-action-connector-adapter', ]; export function createTestConfig(name: string, options: CreateTestConfigOptions) { diff --git a/x-pack/test/alerting_api_integration/common/lib/alert_utils.ts b/x-pack/test/alerting_api_integration/common/lib/alert_utils.ts index 3307211694cc06..fb6d84e81772f5 100644 --- a/x-pack/test/alerting_api_integration/common/lib/alert_utils.ts +++ b/x-pack/test/alerting_api_integration/common/lib/alert_utils.ts @@ -6,7 +6,7 @@ */ import { ES_TEST_INDEX_NAME } from '@kbn/alerting-api-integration-helpers'; -import { AlertsFilter } from '@kbn/alerting-plugin/common/rule'; +import { AlertsFilter, RuleActionTypes } from '@kbn/alerting-plugin/common/rule'; import { Space, User } from '../types'; import { ObjectRemover } from './object_remover'; import { getUrlPrefix } from './space_test_utils'; @@ -351,6 +351,36 @@ export class AlertUtils { return response; } + public async createAlwaysFiringSystemAction({ + objectRemover, + overwrites = {}, + reference, + }: CreateAlertWithActionOpts) { + const objRemover = objectRemover || this.objectRemover; + + if (!objRemover) { + throw new Error('objectRemover is required'); + } + + let request = this.supertestWithoutAuth + .post(`${getUrlPrefix(this.space.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo'); + + if (this.user) { + request = request.auth(this.user.username, this.user.password); + } + + const rule = getAlwaysFiringRuleWithSystemAction(reference); + + const response = await request.send({ ...rule, ...overwrites }); + + if (response.statusCode === 200) { + objRemover.add(this.space.id, response.body.id, 'rule', 'alerting'); + } + + return response; + } + public async updateAlwaysFiringAction({ alertId, actionId, @@ -655,3 +685,35 @@ function getPatternFiringRuleWithSummaryAction( ], }; } + +function getAlwaysFiringRuleWithSystemAction(reference: string) { + return { + enabled: true, + name: 'abc', + schedule: { interval: '1m' }, + tags: ['tag-A', 'tag-B'], + rule_type_id: 'test.always-firing-alert-as-data', + consumer: 'alertsFixture', + params: { + index: ES_TEST_INDEX_NAME, + reference, + }, + actions: [ + { + id: 'system-connector-test.system-action-connector-adapter', + actionTypeId: 'test.system-action-connector-adapter', + uuid: '123', + /** + * The injected param required by the action will be set by the corresponding + * connector adapter. Setting it here it will lead to a 400 error by the + * rules API as only the connector adapter can set the injected property. + * + * Adapter: x-pack/test/alerting_api_integration/common/plugins/alerts/server/connector_adapters.ts + * Connector type: x-pack/test/alerting_api_integration/common/plugins/alerts/server/action_types.ts + */ + params: { myParam: 'param from rule action', index: ES_TEST_INDEX_NAME, reference }, + type: RuleActionTypes.SYSTEM, + }, + ], + }; +} diff --git a/x-pack/test/alerting_api_integration/common/plugins/alerts/server/action_types.ts b/x-pack/test/alerting_api_integration/common/plugins/alerts/server/action_types.ts index a7d5dbc138ea43..f2753cb99a24a3 100644 --- a/x-pack/test/alerting_api_integration/common/plugins/alerts/server/action_types.ts +++ b/x-pack/test/alerting_api_integration/common/plugins/alerts/server/action_types.ts @@ -80,6 +80,7 @@ export function defineActionTypes( */ actions.registerType(getSystemActionType()); actions.registerType(getSystemActionTypeWithKibanaPrivileges()); + actions.registerType(getSystemActionTypeWithConnectorAdapter()); /** Sub action framework */ @@ -484,3 +485,67 @@ function getSystemActionTypeWithKibanaPrivileges() { return result; } + +function getSystemActionTypeWithConnectorAdapter() { + const result: ActionType< + {}, + {}, + { myParam: string; injected: string; index?: string; reference?: string } + > = { + id: 'test.system-action-connector-adapter', + name: 'Test system action with a connector adapter set', + minimumLicenseRequired: 'platinum', + supportedFeatureIds: ['alerting'], + validate: { + params: { + /** + * The injected params will be set by the + * connector adapter while executing the action. + * + * Adapter: x-pack/test/alerting_api_integration/common/plugins/alerts/server/connector_adapters.ts + */ + schema: schema.object({ + myParam: schema.string(), + injected: schema.string(), + index: schema.maybe(schema.string()), + reference: schema.maybe(schema.string()), + }), + }, + + config: { + schema: schema.any(), + }, + secrets: { + schema: schema.any(), + }, + }, + isSystemActionType: true, + /** + * The executor writes a doc to the + * testing index. The test uses the doc + * to verify that the action is executed + * correctly + */ + async executor({ params, services, actionId }) { + const { index, reference } = params; + + if (index == null || reference == null) { + return { status: 'ok', actionId }; + } + + await services.scopedClusterClient.index({ + index, + refresh: 'wait_for', + body: { + params, + reference, + source: 'action:test.system-action-connector-adapter', + }, + }); + + return { status: 'ok', actionId }; + }, + }; + + return result; +} diff --git a/x-pack/test/alerting_api_integration/common/plugins/alerts/server/connector_adapters.ts b/x-pack/test/alerting_api_integration/common/plugins/alerts/server/connector_adapters.ts new file mode 100644 index 00000000000000..41526e0949de3e --- /dev/null +++ b/x-pack/test/alerting_api_integration/common/plugins/alerts/server/connector_adapters.ts @@ -0,0 +1,37 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { ConnectorAdapter } from '@kbn/alerting-plugin/server'; +import { CoreSetup } from '@kbn/core/server'; +import { schema } from '@kbn/config-schema'; +import { FixtureStartDeps, FixtureSetupDeps } from './plugin'; + +export function defineConnectorAdapters( + core: CoreSetup, + { alerting }: Pick +) { + const systemActionConnectorAdapter: ConnectorAdapter = { + connectorTypeId: 'test.system-action-connector-adapter', + ruleActionParamsSchema: schema.object({ + myParam: schema.string(), + index: schema.maybe(schema.string()), + reference: schema.maybe(schema.string()), + }), + /** + * The connector adapter will inject a new param property which is required + * by the action. The injected value cannot be set in the actions of the rule + * as the schema validation will thrown an error. Only through the connector + * adapter this value can be set. The tests are using the connector adapter test + * that the new property is injected correctly + */ + buildActionParams: ({ alerts, rule, params, spaceId, ruleUrl }) => { + return { ...params, injected: 'param from connector adapter' }; + }, + }; + + alerting.registerConnectorAdapter(systemActionConnectorAdapter); +} diff --git a/x-pack/test/alerting_api_integration/common/plugins/alerts/server/plugin.ts b/x-pack/test/alerting_api_integration/common/plugins/alerts/server/plugin.ts index 7a257d214f26a4..c24a15682649fe 100644 --- a/x-pack/test/alerting_api_integration/common/plugins/alerts/server/plugin.ts +++ b/x-pack/test/alerting_api_integration/common/plugins/alerts/server/plugin.ts @@ -27,6 +27,7 @@ import { NotificationsPluginStart } from '@kbn/notifications-plugin/server'; import { defineRoutes } from './routes'; import { defineActionTypes } from './action_types'; import { defineAlertTypes } from './alert_types'; +import { defineConnectorAdapters } from './connector_adapters'; export interface FixtureSetupDeps { features: FeaturesPluginSetup; @@ -162,6 +163,7 @@ export class FixturePlugin implements Plugin objectRemover.removeAll()); + after(async () => { await esTestIndexTool.destroy(); await es.indices.delete({ index: authorizationIndex }); @@ -1866,6 +1868,76 @@ instanceStateValue: true }); }); } + + describe('connector adapters', () => { + const space = SuperuserAtSpace1.space; + + const connectorId = 'system-connector-test.system-action-connector-adapter'; + const name = 'System action: test.system-action-connector-adapter'; + + it('should use connector adapters correctly on system actions', async () => { + const alertUtils = new AlertUtils({ + supertestWithoutAuth, + objectRemover, + space, + user: SuperuserAtSpace1.user, + }); + + const startDate = new Date().toISOString(); + const reference = alertUtils.generateReference(); + /** + * Creates a rule that always fire with a system action + * that has configured a connector adapter. + * + * System action: x-pack/test/alerting_api_integration/common/plugins/alerts/server/action_types.ts + * Adapter: x-pack/test/alerting_api_integration/common/plugins/alerts/server/connector_adapters.ts + */ + const response = await alertUtils.createAlwaysFiringSystemAction({ + reference, + overwrites: { schedule: { interval: '1s' } }, + }); + + expect(response.status).to.eql(200); + + await validateSystemActionEventLog({ + spaceId: space.id, + connectorId, + outcome: 'success', + message: `action executed: test.system-action-connector-adapter:${connectorId}: ${name}`, + startDate, + }); + + /** + * The executor function of the system action + * writes the params in the test index. We + * get the doc to verify that the connector adapter + * injected the param correctly. + */ + await esTestIndexTool.waitForDocs( + 'action:test.system-action-connector-adapter', + reference, + 1 + ); + + const docs = await esTestIndexTool.search( + 'action:test.system-action-connector-adapter', + reference + ); + + const doc = docs.body.hits.hits[0]._source as { params: Record }; + + expect(doc.params).to.eql({ + myParam: 'param from rule action', + index: '.kibana-alerting-test-data', + reference: 'alert-utils-ref:1:superuser', + /** + * Param was injected by the connector adapter in + * x-pack/test/alerting_api_integration/common/plugins/alerts/server/connector_adapters.ts + */ + injected: 'param from connector adapter', + }); + }); + }); }); interface ValidateEventLogParams { @@ -1970,4 +2042,46 @@ instanceStateValue: true expect(event?.error?.message).to.eql(errorMessage); } } + + interface ValidateSystemActionEventLogParams { + spaceId: string; + connectorId: string; + outcome: string; + message: string; + startDate: string; + errorMessage?: string; + } + + const validateSystemActionEventLog = async ( + params: ValidateSystemActionEventLogParams + ): Promise => { + const { spaceId, connectorId, outcome, message, startDate, errorMessage } = params; + + const events: IValidatedEvent[] = await retry.try(async () => { + const events_ = await getEventLog({ + getService, + spaceId, + type: 'action', + id: connectorId, + provider: 'actions', + actions: new Map([['execute', { gte: 1 }]]), + }); + + const filteredEvents = events_.filter((event) => event!['@timestamp']! >= startDate); + if (filteredEvents.length < 1) throw new Error('no recent events found yet'); + + return filteredEvents; + }); + + expect(events.length).to.be(1); + + const event = events[0]; + + expect(event?.message).to.eql(message); + expect(event?.event?.outcome).to.eql(outcome); + + if (errorMessage) { + expect(event?.error?.message).to.eql(errorMessage); + } + }; } From d68f1cb03831c57bfdfa71f5c4e1ff9f13295757 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 3 Oct 2023 14:34:24 +0000 Subject: [PATCH 07/14] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- x-pack/plugins/alerting/server/types.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/alerting/server/types.ts b/x-pack/plugins/alerting/server/types.ts index 1e5e91ad764fa5..c212c1feaa00d5 100644 --- a/x-pack/plugins/alerting/server/types.ts +++ b/x-pack/plugins/alerting/server/types.ts @@ -57,7 +57,6 @@ import { AlertsFilter, AlertsFilterTimeframe, RuleAlertData, - RuleActionResponse, } from '../common'; import { PublicAlertFactory } from './alert/create_alert_factory'; import { RulesSettingsFlappingProperties } from '../common/rules_settings'; From f9dccc5b1932625cb0e401ae2bb9051e745b04c3 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Fri, 6 Oct 2023 15:59:12 +0300 Subject: [PATCH 08/14] Do not persist the type of the action in ES --- .../common/routes/rule/response/schemas/v1.ts | 44 ++++++++- .../rule/methods/create/create_rule.test.ts | 34 ++++++- .../rule/methods/create/create_rule.ts | 22 ++--- ...form_domain_actions_to_raw_actions.test.ts | 99 +++++++++++++++++++ ...transform_domain_actions_to_raw_actions.ts | 33 +++++++ ...form_raw_actions_to_domain_actions.test.ts | 26 ----- ...transform_raw_actions_to_domain_actions.ts | 2 +- ...ransform_rule_domain_to_rule_attributes.ts | 28 ++++-- .../transforms/transform_create_body/v1.ts | 24 ++++- .../rules_client/lib/denormalize_actions.ts | 17 +++- .../rules_client/lib/extract_references.ts | 6 +- .../rules_client/lib/validate_actions.test.ts | 95 +++++++++++++----- .../rules_client/lib/validate_actions.ts | 27 +++-- 13 files changed, 362 insertions(+), 95 deletions(-) create mode 100644 x-pack/plugins/alerting/server/application/rule/transforms/transform_domain_actions_to_raw_actions.test.ts create mode 100644 x-pack/plugins/alerting/server/application/rule/transforms/transform_domain_actions_to_raw_actions.ts diff --git a/x-pack/plugins/alerting/common/routes/rule/response/schemas/v1.ts b/x-pack/plugins/alerting/common/routes/rule/response/schemas/v1.ts index 1c093314f7a471..daa90b84faad72 100644 --- a/x-pack/plugins/alerting/common/routes/rule/response/schemas/v1.ts +++ b/x-pack/plugins/alerting/common/routes/rule/response/schemas/v1.ts @@ -6,7 +6,6 @@ */ import { schema } from '@kbn/config-schema'; -import { rRuleResponseSchemaV1 } from '../../../r_rule'; import { ruleNotifyWhen as ruleNotifyWhenV1, ruleExecutionStatusValues as ruleExecutionStatusValuesV1, @@ -72,7 +71,7 @@ const actionAlertsFilterSchema = schema.object({ const actionSchema = schema.object({ uuid: schema.maybe(schema.string()), - group: schema.string(), + group: schema.maybe(schema.string()), id: schema.string(), connector_type_id: schema.string(), params: actionParamsSchema, @@ -181,9 +180,48 @@ export const monitoringSchema = schema.object({ }), }); +export const rRuleSchema = schema.object({ + dtstart: schema.string(), + tzid: schema.string(), + freq: schema.maybe( + schema.oneOf([ + schema.literal(0), + schema.literal(1), + schema.literal(2), + schema.literal(3), + schema.literal(4), + schema.literal(5), + schema.literal(6), + ]) + ), + until: schema.maybe(schema.string()), + count: schema.maybe(schema.number()), + interval: schema.maybe(schema.number()), + wkst: schema.maybe( + schema.oneOf([ + schema.literal('MO'), + schema.literal('TU'), + schema.literal('WE'), + schema.literal('TH'), + schema.literal('FR'), + schema.literal('SA'), + schema.literal('SU'), + ]) + ), + byweekday: schema.maybe(schema.arrayOf(schema.oneOf([schema.string(), schema.number()]))), + bymonth: schema.maybe(schema.arrayOf(schema.number())), + bysetpos: schema.maybe(schema.arrayOf(schema.number())), + bymonthday: schema.arrayOf(schema.number()), + byyearday: schema.arrayOf(schema.number()), + byweekno: schema.arrayOf(schema.number()), + byhour: schema.arrayOf(schema.number()), + byminute: schema.arrayOf(schema.number()), + bysecond: schema.arrayOf(schema.number()), +}); + export const ruleSnoozeScheduleSchema = schema.object({ duration: schema.number(), - rRule: rRuleResponseSchemaV1, + rRule: rRuleSchema, id: schema.maybe(schema.string()), skipRecurrences: schema.maybe(schema.arrayOf(schema.string())), }); diff --git a/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.test.ts b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.test.ts index bc3a9c1f39a689..d0985ebe682cb0 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.test.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.test.ts @@ -88,6 +88,8 @@ const rulesClientParams: jest.Mocked = { getAuthenticationAPIKey: jest.fn(), getAlertIndicesAlias: jest.fn(), alertsService: null, + connectorAdapterRegistry, + isSystemAction: jest.fn(), }; beforeEach(() => { @@ -1029,10 +1031,10 @@ describe('create()', () => { isSystemAction: false, }, ]); + actionsClient.isPreconfigured.mockReset(); - actionsClient.isPreconfigured.mockReturnValueOnce(false); - actionsClient.isPreconfigured.mockReturnValueOnce(true); - actionsClient.isPreconfigured.mockReturnValueOnce(false); + actionsClient.isPreconfigured.mockImplementation((id) => id === 'preconfigured'); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', @@ -1222,7 +1224,7 @@ describe('create()', () => { ], } ); - expect(actionsClient.isPreconfigured).toHaveBeenCalledTimes(3); + expect(actionsClient.isPreconfigured).toHaveBeenCalledTimes(6); }); test('creates a disabled alert', async () => { @@ -1333,6 +1335,7 @@ describe('create()', () => { async executor() { return { state: {} }; }, + category: 'test', producer: 'alerts', useSavedObjectReferences: { extractReferences: extractReferencesFn, @@ -1341,6 +1344,7 @@ describe('create()', () => { validate: { params: { validate: (params) => params }, }, + validLegacyConsumers: [], })); const data = getMockData({ params: ruleParams, @@ -1523,6 +1527,8 @@ describe('create()', () => { async executor() { return { state: {} }; }, + category: 'test', + validLegacyConsumers: [], producer: 'alerts', useSavedObjectReferences: { extractReferences: extractReferencesFn, @@ -2352,6 +2358,8 @@ describe('create()', () => { name: 'Default', }, ], + category: 'test', + validLegacyConsumers: [], defaultActionGroupId: 'default', recoveryActionGroup: RecoveredActionGroup, validate: { @@ -2837,6 +2845,8 @@ describe('create()', () => { async executor() { return { state: {} }; }, + category: 'test', + validLegacyConsumers: [], producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -2910,6 +2920,8 @@ describe('create()', () => { async executor() { return { state: {} }; }, + category: 'test', + validLegacyConsumers: [], producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -2948,6 +2960,8 @@ describe('create()', () => { async executor() { return { state: {} }; }, + category: 'test', + validLegacyConsumers: [], producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3044,6 +3058,8 @@ describe('create()', () => { async executor() { return { state: {} }; }, + category: 'test', + validLegacyConsumers: [], producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3094,6 +3110,8 @@ describe('create()', () => { async executor() { return { state: {} }; }, + category: 'test', + validLegacyConsumers: [], producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3158,6 +3176,8 @@ describe('create()', () => { async executor() { return { state: {} }; }, + category: 'test', + validLegacyConsumers: [], producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3241,6 +3261,8 @@ describe('create()', () => { async executor() { return { state: {} }; }, + category: 'test', + validLegacyConsumers: [], producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3449,6 +3471,8 @@ describe('create()', () => { async executor() { return { state: {} }; }, + category: 'test', + validLegacyConsumers: [], producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), @@ -3507,6 +3531,8 @@ describe('create()', () => { async executor() { return { state: {} }; }, + category: 'test', + validLegacyConsumers: [], producer: 'alerts', useSavedObjectReferences: { extractReferences: jest.fn(), diff --git a/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts index 2a1f75d434e197..7c36d8b382bf5c 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts @@ -150,12 +150,9 @@ export async function createRule( } // Extract saved object references for this rule - const { - references, - params: updatedParams, - actions, - } = await withSpan({ name: 'extractReferences', type: 'rules' }, () => - extractReferences(context, ruleType, data.actions, validatedAlertTypeParams) + const { references, params: updatedParams } = await withSpan( + { name: 'extractReferences', type: 'rules' }, + () => extractReferences(context, ruleType, data.actions, validatedAlertTypeParams) ); const createTime = Date.now(); @@ -165,8 +162,10 @@ export async function createRule( const throttle = data.throttle ?? null; // Convert domain rule object to ES rule attributes - const ruleAttributes = transformRuleDomainToRuleAttributes( - { + const ruleAttributes = await transformRuleDomainToRuleAttributes({ + actions: data.actions, + context, + rule: { ...data, // TODO (http-versioning) create a rule domain version of this function // Right now this works because the 2 types can interop but it's not ideal @@ -186,12 +185,11 @@ export async function createRule( revision: 0, running: false, }, - { + params: { legacyId, - actionsWithRefs: actions, paramsWithRefs: updatedParams, - } - ); + }, + }); const createdRuleSavedObject: SavedObject = await withSpan( { name: 'createRuleSavedObject', type: 'rules' }, diff --git a/x-pack/plugins/alerting/server/application/rule/transforms/transform_domain_actions_to_raw_actions.test.ts b/x-pack/plugins/alerting/server/application/rule/transforms/transform_domain_actions_to_raw_actions.test.ts new file mode 100644 index 00000000000000..a167f2ba3a2a7c --- /dev/null +++ b/x-pack/plugins/alerting/server/application/rule/transforms/transform_domain_actions_to_raw_actions.test.ts @@ -0,0 +1,99 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { actionsClientMock } from '@kbn/actions-plugin/server/mocks'; +import { RuleActionTypes } from '../../../../common'; +import { transformDomainActionsToRawActions } from './transform_domain_actions_to_raw_actions'; + +const defaultAction = { + id: 'test-default-action', + group: 'default', + uuid: '1', + actionTypeId: '.test', + params: {}, + frequency: { + summary: false, + notifyWhen: 'onThrottleInterval' as const, + throttle: '1m', + }, + alertsFilter: { query: { kql: 'test:1', dsl: '{}', filters: [] } }, + type: RuleActionTypes.DEFAULT, +}; + +const systemAction = { + id: 'my-system-action-id', + uuid: '123', + actionTypeId: '.test-system-action', + params: {}, + type: RuleActionTypes.SYSTEM, +}; + +const actionsClient = actionsClientMock.create(); +actionsClient.isSystemAction.mockImplementation((id) => id === systemAction.id); + +const context = { + getActionsClient: jest.fn().mockResolvedValue(actionsClient), +}; + +describe('transformDomainActionsToRawActions', () => { + actionsClient.getBulk.mockResolvedValue([ + { + id: 'test-default-action', + actionTypeId: '.test', + name: 'Default action', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: false, + }, + { + id: 'my-system-action-id', + actionTypeId: '.test-system-action', + name: 'System action', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: true, + }, + ]); + + it('transforms the actions correctly', async () => { + const res = await transformDomainActionsToRawActions({ + actions: [defaultAction, systemAction], + // @ts-ignore: no need to pass all properties of the context + context, + }); + + expect(res).toMatchInlineSnapshot(` + Array [ + Object { + "actionRef": "action_0", + "actionTypeId": ".test", + "alertsFilter": Object { + "query": Object { + "dsl": "{}", + "filters": Array [], + "kql": "test:1", + }, + }, + "frequency": Object { + "notifyWhen": "onThrottleInterval", + "summary": false, + "throttle": "1m", + }, + "group": "default", + "params": Object {}, + "uuid": "1", + }, + Object { + "actionRef": "system_action:my-system-action-id", + "actionTypeId": ".test-system-action", + "params": Object {}, + "uuid": "123", + }, + ] + `); + }); +}); diff --git a/x-pack/plugins/alerting/server/application/rule/transforms/transform_domain_actions_to_raw_actions.ts b/x-pack/plugins/alerting/server/application/rule/transforms/transform_domain_actions_to_raw_actions.ts new file mode 100644 index 00000000000000..e3fcce033ca38f --- /dev/null +++ b/x-pack/plugins/alerting/server/application/rule/transforms/transform_domain_actions_to_raw_actions.ts @@ -0,0 +1,33 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { RuleAttributes } from '../../../data/rule/types'; +import { + NormalizedAlertActionWithGeneratedValues, + RulesClientContext, +} from '../../../rules_client'; +import { denormalizeActions } from '../../../rules_client/lib/denormalize_actions'; + +interface Args { + actions: NormalizedAlertActionWithGeneratedValues[]; + context: RulesClientContext; +} + +export const transformDomainActionsToRawActions = async ({ + actions, + context, +}: Args): Promise => { + const { actions: actionsWithExtractedRefs } = await denormalizeActions(context, actions); + + const actionsWithoutType = actionsWithExtractedRefs.map((action) => { + const { type, ...restAction } = action; + + return restAction; + }); + + return actionsWithoutType; +}; diff --git a/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.test.ts b/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.test.ts index eb9f21979e4fbd..bbe84ed2534ce5 100644 --- a/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.test.ts +++ b/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.test.ts @@ -5,35 +5,9 @@ * 2.0. */ -import { RecoveredActionGroup } from '../../../../common'; -import { UntypedNormalizedRuleType } from '../../../rule_type_registry'; import { RuleActionAttributes } from '../../../data/rule/types'; import { transformRawActionsToDomainActions } from './transform_raw_actions_to_domain_actions'; -const ruleType: jest.Mocked = { - id: 'test.rule-type', - name: 'My test rule', - actionGroups: [{ id: 'default', name: 'Default' }, RecoveredActionGroup], - defaultActionGroupId: 'default', - minimumLicenseRequired: 'basic', - isExportable: true, - recoveryActionGroup: RecoveredActionGroup, - executor: jest.fn(), - producer: 'alerts', - cancelAlertsOnRuleTimeout: true, - ruleTaskTimeout: '5m', - autoRecoverAlerts: true, - doesSetRecoveryContext: true, - validate: { - params: { validate: (params) => params }, - }, - alerts: { - context: 'test', - mappings: { fieldMap: { field: { type: 'keyword', required: false } } }, - shouldWrite: true, - }, -}; - const defaultAction: RuleActionAttributes = { group: 'default', uuid: '1', diff --git a/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.ts b/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.ts index d6c7533befdf10..aad02639e69c78 100644 --- a/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.ts +++ b/x-pack/plugins/alerting/server/application/rule/transforms/transform_raw_actions_to_domain_actions.ts @@ -26,7 +26,7 @@ export const transformRawActionsToDomainActions = ({ references, omitGeneratedValues = true, isSystemAction, -}: Args) => { +}: Args): RuleDomain['actions'] => { const actionsWithInjectedRefs = actions ? injectReferencesIntoActions(ruleId, actions, references || []) : []; diff --git a/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_domain_to_rule_attributes.ts b/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_domain_to_rule_attributes.ts index 20eef8f851e082..fc69ff9a73ec4a 100644 --- a/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_domain_to_rule_attributes.ts +++ b/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_domain_to_rule_attributes.ts @@ -4,24 +4,40 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ +import { + NormalizedAlertActionWithGeneratedValues, + RulesClientContext, +} from '../../../rules_client'; import { RuleDomain } from '../types'; import { RuleAttributes } from '../../../data/rule/types'; import { getMappedParams } from '../../../rules_client/common'; +import { transformDomainActionsToRawActions } from './transform_domain_actions_to_raw_actions'; interface TransformRuleToEsParams { legacyId: RuleAttributes['legacyId']; - actionsWithRefs: RuleAttributes['actions']; paramsWithRefs: RuleAttributes['params']; meta?: RuleAttributes['meta']; } -export const transformRuleDomainToRuleAttributes = ( - rule: Omit, - params: TransformRuleToEsParams -): RuleAttributes => { - const { legacyId, actionsWithRefs, paramsWithRefs, meta } = params; +export const transformRuleDomainToRuleAttributes = async ({ + actions, + rule, + params, + context, +}: { + actions: NormalizedAlertActionWithGeneratedValues[]; + rule: Omit; + params: TransformRuleToEsParams; + context: RulesClientContext; +}): Promise => { + const { legacyId, paramsWithRefs, meta } = params; const mappedParams = getMappedParams(paramsWithRefs); + const actionsWithRefs = await transformDomainActionsToRawActions({ + actions, + context, + }); + return { name: rule.name, tags: rule.tags, diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/create/transforms/transform_create_body/v1.ts b/x-pack/plugins/alerting/server/routes/rule/apis/create/transforms/transform_create_body/v1.ts index c6b29f4577f4c3..51ccc96d4e9c6c 100644 --- a/x-pack/plugins/alerting/server/routes/rule/apis/create/transforms/transform_create_body/v1.ts +++ b/x-pack/plugins/alerting/server/routes/rule/apis/create/transforms/transform_create_body/v1.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { RuleActionTypes } from '../../../../../../../common'; import type { CreateRuleActionV1, CreateRuleRequestBodyV1, @@ -12,12 +13,25 @@ import type { import type { CreateRuleData } from '../../../../../../application/rule/methods/create'; import type { RuleParams } from '../../../../../../application/rule/types'; -const transformCreateBodyActions = (actions: CreateRuleActionV1[]): CreateRuleData['actions'] => { +const transformCreateBodyActions = ( + actions: CreateRuleActionV1[], + isSystemAction: (connectorId: string) => boolean +): CreateRuleData['actions'] => { if (!actions) return []; return actions.map(({ frequency, alerts_filter: alertsFilter, ...action }) => { + if (isSystemAction(action.id)) { + return { + id: action.id, + params: action.params, + actionTypeId: action.actionTypeId, + ...(action.uuid ? { uuid: action.uuid } : {}), + type: RuleActionTypes.SYSTEM, + }; + } + return { - group: action.group, + group: action.group ?? 'default', id: action.id, params: action.params, actionTypeId: action.actionTypeId, @@ -32,12 +46,14 @@ const transformCreateBodyActions = (actions: CreateRuleActionV1[]): CreateRuleDa } : {}), ...(alertsFilter ? { alertsFilter } : {}), + type: RuleActionTypes.DEFAULT, }; }); }; export const transformCreateBody = ( - createBody: CreateRuleRequestBodyV1 + createBody: CreateRuleRequestBodyV1, + isSystemAction: (connectorId: string) => boolean ): CreateRuleData => { return { name: createBody.name, @@ -48,7 +64,7 @@ export const transformCreateBody = ( ...(createBody.throttle ? { throttle: createBody.throttle } : {}), params: createBody.params, schedule: createBody.schedule, - actions: transformCreateBodyActions(createBody.actions), + actions: transformCreateBodyActions(createBody.actions, isSystemAction), ...(createBody.notify_when ? { notifyWhen: createBody.notify_when } : {}), }; }; diff --git a/x-pack/plugins/alerting/server/rules_client/lib/denormalize_actions.ts b/x-pack/plugins/alerting/server/rules_client/lib/denormalize_actions.ts index db1dca186aee9f..5652357510b533 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/denormalize_actions.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/denormalize_actions.ts @@ -5,20 +5,26 @@ * 2.0. */ +import { DistributiveOmit } from '@elastic/eui'; import { SavedObjectReference } from '@kbn/core/server'; -import { RawRule } from '../../types'; import { preconfiguredConnectorActionRefPrefix, systemConnectorActionRefPrefix, } from '../common/constants'; import { NormalizedAlertActionWithGeneratedValues, RulesClientContext } from '../types'; +type DenormalizedAction = DistributiveOmit & { + actionRef: string; + actionTypeId: string; +}; + export async function denormalizeActions( context: RulesClientContext, alertActions: NormalizedAlertActionWithGeneratedValues[] -): Promise<{ actions: RawRule['actions']; references: SavedObjectReference[] }> { +): Promise<{ actions: DenormalizedAction[]; references: SavedObjectReference[] }> { const references: SavedObjectReference[] = []; - const actions: RawRule['actions'] = []; + const actions: DenormalizedAction[] = []; + if (alertActions.length) { const actionsClient = await context.getActionsClient(); const actionIds = [...new Set(alertActions.map((alertAction) => alertAction.id))]; @@ -29,12 +35,15 @@ export async function denormalizeActions( }); const actionTypeIds = [...new Set(actionResults.map((action) => action.actionTypeId))]; + actionTypeIds.forEach((id) => { // Notify action type usage via "isActionTypeEnabled" function actionsClient.isActionTypeEnabled(id, { notifyUsage: true }); }); + alertActions.forEach(({ id, ...alertAction }, i) => { const actionResultValue = actionResults.find((action) => action.id === id); + if (actionResultValue) { if (actionsClient.isPreconfigured(id)) { actions.push({ @@ -50,11 +59,13 @@ export async function denormalizeActions( }); } else { const actionRef = `action_${i}`; + references.push({ id, name: actionRef, type: 'action', }); + actions.push({ ...alertAction, actionRef, diff --git a/x-pack/plugins/alerting/server/rules_client/lib/extract_references.ts b/x-pack/plugins/alerting/server/rules_client/lib/extract_references.ts index a7525cf9b5b191..c3e7b5cd2a682f 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/extract_references.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/extract_references.ts @@ -6,7 +6,7 @@ */ import { SavedObjectReference } from '@kbn/core/server'; -import { RawRule, RuleTypeParams } from '../../types'; +import { RuleTypeParams } from '../../types'; import { UntypedNormalizedRuleType } from '../../rule_type_registry'; import { NormalizedAlertActionWithGeneratedValues } from '../types'; import { extractedSavedObjectParamReferenceNamePrefix } from '../common/constants'; @@ -22,11 +22,10 @@ export async function extractReferences< ruleActions: NormalizedAlertActionWithGeneratedValues[], ruleParams: Params ): Promise<{ - actions: RawRule['actions']; params: ExtractedParams; references: SavedObjectReference[]; }> { - const { references: actionReferences, actions } = await denormalizeActions(context, ruleActions); + const { references: actionReferences } = await denormalizeActions(context, ruleActions); // Extracts any references using configured reference extractor if available const extractedRefsAndParams = ruleType?.useSavedObjectReferences?.extractReferences @@ -44,7 +43,6 @@ export async function extractReferences< const references = [...actionReferences, ...paramReferences]; return { - actions, params, references, }; diff --git a/x-pack/plugins/alerting/server/rules_client/lib/validate_actions.test.ts b/x-pack/plugins/alerting/server/rules_client/lib/validate_actions.test.ts index 74fd3b3291d4ea..ac09a788b528b1 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/validate_actions.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/validate_actions.test.ts @@ -7,8 +7,14 @@ import { validateActions, ValidateActionsData } from './validate_actions'; import { UntypedNormalizedRuleType } from '../../rule_type_registry'; -import { AlertsFilter, RecoveredActionGroup, RuleNotifyWhen } from '../../../common'; -import { RulesClientContext } from '..'; +import { + AlertsFilter, + RecoveredActionGroup, + RuleActionTypes, + RuleDefaultAction, + RuleNotifyWhen, +} from '../../../common'; +import { NormalizedAlertAction, RulesClientContext } from '..'; describe('validateActions', () => { const loggerErrorMock = jest.fn(); @@ -22,7 +28,6 @@ describe('validateActions', () => { isExportable: true, recoveryActionGroup: RecoveredActionGroup, executor: jest.fn(), - category: 'test', producer: 'alerts', cancelAlertsOnRuleTimeout: true, ruleTaskTimeout: '5m', @@ -33,28 +38,35 @@ describe('validateActions', () => { context: 'context', mappings: { fieldMap: { field: { type: 'fieldType', required: false } } }, }, - validLegacyConsumers: [], + }; + + const defaultAction: NormalizedAlertAction = { + uuid: '111', + group: 'default', + id: '1', + params: {}, + frequency: { + summary: false, + notifyWhen: RuleNotifyWhen.ACTIVE, + throttle: null, + }, + alertsFilter: { + query: { kql: 'test:1', filters: [] }, + timeframe: { days: [1], hours: { start: '10:00', end: '17:00' }, timezone: 'UTC' }, + }, + type: RuleActionTypes.DEFAULT, + }; + + const systemAction: NormalizedAlertAction = { + uuid: '111', + id: '1', + params: {}, + type: RuleActionTypes.SYSTEM, }; const data = { schedule: { interval: '1m' }, - actions: [ - { - uuid: '111', - group: 'default', - id: '1', - params: {}, - frequency: { - summary: false, - notifyWhen: RuleNotifyWhen.ACTIVE, - throttle: null, - }, - alertsFilter: { - query: { kql: 'test:1', filters: [] }, - timeframe: { days: [1], hours: { start: '10:00', end: '17:00' }, timezone: 'UTC' }, - }, - }, - ], + actions: [defaultAction], } as unknown as ValidateActionsData; const context = { @@ -91,6 +103,22 @@ describe('validateActions', () => { ); }); + it('should return error message if actions have duplicated uuid and there is a system action', async () => { + await expect( + validateActions( + context as unknown as RulesClientContext, + ruleType, + { + ...data, + actions: [defaultAction, systemAction], + }, + false + ) + ).rejects.toThrowErrorMatchingInlineSnapshot( + '"Failed to validate actions due to the following error: Actions have duplicated UUIDs"' + ); + }); + it('should return error message if any action have isMissingSecrets', async () => { getBulkMock.mockResolvedValue([{ isMissingSecrets: true, name: 'test name' }]); await expect( @@ -105,7 +133,7 @@ describe('validateActions', () => { validateActions( context as unknown as RulesClientContext, ruleType, - { ...data, actions: [{ ...data.actions[0], group: 'invalid' }] }, + { ...data, actions: [{ ...data.actions[0], group: 'invalid' } as RuleDefaultAction] }, false ) ).rejects.toThrowErrorMatchingInlineSnapshot( @@ -144,7 +172,7 @@ describe('validateActions', () => { validateActions( context as unknown as RulesClientContext, ruleType, - { ...data, actions: [{ ...data.actions[0], frequency: undefined }] }, + { ...data, actions: [{ ...data.actions[0], frequency: undefined } as RuleDefaultAction] }, false ) ).rejects.toThrowErrorMatchingInlineSnapshot( @@ -163,7 +191,7 @@ describe('validateActions', () => { { ...data.actions[0], frequency: { summary: false, notifyWhen: 'onThrottleInterval', throttle: '1s' }, - }, + } as RuleDefaultAction, ], }, false @@ -184,7 +212,7 @@ describe('validateActions', () => { { ...data.actions[0], alertsFilter: {} as AlertsFilter, - }, + } as RuleDefaultAction, ], }, false @@ -208,7 +236,7 @@ describe('validateActions', () => { query: { kql: 'test:1', filters: [] }, timeframe: { days: [1], hours: { start: '30:00', end: '17:00' }, timezone: 'UTC' }, }, - }, + } as NormalizedAlertAction, ], }, false @@ -255,6 +283,7 @@ describe('validateActions', () => { '"Failed to validate actions due to the following error: Action\'s alertsFilter timeframe has missing fields: days, hours or timezone: 111"' ); }); + it('should return error message if any action has alertsFilter timeframe has invalid days', async () => { await expect( validateActions( @@ -283,4 +312,18 @@ describe('validateActions', () => { '"Failed to validate actions due to the following error: Action\'s alertsFilter days has invalid values: (111:[0,8]) "' ); }); + + it('should not throw an error on system actions that do not contain properties like frequency or group', async () => { + const res = await validateActions( + context as unknown as RulesClientContext, + ruleType, + { + ...data, + actions: [systemAction], + }, + false + ); + + expect(res).toBe(undefined); + }); }); diff --git a/x-pack/plugins/alerting/server/rules_client/lib/validate_actions.ts b/x-pack/plugins/alerting/server/rules_client/lib/validate_actions.ts index 5a1e9911b5fcbe..52c9075b6fa185 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/validate_actions.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/validate_actions.ts @@ -9,7 +9,7 @@ import Boom from '@hapi/boom'; import { map } from 'lodash'; import { i18n } from '@kbn/i18n'; import { validateHours } from '../../routes/lib/validate_hours'; -import { RawRule, RuleNotifyWhen } from '../../types'; +import { RuleDefaultAction, RawRule, RuleActionTypes, RuleNotifyWhen } from '../../types'; import { UntypedNormalizedRuleType } from '../../rule_type_registry'; import { NormalizedAlertAction } from '../types'; import { RulesClientContext } from '../types'; @@ -43,9 +43,18 @@ export async function validateActions( ); } + /** + * All the validation below are not applicable for system actions + * as users are not allowed to set fields like the group + * or the throttle + */ + const actionsWithoutSystemActions = actions.filter( + (action): action is RuleDefaultAction => action.type !== RuleActionTypes.SYSTEM + ); + // check for actions using connectors with missing secrets const actionsClient = await context.getActionsClient(); - const actionIds = [...new Set(actions.map((action) => action.id))]; + const actionIds = [...new Set(actionsWithoutSystemActions.map((action) => action.id))]; const actionResults = (await actionsClient.getBulk({ ids: actionIds, throwIfSystemAction: false })) || []; @@ -76,7 +85,7 @@ export async function validateActions( } // check for actions with invalid action groups const { actionGroups: alertTypeActionGroups } = ruleType; - const usedAlertActionGroups = actions.map((action) => action.group); + const usedAlertActionGroups = actionsWithoutSystemActions.map((action) => action.group); const availableAlertTypeActionGroups = new Set(map(alertTypeActionGroups, 'id')); const invalidActionGroups = usedAlertActionGroups.filter( (group) => !availableAlertTypeActionGroups.has(group) @@ -94,7 +103,10 @@ export async function validateActions( // check for actions using frequency params if the rule has rule-level frequency params defined if (hasRuleLevelNotifyWhen || hasRuleLevelThrottle) { - const actionsWithFrequency = actions.filter((action) => Boolean(action.frequency)); + const actionsWithFrequency = actionsWithoutSystemActions.filter((action) => + Boolean(action.frequency) + ); + if (actionsWithFrequency.length) { errors.push( i18n.translate('xpack.alerting.rulesClient.validateActions.mixAndMatchFreqParams', { @@ -107,7 +119,10 @@ export async function validateActions( ); } } else { - const actionsWithoutFrequency = actions.filter((action) => !action.frequency); + const actionsWithoutFrequency = actionsWithoutSystemActions.filter( + (action) => !action.frequency + ); + if (actionsWithoutFrequency.length) { errors.push( i18n.translate('xpack.alerting.rulesClient.validateActions.notAllActionsWithFreq', { @@ -128,7 +143,7 @@ export async function validateActions( const actionsWithInvalidDays = []; const actionsWithAlertsFilterWithoutAlertsMapping = []; - for (const action of actions) { + for (const action of actionsWithoutSystemActions) { const { alertsFilter } = action; // check for actions throttled shorter than the rule schedule From 2e50e0d4a6f678552598dd1e44062391b27c4b5d Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Fri, 6 Oct 2023 16:53:57 +0300 Subject: [PATCH 09/14] Allow sysetm actions in the bulk edit API --- .../routes/rule/apis/bulk_edit/schemas/v1.ts | 2 +- .../methods/bulk_edit/bulk_edit_rules.test.ts | 675 +++++++++++++++++- .../rule/methods/bulk_edit/bulk_edit_rules.ts | 67 +- .../schemas/bulk_edit_rules_option_schemas.ts | 16 +- .../bulk_edit/bulk_edit_rules_route.test.ts | 193 ++++- .../apis/bulk_edit/bulk_edit_rules_route.ts | 9 +- .../rule/apis/bulk_edit/transforms/index.ts | 10 + .../transforms/transform_operations/latest.ts | 8 + .../transform_operations/v1.test.ts | 66 ++ .../transforms/transform_operations/v1.ts | 56 ++ .../common/apply_bulk_edit_operation.test.ts | 87 ++- .../tests/alerting/group2/bulk_edit.ts | 181 +++++ .../tests/alerting/group2/index.ts | 1 + 13 files changed, 1314 insertions(+), 57 deletions(-) create mode 100644 x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/index.ts create mode 100644 x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/latest.ts create mode 100644 x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/v1.test.ts create mode 100644 x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/v1.ts create mode 100644 x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/bulk_edit.ts diff --git a/x-pack/plugins/alerting/common/routes/rule/apis/bulk_edit/schemas/v1.ts b/x-pack/plugins/alerting/common/routes/rule/apis/bulk_edit/schemas/v1.ts index 54e70cde689acd..958bb61863a0ea 100644 --- a/x-pack/plugins/alerting/common/routes/rule/apis/bulk_edit/schemas/v1.ts +++ b/x-pack/plugins/alerting/common/routes/rule/apis/bulk_edit/schemas/v1.ts @@ -38,7 +38,7 @@ const ruleSnoozeScheduleSchemaWithValidation = schema.object( ); const ruleActionSchema = schema.object({ - group: schema.string(), + group: schema.maybe(schema.string()), id: schema.string(), params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }), uuid: schema.maybe(schema.string()), diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.test.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.test.ts index 0e9761de43ea6a..33b354529ee69b 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.test.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.test.ts @@ -18,7 +18,7 @@ import { import { taskManagerMock } from '@kbn/task-manager-plugin/server/mocks'; import { ruleTypeRegistryMock } from '../../../../rule_type_registry.mock'; import { alertingAuthorizationMock } from '../../../../authorization/alerting_authorization.mock'; -import { RecoveredActionGroup, RuleTypeParams } from '../../../../../common'; +import { RecoveredActionGroup, RuleActionTypes, RuleTypeParams } from '../../../../../common'; import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks'; import { actionsAuthorizationMock } from '@kbn/actions-plugin/server/mocks'; import { AlertingAuthorization } from '../../../../authorization/alerting_authorization'; @@ -26,7 +26,6 @@ import { ActionsAuthorization, ActionsClient } from '@kbn/actions-plugin/server' import { auditLoggerMock } from '@kbn/security-plugin/server/audit/mocks'; import { getBeforeSetup, setGlobalDate } from '../../../../rules_client/tests/lib'; import { bulkMarkApiKeysForInvalidation } from '../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation'; -import { NormalizedAlertAction } from '../../../../rules_client/types'; import { enabledRule1, enabledRule2, @@ -36,6 +35,9 @@ import { import { migrateLegacyActions } from '../../../../rules_client/lib'; import { migrateLegacyActionsMock } from '../../../../rules_client/lib/siem_legacy_actions/retrieve_migrated_legacy_actions.mock'; import { ConnectorAdapterRegistry } from '../../../../connector_adapters/connector_adapter_registry'; +import { ConnectorAdapter } from '../../../../connector_adapters/types'; +import { RuleAttributes } from '../../../../data/rule/types'; +import { SavedObject } from '@kbn/core/server'; jest.mock('../../../../rules_client/lib/siem_legacy_actions/migrate_legacy_actions', () => { return { @@ -104,6 +106,7 @@ const rulesClientParams: jest.Mocked = { isAuthenticationTypeAPIKey: isAuthenticationTypeApiKeyMock, getAuthenticationAPIKey: getAuthenticationApiKeyMock, connectorAdapterRegistry: new ConnectorAdapterRegistry(), + isSystemAction: jest.fn(), getAlertIndicesAlias: jest.fn(), alertsService: null, }; @@ -244,14 +247,17 @@ describe('bulkEdit()', () => { return { state: {} }; }, category: 'test', + validLegacyConsumers: [], producer: 'alerts', validate: { params: { validate: (params) => params }, }, - validLegacyConsumers: [], }); (migrateLegacyActions as jest.Mock).mockResolvedValue(migrateLegacyActionsMock); + + rulesClientParams.isSystemAction.mockImplementation((id: string) => id === 'system_action-id'); + actionsClient.isSystemAction.mockImplementation((id: string) => id === 'system_action-id'); }); describe('tags operations', () => { @@ -536,6 +542,14 @@ describe('bulkEdit()', () => { }); describe('actions operations', () => { + const connectorAdapter: ConnectorAdapter = { + connectorTypeId: '.test', + ruleActionParamsSchema: schema.object({ foo: schema.string() }), + buildActionParams: jest.fn(), + }; + + rulesClientParams.connectorAdapterRegistry.register(connectorAdapter); + beforeEach(() => { mockCreatePointInTimeFinderAsInternalUser({ saved_objects: [existingDecryptedRule], @@ -545,7 +559,7 @@ describe('bulkEdit()', () => { test('should add uuid to new actions', async () => { const existingAction = { frequency: { - notifyWhen: 'onActiveAlert', + notifyWhen: 'onActiveAlert' as const, summary: false, throttle: null, }, @@ -553,26 +567,31 @@ describe('bulkEdit()', () => { id: '1', params: {}, uuid: '111', + type: RuleActionTypes.DEFAULT, }; + const newAction = { frequency: { - notifyWhen: 'onActiveAlert', + notifyWhen: 'onActiveAlert' as const, summary: false, throttle: null, }, group: 'default', id: '2', params: {}, + type: RuleActionTypes.DEFAULT, }; + const newAction2 = { frequency: { - notifyWhen: 'onActiveAlert', + notifyWhen: 'onActiveAlert' as const, summary: false, throttle: null, }, group: 'default', id: '3', params: {}, + type: RuleActionTypes.DEFAULT, }; unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ @@ -585,10 +604,12 @@ describe('bulkEdit()', () => { { ...existingAction, actionRef: 'action_0', + actionTypeId: 'test-0', }, { ...newAction, actionRef: 'action_1', + actionTypeId: 'test-1', uuid: '222', }, ], @@ -615,7 +636,7 @@ describe('bulkEdit()', () => { { field: 'actions', operation: 'add', - value: [existingAction, newAction, newAction2] as NormalizedAlertAction[], + value: [existingAction, newAction, newAction2], }, ], }); @@ -677,7 +698,10 @@ describe('bulkEdit()', () => { ...existingRule.attributes.executionStatus, lastExecutionDate: new Date(existingRule.attributes.executionStatus.lastExecutionDate), }, - actions: [existingAction, { ...newAction, uuid: '222' }], + actions: [ + { ...existingAction, type: RuleActionTypes.DEFAULT, actionTypeId: 'test-0' }, + { ...newAction, uuid: '222', type: RuleActionTypes.DEFAULT, actionTypeId: 'test-1' }, + ], id: existingRule.id, snoozeSchedule: [], }); @@ -708,6 +732,7 @@ describe('bulkEdit()', () => { params: { message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', }, + type: RuleActionTypes.DEFAULT, }, ], }, @@ -742,7 +767,6 @@ describe('bulkEdit()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', validate: { params: { validate: (params) => params }, @@ -752,11 +776,13 @@ describe('bulkEdit()', () => { mappings: { fieldMap: { field: { type: 'keyword', required: false } } }, shouldWrite: true, }, + category: 'test', validLegacyConsumers: [], }); + const existingAction = { frequency: { - notifyWhen: 'onActiveAlert', + notifyWhen: 'onActiveAlert' as const, summary: false, throttle: null, }, @@ -776,10 +802,11 @@ describe('bulkEdit()', () => { timezone: 'UTC', }, }, + type: RuleActionTypes.DEFAULT, }; const newAction = { frequency: { - notifyWhen: 'onActiveAlert', + notifyWhen: 'onActiveAlert' as const, summary: false, throttle: null, }, @@ -788,6 +815,7 @@ describe('bulkEdit()', () => { params: {}, uuid: '222', alertsFilter: { query: { kql: 'test:1', dsl: 'test', filters: [] } }, + type: RuleActionTypes.DEFAULT, }; unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ @@ -833,7 +861,7 @@ describe('bulkEdit()', () => { { field: 'actions', operation: 'add', - value: [existingAction, newAction] as NormalizedAlertAction[], + value: [existingAction, newAction], }, ], }); @@ -911,6 +939,541 @@ describe('bulkEdit()', () => { snoozeSchedule: [], }); }); + + test('should add system and default actions', async () => { + const newAction = { + frequency: { + notifyWhen: 'onActiveAlert' as const, + summary: false, + throttle: null, + }, + group: 'default', + id: '1', + params: {}, + type: RuleActionTypes.DEFAULT, + }; + + const newAction2 = { + id: 'system_action-id', + params: {}, + type: RuleActionTypes.SYSTEM, + }; + + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ + saved_objects: [ + { + ...existingRule, + attributes: { + ...existingRule.attributes, + actions: [ + { + ...newAction, + actionRef: 'action_0', + actionTypeId: 'test-1', + uuid: '222', + }, + { + ...newAction2, + actionRef: 'system_action:system_action-id', + actionTypeId: 'test-2', + uuid: '222', + }, + ], + }, + references: [ + { + name: 'action_0', + type: 'action', + id: '1', + }, + ], + }, + ], + }); + + actionsClient.getBulk.mockResolvedValue([ + { + id: '1', + actionTypeId: 'test-1', + config: {}, + isMissingSecrets: false, + name: 'test default connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: false, + }, + { + id: 'system_action-id', + actionTypeId: 'test-2', + config: {}, + isMissingSecrets: false, + name: 'system action connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: true, + }, + ]); + + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'actions', + operation: 'add', + value: [newAction, newAction2], + }, + ], + }); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith( + [ + { + ...existingRule, + attributes: { + ...existingRule.attributes, + actions: [ + { + actionRef: 'action_0', + actionTypeId: 'test-1', + frequency: { notifyWhen: 'onActiveAlert', summary: false, throttle: null }, + group: 'default', + params: {}, + uuid: '103', + }, + { + actionRef: 'system_action:system_action-id', + actionTypeId: 'test-2', + params: {}, + uuid: '104', + }, + ], + apiKey: null, + apiKeyOwner: null, + apiKeyCreatedByUser: null, + meta: { versionApiKeyLastmodified: 'v8.2.0' }, + name: 'my rule name', + enabled: false, + updatedAt: '2019-02-12T21:01:22.479Z', + updatedBy: 'elastic', + tags: ['foo'], + revision: 1, + }, + references: [{ id: '1', name: 'action_0', type: 'action' }], + }, + ], + { overwrite: true } + ); + + expect(result.rules[0]).toEqual({ + ...omit(existingRule.attributes, 'legacyId'), + createdAt: new Date(existingRule.attributes.createdAt), + updatedAt: new Date(existingRule.attributes.updatedAt), + executionStatus: { + ...existingRule.attributes.executionStatus, + lastExecutionDate: new Date(existingRule.attributes.executionStatus.lastExecutionDate), + }, + actions: [ + { ...newAction, actionTypeId: 'test-1', uuid: '222' }, + { ...newAction2, actionTypeId: 'test-2', uuid: '222' }, + ], + id: existingRule.id, + snoozeSchedule: [], + }); + }); + + test('should construct the refs correctly and not persist the type of the action', async () => { + const newAction = { + frequency: { + notifyWhen: 'onActiveAlert' as const, + summary: false, + throttle: null, + }, + group: 'default', + id: '1', + params: {}, + type: RuleActionTypes.DEFAULT, + }; + + const newAction2 = { + id: 'system_action-id', + params: {}, + type: RuleActionTypes.SYSTEM, + }; + + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ + saved_objects: [ + { + ...existingRule, + attributes: { + ...existingRule.attributes, + actions: [ + { + ...newAction, + actionRef: 'action_0', + actionTypeId: 'test-1', + uuid: '222', + }, + { + ...newAction2, + actionRef: 'system_action:system_action-id', + actionTypeId: 'test-2', + uuid: '222', + }, + ], + }, + references: [ + { + name: 'action_0', + type: 'action', + id: '1', + }, + ], + }, + ], + }); + + actionsClient.getBulk.mockResolvedValue([ + { + id: '1', + actionTypeId: 'test-1', + config: {}, + isMissingSecrets: false, + name: 'test default connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: false, + }, + { + id: 'system_action-id', + actionTypeId: 'test-2', + config: {}, + isMissingSecrets: false, + name: 'system action connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: true, + }, + ]); + + await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'actions', + operation: 'add', + value: [newAction, newAction2], + }, + ], + }); + + const rule = unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0] as Array< + SavedObject + >; + + expect(rule[0].attributes.actions).toEqual([ + { + actionRef: 'action_0', + actionTypeId: 'test-1', + frequency: { notifyWhen: 'onActiveAlert', summary: false, throttle: null }, + group: 'default', + params: {}, + uuid: '105', + }, + { + actionRef: 'system_action:system_action-id', + actionTypeId: 'test-2', + params: {}, + uuid: '106', + }, + ]); + }); + + test('should add the actions type to the response correctly', async () => { + const newAction = { + frequency: { + notifyWhen: 'onActiveAlert' as const, + summary: false, + throttle: null, + }, + group: 'default', + id: '1', + params: {}, + type: RuleActionTypes.DEFAULT, + }; + + const newAction2 = { + id: 'system_action-id', + params: {}, + type: RuleActionTypes.SYSTEM, + }; + + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ + saved_objects: [ + { + ...existingRule, + attributes: { + ...existingRule.attributes, + actions: [ + { + ...newAction, + actionRef: 'action_0', + actionTypeId: 'test-1', + uuid: '222', + }, + { + ...newAction2, + actionRef: 'system_action:system_action-id', + actionTypeId: 'test-2', + uuid: '222', + }, + ], + }, + references: [ + { + name: 'action_0', + type: 'action', + id: '1', + }, + ], + }, + ], + }); + + actionsClient.getBulk.mockResolvedValue([ + { + id: '1', + actionTypeId: 'test-1', + config: {}, + isMissingSecrets: false, + name: 'test default connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: false, + }, + { + id: 'system_action-id', + actionTypeId: 'test-2', + config: {}, + isMissingSecrets: false, + name: 'system action connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: true, + }, + ]); + + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'actions', + operation: 'add', + value: [newAction, newAction2], + }, + ], + }); + + expect(result.rules[0].actions).toEqual([ + { ...newAction, actionTypeId: 'test-1', uuid: '222' }, + { ...newAction2, actionTypeId: 'test-2', uuid: '222' }, + ]); + }); + + it('should return an error if the system action does not exist', async () => { + const action = { + id: 'system_action-id', + uuid: '123', + params: {}, + type: RuleActionTypes.SYSTEM, + }; + + actionsClient.isSystemAction.mockReturnValue(false); + actionsClient.getBulk.mockResolvedValue([ + { + id: 'system_action-id', + actionTypeId: 'test-2', + config: {}, + isMissingSecrets: false, + name: 'system action connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: true, + }, + ]); + + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'actions', + operation: 'add', + value: [action], + }, + ], + }); + + expect(result).toMatchInlineSnapshot(` + Object { + "errors": Array [ + Object { + "message": "Action system_action-id is not a system action", + "rule": Object { + "id": "1", + "name": "my rule name", + }, + }, + ], + "rules": Array [], + "skipped": Array [], + "total": 1, + } + `); + + expect(actionsClient.getBulk).toBeCalledWith({ + ids: ['system_action-id'], + throwIfSystemAction: false, + }); + }); + + it('should throw an error if the system action contains the frequency', async () => { + const action = { + id: 'system_action-id', + uuid: '123', + params: {}, + frequency: { + notifyWhen: 'onActiveAlert' as const, + summary: false, + throttle: null, + }, + type: RuleActionTypes.SYSTEM, + }; + + actionsClient.isSystemAction.mockReturnValue(true); + actionsClient.getBulk.mockResolvedValue([ + { + id: 'system_action-id', + actionTypeId: 'test-2', + config: {}, + isMissingSecrets: false, + name: 'system action connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: true, + }, + ]); + + await expect( + rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'actions', + operation: 'add', + value: [action], + }, + ], + }) + ).rejects.toMatchInlineSnapshot(` + [Error: Error validating bulk edit rules operations - [0]: types that failed validation: + - [0.0.field]: expected value to equal [tags] + - [0.1.value.0]: types that failed validation: + - [0.value.0.0.group]: expected value of type [string] but got [undefined] + - [0.value.0.1.frequency]: definition for this key is missing + - [0.2.operation]: expected value to equal [set] + - [0.3.operation]: expected value to equal [set] + - [0.4.operation]: expected value to equal [set] + - [0.5.operation]: expected value to equal [set] + - [0.6.operation]: expected value to equal [delete] + - [0.7.operation]: expected value to equal [set]] + `); + }); + + it('should throw an error if the system action contains the alertsFilter', async () => { + const action = { + id: 'system_action-id', + uuid: '123', + params: {}, + alertsFilter: { + query: { kql: 'test:1', filters: [] }, + }, + type: RuleActionTypes.SYSTEM, + }; + + actionsClient.isSystemAction.mockReturnValue(true); + actionsClient.getBulk.mockResolvedValue([ + { + id: 'system_action-id', + actionTypeId: 'test-2', + config: {}, + isMissingSecrets: false, + name: 'system action connector', + isPreconfigured: false, + isDeprecated: false, + isSystemAction: true, + }, + ]); + + await expect( + rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'actions', + operation: 'add', + value: [action], + }, + ], + }) + ).rejects.toMatchInlineSnapshot(` + [Error: Error validating bulk edit rules operations - [0]: types that failed validation: + - [0.0.field]: expected value to equal [tags] + - [0.1.value.0]: types that failed validation: + - [0.value.0.0.group]: expected value of type [string] but got [undefined] + - [0.value.0.1.alertsFilter]: definition for this key is missing + - [0.2.operation]: expected value to equal [set] + - [0.3.operation]: expected value to equal [set] + - [0.4.operation]: expected value to equal [set] + - [0.5.operation]: expected value to equal [set] + - [0.6.operation]: expected value to equal [delete] + - [0.7.operation]: expected value to equal [set]] + `); + }); + + it('should throw an error if the default action does not contain the group', async () => { + const action = { + id: '1', + params: {}, + type: RuleActionTypes.DEFAULT, + }; + + actionsClient.isSystemAction.mockReturnValue(false); + + await expect( + rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'actions', + operation: 'add', + // @ts-expect-error: group is missing + value: [action], + }, + ], + }) + ).rejects.toMatchInlineSnapshot(` + [Error: Error validating bulk edit rules operations - [0]: types that failed validation: + - [0.0.field]: expected value to equal [tags] + - [0.1.value.0]: types that failed validation: + - [0.value.0.0.group]: expected value of type [string] but got [undefined] + - [0.value.0.1.type]: expected value to equal [system] + - [0.2.operation]: expected value to equal [set] + - [0.3.operation]: expected value to equal [set] + - [0.4.operation]: expected value to equal [set] + - [0.5.operation]: expected value to equal [set] + - [0.6.operation]: expected value to equal [delete] + - [0.7.operation]: expected value to equal [set]] + `); + }); }); describe('index pattern operations', () => { @@ -968,7 +1531,13 @@ describe('bulkEdit()', () => { const result = await rulesClient.bulkEdit({ filter: '', - operations: [], + operations: [ + { + field: 'tags', + operation: 'add', + value: ['test-tag'], + }, + ], paramsModifier, }); @@ -1037,7 +1606,13 @@ describe('bulkEdit()', () => { const result = await rulesClient.bulkEdit({ filter: '', - operations: [], + operations: [ + { + field: 'tags', + operation: 'add', + value: ['test-tag'], + }, + ], paramsModifier, }); @@ -1071,7 +1646,13 @@ describe('bulkEdit()', () => { const result = await rulesClient.bulkEdit({ filter: '', - operations: [], + operations: [ + { + field: 'tags', + operation: 'add', + value: ['test-tag'], + }, + ], paramsModifier, }); @@ -2358,8 +2939,8 @@ describe('bulkEdit()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', + category: 'test', validLegacyConsumers: [], }); @@ -2404,8 +2985,8 @@ describe('bulkEdit()', () => { async executor() { return { state: {} }; }, - category: 'test', producer: 'alerts', + category: 'test', validLegacyConsumers: [], }); @@ -2444,7 +3025,13 @@ describe('bulkEdit()', () => { const result = await rulesClient.bulkEdit({ filter: '', - operations: [], + operations: [ + { + field: 'tags', + operation: 'add', + value: ['test-1'], + }, + ], paramsModifier: async (params) => { params.index = ['test-index-*']; @@ -2579,6 +3166,50 @@ describe('bulkEdit()', () => { expect(validateScheduleLimit).toHaveBeenCalledTimes(1); }); + + test('should not validate scheduling on system actions', async () => { + mockCreatePointInTimeFinderAsInternalUser({ + saved_objects: [ + { + ...existingDecryptedRule, + attributes: { + ...existingDecryptedRule.attributes, + actions: [ + { + actionRef: 'action_0', + actionTypeId: 'test', + params: {}, + uuid: '111', + type: RuleActionTypes.SYSTEM, + }, + ], + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any, + references: [ + { + name: 'action_0', + type: 'action', + id: '1', + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ] as any, + }, + ], + }); + + const result = await rulesClient.bulkEdit({ + operations: [ + { + field: 'schedule', + operation: 'set', + value: { interval: '10m' }, + }, + ], + }); + + expect(result.errors).toHaveLength(0); + expect(result.rules).toHaveLength(1); + }); }); describe('paramsModifier', () => { @@ -2612,7 +3243,13 @@ describe('bulkEdit()', () => { const result = await rulesClient.bulkEdit({ filter: '', - operations: [], + operations: [ + { + field: 'tags', + operation: 'add', + value: ['test-1'], + }, + ], paramsModifier: async (params) => { params.index = ['test-index-*']; diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.ts index d76162696ead27..ee6e4732618193 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.ts @@ -16,6 +16,9 @@ import { SavedObjectsFindResult, SavedObjectsUpdateResponse, } from '@kbn/core/server'; +import { validateSystemActions } from '../../../../lib/validate_system_actions'; +import { RuleActionTypes, RuleDefaultAction, RuleSystemAction } from '../../../../../common'; +import { isSystemAction } from '../../../../../common/system_actions/is_system_action'; import { BulkActionSkipResult } from '../../../../../common/bulk_edit'; import { RuleTypeRegistry } from '../../../../types'; import { @@ -32,7 +35,6 @@ import { retryIfBulkEditConflicts, applyBulkEditOperation, buildKueryNodeFilter, - injectReferencesIntoActions, getBulkSnooze, getBulkUnsnooze, verifySnoozeScheduleLimit, @@ -78,6 +80,7 @@ import { transformRuleDomainToRule, } from '../../transforms'; import { validateScheduleLimit } from '../get_schedule_frequency'; +import { bulkEditOperationsSchema } from './schemas'; const isValidInterval = (interval: string | undefined): interval is string => { return interval !== undefined; @@ -115,8 +118,15 @@ export async function bulkEditRules( context: RulesClientContext, options: BulkEditOptions ): Promise> { + try { + bulkEditOperationsSchema.validate(options.operations); + } catch (error) { + throw Boom.badRequest(`Error validating bulk edit rules operations - ${error.message}`); + } + const queryFilter = (options as BulkEditOptionsFilter).filter; const ids = (options as BulkEditOptionsIds).ids; + const actionsClient = await context.getActionsClient(); if (ids && queryFilter) { throw Boom.badRequest( @@ -231,13 +241,17 @@ export async function bulkEditRules( // fix the type cast from SavedObjectsBulkUpdateObject to SavedObjectsBulkUpdateObject // when we are doing the bulk create and this should fix itself const ruleType = context.ruleTypeRegistry.get(attributes.alertTypeId!); - const ruleDomain = transformRuleAttributesToRuleDomain(attributes as RuleAttributes, { - id, - logger: context.logger, - ruleType, - references, - omitGeneratedValues: false, - }); + const ruleDomain = transformRuleAttributesToRuleDomain( + attributes as RuleAttributes, + { + id, + logger: context.logger, + ruleType, + references, + omitGeneratedValues: false, + }, + (connectorId: string) => actionsClient.isSystemAction(connectorId) + ); try { ruleDomainSchema.validate(ruleDomain); } catch (e) { @@ -458,12 +472,6 @@ async function updateRuleAttributesAndParamsInMemory( rule.references = migratedActions.resultedReferences; } - const ruleActions = injectReferencesIntoActions( - rule.id, - rule.attributes.actions || [], - rule.references || [] - ); - const ruleDomain: RuleDomain = transformRuleAttributesToRuleDomain( rule.attributes, { @@ -471,7 +479,8 @@ async function updateRuleAttributesAndParamsInMemory( logger: context.logger, ruleType: context.ruleTypeRegistry.get(rule.attributes.alertTypeId), references: rule.references, - } + }, + context.isSystemAction ); const { @@ -483,7 +492,7 @@ async function updateRuleAttributesAndParamsInMemory( context, operations, rule: ruleDomain, - ruleActions, + ruleActions: ruleDomain.actions, ruleType, }); @@ -617,6 +626,8 @@ async function getUpdatedAttributesFromOperations({ ruleActions: RuleDomain['actions']; ruleType: RuleType; }) { + const actionsClient = await context.getActionsClient(); + let updatedRule = cloneDeep(rule); let updatedRuleActions = ruleActions; let hasUpdateApiKeyOperation = false; @@ -634,6 +645,16 @@ async function getUpdatedAttributesFromOperations({ value: addGeneratedActionValues(operation.value), }; + const systemActions = operation.value.filter( + (action): action is RuleSystemAction => action.type === RuleActionTypes.SYSTEM + ); + + await validateSystemActions({ + actionsClient, + connectorAdapterRegistry: context.connectorAdapterRegistry, + systemActions, + }); + try { await validateActions(context, ruleType, { ...updatedRule, @@ -662,6 +683,7 @@ async function getUpdatedAttributesFromOperations({ break; } + case 'snoozeSchedule': { // Silently skip adding snooze or snooze schedules on security // rules until we implement snoozing of their rules @@ -672,22 +694,26 @@ async function getUpdatedAttributesFromOperations({ isAttributesUpdateSkipped = false; break; } + if (operation.operation === 'set') { const snoozeAttributes = getBulkSnooze( updatedRule, operation.value as RuleSnoozeSchedule ); + try { verifySnoozeScheduleLimit(snoozeAttributes.snoozeSchedule); } catch (error) { throw Error(`Error updating rule: could not add snooze - ${error.message}`); } + updatedRule = { ...updatedRule, muteAll: snoozeAttributes.muteAll, snoozeSchedule: snoozeAttributes.snoozeSchedule as RuleDomain['snoozeSchedule'], }; } + if (operation.operation === 'delete') { const idsToDelete = operation.value && [...operation.value]; if (idsToDelete?.length === 0) { @@ -704,18 +730,22 @@ async function getUpdatedAttributesFromOperations({ snoozeSchedule: snoozeAttributes.snoozeSchedule as RuleDomain['snoozeSchedule'], }; } + isAttributesUpdateSkipped = false; break; } + case 'apiKey': { hasUpdateApiKeyOperation = true; isAttributesUpdateSkipped = false; break; } + default: { if (operation.field === 'schedule') { validateScheduleOperation(operation.value, updatedRule.actions, rule.id); } + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( operation, updatedRule @@ -780,8 +810,11 @@ function validateScheduleOperation( ): void { const scheduleInterval = parseDuration(schedule.interval); const actionsWithInvalidThrottles = []; + const actionsWithoutSystemActions = actions.filter( + (action): action is RuleDefaultAction => !isSystemAction(action) + ); - for (const action of actions) { + for (const action of actionsWithoutSystemActions) { // check for actions throttled shorter than the rule schedule if ( action.frequency?.notifyWhen === ruleNotifyWhen.THROTTLE && diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/schemas/bulk_edit_rules_option_schemas.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/schemas/bulk_edit_rules_option_schemas.ts index f80d63210cf4ab..e8ecb18af52b03 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/schemas/bulk_edit_rules_option_schemas.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/schemas/bulk_edit_rules_option_schemas.ts @@ -5,8 +5,9 @@ * 2.0. */ import { schema } from '@kbn/config-schema'; +import { RuleActionTypes } from '../../../../../../common'; import { rRuleRequestSchema } from '../../../../r_rule/schemas'; -import { notifyWhenSchema } from '../../../schemas'; +import { notifyWhenSchema, actionAlertsFilterSchema } from '../../../schemas'; import { validateDuration } from '../../../validation'; import { validateSnoozeSchedule } from '../validation'; @@ -26,7 +27,7 @@ const bulkEditRuleSnoozeScheduleSchemaWithValidation = schema.object( { validate: validateSnoozeSchedule } ); -const bulkEditActionSchema = schema.object({ +const bulkEditDefaultActionSchema = schema.object({ group: schema.string(), id: schema.string(), params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }), @@ -38,6 +39,15 @@ const bulkEditActionSchema = schema.object({ notifyWhen: notifyWhenSchema, }) ), + alertsFilter: schema.maybe(actionAlertsFilterSchema), + type: schema.literal(RuleActionTypes.DEFAULT), +}); + +export const bulkEditSystemActionSchema = schema.object({ + id: schema.string(), + params: schema.recordOf(schema.string(), schema.maybe(schema.any()), { defaultValue: {} }), + uuid: schema.maybe(schema.string()), + type: schema.literal(RuleActionTypes.SYSTEM), }); const bulkEditTagSchema = schema.object({ @@ -49,7 +59,7 @@ const bulkEditTagSchema = schema.object({ const bulkEditActionsSchema = schema.object({ operation: schema.oneOf([schema.literal('add'), schema.literal('set')]), field: schema.literal('actions'), - value: schema.arrayOf(bulkEditActionSchema), + value: schema.arrayOf(schema.oneOf([bulkEditDefaultActionSchema, bulkEditSystemActionSchema])), }); const bulkEditScheduleSchema = schema.object({ diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/bulk_edit_rules_route.test.ts b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/bulk_edit_rules_route.test.ts index 1b1ed454c52076..bf27692bece815 100644 --- a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/bulk_edit_rules_route.test.ts +++ b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/bulk_edit_rules_route.test.ts @@ -13,7 +13,14 @@ import { verifyApiAccess } from '../../../../lib/license_api_access'; import { RuleTypeDisabledError } from '../../../../lib/errors/rule_type_disabled'; import { mockHandlerArguments } from '../../../_mock_handler_arguments'; import { rulesClientMock } from '../../../../rules_client.mock'; -import { SanitizedRule } from '../../../../types'; +import { + RuleActionTypes, + RuleDefaultAction, + RuleSystemAction, + SanitizedRule, +} from '../../../../types'; +import { actionsClientMock } from '@kbn/actions-plugin/server/mocks'; +import { omit } from 'lodash'; const rulesClient = rulesClientMock.create(); jest.mock('../../../../lib/license_api_access', () => ({ @@ -42,6 +49,7 @@ describe('bulkEditRulesRoute', () => { foo: true, }, uuid: '123-456', + type: RuleActionTypes.DEFAULT, }, ], consumer: 'bar', @@ -189,4 +197,187 @@ describe('bulkEditRulesRoute', () => { expect(res.forbidden).toHaveBeenCalledWith({ body: { message: 'Fail' } }); }); + + describe('actions', () => { + const action: RuleDefaultAction = { + actionTypeId: 'test', + group: 'default', + id: '2', + params: { + foo: true, + }, + uuid: '123-456', + type: RuleActionTypes.DEFAULT, + }; + + const systemAction: RuleSystemAction = { + actionTypeId: 'test-2', + id: 'system_action-id', + params: { + foo: true, + }, + uuid: '123-456', + type: RuleActionTypes.SYSTEM, + }; + + const mockedActionAlerts: Array> = [ + { ...mockedAlert, actions: [action, systemAction] }, + ]; + + const bulkEditActionsRequest = { + filter: '', + operations: [ + { + operation: 'add', + field: 'actions', + value: [omit(action, 'type'), omit(systemAction, 'type')], + }, + ], + }; + + const bulkEditActionsResult = { rules: mockedActionAlerts, errors: [], total: 1, skipped: [] }; + + it('adds the type of the actions correctly before passing the request to the rules client', async () => { + const licenseState = licenseStateMock.create(); + const router = httpServiceMock.createRouter(); + const actionsClient = actionsClientMock.create(); + actionsClient.isSystemAction.mockImplementation((id: string) => id === 'system_action-id'); + + bulkEditInternalRulesRoute(router, licenseState); + + const [_, handler] = router.post.mock.calls[0]; + + rulesClient.bulkEdit.mockResolvedValueOnce(bulkEditActionsResult); + + const [context, req, res] = mockHandlerArguments( + { rulesClient, actionsClient }, + { + body: bulkEditActionsRequest, + }, + ['ok'] + ); + + await handler(context, req, res); + + expect(rulesClient.bulkEdit.mock.calls[0][0]).toMatchInlineSnapshot(` + Object { + "filter": "", + "ids": undefined, + "operations": Array [ + Object { + "field": "actions", + "operation": "add", + "value": Array [ + Object { + "frequency": undefined, + "group": "default", + "id": "2", + "params": Object { + "foo": true, + }, + "type": "default", + "uuid": "123-456", + }, + Object { + "id": "system_action-id", + "params": Object { + "foo": true, + }, + "type": "system", + "uuid": "123-456", + }, + ], + }, + ], + } + `); + }); + + it('removes the type from the actions correctly before sending the response', async () => { + const licenseState = licenseStateMock.create(); + const router = httpServiceMock.createRouter(); + const actionsClient = actionsClientMock.create(); + actionsClient.isSystemAction.mockImplementation((id: string) => id === 'system_action-id'); + + bulkEditInternalRulesRoute(router, licenseState); + + const [_, handler] = router.post.mock.calls[0]; + + rulesClient.bulkEdit.mockResolvedValueOnce(bulkEditActionsResult); + + const [context, req, res] = mockHandlerArguments( + { rulesClient, actionsClient }, + { + body: bulkEditActionsRequest, + }, + ['ok'] + ); + + const routeRes = await handler(context, req, res); + + // @ts-expect-error: body exists + expect(routeRes.body.rules[0].actions).toEqual([ + { + connector_type_id: 'test', + group: 'default', + id: '2', + params: { + foo: true, + }, + uuid: '123-456', + }, + { + connector_type_id: 'test-2', + id: 'system_action-id', + params: { + foo: true, + }, + uuid: '123-456', + }, + ]); + }); + + it('fails if the action contains a type in the request', async () => { + const actionToValidate = { + group: 'default', + id: '2', + params: { + foo: true, + }, + uuid: '123-456', + type: RuleActionTypes.DEFAULT, + }; + + const licenseState = licenseStateMock.create(); + const router = httpServiceMock.createRouter(); + + bulkEditInternalRulesRoute(router, licenseState); + + const [config, _] = router.post.mock.calls[0]; + + expect(() => + // @ts-expect-error: body exists + config.validate.body.validate({ + ...bulkEditActionsRequest, + operations: [ + { + operation: 'add', + field: 'actions', + value: [actionToValidate], + }, + ], + }) + ).toThrowErrorMatchingInlineSnapshot(` + "[operations.0]: types that failed validation: + - [operations.0.0.field]: expected value to equal [tags] + - [operations.0.1.value.0.type]: definition for this key is missing + - [operations.0.2.operation]: expected value to equal [set] + - [operations.0.3.operation]: expected value to equal [set] + - [operations.0.4.operation]: expected value to equal [set] + - [operations.0.5.operation]: expected value to equal [set] + - [operations.0.6.operation]: expected value to equal [delete] + - [operations.0.7.operation]: expected value to equal [set]" + `); + }); + }); }); diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/bulk_edit_rules_route.ts b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/bulk_edit_rules_route.ts index ae39ceba1ceb34..9574c6c475ae45 100644 --- a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/bulk_edit_rules_route.ts +++ b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/bulk_edit_rules_route.ts @@ -20,6 +20,7 @@ import { Rule } from '../../../../application/rule/types'; import type { RuleParamsV1 } from '../../../../../common/routes/rule/response'; import { transformRuleToRuleResponseV1 } from '../../transforms'; +import { transformOperationsV1 } from './transforms'; interface BuildBulkEditRulesRouteParams { licenseState: ILicenseState; @@ -39,15 +40,19 @@ const buildBulkEditRulesRoute = ({ licenseState, path, router }: BuildBulkEditRu router.handleLegacyErrors( verifyAccessAndContext(licenseState, async function (context, req, res) { const rulesClient = (await context.alerting).getRulesClient(); - const bulkEditData: BulkEditRulesRequestBodyV1 = req.body; + const actionsClient = (await context.actions).getActionsClient(); + const bulkEditData: BulkEditRulesRequestBodyV1 = req.body; const { filter, operations, ids } = bulkEditData; try { const bulkEditResults = await rulesClient.bulkEdit({ filter, ids, - operations, + operations: transformOperationsV1({ + operations, + isSystemAction: (connectorId: string) => actionsClient.isSystemAction(connectorId), + }), }); const resultBody: BulkEditRulesResponseV1 = { diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/index.ts b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/index.ts new file mode 100644 index 00000000000000..e7d1a1dc434783 --- /dev/null +++ b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/index.ts @@ -0,0 +1,10 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export { transformOperations } from './transform_operations/latest'; + +export { transformOperations as transformOperationsV1 } from './transform_operations/v1'; diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/latest.ts b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/latest.ts new file mode 100644 index 00000000000000..25300c97a6d2e1 --- /dev/null +++ b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/latest.ts @@ -0,0 +1,8 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export * from './v1'; diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/v1.test.ts b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/v1.test.ts new file mode 100644 index 00000000000000..e186621490dea0 --- /dev/null +++ b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/v1.test.ts @@ -0,0 +1,66 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { transformOperations } from './v1'; + +describe('transformOperations', () => { + const isSystemAction = (id: string) => id === 'my-system-action-id'; + + describe('actions', () => { + const defaultAction = { + id: 'default-action', + params: {}, + }; + + const systemAction = { + id: 'my-system-action-id', + params: {}, + }; + + it('transform the actions correctly', async () => { + expect( + transformOperations({ + operations: [ + { field: 'actions', operation: 'add', value: [defaultAction, systemAction] }, + ], + isSystemAction, + }) + ).toEqual([ + { + field: 'actions', + operation: 'add', + value: [ + { + group: 'default', + id: 'default-action', + params: {}, + type: 'default', + }, + { id: 'my-system-action-id', params: {}, type: 'system' }, + ], + }, + ]); + }); + + it('returns an empty array if the operations are empty', async () => { + expect( + transformOperations({ + operations: [], + isSystemAction, + }) + ).toEqual([]); + }); + + it('returns an empty array if the operations are undefined', async () => { + expect( + transformOperations({ + isSystemAction, + }) + ).toEqual([]); + }); + }); +}); diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/v1.ts b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/v1.ts new file mode 100644 index 00000000000000..aa88e9423f5687 --- /dev/null +++ b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/v1.ts @@ -0,0 +1,56 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { RuleActionTypes } from '../../../../../../../common'; +import { BulkEditOperation } from '../../../../../../application/rule/methods/bulk_edit'; +import { BulkEditRulesRequestBodyV1 } from '../../../../../../../common/routes/rule/apis/bulk_edit'; + +export const transformOperations = ({ + operations, + isSystemAction, +}: { + operations?: BulkEditRulesRequestBodyV1['operations']; + isSystemAction: (connectorId: string) => boolean; +}): BulkEditOperation[] => { + if (operations == null || operations.length === 0) { + return []; + } + + return operations.map((operation) => { + if (operation.field !== 'actions') { + return operation; + } + + const actions = operation.value.map((action) => { + if (isSystemAction(action.id)) { + return { + id: action.id, + params: action.params, + ...(action.uuid && { frequency: action.uuid }), + type: RuleActionTypes.SYSTEM, + }; + } + + return { + id: action.id, + group: action.group ?? 'default', + params: action.params, + uuid: action.uuid, + ...(action.frequency && { frequency: action.frequency }), + ...(action.uuid && { frequency: action.uuid }), + frequency: action.frequency, + type: RuleActionTypes.DEFAULT, + }; + }); + + return { + field: operation.field, + operation: operation.operation, + value: actions, + }; + }); +}; diff --git a/x-pack/plugins/alerting/server/rules_client/common/apply_bulk_edit_operation.test.ts b/x-pack/plugins/alerting/server/rules_client/common/apply_bulk_edit_operation.test.ts index 49ed183ceb39d0..0ca691dc9ddede 100644 --- a/x-pack/plugins/alerting/server/rules_client/common/apply_bulk_edit_operation.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/common/apply_bulk_edit_operation.test.ts @@ -6,7 +6,7 @@ */ import { applyBulkEditOperation } from './apply_bulk_edit_operation'; -import type { Rule } from '../../types'; +import { Rule, RuleActionTypes } from '../../types'; describe('applyBulkEditOperation', () => { describe('tags operations', () => { @@ -180,61 +180,120 @@ describe('applyBulkEditOperation', () => { describe('actions operations', () => { test('should add actions', () => { const ruleMock = { - actions: [{ id: 'mock-action-id', group: 'default', params: {} }], + actions: [ + { id: 'mock-action-id', group: 'default', params: {}, type: RuleActionTypes.DEFAULT }, + ], }; + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( { field: 'actions', value: [ - { id: 'mock-add-action-id-1', group: 'default', params: {} }, - { id: 'mock-add-action-id-2', group: 'default', params: {} }, + { + id: 'mock-add-action-id-1', + group: 'default', + params: {}, + type: RuleActionTypes.DEFAULT, + }, + { + id: 'mock-add-action-id-2', + group: 'default', + params: {}, + type: RuleActionTypes.DEFAULT, + }, ], operation: 'add', }, ruleMock ); + expect(modifiedAttributes).toHaveProperty('actions', [ - { id: 'mock-action-id', group: 'default', params: {} }, - { id: 'mock-add-action-id-1', group: 'default', params: {} }, - { id: 'mock-add-action-id-2', group: 'default', params: {} }, + { id: 'mock-action-id', group: 'default', params: {}, type: RuleActionTypes.DEFAULT }, + { id: 'mock-add-action-id-1', group: 'default', params: {}, type: RuleActionTypes.DEFAULT }, + { id: 'mock-add-action-id-2', group: 'default', params: {}, type: RuleActionTypes.DEFAULT }, ]); + expect(isAttributeModified).toBe(true); }); test('should add action with different params and same id', () => { const ruleMock = { - actions: [{ id: 'mock-action-id', group: 'default', params: { test: 1 } }], + actions: [ + { + id: 'mock-action-id', + group: 'default', + params: { test: 1 }, + type: RuleActionTypes.DEFAULT, + }, + ], }; + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( { field: 'actions', - value: [{ id: 'mock-action-id', group: 'default', params: { test: 2 } }], + value: [ + { + id: 'mock-action-id', + group: 'default', + params: { test: 2 }, + type: RuleActionTypes.DEFAULT, + }, + ], operation: 'add', }, ruleMock ); + expect(modifiedAttributes).toHaveProperty('actions', [ - { id: 'mock-action-id', group: 'default', params: { test: 1 } }, - { id: 'mock-action-id', group: 'default', params: { test: 2 } }, + { + id: 'mock-action-id', + group: 'default', + params: { test: 1 }, + type: RuleActionTypes.DEFAULT, + }, + { + id: 'mock-action-id', + group: 'default', + params: { test: 2 }, + type: RuleActionTypes.DEFAULT, + }, ]); + expect(isAttributeModified).toBe(true); }); test('should rewrite actions', () => { const ruleMock = { - actions: [{ id: 'mock-action-id', group: 'default', params: {} }], + actions: [ + { id: 'mock-action-id', group: 'default', params: {}, type: RuleActionTypes.DEFAULT }, + ], }; + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( { field: 'actions', - value: [{ id: 'mock-rewrite-action-id-1', group: 'default', params: {} }], + value: [ + { + id: 'mock-rewrite-action-id-1', + group: 'default', + params: {}, + type: RuleActionTypes.DEFAULT, + }, + ], operation: 'set', }, ruleMock ); + expect(modifiedAttributes).toHaveProperty('actions', [ - { id: 'mock-rewrite-action-id-1', group: 'default', params: {} }, + { + id: 'mock-rewrite-action-id-1', + group: 'default', + params: {}, + type: RuleActionTypes.DEFAULT, + }, ]); + expect(isAttributeModified).toBe(true); }); }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/bulk_edit.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/bulk_edit.ts new file mode 100644 index 00000000000000..0b1ed0f6ec38f5 --- /dev/null +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/bulk_edit.ts @@ -0,0 +1,181 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import expect from '@kbn/expect'; +import { RuleActionTypes } from '@kbn/alerting-plugin/common'; +import { omit } from 'lodash'; +import { Spaces } from '../../../scenarios'; +import { getUrlPrefix, getTestRuleData, ObjectRemover } from '../../../../common/lib'; +import { FtrProviderContext } from '../../../../common/ftr_provider_context'; + +// eslint-disable-next-line import/no-default-export +export default function createUpdateTests({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + + describe('bulkEdit', () => { + const objectRemover = new ObjectRemover(supertest); + + after(() => objectRemover.removeAll()); + + describe('system actions', () => { + const systemAction = { + id: 'system-connector-test.system-action', + uuid: '123', + params: {}, + type: RuleActionTypes.SYSTEM, + }; + + it('should bulk edit system actions correctly', async () => { + const { body: rule } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send(getTestRuleData()) + .expect(200); + + objectRemover.add(Spaces.space1.id, rule.id, 'rule', 'alerting'); + + const payload = { + ids: [rule.id], + operations: [ + { + operation: 'add', + field: 'actions', + value: [systemAction], + }, + ], + }; + + const res = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rules/_bulk_edit`) + .set('kbn-xsrf', 'foo') + .send(payload) + .expect(200); + + expect(res.body.rules[0].actions).to.eql([ + { + id: 'system-connector-test.system-action', + connector_type_id: 'test.system-action', + params: {}, + uuid: '123', + type: RuleActionTypes.SYSTEM, + }, + ]); + }); + + it('should throw 400 if the system action is missing required properties', async () => { + const { body: rule } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send(getTestRuleData()) + .expect(200); + + objectRemover.add(Spaces.space1.id, rule.id, 'rule', 'alerting'); + + for (const propertyToOmit of ['id', 'uuid']) { + const systemActionWithoutProperty = omit(systemAction, propertyToOmit); + + const payload = { + ids: [rule.id], + operations: [ + { + operation: 'add', + field: 'actions', + value: [systemActionWithoutProperty], + }, + ], + }; + + await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rules/_bulk_edit`) + .set('kbn-xsrf', 'foo') + .send(payload) + .expect(400); + } + }); + + it('should throw 400 if the system action contain properties from the default actions', async () => { + const { body: rule } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send(getTestRuleData()) + .expect(200); + + objectRemover.add(Spaces.space1.id, rule.id, 'rule', 'alerting'); + + for (const propertyAdd of [ + { group: 'test' }, + { + frequency: { + notify_when: 'onThrottleInterval' as const, + summary: true, + throttle: '1h', + }, + }, + { + alerts_filter: { + query: { dsl: '{test:1}', kql: 'test:1s', filters: [] }, + }, + }, + ]) { + const payload = { + ids: [rule.id], + operations: [ + { + operation: 'add', + field: 'actions', + value: [{ ...systemAction, ...propertyAdd }], + }, + ], + }; + + await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rules/_bulk_edit`) + .set('kbn-xsrf', 'foo') + .send(payload) + .expect(400); + } + }); + + it('should throw 400 if the system action is missing required params', async () => { + const { body: rule } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send(getTestRuleData()) + .expect(200); + + objectRemover.add(Spaces.space1.id, rule.id, 'rule', 'alerting'); + + const payload = { + ids: [rule.id], + operations: [ + { + operation: 'add', + field: 'actions', + value: [ + { + ...systemAction, + params: {}, + id: 'system-connector-test.system-action-connector-adapter', + }, + ], + }, + ], + }; + + const res = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rules/_bulk_edit`) + .set('kbn-xsrf', 'foo') + .send(payload) + .expect(200); + + expect(res.body.errors[0].message).to.eql( + 'Invalid system action params. System action type: test.system-action-connector-adapter - [myParam]: expected value of type [string] but got [undefined]' + ); + }); + }); + }); +} diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/index.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/index.ts index 1ccbb1c8f722d3..5b03010a602350 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/index.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/index.ts @@ -27,5 +27,6 @@ export default function alertingTests({ loadTestFile, getService }: FtrProviderC loadTestFile(require.resolve('./alerts_default_space')); loadTestFile(require.resolve('./transform_rule_types')); loadTestFile(require.resolve('./ml_rule_types')); + loadTestFile(require.resolve('./bulk_edit')); }); } From 432d35cfb0f57581e73b4463874e536f1f684455 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Fri, 6 Oct 2023 17:22:29 +0300 Subject: [PATCH 10/14] Fix tests --- .../methods/bulk_edit/bulk_edit_rules.test.ts | 62 +++++++++++++------ .../rule/methods/bulk_edit/bulk_edit_rules.ts | 20 +++--- 2 files changed, 53 insertions(+), 29 deletions(-) diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.test.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.test.ts index 33b354529ee69b..9b0ff96f400664 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.test.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.test.ts @@ -941,7 +941,7 @@ describe('bulkEdit()', () => { }); test('should add system and default actions', async () => { - const newAction = { + const defaultAction = { frequency: { notifyWhen: 'onActiveAlert' as const, summary: false, @@ -953,7 +953,7 @@ describe('bulkEdit()', () => { type: RuleActionTypes.DEFAULT, }; - const newAction2 = { + const systemAction = { id: 'system_action-id', params: {}, type: RuleActionTypes.SYSTEM, @@ -967,13 +967,21 @@ describe('bulkEdit()', () => { ...existingRule.attributes, actions: [ { - ...newAction, + frequency: { + notifyWhen: 'onActiveAlert' as const, + summary: false, + throttle: null, + }, + group: 'default', + id: '1', + params: {}, actionRef: 'action_0', actionTypeId: 'test-1', uuid: '222', }, { - ...newAction2, + id: 'system_action-id', + params: {}, actionRef: 'system_action:system_action-id', actionTypeId: 'test-2', uuid: '222', @@ -1020,7 +1028,7 @@ describe('bulkEdit()', () => { { field: 'actions', operation: 'add', - value: [newAction, newAction2], + value: [defaultAction, systemAction], }, ], }); @@ -1073,8 +1081,8 @@ describe('bulkEdit()', () => { lastExecutionDate: new Date(existingRule.attributes.executionStatus.lastExecutionDate), }, actions: [ - { ...newAction, actionTypeId: 'test-1', uuid: '222' }, - { ...newAction2, actionTypeId: 'test-2', uuid: '222' }, + { ...defaultAction, actionTypeId: 'test-1', uuid: '222' }, + { ...systemAction, actionTypeId: 'test-2', uuid: '222' }, ], id: existingRule.id, snoozeSchedule: [], @@ -1082,7 +1090,7 @@ describe('bulkEdit()', () => { }); test('should construct the refs correctly and not persist the type of the action', async () => { - const newAction = { + const defaultAction = { frequency: { notifyWhen: 'onActiveAlert' as const, summary: false, @@ -1094,7 +1102,7 @@ describe('bulkEdit()', () => { type: RuleActionTypes.DEFAULT, }; - const newAction2 = { + const systemAction = { id: 'system_action-id', params: {}, type: RuleActionTypes.SYSTEM, @@ -1108,13 +1116,21 @@ describe('bulkEdit()', () => { ...existingRule.attributes, actions: [ { - ...newAction, + frequency: { + notifyWhen: 'onActiveAlert' as const, + summary: false, + throttle: null, + }, + group: 'default', + id: '1', + params: {}, actionRef: 'action_0', actionTypeId: 'test-1', uuid: '222', }, { - ...newAction2, + id: 'system_action-id', + params: {}, actionRef: 'system_action:system_action-id', actionTypeId: 'test-2', uuid: '222', @@ -1161,7 +1177,7 @@ describe('bulkEdit()', () => { { field: 'actions', operation: 'add', - value: [newAction, newAction2], + value: [defaultAction, systemAction], }, ], }); @@ -1189,7 +1205,7 @@ describe('bulkEdit()', () => { }); test('should add the actions type to the response correctly', async () => { - const newAction = { + const defaultAction = { frequency: { notifyWhen: 'onActiveAlert' as const, summary: false, @@ -1201,7 +1217,7 @@ describe('bulkEdit()', () => { type: RuleActionTypes.DEFAULT, }; - const newAction2 = { + const systemAction = { id: 'system_action-id', params: {}, type: RuleActionTypes.SYSTEM, @@ -1215,13 +1231,21 @@ describe('bulkEdit()', () => { ...existingRule.attributes, actions: [ { - ...newAction, + frequency: { + notifyWhen: 'onActiveAlert' as const, + summary: false, + throttle: null, + }, + group: 'default', + id: '1', + params: {}, actionRef: 'action_0', actionTypeId: 'test-1', uuid: '222', }, { - ...newAction2, + id: 'system_action-id', + params: {}, actionRef: 'system_action:system_action-id', actionTypeId: 'test-2', uuid: '222', @@ -1268,14 +1292,14 @@ describe('bulkEdit()', () => { { field: 'actions', operation: 'add', - value: [newAction, newAction2], + value: [defaultAction, systemAction], }, ], }); expect(result.rules[0].actions).toEqual([ - { ...newAction, actionTypeId: 'test-1', uuid: '222' }, - { ...newAction2, actionTypeId: 'test-2', uuid: '222' }, + { ...defaultAction, actionTypeId: 'test-1', uuid: '222' }, + { ...systemAction, actionTypeId: 'test-2', uuid: '222' }, ]); }); diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.ts index ee6e4732618193..a99b060ba563ef 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.ts @@ -533,21 +533,21 @@ async function updateRuleAttributesAndParamsInMemory( ruleType.validate.params ); - const { - actions: rawAlertActions, - references, - params: updatedParams, - } = await extractReferences( + const { references, params: updatedParams } = await extractReferences( context, ruleType, updatedRuleActions as NormalizedAlertActionWithGeneratedValues[], validatedMutatedAlertTypeParams ); - const ruleAttributes = transformRuleDomainToRuleAttributes(updatedRule, { - legacyId: rule.attributes.legacyId, - actionsWithRefs: rawAlertActions, - paramsWithRefs: updatedParams as RuleAttributes['params'], + const ruleAttributes = await transformRuleDomainToRuleAttributes({ + actions: updatedRuleActions as NormalizedAlertActionWithGeneratedValues[], + context, + rule: updatedRule, + params: { + legacyId: rule.attributes.legacyId, + paramsWithRefs: updatedParams as RuleAttributes['params'], + }, }); const { apiKeyAttributes } = await prepareApiKeys( @@ -565,7 +565,7 @@ async function updateRuleAttributesAndParamsInMemory( ruleAttributes, apiKeyAttributes, updatedParams, - rawAlertActions, + ruleAttributes.actions, username ); From 853d3761b0b16e8f3482f5c728066cac5ee131de Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Thu, 26 Oct 2023 15:53:42 +0200 Subject: [PATCH 11/14] fixes after merge with system_actions_mvp branch --- x-pack/plugins/alerting/common/index.ts | 2 - .../common/routes/rule/response/schemas/v1.ts | 42 +------------------ .../rule/methods/bulk_edit/bulk_edit_rules.ts | 9 +++- 3 files changed, 9 insertions(+), 44 deletions(-) diff --git a/x-pack/plugins/alerting/common/index.ts b/x-pack/plugins/alerting/common/index.ts index 8b0aee7f12d60f..ea85bf8983a387 100644 --- a/x-pack/plugins/alerting/common/index.ts +++ b/x-pack/plugins/alerting/common/index.ts @@ -41,8 +41,6 @@ export * from './rule_circuit_breaker_error_message'; export { isSystemAction } from './system_actions/is_system_action'; -export { isSystemAction } from './system_actions/is_system_action'; - export type { MaintenanceWindowModificationMetadata, DateRange, diff --git a/x-pack/plugins/alerting/common/routes/rule/response/schemas/v1.ts b/x-pack/plugins/alerting/common/routes/rule/response/schemas/v1.ts index daa90b84faad72..93676f26689fca 100644 --- a/x-pack/plugins/alerting/common/routes/rule/response/schemas/v1.ts +++ b/x-pack/plugins/alerting/common/routes/rule/response/schemas/v1.ts @@ -6,6 +6,7 @@ */ import { schema } from '@kbn/config-schema'; +import { rRuleResponseSchemaV1 } from '../../../r_rule'; import { ruleNotifyWhen as ruleNotifyWhenV1, ruleExecutionStatusValues as ruleExecutionStatusValuesV1, @@ -180,48 +181,9 @@ export const monitoringSchema = schema.object({ }), }); -export const rRuleSchema = schema.object({ - dtstart: schema.string(), - tzid: schema.string(), - freq: schema.maybe( - schema.oneOf([ - schema.literal(0), - schema.literal(1), - schema.literal(2), - schema.literal(3), - schema.literal(4), - schema.literal(5), - schema.literal(6), - ]) - ), - until: schema.maybe(schema.string()), - count: schema.maybe(schema.number()), - interval: schema.maybe(schema.number()), - wkst: schema.maybe( - schema.oneOf([ - schema.literal('MO'), - schema.literal('TU'), - schema.literal('WE'), - schema.literal('TH'), - schema.literal('FR'), - schema.literal('SA'), - schema.literal('SU'), - ]) - ), - byweekday: schema.maybe(schema.arrayOf(schema.oneOf([schema.string(), schema.number()]))), - bymonth: schema.maybe(schema.arrayOf(schema.number())), - bysetpos: schema.maybe(schema.arrayOf(schema.number())), - bymonthday: schema.arrayOf(schema.number()), - byyearday: schema.arrayOf(schema.number()), - byweekno: schema.arrayOf(schema.number()), - byhour: schema.arrayOf(schema.number()), - byminute: schema.arrayOf(schema.number()), - bysecond: schema.arrayOf(schema.number()), -}); - export const ruleSnoozeScheduleSchema = schema.object({ duration: schema.number(), - rRule: rRuleSchema, + rRule: rRuleResponseSchemaV1, id: schema.maybe(schema.string()), skipRecurrences: schema.maybe(schema.arrayOf(schema.string())), }); diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.ts index 4dbf781b25518a..efa589919c39d9 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.ts @@ -80,6 +80,7 @@ import { } from '../../transforms'; import { validateScheduleLimit, ValidateScheduleLimitResult } from '../get_schedule_frequency'; import { bulkEditOperationsSchema } from './schemas'; +import { denormalizeActions } from '../../../../rules_client/lib/denormalize_actions'; const isValidInterval = (interval: string | undefined): interval is string => { return interval !== undefined; @@ -546,9 +547,13 @@ async function updateRuleAttributesAndParamsInMemory( validatedMutatedAlertTypeParams ); - const ruleAttributes = await transformRuleDomainToRuleAttributes({ - actions: updatedRuleActions as NormalizedAlertActionWithGeneratedValues[], + const { actions: actionsWithRefs } = await denormalizeActions( context, + updatedRuleActions as NormalizedAlertActionWithGeneratedValues[] + ); + + const ruleAttributes = transformRuleDomainToRuleAttributes({ + actionsWithRefs, rule: updatedRule, params: { legacyId: rule.attributes.legacyId, From bc18d2732289e0134c22853aa9ce6e2915df4945 Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Fri, 27 Oct 2023 12:45:50 +0200 Subject: [PATCH 12/14] fix bulk edit rule unit tests --- .../rule/methods/bulk_edit/bulk_edit_rules.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.test.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.test.ts index 4013bc66eb41fb..06ccb726afd2d7 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.test.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/bulk_edit/bulk_edit_rules.test.ts @@ -38,6 +38,7 @@ import { ConnectorAdapterRegistry } from '../../../../connector_adapters/connect import { ConnectorAdapter } from '../../../../connector_adapters/types'; import { RuleAttributes } from '../../../../data/rule/types'; import { SavedObject } from '@kbn/core/server'; +import { bulkEditOperationsSchema } from './schemas'; jest.mock('../../../../rules_client/lib/siem_legacy_actions/migrate_legacy_actions', () => { return { @@ -1661,6 +1662,9 @@ describe('bulkEdit()', () => { }); test('should skip operation when params modifiers does not modify index pattern array', async () => { + const originalValidate = bulkEditOperationsSchema.validate; + bulkEditOperationsSchema.validate = jest.fn(); + paramsModifier.mockResolvedValue({ modifiedParams: { index: ['test-1', 'test-2'], @@ -1670,13 +1674,7 @@ describe('bulkEdit()', () => { const result = await rulesClient.bulkEdit({ filter: '', - operations: [ - { - field: 'tags', - operation: 'add', - value: ['test-tag'], - }, - ], + operations: [], paramsModifier, }); @@ -1685,6 +1683,8 @@ describe('bulkEdit()', () => { expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(0); expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(0); + + bulkEditOperationsSchema.validate = originalValidate; }); }); From 865b46ed1bcfd05a95d3afdc4b2012f727d1611a Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Fri, 27 Oct 2023 14:36:33 +0200 Subject: [PATCH 13/14] fix typo --- .../rule/apis/bulk_edit/transforms/transform_operations/v1.ts | 4 ++-- .../server/routes/rule/apis/create/create_rule_route.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/v1.ts b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/v1.ts index aa88e9423f5687..0b6838bbdaaccf 100644 --- a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/v1.ts +++ b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_edit/transforms/transform_operations/v1.ts @@ -30,7 +30,7 @@ export const transformOperations = ({ return { id: action.id, params: action.params, - ...(action.uuid && { frequency: action.uuid }), + ...(action.uuid && { uuid: action.uuid }), type: RuleActionTypes.SYSTEM, }; } @@ -41,7 +41,7 @@ export const transformOperations = ({ params: action.params, uuid: action.uuid, ...(action.frequency && { frequency: action.frequency }), - ...(action.uuid && { frequency: action.uuid }), + ...(action.uuid && { uuid: action.uuid }), frequency: action.frequency, type: RuleActionTypes.DEFAULT, }; diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/create/create_rule_route.ts b/x-pack/plugins/alerting/server/routes/rule/apis/create/create_rule_route.ts index a6b252497bace5..8e3bd597d6c7ad 100644 --- a/x-pack/plugins/alerting/server/routes/rule/apis/create/create_rule_route.ts +++ b/x-pack/plugins/alerting/server/routes/rule/apis/create/create_rule_route.ts @@ -13,7 +13,7 @@ import { } from '../../../lib'; import { BASE_ALERTING_API_PATH } from '../../../../types'; import { RouteOptions } from '../../..'; -import { +import type { CreateRuleRequestBodyV1, CreateRuleRequestParamsV1, CreateRuleResponseV1, From 5045a472de476beac165b1559d1156d51b00998c Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Fri, 27 Oct 2023 21:06:18 +0200 Subject: [PATCH 14/14] fix integrational tests --- .../tests/alerting/group2/bulk_edit.ts | 48 +------------------ 1 file changed, 1 insertion(+), 47 deletions(-) diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/bulk_edit.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/bulk_edit.ts index 0b1ed0f6ec38f5..c1311bd16329ae 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/bulk_edit.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/bulk_edit.ts @@ -6,7 +6,6 @@ */ import expect from '@kbn/expect'; -import { RuleActionTypes } from '@kbn/alerting-plugin/common'; import { omit } from 'lodash'; import { Spaces } from '../../../scenarios'; import { getUrlPrefix, getTestRuleData, ObjectRemover } from '../../../../common/lib'; @@ -26,7 +25,6 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { id: 'system-connector-test.system-action', uuid: '123', params: {}, - type: RuleActionTypes.SYSTEM, }; it('should bulk edit system actions correctly', async () => { @@ -61,7 +59,6 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { connector_type_id: 'test.system-action', params: {}, uuid: '123', - type: RuleActionTypes.SYSTEM, }, ]); }); @@ -75,7 +72,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { objectRemover.add(Spaces.space1.id, rule.id, 'rule', 'alerting'); - for (const propertyToOmit of ['id', 'uuid']) { + for (const propertyToOmit of ['id']) { const systemActionWithoutProperty = omit(systemAction, propertyToOmit); const payload = { @@ -97,49 +94,6 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { } }); - it('should throw 400 if the system action contain properties from the default actions', async () => { - const { body: rule } = await supertest - .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) - .set('kbn-xsrf', 'foo') - .send(getTestRuleData()) - .expect(200); - - objectRemover.add(Spaces.space1.id, rule.id, 'rule', 'alerting'); - - for (const propertyAdd of [ - { group: 'test' }, - { - frequency: { - notify_when: 'onThrottleInterval' as const, - summary: true, - throttle: '1h', - }, - }, - { - alerts_filter: { - query: { dsl: '{test:1}', kql: 'test:1s', filters: [] }, - }, - }, - ]) { - const payload = { - ids: [rule.id], - operations: [ - { - operation: 'add', - field: 'actions', - value: [{ ...systemAction, ...propertyAdd }], - }, - ], - }; - - await supertest - .post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rules/_bulk_edit`) - .set('kbn-xsrf', 'foo') - .send(payload) - .expect(400); - } - }); - it('should throw 400 if the system action is missing required params', async () => { const { body: rule } = await supertest .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`)