Changed AlertsClient to use ActionsClient instead of direct interaction with the action saved objects (#67562)

x-pack/plugins/actions/server/actions_client.mock.ts

@@ -16,6 +16,7 @@ const createActionsClientMock = () => {
     delete: jest.fn(),
     update: jest.fn(),
     getAll: jest.fn(),
+    getBulk: jest.fn(),
   };
   return mocked;
 };

x-pack/plugins/actions/server/actions_client.test.ts

@@ -423,6 +423,74 @@ describe('getAll()', () => {
   });
 });
 
+describe('getBulk()', () => {
+  test('calls getBulk savedObjectsClient with parameters', async () => {
+    savedObjectsClient.bulkGet.mockResolvedValueOnce({
+      saved_objects: [
+        {
+          id: '1',
+          type: 'action',
+          attributes: {
+            actionTypeId: 'test',
+            name: 'test',
+            config: {
+              foo: 'bar',
+            },
+          },
+          references: [],
+        },
+      ],
+    });
+    scopedClusterClient.callAsInternalUser.mockResolvedValueOnce({
+      aggregations: {
+        '1': { doc_count: 6 },
+        testPreconfigured: { doc_count: 2 },
+      },
+    });
+
+    actionsClient = new ActionsClient({
+      actionTypeRegistry,
+      savedObjectsClient,
+      scopedClusterClient,
+      defaultKibanaIndex,
+      preconfiguredActions: [
+        {
+          id: 'testPreconfigured',
+          actionTypeId: '.slack',
+          secrets: {},
+          isPreconfigured: true,
+          name: 'test',
+          config: {
+            foo: 'bar',
+          },
+        },
+      ],
+    });
+    const result = await actionsClient.getBulk(['1', 'testPreconfigured']);
+    expect(result).toEqual([
+      {
+        actionTypeId: '.slack',
+        config: {
+          foo: 'bar',
+        },
+        id: 'testPreconfigured',
+        isPreconfigured: true,
+        name: 'test',
+        secrets: {},
+      },
+      {
+        actionTypeId: 'test',
+        config: {
+          foo: 'bar',
+        },
+        id: '1',
+        isPreconfigured: false,
+        name: 'test',
+      },
+    ]);
+  });
+});
+
 describe('delete()', () => {
   test('calls savedObjectsClient with id', async () => {
     const expectedResult = Symbol();

x-pack/plugins/actions/server/actions_client.ts

@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-
+import Boom from 'boom';
 import {
   IScopedClusterClient,
   SavedObjectsClientContract,
@@ -193,6 +193,44 @@ export class ActionsClient {
     );
   }
 
+  /**
+   * Get bulk actions with preconfigured list
+   */
+  public async getBulk(ids: string[]): Promise<ActionResult[]> {
+    const actionResults = new Array<ActionResult>();
+    for (const actionId of ids) {
+      const action = this.preconfiguredActions.find(
+        (preconfiguredAction) => preconfiguredAction.id === actionId
+      );
+      if (action !== undefined) {
+        actionResults.push(action);
+      }
+    }
+
+    // Fetch action objects in bulk
+    // Excluding preconfigured actions to avoid an not found error, which is already added
+    const actionSavedObjectsIds = [
+      ...new Set(
+        ids.filter(
+          (actionId) => !actionResults.find((actionResult) => actionResult.id === actionId)
+        )
+      ),
+    ];
+
+    const bulkGetOpts = actionSavedObjectsIds.map((id) => ({ id, type: 'action' }));
+    const bulkGetResult = await this.savedObjectsClient.bulkGet<RawAction>(bulkGetOpts);
+
+    for (const action of bulkGetResult.saved_objects) {
+      if (action.error) {
+        throw Boom.badRequest(
+          `Failed to load action ${action.id} (${action.error.statusCode}): ${action.error.message}`
+        );
+      }
+      actionResults.push(actionFromSavedObject(action));
+    }
+    return actionResults;
+  }
+
   /**
    * Delete action
    */

x-pack/plugins/alerting/server/alerts_client.test.ts

@@ -13,6 +13,7 @@ import { TaskStatus } from '../../../plugins/task_manager/server';
 import { IntervalSchedule } from './types';
 import { resolvable } from './test_utils';
 import { encryptedSavedObjectsMock } from '../../../plugins/encrypted_saved_objects/server/mocks';
+import { actionsClientMock } from '../../actions/server/mocks';
 
 const taskManager = taskManagerMock.start();
 const alertTypeRegistry = alertTypeRegistryMock.create();
@@ -30,7 +31,7 @@ const alertsClientParams = {
   invalidateAPIKey: jest.fn(),
   logger: loggingServiceMock.create().get(),
   encryptedSavedObjectsClient: encryptedSavedObjects,
-  preconfiguredActions: [],
+  getActionsClient: jest.fn(),
 };
 
 beforeEach(() => {
@@ -42,6 +43,34 @@ beforeEach(() => {
   });
   alertsClientParams.getUserName.mockResolvedValue('elastic');
   taskManager.runNow.mockResolvedValue({ id: '' });
+  const actionsClient = actionsClientMock.create();
+  actionsClient.getBulk.mockResolvedValueOnce([
+    {
+      id: '1',
+      isPreconfigured: false,
+      actionTypeId: 'test',
+      name: 'test',
+      config: {
+        foo: 'bar',
+      },
+    },
+    {
+      id: '2',
+      isPreconfigured: false,
+      actionTypeId: 'test2',
+      name: 'test2',
+      config: {
+        foo: 'bar',
+      },
+    },
+    {
+      id: 'testPreconfigured',
+      actionTypeId: '.slack',
+      isPreconfigured: true,
+      name: 'test',
+    },
+  ]);
+  alertsClientParams.getActionsClient.mockResolvedValue(actionsClient);
 });
 
 const mockedDate = new Date('2019-02-12T21:01:22.479Z');
@@ -97,18 +126,6 @@ describe('create()', () => {
 
   test('creates an alert', async () => {
     const data = getMockData();
-    savedObjectsClient.bulkGet.mockResolvedValueOnce({
-      saved_objects: [
-        {
-          id: '1',
-          type: 'action',
-          attributes: {
-            actionTypeId: 'test',
-          },
-          references: [],
-        },
-      ],
-    });
     savedObjectsClient.create.mockResolvedValueOnce({
       id: '1',
       type: 'alert',
@@ -297,26 +314,6 @@ describe('create()', () => {
         },
       ],
     });
-    savedObjectsClient.bulkGet.mockResolvedValueOnce({
-      saved_objects: [
-        {
-          id: '1',
-          type: 'action',
-          attributes: {
-            actionTypeId: 'test',
-          },
-          references: [],
-        },
-        {
-          id: '2',
-          type: 'action',
-          attributes: {
-            actionTypeId: 'test2',
-          },
-          references: [],
-        },
-      ],
-    });
     savedObjectsClient.create.mockResolvedValueOnce({
       id: '1',
       type: 'alert',
@@ -435,16 +432,6 @@ describe('create()', () => {
         "updatedAt": 2019-02-12T21:01:22.479Z,
       }
     `);
-    expect(savedObjectsClient.bulkGet).toHaveBeenCalledWith([
-      {
-        id: '1',
-        type: 'action',
-      },
-      {
-        id: '2',
-        type: 'action',
-      },
-    ]);
   });
 
   test('creates a disabled alert', async () => {
@@ -549,7 +536,9 @@ describe('create()', () => {
 
   test('throws error if loading actions fails', async () => {
     const data = getMockData();
-    savedObjectsClient.bulkGet.mockRejectedValueOnce(new Error('Test Error'));
+    const actionsClient = actionsClientMock.create();
+    actionsClient.getBulk.mockRejectedValueOnce(new Error('Test Error'));
+    alertsClientParams.getActionsClient.mockResolvedValue(actionsClient);
     await expect(alertsClient.create({ data })).rejects.toThrowErrorMatchingInlineSnapshot(
       `"Test Error"`
     );
@@ -1903,26 +1892,6 @@ describe('update()', () => {
   });
 
   test('updates given parameters', async () => {
-    savedObjectsClient.bulkGet.mockResolvedValueOnce({
-      saved_objects: [
-        {
-          id: '1',
-          type: 'action',
-          attributes: {
-            actionTypeId: 'test',
-          },
-          references: [],
-        },
-        {
-          id: '2',
-          type: 'action',
-          attributes: {
-            actionTypeId: 'test2',
-          },
-          references: [],
-        },
-      ],
-    });
     savedObjectsClient.update.mockResolvedValueOnce({
       id: '1',
       type: 'alert',

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

@@ -13,7 +13,7 @@ import {
   SavedObjectReference,
   SavedObject,
 } from 'src/core/server';
-import { PreConfiguredAction } from '../../actions/server';
+import { ActionsClient } from '../../actions/server';
 import {
   Alert,
   PartialAlert,
@@ -24,7 +24,6 @@ import {
   IntervalSchedule,
   SanitizedAlert,
   AlertTaskState,
-  RawAlertAction,
 } from './types';
 import { validateAlertTypeParams } from './lib';
 import {
@@ -56,7 +55,7 @@ interface ConstructorOptions {
   getUserName: () => Promise<string | null>;
   createAPIKey: () => Promise<CreateAPIKeyResult>;
   invalidateAPIKey: (params: InvalidateAPIKeyParams) => Promise<InvalidateAPIKeyResult>;
-  preconfiguredActions: PreConfiguredAction[];
+  getActionsClient: () => Promise<ActionsClient>;
 }
 
 export interface FindOptions {
@@ -127,7 +126,7 @@ export class AlertsClient {
   private readonly invalidateAPIKey: (
     params: InvalidateAPIKeyParams
   ) => Promise<InvalidateAPIKeyResult>;
-  private preconfiguredActions: PreConfiguredAction[];
+  private readonly getActionsClient: () => Promise<ActionsClient>;
   encryptedSavedObjectsClient: EncryptedSavedObjectsClient;
 
   constructor({
@@ -141,7 +140,7 @@ export class AlertsClient {
     createAPIKey,
     invalidateAPIKey,
     encryptedSavedObjectsClient,
-    preconfiguredActions,
+    getActionsClient,
   }: ConstructorOptions) {
     this.logger = logger;
     this.getUserName = getUserName;
@@ -153,7 +152,7 @@ export class AlertsClient {
     this.createAPIKey = createAPIKey;
     this.invalidateAPIKey = invalidateAPIKey;
     this.encryptedSavedObjectsClient = encryptedSavedObjectsClient;
-    this.preconfiguredActions = preconfiguredActions;
+    this.getActionsClient = getActionsClient;
   }
 
   public async create({ data, options }: CreateOptions): Promise<Alert> {
@@ -600,7 +599,7 @@ export class AlertsClient {
     actions: RawAlert['actions'],
     references: SavedObjectReference[]
   ) {
-    return actions.map((action, i) => {
+    return actions.map((action) => {
       const reference = references.find((ref) => ref.name === action.actionRef);
       if (!reference) {
         throw new Error(`Reference ${action.actionRef} not found`);
@@ -666,58 +665,31 @@ export class AlertsClient {
   private async denormalizeActions(
     alertActions: NormalizedAlertAction[]
   ): Promise<{ actions: RawAlert['actions']; references: SavedObjectReference[] }> {
-    const actionMap = new Map<string, unknown>();
-    // map preconfigured actions
-    for (const alertAction of alertActions) {
-      const action = this.preconfiguredActions.find(
-        (preconfiguredAction) => preconfiguredAction.id === alertAction.id
-      );
-      if (action !== undefined) {
-        actionMap.set(action.id, action);
-      }
-    }
-    // Fetch action objects in bulk
-    // Excluding preconfigured actions to avoid an not found error, which is already mapped
-    const actionIds = [
-      ...new Set(
-        alertActions
-          .filter((alertAction) => !actionMap.has(alertAction.id))
-          .map((alertAction) => alertAction.id)
-      ),
-    ];
-    if (actionIds.length > 0) {
-      const bulkGetOpts = actionIds.map((id) => ({ id, type: 'action' }));
-      const bulkGetResult = await this.savedObjectsClient.bulkGet(bulkGetOpts);
-
-      for (const action of bulkGetResult.saved_objects) {
-        if (action.error) {
-          throw Boom.badRequest(
-            `Failed to load action ${action.id} (${action.error.statusCode}): ${action.error.message}`
-          );
-        }
-        actionMap.set(action.id, action);
-      }
-    }
-    // Extract references and set actionTypeId
+    const actionsClient = await this.getActionsClient();
+    const actionIds = [...new Set(alertActions.map((alertAction) => alertAction.id))];
+    const actionResults = await actionsClient.getBulk(actionIds);
     const references: SavedObjectReference[] = [];
     const actions = alertActions.map(({ id, ...alertAction }, i) => {
-      const actionRef = `action_${i}`;
-      references.push({
-        id,
-        name: actionRef,
-        type: 'action',
-      });
-      const actionMapValue = actionMap.get(id);
-      // if action is a save object, than actionTypeId should be under attributes property
-      // if action is a preconfigured, than actionTypeId is the action property
-      const actionTypeId = actionIds.find((actionId) => actionId === id)
-        ? (actionMapValue as SavedObject<Record<string, string>>).attributes.actionTypeId
-        : (actionMapValue as RawAlertAction).actionTypeId;
-      return {
-        ...alertAction,
-        actionRef,
-        actionTypeId,
-      };
+      const actionResultValue = actionResults.find((action) => action.id === id);
+      if (actionResultValue) {
+        const actionRef = `action_${i}`;
+        references.push({
+          id,
+          name: actionRef,
+          type: 'action',
+        });
+        return {
+          ...alertAction,
+          actionRef,
+          actionTypeId: actionResultValue.actionTypeId,
+        };
+      } else {
+        return {
+          ...alertAction,
+          actionRef: '',
+          actionTypeId: '',
+        };
+      }
     });
     return {
       actions,

x-pack/plugins/alerting/server/alerts_client_factory.test.ts

@@ -13,6 +13,7 @@ import { loggingServiceMock, savedObjectsClientMock } from '../../../../src/core
 import { encryptedSavedObjectsMock } from '../../../plugins/encrypted_saved_objects/server/mocks';
 import { AuthenticatedUser } from '../../../plugins/security/public';
 import { securityMock } from '../../../plugins/security/server/mocks';
+import { actionsMock } from '../../actions/server/mocks';
 
 jest.mock('./alerts_client');
 
@@ -25,7 +26,7 @@ const alertsClientFactoryParams: jest.Mocked<AlertsClientFactoryOpts> = {
   getSpaceId: jest.fn(),
   spaceIdToNamespace: jest.fn(),
   encryptedSavedObjectsClient: encryptedSavedObjectsMock.createClient(),
-  preconfiguredActions: [],
+  actions: actionsMock.createStart(),
 };
 const fakeRequest = ({
   headers: {},
@@ -65,7 +66,7 @@ test('creates an alerts client with proper constructor arguments', async () => {
     createAPIKey: expect.any(Function),
     invalidateAPIKey: expect.any(Function),
     encryptedSavedObjectsClient: alertsClientFactoryParams.encryptedSavedObjectsClient,
-    preconfiguredActions: [],
+    getActionsClient: expect.any(Function),
   });
 });
 
@@ -95,6 +96,16 @@ test('getUserName() returns a name when security is enabled', async () => {
   expect(userNameResult).toEqual('bob');
 });
 
+test('getActionsClient() returns ActionsClient', async () => {
+  const factory = new AlertsClientFactory();
+  factory.initialize(alertsClientFactoryParams);
+  factory.create(KibanaRequest.from(fakeRequest), savedObjectsClient);
+  const constructorCall = jest.requireMock('./alerts_client').AlertsClient.mock.calls[0][0];
+
+  const actionsClient = await constructorCall.getActionsClient();
+  expect(actionsClient).not.toBe(null);
+});
+
 test('createAPIKey() returns { apiKeysEnabled: false } when security is disabled', async () => {
   const factory = new AlertsClientFactory();
   factory.initialize(alertsClientFactoryParams);

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

@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { PreConfiguredAction } from '../../actions/server';
+import { PluginStartContract as ActionsPluginStartContract } from '../../actions/server';
 import { AlertsClient } from './alerts_client';
 import { AlertTypeRegistry, SpaceIdToNamespaceFunction } from './types';
 import { KibanaRequest, Logger, SavedObjectsClientContract } from '../../../../src/core/server';
@@ -20,7 +20,7 @@ export interface AlertsClientFactoryOpts {
   getSpaceId: (request: KibanaRequest) => string | undefined;
   spaceIdToNamespace: SpaceIdToNamespaceFunction;
   encryptedSavedObjectsClient: EncryptedSavedObjectsClient;
-  preconfiguredActions: PreConfiguredAction[];
+  actions: ActionsPluginStartContract;
 }
 
 export class AlertsClientFactory {
@@ -32,7 +32,7 @@ export class AlertsClientFactory {
   private getSpaceId!: (request: KibanaRequest) => string | undefined;
   private spaceIdToNamespace!: SpaceIdToNamespaceFunction;
   private encryptedSavedObjectsClient!: EncryptedSavedObjectsClient;
-  private preconfiguredActions!: PreConfiguredAction[];
+  private actions!: ActionsPluginStartContract;
 
   public initialize(options: AlertsClientFactoryOpts) {
     if (this.isInitialized) {
@@ -46,14 +46,14 @@ export class AlertsClientFactory {
     this.securityPluginSetup = options.securityPluginSetup;
     this.spaceIdToNamespace = options.spaceIdToNamespace;
     this.encryptedSavedObjectsClient = options.encryptedSavedObjectsClient;
-    this.preconfiguredActions = options.preconfiguredActions;
+    this.actions = options.actions;
   }
 
   public create(
     request: KibanaRequest,
     savedObjectsClient: SavedObjectsClientContract
   ): AlertsClient {
-    const { securityPluginSetup } = this;
+    const { securityPluginSetup, actions } = this;
     const spaceId = this.getSpaceId(request);
     return new AlertsClient({
       spaceId,
@@ -104,7 +104,9 @@ export class AlertsClientFactory {
           result: invalidateAPIKeyResult,
         };
       },
-      preconfiguredActions: this.preconfiguredActions,
+      async getActionsClient() {
+        return actions.getActionsClientWithRequest(request);
+      },
     });
   }
 }

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

@@ -216,7 +216,7 @@ export class AlertingPlugin {
       getSpaceId(request: KibanaRequest) {
         return spaces?.getSpaceId(request);
       },
-      preconfiguredActions: plugins.actions.preconfiguredActions,
+      actions: plugins.actions,
     });
 
     taskRunnerFactory.initialize({