Skip to content

Commit 2cf1f0d

Browse files
pgayvalletkibanamachine
authored andcommitted
[SO migration] fail the migration if unknown types are encountered (#118300)
* manually revert #105213 * enabled xpack to register types for some IT tests * fiz doc size Co-authored-by: Kibana Machine <[email protected]>
1 parent ab66db1 commit 2cf1f0d

File tree

13 files changed

+96
-301
lines changed

13 files changed

+96
-301
lines changed

src/core/server/saved_objects/migrations/actions/check_for_unknown_docs.test.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,10 @@ describe('checkForUnknownDocs', () => {
9898
const result = await task();
9999

100100
expect(Either.isRight(result)).toBe(true);
101-
expect((result as Either.Right<any>).right).toEqual({
102-
unknownDocs: [],
103-
});
101+
expect((result as Either.Right<any>).right).toEqual({});
104102
});
105103

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

125123
const result = await task();
126124

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

152151
const result = await task();
153152

154-
expect(Either.isRight(result)).toBe(true);
155-
expect((result as Either.Right<any>).right).toEqual({
153+
expect(Either.isLeft(result)).toBe(true);
154+
expect((result as Either.Left<any>).left).toEqual({
155+
type: 'unknown_docs_found',
156156
unknownDocs: [{ id: '12', type: 'unknown' }],
157157
});
158158
});

src/core/server/saved_objects/migrations/actions/check_for_unknown_docs.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export interface CheckForUnknownDocsFoundDoc {
3232

3333
/** @internal */
3434
export interface UnknownDocsFound {
35+
type: 'unknown_docs_found';
3536
unknownDocs: CheckForUnknownDocsFoundDoc[];
3637
}
3738

@@ -41,7 +42,10 @@ export const checkForUnknownDocs =
4142
indexName,
4243
unusedTypesQuery,
4344
knownTypes,
44-
}: CheckForUnknownDocsParams): TaskEither.TaskEither<RetryableEsClientError, UnknownDocsFound> =>
45+
}: CheckForUnknownDocsParams): TaskEither.TaskEither<
46+
RetryableEsClientError | UnknownDocsFound,
47+
{}
48+
> =>
4549
() => {
4650
const query = createUnknownDocQuery(unusedTypesQuery, knownTypes);
4751

@@ -54,9 +58,14 @@ export const checkForUnknownDocs =
5458
})
5559
.then((response) => {
5660
const { hits } = response.body.hits;
57-
return Either.right({
58-
unknownDocs: hits.map((hit) => ({ id: hit._id, type: hit._source?.type ?? 'unknown' })),
59-
});
61+
if (hits.length) {
62+
return Either.left({
63+
type: 'unknown_docs_found' as const,
64+
unknownDocs: hits.map((hit) => ({ id: hit._id, type: hit._source?.type ?? 'unknown' })),
65+
});
66+
} else {
67+
return Either.right({});
68+
}
6069
})
6170
.catch(catchRetryableEsClientErrors);
6271
};

src/core/server/saved_objects/migrations/actions/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ export type {
8080
} from './update_and_pickup_mappings';
8181
export { updateAndPickupMappings } from './update_and_pickup_mappings';
8282

