Skip to content

Commit

Permalink
[Alerts] Replaces legacy es client with the ElasticsearchClient for a…
Browse files Browse the repository at this point in the history
…lerts and triggers_actions_ui plugins. (#93364)

* [Alerts] Replaces legasy es client with the ElasticsearchClient

* fixed build

* fixed build

* fixed ci build

* fixed ci build

* fixed infra callCLuster

* fixed infra callCLuster

* fixed infra callCLuster

* fixed ci build

* fixed ci build

* fixed ci build

* fixed infra tests

* fixed security tests

* fixed security tests

* fixed security tests

* fixed tests

* fixed monitoring unit tests

* fixed monitoring unit tests

* fixed type checks

* fixed type checks

* fixed type checks

* migrated lists plugin

* fixed type checks

* fixed tests

* fixed security tests

* fixed type checks

* fixed tests

* fixed type checks

* fixed tests

* fixed tests

* fixed tests

* fixed due to comments

* fixed tests

* fixed comment

* fixed tests

* fixed tests

* fixed searh

* fixed searh

* fixed test

* fixed due to comment

* fixed detections failing test and replaces scopedClusterClient exposure with IScopedClusterClient instead of ElasticsearchClient asCurrentUser

* fixed test

* fixed test

* fixed test

* fixed typecheck

* fixed typecheck

* fixed typecheck

* fixed merge
  • Loading branch information
YulNaumenko authored Mar 16, 2021
1 parent ec41ae3 commit 21587dc
Show file tree
Hide file tree
Showing 191 changed files with 2,025 additions and 1,275 deletions.
3 changes: 1 addition & 2 deletions x-pack/plugins/alerting/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,8 @@ This is the primary function for an alert type. Whenever the alert needs to exec

|Property|Description|
|---|---|
|services.callCluster(path, opts)|Use this to do Elasticsearch queries on the cluster Kibana connects to. This function is the same as any other `callCluster` in Kibana but in the context of the user who created the alert when security is enabled.|
|services.scopedClusterClient|This is an instance of the Elasticsearch client. Use this to do Elasticsearch queries in the context of the user who created the alert when security is enabled.|
|services.savedObjectsClient|This is an instance of the saved objects client. This provides the ability to do CRUD on any saved objects within the same space the alert lives in.<br><br>The scope of the saved objects client is tied to the user who created the alert (only when security isenabled).|
|services.getLegacyScopedClusterClient|This function returns an instance of the LegacyScopedClusterClient scoped to the user who created the alert when security is enabled.|
|services.alertInstanceFactory(id)|This [alert instance factory](#alert-instance-factory) creates instances of alerts and must be used in order to execute actions. The id you give to the alert instance factory is a unique identifier to the alert instance.|
|services.log(tags, [data], [timestamp])|Use this to create server logs. (This is the same function as server.log)|
|startedAt|The date and time the alert type started execution.|
Expand Down
4 changes: 1 addition & 3 deletions x-pack/plugins/alerting/server/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,8 @@ const createAlertServicesMock = <
alertInstanceFactory: jest
.fn<jest.Mocked<AlertInstance<InstanceState, InstanceContext>>, [string]>()
.mockReturnValue(alertInstanceFactoryMock),
callCluster: elasticsearchServiceMock.createLegacyScopedClusterClient().callAsCurrentUser,
getLegacyScopedClusterClient: jest.fn(),
savedObjectsClient: savedObjectsClientMock.create(),
scopedClusterClient: elasticsearchServiceMock.createScopedClusterClient().asCurrentUser,
scopedClusterClient: elasticsearchServiceMock.createScopedClusterClient(),
};
};
export type AlertServicesMock = ReturnType<typeof createAlertServicesMock>;
Expand Down
7 changes: 1 addition & 6 deletions x-pack/plugins/alerting/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {
SavedObjectsServiceStart,
IContextProvider,
ElasticsearchServiceStart,
ILegacyClusterClient,
StatusServiceSetup,
ServiceStatus,
SavedObjectsBulkGetObject,
Expand Down Expand Up @@ -420,12 +419,8 @@ export class AlertingPlugin {
elasticsearch: ElasticsearchServiceStart
): (request: KibanaRequest) => Services {
return (request) => ({
callCluster: elasticsearch.legacy.client.asScoped(request).callAsCurrentUser,
savedObjectsClient: this.getScopedClientWithAlertSavedObjectType(savedObjects, request),
scopedClusterClient: elasticsearch.client.asScoped(request).asCurrentUser,
getLegacyScopedClusterClient(clusterClient: ILegacyClusterClient) {
return clusterClient.asScoped(request);
},
scopedClusterClient: elasticsearch.client.asScoped(request),
});
}

Expand Down
10 changes: 6 additions & 4 deletions x-pack/plugins/alerting/server/routes/_mock_handler_arguments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
* 2.0.
*/

import { KibanaRequest, KibanaResponseFactory, ILegacyClusterClient } from 'kibana/server';
import { KibanaRequest, KibanaResponseFactory } from 'kibana/server';
import { identity } from 'lodash';
import type { MethodKeysOf } from '@kbn/utility-types';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { ScopedClusterClientMock } from '../../../../../src/core/server/elasticsearch/client/mocks';
import { httpServerMock } from '../../../../../src/core/server/mocks';
import { alertsClientMock, AlertsClientMock } from '../alerts_client.mock';
import { AlertsHealth, AlertType } from '../../common';
Expand All @@ -18,12 +20,12 @@ export function mockHandlerArguments(
{
alertsClient = alertsClientMock.create(),
listTypes: listTypesRes = [],
esClient = elasticsearchServiceMock.createLegacyClusterClient(),
esClient = elasticsearchServiceMock.createScopedClusterClient(),
getFrameworkHealth,
}: {
alertsClient?: AlertsClientMock;
listTypes?: AlertType[];
esClient?: jest.Mocked<ILegacyClusterClient>;
esClient?: jest.Mocked<ScopedClusterClientMock>;
getFrameworkHealth?: jest.MockInstance<Promise<AlertsHealth>, []> &
(() => Promise<AlertsHealth>);
},
Expand All @@ -37,7 +39,7 @@ export function mockHandlerArguments(
const listTypes = jest.fn(() => listTypesRes);
return [
({
core: { elasticsearch: { legacy: { client: esClient } } },
core: { elasticsearch: { client: esClient } },
alerting: {
listTypes,
getAlertsClient() {
Expand Down
51 changes: 33 additions & 18 deletions x-pack/plugins/alerting/server/routes/health.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/serv
import { alertsClientMock } from '../alerts_client.mock';
import { HealthStatus } from '../types';
import { alertsMock } from '../mocks';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { elasticsearchClientMock } from '../../../../../src/core/server/elasticsearch/client/mocks';
const alertsClient = alertsClientMock.create();

jest.mock('../lib/license_api_access.ts', () => ({
Expand Down Expand Up @@ -63,18 +65,19 @@ describe('healthRoute', () => {
healthRoute(router, licenseState, encryptedSavedObjects);
const [, handler] = router.get.mock.calls[0];

const esClient = elasticsearchServiceMock.createLegacyClusterClient();
esClient.callAsInternalUser.mockReturnValue(Promise.resolve({}));
const esClient = elasticsearchServiceMock.createScopedClusterClient();
esClient.asInternalUser.transport.request.mockReturnValue(
elasticsearchClientMock.createSuccessTransportRequestPromise({})
);

const [context, req, res] = mockHandlerArguments({ esClient, alertsClient }, {}, ['ok']);

await handler(context, req, res);

expect(verifyApiAccess).toHaveBeenCalledWith(licenseState);

expect(esClient.callAsInternalUser.mock.calls[0]).toMatchInlineSnapshot(`
expect(esClient.asInternalUser.transport.request.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"transport.request",
Object {
"method": "GET",
"path": "/_xpack/usage",
Expand All @@ -91,8 +94,10 @@ describe('healthRoute', () => {
healthRoute(router, licenseState, encryptedSavedObjects);
const [, handler] = router.get.mock.calls[0];

const esClient = elasticsearchServiceMock.createLegacyClusterClient();
esClient.callAsInternalUser.mockReturnValue(Promise.resolve({}));
const esClient = elasticsearchServiceMock.createScopedClusterClient();
esClient.asInternalUser.transport.request.mockReturnValue(
elasticsearchClientMock.createSuccessTransportRequestPromise({})
);

const [context, req, res] = mockHandlerArguments(
{ esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
Expand Down Expand Up @@ -130,8 +135,10 @@ describe('healthRoute', () => {
healthRoute(router, licenseState, encryptedSavedObjects);
const [, handler] = router.get.mock.calls[0];

const esClient = elasticsearchServiceMock.createLegacyClusterClient();
esClient.callAsInternalUser.mockReturnValue(Promise.resolve({}));
const esClient = elasticsearchServiceMock.createScopedClusterClient();
esClient.asInternalUser.transport.request.mockReturnValue(
elasticsearchClientMock.createSuccessTransportRequestPromise({})
);

const [context, req, res] = mockHandlerArguments(
{ esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
Expand Down Expand Up @@ -169,8 +176,10 @@ describe('healthRoute', () => {
healthRoute(router, licenseState, encryptedSavedObjects);
const [, handler] = router.get.mock.calls[0];

const esClient = elasticsearchServiceMock.createLegacyClusterClient();
esClient.callAsInternalUser.mockReturnValue(Promise.resolve({ security: {} }));
const esClient = elasticsearchServiceMock.createScopedClusterClient();
esClient.asInternalUser.transport.request.mockReturnValue(
elasticsearchClientMock.createSuccessTransportRequestPromise({ security: {} })
);

const [context, req, res] = mockHandlerArguments(
{ esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
Expand Down Expand Up @@ -208,8 +217,10 @@ describe('healthRoute', () => {
healthRoute(router, licenseState, encryptedSavedObjects);
const [, handler] = router.get.mock.calls[0];

const esClient = elasticsearchServiceMock.createLegacyClusterClient();
esClient.callAsInternalUser.mockReturnValue(Promise.resolve({ security: { enabled: true } }));
const esClient = elasticsearchServiceMock.createScopedClusterClient();
esClient.asInternalUser.transport.request.mockReturnValue(
elasticsearchClientMock.createSuccessTransportRequestPromise({ security: { enabled: true } })
);

const [context, req, res] = mockHandlerArguments(
{ esClient, alertsClient, getFrameworkHealth: alerting.getFrameworkHealth },
Expand Down Expand Up @@ -247,9 +258,11 @@ describe('healthRoute', () => {
healthRoute(router, licenseState, encryptedSavedObjects);
const [, handler] = router.get.mock.calls[0];

const esClient = elasticsearchServiceMock.createLegacyClusterClient();
esClient.callAsInternalUser.mockReturnValue(
Promise.resolve({ security: { enabled: true, ssl: {} } })
const esClient = elasticsearchServiceMock.createScopedClusterClient();
esClient.asInternalUser.transport.request.mockReturnValue(
elasticsearchClientMock.createSuccessTransportRequestPromise({
security: { enabled: true, ssl: {} },
})
);

const [context, req, res] = mockHandlerArguments(
Expand Down Expand Up @@ -288,9 +301,11 @@ describe('healthRoute', () => {
healthRoute(router, licenseState, encryptedSavedObjects);
const [, handler] = router.get.mock.calls[0];

const esClient = elasticsearchServiceMock.createLegacyClusterClient();
esClient.callAsInternalUser.mockReturnValue(
Promise.resolve({ security: { enabled: true, ssl: { http: { enabled: true } } } })
const esClient = elasticsearchServiceMock.createScopedClusterClient();
esClient.asInternalUser.transport.request.mockReturnValue(
elasticsearchClientMock.createSuccessTransportRequestPromise({
security: { enabled: true, ssl: { http: { enabled: true } } },
})
);

const [context, req, res] = mockHandlerArguments(
Expand Down
17 changes: 9 additions & 8 deletions x-pack/plugins/alerting/server/routes/health.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { ApiResponse } from '@elastic/elasticsearch';
import type { AlertingRouter } from '../types';
import { ILicenseState } from '../lib/license_state';
import { verifyApiAccess } from '../lib/license_api_access';
Expand Down Expand Up @@ -39,14 +40,14 @@ export function healthRoute(
}
try {
const {
security: {
enabled: isSecurityEnabled = false,
ssl: { http: { enabled: isTLSEnabled = false } = {} } = {},
} = {},
}: XPackUsageSecurity = await context.core.elasticsearch.legacy.client
// `transport.request` is potentially unsafe when combined with untrusted user input.
// Do not augment with such input.
.callAsInternalUser('transport.request', {
body: {
security: {
enabled: isSecurityEnabled = false,
ssl: { http: { enabled: isTLSEnabled = false } = {} } = {},
} = {},
},
}: ApiResponse<XPackUsageSecurity> = await context.core.elasticsearch.client.asInternalUser.transport // Do not augment with such input. // `transport.request` is potentially unsafe when combined with untrusted user input.
.request({
method: 'GET',
path: '/_xpack/usage',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ describe('Task Runner', () => {
expect(call.createdBy).toBe('alert-creator');
expect(call.updatedBy).toBe('alert-updater');
expect(call.services.alertInstanceFactory).toBeTruthy();
expect(call.services.callCluster).toBeTruthy();
expect(call.services.scopedClusterClient).toBeTruthy();
expect(call.services).toBeTruthy();

const logger = taskRunnerFactoryInitializerParams.logger;
Expand Down
11 changes: 2 additions & 9 deletions x-pack/plugins/alerting/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ import { PluginSetupContract, PluginStartContract } from './plugin';
import { AlertsClient } from './alerts_client';
export * from '../common';
import {
ElasticsearchClient,
ILegacyClusterClient,
ILegacyScopedClusterClient,
IScopedClusterClient,
KibanaRequest,
SavedObjectAttributes,
SavedObjectsClientContract,
Expand Down Expand Up @@ -63,13 +61,8 @@ export interface AlertingRequestHandlerContext extends RequestHandlerContext {
export type AlertingRouter = IRouter<AlertingRequestHandlerContext>;

export interface Services {
/**
* @deprecated Use `scopedClusterClient` instead.
*/
callCluster: ILegacyScopedClusterClient['callAsCurrentUser'];
savedObjectsClient: SavedObjectsClientContract;
scopedClusterClient: ElasticsearchClient;
getLegacyScopedClusterClient(clusterClient: ILegacyClusterClient): ILegacyScopedClusterClient;
scopedClusterClient: IScopedClusterClient;
}

export interface AlertServices<
Expand Down
28 changes: 16 additions & 12 deletions x-pack/plugins/alerting/server/usage/alerts_telemetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,31 @@
* 2.0.
*/

// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { elasticsearchClientMock } from '../../../../../src/core/server/elasticsearch/client/mocks';
import { getTotalCountInUse } from './alerts_telemetry';

describe('alerts telemetry', () => {
test('getTotalCountInUse should replace first "." symbol to "__" in alert types names', async () => {
const mockEsClient = jest.fn();
mockEsClient.mockReturnValue({
aggregations: {
byAlertTypeId: {
value: {
types: { '.index-threshold': 2, 'logs.alert.document.count': 1, 'document.test.': 1 },
const mockEsClient = elasticsearchClientMock.createClusterClient().asScoped().asInternalUser;
mockEsClient.search.mockReturnValue(
elasticsearchClientMock.createSuccessTransportRequestPromise({
aggregations: {
byAlertTypeId: {
value: {
types: { '.index-threshold': 2, 'logs.alert.document.count': 1, 'document.test.': 1 },
},
},
},
},
hits: {
hits: [],
},
});
hits: {
hits: [],
},
})
);

const telemetry = await getTotalCountInUse(mockEsClient, 'test');

expect(mockEsClient).toHaveBeenCalledTimes(1);
expect(mockEsClient.search).toHaveBeenCalledTimes(1);

expect(telemetry).toMatchInlineSnapshot(`
Object {
Expand Down
15 changes: 7 additions & 8 deletions x-pack/plugins/alerting/server/usage/alerts_telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
* 2.0.
*/

import { LegacyAPICaller } from 'kibana/server';
import { SearchResponse } from 'elasticsearch';
import { ElasticsearchClient } from 'kibana/server';
import { AlertsUsage } from './types';

const alertTypeMetric = {
Expand Down Expand Up @@ -36,7 +35,7 @@ const alertTypeMetric = {
};

export async function getTotalCountAggregations(
callCluster: LegacyAPICaller,
esClient: ElasticsearchClient,
kibanaInex: string
): Promise<
Pick<
Expand Down Expand Up @@ -223,7 +222,7 @@ export async function getTotalCountAggregations(
},
};

const results = await callCluster('search', {
const { body: results } = await esClient.search({
index: kibanaInex,
body: {
query: {
Expand Down Expand Up @@ -256,7 +255,7 @@ export async function getTotalCountAggregations(
return {
count_total: totalAlertsCount,
count_by_type: Object.keys(results.aggregations.byAlertTypeId.value.types).reduce(
// ES DSL aggregations are returned as `any` by callCluster
// ES DSL aggregations are returned as `any` by esClient.search
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(obj: any, key: string) => ({
...obj,
Expand Down Expand Up @@ -295,8 +294,8 @@ export async function getTotalCountAggregations(
};
}

export async function getTotalCountInUse(callCluster: LegacyAPICaller, kibanaInex: string) {
const searchResult: SearchResponse<unknown> = await callCluster('search', {
export async function getTotalCountInUse(esClient: ElasticsearchClient, kibanaInex: string) {
const { body: searchResult } = await esClient.search({
index: kibanaInex,
body: {
query: {
Expand All @@ -316,7 +315,7 @@ export async function getTotalCountInUse(callCluster: LegacyAPICaller, kibanaIne
0
),
countByType: Object.keys(searchResult.aggregations.byAlertTypeId.value.types).reduce(
// ES DSL aggregations are returned as `any` by callCluster
// ES DSL aggregations are returned as `any` by esClient.search
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(obj: any, key: string) => ({
...obj,
Expand Down
Loading

0 comments on commit 21587dc

Please sign in to comment.