Make the actions plugin support generics (#71439) (#74294)

* Initial attempt at making the actions plugin support generics * Export WebhookMethods * Fix typings for registry * Usage of Record<string, unknown> * Apply feedback from Gidi * Cleanup * Fix validate_with_schema * Cleanup pt2 * Fix failing tests * Add generics to ActionType for ActionTypeExecutorResult Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
elastic · Aug 5, 2020 · a600eca · a600eca
1 parent a7b2f18
commit a600eca
Show file tree

Hide file tree

Showing 30 changed files with 466 additions and 247 deletions.
diff --git a/x-pack/plugins/actions/server/action_type_registry.test.ts b/x-pack/plugins/actions/server/action_type_registry.test.ts
@@ -41,7 +41,7 @@ beforeEach(() => {
   };
 });
 
-const executor: ExecutorType = async (options) => {
+const executor: ExecutorType<{}, {}, {}, void> = async (options) => {
   return { status: 'ok', actionId: options.actionId };
 };
 
@@ -203,7 +203,9 @@ describe('isActionTypeEnabled', () => {
     id: 'foo',
     name: 'Foo',
     minimumLicenseRequired: 'basic',
-    executor: async () => {},
+    executor: async (options) => {
+      return { status: 'ok', actionId: options.actionId };
+    },
   };
 
   beforeEach(() => {
@@ -258,7 +260,9 @@ describe('ensureActionTypeEnabled', () => {
     id: 'foo',
     name: 'Foo',
     minimumLicenseRequired: 'basic',
-    executor: async () => {},
+    executor: async (options) => {
+      return { status: 'ok', actionId: options.actionId };
+    },
   };
 
   beforeEach(() => {

diff --git a/x-pack/plugins/actions/server/action_type_registry.ts b/x-pack/plugins/actions/server/action_type_registry.ts
@@ -8,9 +8,15 @@ import Boom from 'boom';
 import { i18n } from '@kbn/i18n';
 import { RunContext, TaskManagerSetupContract } from '../../task_manager/server';
 import { ExecutorError, TaskRunnerFactory, ILicenseState } from './lib';
-import { ActionType, PreConfiguredAction } from './types';
 import { ActionType as CommonActionType } from '../common';
 import { ActionsConfigurationUtilities } from './actions_config';
+import {
+  ActionType,
+  PreConfiguredAction,
+  ActionTypeConfig,
+  ActionTypeSecrets,
+  ActionTypeParams,
+} from './types';
 
 export interface ActionTypeRegistryOpts {
   taskManager: TaskManagerSetupContract;
@@ -77,7 +83,12 @@ export class ActionTypeRegistry {
   /**
    * Registers an action type to the action type registry
    */
-  public register(actionType: ActionType) {
+  public register<
+    Config extends ActionTypeConfig = ActionTypeConfig,
+    Secrets extends ActionTypeSecrets = ActionTypeSecrets,
+    Params extends ActionTypeParams = ActionTypeParams,
+    ExecutorResultData = void
+  >(actionType: ActionType<Config, Secrets, Params, ExecutorResultData>) {
     if (this.has(actionType.id)) {
       throw new Error(
         i18n.translate(
@@ -91,7 +102,7 @@ export class ActionTypeRegistry {
         )
       );
     }
-    this.actionTypes.set(actionType.id, { ...actionType });
+    this.actionTypes.set(actionType.id, { ...actionType } as ActionType);
     this.taskManager.registerTaskDefinitions({
       [`actions:${actionType.id}`]: {
         title: actionType.name,
@@ -112,7 +123,12 @@ export class ActionTypeRegistry {
   /**
    * Returns an action type, throws if not registered
    */
-  public get(id: string): ActionType {
+  public get<
+    Config extends ActionTypeConfig = ActionTypeConfig,
+    Secrets extends ActionTypeSecrets = ActionTypeSecrets,
+    Params extends ActionTypeParams = ActionTypeParams,
+    ExecutorResultData = void
+  >(id: string): ActionType<Config, Secrets, Params, ExecutorResultData> {
     if (!this.has(id)) {
       throw Boom.badRequest(
         i18n.translate('xpack.actions.actionTypeRegistry.get.missingActionTypeErrorMessage', {
@@ -123,7 +139,7 @@ export class ActionTypeRegistry {
         })
       );
     }
-    return this.actionTypes.get(id)!;
+    return this.actionTypes.get(id)! as ActionType<Config, Secrets, Params, ExecutorResultData>;
   }
 
   /**

diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts
@@ -39,7 +39,7 @@ let actionsClient: ActionsClient;
 let mockedLicenseState: jest.Mocked<ILicenseState>;
 let actionTypeRegistry: ActionTypeRegistry;
 let actionTypeRegistryParams: ActionTypeRegistryOpts;
-const executor: ExecutorType = async (options) => {
+const executor: ExecutorType<{}, {}, {}, void> = async (options) => {
   return { status: 'ok', actionId: options.actionId };
 };
 

diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts
@@ -298,7 +298,7 @@ export class ActionsClient {
   public async execute({
     actionId,
     params,
-  }: Omit<ExecuteOptions, 'request'>): Promise<ActionTypeExecutorResult> {
+  }: Omit<ExecuteOptions, 'request'>): Promise<ActionTypeExecutorResult<unknown>> {
     await this.authorization.ensureAuthorized('execute');
     return this.actionExecutor.execute({ actionId, params, request: this.request });
   }

diff --git a/x-pack/plugins/actions/server/builtin_action_types/case/utils.ts b/x-pack/plugins/actions/server/builtin_action_types/case/utils.ts
@@ -10,6 +10,10 @@ import { schema } from '@kbn/config-schema';
 import { ActionTypeExecutorOptions, ActionTypeExecutorResult, ActionType } from '../../types';
 
 import { ExecutorParamsSchema } from './schema';
+import {
+  ExternalIncidentServiceConfiguration,
+  ExternalIncidentServiceSecretConfiguration,
+} from './types';
 
 import {
   CreateExternalServiceArgs,
@@ -23,6 +27,7 @@ import {
   TransformFieldsArgs,
   Comment,
   ExecutorSubActionPushParams,
+  PushToServiceResponse,
 } from './types';
 
 import { transformers } from './transformers';
@@ -63,14 +68,17 @@ export const createConnectorExecutor = ({
   api,
   createExternalService,
 }: CreateExternalServiceBasicArgs) => async (
-  execOptions: ActionTypeExecutorOptions
-): Promise<ActionTypeExecutorResult> => {
+  execOptions: ActionTypeExecutorOptions<
+    ExternalIncidentServiceConfiguration,
+    ExternalIncidentServiceSecretConfiguration,
+    ExecutorParams
+  >
+): Promise<ActionTypeExecutorResult<PushToServiceResponse | {}>> => {
   const { actionId, config, params, secrets } = execOptions;
-  const { subAction, subActionParams } = params as ExecutorParams;
+  const { subAction, subActionParams } = params;
   let data = {};
 
-  const res: Pick<ActionTypeExecutorResult, 'status'> &
-    Pick<ActionTypeExecutorResult, 'actionId'> = {
+  const res: ActionTypeExecutorResult<void> = {
     status: 'ok',
     actionId,
   };

diff --git a/x-pack/plugins/actions/server/builtin_action_types/email.test.ts b/x-pack/plugins/actions/server/builtin_action_types/email.test.ts
@@ -10,7 +10,6 @@ jest.mock('./lib/send_email', () => ({
 
 import { Logger } from '../../../../../src/core/server';
 
-import { ActionType, ActionTypeExecutorOptions } from '../types';
 import { actionsConfigMock } from '../actions_config.mock';
 import { validateConfig, validateSecrets, validateParams } from '../lib';
 import { createActionTypeRegistry } from './index.test';
@@ -21,6 +20,8 @@ import {
   ActionTypeConfigType,
   ActionTypeSecretsType,
   getActionType,
+  EmailActionType,
+  EmailActionTypeExecutorOptions,
 } from './email';
 
 const sendEmailMock = sendEmail as jest.Mock;
@@ -29,13 +30,17 @@ const ACTION_TYPE_ID = '.email';
 
 const services = actionsMock.createServices();
 
-let actionType: ActionType;
+let actionType: EmailActionType;
 let mockedLogger: jest.Mocked<Logger>;
 
 beforeEach(() => {
   jest.resetAllMocks();
   const { actionTypeRegistry } = createActionTypeRegistry();
-  actionType = actionTypeRegistry.get(ACTION_TYPE_ID);
+  actionType = actionTypeRegistry.get<
+    ActionTypeConfigType,
+    ActionTypeSecretsType,
+    ActionParamsType
+  >(ACTION_TYPE_ID);
 });
 
 describe('actionTypeRegistry.get() works', () => {
@@ -242,7 +247,7 @@ describe('execute()', () => {
     };
 
     const actionId = 'some-id';
-    const executorOptions: ActionTypeExecutorOptions = {
+    const executorOptions: EmailActionTypeExecutorOptions = {
       actionId,
       config,
       params,
@@ -306,7 +311,7 @@ describe('execute()', () => {
     };
 
     const actionId = 'some-id';
-    const executorOptions: ActionTypeExecutorOptions = {
+    const executorOptions: EmailActionTypeExecutorOptions = {
       actionId,
       config,
       params,
@@ -363,7 +368,7 @@ describe('execute()', () => {
     };
 
     const actionId = 'some-id';
-    const executorOptions: ActionTypeExecutorOptions = {
+    const executorOptions: EmailActionTypeExecutorOptions = {
       actionId,
       config,
       params,

diff --git a/x-pack/plugins/actions/server/builtin_action_types/email.ts b/x-pack/plugins/actions/server/builtin_action_types/email.ts
@@ -15,6 +15,18 @@ import { Logger } from '../../../../../src/core/server';
 import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types';
 import { ActionsConfigurationUtilities } from '../actions_config';
 
+export type EmailActionType = ActionType<
+  ActionTypeConfigType,
+  ActionTypeSecretsType,
+  ActionParamsType,
+  unknown
+>;
+export type EmailActionTypeExecutorOptions = ActionTypeExecutorOptions<
+  ActionTypeConfigType,
+  ActionTypeSecretsType,
+  ActionParamsType
+>;
+
 // config definition
 export type ActionTypeConfigType = TypeOf<typeof ConfigSchema>;
 
@@ -30,10 +42,9 @@ const ConfigSchema = schema.object(ConfigSchemaProps);
 
 function validateConfig(
   configurationUtilities: ActionsConfigurationUtilities,
-  configObject: unknown
+  configObject: ActionTypeConfigType
 ): string | void {
-  // avoids circular reference ...
-  const config = configObject as ActionTypeConfigType;
+  const config = configObject;
 
   // Make sure service is set, or if not, both host/port must be set.
   // If service is set, host/port are ignored, when the email is sent.
@@ -113,7 +124,7 @@ interface GetActionTypeParams {
 }
 
 // action type definition
-export function getActionType(params: GetActionTypeParams): ActionType {
+export function getActionType(params: GetActionTypeParams): EmailActionType {
   const { logger, configurationUtilities } = params;
   return {
     id: '.email',
@@ -136,12 +147,12 @@ export function getActionType(params: GetActionTypeParams): ActionType {
 
 async function executor(
   { logger }: { logger: Logger },
-  execOptions: ActionTypeExecutorOptions
-): Promise<ActionTypeExecutorResult> {
+  execOptions: EmailActionTypeExecutorOptions
+): Promise<ActionTypeExecutorResult<unknown>> {
   const actionId = execOptions.actionId;
-  const config = execOptions.config as ActionTypeConfigType;
-  const secrets = execOptions.secrets as ActionTypeSecretsType;
-  const params = execOptions.params as ActionParamsType;
+  const config = execOptions.config;
+  const secrets = execOptions.secrets;
+  const params = execOptions.params;
 
   const transport: Transport = {};
 

diff --git a/x-pack/plugins/actions/server/builtin_action_types/es_index.test.ts b/x-pack/plugins/actions/server/builtin_action_types/es_index.test.ts
@@ -8,21 +8,25 @@ jest.mock('./lib/send_email', () => ({
   sendEmail: jest.fn(),
 }));
 
-import { ActionType, ActionTypeExecutorOptions } from '../types';
 import { validateConfig, validateParams } from '../lib';
 import { createActionTypeRegistry } from './index.test';
-import { ActionParamsType, ActionTypeConfigType } from './es_index';
 import { actionsMock } from '../mocks';
+import {
+  ActionParamsType,
+  ActionTypeConfigType,
+  ESIndexActionType,
+  ESIndexActionTypeExecutorOptions,
+} from './es_index';
 
 const ACTION_TYPE_ID = '.index';
 
 const services = actionsMock.createServices();
 
-let actionType: ActionType;
+let actionType: ESIndexActionType;
 
 beforeAll(() => {
   const { actionTypeRegistry } = createActionTypeRegistry();
-  actionType = actionTypeRegistry.get(ACTION_TYPE_ID);
+  actionType = actionTypeRegistry.get<ActionTypeConfigType, {}, ActionParamsType>(ACTION_TYPE_ID);
 });
 
 beforeEach(() => {
@@ -144,12 +148,12 @@ describe('params validation', () => {
 describe('execute()', () => {
   test('ensure parameters are as expected', async () => {
     const secrets = {};
-    let config: Partial<ActionTypeConfigType>;
+    let config: ActionTypeConfigType;
     let params: ActionParamsType;
-    let executorOptions: ActionTypeExecutorOptions;
+    let executorOptions: ESIndexActionTypeExecutorOptions;
 
     // minimal params
-    config = { index: 'index-value', refresh: false };
+    config = { index: 'index-value', refresh: false, executionTimeField: null };
     params = {
       documents: [{ jim: 'bob' }],
     };
@@ -215,7 +219,7 @@ describe('execute()', () => {
     `);
 
     // minimal params
-    config = { index: 'index-value', executionTimeField: undefined, refresh: false };
+    config = { index: 'index-value', executionTimeField: null, refresh: false };
     params = {
       documents: [{ jim: 'bob' }],
     };
@@ -245,7 +249,7 @@ describe('execute()', () => {
     `);
 
     // multiple documents
-    config = { index: 'index-value', executionTimeField: undefined, refresh: false };
+    config = { index: 'index-value', executionTimeField: null, refresh: false };
     params = {
       documents: [{ a: 1 }, { b: 2 }],
     };

diff --git a/x-pack/plugins/actions/server/builtin_action_types/es_index.ts b/x-pack/plugins/actions/server/builtin_action_types/es_index.ts
@@ -11,6 +11,13 @@ import { schema, TypeOf } from '@kbn/config-schema';
 import { Logger } from '../../../../../src/core/server';
 import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types';
 
+export type ESIndexActionType = ActionType<ActionTypeConfigType, {}, ActionParamsType, unknown>;
+export type ESIndexActionTypeExecutorOptions = ActionTypeExecutorOptions<
+  ActionTypeConfigType,
+  {},
+  ActionParamsType
+>;
+
 // config definition
 
 export type ActionTypeConfigType = TypeOf<typeof ConfigSchema>;
@@ -33,7 +40,7 @@ const ParamsSchema = schema.object({
 });
 
 // action type definition
-export function getActionType({ logger }: { logger: Logger }): ActionType {
+export function getActionType({ logger }: { logger: Logger }): ESIndexActionType {
   return {
     id: '.index',
     minimumLicenseRequired: 'basic',
@@ -52,11 +59,11 @@ export function getActionType({ logger }: { logger: Logger }): ActionType {
 
 async function executor(
   { logger }: { logger: Logger },
-  execOptions: ActionTypeExecutorOptions
-): Promise<ActionTypeExecutorResult> {
+  execOptions: ESIndexActionTypeExecutorOptions
+): Promise<ActionTypeExecutorResult<unknown>> {
   const actionId = execOptions.actionId;
-  const config = execOptions.config as ActionTypeConfigType;
-  const params = execOptions.params as ActionParamsType;
+  const config = execOptions.config;
+  const params = execOptions.params;
   const services = execOptions.services;
 
   const index = config.index;