Skip to content

Commit

Permalink
v2 migrations should exit process on corrupt saved object document (#…
Browse files Browse the repository at this point in the history
…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>
  • Loading branch information
rudolf and kibanamachine committed Feb 22, 2021
1 parent b564b0a commit fc82586
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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!]`
);
}
});

Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
[
Expand All @@ -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 () => {
Expand Down
25 changes: 19 additions & 6 deletions src/core/server/saved_objects/migrations/core/migrate_raw_docs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 || {};
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:",
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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}`
);
}
}

0 comments on commit fc82586

Please sign in to comment.