From b310c8aca58c6cd57bdd747b3f30fc17f9282e18 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 3 Oct 2023 14:14:16 +0300 Subject: [PATCH 01/20] 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 98d82abf62be4..498e1501ae033 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 63bd8dd4f8f4a..065816c321aba 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 bf6218614206c..bc3a9c1f39a68 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 616a16a8315ed..2a1f75d434e19 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 ce51e88c5ce73..4cdca61fec89b 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 f123466eca1ab..38f04823870c7 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 0000000000000..eb9f21979e4fb --- /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 0000000000000..d6c7533befdf1 --- /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 0000000000000..a95ff00f45bd3 --- /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 26831b9dff81c..a90d1c900c644 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 0000000000000..abfd56f0e9079 --- /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 aa8adda873cde..239422d5ad28a 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 0000000000000..de3a45d5c09b9 --- /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 0000000000000..ebe9426297ca2 --- /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 e952a72ec3859..b84c53b9b5409 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 6b28b64284904..a6b252497bace 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 0000000000000..b3f041fe26632 --- /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 e43427fe4470a..08b915b744bc0 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 bbf192e4f1cf4..2938c91372325 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 89dce3ed90451..6be77417ddce8 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 31f06344d45f4..79cb83f13da9f 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 337d4daab903b..4e982bae3fa90 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 66e2c3bfa6069..1e5e91ad764fa 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/20] 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 63bd8dd4f8f4a..065816c321aba 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 6aa1c44fe6e81..66d1f707bb623 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 89dce3ed90451..14f85886bb698 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/20] 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 e2e9e477cc4cc..e0a23e50ca222 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 0000000000000..ee5c030c2c5dc --- /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 0000000000000..ae9958b20b6b8 --- /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/20] 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 d12b3428e62eb..fde39f906485a 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 163b9415c7e5d..fb795f4777b34 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 84fb89de01cbf..ca2dceb497841 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 158d5bbcbcf37..94d3486ec96aa 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 f6bb71aef7453..0218d24652155 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 fe50e6e3632b6..2ccfcd84400b7 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 939dfe9406f69..142bfc215a3b1 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 41140b5b25df7..f7528eaffe654 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/20] 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 aa8adda873cde..b0e2a58a72b21 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 62b6259afb43c..8bde9a52b3805 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 31f06344d45f4..857a3e3de43aa 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 337d4daab903b..fba3d89f5abf4 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/20] 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 7fdd7668135a4..2a19e31381fcd 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 3307211694cc0..fb6d84e81772f 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 a7d5dbc138ea4..f2753cb99a24a 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 0000000000000..41526e0949de3 --- /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 7a257d214f26a..c24a15682649f 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/20] [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 1e5e91ad764fa..c212c1feaa00d 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/20] 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 1c093314f7a47..daa90b84faad7 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 bc3a9c1f39a68..d0985ebe682cb 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 2a1f75d434e19..7c36d8b382bf5 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 0000000000000..a167f2ba3a2a7 --- /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 0000000000000..e3fcce033ca38 --- /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 eb9f21979e4fb..bbe84ed2534ce 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 d6c7533befdf1..aad02639e69c7 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 20eef8f851e08..fc69ff9a73ec4 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 c6b29f4577f4c..51ccc96d4e9c6 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 db1dca186aee9..5652357510b53 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 a7525cf9b5b19..c3e7b5cd2a682 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 74fd3b3291d4e..ac09a788b528b 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 5a1e9911b5fcb..52c9075b6fa18 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 395756661b7c75e471dddfcdb30c2ca2d28e5f6b Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Fri, 6 Oct 2023 16:40:31 +0300 Subject: [PATCH 09/20] Add integration test --- .../tests/alerting/group1/create.ts | 146 +++++++++++++++++- 1 file changed, 138 insertions(+), 8 deletions(-) diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/create.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/create.ts index eb9f90cb41f2a..cd3e998fd6336 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/create.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/create.ts @@ -7,15 +7,16 @@ import expect from '@kbn/expect'; import { SavedObject } from '@kbn/core/server'; -import { RawRule } from '@kbn/alerting-plugin/server/types'; +import { RawRule, RuleActionTypes } from '@kbn/alerting-plugin/server/types'; import { ALERTING_CASES_SAVED_OBJECT_INDEX } from '@kbn/core-saved-objects-server'; +import { omit } from 'lodash'; import { Spaces } from '../../../scenarios'; import { checkAAD, getUrlPrefix, getTestRuleData, ObjectRemover, - getUnauthorizedErrorMessage, + getConsumerUnauthorizedErrorMessage, TaskManagerDoc, } from '../../../../common/lib'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; @@ -159,8 +160,10 @@ export default function createAlertTests({ getService }: FtrProviderContext) { }, { id: 'system-connector-test.system-action', - group: 'default', + actionTypeId: 'test.system-action', + uuid: '123', params: {}, + type: RuleActionTypes.SYSTEM, }, ], }) @@ -193,10 +196,10 @@ export default function createAlertTests({ getService }: FtrProviderContext) { }, { id: 'system-connector-test.system-action', - group: 'default', connector_type_id: 'test.system-action', params: {}, - uuid: response.body.actions[2].uuid, + uuid: '123', + type: RuleActionTypes.SYSTEM, }, ], enabled: true, @@ -255,9 +258,9 @@ export default function createAlertTests({ getService }: FtrProviderContext) { { actionRef: 'system_action:system-connector-test.system-action', actionTypeId: 'test.system-action', - group: 'default', params: {}, - uuid: rawActions[2].uuid, + uuid: '123', + type: RuleActionTypes.SYSTEM, }, ]); @@ -444,7 +447,7 @@ export default function createAlertTests({ getService }: FtrProviderContext) { expect(response.status).to.eql(403); expect(response.body).to.eql({ error: 'Forbidden', - message: getUnauthorizedErrorMessage( + message: getConsumerUnauthorizedErrorMessage( 'create', 'test.noop', 'some consumer patrick invented' @@ -464,6 +467,133 @@ export default function createAlertTests({ getService }: FtrProviderContext) { expect(response.body.scheduledTaskId).to.eql(undefined); }); + describe('system actions', () => { + const systemAction = { + id: 'system-connector-test.system-action', + actionTypeId: 'test.system-action', + uuid: '123', + params: {}, + type: RuleActionTypes.SYSTEM, + }; + + it('should create a rule with a system action correctly', async () => { + const response = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send( + getTestRuleData({ + actions: [systemAction], + }) + ); + + expect(response.status).to.eql(200); + + objectRemover.add(Spaces.space1.id, response.body.id, 'rule', 'alerting'); + + expect(response.body.actions).to.eql([ + { + id: 'system-connector-test.system-action', + connector_type_id: 'test.system-action', + params: {}, + uuid: '123', + type: RuleActionTypes.SYSTEM, + }, + ]); + + const esResponse = await es.get>( + { + index: ALERTING_CASES_SAVED_OBJECT_INDEX, + id: `alert:${response.body.id}`, + }, + { meta: true } + ); + + expect(esResponse.statusCode).to.eql(200); + const rawActions = (esResponse.body._source as any)?.alert.actions ?? []; + + expect(rawActions).to.eql([ + { + actionRef: 'system_action:system-connector-test.system-action', + actionTypeId: 'test.system-action', + params: {}, + uuid: '123', + type: RuleActionTypes.SYSTEM, + }, + ]); + + const references = esResponse.body._source?.references ?? []; + + expect(references.length).to.eql(0); + }); + + it('should throw 400 if the system action is missing required properties', async () => { + for (const propertyToOmit of ['id', 'actionTypeId', 'uuid']) { + const systemActionWithoutProperty = omit(systemAction, propertyToOmit); + + await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send( + getTestRuleData({ + actions: [systemActionWithoutProperty], + }) + ) + .expect(400); + } + }); + + it('should throw 400 if the system action contain properties from the default actions', async () => { + 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: [] }, + }, + }, + ]) { + await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send( + getTestRuleData({ + actions: [{ ...systemAction, ...propertyAdd }], + }) + ) + .expect(400); + } + }); + + it('should throw 400 if the system action is missing required params', async () => { + const res = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send( + getTestRuleData({ + actions: [ + { + ...systemAction, + params: {}, + id: 'system-connector-test.system-action-connector-adapter', + actionTypeId: 'test.test.system-action-connector-adapter', + }, + ], + }) + ) + .expect(400); + + expect(res.body.message).to.eql( + 'Invalid system action params. System action type: test.system-action-connector-adapter - [myParam]: expected value of type [string] but got [undefined]' + ); + }); + }); + describe('legacy', () => { it('should handle create alert request appropriately', async () => { const { body: createdAction } = await supertest From 35153cace0d2ae4923fa944bd40a8c1dded7fe9a Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Tue, 24 Oct 2023 11:48:32 +0200 Subject: [PATCH 10/20] devide system actions validation into 2 functions --- .../rule/methods/create/create_rule.ts | 5 ++-- .../server/lib/validate_system_actions.ts | 27 ++++++++++++++++--- 2 files changed, 25 insertions(+), 7 deletions(-) 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 7c36d8b382bf5..4fcec129c8268 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,7 +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 { validateSystemActionsWithRuleTypeId } from '../../../../lib/validate_system_actions'; import { parseDuration } from '../../../../../common/parse_duration'; import { WriteOperations, AlertingAuthorizationEntity } from '../../../../authorization'; import { @@ -80,8 +80,7 @@ export async function createRule( throw Boom.badRequest(`Error validating create data - ${error.message}`); } - await validateSystemActions({ - actionsClient, + validateSystemActionsWithRuleTypeId({ connectorAdapterRegistry: context.connectorAdapterRegistry, systemActions, }); diff --git a/x-pack/plugins/alerting/server/lib/validate_system_actions.ts b/x-pack/plugins/alerting/server/lib/validate_system_actions.ts index ebe9426297ca2..8c4dafb1f4593 100644 --- a/x-pack/plugins/alerting/server/lib/validate_system_actions.ts +++ b/x-pack/plugins/alerting/server/lib/validate_system_actions.ts @@ -11,24 +11,43 @@ import { ConnectorAdapterRegistry } from '../connector_adapters/connector_adapte import { bulkValidateConnectorAdapterActionParams } from '../connector_adapters/validate_rule_action_params'; import { NormalizedSystemAction } from '../rules_client'; import { RuleSystemAction } from '../types'; -interface Params { +interface ValidateSystemActionsWithRuleTypeIdParams { + connectorAdapterRegistry: ConnectorAdapterRegistry; + systemActions: RuleSystemAction[]; +} + +interface ValidateSystemActionsWithoutRuleTypeIdParams { actionsClient: ActionsClient; connectorAdapterRegistry: ConnectorAdapterRegistry; systemActions: Array; } -export const validateSystemActions = async ({ +export const validateSystemActionsWithRuleTypeId = ({ + connectorAdapterRegistry, + systemActions, +}: ValidateSystemActionsWithRuleTypeIdParams) => { + if (systemActions.length === 0) { + return; + } + + bulkValidateConnectorAdapterActionParams({ + connectorAdapterRegistry, + actions: systemActions, + }); +}; + +export const validateSystemActionsWithoutRuleTypeId = async ({ actionsClient, connectorAdapterRegistry, systemActions, -}: Params) => { +}: ValidateSystemActionsWithoutRuleTypeIdParams) => { if (systemActions.length === 0) { return; } /** * When updating or creating a rule the actions may not contain - * the actionTypeId. We need to getBulk using the + * the actionTypeId (for example: bulk operations). 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 From c6a4bf0493c23c7d7fb30a8794f6910c25f1fb5f Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Tue, 24 Oct 2023 12:51:41 +0200 Subject: [PATCH 11/20] make transform functions not async --- .../rule/methods/create/create_rule.ts | 8 +++--- ...transform_domain_actions_to_raw_actions.ts | 25 +++---------------- ...ransform_rule_domain_to_rule_attributes.ts | 22 ++++++---------- .../rules_client/lib/denormalize_actions.ts | 13 ++++------ .../alerting/server/rules_client/types.ts | 8 ++++++ 5 files changed, 30 insertions(+), 46 deletions(-) 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 4fcec129c8268..5517d87ba5c10 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 @@ -38,6 +38,7 @@ import type { CreateRuleData } from './types'; import { createRuleDataSchema } from './schemas'; import { createRuleSavedObject } from '../../../../rules_client/lib'; import { validateScheduleLimit } from '../get_schedule_frequency'; +import { denormalizeActions } from '../../../../rules_client/lib/denormalize_actions'; export interface CreateRuleOptions { id?: string; @@ -160,10 +161,11 @@ export async function createRule( const notifyWhen = getRuleNotifyWhenType(data.notifyWhen ?? null, data.throttle ?? null); const throttle = data.throttle ?? null; + const { actions: actionsWithRefs } = await denormalizeActions(context, data.actions); + // Convert domain rule object to ES rule attributes - const ruleAttributes = await transformRuleDomainToRuleAttributes({ - actions: data.actions, - context, + const ruleAttributes = transformRuleDomainToRuleAttributes({ + actionsWithRefs, rule: { ...data, // TODO (http-versioning) create a rule domain version of this function 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 index e3fcce033ca38..5477fd8726006 100644 --- 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 @@ -6,28 +6,11 @@ */ import { RuleAttributes } from '../../../data/rule/types'; -import { - NormalizedAlertActionWithGeneratedValues, - RulesClientContext, -} from '../../../rules_client'; -import { denormalizeActions } from '../../../rules_client/lib/denormalize_actions'; +import { DenormalizedAction } from '../../../rules_client'; interface Args { - actions: NormalizedAlertActionWithGeneratedValues[]; - context: RulesClientContext; + actions: DenormalizedAction[]; } -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; -}; +export const transformDomainActionsToRawActions = ({ actions }: Args): RuleAttributes['actions'] => + actions.map(({ type, ...restAction }) => restAction); 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 fc69ff9a73ec4..0672062daf5ec 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,14 +4,11 @@ * 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'; +import { DenormalizedAction } from '../../../rules_client'; interface TransformRuleToEsParams { legacyId: RuleAttributes['legacyId']; @@ -19,23 +16,20 @@ interface TransformRuleToEsParams { meta?: RuleAttributes['meta']; } -export const transformRuleDomainToRuleAttributes = async ({ - actions, +export const transformRuleDomainToRuleAttributes = ({ + actionsWithRefs, rule, params, - context, }: { - actions: NormalizedAlertActionWithGeneratedValues[]; + actionsWithRefs: DenormalizedAction[]; rule: Omit; params: TransformRuleToEsParams; - context: RulesClientContext; -}): Promise => { +}): RuleAttributes => { const { legacyId, paramsWithRefs, meta } = params; const mappedParams = getMappedParams(paramsWithRefs); - const actionsWithRefs = await transformDomainActionsToRawActions({ - actions, - context, + const transformedActionsWithRefs = transformDomainActionsToRawActions({ + actions: actionsWithRefs, }); return { @@ -46,7 +40,7 @@ export const transformRuleDomainToRuleAttributes = async ({ consumer: rule.consumer, legacyId, schedule: rule.schedule, - actions: actionsWithRefs, + actions: transformedActionsWithRefs, params: paramsWithRefs, ...(Object.keys(mappedParams).length ? { mapped_params: mappedParams } : {}), ...(rule.scheduledTaskId !== undefined ? { scheduledTaskId: rule.scheduledTaskId } : {}), 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 5652357510b53..3cd1113a13628 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 @@ -4,19 +4,16 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ - -import { DistributiveOmit } from '@elastic/eui'; import { SavedObjectReference } from '@kbn/core/server'; import { preconfiguredConnectorActionRefPrefix, systemConnectorActionRefPrefix, } from '../common/constants'; -import { NormalizedAlertActionWithGeneratedValues, RulesClientContext } from '../types'; - -type DenormalizedAction = DistributiveOmit & { - actionRef: string; - actionTypeId: string; -}; +import { + DenormalizedAction, + NormalizedAlertActionWithGeneratedValues, + RulesClientContext, +} from '../types'; export async function denormalizeActions( context: RulesClientContext, diff --git a/x-pack/plugins/alerting/server/rules_client/types.ts b/x-pack/plugins/alerting/server/rules_client/types.ts index 37fa7f650dd4e..a068352d735a0 100644 --- a/x-pack/plugins/alerting/server/rules_client/types.ts +++ b/x-pack/plugins/alerting/server/rules_client/types.ts @@ -179,3 +179,11 @@ export interface RuleBulkOperationAggregation { }>; }; } + +export type DenormalizedAction = DistributiveOmit< + NormalizedAlertActionWithGeneratedValues, + 'id' +> & { + actionRef: string; + actionTypeId: string; +}; From 71898c9a25f6a521b111437ba978876d4aa9379f Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 24 Oct 2023 17:41:33 +0300 Subject: [PATCH 12/20] Address PR feedback --- .../common/routes/rule/response/schemas/v1.ts | 42 +------------------ .../rule/methods/create/create_rule.ts | 9 +++- .../rule/apis/create/create_rule_route.ts | 2 +- .../alerting/server/rules_client_factory.ts | 10 +++++ 4 files changed, 20 insertions(+), 43 deletions(-) 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 daa90b84faad7..93676f26689fc 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/create/create_rule.ts b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts index 901309fc5d0f1..2dd1ad56d77ac 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 @@ -26,7 +26,12 @@ 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 { RuleActionTypes, RuleSystemAction, SanitizedRule } from '../../../../types'; +import { + getRuleCircuitBreakerErrorMessage, + RuleActionTypes, + RuleSystemAction, + SanitizedRule, +} from '../../../../types'; import { transformRuleAttributesToRuleDomain, transformRuleDomainToRuleAttributes, @@ -37,7 +42,7 @@ import { RuleAttributes } from '../../../../data/rule/types'; import type { CreateRuleData } from './types'; import { createRuleDataSchema } from './schemas'; import { createRuleSavedObject } from '../../../../rules_client/lib'; -import { validateScheduleLimit } from '../get_schedule_frequency'; +import { validateScheduleLimit, ValidateScheduleLimitResult } from '../get_schedule_frequency'; import { denormalizeActions } from '../../../../rules_client/lib/denormalize_actions'; export interface CreateRuleOptions { 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 a6b252497bace..8e3bd597d6c7a 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, diff --git a/x-pack/plugins/alerting/server/rules_client_factory.ts b/x-pack/plugins/alerting/server/rules_client_factory.ts index 4e982bae3fa90..f8cb83755f974 100644 --- a/x-pack/plugins/alerting/server/rules_client_factory.ts +++ b/x-pack/plugins/alerting/server/rules_client_factory.ts @@ -26,6 +26,8 @@ import { RuleTypeRegistry, SpaceIdToNamespaceFunction } from './types'; import { RulesClient } from './rules_client'; import { AlertingAuthorizationClientFactory } from './alerting_authorization_client_factory'; import { AlertingRulesConfig } from './config'; +import { GetAlertIndicesAlias } from './lib'; +import { AlertsService } from './alerts_service/alerts_service'; import { ConnectorAdapterRegistry } from './connector_adapters/connector_adapter_registry'; export interface RulesClientFactoryOpts { logger: Logger; @@ -44,6 +46,8 @@ export interface RulesClientFactoryOpts { eventLogger?: IEventLogger; minimumScheduleInterval: AlertingRulesConfig['minimumScheduleInterval']; maxScheduledPerMinute: AlertingRulesConfig['maxScheduledPerMinute']; + getAlertIndicesAlias: GetAlertIndicesAlias; + alertsService: AlertsService | null; connectorAdapterRegistry: ConnectorAdapterRegistry; } @@ -65,6 +69,8 @@ export class RulesClientFactory { private eventLogger?: IEventLogger; private minimumScheduleInterval!: AlertingRulesConfig['minimumScheduleInterval']; private maxScheduledPerMinute!: AlertingRulesConfig['maxScheduledPerMinute']; + private getAlertIndicesAlias!: GetAlertIndicesAlias; + private alertsService!: AlertsService | null; private connectorAdapterRegistry!: ConnectorAdapterRegistry; public initialize(options: RulesClientFactoryOpts) { @@ -88,6 +94,8 @@ export class RulesClientFactory { this.eventLogger = options.eventLogger; this.minimumScheduleInterval = options.minimumScheduleInterval; this.maxScheduledPerMinute = options.maxScheduledPerMinute; + this.getAlertIndicesAlias = options.getAlertIndicesAlias; + this.alertsService = options.alertsService; this.connectorAdapterRegistry = options.connectorAdapterRegistry; } @@ -117,6 +125,8 @@ export class RulesClientFactory { internalSavedObjectsRepository: this.internalSavedObjectsRepository, encryptedSavedObjectsClient: this.encryptedSavedObjectsClient, auditLogger: securityPluginSetup?.audit.asScoped(request), + getAlertIndicesAlias: this.getAlertIndicesAlias, + alertsService: this.alertsService, connectorAdapterRegistry: this.connectorAdapterRegistry, async getUserName() { if (!securityPluginStart) { From d886bfc956e92f8fa4f13d5c3a225c7d97b3ee7b Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Tue, 24 Oct 2023 18:15:35 +0200 Subject: [PATCH 13/20] fix tests after changes --- .../rule/methods/create/create_rule.test.ts | 20 ------ ...form_domain_actions_to_raw_actions.test.ts | 35 +---------- .../lib/validate_system_actions.test.ts | 63 ++++++++++++++++--- 3 files changed, 59 insertions(+), 59 deletions(-) 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 d0985ebe682cb..4868f21f5485c 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 @@ -4004,26 +4004,6 @@ describe('create()', () => { `); }); - 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', 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 index a167f2ba3a2a7..87b0afcbb211d 100644 --- 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 @@ -5,15 +5,14 @@ * 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', + actionRef: 'action_0', params: {}, frequency: { summary: false, @@ -25,45 +24,17 @@ const defaultAction = { }; const systemAction = { - id: 'my-system-action-id', uuid: '123', actionTypeId: '.test-system-action', + actionRef: 'system_action:my-system-action-id', 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({ + const res = transformDomainActionsToRawActions({ actions: [defaultAction, systemAction], - // @ts-ignore: no need to pass all properties of the context - context, }); expect(res).toMatchInlineSnapshot(` 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 index de3a45d5c09b9..653246f174989 100644 --- a/x-pack/plugins/alerting/server/lib/validate_system_actions.test.ts +++ b/x-pack/plugins/alerting/server/lib/validate_system_actions.test.ts @@ -12,9 +12,12 @@ import { ConnectorAdapterRegistry } from '../connector_adapters/connector_adapte import { ConnectorAdapter } from '../connector_adapters/types'; import { NormalizedSystemAction } from '../rules_client'; import { RuleActionTypes, RuleSystemAction } from '../types'; -import { validateSystemActions } from './validate_system_actions'; +import { + validateSystemActionsWithoutRuleTypeId, + validateSystemActionsWithRuleTypeId, +} from './validate_system_actions'; -describe('validateSystemActions', () => { +describe('validateSystemActionsWithoutRuleTypeId', () => { const connectorAdapter: ConnectorAdapter = { connectorTypeId: '.test', ruleActionParamsSchema: schema.object({ foo: schema.string() }), @@ -42,7 +45,7 @@ describe('validateSystemActions', () => { }); it('should not validate with empty system actions', async () => { - const res = await validateSystemActions({ + const res = await validateSystemActionsWithoutRuleTypeId({ connectorAdapterRegistry: registry, systemActions: [], actionsClient, @@ -69,7 +72,7 @@ describe('validateSystemActions', () => { actionsClient.isSystemAction.mockReturnValue(false); await expect(() => - validateSystemActions({ + validateSystemActionsWithoutRuleTypeId({ connectorAdapterRegistry: registry, systemActions, actionsClient, @@ -93,7 +96,7 @@ describe('validateSystemActions', () => { actionsClient.isSystemAction.mockReturnValue(true); await expect(() => - validateSystemActions({ + validateSystemActionsWithoutRuleTypeId({ connectorAdapterRegistry: registry, systemActions, actionsClient, @@ -117,7 +120,7 @@ describe('validateSystemActions', () => { actionsClient.isSystemAction.mockReturnValue(true); await expect(() => - validateSystemActions({ + validateSystemActionsWithoutRuleTypeId({ connectorAdapterRegistry: registry, systemActions, actionsClient, @@ -171,7 +174,7 @@ describe('validateSystemActions', () => { actionsClient.isSystemAction.mockReturnValue(true); - const res = await validateSystemActions({ + const res = await validateSystemActionsWithoutRuleTypeId({ connectorAdapterRegistry: registry, systemActions, actionsClient, @@ -185,3 +188,49 @@ describe('validateSystemActions', () => { }); }); }); + +describe('validateSystemActionsWithRuleTypeId', () => { + const connectorAdapter: ConnectorAdapter = { + connectorTypeId: '.test', + ruleActionParamsSchema: schema.object({ foo: schema.string() }), + buildActionParams: jest.fn(), + }; + + let registry: ConnectorAdapterRegistry; + + beforeEach(() => { + registry = new ConnectorAdapterRegistry(); + }); + + it('should not validate with empty system actions', async () => { + const res = validateSystemActionsWithRuleTypeId({ + connectorAdapterRegistry: registry, + systemActions: [], + }); + + expect(res).toBe(undefined); + }); + + 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); + + expect(() => + validateSystemActionsWithRuleTypeId({ + connectorAdapterRegistry: registry, + systemActions, + }) + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid system action params. System action type: .test - [foo]: expected value of type [string] but got [undefined]"` + ); + }); +}); From 4a3867e49cadd1d22be8082b5dafc135d4067d16 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 24 Oct 2023 17:02:09 +0000 Subject: [PATCH 14/20] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- .../server/application/rule/methods/create/create_rule.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 4868f21f5485c..c9aefc25e154f 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,7 +24,7 @@ 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, RuleActionTypes, RuleSystemAction } from '../../../../../common'; +import { RecoveredActionGroup, RuleActionTypes } 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'; From cb5a00b42e014d03ec3fbc4659e7a8ccc945492a Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Wed, 25 Oct 2023 09:30:53 +0200 Subject: [PATCH 15/20] fix actions client mock for create rule route tests --- x-pack/plugins/actions/server/mocks.ts | 4 +++- x-pack/plugins/alerting/common/index.ts | 2 -- .../alerting/server/routes/_mock_handler_arguments.ts | 9 +++++++++ .../routes/rule/apis/create/create_rule_route.test.ts | 4 +--- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/actions/server/mocks.ts b/x-pack/plugins/actions/server/mocks.ts index 72af76ef55b9c..a0defcf9d8621 100644 --- a/x-pack/plugins/actions/server/mocks.ts +++ b/x-pack/plugins/actions/server/mocks.ts @@ -12,7 +12,7 @@ import { } from '@kbn/core/server/mocks'; import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks'; import { Logger } from '@kbn/core/server'; -import { actionsClientMock } from './actions_client/actions_client.mock'; +import { actionsClientMock, ActionsClientMock } from './actions_client/actions_client.mock'; import { PluginSetupContract, PluginStartContract, renderActionParameterTemplates } from './plugin'; import { Services } from './types'; import { actionsAuthorizationMock } from './authorization/actions_authorization.mock'; @@ -20,6 +20,8 @@ import { ConnectorTokenClient } from './lib/connector_token_client'; import { unsecuredActionsClientMock } from './unsecured_actions_client/unsecured_actions_client.mock'; export { actionsAuthorizationMock }; export { actionsClientMock }; +export type { ActionsClientMock }; + const logger = loggingSystemMock.create().get() as jest.Mocked; const createSetupMock = () => { diff --git a/x-pack/plugins/alerting/common/index.ts b/x-pack/plugins/alerting/common/index.ts index 8b0aee7f12d60..ea85bf8983a38 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/server/routes/_mock_handler_arguments.ts b/x-pack/plugins/alerting/server/routes/_mock_handler_arguments.ts index 8673614ed2ec0..35f3350dc417f 100644 --- a/x-pack/plugins/alerting/server/routes/_mock_handler_arguments.ts +++ b/x-pack/plugins/alerting/server/routes/_mock_handler_arguments.ts @@ -9,6 +9,8 @@ import { KibanaRequest, KibanaResponseFactory } from '@kbn/core/server'; import { identity } from 'lodash'; import type { MethodKeysOf } from '@kbn/utility-types'; import { httpServerMock } from '@kbn/core/server/mocks'; +import { actionsClientMock } from '@kbn/actions-plugin/server/mocks'; +import type { ActionsClientMock } from '@kbn/actions-plugin/server/mocks'; import { rulesClientMock, RulesClientMock } from '../rules_client.mock'; import { rulesSettingsClientMock, RulesSettingsClientMock } from '../rules_settings_client.mock'; import { @@ -21,6 +23,7 @@ import type { AlertingRequestHandlerContext } from '../types'; export function mockHandlerArguments( { rulesClient = rulesClientMock.create(), + actionsClient = actionsClientMock.create(), rulesSettingsClient = rulesSettingsClientMock.create(), maintenanceWindowClient = maintenanceWindowClientMock.create(), listTypes: listTypesRes = [], @@ -28,6 +31,7 @@ export function mockHandlerArguments( areApiKeysEnabled, }: { rulesClient?: RulesClientMock; + actionsClient?: ActionsClientMock; rulesSettingsClient?: RulesSettingsClientMock; maintenanceWindowClient?: MaintenanceWindowClientMock; listTypes?: RuleType[]; @@ -59,6 +63,11 @@ export function mockHandlerArguments( getFrameworkHealth, areApiKeysEnabled: areApiKeysEnabled ? areApiKeysEnabled : () => Promise.resolve(true), }, + actions: { + getActionsClient() { + return actionsClient || actionsClientMock.create(); + }, + }, } as unknown as AlertingRequestHandlerContext, request as KibanaRequest, mockResponseFactory(response), 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 b84c53b9b5409..c26f9b61e7004 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 @@ -14,13 +14,11 @@ import { mockHandlerArguments } from '../../../_mock_handler_arguments'; import type { CreateRuleRequestBodyV1 } from '../../../../../common/routes/rule/apis/create'; import { rulesClientMock } from '../../../../rules_client.mock'; import { RuleTypeDisabledError } from '../../../../lib'; -import { AsApiContract } from '../../../lib'; 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'; @@ -120,7 +118,7 @@ describe('createRuleRoute', () => { ], }; - const createResult: AsApiContract> = { + const createResult = { ...ruleToCreate, mute_all: mockedAlert.muteAll, created_by: mockedAlert.createdBy, From 4a89ae2912f53b6defe160e3723556df1fcb5d5a Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Wed, 25 Oct 2023 09:44:12 +0200 Subject: [PATCH 16/20] fix types after merging --- x-pack/plugins/actions/server/actions_client/actions_client.ts | 2 +- .../server/application/connector/methods/get_all/get_all.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/actions/server/actions_client/actions_client.ts b/x-pack/plugins/actions/server/actions_client/actions_client.ts index b94b18c00ac51..6036d95ea4f02 100644 --- a/x-pack/plugins/actions/server/actions_client/actions_client.ts +++ b/x-pack/plugins/actions/server/actions_client/actions_client.ts @@ -422,7 +422,7 @@ export class ActionsClient { /** * Get all system connectors */ - public async getAllSystemConnectors(): Promise { + public async getAllSystemConnectors(): Promise { return getAllSystemConnectors({ context: this.context }); } diff --git a/x-pack/plugins/actions/server/application/connector/methods/get_all/get_all.ts b/x-pack/plugins/actions/server/application/connector/methods/get_all/get_all.ts index 68b8180e4baaf..e1593351a574a 100644 --- a/x-pack/plugins/actions/server/application/connector/methods/get_all/get_all.ts +++ b/x-pack/plugins/actions/server/application/connector/methods/get_all/get_all.ts @@ -85,7 +85,7 @@ export async function getAllSystemConnectors({ context, }: { context: GetAllParams['context']; -}): Promise { +}): Promise { try { await context.authorization.ensureAuthorized({ operation: 'get' }); } catch (error) { From eac241101460edfb7e0f8163f65e6a48c85f27d0 Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Wed, 25 Oct 2023 12:24:02 +0200 Subject: [PATCH 17/20] fix integrational tests --- .../spaces_only/tests/alerting/group1/create.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/create.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/create.ts index cd3e998fd6336..9ba3232abfefb 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/create.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/create.ts @@ -7,7 +7,7 @@ import expect from '@kbn/expect'; import { SavedObject } from '@kbn/core/server'; -import { RawRule, RuleActionTypes } from '@kbn/alerting-plugin/server/types'; +import { RawRule } from '@kbn/alerting-plugin/server/types'; import { ALERTING_CASES_SAVED_OBJECT_INDEX } from '@kbn/core-saved-objects-server'; import { omit } from 'lodash'; import { Spaces } from '../../../scenarios'; @@ -16,7 +16,7 @@ import { getUrlPrefix, getTestRuleData, ObjectRemover, - getConsumerUnauthorizedErrorMessage, + getUnauthorizedErrorMessage, TaskManagerDoc, } from '../../../../common/lib'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; @@ -163,7 +163,6 @@ export default function createAlertTests({ getService }: FtrProviderContext) { actionTypeId: 'test.system-action', uuid: '123', params: {}, - type: RuleActionTypes.SYSTEM, }, ], }) @@ -199,7 +198,6 @@ export default function createAlertTests({ getService }: FtrProviderContext) { connector_type_id: 'test.system-action', params: {}, uuid: '123', - type: RuleActionTypes.SYSTEM, }, ], enabled: true, @@ -260,7 +258,6 @@ export default function createAlertTests({ getService }: FtrProviderContext) { actionTypeId: 'test.system-action', params: {}, uuid: '123', - type: RuleActionTypes.SYSTEM, }, ]); @@ -447,7 +444,7 @@ export default function createAlertTests({ getService }: FtrProviderContext) { expect(response.status).to.eql(403); expect(response.body).to.eql({ error: 'Forbidden', - message: getConsumerUnauthorizedErrorMessage( + message: getUnauthorizedErrorMessage( 'create', 'test.noop', 'some consumer patrick invented' @@ -473,7 +470,6 @@ export default function createAlertTests({ getService }: FtrProviderContext) { actionTypeId: 'test.system-action', uuid: '123', params: {}, - type: RuleActionTypes.SYSTEM, }; it('should create a rule with a system action correctly', async () => { @@ -496,7 +492,6 @@ export default function createAlertTests({ getService }: FtrProviderContext) { connector_type_id: 'test.system-action', params: {}, uuid: '123', - type: RuleActionTypes.SYSTEM, }, ]); @@ -517,7 +512,6 @@ export default function createAlertTests({ getService }: FtrProviderContext) { actionTypeId: 'test.system-action', params: {}, uuid: '123', - type: RuleActionTypes.SYSTEM, }, ]); From cce69c0ee68b669e0b18c497e7cfc7be56c3f63f Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Wed, 25 Oct 2023 12:25:53 +0200 Subject: [PATCH 18/20] Revert "devide system actions validation into 2 functions" This reverts commit 35153cace0d2ae4923fa944bd40a8c1dded7fe9a. --- .../rule/methods/create/create_rule.ts | 5 ++-- .../server/lib/validate_system_actions.ts | 27 +++---------------- 2 files changed, 7 insertions(+), 25 deletions(-) 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 2dd1ad56d77ac..76b63ec2e3036 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,7 +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 { validateSystemActionsWithRuleTypeId } from '../../../../lib/validate_system_actions'; +import { validateSystemActions } from '../../../../lib/validate_system_actions'; import { parseDuration } from '../../../../../common/parse_duration'; import { WriteOperations, AlertingAuthorizationEntity } from '../../../../authorization'; import { @@ -80,7 +80,8 @@ export async function createRule( throw Boom.badRequest(`Error validating create data - ${error.message}`); } - validateSystemActionsWithRuleTypeId({ + await validateSystemActions({ + actionsClient, connectorAdapterRegistry: context.connectorAdapterRegistry, systemActions, }); diff --git a/x-pack/plugins/alerting/server/lib/validate_system_actions.ts b/x-pack/plugins/alerting/server/lib/validate_system_actions.ts index 8c4dafb1f4593..ebe9426297ca2 100644 --- a/x-pack/plugins/alerting/server/lib/validate_system_actions.ts +++ b/x-pack/plugins/alerting/server/lib/validate_system_actions.ts @@ -11,43 +11,24 @@ import { ConnectorAdapterRegistry } from '../connector_adapters/connector_adapte import { bulkValidateConnectorAdapterActionParams } from '../connector_adapters/validate_rule_action_params'; import { NormalizedSystemAction } from '../rules_client'; import { RuleSystemAction } from '../types'; -interface ValidateSystemActionsWithRuleTypeIdParams { - connectorAdapterRegistry: ConnectorAdapterRegistry; - systemActions: RuleSystemAction[]; -} - -interface ValidateSystemActionsWithoutRuleTypeIdParams { +interface Params { actionsClient: ActionsClient; connectorAdapterRegistry: ConnectorAdapterRegistry; systemActions: Array; } -export const validateSystemActionsWithRuleTypeId = ({ - connectorAdapterRegistry, - systemActions, -}: ValidateSystemActionsWithRuleTypeIdParams) => { - if (systemActions.length === 0) { - return; - } - - bulkValidateConnectorAdapterActionParams({ - connectorAdapterRegistry, - actions: systemActions, - }); -}; - -export const validateSystemActionsWithoutRuleTypeId = async ({ +export const validateSystemActions = async ({ actionsClient, connectorAdapterRegistry, systemActions, -}: ValidateSystemActionsWithoutRuleTypeIdParams) => { +}: Params) => { if (systemActions.length === 0) { return; } /** * When updating or creating a rule the actions may not contain - * the actionTypeId (for example: bulk operations). We need to getBulk using the + * 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 From 83f71675baecae218d7b6712d73d13eae0f2fbc8 Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Wed, 25 Oct 2023 12:30:37 +0200 Subject: [PATCH 19/20] fix tests after validate system actions revert --- .../lib/validate_system_actions.test.ts | 61 ++----------------- 1 file changed, 6 insertions(+), 55 deletions(-) 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 index 653246f174989..5c57484888fd5 100644 --- a/x-pack/plugins/alerting/server/lib/validate_system_actions.test.ts +++ b/x-pack/plugins/alerting/server/lib/validate_system_actions.test.ts @@ -12,10 +12,7 @@ import { ConnectorAdapterRegistry } from '../connector_adapters/connector_adapte import { ConnectorAdapter } from '../connector_adapters/types'; import { NormalizedSystemAction } from '../rules_client'; import { RuleActionTypes, RuleSystemAction } from '../types'; -import { - validateSystemActionsWithoutRuleTypeId, - validateSystemActionsWithRuleTypeId, -} from './validate_system_actions'; +import { validateSystemActions } from './validate_system_actions'; describe('validateSystemActionsWithoutRuleTypeId', () => { const connectorAdapter: ConnectorAdapter = { @@ -45,7 +42,7 @@ describe('validateSystemActionsWithoutRuleTypeId', () => { }); it('should not validate with empty system actions', async () => { - const res = await validateSystemActionsWithoutRuleTypeId({ + const res = await validateSystemActions({ connectorAdapterRegistry: registry, systemActions: [], actionsClient, @@ -72,7 +69,7 @@ describe('validateSystemActionsWithoutRuleTypeId', () => { actionsClient.isSystemAction.mockReturnValue(false); await expect(() => - validateSystemActionsWithoutRuleTypeId({ + validateSystemActions({ connectorAdapterRegistry: registry, systemActions, actionsClient, @@ -96,7 +93,7 @@ describe('validateSystemActionsWithoutRuleTypeId', () => { actionsClient.isSystemAction.mockReturnValue(true); await expect(() => - validateSystemActionsWithoutRuleTypeId({ + validateSystemActions({ connectorAdapterRegistry: registry, systemActions, actionsClient, @@ -120,7 +117,7 @@ describe('validateSystemActionsWithoutRuleTypeId', () => { actionsClient.isSystemAction.mockReturnValue(true); await expect(() => - validateSystemActionsWithoutRuleTypeId({ + validateSystemActions({ connectorAdapterRegistry: registry, systemActions, actionsClient, @@ -174,7 +171,7 @@ describe('validateSystemActionsWithoutRuleTypeId', () => { actionsClient.isSystemAction.mockReturnValue(true); - const res = await validateSystemActionsWithoutRuleTypeId({ + const res = await validateSystemActions({ connectorAdapterRegistry: registry, systemActions, actionsClient, @@ -188,49 +185,3 @@ describe('validateSystemActionsWithoutRuleTypeId', () => { }); }); }); - -describe('validateSystemActionsWithRuleTypeId', () => { - const connectorAdapter: ConnectorAdapter = { - connectorTypeId: '.test', - ruleActionParamsSchema: schema.object({ foo: schema.string() }), - buildActionParams: jest.fn(), - }; - - let registry: ConnectorAdapterRegistry; - - beforeEach(() => { - registry = new ConnectorAdapterRegistry(); - }); - - it('should not validate with empty system actions', async () => { - const res = validateSystemActionsWithRuleTypeId({ - connectorAdapterRegistry: registry, - systemActions: [], - }); - - expect(res).toBe(undefined); - }); - - 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); - - expect(() => - validateSystemActionsWithRuleTypeId({ - connectorAdapterRegistry: registry, - systemActions, - }) - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid system action params. System action type: .test - [foo]: expected value of type [string] but got [undefined]"` - ); - }); -}); From a278cccdc3f3b7b1bceb6d9f4b1161288e8d1157 Mon Sep 17 00:00:00 2001 From: Julia Guskova Date: Wed, 25 Oct 2023 15:56:06 +0200 Subject: [PATCH 20/20] fix integrational tests --- .../rule/methods/create/create_rule.test.ts | 21 +++++++++++++ .../tests/alerting/group1/create.ts | 30 +------------------ 2 files changed, 22 insertions(+), 29 deletions(-) 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 c9aefc25e154f..2767c3794026d 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 @@ -30,6 +30,7 @@ import { getRuleExecutionStatusPending, getDefaultMonitoring } from '../../../.. import { ConnectorAdapterRegistry } from '../../../../connector_adapters/connector_adapter_registry'; import { ConnectorAdapter } from '../../../../connector_adapters/types'; import { RuleDomain } from '../../types'; +import { RuleSystemAction } from '../../../../types'; jest.mock('../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation', () => ({ bulkMarkApiKeysForInvalidation: jest.fn(), @@ -4004,6 +4005,26 @@ describe('create()', () => { `); }); + 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', diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/create.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/create.ts index 9ba3232abfefb..aa526c98d2d89 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/create.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/create.ts @@ -521,7 +521,7 @@ export default function createAlertTests({ getService }: FtrProviderContext) { }); it('should throw 400 if the system action is missing required properties', async () => { - for (const propertyToOmit of ['id', 'actionTypeId', 'uuid']) { + for (const propertyToOmit of ['id']) { const systemActionWithoutProperty = omit(systemAction, propertyToOmit); await supertest @@ -536,34 +536,6 @@ export default function createAlertTests({ getService }: FtrProviderContext) { } }); - it('should throw 400 if the system action contain properties from the default actions', async () => { - 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: [] }, - }, - }, - ]) { - await supertest - .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) - .set('kbn-xsrf', 'foo') - .send( - getTestRuleData({ - actions: [{ ...systemAction, ...propertyAdd }], - }) - ) - .expect(400); - } - }); - it('should throw 400 if the system action is missing required params', async () => { const res = await supertest .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`)