From fc82586a716387947316e8ce2a0839c504ab2cfd Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Mon, 22 Feb 2021 23:12:36 +0100 Subject: [PATCH] v2 migrations should exit process on corrupt saved object document (#91465) (#92273) * Fail migrations if a corrupt saved object is encountered * Update test description * Use an error class instead of string matching Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../migrations/core/document_migrator.test.ts | 15 ++++++----- .../migrations/core/document_migrator.ts | 5 ++-- .../migrations/core/migrate_raw_docs.test.ts | 26 +++++-------------- .../migrations/core/migrate_raw_docs.ts | 25 +++++++++++++----- .../migrations/kibana/kibana_migrator.test.ts | 2 +- .../migrations_state_action_machine.test.ts | 13 +++++++--- .../migrations_state_action_machine.ts | 24 ++++++++++++++--- 7 files changed, 67 insertions(+), 43 deletions(-) diff --git a/src/core/server/saved_objects/migrations/core/document_migrator.test.ts b/src/core/server/saved_objects/migrations/core/document_migrator.test.ts index f29a8b61b4885..1cf408ea96a56 100644 --- a/src/core/server/saved_objects/migrations/core/document_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/document_migrator.test.ts @@ -722,7 +722,7 @@ describe('DocumentMigrator', () => { }); }); - it('logs the document and transform that failed', () => { + it('logs the original error and throws a transform error if a document transform fails', () => { const log = mockLogger; const migrator = new DocumentMigrator({ ...testOpts(), @@ -747,10 +747,13 @@ describe('DocumentMigrator', () => { migrator.migrate(_.cloneDeep(failedDoc)); expect('Did not throw').toEqual('But it should have!'); } catch (error) { - expect(error.message).toMatch(/Dang diggity!/); - const warning = loggingSystemMock.collect(mockLoggerFactory).warn[0][0]; - expect(warning).toContain(JSON.stringify(failedDoc)); - expect(warning).toContain('dog:1.2.3'); + expect(error.message).toMatchInlineSnapshot(` + "Failed to transform document smelly. Transform: dog:1.2.3 + Doc: {\\"id\\":\\"smelly\\",\\"type\\":\\"dog\\",\\"attributes\\":{},\\"migrationVersion\\":{}}" + `); + expect(loggingSystemMock.collect(mockLoggerFactory).error[0][0]).toMatchInlineSnapshot( + `[Error: Dang diggity!]` + ); } }); @@ -779,7 +782,7 @@ describe('DocumentMigrator', () => { }; migrator.migrate(doc); expect(loggingSystemMock.collect(mockLoggerFactory).info[0][0]).toEqual(logTestMsg); - expect(loggingSystemMock.collect(mockLoggerFactory).warn[1][0]).toEqual(logTestMsg); + expect(loggingSystemMock.collect(mockLoggerFactory).warn[0][0]).toEqual(logTestMsg); }); test('extracts the latest migration version info', () => { diff --git a/src/core/server/saved_objects/migrations/core/document_migrator.ts b/src/core/server/saved_objects/migrations/core/document_migrator.ts index 47f4dda75cdcd..cccd38bf5cc9e 100644 --- a/src/core/server/saved_objects/migrations/core/document_migrator.ts +++ b/src/core/server/saved_objects/migrations/core/document_migrator.ts @@ -678,10 +678,11 @@ function wrapWithTry( } catch (error) { const failedTransform = `${type.name}:${version}`; const failedDoc = JSON.stringify(doc); - log.warn( + log.error(error); + + throw new Error( `Failed to transform document ${doc?.id}. Transform: ${failedTransform}\nDoc: ${failedDoc}` ); - throw error; } }; } diff --git a/src/core/server/saved_objects/migrations/core/migrate_raw_docs.test.ts b/src/core/server/saved_objects/migrations/core/migrate_raw_docs.test.ts index 80565c8c887c2..66750a8abf1db 100644 --- a/src/core/server/saved_objects/migrations/core/migrate_raw_docs.test.ts +++ b/src/core/server/saved_objects/migrations/core/migrate_raw_docs.test.ts @@ -58,12 +58,12 @@ describe('migrateRawDocs', () => { expect(transform).toHaveBeenNthCalledWith(2, obj2); }); - test('passes invalid docs through untouched and logs error', async () => { + test('throws when encountering a corrupt saved object document', async () => { const logger = createSavedObjectsMigrationLoggerMock(); const transform = jest.fn((doc: any) => [ set(_.cloneDeep(doc), 'attributes.name', 'TADA'), ]); - const result = await migrateRawDocs( + const result = migrateRawDocs( new SavedObjectsSerializer(new SavedObjectTypeRegistry()), transform, [ @@ -73,25 +73,11 @@ describe('migrateRawDocs', () => { logger ); - expect(result).toEqual([ - { _id: 'foo:b', _source: { type: 'a', a: { name: 'AAA' } } }, - { - _id: 'c:d', - _source: { type: 'c', c: { name: 'TADA' }, migrationVersion: {}, references: [] }, - }, - ]); - - const obj2 = { - id: 'd', - type: 'c', - attributes: { name: 'DDD' }, - migrationVersion: {}, - references: [], - }; - expect(transform).toHaveBeenCalledTimes(1); - expect(transform).toHaveBeenCalledWith(obj2); + expect(result).rejects.toMatchInlineSnapshot( + `[Error: Unable to migrate the corrupt saved object document with _id: 'foo:b'.]` + ); - expect(logger.error).toBeCalledTimes(1); + expect(transform).toHaveBeenCalledTimes(0); }); test('handles when one document is transformed into multiple documents', async () => { diff --git a/src/core/server/saved_objects/migrations/core/migrate_raw_docs.ts b/src/core/server/saved_objects/migrations/core/migrate_raw_docs.ts index 74c599668a2f8..e75f29e54c876 100644 --- a/src/core/server/saved_objects/migrations/core/migrate_raw_docs.ts +++ b/src/core/server/saved_objects/migrations/core/migrate_raw_docs.ts @@ -18,6 +18,23 @@ import { import { MigrateAndConvertFn } from './document_migrator'; import { SavedObjectsMigrationLogger } from '.'; +/** + * Error thrown when saved object migrations encounter a corrupt saved object. + * Corrupt saved objects cannot be serialized because: + * - there's no `[type]` property which contains the type attributes + * - the type or namespace in the _id doesn't match the `type` or `namespace` + * properties + */ +export class CorruptSavedObjectError extends Error { + constructor(public readonly rawId: string) { + super(`Unable to migrate the corrupt saved object document with _id: '${rawId}'.`); + + // Set the prototype explicitly, see: + // https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work + Object.setPrototypeOf(this, CorruptSavedObjectError.prototype); + } +} + /** * Applies the specified migration function to every saved object document in the list * of raw docs. Any raw docs that are not valid saved objects will simply be passed through. @@ -35,7 +52,7 @@ export async function migrateRawDocs( const migrateDocWithoutBlocking = transformNonBlocking(migrateDoc); const processedDocs = []; for (const raw of rawDocs) { - const options = { namespaceTreatment: 'lax' as 'lax' }; + const options = { namespaceTreatment: 'lax' as const }; if (serializer.isRawSavedObject(raw, options)) { const savedObject = serializer.rawToSavedObject(raw, options); savedObject.migrationVersion = savedObject.migrationVersion || {}; @@ -48,11 +65,7 @@ export async function migrateRawDocs( ) ); } else { - log.error( - `Error: Unable to migrate the corrupt Saved Object document ${raw._id}. To prevent Kibana from performing a migration on every restart, please delete or fix this document by ensuring that the namespace and type in the document's id matches the values in the namespace and type fields.`, - { rawDocument: raw } - ); - processedDocs.push(raw); + throw new CorruptSavedObjectError(raw._id); } } return processedDocs; diff --git a/src/core/server/saved_objects/migrations/kibana/kibana_migrator.test.ts b/src/core/server/saved_objects/migrations/kibana/kibana_migrator.test.ts index 84e5aa3f9c2e3..b8accc462df9a 100644 --- a/src/core/server/saved_objects/migrations/kibana/kibana_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/kibana/kibana_migrator.test.ts @@ -330,7 +330,7 @@ describe('KibanaMigrator', () => { const migrator = new KibanaMigrator(options); migrator.prepareMigrations(); await expect(migrator.runMigrations()).rejects.toMatchInlineSnapshot(` - [Error: Unable to complete saved object migrations for the [.my-index] index. Please check the health of your Elasticsearch cluster and try again. Error: Reindex failed with the following error: + [Error: Unable to complete saved object migrations for the [.my-index] index. Error: Reindex failed with the following error: {"_tag":"Some","value":{"type":"elatsicsearch_exception","reason":"task failed with an error"}}] `); expect(loggingSystemMock.collect(options.logger).error[0][0]).toMatchInlineSnapshot(` diff --git a/src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.test.ts b/src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.test.ts index f452928684267..f7b9c4c368fa0 100644 --- a/src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.test.ts +++ b/src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.test.ts @@ -314,20 +314,25 @@ describe('migrationsStateActionMachine', () => { next: () => { throw new ResponseError( elasticsearchClientMock.createApiResponse({ - body: { error: { type: 'snapshot_in_progress_exception', reason: 'error reason' } }, + body: { + error: { + type: 'snapshot_in_progress_exception', + reason: 'Cannot delete indices that are being snapshotted', + }, + }, }) ); }, }) ).rejects.toMatchInlineSnapshot( - `[Error: Unable to complete saved object migrations for the [.my-so-index] index. Please check the health of your Elasticsearch cluster and try again. ResponseError: snapshot_in_progress_exception]` + `[Error: Unable to complete saved object migrations for the [.my-so-index] index. Please check the health of your Elasticsearch cluster and try again. Error: [snapshot_in_progress_exception]: Cannot delete indices that are being snapshotted]` ); expect(loggingSystemMock.collect(mockLogger)).toMatchInlineSnapshot(` Object { "debug": Array [], "error": Array [ Array [ - "[.my-so-index] [snapshot_in_progress_exception]: error reason", + "[.my-so-index] [snapshot_in_progress_exception]: Cannot delete indices that are being snapshotted", ], Array [ "[.my-so-index] migration failed, dumping execution log:", @@ -352,7 +357,7 @@ describe('migrationsStateActionMachine', () => { }, }) ).rejects.toMatchInlineSnapshot( - `[Error: Unable to complete saved object migrations for the [.my-so-index] index. Please check the health of your Elasticsearch cluster and try again. Error: this action throws]` + `[Error: Unable to complete saved object migrations for the [.my-so-index] index. Error: this action throws]` ); expect(loggingSystemMock.collect(mockLogger)).toMatchInlineSnapshot(` Object { diff --git a/src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.ts b/src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.ts index b5e1b788cbdd6..dddc66d68ad20 100644 --- a/src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.ts +++ b/src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.ts @@ -10,6 +10,7 @@ import { errors as EsErrors } from '@elastic/elasticsearch'; import * as Option from 'fp-ts/lib/Option'; import { performance } from 'perf_hooks'; import { Logger, LogMeta } from '../../logging'; +import { CorruptSavedObjectError } from '../migrations/core/migrate_raw_docs'; import { Model, Next, stateActionMachine } from './state_action_machine'; import { State } from './types'; @@ -153,12 +154,27 @@ export async function migrationStateActionMachine({ logger.error( logMessagePrefix + `[${e.body?.error?.type}]: ${e.body?.error?.reason ?? e.message}` ); + dumpExecutionLog(logger, logMessagePrefix, executionLog); + throw new Error( + `Unable to complete saved object migrations for the [${ + initialState.indexPrefix + }] index. Please check the health of your Elasticsearch cluster and try again. Error: [${ + e.body?.error?.type + }]: ${e.body?.error?.reason ?? e.message}` + ); } else { logger.error(e); + + dumpExecutionLog(logger, logMessagePrefix, executionLog); + if (e instanceof CorruptSavedObjectError) { + throw new Error( + `${e.message} To allow migrations to proceed, please delete this document from the [${initialState.indexPrefix}_${initialState.kibanaVersion}_001] index.` + ); + } + + throw new Error( + `Unable to complete saved object migrations for the [${initialState.indexPrefix}] index. ${e}` + ); } - dumpExecutionLog(logger, logMessagePrefix, executionLog); - throw new Error( - `Unable to complete saved object migrations for the [${initialState.indexPrefix}] index. Please check the health of your Elasticsearch cluster and try again. ${e}` - ); } }