Skip to content

Commit

Permalink
Make alerting properly space aware (#42081) (#42525)
Browse files Browse the repository at this point in the history
* Make alerting properly space aware

* Fix broken jest tests

* Fix broken integration test

* Cleanup pt1

* Add spaces integration tests

* Fix type check failure

* Apply PR feedback pt1

* Use TS types from spaces plugin

* Fix broken tests

* Apply PR feedback pt1

* Fix getBasePath to return server.basePath when spaces is disabled

* Apply new API changes to tests
  • Loading branch information
mikecote authored Aug 2, 2019
1 parent ecf3fba commit a0961f2
Show file tree
Hide file tree
Showing 41 changed files with 1,086 additions and 418 deletions.
6 changes: 2 additions & 4 deletions x-pack/legacy/plugins/actions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,7 @@ The following table describes the properties of the `options` object.
|---|---|---|
|id|The id of the action you want to fire.|string|
|params|The `params` value to give the action type executor.|object|
|namespace|The saved object namespace the action exists within.|string|
|basePath|This is a temporary parameter, but we need to capture and track the value of `request.getBasePath()` until future changes are made.<br><br>In most cases this can be `undefined` unless you need cross spaces support.|string|
|spaceId|The space id the action is within.|string|

### Example

Expand All @@ -155,14 +154,13 @@ This example makes action `3c5b2bd4-5424-4e4b-8cf5-c0a58c762cc5` fire an email.
```
server.plugins.actions.fire({
id: '3c5b2bd4-5424-4e4b-8cf5-c0a58c762cc5',
spaceId: 'default', // The spaceId of the action
params: {
from: 'example@elastic.co',
to: ['destination@elastic.co'],
subject: 'My email subject',
body: 'My email body',
},
namespace: undefined, // The namespace the action exists within
basePath: undefined, // Usually `request.getBasePath();` or `undefined`
});
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ const actionTypeRegistryParams = {
getServices,
taskManager: mockTaskManager,
encryptedSavedObjectsPlugin: encryptedSavedObjectsMock.create(),
spaceIdToNamespace: jest.fn().mockReturnValue(undefined),
getBasePath: jest.fn().mockReturnValue(undefined),
};

beforeEach(() => jest.resetAllMocks());
Expand Down
21 changes: 14 additions & 7 deletions x-pack/legacy/plugins/actions/server/action_type_registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,35 @@ import { ActionType, GetServicesFunction } from './types';
import { TaskManager, TaskRunCreatorFunction } from '../../task_manager';
import { getCreateTaskRunnerFunction, ExecutorError } from './lib';
import { EncryptedSavedObjectsPlugin } from '../../encrypted_saved_objects';
import { SpacesPlugin } from '../../spaces';

interface ConstructorOptions {
taskManager: TaskManager;
getServices: GetServicesFunction;
encryptedSavedObjectsPlugin: EncryptedSavedObjectsPlugin;
spaceIdToNamespace: SpacesPlugin['spaceIdToNamespace'];
getBasePath: SpacesPlugin['getBasePath'];
}

export class ActionTypeRegistry {
private readonly taskRunCreatorFunction: TaskRunCreatorFunction;
private readonly getServices: GetServicesFunction;
private readonly taskManager: TaskManager;
private readonly actionTypes: Map<string, ActionType> = new Map();
private readonly encryptedSavedObjectsPlugin: EncryptedSavedObjectsPlugin;

constructor({ getServices, taskManager, encryptedSavedObjectsPlugin }: ConstructorOptions) {
this.getServices = getServices;
constructor({
getServices,
taskManager,
encryptedSavedObjectsPlugin,
spaceIdToNamespace,
getBasePath,
}: ConstructorOptions) {
this.taskManager = taskManager;
this.encryptedSavedObjectsPlugin = encryptedSavedObjectsPlugin;
this.taskRunCreatorFunction = getCreateTaskRunnerFunction({
getServices,
actionTypeRegistry: this,
getServices: this.getServices,
encryptedSavedObjectsPlugin: this.encryptedSavedObjectsPlugin,
encryptedSavedObjectsPlugin,
spaceIdToNamespace,
getBasePath,
});
}

Expand Down
8 changes: 4 additions & 4 deletions x-pack/legacy/plugins/actions/server/actions_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,14 @@ import { ActionTypeRegistry } from './action_type_registry';
import { ActionsClient } from './actions_client';
import { ExecutorType } from './types';
import { taskManagerMock } from '../../task_manager/task_manager.mock';
import { EncryptedSavedObjectsPlugin } from '../../encrypted_saved_objects';
import { SavedObjectsClientMock } from '../../../../../src/core/server/mocks';
import { encryptedSavedObjectsMock } from '../../encrypted_saved_objects/server/plugin.mock';

const savedObjectsClient = SavedObjectsClientMock.create();

const mockTaskManager = taskManagerMock.create();

const mockEncryptedSavedObjectsPlugin = {
getDecryptedAsInternalUser: jest.fn() as EncryptedSavedObjectsPlugin['getDecryptedAsInternalUser'],
} as EncryptedSavedObjectsPlugin;
const mockEncryptedSavedObjectsPlugin = encryptedSavedObjectsMock.create();

function getServices() {
return {
Expand All @@ -33,6 +31,8 @@ const actionTypeRegistryParams = {
getServices,
taskManager: mockTaskManager,
encryptedSavedObjectsPlugin: mockEncryptedSavedObjectsPlugin,
spaceIdToNamespace: jest.fn().mockReturnValue(undefined),
getBasePath: jest.fn().mockReturnValue(undefined),
};

const executor: ExecutorType = async options => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ beforeAll(() => {
getServices,
taskManager: taskManagerMock.create(),
encryptedSavedObjectsPlugin: mockEncryptedSavedObjectsPlugin,
spaceIdToNamespace: jest.fn().mockReturnValue(undefined),
getBasePath: jest.fn().mockReturnValue(undefined),
});

registerBuiltInActionTypes(actionTypeRegistry);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ beforeAll(() => {
getServices,
taskManager: taskManagerMock.create(),
encryptedSavedObjectsPlugin: mockEncryptedSavedObjectsPlugin,
spaceIdToNamespace: jest.fn().mockReturnValue(undefined),
getBasePath: jest.fn().mockReturnValue(undefined),
});

registerBuiltInActionTypes(actionTypeRegistry);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ beforeAll(() => {
getServices,
taskManager: taskManagerMock.create(),
encryptedSavedObjectsPlugin: mockEncryptedSavedObjectsPlugin,
spaceIdToNamespace: jest.fn().mockReturnValue(undefined),
getBasePath: jest.fn().mockReturnValue(undefined),
});
registerBuiltInActionTypes(actionTypeRegistry);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ beforeAll(() => {
getServices,
taskManager: taskManagerMock.create(),
encryptedSavedObjectsPlugin: mockEncryptedSavedObjectsPlugin,
spaceIdToNamespace: jest.fn().mockReturnValue(undefined),
getBasePath: jest.fn().mockReturnValue(undefined),
});
actionTypeRegistry.register(getActionType({ executor: mockSlackExecutor }));
actionType = actionTypeRegistry.get(ACTION_TYPE_ID);
Expand Down
58 changes: 30 additions & 28 deletions x-pack/legacy/plugins/actions/server/create_fire_function.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { SavedObjectsClientMock } from '../../../../../src/core/server/mocks';

const mockTaskManager = taskManagerMock.create();
const savedObjectsClient = SavedObjectsClientMock.create();
const spaceIdToNamespace = jest.fn();

beforeEach(() => jest.resetAllMocks());

Expand All @@ -18,6 +19,7 @@ describe('fire()', () => {
const fireFn = createFireFunction({
taskManager: mockTaskManager,
internalSavedObjectsRepository: savedObjectsClient,
spaceIdToNamespace,
});
savedObjectsClient.get.mockResolvedValueOnce({
id: '123',
Expand All @@ -27,41 +29,41 @@ describe('fire()', () => {
},
references: [],
});
spaceIdToNamespace.mockReturnValueOnce('namespace1');
await fireFn({
id: '123',
params: { baz: false },
namespace: 'abc',
basePath: '/s/default',
spaceId: 'default',
});
expect(mockTaskManager.schedule).toHaveBeenCalledTimes(1);
expect(mockTaskManager.schedule.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Object {
"params": Object {
"basePath": "/s/default",
"id": "123",
"namespace": "abc",
"params": Object {
"baz": false,
},
},
"scope": Array [
"actions",
],
"state": Object {},
"taskType": "actions:mock-action",
},
]
`);
Array [
Object {
"params": Object {
"id": "123",
"params": Object {
"baz": false,
},
"spaceId": "default",
},
"scope": Array [
"actions",
],
"state": Object {},
"taskType": "actions:mock-action",
},
]
`);
expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
expect(savedObjectsClient.get.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"action",
"123",
Object {
"namespace": "abc",
},
]
`);
Array [
"action",
"123",
Object {
"namespace": "namespace1",
},
]
`);
expect(spaceIdToNamespace).toHaveBeenCalledWith('default');
});
});
14 changes: 8 additions & 6 deletions x-pack/legacy/plugins/actions/server/create_fire_function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,33 @@

import { SavedObjectsClientContract } from 'src/core/server';
import { TaskManager } from '../../task_manager';
import { SpacesPlugin } from '../../spaces';

interface CreateFireFunctionOptions {
taskManager: TaskManager;
internalSavedObjectsRepository: SavedObjectsClientContract;
spaceIdToNamespace: SpacesPlugin['spaceIdToNamespace'];
}

interface FireOptions {
export interface FireOptions {
id: string;
params: Record<string, any>;
namespace?: string;
basePath: string;
spaceId: string;
}

export function createFireFunction({
taskManager,
internalSavedObjectsRepository,
spaceIdToNamespace,
}: CreateFireFunctionOptions) {
return async function fire({ id, params, namespace, basePath }: FireOptions) {
return async function fire({ id, params, spaceId }: FireOptions) {
const namespace = spaceIdToNamespace(spaceId);
const actionSavedObject = await internalSavedObjectsRepository.get('action', id, { namespace });
await taskManager.schedule({
taskType: `actions:${actionSavedObject.attributes.actionTypeId}`,
params: {
id,
basePath,
namespace,
spaceId,
params,
},
state: {},
Expand Down
22 changes: 21 additions & 1 deletion x-pack/legacy/plugins/actions/server/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,19 @@ import {
listActionTypesRoute,
fireRoute,
} from './routes';

import { registerBuiltInActionTypes } from './builtin_action_types';
import { SpacesPlugin } from '../../spaces';
import { createOptionalPlugin } from '../../../server/lib/optional_plugin';

export function init(server: Legacy.Server) {
const config = server.config();
const spaces = createOptionalPlugin<SpacesPlugin>(
config,
'xpack.spaces',
server.plugins,
'spaces'
);

const { callWithInternalUser } = server.plugins.elasticsearch.getCluster('admin');
const savedObjectsRepositoryWithInternalUser = server.savedObjects.getSavedObjectsRepository(
callWithInternalUser
Expand Down Expand Up @@ -58,6 +67,14 @@ export function init(server: Legacy.Server) {
getServices,
taskManager: taskManager!,
encryptedSavedObjectsPlugin: server.plugins.encrypted_saved_objects!,
getBasePath(...args) {
return spaces.isEnabled
? spaces.getBasePath(...args)
: server.config().get('server.basePath');
},
spaceIdToNamespace(...args) {
return spaces.isEnabled ? spaces.spaceIdToNamespace(...args) : undefined;
},
});

registerBuiltInActionTypes(actionTypeRegistry);
Expand All @@ -78,6 +95,9 @@ export function init(server: Legacy.Server) {
const fireFn = createFireFunction({
taskManager: taskManager!,
internalSavedObjectsRepository: savedObjectsRepositoryWithInternalUser,
spaceIdToNamespace(...args) {
return spaces.isEnabled ? spaces.spaceIdToNamespace(...args) : undefined;
},
});

// Expose functions to server
Expand Down
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/actions/server/lib/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { EncryptedSavedObjectsPlugin } from '../../../encrypted_saved_objects';

interface ExecuteOptions {
actionId: string;
namespace: string;
namespace?: string;
services: Services;
params: Record<string, any>;
encryptedSavedObjectsPlugin: EncryptedSavedObjectsPlugin;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { SavedObjectsClientMock } from '../../../../../../src/core/server/mocks'
import { actionTypeRegistryMock } from '../action_type_registry.mock';
import { ExecutorError } from './executor_error';

const spaceIdToNamespace = jest.fn();
const actionTypeRegistry = actionTypeRegistryMock.create();
const mockedEncryptedSavedObjectsPlugin = encryptedSavedObjectsMock.create();

Expand All @@ -34,7 +35,9 @@ const getCreateTaskRunnerFunctionParams = {
};
},
actionTypeRegistry,
spaceIdToNamespace,
encryptedSavedObjectsPlugin: mockedEncryptedSavedObjectsPlugin,
getBasePath: jest.fn().mockReturnValue(undefined),
};

const taskInstanceMock = {
Expand All @@ -43,7 +46,7 @@ const taskInstanceMock = {
params: {
id: '2',
params: { baz: true },
namespace: 'test',
spaceId: 'test',
},
taskType: 'actions:1',
};
Expand All @@ -56,12 +59,14 @@ test('executes the task by calling the executor with proper parameters', async (
const runner = createTaskRunner({ taskInstance: taskInstanceMock });

mockExecute.mockResolvedValueOnce({ status: 'ok' });
spaceIdToNamespace.mockReturnValueOnce('namespace-test');

const runnerResult = await runner.run();

expect(runnerResult).toBeUndefined();
expect(spaceIdToNamespace).toHaveBeenCalledWith('test');
expect(mockExecute).toHaveBeenCalledWith({
namespace: 'test',
namespace: 'namespace-test',
actionId: '2',
actionTypeRegistry,
encryptedSavedObjectsPlugin: mockedEncryptedSavedObjectsPlugin,
Expand Down
Loading

0 comments on commit a0961f2

Please sign in to comment.