From 27944db45ff9ef400d623e30995e253aefba3768 Mon Sep 17 00:00:00 2001 From: Shahzad Date: Thu, 22 Dec 2022 11:10:19 +0100 Subject: [PATCH] [Synthetics] Ignore decryption errors in deletion (#140640) --- .../bulk_cruds/delete_monitor_bulk.ts | 4 +- .../routes/monitor_cruds/delete_monitor.ts | 78 ++++++++++++------- .../synthetics_private_location.test.ts | 2 +- .../synthetics_private_location.ts | 74 ++++++++---------- .../synthetics_monitor_client.ts | 5 +- .../synthetics_service/synthetics_service.ts | 24 +++--- .../apis/synthetics/delete_monitor.ts | 2 +- 7 files changed, 101 insertions(+), 88 deletions(-) diff --git a/x-pack/plugins/synthetics/server/routes/monitor_cruds/bulk_cruds/delete_monitor_bulk.ts b/x-pack/plugins/synthetics/server/routes/monitor_cruds/bulk_cruds/delete_monitor_bulk.ts index 189da5d9825be..305963e1ce430 100644 --- a/x-pack/plugins/synthetics/server/routes/monitor_cruds/bulk_cruds/delete_monitor_bulk.ts +++ b/x-pack/plugins/synthetics/server/routes/monitor_cruds/bulk_cruds/delete_monitor_bulk.ts @@ -15,7 +15,7 @@ import { MonitorFields, SyntheticsMonitor, EncryptedSyntheticsMonitor, - EncryptedSyntheticsMonitorWithId, + SyntheticsMonitorWithId, } from '../../../../common/runtime_types'; import { UptimeServerSetup } from '../../../legacy_uptime/lib/adapters'; import { SyntheticsMonitorClient } from '../../../synthetics_service/synthetics_monitor/synthetics_monitor_client'; @@ -42,7 +42,7 @@ export const deleteMonitorBulk = async ({ monitors.map((normalizedMonitor) => ({ ...normalizedMonitor.attributes, id: normalizedMonitor.attributes[ConfigKey.MONITOR_QUERY_ID], - })) as EncryptedSyntheticsMonitorWithId[], + })) as SyntheticsMonitorWithId[], request, savedObjectsClient, spaceId diff --git a/x-pack/plugins/synthetics/server/routes/monitor_cruds/delete_monitor.ts b/x-pack/plugins/synthetics/server/routes/monitor_cruds/delete_monitor.ts index 25c484cfea6e7..c650587048998 100644 --- a/x-pack/plugins/synthetics/server/routes/monitor_cruds/delete_monitor.ts +++ b/x-pack/plugins/synthetics/server/routes/monitor_cruds/delete_monitor.ts @@ -6,15 +6,16 @@ */ import { schema } from '@kbn/config-schema'; import { + KibanaRequest, SavedObjectsClientContract, SavedObjectsErrorHelpers, - KibanaRequest, } from '@kbn/core/server'; import { SyntheticsMonitorClient } from '../../synthetics_service/synthetics_monitor/synthetics_monitor_client'; import { ConfigKey, - MonitorFields, EncryptedSyntheticsMonitor, + MonitorFields, + SyntheticsMonitorWithId, SyntheticsMonitorWithSecrets, } from '../../../common/runtime_types'; import { SyntheticsRestApiRouteFactory } from '../../legacy_uptime/routes/types'; @@ -22,8 +23,9 @@ import { API_URLS } from '../../../common/constants'; import { syntheticsMonitorType } from '../../legacy_uptime/lib/saved_objects/synthetics_monitor'; import { getMonitorNotFoundResponse } from '../synthetics_service/service_errors'; import { - sendTelemetryEvents, formatTelemetryDeleteEvent, + sendErrorTelemetryEvents, + sendTelemetryEvents, } from '../telemetry/monitor_upgrade_sender'; import { formatSecrets, normalizeSecrets } from '../../synthetics_service/utils/secrets'; import type { UptimeServerSetup } from '../../legacy_uptime/lib/adapters/framework'; @@ -36,6 +38,7 @@ export const deleteSyntheticsMonitorRoute: SyntheticsRestApiRouteFactory = () => monitorId: schema.string({ minLength: 1, maxLength: 1024 }), }), }, + writeAccess: true, handler: async ({ request, response, @@ -84,35 +87,24 @@ export const deleteMonitor = async ({ syntheticsMonitorClient: SyntheticsMonitorClient; request: KibanaRequest; }) => { - const { logger, telemetry, stackVersion, encryptedSavedObjects } = server; + const { logger, telemetry, stackVersion } = server; const spaceId = server.spaces.spacesService.getSpaceId(request); - const encryptedSavedObjectsClient = encryptedSavedObjects.getClient(); - let normalizedMonitor; + const { monitor, monitorWithSecret } = await getMonitorToDelete( + monitorId, + savedObjectsClient, + server + ); try { - const encryptedMonitor = await savedObjectsClient.get( - syntheticsMonitorType, - monitorId - ); - - const monitor = - await encryptedSavedObjectsClient.getDecryptedAsInternalUser( - syntheticsMonitorType, - monitorId, - { - namespace: encryptedMonitor.namespaces?.[0], - } - ); - - normalizedMonitor = normalizeSecrets(monitor); - const deleteSyncPromise = syntheticsMonitorClient.deleteMonitors( [ { - ...normalizedMonitor.attributes, - id: (normalizedMonitor.attributes as MonitorFields)[ConfigKey.MONITOR_QUERY_ID], + ...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[], request, savedObjectsClient, spaceId @@ -128,18 +120,18 @@ export const deleteMonitor = async ({ monitor, stackVersion, new Date().toISOString(), - Boolean((normalizedMonitor.attributes as MonitorFields)[ConfigKey.SOURCE_INLINE]), + Boolean((monitor.attributes as MonitorFields)[ConfigKey.SOURCE_INLINE]), errors ) ); return errors; } catch (e) { - if (normalizedMonitor) { + if (monitorWithSecret) { await restoreDeletedMonitor({ monitorId, normalizedMonitor: formatSecrets({ - ...normalizedMonitor.attributes, + ...monitorWithSecret.attributes, }), savedObjectsClient, }); @@ -148,6 +140,36 @@ export const deleteMonitor = async ({ } }; +const getMonitorToDelete = async ( + monitorId: string, + soClient: SavedObjectsClientContract, + server: UptimeServerSetup +) => { + const encryptedSOClient = server.encryptedSavedObjects.getClient(); + + try { + const monitor = + await encryptedSOClient.getDecryptedAsInternalUser( + syntheticsMonitorType, + monitorId + ); + 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, diff --git a/x-pack/plugins/synthetics/server/synthetics_service/private_location/synthetics_private_location.test.ts b/x-pack/plugins/synthetics/server/synthetics_service/private_location/synthetics_private_location.test.ts index 8dc6688a70f3f..b39db6a7ba9ac 100644 --- a/x-pack/plugins/synthetics/server/synthetics_service/private_location/synthetics_private_location.test.ts +++ b/x-pack/plugins/synthetics/server/synthetics_service/private_location/synthetics_private_location.test.ts @@ -156,7 +156,7 @@ describe('SyntheticsPrivateLocation', () => { ], [ false, - 'Unable to delete Synthetics package policy for monitor. Fleet write permissions are needed to use Synthetics private locations.', + 'Unable to delete Synthetics package policy for monitor Test Monitor. Fleet write permissions are needed to use Synthetics private locations.', ], ])('throws errors for delete monitor', async (writeIntegrationPolicies, error) => { const syntheticsPrivateLocation = new SyntheticsPrivateLocation({ diff --git a/x-pack/plugins/synthetics/server/synthetics_service/private_location/synthetics_private_location.ts b/x-pack/plugins/synthetics/server/synthetics_service/private_location/synthetics_private_location.ts index f51f9e479929e..ecd815f0305e3 100644 --- a/x-pack/plugins/synthetics/server/synthetics_service/private_location/synthetics_private_location.ts +++ b/x-pack/plugins/synthetics/server/synthetics_service/private_location/synthetics_private_location.ts @@ -9,7 +9,6 @@ import { NewPackagePolicy } from '@kbn/fleet-plugin/common'; import { NewPackagePolicyWithId } from '@kbn/fleet-plugin/server/services/package_policy'; import { cloneDeep } from 'lodash'; import { formatSyntheticsPolicy } from '../../../common/formatters/format_synthetics_policy'; -import { getSyntheticsPrivateLocations } from '../../legacy_uptime/lib/saved_objects/private_locations'; import { ConfigKey, HeartbeatConfig, @@ -36,7 +35,7 @@ export class SyntheticsPrivateLocation { ); } - getPolicyId(config: HeartbeatConfig, { id: locId }: PrivateLocation, spaceId: string) { + getPolicyId(config: HeartbeatConfig, locId: string, spaceId: string) { if (config[ConfigKey.MONITOR_SOURCE_TYPE] === SourceType.PROJECT) { return `${config.id}-${locId}`; } @@ -144,7 +143,7 @@ export class SyntheticsPrivateLocation { ); } if (newPolicy) { - newPolicies.push({ ...newPolicy, id: this.getPolicyId(config, location, spaceId) }); + newPolicies.push({ ...newPolicy, id: this.getPolicyId(config, location.id, spaceId) }); } } } catch (e) { @@ -199,7 +198,7 @@ export class SyntheticsPrivateLocation { for (const privateLocation of allPrivateLocations) { const hasLocation = monitorPrivateLocations?.some((loc) => loc.id === privateLocation.id); - const currId = this.getPolicyId(config, privateLocation, spaceId); + const currId = this.getPolicyId(config, privateLocation.id, spaceId); const hasPolicy = existingPolicies?.some((policy) => policy.id === currId); try { if (hasLocation) { @@ -254,7 +253,7 @@ export class SyntheticsPrivateLocation { const listOfPolicies: string[] = []; for (const config of configs) { for (const privateLocation of allPrivateLocations) { - const currId = this.getPolicyId(config, privateLocation, spaceId); + const currId = this.getPolicyId(config, privateLocation.id, spaceId); listOfPolicies.push(currId); } } @@ -319,50 +318,35 @@ export class SyntheticsPrivateLocation { async deleteMonitors( configs: HeartbeatConfig[], request: KibanaRequest, - savedObjectsClient: SavedObjectsClientContract, + soClient: SavedObjectsClientContract, spaceId: string ) { - const soClient = savedObjectsClient; - const esClient = this.server.uptimeEsClient.baseESClient; - - const allPrivateLocations: PrivateLocation[] = await getSyntheticsPrivateLocations(soClient); - - if (soClient && esClient) { - const policyIdsToDelete = []; - for (const config of configs) { - const { locations } = config; + const policyIdsToDelete = []; + for (const config of configs) { + const { locations } = config; - const monitorPrivateLocations = locations.filter((loc) => !loc.isServiceManaged); + const monitorPrivateLocations = locations.filter((loc) => !loc.isServiceManaged); - for (const privateLocation of monitorPrivateLocations) { - const location = allPrivateLocations?.find((loc) => loc.id === privateLocation.id); - if (location) { - await this.checkPermissions( - request, - `Unable to delete Synthetics package policy for monitor ${ - config[ConfigKey.NAME] - }. Fleet write permissions are needed to use Synthetics private locations.` - ); - try { - policyIdsToDelete.push(this.getPolicyId(config, location, spaceId)); - } catch (e) { - this.server.logger.error(e); - throw new Error( - `Unable to delete Synthetics package policy for monitor ${ - config[ConfigKey.NAME] - } with private location ${location.label}` - ); - } - } + for (const privateLocation of monitorPrivateLocations) { + await this.checkPermissions(request, deletePermissionError(config[ConfigKey.NAME])); + try { + policyIdsToDelete.push(this.getPolicyId(config, privateLocation.id, spaceId)); + } catch (e) { + this.server.logger.error(e); + throw new Error( + `Unable to delete Synthetics package policy for monitor ${ + config[ConfigKey.NAME] + } with private location ${privateLocation.label}` + ); } } - if (policyIdsToDelete.length > 0) { - await this.checkPermissions( - request, - `Unable to delete Synthetics package policy for monitor. Fleet write permissions are needed to use Synthetics private locations.` - ); - await this.deletePolicyBulk(policyIdsToDelete, savedObjectsClient); - } + } + if (policyIdsToDelete.length > 0) { + await this.checkPermissions( + request, + `Unable to delete Synthetics package policy for monitor. Fleet write permissions are needed to use Synthetics private locations.` + ); + await this.deletePolicyBulk(policyIdsToDelete, soClient); } } @@ -378,3 +362,7 @@ export class SyntheticsPrivateLocation { return agentPolicies.items; } } + +const deletePermissionError = (name: string) => { + return `Unable to delete Synthetics package policy for monitor ${name}. Fleet write permissions are needed to use Synthetics private locations.`; +}; diff --git a/x-pack/plugins/synthetics/server/synthetics_service/synthetics_monitor/synthetics_monitor_client.ts b/x-pack/plugins/synthetics/server/synthetics_service/synthetics_monitor/synthetics_monitor_client.ts index 7c4471473d569..490354cc15bf6 100644 --- a/x-pack/plugins/synthetics/server/synthetics_service/synthetics_monitor/synthetics_monitor_client.ts +++ b/x-pack/plugins/synthetics/server/synthetics_service/synthetics_monitor/synthetics_monitor_client.ts @@ -20,7 +20,6 @@ import { ConfigKey, MonitorFields, SyntheticsMonitorWithId, - EncryptedSyntheticsMonitorWithId, HeartbeatConfig, PrivateLocation, EncryptedSyntheticsMonitor, @@ -136,13 +135,11 @@ export class SyntheticsMonitorClient { } } async deleteMonitors( - monitors: Array, + monitors: SyntheticsMonitorWithId[], request: KibanaRequest, savedObjectsClient: SavedObjectsClientContract, spaceId: string ) { - /* Type cast encrypted saved objects to decrypted saved objects for delete flow only. - * Deletion does not require all monitor fields */ const privateDeletePromise = this.privateLocationAPI.deleteMonitors( monitors as SyntheticsMonitorWithId[], request, diff --git a/x-pack/plugins/synthetics/server/synthetics_service/synthetics_service.ts b/x-pack/plugins/synthetics/server/synthetics_service/synthetics_service.ts index f50ab8d0c35c1..7e7b304f5edaa 100644 --- a/x-pack/plugins/synthetics/server/synthetics_service/synthetics_service.ts +++ b/x-pack/plugins/synthetics/server/synthetics_service/synthetics_service.ts @@ -361,16 +361,22 @@ export class SyntheticsService { } async deleteConfigs(configs: SyntheticsMonitorWithId[]) { - const output = await this.getOutput(); - if (!output) { - return; - } + const hasPublicLocations = configs.some(({ locations }) => + locations.some(({ isServiceManaged }) => isServiceManaged) + ); - const data = { - output, - monitors: this.formatConfigs(configs), - }; - return await this.apiClient.delete(data); + if (hasPublicLocations) { + const output = await this.getOutput(); + if (!output) { + return; + } + + const data = { + output, + monitors: this.formatConfigs(configs), + }; + return await this.apiClient.delete(data); + } } async deleteAllConfigs() { diff --git a/x-pack/test/api_integration/apis/synthetics/delete_monitor.ts b/x-pack/test/api_integration/apis/synthetics/delete_monitor.ts index 6915ddf26108b..3b9818f2d605d 100644 --- a/x-pack/test/api_integration/apis/synthetics/delete_monitor.ts +++ b/x-pack/test/api_integration/apis/synthetics/delete_monitor.ts @@ -13,7 +13,7 @@ import { getFixtureJson } from '../uptime/rest/helper/get_fixture_json'; import { PrivateLocationTestService } from './services/private_location_test_service'; export default function ({ getService }: FtrProviderContext) { - describe('[DELETE] /internal/uptime/service/monitors', function () { + describe('DeleteMonitorRoute', function () { this.tags('skipCloud'); const supertest = getService('supertest');