From 048a65dc09b910fe558a5494416db58a1d4b8605 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Tue, 24 Jan 2023 15:30:23 +0100 Subject: [PATCH] [APM] Allow calling `createInternalESClient` without `context` (#149320) --- .../create_es_client/call_async_with_debug.ts | 32 ++++----- .../create_apm_event_client/index.ts | 1 - .../create_internal_es_client/index.ts | 70 ++++++++++++++----- .../fleet/register_fleet_policy_callbacks.ts | 23 +++--- .../plugins/apm/server/routes/fleet/route.ts | 4 +- .../settings/agent_configuration/route.ts | 14 ++-- .../routes/settings/custom_link/route.ts | 10 +-- x-pack/plugins/apm/tsconfig.json | 1 + 8 files changed, 96 insertions(+), 59 deletions(-) diff --git a/x-pack/plugins/apm/server/lib/helpers/create_es_client/call_async_with_debug.ts b/x-pack/plugins/apm/server/lib/helpers/create_es_client/call_async_with_debug.ts index a401acafd01d9..c1c89d5b906b6 100644 --- a/x-pack/plugins/apm/server/lib/helpers/create_es_client/call_async_with_debug.ts +++ b/x-pack/plugins/apm/server/lib/helpers/create_es_client/call_async_with_debug.ts @@ -23,7 +23,6 @@ export async function callAsyncWithDebug({ getDebugMessage, debug, request, - requestType, requestParams, operationName, isCalledWithInternalUser, @@ -31,8 +30,7 @@ export async function callAsyncWithDebug({ cb: () => Promise; getDebugMessage: () => { body: string; title: string }; debug: boolean; - request: KibanaRequest; - requestType: string; + request?: KibanaRequest; requestParams: Record; operationName: string; isCalledWithInternalUser: boolean; // only allow inspection of queries that were retrieved with credentials of the end user @@ -70,19 +68,21 @@ export async function callAsyncWithDebug({ console.log(body); console.log(`\n`); - const inspectableEsQueries = inspectableEsQueriesMap.get(request); - if (!isCalledWithInternalUser && inspectableEsQueries) { - inspectableEsQueries.push( - getInspectResponse({ - esError, - esRequestParams: requestParams, - esRequestStatus, - esResponse: res, - kibanaRequest: request, - operationName, - startTime, - }) - ); + if (request) { + const inspectableEsQueries = inspectableEsQueriesMap.get(request); + if (!isCalledWithInternalUser && inspectableEsQueries) { + inspectableEsQueries.push( + getInspectResponse({ + esError, + esRequestParams: requestParams, + esRequestStatus, + esResponse: res, + kibanaRequest: request, + operationName, + startTime, + }) + ); + } } } diff --git a/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/index.ts b/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/index.ts index b41d416b92a7a..870a7971c2e9e 100644 --- a/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/index.ts +++ b/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/index.ts @@ -124,7 +124,6 @@ export class APMEventClient { isCalledWithInternalUser: false, debug: this.debug, request: this.request, - requestType, operationName, requestParams: params, cb: () => { diff --git a/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_internal_es_client/index.ts b/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_internal_es_client/index.ts index f48971588a050..d1cd9054fbac1 100644 --- a/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_internal_es_client/index.ts +++ b/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_internal_es_client/index.ts @@ -8,6 +8,9 @@ import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import { unwrapEsResponse } from '@kbn/observability-plugin/server'; import type { ESSearchResponse, ESSearchRequest } from '@kbn/es-types'; +import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server'; +import { ElasticsearchClient } from '@kbn/core-elasticsearch-server'; +import { APMConfig } from '../../../..'; import { APMRouteHandlerResources } from '../../../../routes/typings'; import { callAsyncWithDebug, @@ -20,48 +23,75 @@ import { getApmIndices } from '../../../../routes/settings/apm_indices/get_apm_i export type APMIndexDocumentParams = estypes.IndexRequest; export type APMInternalESClient = Awaited< - ReturnType + ReturnType >; -export async function createInternalESClient({ - context, +export async function createInternalESClientWithContext({ debug, - request, config, -}: Pick & { + request, + context, +}: { debug: boolean; + config: APMConfig; + request: APMRouteHandlerResources['request']; + context: APMRouteHandlerResources['context']; }) { const coreContext = await context.core; const { asInternalUser } = coreContext.elasticsearch.client; const savedObjectsClient = coreContext.savedObjects.client; + return createInternalESClient({ + debug, + config, + request, + savedObjectsClient, + elasticsearchClient: asInternalUser, + }); +} + +export async function createInternalESClient({ + debug, + config, + request, + savedObjectsClient, + elasticsearchClient, +}: { + debug: boolean; + config: APMConfig; + request?: APMRouteHandlerResources['request']; + savedObjectsClient: SavedObjectsClientContract; + elasticsearchClient: ElasticsearchClient; +}) { function callEs( operationName: string, { - cb, + makeRequestWithSignal, requestType, params, }: { requestType: string; - cb: (signal: AbortSignal) => Promise; + makeRequestWithSignal: (signal: AbortSignal) => Promise; params: Record; } ) { return callAsyncWithDebug({ cb: () => { const controller = new AbortController(); + const res = makeRequestWithSignal(controller.signal); return unwrapEsResponse( - cancelEsRequestOnAbort(cb(controller.signal), request, controller) + request ? cancelEsRequestOnAbort(res, request, controller) : res ); }, - getDebugMessage: () => ({ - title: getDebugTitle(request), - body: getDebugBody({ params, requestType, operationName }), - }), + getDebugMessage: () => { + return { + title: request ? getDebugTitle(request) : 'Internal request', + body: getDebugBody({ params, requestType, operationName }), + }; + }, debug, isCalledWithInternalUser: true, request, - requestType, requestParams: params, operationName, }); @@ -78,8 +108,8 @@ export async function createInternalESClient({ ): Promise> => { return callEs(operationName, { requestType: 'search', - cb: (signal) => - asInternalUser.search(params, { + makeRequestWithSignal: (signal) => + elasticsearchClient.search(params, { signal, meta: true, }) as Promise<{ body: any }>, @@ -89,7 +119,8 @@ export async function createInternalESClient({ index: (operationName: string, params: APMIndexDocumentParams) => { return callEs(operationName, { requestType: 'index', - cb: (signal) => asInternalUser.index(params, { signal, meta: true }), + makeRequestWithSignal: (signal) => + elasticsearchClient.index(params, { signal, meta: true }), params, }); }, @@ -99,7 +130,8 @@ export async function createInternalESClient({ ): Promise<{ result: string }> => { return callEs(operationName, { requestType: 'delete', - cb: (signal) => asInternalUser.delete(params, { signal, meta: true }), + makeRequestWithSignal: (signal) => + elasticsearchClient.delete(params, { signal, meta: true }), params, }); }, @@ -109,8 +141,8 @@ export async function createInternalESClient({ ) => { return callEs(operationName, { requestType: 'indices.create', - cb: (signal) => - asInternalUser.indices.create(params, { signal, meta: true }), + makeRequestWithSignal: (signal) => + elasticsearchClient.indices.create(params, { signal, meta: true }), params, }); }, diff --git a/x-pack/plugins/apm/server/routes/fleet/register_fleet_policy_callbacks.ts b/x-pack/plugins/apm/server/routes/fleet/register_fleet_policy_callbacks.ts index 2dfdb97f36886..9057ab065cacb 100644 --- a/x-pack/plugins/apm/server/routes/fleet/register_fleet_policy_callbacks.ts +++ b/x-pack/plugins/apm/server/routes/fleet/register_fleet_policy_callbacks.ts @@ -14,14 +14,15 @@ import { PutPackagePolicyUpdateCallback, } from '@kbn/fleet-plugin/server'; import { get } from 'lodash'; -import { APMPlugin, APMRouteHandlerResources } from '../..'; -import { createInternalESClient } from '../../lib/helpers/create_es_client/create_internal_es_client'; +import { APMConfig, APMPlugin, APMRouteHandlerResources } from '../..'; import { decoratePackagePolicyWithAgentConfigAndSourceMap } from './merge_package_policy_with_apm'; import { addApiKeysToPackagePolicyIfMissing } from './api_keys/add_api_keys_to_policies_if_missing'; import { AGENT_CONFIG_API_KEY_PATH, SOURCE_MAP_API_KEY_PATH, } from './get_package_policy_decorators'; +import { createInternalESClient } from '../../lib/helpers/create_es_client/create_internal_es_client'; +import { getInternalSavedObjectsClient } from '../../lib/helpers/get_internal_saved_objects_client'; export async function registerFleetPolicyCallbacks({ logger, @@ -43,12 +44,12 @@ export async function registerFleetPolicyCallbacks({ fleetPluginStart.registerExternalCallback( 'packagePolicyUpdate', - onPackagePolicyCreateOrUpdate({ fleetPluginStart, config }) + onPackagePolicyCreateOrUpdate({ fleetPluginStart, config, coreStart }) ); fleetPluginStart.registerExternalCallback( 'packagePolicyCreate', - onPackagePolicyCreateOrUpdate({ fleetPluginStart, config }) + onPackagePolicyCreateOrUpdate({ fleetPluginStart, config, coreStart }) ); fleetPluginStart.registerExternalCallback( @@ -144,23 +145,27 @@ function onPackagePolicyPostCreate({ function onPackagePolicyCreateOrUpdate({ fleetPluginStart, config, + coreStart, }: { fleetPluginStart: FleetStartContract; - config: NonNullable; + config: APMConfig; + coreStart: CoreStart; }): PutPackagePolicyUpdateCallback & PostPackagePolicyCreateCallback { - return async (packagePolicy, context, request) => { + return async (packagePolicy) => { if (packagePolicy.package?.name !== 'apm') { return packagePolicy; } + const { asInternalUser } = coreStart.elasticsearch.client; + const savedObjectsClient = await getInternalSavedObjectsClient(coreStart); const internalESClient = await createInternalESClient({ - context: context as any, debug: false, - request, config, + savedObjectsClient, + elasticsearchClient: asInternalUser, }); - return await decoratePackagePolicyWithAgentConfigAndSourceMap({ + return decoratePackagePolicyWithAgentConfigAndSourceMap({ internalESClient, fleetPluginStart, packagePolicy, diff --git a/x-pack/plugins/apm/server/routes/fleet/route.ts b/x-pack/plugins/apm/server/routes/fleet/route.ts index d4e5d467d61a0..dbc2485eb843e 100644 --- a/x-pack/plugins/apm/server/routes/fleet/route.ts +++ b/x-pack/plugins/apm/server/routes/fleet/route.ts @@ -26,7 +26,7 @@ import { getInternalSavedObjectsClient } from '../../lib/helpers/get_internal_sa import { createApmServerRoute } from '../apm_routes/create_apm_server_route'; import { getLatestApmPackage } from './get_latest_apm_package'; import { getJavaAgentVersionsFromRegistry } from './get_java_agent_versions'; -import { createInternalESClient } from '../../lib/helpers/create_es_client/create_internal_es_client'; +import { createInternalESClientWithContext } from '../../lib/helpers/create_es_client/create_internal_es_client'; const hasFleetDataRoute = createApmServerRoute({ endpoint: 'GET /internal/apm/fleet/has_apm_policies', @@ -249,7 +249,7 @@ const createCloudApmPackagePolicyRoute = createApmServerRoute({ throw Boom.forbidden(CLOUD_SUPERUSER_REQUIRED_MESSAGE); } - const internalESClient = await createInternalESClient({ + const internalESClient = await createInternalESClientWithContext({ context, request, debug: resources.params.query._inspect, diff --git a/x-pack/plugins/apm/server/routes/settings/agent_configuration/route.ts b/x-pack/plugins/apm/server/routes/settings/agent_configuration/route.ts index a631743db9375..4984b422ea259 100644 --- a/x-pack/plugins/apm/server/routes/settings/agent_configuration/route.ts +++ b/x-pack/plugins/apm/server/routes/settings/agent_configuration/route.ts @@ -25,7 +25,7 @@ import { import { getSearchTransactionsEvents } from '../../../lib/helpers/transactions'; import { syncAgentConfigsToApmPackagePolicies } from '../../fleet/sync_agent_configs_to_apm_package_policies'; import { getApmEventClient } from '../../../lib/helpers/get_apm_event_client'; -import { createInternalESClient } from '../../../lib/helpers/create_es_client/create_internal_es_client'; +import { createInternalESClientWithContext } from '../../../lib/helpers/create_es_client/create_internal_es_client'; // get list of configurations const agentConfigurationRoute = createApmServerRoute({ @@ -39,7 +39,7 @@ const agentConfigurationRoute = createApmServerRoute({ >; }> => { const { context, request, params, config } = resources; - const internalESClient = await createInternalESClient({ + const internalESClient = await createInternalESClientWithContext({ context, request, debug: params.query._inspect, @@ -68,7 +68,7 @@ const getSingleAgentConfigurationRoute = createApmServerRoute({ const { name, environment, _inspect } = params.query; const service = { name, environment }; - const internalESClient = await createInternalESClient({ + const internalESClient = await createInternalESClientWithContext({ context, request, debug: _inspect, @@ -114,7 +114,7 @@ const deleteAgentConfigurationRoute = createApmServerRoute({ } = resources; const { service } = params.body; - const internalESClient = await createInternalESClient({ + const internalESClient = await createInternalESClientWithContext({ context, request, debug: params.query._inspect, @@ -179,7 +179,7 @@ const createOrUpdateAgentConfigurationRoute = createApmServerRoute({ } = resources; const { body, query } = params; - const internalESClient = await createInternalESClient({ + const internalESClient = await createInternalESClientWithContext({ context, request, debug: params.query._inspect, @@ -258,7 +258,7 @@ const agentConfigurationSearchRoute = createApmServerRoute({ mark_as_applied_by_agent: markAsAppliedByAgent, } = params.body; - const internalESClient = await createInternalESClient({ + const internalESClient = await createInternalESClientWithContext({ context, request, debug: params.query._inspect, @@ -323,7 +323,7 @@ const listAgentConfigurationEnvironmentsRoute = createApmServerRoute({ }> => { const { context, request, params, config } = resources; const [internalESClient, apmEventClient] = await Promise.all([ - createInternalESClient({ + createInternalESClientWithContext({ context, request, debug: params.query._inspect, diff --git a/x-pack/plugins/apm/server/routes/settings/custom_link/route.ts b/x-pack/plugins/apm/server/routes/settings/custom_link/route.ts index 87087409e3206..110e1fc0dc484 100644 --- a/x-pack/plugins/apm/server/routes/settings/custom_link/route.ts +++ b/x-pack/plugins/apm/server/routes/settings/custom_link/route.ts @@ -19,7 +19,7 @@ import { getTransaction } from './get_transaction'; import { listCustomLinks } from './list_custom_links'; import { createApmServerRoute } from '../../apm_routes/create_apm_server_route'; import { getApmEventClient } from '../../../lib/helpers/get_apm_event_client'; -import { createInternalESClient } from '../../../lib/helpers/create_es_client/create_internal_es_client'; +import { createInternalESClientWithContext } from '../../../lib/helpers/create_es_client/create_internal_es_client'; const customLinkTransactionRoute = createApmServerRoute({ endpoint: 'GET /internal/apm/settings/custom_links/transaction', @@ -63,7 +63,7 @@ const listCustomLinksRoute = createApmServerRoute({ const { query } = params; - const internalESClient = await createInternalESClient({ + const internalESClient = await createInternalESClientWithContext({ context, request, debug: resources.params.query._inspect, @@ -94,7 +94,7 @@ const createCustomLinkRoute = createApmServerRoute({ throw Boom.forbidden(INVALID_LICENSE); } - const internalESClient = await createInternalESClient({ + const internalESClient = await createInternalESClientWithContext({ context, request, debug: resources.params.query._inspect, @@ -130,7 +130,7 @@ const updateCustomLinkRoute = createApmServerRoute({ throw Boom.forbidden(INVALID_LICENSE); } - const internalESClient = await createInternalESClient({ + const internalESClient = await createInternalESClientWithContext({ context, request, debug: resources.params.query._inspect, @@ -166,7 +166,7 @@ const deleteCustomLinkRoute = createApmServerRoute({ throw Boom.forbidden(INVALID_LICENSE); } - const internalESClient = await createInternalESClient({ + const internalESClient = await createInternalESClientWithContext({ context, request, debug: resources.params.query._inspect, diff --git a/x-pack/plugins/apm/tsconfig.json b/x-pack/plugins/apm/tsconfig.json index 51a440ac249da..2193b4c9bf5a7 100644 --- a/x-pack/plugins/apm/tsconfig.json +++ b/x-pack/plugins/apm/tsconfig.json @@ -76,6 +76,7 @@ "@kbn/babel-register", "@kbn/core-saved-objects-migration-server-internal", "@kbn/core-elasticsearch-server", + "@kbn/core-saved-objects-api-server", ], "exclude": [ "target/**/*",