From f5c73a9ac7a3c836bd5907da77f1e32148a99007 Mon Sep 17 00:00:00 2001 From: Ahmad Bamieh Date: Tue, 19 Jan 2021 18:37:48 +0200 Subject: [PATCH] [SavedObjects] Allow migrations to be a function (#88594) Co-authored-by: Rudolf Meijering --- ...gin-core-server.savedobjectmigrationmap.md | 2 +- ...r.savedobjectsservicesetup.registertype.md | 2 +- ...ana-plugin-core-server.savedobjectstype.md | 2 +- ...core-server.savedobjectstype.migrations.md | 4 +- .../migrations/core/document_migrator.test.ts | 128 +++++++++++++----- .../migrations/core/document_migrator.ts | 98 ++++++++++---- .../migrations/core/index_migrator.test.ts | 3 + .../migrations/kibana/kibana_migrator.mock.ts | 3 +- .../migrations/kibana/kibana_migrator.test.ts | 55 +++++++- .../migrations/kibana/kibana_migrator.ts | 4 + .../server/saved_objects/migrations/types.ts | 2 +- .../saved_objects/saved_objects_service.ts | 6 + .../service/lib/repository.test.js | 1 + src/core/server/saved_objects/types.ts | 4 +- src/core/server/server.api.md | 2 +- .../apis/saved_objects/migrations.ts | 2 + 16 files changed, 244 insertions(+), 74 deletions(-) diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectmigrationmap.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectmigrationmap.md index 016442ce67d43..2ab9fcaf428b9 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectmigrationmap.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectmigrationmap.md @@ -18,7 +18,7 @@ export interface SavedObjectMigrationMap ```typescript -const migrations: SavedObjectMigrationMap = { +const migrationsMap: SavedObjectMigrationMap = { '1.0.0': migrateToV1, '2.1.0': migrateToV21 } diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsservicesetup.registertype.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsservicesetup.registertype.md index 54e01d3110a2d..aa813bb7b2956 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsservicesetup.registertype.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsservicesetup.registertype.md @@ -38,7 +38,7 @@ export const myType: SavedObjectsType = { }, migrations: { '2.0.0': migrations.migrateToV2, - '2.1.0': migrations.migrateToV2_1 + '2.1.0': migrations.migrateToV2_1, }, }; diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.md index a8894286de910..e5c3fa2b3e92d 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.md @@ -23,7 +23,7 @@ This is only internal for now, and will only be public when we expose the regist | [indexPattern](./kibana-plugin-core-server.savedobjectstype.indexpattern.md) | string | If defined, the type instances will be stored in the given index instead of the default one. | | [management](./kibana-plugin-core-server.savedobjectstype.management.md) | SavedObjectsTypeManagementDefinition | An optional [saved objects management section](./kibana-plugin-core-server.savedobjectstypemanagementdefinition.md) definition for the type. | | [mappings](./kibana-plugin-core-server.savedobjectstype.mappings.md) | SavedObjectsTypeMappingDefinition | The [mapping definition](./kibana-plugin-core-server.savedobjectstypemappingdefinition.md) for the type. | -| [migrations](./kibana-plugin-core-server.savedobjectstype.migrations.md) | SavedObjectMigrationMap | An optional map of [migrations](./kibana-plugin-core-server.savedobjectmigrationfn.md) to be used to migrate the type. | +| [migrations](./kibana-plugin-core-server.savedobjectstype.migrations.md) | SavedObjectMigrationMap | (() => SavedObjectMigrationMap) | An optional map of [migrations](./kibana-plugin-core-server.savedobjectmigrationfn.md) or a function returning a map of [migrations](./kibana-plugin-core-server.savedobjectmigrationfn.md) to be used to migrate the type. | | [name](./kibana-plugin-core-server.savedobjectstype.name.md) | string | The name of the type, which is also used as the internal id. | | [namespaceType](./kibana-plugin-core-server.savedobjectstype.namespacetype.md) | SavedObjectsNamespaceType | The [namespace type](./kibana-plugin-core-server.savedobjectsnamespacetype.md) for the type. | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.migrations.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.migrations.md index 22513880ab40e..6550d48a1c26a 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.migrations.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectstype.migrations.md @@ -4,10 +4,10 @@ ## SavedObjectsType.migrations property -An optional map of [migrations](./kibana-plugin-core-server.savedobjectmigrationfn.md) to be used to migrate the type. +An optional map of [migrations](./kibana-plugin-core-server.savedobjectmigrationfn.md) or a function returning a map of [migrations](./kibana-plugin-core-server.savedobjectmigrationfn.md) to be used to migrate the type. Signature: ```typescript -migrations?: SavedObjectMigrationMap; +migrations?: SavedObjectMigrationMap | (() => SavedObjectMigrationMap); ``` 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 4cc4f696d307c..3f4522aed8e00 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 @@ -52,50 +52,84 @@ describe('DocumentMigrator', () => { }; } - it('validates individual migration definitions', () => { - const invalidDefinition = { - kibanaVersion: '3.2.3', - typeRegistry: createRegistry({ - name: 'foo', - migrations: _.noop as any, - }), - log: mockLogger, - }; - expect(() => new DocumentMigrator(invalidDefinition)).toThrow( - /Migration for type foo should be an object/i - ); + const createDefinition = (migrations: any) => ({ + kibanaVersion: '3.2.3', + typeRegistry: createRegistry({ + name: 'foo', + migrations: migrations as any, + }), + log: mockLogger, }); - it('validates individual migration semvers', () => { - const invalidDefinition = { - kibanaVersion: '3.2.3', - typeRegistry: createRegistry({ - name: 'foo', - migrations: { - bar: (doc) => doc, - }, - }), - log: mockLogger, - }; - expect(() => new DocumentMigrator(invalidDefinition)).toThrow( - /Expected all properties to be semvers/i + it('validates migration definition', () => { + expect(() => new DocumentMigrator(createDefinition(() => {}))).not.toThrow(); + expect(() => new DocumentMigrator(createDefinition({}))).not.toThrow(); + expect(() => new DocumentMigrator(createDefinition(123))).toThrow( + /Migration for type foo should be an object or a function/i ); }); - it('validates the migration function', () => { - const invalidDefinition = { - kibanaVersion: '3.2.3', + describe('#prepareMigrations', () => { + it('validates individual migration definitions', () => { + const invalidMigrator = new DocumentMigrator(createDefinition(() => 123)); + const voidMigrator = new DocumentMigrator(createDefinition(() => {})); + const emptyObjectMigrator = new DocumentMigrator(createDefinition(() => ({}))); + + expect(invalidMigrator.prepareMigrations).toThrow( + /Migrations map for type foo should be an object/i + ); + expect(voidMigrator.prepareMigrations).not.toThrow(); + expect(emptyObjectMigrator.prepareMigrations).not.toThrow(); + }); + + it('validates individual migration semvers', () => { + const withInvalidVersion = { + bar: (doc: any) => doc, + '1.2.3': (doc: any) => doc, + }; + const migrationFn = new DocumentMigrator(createDefinition(() => withInvalidVersion)); + const migrationObj = new DocumentMigrator(createDefinition(withInvalidVersion)); + + expect(migrationFn.prepareMigrations).toThrow(/Expected all properties to be semvers/i); + expect(migrationObj.prepareMigrations).toThrow(/Expected all properties to be semvers/i); + }); + + it('validates the migration function', () => { + const invalidVersionFunction = { '1.2.3': 23 as any }; + const migrationFn = new DocumentMigrator(createDefinition(() => invalidVersionFunction)); + const migrationObj = new DocumentMigrator(createDefinition(invalidVersionFunction)); + + expect(migrationFn.prepareMigrations).toThrow(/expected a function, but got 23/i); + expect(migrationObj.prepareMigrations).toThrow(/expected a function, but got 23/i); + }); + it('validates definitions with migrations: Function | Objects', () => { + const validMigrationMap = { '1.2.3': () => {} }; + const migrationFn = new DocumentMigrator(createDefinition(() => validMigrationMap)); + const migrationObj = new DocumentMigrator(createDefinition(validMigrationMap)); + expect(migrationFn.prepareMigrations).not.toThrow(); + expect(migrationObj.prepareMigrations).not.toThrow(); + }); + }); + + it('throws if #prepareMigrations is not called before #migrate is called', () => { + const migrator = new DocumentMigrator({ + ...testOpts(), typeRegistry: createRegistry({ - name: 'foo', + name: 'user', migrations: { - '1.2.3': 23 as any, + '1.2.3': setAttr('attributes.name', 'Chris'), }, }), - log: mockLogger, - }; - expect(() => new DocumentMigrator(invalidDefinition)).toThrow( - /expected a function, but got 23/i - ); + }); + + expect(() => + migrator.migrate({ + id: 'me', + type: 'user', + attributes: { name: 'Christopher' }, + migrationVersion: {}, + }) + ).toThrow(/Migrations are not ready. Make sure prepareMigrations is called first./i); }); it('migrates type and attributes', () => { @@ -108,6 +142,8 @@ describe('DocumentMigrator', () => { }, }), }); + migrator.prepareMigrations(); + const actual = migrator.migrate({ id: 'me', type: 'user', @@ -141,6 +177,7 @@ describe('DocumentMigrator', () => { attributes: {}, migrationVersion: {}, }; + migrator.prepareMigrations(); const migratedDoc = migrator.migrate(originalDoc); expect(_.get(originalDoc, 'attributes.name')).toBeUndefined(); expect(_.get(migratedDoc, 'attributes.name')).toBe('Mike'); @@ -156,6 +193,7 @@ describe('DocumentMigrator', () => { }, }), }); + migrator.prepareMigrations(); const actual = migrator.migrate({ id: 'me', type: 'user', @@ -196,6 +234,7 @@ describe('DocumentMigrator', () => { } ), }); + migrator.prepareMigrations(); const actual = migrator.migrate({ id: 'me', type: 'user', @@ -233,6 +272,7 @@ describe('DocumentMigrator', () => { } ), }); + migrator.prepareMigrations(); const actual = migrator.migrate({ id: 'me', type: 'user', @@ -263,6 +303,7 @@ describe('DocumentMigrator', () => { }, }), }); + migrator.prepareMigrations(); const actual = migrator.migrate({ id: 'smelly', type: 'dog', @@ -282,6 +323,7 @@ describe('DocumentMigrator', () => { ...testOpts(), kibanaVersion: '8.0.1', }); + migrator.prepareMigrations(); expect(() => migrator.migrate({ id: 'smelly', @@ -304,6 +346,7 @@ describe('DocumentMigrator', () => { }, }), }); + migrator.prepareMigrations(); expect(() => migrator.migrate({ id: 'fleabag', @@ -329,6 +372,7 @@ describe('DocumentMigrator', () => { }, }), }); + migrator.prepareMigrations(); const actual = migrator.migrate({ id: 'smelly', type: 'dog', @@ -361,6 +405,7 @@ describe('DocumentMigrator', () => { } ), }); + migrator.prepareMigrations(); const actual = migrator.migrate({ id: 'smelly', type: 'dog', @@ -396,6 +441,7 @@ describe('DocumentMigrator', () => { } ), }); + migrator.prepareMigrations(); const actual = migrator.migrate({ id: 'smelly', type: 'foo', @@ -430,6 +476,7 @@ describe('DocumentMigrator', () => { } ), }); + migrator.prepareMigrations(); const actual = migrator.migrate({ id: 'smelly', type: 'dog', @@ -454,6 +501,7 @@ describe('DocumentMigrator', () => { }, }), }); + migrator.prepareMigrations(); expect(() => migrator.migrate({ @@ -477,6 +525,7 @@ describe('DocumentMigrator', () => { }, }), }); + migrator.prepareMigrations(); expect(() => migrator.migrate({ id: 'smelly', @@ -506,6 +555,7 @@ describe('DocumentMigrator', () => { }, }), }); + migrator.prepareMigrations(); const actual = migrator.migrate({ id: 'smelly', type: 'cat', @@ -530,6 +580,7 @@ describe('DocumentMigrator', () => { }, }), }); + migrator.prepareMigrations(); const actual = migrator.migrate({ id: 'smelly', type: 'cat', @@ -565,6 +616,7 @@ describe('DocumentMigrator', () => { migrationVersion: {}, }; try { + migrator.prepareMigrations(); migrator.migrate(_.cloneDeep(failedDoc)); expect('Did not throw').toEqual('But it should have!'); } catch (error) { @@ -597,13 +649,14 @@ describe('DocumentMigrator', () => { attributes: {}, migrationVersion: {}, }; + migrator.prepareMigrations(); migrator.migrate(doc); expect(loggingSystemMock.collect(mockLoggerFactory).info[0][0]).toEqual(logTestMsg); expect(loggingSystemMock.collect(mockLoggerFactory).warn[1][0]).toEqual(logTestMsg); }); test('extracts the latest migration version info', () => { - const { migrationVersion } = new DocumentMigrator({ + const migrator = new DocumentMigrator({ ...testOpts(), typeRegistry: createRegistry( { @@ -624,7 +677,8 @@ describe('DocumentMigrator', () => { ), }); - expect(migrationVersion).toEqual({ + migrator.prepareMigrations(); + expect(migrator.migrationVersion).toEqual({ aaa: '10.4.0', bbb: '3.2.3', }); 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 f30ec4634fb7a..9e89f5967b511 100644 --- a/src/core/server/saved_objects/migrations/core/document_migrator.ts +++ b/src/core/server/saved_objects/migrations/core/document_migrator.ts @@ -69,7 +69,7 @@ import { SavedObjectUnsanitizedDoc } from '../../serialization'; import { SavedObjectsMigrationVersion } from '../../types'; import { MigrationLogger } from './migration_logger'; import { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry'; -import { SavedObjectMigrationFn } from '../types'; +import { SavedObjectMigrationFn, SavedObjectMigrationMap } from '../types'; export type TransformFn = (doc: SavedObjectUnsanitizedDoc) => SavedObjectUnsanitizedDoc; @@ -94,6 +94,7 @@ interface ActiveMigrations { */ export interface VersionedTransformer { migrationVersion: SavedObjectsMigrationVersion; + prepareMigrations: () => void; migrate: TransformFn; } @@ -101,8 +102,9 @@ export interface VersionedTransformer { * A concrete implementation of the VersionedTransformer interface. */ export class DocumentMigrator implements VersionedTransformer { - private migrations: ActiveMigrations; - private transformDoc: TransformFn; + private documentMigratorOptions: DocumentMigratorOptions; + private migrations?: ActiveMigrations; + private transformDoc?: TransformFn; /** * Creates an instance of DocumentMigrator. @@ -115,12 +117,7 @@ export class DocumentMigrator implements VersionedTransformer { */ constructor({ typeRegistry, kibanaVersion, log }: DocumentMigratorOptions) { validateMigrationDefinition(typeRegistry); - - this.migrations = buildActiveMigrations(typeRegistry, log); - this.transformDoc = buildDocumentTransform({ - kibanaVersion, - migrations: this.migrations, - }); + this.documentMigratorOptions = { typeRegistry, kibanaVersion, log }; } /** @@ -131,9 +128,28 @@ export class DocumentMigrator implements VersionedTransformer { * @memberof DocumentMigrator */ public get migrationVersion(): SavedObjectsMigrationVersion { + if (!this.migrations) { + throw new Error('Migrations are not ready. Make sure prepareMigrations is called first.'); + } return _.mapValues(this.migrations, ({ latestVersion }) => latestVersion); } + /** + * Prepares active migrations and document transformer function. + * + * @returns {void} + * @memberof DocumentMigrator + */ + + public prepareMigrations = () => { + const { typeRegistry, kibanaVersion, log } = this.documentMigratorOptions; + this.migrations = buildActiveMigrations(typeRegistry, log); + this.transformDoc = buildDocumentTransform({ + kibanaVersion, + migrations: this.migrations, + }); + }; + /** * Migrates a document to the latest version. * @@ -142,6 +158,10 @@ export class DocumentMigrator implements VersionedTransformer { * @memberof DocumentMigrator */ public migrate = (doc: SavedObjectUnsanitizedDoc): SavedObjectUnsanitizedDoc => { + if (!this.migrations || !this.transformDoc) { + throw new Error('Migrations are not ready. Make sure prepareMigrations is called first.'); + } + // Clone the document to prevent accidental mutations on the original data // Ex: Importing sample data that is cached at import level, migrations would // execute on mutated data the second time. @@ -150,13 +170,7 @@ export class DocumentMigrator implements VersionedTransformer { }; } -/** - * Basic validation that the migraiton definition matches our expectations. We can't - * rely on TypeScript here, as the caller may be JavaScript / ClojureScript / any compile-to-js - * language. So, this is just to provide a little developer-friendly error messaging. Joi was - * giving weird errors, so we're just doing manual validation. - */ -function validateMigrationDefinition(registry: ISavedObjectTypeRegistry) { +function validateMigrationsMapObject(name: string, migrationsMap?: SavedObjectMigrationMap) { function assertObject(obj: any, prefix: string) { if (!obj || typeof obj !== 'object') { throw new Error(`${prefix} Got ${obj}.`); @@ -177,16 +191,38 @@ function validateMigrationDefinition(registry: ISavedObjectTypeRegistry) { } } + if (migrationsMap) { + assertObject( + migrationsMap, + `Migrations map for type ${name} should be an object like { '2.0.0': (doc) => doc }.` + ); + + Object.entries(migrationsMap).forEach(([version, fn]) => { + assertValidSemver(version, name); + assertValidTransform(fn, version, name); + }); + } +} + +/** + * Basic validation that the migraiton definition matches our expectations. We can't + * rely on TypeScript here, as the caller may be JavaScript / ClojureScript / any compile-to-js + * language. So, this is just to provide a little developer-friendly error messaging. Joi was + * giving weird errors, so we're just doing manual validation. + */ +function validateMigrationDefinition(registry: ISavedObjectTypeRegistry) { + function assertObjectOrFunction(entity: any, prefix: string) { + if (!entity || (typeof entity !== 'function' && typeof entity !== 'object')) { + throw new Error(`${prefix} Got! ${typeof entity}, ${JSON.stringify(entity)}.`); + } + } + registry.getAllTypes().forEach((type) => { if (type.migrations) { - assertObject( + assertObjectOrFunction( type.migrations, - `Migration for type ${type.name} should be an object like { '2.0.0': (doc) => doc }.` + `Migration for type ${type.name} should be an object or a function returning an object like { '2.0.0': (doc) => doc }.` ); - Object.entries(type.migrations).forEach(([version, fn]) => { - assertValidSemver(version, type.name); - assertValidTransform(fn, version, type.name); - }); } }); } @@ -201,11 +237,22 @@ function buildActiveMigrations( typeRegistry: ISavedObjectTypeRegistry, log: Logger ): ActiveMigrations { - return typeRegistry + const typesWithMigrationMaps = typeRegistry .getAllTypes() - .filter((type) => type.migrations && Object.keys(type.migrations).length > 0) + .map((type) => ({ + ...type, + migrationsMap: typeof type.migrations === 'function' ? type.migrations() : type.migrations, + })) + .filter((type) => typeof type.migrationsMap !== 'undefined'); + + typesWithMigrationMaps.forEach((type) => + validateMigrationsMapObject(type.name, type.migrationsMap) + ); + + return typesWithMigrationMaps + .filter((type) => type.migrationsMap && Object.keys(type.migrationsMap).length > 0) .reduce((migrations, type) => { - const transforms = Object.entries(type.migrations!) + const transforms = Object.entries(type.migrationsMap!) .map(([version, transform]) => ({ version, transform: wrapWithTry(version, type.name, transform, log), @@ -220,7 +267,6 @@ function buildActiveMigrations( }; }, {} as ActiveMigrations); } - /** * Creates a function which migrates and validates any document that is passed to it. */ diff --git a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts index 13f771c16bc67..5d2597483f5fd 100644 --- a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts @@ -42,6 +42,7 @@ describe('IndexMigrator', () => { documentMigrator: { migrationVersion: {}, migrate: _.identity, + prepareMigrations: jest.fn(), }, serializer: new SavedObjectsSerializer(new SavedObjectTypeRegistry()), }; @@ -326,6 +327,7 @@ describe('IndexMigrator', () => { testOpts.documentMigrator = { migrationVersion: { foo: '1.2.3' }, + prepareMigrations: jest.fn(), migrate: migrateDoc, }; @@ -378,6 +380,7 @@ describe('IndexMigrator', () => { testOpts.documentMigrator = { migrationVersion: { foo: '1.2.3' }, + prepareMigrations: jest.fn(), migrate: migrateDoc, }; diff --git a/src/core/server/saved_objects/migrations/kibana/kibana_migrator.mock.ts b/src/core/server/saved_objects/migrations/kibana/kibana_migrator.mock.ts index da4d39f435038..997945776cfee 100644 --- a/src/core/server/saved_objects/migrations/kibana/kibana_migrator.mock.ts +++ b/src/core/server/saved_objects/migrations/kibana/kibana_migrator.mock.ts @@ -32,7 +32,7 @@ const defaultSavedObjectTypes: SavedObjectsType[] = [ name: { type: 'keyword' }, }, }, - migrations: {}, + migrations: () => ({}), }, ]; @@ -56,6 +56,7 @@ const createMigrator = ( runMigrations: jest.fn(), getActiveMappings: jest.fn(), migrateDocument: jest.fn(), + prepareMigrations: jest.fn(), getStatus$: jest.fn( () => new BehaviorSubject({ diff --git a/src/core/server/saved_objects/migrations/kibana/kibana_migrator.test.ts b/src/core/server/saved_objects/migrations/kibana/kibana_migrator.test.ts index 4248f6fdbeca4..cca7808988896 100644 --- a/src/core/server/saved_objects/migrations/kibana/kibana_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/kibana/kibana_migrator.test.ts @@ -65,7 +65,53 @@ describe('KibanaMigrator', () => { }); }); + describe('migrateDocument', () => { + it('throws an error if documentMigrator.prepareMigrations is not called previously', () => { + const options = mockOptions(); + const kibanaMigrator = new KibanaMigrator(options); + const doc = {} as any; + expect(() => kibanaMigrator.migrateDocument(doc)).toThrowError( + /Migrations are not ready. Make sure prepareMigrations is called first./i + ); + }); + + it('calls documentMigrator.migrate', () => { + const options = mockOptions(); + const kibanaMigrator = new KibanaMigrator(options); + const mockDocumentMigrator = { migrate: jest.fn() }; + // @ts-expect-error `documentMigrator` is readonly. + kibanaMigrator.documentMigrator = mockDocumentMigrator; + const doc = {} as any; + + expect(() => kibanaMigrator.migrateDocument(doc)).not.toThrowError(); + expect(mockDocumentMigrator.migrate).toBeCalledTimes(1); + }); + }); + describe('runMigrations', () => { + it('throws if prepareMigrations is not called first', async () => { + const options = mockOptions(); + + options.client.cat.templates.mockReturnValue( + elasticsearchClientMock.createSuccessTransportRequestPromise( + { templates: [] }, + { statusCode: 404 } + ) + ); + options.client.indices.get.mockReturnValue( + elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) + ); + options.client.indices.getAlias.mockReturnValue( + elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) + ); + + const migrator = new KibanaMigrator(options); + + expect(() => migrator.runMigrations()).rejects.toThrow( + /Migrations are not ready. Make sure prepareMigrations is called first./i + ); + }); + it('only runs migrations once if called multiple times', async () => { const options = mockOptions(); @@ -84,6 +130,7 @@ describe('KibanaMigrator', () => { const migrator = new KibanaMigrator(options); + migrator.prepareMigrations(); await migrator.runMigrations(); await migrator.runMigrations(); @@ -120,6 +167,8 @@ describe('KibanaMigrator', () => { const migrator = new KibanaMigrator(options); const migratorStatus = migrator.getStatus$().pipe(take(3)).toPromise(); + + migrator.prepareMigrations(); await migrator.runMigrations(); expect(options.client.indices.create).toHaveBeenCalledTimes(3); @@ -145,6 +194,7 @@ describe('KibanaMigrator', () => { const migrator = new KibanaMigrator(options); const migratorStatus = migrator.getStatus$().pipe(take(3)).toPromise(); + migrator.prepareMigrations(); await migrator.runMigrations(); const { status, result } = await migratorStatus; expect(status).toEqual('completed'); @@ -171,6 +221,7 @@ describe('KibanaMigrator', () => { const options = mockV2MigrationOptions(); const migrator = new KibanaMigrator(options); const migratorStatus = migrator.getStatus$().pipe(take(3)).toPromise(); + migrator.prepareMigrations(); await migrator.runMigrations(); // Basic assertions that we're creating and reindexing the expected indices @@ -212,6 +263,7 @@ describe('KibanaMigrator', () => { const options = mockV2MigrationOptions(); const migrator = new KibanaMigrator(options); const migratorStatus = migrator.getStatus$().pipe(take(3)).toPromise(); + migrator.prepareMigrations(); await migrator.runMigrations(); const { status, result } = await migratorStatus; @@ -247,6 +299,7 @@ describe('KibanaMigrator', () => { ); const migrator = new KibanaMigrator(options); + migrator.prepareMigrations(); return expect(migrator.runMigrations()).rejects.toMatchInlineSnapshot( `[Error: Unable to complete saved object migrations for the [.my-index] index: The .my-index alias is pointing to a newer version of Kibana: v8.2.4]` ); @@ -263,7 +316,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: {"_tag":"Some","value":{"type":"elatsicsearch_exception","reason":"task failed with an error"}}] diff --git a/src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts b/src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts index 12db79a1067ed..4618802eb24ac 100644 --- a/src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts +++ b/src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts @@ -158,6 +158,10 @@ export class KibanaMigrator { return this.migrationResult; } + public prepareMigrations() { + this.documentMigrator.prepareMigrations(); + } + public getStatus$() { return this.status$.asObservable(); } diff --git a/src/core/server/saved_objects/migrations/types.ts b/src/core/server/saved_objects/migrations/types.ts index 5e55a34193a96..33375cec078d5 100644 --- a/src/core/server/saved_objects/migrations/types.ts +++ b/src/core/server/saved_objects/migrations/types.ts @@ -79,7 +79,7 @@ export interface SavedObjectMigrationContext { * * @example * ```typescript - * const migrations: SavedObjectMigrationMap = { + * const migrationsMap: SavedObjectMigrationMap = { * '1.0.0': migrateToV1, * '2.1.0': migrateToV21 * } diff --git a/src/core/server/saved_objects/saved_objects_service.ts b/src/core/server/saved_objects/saved_objects_service.ts index c34da35a35531..277f94c3cdf2f 100644 --- a/src/core/server/saved_objects/saved_objects_service.ts +++ b/src/core/server/saved_objects/saved_objects_service.ts @@ -388,6 +388,12 @@ export class SavedObjectsService */ const skipMigrations = this.config.migration.skip || !pluginsInitialized; + /** + * Note: Prepares all migrations maps. If a saved object type was registered with property `migrations` + * of type function; this function will be called to get the type's SavedObjectMigrationMap. + */ + migrator.prepareMigrations(); + if (skipMigrations) { this.logger.warn( 'Skipping Saved Object migrations on startup. Note: Individual documents will still be migrated when read or written.' diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 309a817d4d04f..44c290946345c 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -217,6 +217,7 @@ describe('SavedObjectsRepository', () => { beforeEach(() => { client = elasticsearchClientMock.createElasticsearchClient(); migrator = mockKibanaMigrator.create(); + documentMigrator.prepareMigrations(); migrator.migrateDocument = jest.fn().mockImplementation(documentMigrator.migrate); migrator.runMigrations = async () => ({ status: 'skipped' }); diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index c8f8b47949ca5..6834fff0de78f 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -245,9 +245,9 @@ export interface SavedObjectsType { */ mappings: SavedObjectsTypeMappingDefinition; /** - * An optional map of {@link SavedObjectMigrationFn | migrations} to be used to migrate the type. + * An optional map of {@link SavedObjectMigrationFn | migrations} or a function returning a map of {@link SavedObjectMigrationFn | migrations} to be used to migrate the type. */ - migrations?: SavedObjectMigrationMap; + migrations?: SavedObjectMigrationMap | (() => SavedObjectMigrationMap); /** * An optional {@link SavedObjectsTypeManagementDefinition | saved objects management section} definition for the type. */ diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 75f1580ceba8e..905a237090bc5 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -2774,7 +2774,7 @@ export interface SavedObjectsType { indexPattern?: string; management?: SavedObjectsTypeManagementDefinition; mappings: SavedObjectsTypeMappingDefinition; - migrations?: SavedObjectMigrationMap; + migrations?: SavedObjectMigrationMap | (() => SavedObjectMigrationMap); name: string; namespaceType: SavedObjectsNamespaceType; } diff --git a/test/api_integration/apis/saved_objects/migrations.ts b/test/api_integration/apis/saved_objects/migrations.ts index fa9c2fd1a2d7f..e15f0a35cb43c 100644 --- a/test/api_integration/apis/saved_objects/migrations.ts +++ b/test/api_integration/apis/saved_objects/migrations.ts @@ -400,6 +400,8 @@ async function migrateIndex({ log: getLogMock(), }); + documentMigrator.prepareMigrations(); + const migrator = new IndexMigrator({ client: createMigrationEsClient(esClient, getLogMock()), documentMigrator,