From 591f5c3151aafdbf618956891a91d187b2d1e6b1 Mon Sep 17 00:00:00 2001 From: Shahzad Date: Mon, 28 Oct 2024 20:38:53 +0100 Subject: [PATCH] [Synthetics] Refactor delete route !! (#195387) ## Summary Fixes https://github.com/elastic/kibana/issues/193790 !! Refactor delete route !! Make sure to send delete response in bulk to synthetics service !! (cherry picked from commit 72888f651a1033ef566ac1600edc509913006db6) --- .../add_monitor/add_monitor_api.ts | 8 +- .../monitor_cruds/add_monitor_project.ts | 5 +- .../bulk_cruds/add_monitor_bulk.ts | 22 +- .../bulk_cruds/delete_monitor_bulk.ts | 24 +- .../routes/monitor_cruds/delete_monitor.ts | 207 ++---------------- .../monitor_cruds/delete_monitor_project.ts | 10 +- .../services/delete_monitor_api.ts | 122 +++++++++++ .../services/validate_space_id.ts | 21 ++ 8 files changed, 184 insertions(+), 235 deletions(-) create mode 100644 x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/services/delete_monitor_api.ts create mode 100644 x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/services/validate_space_id.ts diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/add_monitor/add_monitor_api.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/add_monitor/add_monitor_api.ts index b80a4f5be6825..f8c7fa9ed9b23 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/add_monitor/add_monitor_api.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/add_monitor/add_monitor_api.ts @@ -10,10 +10,10 @@ import { SavedObject } from '@kbn/core-saved-objects-common/src/server_types'; import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server'; import { isValidNamespace } from '@kbn/fleet-plugin/common'; import { i18n } from '@kbn/i18n'; +import { DeleteMonitorAPI } from '../services/delete_monitor_api'; import { parseMonitorLocations } from './utils'; import { MonitorValidationError } from '../monitor_validation'; import { getSavedObjectKqlFilter } from '../../common'; -import { deleteMonitor } from '../delete_monitor'; import { monitorAttributes, syntheticsMonitorType } from '../../../../common/types/saved_objects'; import { PrivateLocationAttributes } from '../../../runtime_types/private_locations'; import { ConfigKey } from '../../../../common/constants/monitor_management'; @@ -339,9 +339,9 @@ export class AddEditMonitorAPI { if (encryptedMonitor) { await savedObjectsClient.delete(syntheticsMonitorType, newMonitorId); - await deleteMonitor({ - routeContext: this.routeContext, - monitorId: newMonitorId, + const deleteMonitorAPI = new DeleteMonitorAPI(this.routeContext); + await deleteMonitorAPI.execute({ + monitorIds: [newMonitorId], }); } } catch (e) { diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/add_monitor_project.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/add_monitor_project.ts index 75427a22aced2..8311a6407bf6a 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/add_monitor_project.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/add_monitor_project.ts @@ -7,6 +7,7 @@ import { schema } from '@kbn/config-schema'; import { i18n } from '@kbn/i18n'; import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common'; +import { validateSpaceId } from './services/validate_space_id'; import { RouteContext, SyntheticsRestApiRouteFactory } from '../types'; import { ProjectMonitor } from '../../../common/runtime_types'; @@ -46,9 +47,7 @@ export const addSyntheticsProjectMonitorRoute: SyntheticsRestApiRouteFactory = ( } try { - const { id: spaceId } = (await server.spaces?.spacesService.getActiveSpace(request)) ?? { - id: DEFAULT_SPACE_ID, - }; + const spaceId = await validateSpaceId(routeContext); const permissionError = await validatePermissions(routeContext, monitors); diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/bulk_cruds/add_monitor_bulk.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/bulk_cruds/add_monitor_bulk.ts index b6a090165382b..2ecbbf83d471c 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/bulk_cruds/add_monitor_bulk.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/bulk_cruds/add_monitor_bulk.ts @@ -10,10 +10,10 @@ import { SavedObjectsBulkResponse } from '@kbn/core-saved-objects-api-server'; import { v4 as uuidV4 } from 'uuid'; import { NewPackagePolicy } from '@kbn/fleet-plugin/common'; import { SavedObjectError } from '@kbn/core-saved-objects-common'; +import { deleteMonitorBulk } from './delete_monitor_bulk'; import { SyntheticsServerSetup } from '../../../types'; import { RouteContext } from '../../types'; import { formatTelemetryEvent, sendTelemetryEvents } from '../../telemetry/monitor_upgrade_sender'; -import { deleteMonitor } from '../delete_monitor'; import { formatSecrets } from '../../../synthetics_service/utils'; import { syntheticsMonitorType } from '../../../../common/types/saved_objects'; import { @@ -24,6 +24,7 @@ import { SyntheticsMonitor, type SyntheticsPrivateLocations, } from '../../../../common/runtime_types'; +import { DeleteMonitorAPI } from '../services/delete_monitor_api'; export const createNewSavedObjectMonitorBulk = async ({ soClient, @@ -146,15 +147,10 @@ const rollBackNewMonitorBulk = async ( ) => { const { server } = routeContext; try { - await pMap( - monitorsToCreate, - async (monitor) => - deleteMonitor({ - routeContext, - monitorId: monitor.id, - }), - { concurrency: 100, stopOnError: false } - ); + const deleteMonitorAPI = new DeleteMonitorAPI(routeContext); + await deleteMonitorAPI.execute({ + monitorIds: monitorsToCreate.map(({ id }) => id), + }); } catch (e) { // ignore errors here server.logger.error(e); @@ -194,11 +190,9 @@ export const deleteMonitorIfCreated = async ({ newMonitorId ); if (encryptedMonitor) { - await savedObjectsClient.delete(syntheticsMonitorType, newMonitorId); - - await deleteMonitor({ + await deleteMonitorBulk({ + monitors: [encryptedMonitor], routeContext, - monitorId: newMonitorId, }); } } catch (e) { diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/bulk_cruds/delete_monitor_bulk.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/bulk_cruds/delete_monitor_bulk.ts index 7df12b17b6092..9a031b3e7111a 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/bulk_cruds/delete_monitor_bulk.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/bulk_cruds/delete_monitor_bulk.ts @@ -4,10 +4,7 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import { SavedObjectsClientContract, KibanaRequest } from '@kbn/core/server'; import { SavedObject } from '@kbn/core-saved-objects-server'; -import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common'; -import { SyntheticsServerSetup } from '../../../types'; import { formatTelemetryDeleteEvent, sendTelemetryEvents, @@ -19,29 +16,20 @@ import { EncryptedSyntheticsMonitorAttributes, SyntheticsMonitorWithId, } from '../../../../common/runtime_types'; -import { SyntheticsMonitorClient } from '../../../synthetics_service/synthetics_monitor/synthetics_monitor_client'; import { syntheticsMonitorType } from '../../../../common/types/saved_objects'; +import { RouteContext } from '../../types'; export const deleteMonitorBulk = async ({ - savedObjectsClient, - server, monitors, - syntheticsMonitorClient, - request, + routeContext, }: { - savedObjectsClient: SavedObjectsClientContract; - server: SyntheticsServerSetup; monitors: Array>; - syntheticsMonitorClient: SyntheticsMonitorClient; - request: KibanaRequest; + routeContext: RouteContext; }) => { + const { savedObjectsClient, server, spaceId, syntheticsMonitorClient } = routeContext; const { logger, telemetry, stackVersion } = server; try { - const { id: spaceId } = (await server.spaces?.spacesService.getActiveSpace(request)) ?? { - id: DEFAULT_SPACE_ID, - }; - const deleteSyncPromise = syntheticsMonitorClient.deleteMonitors( monitors.map((normalizedMonitor) => ({ ...normalizedMonitor.attributes, @@ -55,7 +43,7 @@ export const deleteMonitorBulk = async ({ monitors.map((monitor) => ({ type: syntheticsMonitorType, id: monitor.id })) ); - const [errors] = await Promise.all([deleteSyncPromise, deletePromises]); + const [errors, result] = await Promise.all([deleteSyncPromise, deletePromises]); monitors.forEach((monitor) => { sendTelemetryEvents( @@ -71,7 +59,7 @@ export const deleteMonitorBulk = async ({ ); }); - return errors; + return { errors, result }; } catch (e) { throw e; } diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/delete_monitor.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/delete_monitor.ts index b7a1d0b2d48d8..f40f06f66b1ff 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/delete_monitor.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/delete_monitor.ts @@ -5,27 +5,10 @@ * 2.0. */ import { schema } from '@kbn/config-schema'; -import { SavedObjectsClientContract, SavedObjectsErrorHelpers } from '@kbn/core/server'; -import pMap from 'p-map'; -import { validatePermissions } from './edit_monitor'; -import { SyntheticsServerSetup } from '../../types'; -import { RouteContext, SyntheticsRestApiRouteFactory } from '../types'; -import { syntheticsMonitorType } from '../../../common/types/saved_objects'; -import { - ConfigKey, - DeleteParamsResponse, - EncryptedSyntheticsMonitorAttributes, - MonitorFields, - SyntheticsMonitorWithId, - SyntheticsMonitorWithSecretsAttributes, -} from '../../../common/runtime_types'; +import { DeleteMonitorAPI } from './services/delete_monitor_api'; +import { SyntheticsRestApiRouteFactory } from '../types'; +import { DeleteParamsResponse } from '../../../common/runtime_types'; import { SYNTHETICS_API_URLS } from '../../../common/constants'; -import { - formatTelemetryDeleteEvent, - sendErrorTelemetryEvents, - sendTelemetryEvents, -} from '../telemetry/monitor_upgrade_sender'; -import { formatSecrets, normalizeSecrets } from '../../synthetics_service/utils/secrets'; export const deleteSyntheticsMonitorRoute: SyntheticsRestApiRouteFactory< DeleteParamsResponse[], @@ -62,7 +45,6 @@ export const deleteSyntheticsMonitorRoute: SyntheticsRestApiRouteFactory< }); } - const result: Array<{ id: string; deleted: boolean; error?: string }> = []; const idsToDelete = [...(ids ?? []), ...(queryId ? [queryId] : [])]; if (idsToDelete.length === 0) { return response.badRequest({ @@ -70,178 +52,21 @@ export const deleteSyntheticsMonitorRoute: SyntheticsRestApiRouteFactory< }); } - await pMap(idsToDelete, async (id) => { - try { - const { errors, res } = await deleteMonitor({ - routeContext, - monitorId: id, - }); - if (res) { - return res; - } - - if (errors && errors.length > 0) { - return response.ok({ - body: { message: 'error pushing monitor to the service', attributes: { errors } }, - }); - } + const deleteMonitorAPI = new DeleteMonitorAPI(routeContext); + try { + const { errors } = await deleteMonitorAPI.execute({ + monitorIds: idsToDelete, + }); - result.push({ id, deleted: true }); - } catch (getErr) { - if (SavedObjectsErrorHelpers.isNotFoundError(getErr)) { - result.push({ id, deleted: false, error: `Monitor id ${id} not found!` }); - } else { - throw getErr; - } + if (errors && errors.length > 0) { + return response.ok({ + body: { message: 'error pushing monitor to the service', attributes: { errors } }, + }); } - }); + } catch (getErr) { + throw getErr; + } - return result; + return deleteMonitorAPI.result; }, }); - -export const deleteMonitor = async ({ - routeContext, - monitorId, -}: { - routeContext: RouteContext; - monitorId: string; -}) => { - const { response, spaceId, savedObjectsClient, server, syntheticsMonitorClient } = routeContext; - const { logger, telemetry, stackVersion } = server; - - const { monitor, monitorWithSecret } = await getMonitorToDelete( - monitorId, - savedObjectsClient, - server, - spaceId - ); - - const err = await validatePermissions(routeContext, monitor.attributes.locations); - if (err) { - return { - res: response.forbidden({ - body: { - message: err, - }, - }), - }; - } - - let deletePromise; - - try { - deletePromise = savedObjectsClient.delete(syntheticsMonitorType, monitorId); - - const deleteSyncPromise = syntheticsMonitorClient.deleteMonitors( - [ - { - ...monitor.attributes, - id: (monitor.attributes as MonitorFields)[ConfigKey.MONITOR_QUERY_ID], - }, - /* Type cast encrypted saved objects to decrypted saved objects for delete flow only. - * Deletion does not require all monitor fields */ - ] as SyntheticsMonitorWithId[], - savedObjectsClient, - spaceId - ); - - const [errors] = await Promise.all([deleteSyncPromise, deletePromise]).catch((e) => { - server.logger.error(e); - throw e; - }); - - sendTelemetryEvents( - logger, - telemetry, - formatTelemetryDeleteEvent( - monitor, - stackVersion, - new Date().toISOString(), - Boolean((monitor.attributes as MonitorFields)[ConfigKey.SOURCE_INLINE]), - errors - ) - ); - - return { errors }; - } catch (e) { - if (deletePromise) { - await deletePromise; - } - server.logger.error( - `Unable to delete Synthetics monitor ${monitor.attributes[ConfigKey.NAME]}` - ); - - if (monitorWithSecret) { - await restoreDeletedMonitor({ - monitorId, - normalizedMonitor: formatSecrets({ - ...monitorWithSecret.attributes, - }), - savedObjectsClient, - }); - } - throw e; - } -}; - -const getMonitorToDelete = async ( - monitorId: string, - soClient: SavedObjectsClientContract, - server: SyntheticsServerSetup, - spaceId: string -) => { - const encryptedSOClient = server.encryptedSavedObjects.getClient(); - - try { - const monitor = - await encryptedSOClient.getDecryptedAsInternalUser( - syntheticsMonitorType, - monitorId, - { - namespace: spaceId, - } - ); - return { monitor: normalizeSecrets(monitor), monitorWithSecret: normalizeSecrets(monitor) }; - } catch (e) { - server.logger.error(`Failed to decrypt monitor to delete ${monitorId}${e}`); - sendErrorTelemetryEvents(server.logger, server.telemetry, { - reason: `Failed to decrypt monitor to delete ${monitorId}`, - message: e?.message, - type: 'deletionError', - code: e?.code, - status: e.status, - stackVersion: server.stackVersion, - }); - } - - const monitor = await soClient.get( - syntheticsMonitorType, - monitorId - ); - return { monitor, withSecrets: false }; -}; - -const restoreDeletedMonitor = async ({ - monitorId, - savedObjectsClient, - normalizedMonitor, -}: { - monitorId: string; - normalizedMonitor: SyntheticsMonitorWithSecretsAttributes; - savedObjectsClient: SavedObjectsClientContract; -}) => { - try { - await savedObjectsClient.get( - syntheticsMonitorType, - monitorId - ); - } catch (e) { - if (SavedObjectsErrorHelpers.isNotFoundError(e)) { - await savedObjectsClient.create(syntheticsMonitorType, normalizedMonitor, { - id: monitorId, - overwrite: true, - }); - } - } -}; diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/delete_monitor_project.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/delete_monitor_project.ts index 2136634be7ef7..7b36780937694 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/delete_monitor_project.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/delete_monitor_project.ts @@ -12,6 +12,7 @@ import { ConfigKey } from '../../../common/runtime_types'; import { SYNTHETICS_API_URLS } from '../../../common/constants'; import { getMonitors, getSavedObjectKqlFilter } from '../common'; import { deleteMonitorBulk } from './bulk_cruds/delete_monitor_bulk'; +import { validateSpaceId } from './services/validate_space_id'; export const deleteSyntheticsMonitorProjectRoute: SyntheticsRestApiRouteFactory = () => ({ method: 'DELETE', @@ -25,7 +26,7 @@ export const deleteSyntheticsMonitorProjectRoute: SyntheticsRestApiRouteFactory }), }, handler: async (routeContext): Promise => { - const { request, response, savedObjectsClient, server, syntheticsMonitorClient } = routeContext; + const { request, response } = routeContext; const { projectName } = request.params; const { monitors: monitorsToDelete } = request.body; const decodedProjectName = decodeURI(projectName); @@ -37,6 +38,8 @@ export const deleteSyntheticsMonitorProjectRoute: SyntheticsRestApiRouteFactory }); } + await validateSpaceId(routeContext); + const deleteFilter = `${syntheticsMonitorType}.attributes.${ ConfigKey.PROJECT_ID }: "${decodedProjectName}" AND ${getSavedObjectKqlFilter({ @@ -57,10 +60,7 @@ export const deleteSyntheticsMonitorProjectRoute: SyntheticsRestApiRouteFactory await deleteMonitorBulk({ monitors, - server, - savedObjectsClient, - syntheticsMonitorClient, - request, + routeContext, }); return { diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/services/delete_monitor_api.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/services/delete_monitor_api.ts new file mode 100644 index 0000000000000..bd162fc043592 --- /dev/null +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/services/delete_monitor_api.ts @@ -0,0 +1,122 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import pMap from 'p-map'; +import { SavedObject, SavedObjectsErrorHelpers } from '@kbn/core-saved-objects-server'; +import { deleteMonitorBulk } from '../bulk_cruds/delete_monitor_bulk'; +import { validatePermissions } from '../edit_monitor'; +import { + EncryptedSyntheticsMonitorAttributes, + SyntheticsMonitor, + SyntheticsMonitorWithSecretsAttributes, +} from '../../../../common/runtime_types'; +import { syntheticsMonitorType } from '../../../../common/types/saved_objects'; +import { normalizeSecrets } from '../../../synthetics_service/utils'; +import { sendErrorTelemetryEvents } from '../../telemetry/monitor_upgrade_sender'; +import { RouteContext } from '../../types'; + +export class DeleteMonitorAPI { + routeContext: RouteContext; + result: Array<{ id: string; deleted: boolean; error?: string }> = []; + constructor(routeContext: RouteContext) { + this.routeContext = routeContext; + } + + async getMonitorsToDelete(monitorIds: string[]) { + const result: Array> = []; + await pMap( + monitorIds, + async (monitorId) => { + const monitor = await this.getMonitorToDelete(monitorId); + if (monitor) { + result.push(monitor); + } + }, + { + stopOnError: false, + } + ); + return result; + } + + async getMonitorToDelete(monitorId: string) { + const { spaceId, savedObjectsClient, server } = this.routeContext; + try { + const encryptedSOClient = server.encryptedSavedObjects.getClient(); + + const monitor = + await encryptedSOClient.getDecryptedAsInternalUser( + syntheticsMonitorType, + monitorId, + { + namespace: spaceId, + } + ); + return normalizeSecrets(monitor); + } catch (e) { + if (SavedObjectsErrorHelpers.isNotFoundError(e)) { + this.result.push({ + id: monitorId, + deleted: false, + error: `Monitor id ${monitorId} not found!`, + }); + } else { + server.logger.error(`Failed to decrypt monitor to delete ${monitorId}${e}`); + sendErrorTelemetryEvents(server.logger, server.telemetry, { + reason: `Failed to decrypt monitor to delete ${monitorId}`, + message: e?.message, + type: 'deletionError', + code: e?.code, + status: e.status, + stackVersion: server.stackVersion, + }); + return await savedObjectsClient.get( + syntheticsMonitorType, + monitorId + ); + } + } + } + + async execute({ monitorIds }: { monitorIds: string[] }) { + const { response, server } = this.routeContext; + + const monitors = await this.getMonitorsToDelete(monitorIds); + for (const monitor of monitors) { + const err = await validatePermissions(this.routeContext, monitor.attributes.locations); + if (err) { + return { + res: response.forbidden({ + body: { + message: err, + }, + }), + }; + } + } + + try { + const { errors, result } = await deleteMonitorBulk({ + monitors, + routeContext: this.routeContext, + }); + + result.statuses?.forEach((res) => { + this.result.push({ + id: res.id, + deleted: res.success, + }); + }); + + return { errors }; + } catch (e) { + server.logger.error(`Unable to delete Synthetics monitor with error ${e.message}`); + server.logger.error(e); + throw e; + } + } +} diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/services/validate_space_id.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/services/validate_space_id.ts new file mode 100644 index 0000000000000..9f456efc3f5b1 --- /dev/null +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/monitor_cruds/services/validate_space_id.ts @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common'; +import { RouteContext } from '../../types'; + +export const validateSpaceId = async (routeContext: RouteContext) => { + const { server, request, spaceId } = routeContext; + // If the spaceId is the default space, return it, it always exists + if (spaceId === DEFAULT_SPACE_ID) { + return spaceId; + } + const { id } = (await server.spaces?.spacesService.getActiveSpace(request)) ?? { + id: DEFAULT_SPACE_ID, + }; + return id; +};