Skip to content

Commit

Permalink
Merge action type validation with mocks refactor PR
Browse files Browse the repository at this point in the history
  • Loading branch information
mikecote committed Mar 5, 2020
2 parents f9cbe93 + 8b1a12f commit a50ae68
Show file tree
Hide file tree
Showing 26 changed files with 310 additions and 61 deletions.
51 changes: 49 additions & 2 deletions x-pack/plugins/actions/server/action_type_registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,28 @@

import { taskManagerMock } from '../../task_manager/server/task_manager.mock';
import { ActionTypeRegistry, ActionTypeRegistryOpts } from './action_type_registry';
import { ExecutorType } from './types';
import { ActionExecutor, ExecutorError, TaskRunnerFactory } from './lib';
import { ActionType, ExecutorType } from './types';
import { ActionExecutor, ExecutorError, ILicenseState, TaskRunnerFactory } from './lib';
import { actionsConfigMock } from './actions_config.mock';
import { licenseStateMock } from './lib/license_state.mock';
import { ActionsConfigurationUtilities } from './actions_config';

const mockTaskManager = taskManagerMock.setup();
let mockedLicenseState: jest.Mocked<ILicenseState>;
let mockedActionsConfig: jest.Mocked<ActionsConfigurationUtilities>;
let actionTypeRegistryParams: ActionTypeRegistryOpts;

beforeEach(() => {
jest.resetAllMocks();
mockedLicenseState = licenseStateMock.create();
mockedActionsConfig = actionsConfigMock.create();
actionTypeRegistryParams = {
taskManager: mockTaskManager,
taskRunnerFactory: new TaskRunnerFactory(
new ActionExecutor({ isESOUsingEphemeralEncryptionKey: false })
),
actionsConfigUtils: mockedActionsConfig,
licenseState: mockedLicenseState,
};
});

Expand Down Expand Up @@ -163,3 +167,46 @@ describe('has()', () => {
expect(actionTypeRegistry.has('my-action-type'));
});
});

describe('ensureActionTypeEnabled', () => {
let actionTypeRegistry: ActionTypeRegistry;
const fooActionType: ActionType = {
id: 'foo',
name: 'Foo',
minimumLicenseRequired: 'basic',
executor: async () => {},
};

beforeEach(() => {
actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
actionTypeRegistry.register(fooActionType);
});

test('should call ensureActionTypeEnabled of the action config', async () => {
actionTypeRegistry.ensureActionTypeEnabled('foo');
expect(mockedActionsConfig.ensureActionTypeEnabled).toHaveBeenCalledWith('foo');
});

test('should call ensureLicenseForActionType on the license state', async () => {
actionTypeRegistry.ensureActionTypeEnabled('foo');
expect(mockedLicenseState.ensureLicenseForActionType).toHaveBeenCalledWith(fooActionType);
});

test('should throw when ensureActionTypeEnabled throws', async () => {
mockedActionsConfig.ensureActionTypeEnabled.mockImplementation(() => {
throw new Error('Fail');
});
expect(() =>
actionTypeRegistry.ensureActionTypeEnabled('foo')
).toThrowErrorMatchingInlineSnapshot(`"Fail"`);
});

test('should throw when ensureLicenseForActionType throws', async () => {
mockedLicenseState.ensureLicenseForActionType.mockImplementation(() => {
throw new Error('Fail');
});
expect(() =>
actionTypeRegistry.ensureActionTypeEnabled('foo')
).toThrowErrorMatchingInlineSnapshot(`"Fail"`);
});
});
6 changes: 5 additions & 1 deletion x-pack/plugins/actions/server/action_type_registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import Boom from 'boom';
import { i18n } from '@kbn/i18n';
import { RunContext, TaskManagerSetupContract } from '../../task_manager/server';
import { ExecutorError, TaskRunnerFactory } from './lib';
import { ExecutorError, TaskRunnerFactory, ILicenseState } from './lib';
import { ActionType } from './types';
import { ActionType as CommonActionType } from '../common';
import { ActionsConfigurationUtilities } from './actions_config';
Expand All @@ -16,18 +16,21 @@ export interface ActionTypeRegistryOpts {
taskManager: TaskManagerSetupContract;
taskRunnerFactory: TaskRunnerFactory;
actionsConfigUtils: ActionsConfigurationUtilities;
licenseState: ILicenseState;
}

