From 05fba9bd9c75774880f9ef2ddc11ead3970d0cce Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Thu, 3 Sep 2020 17:13:17 -0700 Subject: [PATCH] Revert back ESO migration for alerting, added try/catch logic to avoid failing Kibana on start (#76220) * Revert back ESO migration for alerting, added try/catch logic to avoid failing Kibana on start * Revert back ESO migration for alerting, added try/catch logic to avoid failing Kibana on start * fixed due to comments * removed unused logger * fixed type checks * did renaming from 7.9 to 7.10 * Added migration failure unit test --- x-pack/plugins/alerts/server/plugin.ts | 7 +- .../alerts/server/saved_objects/index.ts | 2 + .../server/saved_objects/migrations.test.ts | 135 ++++++++++++++++++ .../alerts/server/saved_objects/migrations.ts | 76 ++++++++++ .../spaces_only/tests/alerting/index.ts | 3 + .../spaces_only/tests/alerting/migrations.ts | 43 ++++++ 6 files changed, 263 insertions(+), 3 deletions(-) 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/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index d5843bd531d8..b16ded9fb5c9 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -152,13 +152,14 @@ export class AlertingPlugin { ); } + this.eventLogger = plugins.eventLog.getLogger({ + event: { provider: EVENT_LOG_PROVIDER }, + }); + 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 }, - }); 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 c98d9bcbd9ae..06ce8d673e6b 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -6,6 +6,7 @@ import { SavedObjectsServiceSetup } from 'kibana/server'; import mappings from './mappings.json'; +import { getMigrations } from './migrations'; import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; export function setupSavedObjects( @@ -16,6 +17,7 @@ export function setupSavedObjects( name: 'alert', hidden: true, namespaceType: 'single', + 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 new file mode 100644 index 000000000000..46fa2bcd512f --- /dev/null +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts @@ -0,0 +1,135 @@ +/* + * 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.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', + }, + }); + }); + + test('migrates the consumer for alerting', () => { + const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; + const alert = getMockData({ + consumer: 'alerting', + }); + expect(migration710(alert, { log })).toMatchObject({ + ...alert, + attributes: { + ...alert.attributes, + consumer: 'alerts', + }, + }); + }); +}); + +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 { + 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 000000000000..30570eeb0a45 --- /dev/null +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.ts @@ -0,0 +1,76 @@ +/* + * 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, + SavedObjectMigrationContext, +} from '../../../../../src/core/server'; +import { RawAlert } from '../types'; +import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; + +export function getMigrations( + encryptedSavedObjects: EncryptedSavedObjectsPluginSetup +): SavedObjectMigrationMap { + const alertsMigration = changeAlertingConsumer(encryptedSavedObjects, 'alerting', 'alerts'); + + 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; + }, + }; +} + +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 doc; +} + +function changeAlertingConsumer( + encryptedSavedObjects: EncryptedSavedObjectsPluginSetup, + from: string, + 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, + }, + }; + } + ); +} 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 b927b563eb54..78ca2af12ec3 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')); }); } 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 000000000000..81f7c8c97ba8 --- /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.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` + ); + + 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'); + }); + }); +}