Shim alerting plugin for new platform (#46954) (#47538)

* Routes to register inside the init function * Start shimming plugins and core * More shimming * Move code over to plugin.ts * Use new platform basePath service * Cleanup * Create alerts client factory * Move some functions to plugin start * Remove some dependencies from Hapi * Apply PR feedback; use new platform logger, rename interfaces * Use new platform mocks for logger * Use new platform elasticsearch service * Fix type check failures * Fix failing jest tests * Remove logger from provided alert type service * Add unit tests for alerts client factory * Cleanup and fix type check errors * Change structure exposed via plugin * Use shim file, split plugin dependencies between setup and start contract, new task runner factory * Apply PR feedback * Fix ESLint errors * Set return type on plugin functions * Typo * Fix typecheck errors * IClusterClient
elastic · Oct 7, 2019 · 2130013 · 2130013
1 parent 5ee5a82
commit 2130013
Show file tree

Hide file tree

Showing 50 changed files with 1,080 additions and 769 deletions.
diff --git a/x-pack/legacy/plugins/alerting/README.md b/x-pack/legacy/plugins/alerting/README.md
@@ -37,7 +37,7 @@ When security is enabled, users who create alerts will need the `manage_api_key`
 
 ### Methods
 
-**server.plugins.alerting.registerType(options)**
+**server.plugins.alerting.setup.registerType(options)**
 
 The following table describes the properties of the `options` object.
 
@@ -71,7 +71,7 @@ This example receives server and threshold as parameters. It will read the CPU u
 ```
 import { schema } from '@kbn/config-schema';
 ...
-server.plugins.alerting.registerType({
+server.plugins.alerting.setup.registerType({
 	id: 'my-alert-type',
 	name: 'My alert type',
 	validate: {
@@ -129,7 +129,7 @@ server.plugins.alerting.registerType({
 This example only receives threshold as a parameter. It will read the CPU usage of all the servers and schedule individual actions if the reading for a server is greater than the threshold. This is a better implementation than above as only one query is performed for all the servers instead of one query per server.
 
 ```
-server.plugins.alerting.registerType({
+server.plugins.alerting.setup.registerType({
 	id: 'my-alert-type',
 	name: 'My alert type',
 	validate: {

diff --git a/x-pack/legacy/plugins/alerting/index.ts b/x-pack/legacy/plugins/alerting/index.ts
@@ -9,7 +9,14 @@ import { Root } from 'joi';
 import { init } from './server';
 import mappings from './mappings.json';
 
-export { AlertingPlugin, AlertsClient, AlertType, AlertExecutorOptions } from './server';
+export {
+  AlertingPlugin,
+  AlertsClient,
+  AlertType,
+  AlertExecutorOptions,
+  PluginSetupContract,
+  PluginStartContract,
+} from './server';
 
 export function alerting(kibana: any) {
   return new kibana.Plugin({

diff --git a/x-pack/legacy/plugins/alerting/server/alert_type_registry.test.ts b/x-pack/legacy/plugins/alerting/server/alert_type_registry.test.ts
@@ -4,31 +4,15 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-jest.mock('./lib/get_create_task_runner_function', () => ({
-  getCreateTaskRunnerFunction: jest.fn().mockReturnValue(jest.fn()),
-}));
-
+import { TaskRunnerFactory } from './lib';
 import { AlertTypeRegistry } from './alert_type_registry';
-import { SavedObjectsClientMock } from '../../../../../src/core/server/mocks';
 import { taskManagerMock } from '../../task_manager/task_manager.mock';
-import { encryptedSavedObjectsMock } from '../../encrypted_saved_objects/server/plugin.mock';
 
 const taskManager = taskManagerMock.create();
 
 const alertTypeRegistryParams = {
-  isSecurityEnabled: true,
-  getServices() {
-    return {
-      log: jest.fn(),
-      callCluster: jest.fn(),
-      savedObjectsClient: SavedObjectsClientMock.create(),
-    };
-  },
   taskManager,
-  executeAction: jest.fn(),
-  getBasePath: jest.fn().mockReturnValue(undefined),
-  spaceIdToNamespace: jest.fn().mockReturnValue(undefined),
-  encryptedSavedObjectsPlugin: encryptedSavedObjectsMock.create(),
+  taskRunnerFactory: new TaskRunnerFactory(),
 };
 
 beforeEach(() => jest.resetAllMocks());
@@ -60,31 +44,20 @@ describe('register()', () => {
       executor: jest.fn(),
     };
     // eslint-disable-next-line @typescript-eslint/no-var-requires
-    const { getCreateTaskRunnerFunction } = require('./lib/get_create_task_runner_function');
     const registry = new AlertTypeRegistry(alertTypeRegistryParams);
-    getCreateTaskRunnerFunction.mockReturnValue(jest.fn());
     registry.register(alertType);
     expect(taskManager.registerTaskDefinitions).toHaveBeenCalledTimes(1);
     expect(taskManager.registerTaskDefinitions.mock.calls[0]).toMatchInlineSnapshot(`
       Array [
         Object {
           "alerting:test": Object {
-            "createTaskRunner": [MockFunction],
+            "createTaskRunner": [Function],
             "title": "Test",
             "type": "alerting:test",
           },
         },
       ]
     `);
-    expect(getCreateTaskRunnerFunction).toHaveBeenCalledWith({
-      alertType,
-      isSecurityEnabled: true,
-      getServices: alertTypeRegistryParams.getServices,
-      encryptedSavedObjectsPlugin: alertTypeRegistryParams.encryptedSavedObjectsPlugin,
-      getBasePath: alertTypeRegistryParams.getBasePath,
-      spaceIdToNamespace: alertTypeRegistryParams.spaceIdToNamespace,
-      executeAction: alertTypeRegistryParams.executeAction,
-    });
   });
 
   test('should throw an error if type is already registered', () => {

diff --git a/x-pack/legacy/plugins/alerting/server/alert_type_registry.ts b/x-pack/legacy/plugins/alerting/server/alert_type_registry.ts
@@ -6,53 +6,24 @@
 
 import Boom from 'boom';
 import { i18n } from '@kbn/i18n';
-import { TaskManager } from '../../task_manager';
-import { getCreateTaskRunnerFunction } from './lib';
-import { ActionsPlugin } from '../../actions';
-import { EncryptedSavedObjectsPlugin } from '../../encrypted_saved_objects';
-import {
-  AlertType,
-  GetBasePathFunction,
-  GetServicesFunction,
-  SpaceIdToNamespaceFunction,
-} from './types';
+import { TaskRunnerFactory } from './lib';
+import { RunContext } from '../../task_manager';
+import { TaskManagerSetupContract } from './shim';
+import { AlertType } from './types';
 
 interface ConstructorOptions {
-  isSecurityEnabled: boolean;
-  getServices: GetServicesFunction;
-  taskManager: TaskManager;
-  executeAction: ActionsPlugin['execute'];
-  encryptedSavedObjectsPlugin: EncryptedSavedObjectsPlugin;
-  spaceIdToNamespace: SpaceIdToNamespaceFunction;
-  getBasePath: GetBasePathFunction;
+  taskManager: TaskManagerSetupContract;
+  taskRunnerFactory: TaskRunnerFactory;
 }
 
 export class AlertTypeRegistry {
-  private readonly getServices: GetServicesFunction;
-  private readonly taskManager: TaskManager;
-  private readonly executeAction: ActionsPlugin['execute'];
+  private readonly taskManager: TaskManagerSetupContract;
   private readonly alertTypes: Map<string, AlertType> = new Map();
-  private readonly encryptedSavedObjectsPlugin: EncryptedSavedObjectsPlugin;
-  private readonly spaceIdToNamespace: SpaceIdToNamespaceFunction;
-  private readonly getBasePath: GetBasePathFunction;
-  private readonly isSecurityEnabled: boolean;
+  private readonly taskRunnerFactory: TaskRunnerFactory;
 
-  constructor({
-    encryptedSavedObjectsPlugin,
-    executeAction,
-    taskManager,
-    getServices,
-    spaceIdToNamespace,
-    getBasePath,
-    isSecurityEnabled,
-  }: ConstructorOptions) {
+  constructor({ taskManager, taskRunnerFactory }: ConstructorOptions) {
     this.taskManager = taskManager;
-    this.executeAction = executeAction;
-    this.encryptedSavedObjectsPlugin = encryptedSavedObjectsPlugin;
-    this.getServices = getServices;
-    this.getBasePath = getBasePath;
-    this.spaceIdToNamespace = spaceIdToNamespace;
-    this.isSecurityEnabled = isSecurityEnabled;
+    this.taskRunnerFactory = taskRunnerFactory;
   }
 
   public has(id: string) {
@@ -75,15 +46,8 @@ export class AlertTypeRegistry {
       [`alerting:${alertType.id}`]: {
         title: alertType.name,
         type: `alerting:${alertType.id}`,
-        createTaskRunner: getCreateTaskRunnerFunction({
-          alertType,
-          isSecurityEnabled: this.isSecurityEnabled,
-          getServices: this.getServices,
-          executeAction: this.executeAction,
-          encryptedSavedObjectsPlugin: this.encryptedSavedObjectsPlugin,
-          getBasePath: this.getBasePath,
-          spaceIdToNamespace: this.spaceIdToNamespace,
-        }),
+        createTaskRunner: (context: RunContext) =>
+          this.taskRunnerFactory.create(alertType, context),
       },
     });
   }

diff --git a/x-pack/legacy/plugins/alerting/server/alerts_client.test.ts b/x-pack/legacy/plugins/alerting/server/alerts_client.test.ts
@@ -6,7 +6,7 @@
 
 import { schema } from '@kbn/config-schema';
 import { AlertsClient } from './alerts_client';
-import { SavedObjectsClientMock } from '../../../../../src/core/server/mocks';
+import { SavedObjectsClientMock, loggingServiceMock } from '../../../../../src/core/server/mocks';
 import { taskManagerMock } from '../../task_manager/task_manager.mock';
 import { alertTypeRegistryMock } from './alert_type_registry.mock';
 
@@ -15,13 +15,13 @@ const alertTypeRegistry = alertTypeRegistryMock.create();
 const savedObjectsClient = SavedObjectsClientMock.create();
 
 const alertsClientParams = {
-  log: jest.fn(),
   taskManager,
   alertTypeRegistry,
   savedObjectsClient,
   spaceId: 'default',
   getUserName: jest.fn(),
   createAPIKey: jest.fn(),
+  logger: loggingServiceMock.create().get(),
 };
 
 beforeEach(() => {
@@ -418,16 +418,9 @@ describe('create()', () => {
     await expect(alertsClient.create({ data })).rejects.toThrowErrorMatchingInlineSnapshot(
       `"Task manager error"`
     );
-    expect(alertsClientParams.log).toHaveBeenCalledTimes(1);
-    expect(alertsClientParams.log.mock.calls[0]).toMatchInlineSnapshot(`
-                                                                                                                  Array [
-                                                                                                                    Array [
-                                                                                                                      "alerting",
-                                                                                                                      "error",
-                                                                                                                    ],
-                                                                                                                    "Failed to cleanup alert \\"1\\" after scheduling task failed. Error: Saved object delete error",
-                                                                                                                  ]
-                                                                            `);
+    expect(alertsClientParams.logger.error).toHaveBeenCalledWith(
+      'Failed to cleanup alert "1" after scheduling task failed. Error: Saved object delete error'
+    );
   });
 
   test('throws an error if alert type not registerd', async () => {

diff --git a/x-pack/legacy/plugins/alerting/server/alerts_client.ts b/x-pack/legacy/plugins/alerting/server/alerts_client.ts
@@ -7,9 +7,9 @@
 import Boom from 'boom';
 import { omit } from 'lodash';
 import { i18n } from '@kbn/i18n';
-import { SavedObjectsClientContract, SavedObjectReference } from 'src/core/server';
-import { Alert, RawAlert, AlertTypeRegistry, AlertAction, Log, AlertType } from './types';
-import { TaskManager } from '../../task_manager';
+import { Logger, SavedObjectsClientContract, SavedObjectReference } from 'src/core/server';
+import { Alert, RawAlert, AlertTypeRegistry, AlertAction, AlertType } from './types';
+import { TaskManagerStartContract } from './shim';
 import { validateAlertTypeParams } from './lib';
 import { CreateAPIKeyResult as SecurityPluginCreateAPIKeyResult } from '../../../../plugins/security/server';
 
@@ -23,8 +23,8 @@ interface SuccessCreateAPIKeyResult {
 export type CreateAPIKeyResult = FailedCreateAPIKeyResult | SuccessCreateAPIKeyResult;
 
 interface ConstructorOptions {
-  log: Log;
-  taskManager: TaskManager;
+  logger: Logger;
+  taskManager: TaskManagerStartContract;
   savedObjectsClient: SavedObjectsClientContract;
   alertTypeRegistry: AlertTypeRegistry;
   spaceId?: string;
@@ -78,10 +78,10 @@ interface UpdateOptions {
 }
 
 export class AlertsClient {
-  private readonly log: Log;
+  private readonly logger: Logger;
   private readonly getUserName: () => Promise<string | null>;
   private readonly spaceId?: string;
-  private readonly taskManager: TaskManager;
+  private readonly taskManager: TaskManagerStartContract;
   private readonly savedObjectsClient: SavedObjectsClientContract;
   private readonly alertTypeRegistry: AlertTypeRegistry;
   private readonly createAPIKey: () => Promise<CreateAPIKeyResult>;
@@ -90,12 +90,12 @@ export class AlertsClient {
     alertTypeRegistry,
     savedObjectsClient,
     taskManager,
-    log,
+    logger,
     spaceId,
     getUserName,
     createAPIKey,
   }: ConstructorOptions) {
-    this.log = log;
+    this.logger = logger;
     this.getUserName = getUserName;
     this.spaceId = spaceId;
     this.taskManager = taskManager;
@@ -143,8 +143,7 @@ export class AlertsClient {
           await this.savedObjectsClient.delete('alert', createdAlert.id);
         } catch (err) {
           // Skip the cleanup error and throw the task manager error to avoid confusion
-          this.log(
-            ['alerting', 'error'],
+          this.logger.error(
             `Failed to cleanup alert "${createdAlert.id}" after scheduling task failed. Error: ${err.message}`
           );
         }

diff --git a/x-pack/legacy/plugins/alerting/server/index.ts b/x-pack/legacy/plugins/alerting/server/index.ts
@@ -7,3 +7,4 @@
 export { init } from './init';
 export { AlertType, AlertingPlugin, AlertExecutorOptions } from './types';
 export { AlertsClient } from './alerts_client';
+export { PluginSetupContract, PluginStartContract } from './plugin';