Skip to content

Commit

Permalink
[SO migration] fail the migration if unknown types are encountered (e…
Browse files Browse the repository at this point in the history
…lastic#118300)

* manually revert elastic#105213

* enabled xpack to register types for some IT tests

* fiz doc size

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
2 people authored and fkanout committed Nov 17, 2021
1 parent f1f525a commit 970a2ec
Show file tree
Hide file tree
Showing 13 changed files with 96 additions and 301 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,10 @@ describe('checkForUnknownDocs', () => {
const result = await task();

expect(Either.isRight(result)).toBe(true);
expect((result as Either.Right<any>).right).toEqual({
unknownDocs: [],
});
expect((result as Either.Right<any>).right).toEqual({});
});

it('resolves with `Either.right` when unknown docs are found', async () => {
it('resolves with `Either.left` when unknown docs are found', async () => {
const client = elasticsearchClientMock.createInternalClient(
elasticsearchClientMock.createSuccessTransportRequestPromise({
hits: {
Expand All @@ -124,8 +122,9 @@ describe('checkForUnknownDocs', () => {

const result = await task();

expect(Either.isRight(result)).toBe(true);
expect((result as Either.Right<any>).right).toEqual({
expect(Either.isLeft(result)).toBe(true);
expect((result as Either.Left<any>).left).toEqual({
type: 'unknown_docs_found',
unknownDocs: [
{ id: '12', type: 'foo' },
{ id: '14', type: 'bar' },
Expand All @@ -151,8 +150,9 @@ describe('checkForUnknownDocs', () => {

const result = await task();

expect(Either.isRight(result)).toBe(true);
expect((result as Either.Right<any>).right).toEqual({
expect(Either.isLeft(result)).toBe(true);
expect((result as Either.Left<any>).left).toEqual({
type: 'unknown_docs_found',
unknownDocs: [{ id: '12', type: 'unknown' }],
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export interface CheckForUnknownDocsFoundDoc {

/** @internal */
export interface UnknownDocsFound {
type: 'unknown_docs_found';
unknownDocs: CheckForUnknownDocsFoundDoc[];
}

Expand All @@ -41,7 +42,10 @@ export const checkForUnknownDocs =
indexName,
unusedTypesQuery,
knownTypes,
}: CheckForUnknownDocsParams): TaskEither.TaskEither<RetryableEsClientError, UnknownDocsFound> =>
}: CheckForUnknownDocsParams): TaskEither.TaskEither<
RetryableEsClientError | UnknownDocsFound,
{}
> =>
() => {
const query = createUnknownDocQuery(unusedTypesQuery, knownTypes);

Expand All @@ -54,9 +58,14 @@ export const checkForUnknownDocs =
})
.then((response) => {
const { hits } = response.body.hits;
return Either.right({
unknownDocs: hits.map((hit) => ({ id: hit._id, type: hit._source?.type ?? 'unknown' })),
});
if (hits.length) {
return Either.left({
type: 'unknown_docs_found' as const,
unknownDocs: hits.map((hit) => ({ id: hit._id, type: hit._source?.type ?? 'unknown' })),
});
} else {
return Either.right({});
}
})
.catch(catchRetryableEsClientErrors);
};
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/saved_objects/migrations/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export type {
} from './update_and_pickup_mappings';
export { updateAndPickupMappings } from './update_and_pickup_mappings';

import type { UnknownDocsFound } from './check_for_unknown_docs';
export type {
CheckForUnknownDocsParams,
UnknownDocsFound,
Expand Down Expand Up @@ -141,6 +142,7 @@ export interface ActionErrorTypeMap {
remove_index_not_a_concrete_index: RemoveIndexNotAConcreteIndex;
documents_transform_failed: DocumentsTransformFailed;
request_entity_too_large_exception: RequestEntityTooLargeException;
unknown_docs_found: UnknownDocsFound;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ describe('migrateRawDocsSafely', () => {
]);
const task = migrateRawDocsSafely({
serializer: new SavedObjectsSerializer(new SavedObjectTypeRegistry()),
knownTypes: new Set(['a', 'c']),
migrateDoc: transform,
rawDocs: [
{ _id: 'a:b', _source: { type: 'a', a: { name: 'AAA' } } },
Expand Down Expand Up @@ -184,7 +183,6 @@ describe('migrateRawDocsSafely', () => {
]);
const task = migrateRawDocsSafely({
serializer: new SavedObjectsSerializer(new SavedObjectTypeRegistry()),
knownTypes: new Set(['a', 'c']),
migrateDoc: transform,
rawDocs: [
{ _id: 'foo:b', _source: { type: 'a', a: { name: 'AAA' } } },
Expand All @@ -206,7 +204,6 @@ describe('migrateRawDocsSafely', () => {
]);
const task = migrateRawDocsSafely({
serializer: new SavedObjectsSerializer(new SavedObjectTypeRegistry()),
knownTypes: new Set(['a', 'c']),
migrateDoc: transform,
rawDocs: [{ _id: 'a:b', _source: { type: 'a', a: { name: 'AAA' } } }],
});
Expand Down Expand Up @@ -240,7 +237,6 @@ describe('migrateRawDocsSafely', () => {
});
const task = migrateRawDocsSafely({
serializer: new SavedObjectsSerializer(new SavedObjectTypeRegistry()),
knownTypes: new Set(['a', 'c']),
migrateDoc: transform,
rawDocs: [{ _id: 'a:b', _source: { type: 'a', a: { name: 'AAA' } } }], // this is the raw doc
});
Expand All @@ -256,43 +252,4 @@ describe('migrateRawDocsSafely', () => {
}
`);
});

test('skips documents of unknown types', async () => {
const transform = jest.fn<any, any>((doc: any) => [
set(_.cloneDeep(doc), 'attributes.name', 'HOI!'),
]);
const task = migrateRawDocsSafely({
serializer: new SavedObjectsSerializer(new SavedObjectTypeRegistry()),
knownTypes: new Set(['a']),
migrateDoc: transform,
rawDocs: [
{ _id: 'a:b', _source: { type: 'a', a: { name: 'AAA' } } },
{ _id: 'c:d', _source: { type: 'c', c: { name: 'DDD' } } },
],
});

const result = (await task()) as Either.Right<DocumentsTransformSuccess>;
expect(result._tag).toEqual('Right');
expect(result.right.processedDocs).toEqual([
{
_id: 'a:b',
_source: { type: 'a', a: { name: 'HOI!' }, migrationVersion: {}, references: [] },
},
{
_id: 'c:d',
// name field is not migrated on unknown type
_source: { type: 'c', c: { name: 'DDD' } },
},
]);

const obj1 = {
id: 'b',
type: 'a',
attributes: { name: 'AAA' },
migrationVersion: {},
references: [],
};
expect(transform).toHaveBeenCalledTimes(1);
expect(transform).toHaveBeenNthCalledWith(1, obj1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,16 @@ export interface DocumentsTransformFailed {
readonly corruptDocumentIds: string[];
readonly transformErrors: TransformErrorObjects[];
}

export interface DocumentsTransformSuccess {
readonly processedDocs: SavedObjectsRawDoc[];
}

export interface TransformErrorObjects {
readonly rawId: string;
readonly err: TransformSavedObjectDocumentError | Error;
}

type MigrateFn = (
doc: SavedObjectUnsanitizedDoc<unknown>
) => Promise<Array<SavedObjectUnsanitizedDoc<unknown>>>;
Expand Down Expand Up @@ -83,7 +86,6 @@ export async function migrateRawDocs(

interface MigrateRawDocsSafelyDeps {
serializer: SavedObjectsSerializer;
knownTypes: ReadonlySet<string>;
migrateDoc: MigrateAndConvertFn;
rawDocs: SavedObjectsRawDoc[];
}
Expand All @@ -97,7 +99,6 @@ interface MigrateRawDocsSafelyDeps {
*/
export function migrateRawDocsSafely({
serializer,
knownTypes,
migrateDoc,
rawDocs,
}: MigrateRawDocsSafelyDeps): TaskEither.TaskEither<
Expand All @@ -111,10 +112,7 @@ export function migrateRawDocsSafely({
const corruptSavedObjectIds: string[] = [];
const options = { namespaceTreatment: 'lax' as const };
for (const raw of rawDocs) {
// Do not transform documents of unknown types
if (raw?._source?.type && !knownTypes.has(raw._source.type)) {
processedDocs.push(raw);
} else if (serializer.isRawSavedObject(raw, options)) {
if (serializer.isRawSavedObject(raw, options)) {
try {
const savedObject = convertToRawAddMigrationVersion(raw, options, serializer);
processedDocs.push(
Expand Down
Loading

0 comments on commit 970a2ec

Please sign in to comment.