From 6a523ccc808c3070a1a3400594cd834633c7ee1f Mon Sep 17 00:00:00 2001 From: Ying Date: Mon, 27 Feb 2023 11:12:09 -0500 Subject: [PATCH] cleanup --- .../group2/check_registered_types.test.ts | 2 +- .../create_unsecured_execute_function.ts | 6 ++-- .../unsecured_actions_client.test.ts | 30 +++++++++++++++---- .../unsecured_actions_client.ts | 26 ++++++++++++++-- .../services/connectors_email_service.test.ts | 28 ----------------- .../services/connectors_email_service.ts | 9 +----- .../group2/tests/alerting/alerts.ts | 9 +----- .../actions/schedule_unsecured_action.ts | 5 ---- .../tests/alerting/group1/event_log.ts | 8 +++++ 9 files changed, 63 insertions(+), 60 deletions(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index 57792759e1fab..d8303fe258b9b 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -56,7 +56,7 @@ describe('checking migration metadata changes on all registered SO types', () => expect(hashMap).toMatchInlineSnapshot(` Object { "action": "6cfc277ed3211639e37546ac625f4a68f2494215", - "action_task_params": "c2cb5c9060322e3e51c9e65debd8df48c0e54b8f", + "action_task_params": "7cfbd949cbb24f7ede1eeb7a62fb711bdeec501d", "alert": "2568bf6d8ba0876441c61c9e58e08016c1dc1617", "api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee", "apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2", diff --git a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts index c5870420178ff..c16b5e4d0b1a7 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -31,7 +31,7 @@ interface CreateBulkUnsecuredExecuteFunctionOptions { export interface ExecuteOptions extends Pick { id: string; - source: ActionExecutionSource; + source?: ActionExecutionSource; } interface ActionTaskParams @@ -115,8 +115,8 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ actionId: actionToExecute.id, params: actionToExecute.params, apiKey: null, - source: actionToExecute.source.type, relatedSavedObjects: relatedSavedObjectWithRefs, + ...(actionToExecute.source ? { source: actionToExecute.source.type } : {}), }, references: taskReferences, }; @@ -140,7 +140,7 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ }; } -function executionSourceAsSavedObjectReferences(executionSource: ActionExecutionSource) { +function executionSourceAsSavedObjectReferences(executionSource?: ActionExecutionSource) { return isSavedObjectExecutionSource(executionSource) ? { references: [ diff --git a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts index b07c80f8da3a0..7d0d4d687a71a 100644 --- a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts @@ -29,13 +29,11 @@ describe('bulkEnqueueExecution()', () => { id: 'preconfigured1', params: {}, executionId: '123abc', - source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, { id: 'preconfigured2', params: {}, executionId: '456def', - source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]; await expect( @@ -51,19 +49,41 @@ describe('bulkEnqueueExecution()', () => { id: 'preconfigured1', params: {}, executionId: '123abc', - source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, { id: 'preconfigured2', params: {}, executionId: '456def', - source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]; await expect( - unsecuredActionsClient.bulkEnqueueExecution('notifications', opts) + unsecuredActionsClient.bulkEnqueueExecution('functional_tester', opts) ).resolves.toMatchInlineSnapshot(`undefined`); expect(executionEnqueuer).toHaveBeenCalledWith(internalSavedObjectsRepository, opts); }); + + test('injects source and calls the executionEnqueuer with the appropriate parameters when requester is "notifications"', async () => { + const opts = [ + { + id: 'preconfigured1', + params: {}, + executionId: '123abc', + }, + { + id: 'preconfigured2', + params: {}, + executionId: '456def', + }, + ]; + await expect( + unsecuredActionsClient.bulkEnqueueExecution('notifications', opts) + ).resolves.toMatchInlineSnapshot(`undefined`); + + const optsWithSource = opts.map((opt) => ({ + ...opt, + source: asNotificationExecutionSource({ connectorId: opt.id, requesterId: 'notifications' }), + })); + expect(executionEnqueuer).toHaveBeenCalledWith(internalSavedObjectsRepository, optsWithSource); + }); }); diff --git a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts index 333490389013a..a2d87c8a5db4a 100644 --- a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts @@ -10,11 +10,14 @@ import { BulkUnsecuredExecutionEnqueuer, ExecuteOptions, } from '../create_unsecured_execute_function'; +import { asNotificationExecutionSource } from '../lib'; + +const NOTIFICATION_REQUESTER_ID = 'notifications'; // allowlist for features wanting access to the unsecured actions client // which allows actions to be enqueued for execution without a user request const ALLOWED_REQUESTER_IDS = [ - 'notifications', + NOTIFICATION_REQUESTER_ID, // For functional testing 'functional_tester', ]; @@ -47,6 +50,25 @@ export class UnsecuredActionsClient { `"${requesterId}" feature is not allow-listed for UnsecuredActionsClient access.` ); } - return this.executionEnqueuer(this.internalSavedObjectsRepository, actionsToExecute); + // Inject source based on requesterId + return this.executionEnqueuer( + this.internalSavedObjectsRepository, + this.injectSource(requesterId, actionsToExecute) + ); + } + + private injectSource(requesterId: string, actionsToExecute: ExecuteOptions[]): ExecuteOptions[] { + switch (requesterId) { + case NOTIFICATION_REQUESTER_ID: + return actionsToExecute.map((actionToExecute) => ({ + ...actionToExecute, + source: asNotificationExecutionSource({ + requesterId, + connectorId: actionToExecute.id, + }), + })); + default: + return actionsToExecute; + } } } diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts b/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts index c9fd5d8004411..f07b3a3ab34ae 100644 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts @@ -34,13 +34,6 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, - source: { - source: { - connectorId: CONNECTOR_ID, - requesterId: REQUESTER_ID, - }, - type: 'NOTIFICATION', - }, }, ]); }); @@ -74,13 +67,6 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, - source: { - source: { - connectorId: CONNECTOR_ID, - requesterId: REQUESTER_ID, - }, - type: 'NOTIFICATION', - }, relatedSavedObjects: [ { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', @@ -96,13 +82,6 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, - source: { - source: { - connectorId: CONNECTOR_ID, - requesterId: REQUESTER_ID, - }, - type: 'NOTIFICATION', - }, relatedSavedObjects: [ { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', @@ -118,13 +97,6 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, - source: { - source: { - connectorId: CONNECTOR_ID, - requesterId: REQUESTER_ID, - }, - type: 'NOTIFICATION', - }, relatedSavedObjects: [ { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service.ts b/x-pack/plugins/notifications/server/services/connectors_email_service.ts index 630ad8d22fa9c..55586dd05b078 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -5,10 +5,7 @@ * 2.0. */ -import { - asNotificationExecutionSource, - type IUnsecuredActionsClient, -} from '@kbn/actions-plugin/server'; +import type { IUnsecuredActionsClient } from '@kbn/actions-plugin/server'; import type { EmailService, PlainTextEmail } from './types'; export class ConnectorsEmailService implements EmailService { @@ -26,10 +23,6 @@ export class ConnectorsEmailService implements EmailService { subject: params.subject, message: params.message, }, - source: asNotificationExecutionSource({ - requesterId: this.requesterId, - connectorId: this.connectorId, - }), relatedSavedObjects: params.context?.relatedObjects, })); return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/alerts.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/alerts.ts index 03f7fbd387f1e..e97c0df7bf1df 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/alerts.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/alerts.ts @@ -12,7 +12,6 @@ import { IValidatedEvent, nanosToMillis } from '@kbn/event-log-plugin/server'; import { TaskRunning, TaskRunningStage } from '@kbn/task-manager-plugin/server/task_running'; import { ConcreteTaskInstance } from '@kbn/task-manager-plugin/server'; import { ESTestIndexTool, ES_TEST_INDEX_NAME } from '@kbn/alerting-api-integration-helpers'; -import { ActionExecutionSourceType } from '@kbn/actions-plugin/server/lib/action_execution_source'; import { UserAtSpaceScenarios, Superuser } from '../../../scenarios'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { @@ -231,7 +230,6 @@ instanceStateValue: true outcome: 'success', message: `rule executed: test.always-firing:${alertId}: 'abc'`, ruleObject: alertSearchResultWithoutDates, - source: ActionExecutionSourceType.SAVED_OBJECT, }); break; default: @@ -1367,11 +1365,10 @@ instanceStateValue: true message: string; errorMessage?: string; ruleObject: any; - source?: string; } async function validateEventLog(params: ValidateEventLogParams): Promise { - const { spaceId, alertId, outcome, message, errorMessage, ruleObject, source } = params; + const { spaceId, alertId, outcome, message, errorMessage, ruleObject } = params; const events: IValidatedEvent[] = await retry.try(async () => { return await getEventLog({ @@ -1458,10 +1455,6 @@ instanceStateValue: true expect(event?.message).to.eql(message); - if (source) { - expect(event?.kibana?.action?.execution?.source).to.eql(source); - } - if (errorMessage) { expect(event?.error?.message).to.eql(errorMessage); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts index ed84a6f589f1d..9a5719b7fa700 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts @@ -7,7 +7,6 @@ import expect from '@kbn/expect'; import type { SearchTotalHits } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; -import { ActionExecutionSourceType } from '@kbn/actions-plugin/server/lib/action_execution_source'; import { Spaces } from '../../scenarios'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; @@ -109,10 +108,6 @@ export default function createUnsecuredActionTests({ getService }: FtrProviderCo expect(hit?._source?.message).to.eql( `action executed: .email:my-test-email: TestEmail#xyz` ); - // @ts-expect-error _source: unknown - expect(hit?._source?.kibana?.actions?.execution?.source).to.eql( - ActionExecutionSourceType.NOTIFICATION - ); }); }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts index 2ee65b9edc3e7..39f9ca708e10f 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts @@ -9,6 +9,7 @@ import expect from '@kbn/expect'; import { IValidatedEvent, nanosToMillis } from '@kbn/event-log-plugin/server'; import { RuleNotifyWhen } from '@kbn/alerting-plugin/common'; import { ES_TEST_INDEX_NAME, ESTestIndexTool } from '@kbn/alerting-api-integration-helpers'; +import { ActionExecutionSourceType } from '@kbn/actions-plugin/server/lib/action_execution_source'; import { Spaces } from '../../../scenarios'; import { getUrlPrefix, @@ -319,6 +320,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) { ruleTypeId: response.body.rule_type_id, rule: undefined, consumer: 'alertsFixture', + source: ActionExecutionSourceType.SAVED_OBJECT, }); break; } @@ -997,6 +999,7 @@ interface ValidateEventLogParams { namespace?: string; }; flapping?: boolean; + source?: string; } export function validateEvent(event: IValidatedEvent, params: ValidateEventLogParams): void { @@ -1016,6 +1019,7 @@ export function validateEvent(event: IValidatedEvent, params: ValidateEventLogPa consumer, ruleTypeId, flapping, + source, } = params; const { status, actionGroupId, instanceId, reason, shouldHaveEventEnd } = params; @@ -1069,6 +1073,10 @@ export function validateEvent(event: IValidatedEvent, params: ValidateEventLogPa expect(event?.kibana?.alert?.flapping).to.be(flapping); } + if (source) { + expect(event?.kibana?.action?.execution?.source).to.be(source); + } + expect(event?.kibana?.alert?.rule?.rule_type_id).to.be(ruleTypeId); expect(event?.kibana?.space_ids?.[0]).to.equal(spaceId);