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 71e5565ebcbef..79f5bd09889db 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 @@ -748,14 +748,8 @@ describe('DocumentMigrator', () => { migrator.migrate(_.cloneDeep(failedDoc)); expect('Did not throw').toEqual('But it should have!'); } catch (error) { - expect(error.message).toMatchInlineSnapshot(` - "Failed to transform document smelly. Transform: dog:1.2.3 - Doc: {\\"id\\":\\"smelly\\",\\"type\\":\\"dog\\",\\"attributes\\":{},\\"migrationVersion\\":{}}" - `); + expect(error.message).toBe('Dang diggity!'); expect(error).toBeInstanceOf(TransformSavedObjectDocumentError); - expect(loggingSystemMock.collect(mockLoggerFactory).error[0][0]).toMatchInlineSnapshot( - `[Error: Dang diggity!]` - ); } }); 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 c96de6ebbfcdd..a32cc999c5559 100644 --- a/src/core/server/saved_objects/migrations/core/document_migrator.ts +++ b/src/core/server/saved_objects/migrations/core/document_migrator.ts @@ -679,19 +679,8 @@ function wrapWithTry( return { transformedDoc: result, additionalDocs: [] }; } catch (error) { - const failedTransform = `${type.name}:${version}`; - const failedDoc = JSON.stringify(doc); log.error(error); - // To make debugging failed migrations easier, we add items needed to convert the - // saved object id to the full raw id (the id only contains the uuid part) and the full error itself - throw new TransformSavedObjectDocumentError( - doc.id, - doc.type, - doc.namespace, - failedTransform, - failedDoc, - error - ); + throw new TransformSavedObjectDocumentError(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 1d43e2f54a726..7a6f72a881cd6 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 @@ -233,34 +233,7 @@ describe('migrateRawDocsSafely', () => { test('instance of Either.left containing transform errors when the transform function throws a TransformSavedObjectDocument error', async () => { const transform = jest.fn((doc: any) => { - throw new TransformSavedObjectDocumentError( - `${doc.id}`, - `${doc.type}`, - `${doc.namespace}`, - `${doc.type}1.2.3`, - JSON.stringify(doc), - new Error('error during transform') - ); - }); - const task = migrateRawDocsSafely( - new SavedObjectsSerializer(new SavedObjectTypeRegistry()), - transform, - [{ _id: 'a:b', _source: { type: 'a', a: { name: 'AAA' } } }] // this is the raw doc - ); - const result = (await task()) as Either.Left; - expect(transform).toHaveBeenCalledTimes(1); - expect(result._tag).toEqual('Left'); - expect(result.left.corruptDocumentIds.length).toEqual(0); - expect(result.left.transformErrors.length).toEqual(1); - expect(result.left.transformErrors[0].err.message).toMatchInlineSnapshot(` - "Failed to transform document b. Transform: a1.2.3 - Doc: {\\"type\\":\\"a\\",\\"id\\":\\"b\\",\\"attributes\\":{\\"name\\":\\"AAA\\"},\\"references\\":[],\\"migrationVersion\\":{}}" - `); - }); - - test("instance of Either.left containing errors when the transform function throws an error that isn't a TransformSavedObjectDocument error", async () => { - const transform = jest.fn((doc: any) => { - throw new Error('error during transform'); + throw new TransformSavedObjectDocumentError(new Error('error during transform')); }); const task = migrateRawDocsSafely( new SavedObjectsSerializer(new SavedObjectTypeRegistry()), diff --git a/src/core/server/saved_objects/migrations/core/transform_saved_object_document_error.test.ts b/src/core/server/saved_objects/migrations/core/transform_saved_object_document_error.test.ts index 80c670edd39ba..1efb1bd726216 100644 --- a/src/core/server/saved_objects/migrations/core/transform_saved_object_document_error.test.ts +++ b/src/core/server/saved_objects/migrations/core/transform_saved_object_document_error.test.ts @@ -10,51 +10,10 @@ import { TransformSavedObjectDocumentError } from './transform_saved_object_docu describe('TransformSavedObjectDocumentError', () => { it('is a special error', () => { const originalError = new Error('Dang diggity!'); - const err = new TransformSavedObjectDocumentError( - 'id', - 'type', - 'namespace', - 'failedTransform', - 'failedDoc', - originalError - ); + const err = new TransformSavedObjectDocumentError(originalError); expect(err).toBeInstanceOf(TransformSavedObjectDocumentError); - expect(err.id).toEqual('id'); - expect(err.namespace).toEqual('namespace'); expect(err.stack).not.toBeNull(); - }); - it('constructs an special error message', () => { - const originalError = new Error('Dang diggity!'); - const err = new TransformSavedObjectDocumentError( - 'id', - 'type', - 'namespace', - 'failedTransform', - 'failedDoc', - originalError - ); - expect(err.message).toMatchInlineSnapshot( - ` - "Failed to transform document id. Transform: failedTransform - Doc: failedDoc" - ` - ); - }); - it('handles undefined namespace', () => { - const originalError = new Error('Dang diggity!'); - const err = new TransformSavedObjectDocumentError( - 'id', - 'type', - undefined, - 'failedTransform', - 'failedDoc', - originalError - ); - expect(err.message).toMatchInlineSnapshot( - ` - "Failed to transform document id. Transform: failedTransform - Doc: failedDoc" - ` - ); + expect(err.originalError).toBe(originalError); + expect(err.message).toMatchInlineSnapshot(`"Dang diggity!"`); }); }); diff --git a/src/core/server/saved_objects/migrations/core/transform_saved_object_document_error.ts b/src/core/server/saved_objects/migrations/core/transform_saved_object_document_error.ts index 6a6f87ea1eeb2..2dc553545a08d 100644 --- a/src/core/server/saved_objects/migrations/core/transform_saved_object_document_error.ts +++ b/src/core/server/saved_objects/migrations/core/transform_saved_object_document_error.ts @@ -9,24 +9,10 @@ /** * Error thrown when saved object migrations encounter a transformation error. * Transformation errors happen when a transform function throws an error for an unsanitized saved object - * The id (doc.id) reported in this error class is just the uuid part and doesn't tell users what the full elasticsearch id is. - * in order to convert the id to the serialized version further upstream using serializer.generateRawId, we need to provide the following items: - * - namespace: doc.namespace, - * - type: doc.type, - * - id: doc.id, - * The new error class helps with v2 migrations. - * For backward compatibility with v1 migrations, the error message is the same as what was previously thrown as a plain error */ export class TransformSavedObjectDocumentError extends Error { - constructor( - public readonly id: string, - public readonly type: string, - public readonly namespace: string | undefined, - public readonly failedTransform: string, // created by document_migrator wrapWithTry as `${type.name}:${version}`; - public readonly failedDoc: string, - public readonly originalError: Error - ) { - super(`Failed to transform document ${id}. Transform: ${failedTransform}\nDoc: ${failedDoc}`); + constructor(public readonly originalError: Error) { + super(`${originalError.message}`); } } diff --git a/src/core/server/saved_objects/migrationsv2/integration_tests/archives/7_13_corrupt_and_transform_failures_docs.zip b/src/core/server/saved_objects/migrationsv2/integration_tests/archives/7_13_corrupt_and_transform_failures_docs.zip new file mode 100644 index 0000000000000..30ee6ee23dbf3 Binary files /dev/null and b/src/core/server/saved_objects/migrationsv2/integration_tests/archives/7_13_corrupt_and_transform_failures_docs.zip differ diff --git a/src/core/server/saved_objects/migrationsv2/integration_tests/corrupt_outdated_docs.test.ts b/src/core/server/saved_objects/migrationsv2/integration_tests/corrupt_outdated_docs.test.ts index a114263f5b985..526f6275f3f1b 100644 --- a/src/core/server/saved_objects/migrationsv2/integration_tests/corrupt_outdated_docs.test.ts +++ b/src/core/server/saved_objects/migrationsv2/integration_tests/corrupt_outdated_docs.test.ts @@ -56,7 +56,7 @@ describe.skip('migration v2 with corrupt saved object documents', () => { // }, // original corrupt SO example: // { - // id: 'bar:123' + // id: 'bar:123' // '123' etc // type: 'foo', // foo: {}, // migrationVersion: { @@ -108,16 +108,39 @@ describe.skip('migration v2 with corrupt saved object documents', () => { try { await root.start(); } catch (err) { - const corruptFooSOs = /foo:/g; - const corruptBarSOs = /bar:/g; - const corruptBazSOs = /baz:/g; + const errorMessage = err.message; expect( - [ - ...err.message.matchAll(corruptFooSOs), - ...err.message.matchAll(corruptBarSOs), - ...err.message.matchAll(corruptBazSOs), - ].length - ).toEqual(16); + errorMessage.startsWith( + 'Unable to complete saved object migrations for the [.kibana] index: Migrations failed. Reason: Corrupt saved object documents: ' + ) + ).toBeTruthy(); + expect( + errorMessage.endsWith(' To allow migrations to proceed, please delete these documents.') + ).toBeTruthy(); + const expectedCorruptDocIds = [ + '"foo:my_name"', + '"123"', + '"456"', + '"789"', + '"foo:other_name"', + '"bar:123"', + '"baz:123"', + '"bar:345"', + '"bar:890"', + '"baz:456"', + '"baz:789"', + '"bar:other_name"', + '"baz:other_name"', + '"bar:my_name"', + '"baz:my_name"', + '"foo:123"', + '"foo:456"', + '"foo:789"', + '"foo:other"', + ]; + for (const corruptDocId of expectedCorruptDocIds) { + expect(errorMessage.includes(corruptDocId)).toBeTruthy(); + } } }); }); diff --git a/src/core/server/saved_objects/migrationsv2/integration_tests/migration_7_13_0_transform_failures.test.ts b/src/core/server/saved_objects/migrationsv2/integration_tests/migration_7_13_0_transform_failures.test.ts new file mode 100644 index 0000000000000..c014f7de395e0 --- /dev/null +++ b/src/core/server/saved_objects/migrationsv2/integration_tests/migration_7_13_0_transform_failures.test.ts @@ -0,0 +1,157 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import Path from 'path'; +import Fs from 'fs'; +import Util from 'util'; +import * as kbnTestServer from '../../../../test_helpers/kbn_server'; +import { Root } from '../../../root'; + +const logFilePath = Path.join(__dirname, '7_13_corrupt_transform_failures_test.log'); + +const asyncUnlink = Util.promisify(Fs.unlink); +async function removeLogFile() { + // ignore errors if it doesn't exist + await asyncUnlink(logFilePath).catch(() => void 0); +} + +describe('migration v2', () => { + let esServer: kbnTestServer.TestElasticsearchUtils; + let root: Root; + + beforeAll(async () => { + await removeLogFile(); + }); + + afterAll(async () => { + if (root) { + await root.shutdown(); + } + if (esServer) { + await esServer.stop(); + } + + await new Promise((resolve) => setTimeout(resolve, 10000)); + }); + + it('migrates the documents to the highest version', async () => { + const { startES } = kbnTestServer.createTestServers({ + adjustTimeout: (t: number) => jest.setTimeout(t), + settings: { + es: { + license: 'basic', + // example of original 'foo' SO with corrupt id: + // _id: one + // { + // foo: { + // name: 'one', + // }, + // type: 'foo', + // references: [], + // migrationVersion: { + // foo: '7.13.0', + // }, + // "coreMigrationVersion": "7.13.0", + // "updated_at": "2021-05-16T18:16:45.450Z" + // }, + + // SO that will fail transformation: + // { + // type: 'space', + // space: {}, + // }, + // + // + dataArchive: Path.join( + __dirname, + 'archives', + '7_13_corrupt_and_transform_failures_docs.zip' + ), + }, + }, + }); + + root = createRoot(); + + esServer = await startES(); + const coreSetup = await root.setup(); + + coreSetup.savedObjects.registerType({ + name: 'foo', + hidden: false, + mappings: { + properties: {}, + }, + namespaceType: 'agnostic', + migrations: { + '7.14.0': (doc) => doc, + }, + }); + try { + await root.start(); + } catch (err) { + const errorMessage = err.message; + expect( + errorMessage.startsWith( + 'Unable to complete saved object migrations for the [.kibana] index: Migrations failed. Reason: Corrupt saved object documents: ' + ) + ).toBeTruthy(); + expect( + errorMessage.endsWith(' To allow migrations to proceed, please delete these documents.') + ).toBeTruthy(); + + const expectedCorruptDocIds = [ + 'P2SQfHkBs3dBRGh--No5', + 'QGSZfHkBs3dBRGh-ANoD', + 'QWSZfHkBs3dBRGh-hNob', + 'QmSZfHkBs3dBRGh-w9qH', + 'one', + 'two', + 'Q2SZfHkBs3dBRGh-9dp2', + ]; + for (const corruptDocId of expectedCorruptDocIds) { + expect(errorMessage.includes(corruptDocId)).toBeTruthy(); + } + const expectedTransformErrorMessage = + 'Transformation errors: space:default: Document "default" has property "space" which belongs to a more recent version of Kibana [6.6.0]. The last known version is [undefined]'; + expect(errorMessage.includes(expectedTransformErrorMessage)).toBeTruthy(); + } + }); +}); + +function createRoot() { + return kbnTestServer.createRootWithCorePlugins( + { + migrations: { + skip: false, + enableV2: true, + batchSize: 5, + }, + logging: { + appenders: { + file: { + type: 'file', + fileName: logFilePath, + layout: { + type: 'json', + }, + }, + }, + loggers: [ + { + name: 'root', + appenders: ['file'], + }, + ], + }, + }, + { + oss: true, + } + ); +} diff --git a/src/core/server/saved_objects/migrationsv2/model.test.ts b/src/core/server/saved_objects/migrationsv2/model.test.ts index 7a47e58f1947c..186cb24b4a34a 100644 --- a/src/core/server/saved_objects/migrationsv2/model.test.ts +++ b/src/core/server/saved_objects/migrationsv2/model.test.ts @@ -1158,14 +1158,7 @@ describe('migrations v2 model', () => { it('OUTDATED_DOCUMENTS_SEARCH_READ -> FATAL if no outdated documents to transform and we have failed document migrations', () => { const corruptDocumentIdsCarriedOver = ['a:somethingelse']; const originalTransformError = new Error('something went wrong'); - const transFormErr = new TransformSavedObjectDocumentError( - '123', - 'vis', - undefined, - 'randomvis: 7.12.0', - 'failedDoc', - originalTransformError - ); + const transFormErr = new TransformSavedObjectDocumentError(originalTransformError); const transformationErrors = [ { rawId: 'bob:tail', err: transFormErr }, ] as TransformErrorObjects[]; @@ -1184,7 +1177,7 @@ describe('migrations v2 model', () => { expect(newState.reason.includes('Migrations failed. Reason:')).toBe(true); expect(newState.reason.includes('Corrupt saved object documents: ')).toBe(true); expect(newState.reason.includes('Transformation errors: ')).toBe(true); - expect(newState.reason.includes('randomvis: 7.12.0')).toBe(true); + expect(newState.reason.includes('bob:tail')).toBe(true); expect(newState.logs).toStrictEqual([]); // No logs because no hits }); }); @@ -1229,14 +1222,7 @@ describe('migrations v2 model', () => { const outdatedDocuments = [{ _id: '1', _source: { type: 'vis' } }]; const corruptDocumentIds = ['a:somethingelse']; const originalTransformError = new Error('Dang diggity!'); - const transFormErr = new TransformSavedObjectDocumentError( - 'id', - 'type', - 'namespace', - 'failedTransform', - 'failedDoc', - originalTransformError - ); + const transFormErr = new TransformSavedObjectDocumentError(originalTransformError); const transformationErrors = [ { rawId: 'bob:tail', err: transFormErr }, ] as TransformErrorObjects[]; diff --git a/src/core/server/saved_objects/migrationsv2/model.ts b/src/core/server/saved_objects/migrationsv2/model.ts index f4185225ae073..252d7424c339c 100644 --- a/src/core/server/saved_objects/migrationsv2/model.ts +++ b/src/core/server/saved_objects/migrationsv2/model.ts @@ -109,7 +109,7 @@ function getAliases(indices: FetchIndexResponse) { function extractTransformFailuresReason( corruptDocumentIds: string[], transformErrors: TransformErrorObjects[] -): { corruptDocsReason: string; transformErrsReason: string } { +): string { const corruptDocumentIdReason = corruptDocumentIds.length > 0 ? ` Corrupt saved object documents: ${corruptDocumentIds.join(',')}` @@ -122,10 +122,7 @@ function extractTransformFailuresReason( .map((errObj) => `${errObj.rawId}: ${errObj.err.message}\n ${errObj.err.stack ?? ''}`) .join('/n') : ''; - return { - corruptDocsReason: corruptDocumentIdReason, - transformErrsReason: transformErrorsReason, - }; + return `Migrations failed. Reason:${corruptDocumentIdReason}${transformErrorsReason}. To allow migrations to proceed, please delete these documents.`; } const delayRetryState = ( @@ -538,14 +535,14 @@ export const model = (currentState: State, resW: ResponseType): } else { // we don't have any more outdated documents and need to either fail or move on to updating the target mappings. if (stateP.corruptDocumentIds.length > 0 || stateP.transformErrors.length > 0) { - const { corruptDocsReason, transformErrsReason } = extractTransformFailuresReason( + const transformFailureReason = extractTransformFailuresReason( stateP.corruptDocumentIds, stateP.transformErrors ); return { ...stateP, controlState: 'FATAL', - reason: `Migrations failed. Reason:${corruptDocsReason}${transformErrsReason}. To allow migrations to proceed, please delete these documents.`, + reason: transformFailureReason, }; } else { // we don't have any more outdated documents and we haven't encountered any document transformation issues. @@ -722,14 +719,14 @@ export const model = (currentState: State, resW: ResponseType): } else { // we don't have any more outdated documents and need to either fail or move on to updating the target mappings. if (stateP.corruptDocumentIds.length > 0 || stateP.transformErrors.length > 0) { - const { corruptDocsReason, transformErrsReason } = extractTransformFailuresReason( + const transformFailureReason = extractTransformFailuresReason( stateP.corruptDocumentIds, stateP.transformErrors ); return { ...stateP, controlState: 'FATAL', - reason: `Migrations failed. Reason:${corruptDocsReason}${transformErrsReason}. To allow migrations to proceed, please delete these documents.`, + reason: transformFailureReason, }; } else { // If there are no more results we have transformed all outdated