Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Alerts] Replaces legacy es client with the ElasticsearchClient for alerts and triggers_actions_ui plugins. #93364

Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
58a1e61
[Alerts] Replaces legasy es client with the ElasticsearchClient
YulNaumenko Mar 3, 2021
fe562d3
fixed build
YulNaumenko Mar 3, 2021
b411fbd
fixed build
YulNaumenko Mar 3, 2021
2e3b0ef
fixed ci build
YulNaumenko Mar 3, 2021
8881c3b
fixed ci build
YulNaumenko Mar 3, 2021
8e94717
fixed infra callCLuster
YulNaumenko Mar 3, 2021
2ecfc15
fixed infra callCLuster
YulNaumenko Mar 3, 2021
86f5d4b
fixed infra callCLuster
YulNaumenko Mar 5, 2021
29e64ef
fixed ci build
YulNaumenko Mar 5, 2021
c458fe5
Merge remote-tracking branch upstream/master
YulNaumenko Mar 5, 2021
4c9384b
fixed ci build
YulNaumenko Mar 6, 2021
3163f20
fixed ci build
YulNaumenko Mar 6, 2021
4242b1e
Merge remote-tracking branch upstream/master
YulNaumenko Mar 6, 2021
d40e90e
fixed infra tests
YulNaumenko Mar 6, 2021
176b8af
fixed security tests
YulNaumenko Mar 6, 2021
bec9e71
fixed security tests
YulNaumenko Mar 6, 2021
557c34c
fixed security tests
YulNaumenko Mar 6, 2021
07e6755
fixed tests
YulNaumenko Mar 7, 2021
585cca8
fixed monitoring unit tests
YulNaumenko Mar 7, 2021
a340c71
fixed monitoring unit tests
YulNaumenko Mar 7, 2021
4d748ab
fixed type checks
YulNaumenko Mar 7, 2021
a58b0b7
Merge remote-tracking branch upstream/master
YulNaumenko Mar 7, 2021
f416fa1
fixed type checks
YulNaumenko Mar 8, 2021
17377bf
fixed type checks
YulNaumenko Mar 8, 2021
3586481
migrated lists plugin
YulNaumenko Mar 8, 2021
55f1eaa
fixed type checks
YulNaumenko Mar 8, 2021
4cc2069
fixed tests
YulNaumenko Mar 8, 2021
2326e80
fixed security tests
YulNaumenko Mar 8, 2021
f9a9042
fixed type checks
YulNaumenko Mar 8, 2021
98d2fb4
Merge remote-tracking branch upstream/master
YulNaumenko Mar 8, 2021
633c230
Merge remote-tracking branch upstream/master
YulNaumenko Mar 8, 2021
b36a879
fixed tests
YulNaumenko Mar 9, 2021
3ffb5cf
fixed type checks
YulNaumenko Mar 9, 2021
4ae826e
fixed tests
YulNaumenko Mar 9, 2021
c87626c
fixed tests
YulNaumenko Mar 9, 2021
9a0c821
fixed tests
YulNaumenko Mar 9, 2021
f8a8a95
fixed due to comments
YulNaumenko Mar 9, 2021
9590624
fixed tests
YulNaumenko Mar 9, 2021
6ac3f7c
Merge remote-tracking branch upstream/master
YulNaumenko Mar 9, 2021
256538c
fixed comment
YulNaumenko Mar 9, 2021
482c206
fixed tests
YulNaumenko Mar 9, 2021
9ad926c
Merge remote-tracking branch upstream/master
YulNaumenko Mar 9, 2021
356fdc4
fixed tests
YulNaumenko Mar 9, 2021
8d52447
fixed searh
YulNaumenko Mar 10, 2021
a5743af
fixed searh
YulNaumenko Mar 10, 2021
3643065
Merge remote-tracking branch upstream/master
YulNaumenko Mar 11, 2021
a821b9a
fixed test
YulNaumenko Mar 11, 2021
694afbd
fixed due to comment
YulNaumenko Mar 11, 2021
3651f96
fixed detections failing test and replaces scopedClusterClient exposu…
YulNaumenko Mar 15, 2021
97eaaf8
Merge remote-tracking branch upstream/master
YulNaumenko Mar 15, 2021
e595e85
fixed test
YulNaumenko Mar 15, 2021
3acd943
fixed test
YulNaumenko Mar 16, 2021
c0ff617
fixed test
YulNaumenko Mar 16, 2021
c62ab0c
fixed typecheck
YulNaumenko Mar 16, 2021
04aeeff
fixed typecheck
YulNaumenko Mar 16, 2021
7da1ae0
Merge remote-tracking branch upstream/master
YulNaumenko Mar 16, 2021
51d6f30
fixed typecheck
YulNaumenko Mar 16, 2021
d7947f9
fixed merge
YulNaumenko Mar 16, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
2 changes: 0 additions & 2 deletions x-pack/plugins/alerting/server/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ 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,
};
Expand Down
5 changes: 0 additions & 5 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);
},
});
}

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
7 changes: 0 additions & 7 deletions x-pack/plugins/alerting/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import { AlertsClient } from './alerts_client';
export * from '../common';
import {
ElasticsearchClient,
ILegacyClusterClient,
ILegacyScopedClusterClient,
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;
}

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
18 changes: 11 additions & 7 deletions x-pack/plugins/alerting/server/usage/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { Logger, CoreSetup, LegacyAPICaller } from 'kibana/server';
import { Logger, CoreSetup } from 'kibana/server';
import moment from 'moment';
import {
RunContext,
Expand Down Expand Up @@ -65,17 +65,21 @@ async function scheduleTasks(logger: Logger, taskManager: TaskManagerStartContra
export function telemetryTaskRunner(logger: Logger, core: CoreSetup, kibanaIndex: string) {
return ({ taskInstance }: RunContext) => {
const { state } = taskInstance;
const callCluster = (...args: Parameters<LegacyAPICaller>) => {
return core.getStartServices().then(([{ elasticsearch: { legacy: { client } } }]) =>
client.callAsInternalUser(...args)
const getEsClient = () =>
core.getStartServices().then(
([
{
elasticsearch: { client },
},
]) => client.asInternalUser
);
};

return {
async run() {
const esClient = await getEsClient();
return Promise.all([
getTotalCountAggregations(callCluster, kibanaIndex),
getTotalCountInUse(callCluster, kibanaIndex),
getTotalCountAggregations(esClient, kibanaIndex),
getTotalCountInUse(esClient, kibanaIndex),
])
.then(([totalCountAggregations, totalInUse]) => {
return {
Expand Down
Loading