Return referenced by count with each action on the find API (#49104)

* Initial work

* .kibana index configurable, NP ready implementation

* Fix broken jest tests

* Fix broken functional tests

* Add functional test

* Cleanup actions_client.test.ts
This commit is contained in:
Mike Côté 2019-11-01 15:15:54 -04:00 committed by GitHub
parent bfdf61714e
commit 2b6bd2f01e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 220 additions and 61 deletions

View file

@ -11,9 +11,14 @@ import { ActionsClient } from './actions_client';
import { ExecutorType } from './types';
import { ActionExecutor, TaskRunnerFactory } from './lib';
import { taskManagerMock } from '../../task_manager/task_manager.mock';
import { savedObjectsClientMock } from '../../../../../src/core/server/mocks';
import {
elasticsearchServiceMock,
savedObjectsClientMock,
} from '../../../../../src/core/server/mocks';
const defaultKibanaIndex = '.kibana';
const savedObjectsClient = savedObjectsClientMock.create();
const scopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
const mockTaskManager = taskManagerMock.create();
@ -22,11 +27,22 @@ const actionTypeRegistryParams = {
taskRunnerFactory: new TaskRunnerFactory(new ActionExecutor()),
};
let actionsClient: ActionsClient;
let actionTypeRegistry: ActionTypeRegistry;
const executor: ExecutorType = async options => {
return { status: 'ok' };
};
beforeEach(() => jest.resetAllMocks());
beforeEach(() => {
jest.resetAllMocks();
actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
actionsClient = new ActionsClient({
actionTypeRegistry,
savedObjectsClient,
scopedClusterClient,
defaultKibanaIndex,
});
});
describe('create()', () => {
test('creates an action with all given properties', async () => {
@ -40,16 +56,11 @@ describe('create()', () => {
},
references: [],
};
const actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
actionTypeRegistry.register({
id: 'my-action-type',
name: 'My action type',
executor,
});
const actionsClient = new ActionsClient({
actionTypeRegistry,
savedObjectsClient,
});
savedObjectsClient.create.mockResolvedValueOnce(savedObjectCreateResult);
const result = await actionsClient.create({
action: {
@ -80,11 +91,6 @@ describe('create()', () => {
});
test('validates config', async () => {
const actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
const actionsClient = new ActionsClient({
actionTypeRegistry,
savedObjectsClient,
});
actionTypeRegistry.register({
id: 'my-action-type',
name: 'My action type',
@ -110,11 +116,6 @@ describe('create()', () => {
});
test(`throws an error when an action type doesn't exist`, async () => {
const actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
const actionsClient = new ActionsClient({
actionTypeRegistry,
savedObjectsClient,
});
await expect(
actionsClient.create({
action: {
@ -130,16 +131,11 @@ describe('create()', () => {
});
test('encrypts action type options unless specified not to', async () => {
const actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
actionTypeRegistry.register({
id: 'my-action-type',
name: 'My action type',
executor,
});
const actionsClient = new ActionsClient({
actionTypeRegistry,
savedObjectsClient,
});
savedObjectsClient.create.mockResolvedValueOnce({
id: '1',
type: 'type',
@ -198,11 +194,6 @@ describe('create()', () => {
describe('get()', () => {
test('calls savedObjectsClient with id', async () => {
const actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
const actionsClient = new ActionsClient({
actionTypeRegistry,
savedObjectsClient,
});
savedObjectsClient.get.mockResolvedValueOnce({
id: '1',
type: 'type',
@ -242,12 +233,12 @@ describe('find()', () => {
},
],
};
const actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
const actionsClient = new ActionsClient({
actionTypeRegistry,
savedObjectsClient,
});
savedObjectsClient.find.mockResolvedValueOnce(expectedResult);
scopedClusterClient.callAsInternalUser.mockResolvedValueOnce({
aggregations: {
'1': { doc_count: 6 },
},
});
const result = await actionsClient.find({});
expect(result).toEqual({
total: 1,
@ -259,6 +250,7 @@ describe('find()', () => {
config: {
foo: 'bar',
},
referencedByCount: 6,
},
],
});
@ -276,11 +268,6 @@ describe('find()', () => {
describe('delete()', () => {
test('calls savedObjectsClient with id', async () => {
const expectedResult = Symbol();
const actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
const actionsClient = new ActionsClient({
actionTypeRegistry,
savedObjectsClient,
});
savedObjectsClient.delete.mockResolvedValueOnce(expectedResult);
const result = await actionsClient.delete({ id: '1' });
expect(result).toEqual(expectedResult);
@ -296,16 +283,11 @@ describe('delete()', () => {
describe('update()', () => {
test('updates an action with all given properties', async () => {
const actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
actionTypeRegistry.register({
id: 'my-action-type',
name: 'My action type',
executor,
});
const actionsClient = new ActionsClient({
actionTypeRegistry,
savedObjectsClient,
});
savedObjectsClient.get.mockResolvedValueOnce({
id: '1',
type: 'action',
@ -362,11 +344,6 @@ describe('update()', () => {
});
test('validates config', async () => {
const actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
const actionsClient = new ActionsClient({
actionTypeRegistry,
savedObjectsClient,
});
actionTypeRegistry.register({
id: 'my-action-type',
name: 'My action type',
@ -400,16 +377,11 @@ describe('update()', () => {
});
test('encrypts action type options unless specified not to', async () => {
const actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
actionTypeRegistry.register({
id: 'my-action-type',
name: 'My action type',
executor,
});
const actionsClient = new ActionsClient({
actionTypeRegistry,
savedObjectsClient,
});
savedObjectsClient.get.mockResolvedValueOnce({
id: 'my-action',
type: 'action',

View file

@ -4,10 +4,16 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { SavedObjectsClientContract, SavedObjectAttributes, SavedObject } from 'src/core/server';
import {
IScopedClusterClient,
SavedObjectsClientContract,
SavedObjectAttributes,
SavedObject,
} from 'src/core/server';
import { ActionTypeRegistry } from './action_type_registry';
import { validateConfig, validateSecrets } from './lib';
import { ActionResult } from './types';
import { ActionResult, FindActionResult, RawAction } from './types';
interface ActionUpdate extends SavedObjectAttributes {
description: string;
@ -44,10 +50,12 @@ interface FindResult {
page: number;
perPage: number;
total: number;
data: ActionResult[];
data: FindActionResult[];
}
interface ConstructorOptions {
defaultKibanaIndex: string;
scopedClusterClient: IScopedClusterClient;
actionTypeRegistry: ActionTypeRegistry;
savedObjectsClient: SavedObjectsClientContract;
}
@ -58,12 +66,21 @@ interface UpdateOptions {
}
export class ActionsClient {
private readonly defaultKibanaIndex: string;
private readonly scopedClusterClient: IScopedClusterClient;
private readonly savedObjectsClient: SavedObjectsClientContract;
private readonly actionTypeRegistry: ActionTypeRegistry;
constructor({ actionTypeRegistry, savedObjectsClient }: ConstructorOptions) {
constructor({
actionTypeRegistry,
defaultKibanaIndex,
scopedClusterClient,
savedObjectsClient,
}: ConstructorOptions) {
this.actionTypeRegistry = actionTypeRegistry;
this.savedObjectsClient = savedObjectsClient;
this.scopedClusterClient = scopedClusterClient;
this.defaultKibanaIndex = defaultKibanaIndex;
}
/**
@ -134,16 +151,22 @@ export class ActionsClient {
* Find actions
*/
public async find({ options = {} }: FindOptions): Promise<FindResult> {
const findResult = await this.savedObjectsClient.find({
const findResult = await this.savedObjectsClient.find<RawAction>({
...options,
type: 'action',
});
const data = await injectExtraFindData(
this.defaultKibanaIndex,
this.scopedClusterClient,
findResult.saved_objects.map(actionFromSavedObject)
);
return {
page: findResult.page,
perPage: findResult.per_page,
total: findResult.total,
data: findResult.saved_objects.map(actionFromSavedObject),
data,
};
}
@ -155,9 +178,64 @@ export class ActionsClient {
}
}
function actionFromSavedObject(savedObject: SavedObject) {
function actionFromSavedObject(savedObject: SavedObject<RawAction>): ActionResult {
return {
id: savedObject.id,
...savedObject.attributes,
};
}
async function injectExtraFindData(
defaultKibanaIndex: string,
scopedClusterClient: IScopedClusterClient,
actionResults: ActionResult[]
): Promise<FindActionResult[]> {
const aggs: Record<string, any> = {};
for (const actionResult of actionResults) {
aggs[actionResult.id] = {
filter: {
bool: {
must: {
nested: {
path: 'references',
query: {
bool: {
filter: {
bool: {
must: [
{
term: {
'references.id': actionResult.id,
},
},
{
term: {
'references.type': 'action',
},
},
],
},
},
},
},
},
},
},
},
};
}
const aggregationResult = await scopedClusterClient.callAsInternalUser('search', {
index: defaultKibanaIndex,
body: {
aggs,
size: 0,
query: {
match_all: {},
},
},
});
return actionResults.map(actionResult => ({
...actionResult,
referencedByCount: aggregationResult.aggregations[actionResult.id].doc_count,
}));
}

View file

@ -22,6 +22,7 @@ import {
ActionsCoreStart,
ActionsPluginsSetup,
ActionsPluginsStart,
KibanaConfig,
} from './shim';
import {
createActionRoute,
@ -44,6 +45,7 @@ export interface PluginStartContract {
}
export class Plugin {
private readonly kibana$: Observable<KibanaConfig>;
private readonly config$: Observable<ActionsConfigType>;
private readonly logger: Logger;
private serverBasePath?: string;
@ -51,10 +53,12 @@ export class Plugin {
private taskRunnerFactory?: TaskRunnerFactory;
private actionTypeRegistry?: ActionTypeRegistry;
private actionExecutor?: ActionExecutor;
private defaultKibanaIndex?: string;
constructor(initializerContext: ActionsPluginInitializerContext) {
this.logger = initializerContext.logger.get('plugins', 'alerting');
this.config$ = initializerContext.config.create();
this.kibana$ = initializerContext.config.kibana$;
}
public async setup(
@ -63,6 +67,7 @@ export class Plugin {
): Promise<PluginSetupContract> {
const config = await this.config$.pipe(first()).toPromise();
this.adminClient = await core.elasticsearch.adminClient$.pipe(first()).toPromise();
this.defaultKibanaIndex = (await this.kibana$.pipe(first()).toPromise()).index;
plugins.xpack_main.registerFeature({
id: 'actions',
@ -141,6 +146,7 @@ export class Plugin {
adminClient,
serverBasePath,
taskRunnerFactory,
defaultKibanaIndex,
} = this;
function getServices(request: any): Services {
@ -186,6 +192,8 @@ export class Plugin {
return new ActionsClient({
savedObjectsClient,
actionTypeRegistry: actionTypeRegistry!,
defaultKibanaIndex: defaultKibanaIndex!,
scopedClusterClient: adminClient!.asScoped(request),
});
},
};

View file

@ -31,6 +31,10 @@ export interface Server extends Legacy.Server {
plugins: Plugins;
}
export interface KibanaConfig {
index: string;
}
/**
* Shim what we're thinking setup and start contracts will look like
*/
@ -54,6 +58,7 @@ export type EncryptedSavedObjectsStartContract = Pick<
export interface ActionsPluginInitializerContext {
logger: LoggerFactory;
config: {
kibana$: Rx.Observable<KibanaConfig>;
create(): Rx.Observable<ActionsConfigType>;
};
}
@ -101,6 +106,9 @@ export function shim(
const initializerContext: ActionsPluginInitializerContext = {
logger: newPlatform.coreContext.logger,
config: {
kibana$: Rx.of({
index: server.config().get('kibana.index'),
}),
create() {
return Rx.of({
enabled: server.config().get('xpack.actions.enabled') as boolean,

View file

@ -45,6 +45,10 @@ export interface ActionResult {
config: Record<string, any>;
}
export interface FindActionResult extends ActionResult {
referencedByCount: number;
}
// the result returned from an action type executor function
export interface ActionTypeExecutorResult {
status: 'ok' | 'error';

View file

@ -15,6 +15,13 @@ export default function(kibana: any) {
name: 'alerts',
init(server: any) {
// Action types
const noopActionType: ActionType = {
id: 'test.noop',
name: 'Test: Noop',
async executor() {
return { status: 'ok' };
},
};
const indexRecordActionType: ActionType = {
id: 'test.index-record',
name: 'Test: Index Record',
@ -158,6 +165,7 @@ export default function(kibana: any) {
};
},
};
server.plugins.actions.setup.registerType(noopActionType);
server.plugins.actions.setup.registerType(indexRecordActionType);
server.plugins.actions.setup.registerType(failingActionType);
server.plugins.actions.setup.registerType(rateLimitedActionType);
@ -311,7 +319,7 @@ export default function(kibana: any) {
const noopAlertType: AlertType = {
id: 'test.noop',
name: 'Test: Noop',
actionGroups: [],
actionGroups: ['default'],
async executor({ services, params, state }: AlertExecutorOptions) {},
};
server.plugins.alerting.setup.registerType(alwaysFiringAlertType);

View file

@ -6,7 +6,7 @@
import expect from '@kbn/expect';
import { UserAtSpaceScenarios } from '../../scenarios';
import { getUrlPrefix, ObjectRemover } from '../../../common/lib';
import { getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib';
import { FtrProviderContext } from '../../../common/ftr_provider_context';
// eslint-disable-next-line import/no-default-export
@ -73,6 +73,7 @@ export default function findActionTests({ getService }: FtrProviderContext) {
config: {
unencrypted: `This value shouldn't get encrypted`,
},
referencedByCount: 0,
},
],
});
@ -133,6 +134,85 @@ export default function findActionTests({ getService }: FtrProviderContext) {
config: {
unencrypted: `This value shouldn't get encrypted`,
},
referencedByCount: 0,
},
],
});
break;
default:
throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`);
}
});
it('should handle find request appropriately with proper referencedByCount', async () => {
const { body: createdAction } = await supertest
.post(`${getUrlPrefix(space.id)}/api/action`)
.set('kbn-xsrf', 'foo')
.send({
description: 'My action',
actionTypeId: 'test.index-record',
config: {
unencrypted: `This value shouldn't get encrypted`,
},
secrets: {
encrypted: 'This value should be encrypted',
},
})
.expect(200);
objectRemover.add(space.id, createdAction.id, 'action');
const { body: createdAlert } = await supertest
.post(`${getUrlPrefix(space.id)}/api/alert`)
.set('kbn-xsrf', 'foo')
.send(
getTestAlertData({
actions: [
{
group: 'default',
id: createdAction.id,
params: {},
},
],
})
)
.expect(200);
objectRemover.add(space.id, createdAlert.id, 'alert');
const response = await supertestWithoutAuth
.get(
`${getUrlPrefix(
space.id
)}/api/action/_find?filter=action.attributes.actionTypeId:test.index-record`
)
.auth(user.username, user.password);
switch (scenario.id) {
case 'no_kibana_privileges at space1':
case 'space_1_all at space2':
expect(response.statusCode).to.eql(404);
expect(response.body).to.eql({
statusCode: 404,
error: 'Not Found',
message: 'Not Found',
});
break;
case 'global_read at space1':
case 'superuser at space1':
case 'space_1_all at space1':
expect(response.statusCode).to.eql(200);
expect(response.body).to.eql({
page: 1,
perPage: 20,
total: 1,
data: [
{
id: createdAction.id,
description: 'My action',
actionTypeId: 'test.index-record',
config: {
unencrypted: `This value shouldn't get encrypted`,
},
referencedByCount: 1,
},
],
});

View file

@ -52,6 +52,7 @@ export default function findActionTests({ getService }: FtrProviderContext) {
config: {
unencrypted: `This value shouldn't get encrypted`,
},
referencedByCount: 0,
},
],
});