From ee012f786872df1475ccdba6748e9e5611ffd27b Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Thu, 27 Aug 2020 17:57:49 -0700 Subject: [PATCH 1/7] Revert back ESO migration for alerting, added try/catch logic to avoid failing Kibana on start --- .../server/saved_objects/migrations.test.ts | 121 ++++++++++++++++++ .../alerts/server/saved_objects/migrations.ts | 60 +++++++++ .../spaces_only/tests/alerting/migrations.ts | 43 +++++++ 3 files changed, 224 insertions(+) create mode 100644 x-pack/plugins/alerts/server/saved_objects/migrations.test.ts create mode 100644 x-pack/plugins/alerts/server/saved_objects/migrations.ts create mode 100644 x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts new file mode 100644 index 0000000000000..19f4e918b7862 --- /dev/null +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts @@ -0,0 +1,121 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import uuid from 'uuid'; +import { getMigrations } from './migrations'; +import { RawAlert } from '../types'; +import { SavedObjectUnsanitizedDoc } from 'kibana/server'; +import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/mocks'; +import { migrationMocks } from 'src/core/server/mocks'; + +const { log } = migrationMocks.createContext(); +const encryptedSavedObjectsSetup = encryptedSavedObjectsMock.createSetup(); + +describe('7.9.0', () => { + beforeEach(() => { + jest.resetAllMocks(); + encryptedSavedObjectsSetup.createMigration.mockImplementation( + (shouldMigrateWhenPredicate, migration) => migration + ); + }); + + test('changes nothing on alerts by other plugins', () => { + const migration790 = getMigrations(encryptedSavedObjectsSetup)['7.9.0']; + const alert = getMockData({}); + expect(migration790(alert, { log })).toMatchObject(alert); + + expect(encryptedSavedObjectsSetup.createMigration).toHaveBeenCalledWith( + expect.any(Function), + expect.any(Function) + ); + }); + + test('migrates the consumer for alerting', () => { + const migration790 = getMigrations(encryptedSavedObjectsSetup)['7.9.0']; + const alert = getMockData({ + consumer: 'alerting', + }); + expect(migration790(alert, { log })).toMatchObject({ + ...alert, + attributes: { + ...alert.attributes, + consumer: 'alerts', + }, + }); + }); +}); + +describe('7.10.0', () => { + beforeEach(() => { + jest.resetAllMocks(); + encryptedSavedObjectsSetup.createMigration.mockImplementation( + (shouldMigrateWhenPredicate, migration) => migration + ); + }); + + test('changes nothing on alerts by other plugins', () => { + const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; + const alert = getMockData({}); + expect(migration710(alert, { log })).toMatchObject(alert); + + expect(encryptedSavedObjectsSetup.createMigration).toHaveBeenCalledWith( + expect.any(Function), + expect.any(Function) + ); + }); + + test('migrates the consumer for metrics', () => { + const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; + const alert = getMockData({ + consumer: 'metrics', + }); + expect(migration710(alert, { log })).toMatchObject({ + ...alert, + attributes: { + ...alert.attributes, + consumer: 'infrastructure', + }, + }); + }); +}); + +function getMockData( + overwrites: Record = {} +): SavedObjectUnsanitizedDoc { + return { + attributes: { + enabled: true, + name: 'abc', + tags: ['foo'], + alertTypeId: '123', + consumer: 'bar', + apiKey: '', + apiKeyOwner: '', + schedule: { interval: '10s' }, + throttle: null, + params: { + bar: true, + }, + muteAll: false, + mutedInstanceIds: [], + createdBy: new Date().toISOString(), + updatedBy: new Date().toISOString(), + createdAt: new Date().toISOString(), + actions: [ + { + group: 'default', + actionRef: '1', + actionTypeId: '1', + params: { + foo: true, + }, + }, + ], + ...overwrites, + }, + id: uuid.v4(), + type: 'alert', + }; +} diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.ts new file mode 100644 index 0000000000000..213fe6323b1d9 --- /dev/null +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.ts @@ -0,0 +1,60 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { + SavedObjectMigrationMap, + SavedObjectUnsanitizedDoc, + SavedObjectMigrationFn, + Logger +} from '../../../../../src/core/server'; +import { RawAlert } from '../types'; +import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; + +export function getMigrations( + encryptedSavedObjects: EncryptedSavedObjectsPluginSetup, + eventLogger: Logger +): SavedObjectMigrationMap { + const migrations: SavedObjectMigrationMap = {}; + + const alertsMigration = changeAlertingConsumer(encryptedSavedObjects, 'alerting', 'alerts', eventLogger); + if (alertsMigration) { + migrations['7.9.0'] = alertsMigration; + } + + const infrastructureMigration = changeAlertingConsumer(encryptedSavedObjects, 'metrics', 'infrastructure', eventLogger); + if (infrastructureMigration) { + migrations['7.10.0'] = infrastructureMigration; + } + return migrations; +} + +function changeAlertingConsumer( + encryptedSavedObjects: EncryptedSavedObjectsPluginSetup, + from: string, + to: string, + eventLogger: Logger +): SavedObjectMigrationFn | undefined { + try { + return encryptedSavedObjects.createMigration( + function shouldBeMigrated(doc): doc is SavedObjectUnsanitizedDoc { + return doc.attributes.consumer === from; + }, + (doc: SavedObjectUnsanitizedDoc): SavedObjectUnsanitizedDoc => { + const { + attributes: { consumer }, + } = doc; + return { + ...doc, + attributes: { + ...doc.attributes, + consumer: consumer === from ? to : consumer, + }, + }; + } + ); + } catch (ex) { + eventLogger.error(`encryptedSavedObject migration from ${from} to ${to} failed with error: ${ex.message}`) + } +} diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts new file mode 100644 index 0000000000000..d0e1be12e762f --- /dev/null +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts @@ -0,0 +1,43 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import expect from '@kbn/expect'; +import { getUrlPrefix } from '../../../common/lib'; +import { FtrProviderContext } from '../../../common/ftr_provider_context'; + +// eslint-disable-next-line import/no-default-export +export default function createGetTests({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + const esArchiver = getService('esArchiver'); + + describe('migrations', () => { + before(async () => { + await esArchiver.load('alerts'); + }); + + after(async () => { + await esArchiver.unload('alerts'); + }); + + it('7.9.0 migrates the `alerting` consumer to be the `alerts`', async () => { + const response = await supertest.get( + `${getUrlPrefix(``)}/api/alerts/alert/74f3e6d7-b7bb-477d-ac28-92ee22728e6e` + ); + + expect(response.status).to.eql(200); + expect(response.body.consumer).to.equal('alerts'); + }); + + it('7.10.0 migrates the `metrics` consumer to be the `infrastructure`', async () => { + const response = await supertest.get( + `${getUrlPrefix(``)}/api/alerts/alert/74f3e6d7-b7bb-477d-ac28-fdf248d5f2a4` + ); + + expect(response.status).to.eql(200); + expect(response.body.consumer).to.equal('infrastructure'); + }); + }); +} From c685d12eb57012c4e00d34658e7333d1450048b3 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Thu, 27 Aug 2020 17:58:22 -0700 Subject: [PATCH 2/7] Revert back ESO migration for alerting, added try/catch logic to avoid failing Kibana on start --- x-pack/plugins/alerts/server/plugin.ts | 10 ++++++---- x-pack/plugins/alerts/server/saved_objects/index.ts | 8 +++++++- .../spaces_only/tests/alerting/index.ts | 3 +++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index d5843bd531d84..653f8bd2ef9af 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -152,14 +152,16 @@ export class AlertingPlugin { ); } - setupSavedObjects(core.savedObjects, plugins.encryptedSavedObjects); - - this.eventLogService = plugins.eventLog; - plugins.eventLog.registerProviderActions(EVENT_LOG_PROVIDER, Object.values(EVENT_LOG_ACTIONS)); this.eventLogger = plugins.eventLog.getLogger({ event: { provider: EVENT_LOG_PROVIDER }, }); + setupSavedObjects(core.savedObjects, plugins.encryptedSavedObjects, this.logger); + + this.eventLogService = plugins.eventLog; + plugins.eventLog.registerProviderActions(EVENT_LOG_PROVIDER, Object.values(EVENT_LOG_ACTIONS)); + + const alertTypeRegistry = new AlertTypeRegistry({ taskManager: plugins.taskManager, taskRunnerFactory: this.taskRunnerFactory, diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index c98d9bcbd9ae5..2719c55dfb8bb 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -6,16 +6,22 @@ import { SavedObjectsServiceSetup } from 'kibana/server'; import mappings from './mappings.json'; +import { getMigrations } from './migrations'; import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; +import { + Logger +} from '../../../../../src/core/server'; export function setupSavedObjects( savedObjects: SavedObjectsServiceSetup, - encryptedSavedObjects: EncryptedSavedObjectsPluginSetup + encryptedSavedObjects: EncryptedSavedObjectsPluginSetup, + logger: Logger ) { savedObjects.registerType({ name: 'alert', hidden: true, namespaceType: 'single', + migrations: getMigrations(encryptedSavedObjects, logger), mappings: mappings.alert, }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts index b927b563eb54a..78ca2af12ec3f 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts @@ -28,5 +28,8 @@ export default function alertingTests({ loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./alerts_space1')); loadTestFile(require.resolve('./alerts_default_space')); loadTestFile(require.resolve('./builtin_alert_types')); + + // note that this test will destroy existing spaces + loadTestFile(require.resolve('./migrations')); }); } From d9267dbe9418e4407d3ba0a66ffe73c28ed66bbf Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Tue, 1 Sep 2020 14:08:44 -0700 Subject: [PATCH 3/7] fixed due to comments --- x-pack/plugins/alerts/server/plugin.ts | 1 - .../alerts/server/saved_objects/index.ts | 4 +- .../server/saved_objects/migrations.test.ts | 11 ++- .../alerts/server/saved_objects/migrations.ts | 88 +++++++++++-------- 4 files changed, 61 insertions(+), 43 deletions(-) diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index 653f8bd2ef9af..4218b02bf6fc0 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -160,7 +160,6 @@ export class AlertingPlugin { this.eventLogService = plugins.eventLog; plugins.eventLog.registerProviderActions(EVENT_LOG_PROVIDER, Object.values(EVENT_LOG_ACTIONS)); - const alertTypeRegistry = new AlertTypeRegistry({ taskManager: plugins.taskManager, diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index 2719c55dfb8bb..6d9452eac79b7 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -8,9 +8,7 @@ import { SavedObjectsServiceSetup } from 'kibana/server'; import mappings from './mappings.json'; import { getMigrations } from './migrations'; import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; -import { - Logger -} from '../../../../../src/core/server'; +import { Logger } from '../../../../../src/core/server'; export function setupSavedObjects( savedObjects: SavedObjectsServiceSetup, diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts index 19f4e918b7862..082ebb8f5cfae 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts @@ -6,9 +6,12 @@ import uuid from 'uuid'; import { getMigrations } from './migrations'; import { RawAlert } from '../types'; +import { Logger } from '../../../../../src/core/server'; import { SavedObjectUnsanitizedDoc } from 'kibana/server'; import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/mocks'; import { migrationMocks } from 'src/core/server/mocks'; +import { loggingSystemMock } from '../../../../../src/core/server/mocks'; +const logger = loggingSystemMock.create().get() as jest.Mocked; const { log } = migrationMocks.createContext(); const encryptedSavedObjectsSetup = encryptedSavedObjectsMock.createSetup(); @@ -22,7 +25,7 @@ describe('7.9.0', () => { }); test('changes nothing on alerts by other plugins', () => { - const migration790 = getMigrations(encryptedSavedObjectsSetup)['7.9.0']; + const migration790 = getMigrations(encryptedSavedObjectsSetup, logger)['7.10.0']; const alert = getMockData({}); expect(migration790(alert, { log })).toMatchObject(alert); @@ -33,7 +36,7 @@ describe('7.9.0', () => { }); test('migrates the consumer for alerting', () => { - const migration790 = getMigrations(encryptedSavedObjectsSetup)['7.9.0']; + const migration790 = getMigrations(encryptedSavedObjectsSetup, logger)['7.10.0']; const alert = getMockData({ consumer: 'alerting', }); @@ -56,7 +59,7 @@ describe('7.10.0', () => { }); test('changes nothing on alerts by other plugins', () => { - const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; + const migration710 = getMigrations(encryptedSavedObjectsSetup, logger)['7.10.0']; const alert = getMockData({}); expect(migration710(alert, { log })).toMatchObject(alert); @@ -67,7 +70,7 @@ describe('7.10.0', () => { }); test('migrates the consumer for metrics', () => { - const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; + const migration710 = getMigrations(encryptedSavedObjectsSetup, logger)['7.10.0']; const alert = getMockData({ consumer: 'metrics', }); diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.ts index 213fe6323b1d9..3ffd140af5a60 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.ts @@ -7,54 +7,72 @@ import { SavedObjectMigrationMap, SavedObjectUnsanitizedDoc, SavedObjectMigrationFn, - Logger + SavedObjectMigrationContext, + Logger, } from '../../../../../src/core/server'; import { RawAlert } from '../types'; import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; export function getMigrations( encryptedSavedObjects: EncryptedSavedObjectsPluginSetup, - eventLogger: Logger + logger: Logger ): SavedObjectMigrationMap { - const migrations: SavedObjectMigrationMap = {}; + const alertsMigration = changeAlertingConsumer(encryptedSavedObjects, 'alerting', 'alerts'); - const alertsMigration = changeAlertingConsumer(encryptedSavedObjects, 'alerting', 'alerts', eventLogger); - if (alertsMigration) { - migrations['7.9.0'] = alertsMigration; - } + const infrastructureMigration = changeAlertingConsumer( + encryptedSavedObjects, + 'metrics', + 'infrastructure' + ); + + return { + '7.10.0': (doc: SavedObjectUnsanitizedDoc, context: SavedObjectMigrationContext) => { + if (doc.attributes.consumer === 'alerting') { + return executeMigration(doc, context, alertsMigration); + } else if (doc.attributes.consumer === 'metrics') { + return executeMigration(doc, context, infrastructureMigration); + } + return doc; + }, + }; +} - const infrastructureMigration = changeAlertingConsumer(encryptedSavedObjects, 'metrics', 'infrastructure', eventLogger); - if (infrastructureMigration) { - migrations['7.10.0'] = infrastructureMigration; +function executeMigration( + doc: SavedObjectUnsanitizedDoc, + context: SavedObjectMigrationContext, + migrationFunc: SavedObjectMigrationFn +) { + try { + return migrationFunc(doc, context); + } catch (ex) { + context.log.error( + `encryptedSavedObject migration failed for alert ${doc.id} with error: ${ex.message}`, + { alertDocument: doc } + ); } - return migrations; + return doc; } function changeAlertingConsumer( encryptedSavedObjects: EncryptedSavedObjectsPluginSetup, from: string, - to: string, - eventLogger: Logger -): SavedObjectMigrationFn | undefined { - try { - return encryptedSavedObjects.createMigration( - function shouldBeMigrated(doc): doc is SavedObjectUnsanitizedDoc { - return doc.attributes.consumer === from; - }, - (doc: SavedObjectUnsanitizedDoc): SavedObjectUnsanitizedDoc => { - const { - attributes: { consumer }, - } = doc; - return { - ...doc, - attributes: { - ...doc.attributes, - consumer: consumer === from ? to : consumer, - }, - }; - } - ); - } catch (ex) { - eventLogger.error(`encryptedSavedObject migration from ${from} to ${to} failed with error: ${ex.message}`) - } + to: string +): SavedObjectMigrationFn { + return encryptedSavedObjects.createMigration( + function shouldBeMigrated(doc): doc is SavedObjectUnsanitizedDoc { + return doc.attributes.consumer === from; + }, + (doc: SavedObjectUnsanitizedDoc): SavedObjectUnsanitizedDoc => { + const { + attributes: { consumer }, + } = doc; + return { + ...doc, + attributes: { + ...doc.attributes, + consumer: consumer === from ? to : consumer, + }, + }; + } + ); } From ba01d4171a5f4826a4535ae1bbc882659a9b85b2 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Tue, 1 Sep 2020 14:21:46 -0700 Subject: [PATCH 4/7] removed unused logger --- x-pack/plugins/alerts/server/plugin.ts | 2 +- x-pack/plugins/alerts/server/saved_objects/index.ts | 5 ++--- .../alerts/server/saved_objects/migrations.test.ts | 11 ++++------- .../plugins/alerts/server/saved_objects/migrations.ts | 3 +-- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index 4218b02bf6fc0..b16ded9fb5c91 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -156,7 +156,7 @@ export class AlertingPlugin { event: { provider: EVENT_LOG_PROVIDER }, }); - setupSavedObjects(core.savedObjects, plugins.encryptedSavedObjects, this.logger); + setupSavedObjects(core.savedObjects, plugins.encryptedSavedObjects); this.eventLogService = plugins.eventLog; plugins.eventLog.registerProviderActions(EVENT_LOG_PROVIDER, Object.values(EVENT_LOG_ACTIONS)); diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index 6d9452eac79b7..2421a8bfae595 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -12,14 +12,13 @@ import { Logger } from '../../../../../src/core/server'; export function setupSavedObjects( savedObjects: SavedObjectsServiceSetup, - encryptedSavedObjects: EncryptedSavedObjectsPluginSetup, - logger: Logger + encryptedSavedObjects: EncryptedSavedObjectsPluginSetup ) { savedObjects.registerType({ name: 'alert', hidden: true, namespaceType: 'single', - migrations: getMigrations(encryptedSavedObjects, logger), + migrations: getMigrations(encryptedSavedObjects), mappings: mappings.alert, }); diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts index 082ebb8f5cfae..ec746fe1f5b2d 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts @@ -6,12 +6,9 @@ import uuid from 'uuid'; import { getMigrations } from './migrations'; import { RawAlert } from '../types'; -import { Logger } from '../../../../../src/core/server'; import { SavedObjectUnsanitizedDoc } from 'kibana/server'; import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/mocks'; import { migrationMocks } from 'src/core/server/mocks'; -import { loggingSystemMock } from '../../../../../src/core/server/mocks'; -const logger = loggingSystemMock.create().get() as jest.Mocked; const { log } = migrationMocks.createContext(); const encryptedSavedObjectsSetup = encryptedSavedObjectsMock.createSetup(); @@ -25,7 +22,7 @@ describe('7.9.0', () => { }); test('changes nothing on alerts by other plugins', () => { - const migration790 = getMigrations(encryptedSavedObjectsSetup, logger)['7.10.0']; + const migration790 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; const alert = getMockData({}); expect(migration790(alert, { log })).toMatchObject(alert); @@ -36,7 +33,7 @@ describe('7.9.0', () => { }); test('migrates the consumer for alerting', () => { - const migration790 = getMigrations(encryptedSavedObjectsSetup, logger)['7.10.0']; + const migration790 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; const alert = getMockData({ consumer: 'alerting', }); @@ -59,7 +56,7 @@ describe('7.10.0', () => { }); test('changes nothing on alerts by other plugins', () => { - const migration710 = getMigrations(encryptedSavedObjectsSetup, logger)['7.10.0']; + const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; const alert = getMockData({}); expect(migration710(alert, { log })).toMatchObject(alert); @@ -70,7 +67,7 @@ describe('7.10.0', () => { }); test('migrates the consumer for metrics', () => { - const migration710 = getMigrations(encryptedSavedObjectsSetup, logger)['7.10.0']; + const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; const alert = getMockData({ consumer: 'metrics', }); diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.ts index 3ffd140af5a60..390c2ef05e4d3 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.ts @@ -14,8 +14,7 @@ import { RawAlert } from '../types'; import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; export function getMigrations( - encryptedSavedObjects: EncryptedSavedObjectsPluginSetup, - logger: Logger + encryptedSavedObjects: EncryptedSavedObjectsPluginSetup ): SavedObjectMigrationMap { const alertsMigration = changeAlertingConsumer(encryptedSavedObjects, 'alerting', 'alerts'); From cc3d560b0fce0a48da28f11388bb369482999669 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Tue, 1 Sep 2020 14:26:30 -0700 Subject: [PATCH 5/7] fixed type checks --- .../alerts/server/saved_objects/index.ts | 1 - .../server/saved_objects/migrations.test.ts | 46 ++++++------------- .../alerts/server/saved_objects/migrations.ts | 1 - 3 files changed, 13 insertions(+), 35 deletions(-) diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index 2421a8bfae595..06ce8d673e6b7 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -8,7 +8,6 @@ import { SavedObjectsServiceSetup } from 'kibana/server'; import mappings from './mappings.json'; import { getMigrations } from './migrations'; import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; -import { Logger } from '../../../../../src/core/server'; export function setupSavedObjects( savedObjects: SavedObjectsServiceSetup, diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts index ec746fe1f5b2d..e3ca88331cfae 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts @@ -13,7 +13,7 @@ import { migrationMocks } from 'src/core/server/mocks'; const { log } = migrationMocks.createContext(); const encryptedSavedObjectsSetup = encryptedSavedObjectsMock.createSetup(); -describe('7.9.0', () => { +describe('7.10.0', () => { beforeEach(() => { jest.resetAllMocks(); encryptedSavedObjectsSetup.createMigration.mockImplementation( @@ -22,9 +22,9 @@ describe('7.9.0', () => { }); test('changes nothing on alerts by other plugins', () => { - const migration790 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; + const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; const alert = getMockData({}); - expect(migration790(alert, { log })).toMatchObject(alert); + expect(migration710(alert, { log })).toMatchObject(alert); expect(encryptedSavedObjectsSetup.createMigration).toHaveBeenCalledWith( expect.any(Function), @@ -32,50 +32,30 @@ describe('7.9.0', () => { ); }); - test('migrates the consumer for alerting', () => { - const migration790 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; + test('migrates the consumer for metrics', () => { + const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; const alert = getMockData({ - consumer: 'alerting', + consumer: 'metrics', }); - expect(migration790(alert, { log })).toMatchObject({ + expect(migration710(alert, { log })).toMatchObject({ ...alert, attributes: { ...alert.attributes, - consumer: 'alerts', + consumer: 'infrastructure', }, }); }); -}); - -describe('7.10.0', () => { - beforeEach(() => { - jest.resetAllMocks(); - encryptedSavedObjectsSetup.createMigration.mockImplementation( - (shouldMigrateWhenPredicate, migration) => migration - ); - }); - - test('changes nothing on alerts by other plugins', () => { - const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; - const alert = getMockData({}); - expect(migration710(alert, { log })).toMatchObject(alert); - - expect(encryptedSavedObjectsSetup.createMigration).toHaveBeenCalledWith( - expect.any(Function), - expect.any(Function) - ); - }); - test('migrates the consumer for metrics', () => { - const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; + test('migrates the consumer for alerting', () => { + const migration790 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; const alert = getMockData({ - consumer: 'metrics', + consumer: 'alerting', }); - expect(migration710(alert, { log })).toMatchObject({ + expect(migration790(alert, { log })).toMatchObject({ ...alert, attributes: { ...alert.attributes, - consumer: 'infrastructure', + consumer: 'alerts', }, }); }); diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.ts index 390c2ef05e4d3..30570eeb0a453 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.ts @@ -8,7 +8,6 @@ import { SavedObjectUnsanitizedDoc, SavedObjectMigrationFn, SavedObjectMigrationContext, - Logger, } from '../../../../../src/core/server'; import { RawAlert } from '../types'; import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; From af752ac3c6cf35634cddd0768f5fdd2b1604b151 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Tue, 1 Sep 2020 14:29:38 -0700 Subject: [PATCH 6/7] did renaming from 7.9 to 7.10 --- x-pack/plugins/alerts/server/saved_objects/migrations.test.ts | 4 ++-- .../spaces_only/tests/alerting/migrations.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts index e3ca88331cfae..99078b68bce78 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts @@ -47,11 +47,11 @@ describe('7.10.0', () => { }); test('migrates the consumer for alerting', () => { - const migration790 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; + const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; const alert = getMockData({ consumer: 'alerting', }); - expect(migration790(alert, { log })).toMatchObject({ + expect(migration710(alert, { log })).toMatchObject({ ...alert, attributes: { ...alert.attributes, diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts index d0e1be12e762f..81f7c8c97ba8c 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts @@ -22,7 +22,7 @@ export default function createGetTests({ getService }: FtrProviderContext) { await esArchiver.unload('alerts'); }); - it('7.9.0 migrates the `alerting` consumer to be the `alerts`', async () => { + it('7.10.0 migrates the `alerting` consumer to be the `alerts`', async () => { const response = await supertest.get( `${getUrlPrefix(``)}/api/alerts/alert/74f3e6d7-b7bb-477d-ac28-92ee22728e6e` ); From 6cca8ecd9332e23e9fe1c3225ffa89e128a28e88 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Thu, 3 Sep 2020 14:13:56 -0700 Subject: [PATCH 7/7] Added migration failure unit test --- .../server/saved_objects/migrations.test.ts | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts index 99078b68bce78..46fa2bcd512ff 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts @@ -61,6 +61,40 @@ describe('7.10.0', () => { }); }); +describe('7.10.0 migrates with failure', () => { + beforeEach(() => { + jest.resetAllMocks(); + encryptedSavedObjectsSetup.createMigration.mockRejectedValueOnce( + new Error(`Can't migrate!`) as never + ); + }); + + test('should show the proper exception', () => { + const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; + const alert = getMockData({ + consumer: 'alerting', + }); + const res = migration710(alert, { log }); + expect(res).toMatchObject({ + ...alert, + attributes: { + ...alert.attributes, + }, + }); + expect(log.error).toHaveBeenCalledWith( + `encryptedSavedObject migration failed for alert ${alert.id} with error: migrationFunc is not a function`, + { + alertDocument: { + ...alert, + attributes: { + ...alert.attributes, + }, + }, + } + ); + }); +}); + function getMockData( overwrites: Record = {} ): SavedObjectUnsanitizedDoc {