83+
import type { UnknownDocsFound } from './check_for_unknown_docs';
8384
export type {
8485
CheckForUnknownDocsParams,
8586
UnknownDocsFound,
@@ -141,6 +142,7 @@ export interface ActionErrorTypeMap {
141142
remove_index_not_a_concrete_index: RemoveIndexNotAConcreteIndex;
142143
documents_transform_failed: DocumentsTransformFailed;
143144
request_entity_too_large_exception: RequestEntityTooLargeException;
145+
unknown_docs_found: UnknownDocsFound;
144146
}
145147

146148
/**

src/core/server/saved_objects/migrations/core/migrate_raw_docs.test.ts

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ describe('migrateRawDocsSafely', () => {
139139
]);
140140
const task = migrateRawDocsSafely({
141141
serializer: new SavedObjectsSerializer(new SavedObjectTypeRegistry()),
142-
knownTypes: new Set(['a', 'c']),
143142
migrateDoc: transform,
144143
rawDocs: [
145144
{ _id: 'a:b', _source: { type: 'a', a: { name: 'AAA' } } },
@@ -184,7 +183,6 @@ describe('migrateRawDocsSafely', () => {
184183
]);
185184
const task = migrateRawDocsSafely({
186185
serializer: new SavedObjectsSerializer(new SavedObjectTypeRegistry()),
187-
knownTypes: new Set(['a', 'c']),
188186
migrateDoc: transform,
189187
rawDocs: [
190188
{ _id: 'foo:b', _source: { type: 'a', a: { name: 'AAA' } } },
@@ -206,7 +204,6 @@ describe('migrateRawDocsSafely', () => {
206204
]);
207205
const task = migrateRawDocsSafely({
208206
serializer: new SavedObjectsSerializer(new SavedObjectTypeRegistry()),
209-
knownTypes: new Set(['a', 'c']),
210207
migrateDoc: transform,
211208
rawDocs: [{ _id: 'a:b', _source: { type: 'a', a: { name: 'AAA' } } }],
212209
});
@@ -240,7 +237,6 @@ describe('migrateRawDocsSafely', () => {
240237
});
241238
const task = migrateRawDocsSafely({
242239
serializer: new SavedObjectsSerializer(new SavedObjectTypeRegistry()),
243-
knownTypes: new Set(['a', 'c']),
244240
migrateDoc: transform,
245241
rawDocs: [{ _id: 'a:b', _source: { type: 'a', a: { name: 'AAA' } } }], // this is the raw doc
246242
});
@@ -256,43 +252,4 @@ describe('migrateRawDocsSafely', () => {
256252
}
257253
`);
258254
});
259-
260-
test('skips documents of unknown types', async () => {
261-
const transform = jest.fn<any, any>((doc: any) => [
262-
set(_.cloneDeep(doc), 'attributes.name', 'HOI!'),
263-
]);
264-
const task = migrateRawDocsSafely({
265-
serializer: new SavedObjectsSerializer(new SavedObjectTypeRegistry()),
266-
knownTypes: new Set(['a']),
267-
migrateDoc: transform,
268-
rawDocs: [
269-
{ _id: 'a:b', _source: { type: 'a', a: { name: 'AAA' } } },
270-
{ _id: 'c:d', _source: { type: 'c', c: { name: 'DDD' } } },
271-
],
272-
});
273-
274-
const result = (await task()) as Either.Right<DocumentsTransformSuccess>;
275-
expect(result._tag).toEqual('Right');
276-
expect(result.right.processedDocs).toEqual([
277-
{
278-
_id: 'a:b',
279-
_source: { type: 'a', a: { name: 'HOI!' }, migrationVersion: {}, references: [] },
280-
},
281-
{
282-
_id: 'c:d',
283-
// name field is not migrated on unknown type
284-
_source: { type: 'c', c: { name: 'DDD' } },
285-
},
286-
]);
287-
288-
const obj1 = {
289-
id: 'b',
290-
type: 'a',
291-
attributes: { name: 'AAA' },
292-
migrationVersion: {},
293-
references: [],
294-
};
295-
expect(transform).toHaveBeenCalledTimes(1);
296-
expect(transform).toHaveBeenNthCalledWith(1, obj1);
297-
});
298255
});

src/core/server/saved_objects/migrations/core/migrate_raw_docs.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,16 @@ export interface DocumentsTransformFailed {
2525
readonly corruptDocumentIds: string[];
2626
readonly transformErrors: TransformErrorObjects[];
2727
}
28+
2829
export interface DocumentsTransformSuccess {
2930
readonly processedDocs: SavedObjectsRawDoc[];
3031
}
32+
3133
export interface TransformErrorObjects {
3234
readonly rawId: string;
3335
readonly err: TransformSavedObjectDocumentError | Error;
3436
}
37+
3538
type MigrateFn = (
3639
doc: SavedObjectUnsanitizedDoc<unknown>
3740
) => Promise<Array<SavedObjectUnsanitizedDoc<unknown>>>;
@@ -83,7 +86,6 @@ export async function migrateRawDocs(
8386

8487
interface MigrateRawDocsSafelyDeps {
8588
serializer: SavedObjectsSerializer;
86-
knownTypes: ReadonlySet<string>;
8789
migrateDoc: MigrateAndConvertFn;
8890
rawDocs: SavedObjectsRawDoc[];
8991
}
@@ -97,7 +99,6 @@ interface MigrateRawDocsSafelyDeps {
9799
*/
98100
export function migrateRawDocsSafely({
99101
serializer,
100-
knownTypes,
101102
migrateDoc,
102103
rawDocs,
103104
}: MigrateRawDocsSafelyDeps): TaskEither.TaskEither<
@@ -111,10 +112,7 @@ export function migrateRawDocsSafely({
111112
const corruptSavedObjectIds: string[] = [];
112113
const options = { namespaceTreatment: 'lax' as const };
113114
for (const raw of rawDocs) {
114-
// Do not transform documents of unknown types
115-
if (raw?._source?.type && !knownTypes.has(raw._source.type)) {
116-
processedDocs.push(raw);
117-
} else if (serializer.isRawSavedObject(raw, options)) {
115+
if (serializer.isRawSavedObject(raw, options)) {
118116
try {
119117
const savedObject = convertToRawAddMigrationVersion(raw, options, serializer);
120118
processedDocs.push(

0 commit comments

Comments
 (0)