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>

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', () => {

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;
     }
   };
 }

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<any, any>((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: [] },
-      },
-    ]);
+    expect(result).rejects.toMatchInlineSnapshot(
+      `[Error: Unable to migrate the corrupt saved object document with _id: 'foo:b'.]`
+    );
 
-    const obj2 = {
-      id: 'd',
-      type: 'c',
-      attributes: { name: 'DDD' },
-      migrationVersion: {},
-      references: [],
-    };
-    expect(transform).toHaveBeenCalledTimes(1);
-    expect(transform).toHaveBeenCalledWith(obj2);
-
-    expect(logger.error).toBeCalledTimes(1);
+    expect(transform).toHaveBeenCalledTimes(0);
   });
 
   test('handles when one document is transformed into multiple documents', async () => {

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;

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(`

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 {

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}`
-    );
   }
 }