export class ActionTypeRegistry {
private readonly taskManager: TaskManagerSetupContract;
private readonly actionTypes: Map<string, ActionType> = new Map();
private readonly taskRunnerFactory: TaskRunnerFactory;
private readonly actionsConfigUtils: ActionsConfigurationUtilities;
private readonly licenseState: ILicenseState;

constructor(constructorParams: ActionTypeRegistryOpts) {
this.taskManager = constructorParams.taskManager;
this.taskRunnerFactory = constructorParams.taskRunnerFactory;
this.actionsConfigUtils = constructorParams.actionsConfigUtils;
this.licenseState = constructorParams.licenseState;
}

/**
Expand All @@ -42,6 +45,7 @@ export class ActionTypeRegistry {
*/
public ensureActionTypeEnabled(id: string) {
this.actionsConfigUtils.ensureActionTypeEnabled(id);
this.licenseState.ensureLicenseForActionType(this.get(id));
}

/**
Expand Down
99 changes: 89 additions & 10 deletions x-pack/plugins/actions/server/actions_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@

import { schema } from '@kbn/config-schema';

import { ActionTypeRegistry } from './action_type_registry';
import { ActionTypeRegistry, ActionTypeRegistryOpts } from './action_type_registry';
import { ActionsClient } from './actions_client';
import { ExecutorType } from './types';
import { ActionExecutor, TaskRunnerFactory } from './lib';
import { ActionExecutor, TaskRunnerFactory, ILicenseState } from './lib';
import { taskManagerMock } from '../../task_manager/server/task_manager.mock';
import { actionsConfigMock } from './actions_config.mock';
import { getActionsConfigurationUtilities } from './actions_config';
import { licenseStateMock } from './lib/license_state.mock';

import {
elasticsearchServiceMock,
Expand All @@ -25,22 +26,25 @@ const scopedClusterClient = elasticsearchServiceMock.createScopedClusterClient()

const mockTaskManager = taskManagerMock.setup();

const actionTypeRegistryParams = {
taskManager: mockTaskManager,
taskRunnerFactory: new TaskRunnerFactory(
new ActionExecutor({ isESOUsingEphemeralEncryptionKey: false })
),
actionsConfigUtils: actionsConfigMock.create(),
};

let actionsClient: ActionsClient;
let mockedLicenseState: jest.Mocked<ILicenseState>;
let actionTypeRegistry: ActionTypeRegistry;
let actionTypeRegistryParams: ActionTypeRegistryOpts;
const executor: ExecutorType = async options => {
return { status: 'ok', actionId: options.actionId };
};

beforeEach(() => {
jest.resetAllMocks();
mockedLicenseState = licenseStateMock.create();
actionTypeRegistryParams = {
taskManager: mockTaskManager,
taskRunnerFactory: new TaskRunnerFactory(
new ActionExecutor({ isESOUsingEphemeralEncryptionKey: false })
),
actionsConfigUtils: actionsConfigMock.create(),
licenseState: mockedLicenseState,
};
actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
actionsClient = new ActionsClient({
actionTypeRegistry,
Expand Down Expand Up @@ -213,6 +217,7 @@ describe('create()', () => {
new ActionExecutor({ isESOUsingEphemeralEncryptionKey: false })
),
actionsConfigUtils: localConfigUtils,
licenseState: licenseStateMock.create(),
};

actionTypeRegistry = new ActionTypeRegistry(localActionTypeRegistryParams);
Expand Down Expand Up @@ -254,6 +259,39 @@ describe('create()', () => {
`"action type \\"my-action-type\\" is not enabled in the Kibana config xpack.actions.enabledActionTypes"`
);
});

test('throws error when ensureActionTypeEnabled throws', async () => {
const savedObjectCreateResult = {
id: '1',
type: 'type',
attributes: {
name: 'my name',
actionTypeId: 'my-action-type',
config: {},
},
references: [],
};
actionTypeRegistry.register({
id: 'my-action-type',
name: 'My action type',
minimumLicenseRequired: 'basic',
executor,
});
mockedLicenseState.ensureLicenseForActionType.mockImplementation(() => {
throw new Error('Fail');
});
savedObjectsClient.create.mockResolvedValueOnce(savedObjectCreateResult);
await expect(
actionsClient.create({
action: {
name: 'my name',
actionTypeId: 'my-action-type',
config: {},
secrets: {},
},
})
).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`);
});
});

describe('get()', () => {
Expand Down Expand Up @@ -512,4 +550,45 @@ describe('update()', () => {
]
`);
});

test('throws an error when ensureActionTypeEnabled throws', async () => {
actionTypeRegistry.register({
id: 'my-action-type',
name: 'My action type',
minimumLicenseRequired: 'basic',
executor,
});
mockedLicenseState.ensureLicenseForActionType.mockImplementation(() => {
throw new Error('Fail');
});
savedObjectsClient.get.mockResolvedValueOnce({
id: '1',
type: 'action',
attributes: {
actionTypeId: 'my-action-type',
},
references: [],
});
savedObjectsClient.update.mockResolvedValueOnce({
id: 'my-action',
type: 'action',
attributes: {
actionTypeId: 'my-action-type',
name: 'my name',
config: {},
secrets: {},
},
references: [],
});
await expect(
actionsClient.update({
id: 'my-action',
action: {
name: 'my name',
config: {},
secrets: {},
},
})
).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`);
});
});
9 changes: 3 additions & 6 deletions x-pack/plugins/actions/server/actions_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/

import Boom from 'boom';
import {
IScopedClusterClient,
SavedObjectsClientContract,
Expand Down Expand Up @@ -93,11 +92,7 @@ export class ActionsClient {
const validatedActionTypeConfig = validateConfig(actionType, config);
const validatedActionTypeSecrets = validateSecrets(actionType, secrets);

try {
this.actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);
} catch (err) {
throw Boom.badRequest(err.message);
}
this.actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);

const result = await this.savedObjectsClient.create('action', {
actionTypeId,
Expand Down Expand Up @@ -125,6 +120,8 @@ export class ActionsClient {
const validatedActionTypeConfig = validateConfig(actionType, config);
const validatedActionTypeSecrets = validateSecrets(actionType, secrets);

this.actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);

const result = await this.savedObjectsClient.update('action', id, {
actionTypeId,
name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { registerBuiltInActionTypes } from './index';
import { Logger } from '../../../../../src/core/server';
import { loggingServiceMock } from '../../../../../src/core/server/mocks';
import { actionsConfigMock } from '../actions_config.mock';
import { licenseStateMock } from '../lib/license_state.mock';

const ACTION_TYPE_IDS = ['.index', '.email', '.pagerduty', '.server-log', '.slack', '.webhook'];

Expand All @@ -25,6 +26,7 @@ export function createActionTypeRegistry(): {
new ActionExecutor({ isESOUsingEphemeralEncryptionKey: false })
),
actionsConfigUtils: actionsConfigMock.create(),
licenseState: licenseStateMock.create(),
});
registerBuiltInActionTypes({
logger,
Expand Down
7 changes: 7 additions & 0 deletions x-pack/plugins/actions/server/lib/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

export class ForbiddenError extends Error {}
3 changes: 3 additions & 0 deletions x-pack/plugins/actions/server/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ export { ExecutorError } from './executor_error';
export { validateParams, validateConfig, validateSecrets } from './validate_with_schema';
export { TaskRunnerFactory } from './task_runner_factory';
export { ActionExecutor, ActionExecutorContract } from './action_executor';
export { ILicenseState, LicenseState } from './license_state';
export { verifyApiAccess } from './verify_api_access';
export { ForbiddenError } from './errors';
1 change: 1 addition & 0 deletions x-pack/plugins/actions/server/lib/license_state.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const createLicenseStateMock = () => {
const licenseState: jest.Mocked<ILicenseState> = {
clean: jest.fn(),
getLicenseInformation: jest.fn(),
ensureLicenseForActionType: jest.fn(),
checkLicense: jest.fn().mockResolvedValue({
state: LICENSE_CHECK_STATE.Valid,
}),
Expand Down
Loading

0 comments on commit a50ae68

Please sign in to comment.