Adapt es-archiver to call _migrate endpoint instead of creating a migrator (#56971) (#58575)

* es-archiver call _migrate endpoint instead of creating a migrator

* fix crlf....

* use promise instead of callback accessor

* attempt with disabled authent

* enable security again

* set mapping to dynamic before calling migration endpoint

* rename space data from non-spaced tests

* add documentation on the `rerun` flag

* create router with the `/api/saved_objects` prefix

* add unit test about strict mapping

* add integration test on migrate endpoint

* wrap route handler with handleLegacyErrors

* add remark about limitations of the rerun flag

* Apply suggestions from code review

Co-Authored-By: Aleh Zasypkin <aleh.zasypkin@gmail.com>

* do not return detailed index result

* add access tag to migrate route

* use /internal prefix for migrate endpoint

* fix integ tests

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Josh Dover <me@joshdover.com>
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Josh Dover <me@joshdover.com>
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>

packages/kbn-dev-utils/src/kbn_client/kbn_client_saved_objects.ts

@@ -57,9 +57,28 @@ interface UpdateOptions<Attributes> extends IndexOptions<Attributes> {
   id: string;
 }
 
+interface MigrateResponse {
+  success: boolean;
+  result: Array<{ status: string }>;
+}
+
 export class KbnClientSavedObjects {
   constructor(private readonly log: ToolingLog, private readonly requester: KbnClientRequester) {}
 
+  /**
+   * Run the saved objects migration
+   */
+  public async migrate() {
+    this.log.debug('Migrating saved objects');
+
+    return await this.requester.request<MigrateResponse>({
+      description: 'migrate saved objects',
+      path: uriencode`/internal/saved_objects/_migrate`,
+      method: 'POST',
+      body: {},
+    });
+  }
+
   /**
    * Get an object
    */

packages/kbn-pm/dist/index.js

@@ -43784,6 +43784,18 @@ class KbnClientSavedObjects {
         this.log = log;
         this.requester = requester;
     }
+    /**
+     * Run the saved objects migration
+     */
+    async migrate() {
+        this.log.debug('Migrating saved objects');
+        return await this.requester.request({
+            description: 'migrate saved objects',
+            path: kbn_client_requester_1.uriencode `/internal/saved_objects/_migrate`,
+            method: 'POST',
+            body: {},
+        });
+    }
     /**
      * Get an object
      */

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

@@ -21,6 +21,11 @@ import { IndexMapping } from './../../mappings';
 import { buildActiveMappings, diffMappings } from './build_active_mappings';
 
 describe('buildActiveMappings', () => {
+  test('creates a strict mapping', () => {
+    const mappings = buildActiveMappings({});
+    expect(mappings.dynamic).toEqual('strict');
+  });
+
   test('combines all mappings and includes core mappings', () => {
     const properties = {
       aaa: { type: 'text' },

src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts

@@ -88,15 +88,27 @@ export class KibanaMigrator {
   }
 
   /**
-   * Migrates the mappings and documents in the Kibana index. This will run only
+   * Migrates the mappings and documents in the Kibana index. By default, this will run only
    * once and subsequent calls will return the result of the original call.
    *
+   * @param rerun - If true, method will run a new migration when called again instead of
+   * returning the result of the initial migration. This should only be used when factors external
+   * to Kibana itself alter the kibana index causing the saved objects mappings or data to change
+   * after the Kibana server performed the initial migration.
+   *
+   * @remarks When the `rerun` parameter is set to true, no checks are performed to ensure that no migration
+   * is currently running. Chained or concurrent calls to `runMigrations({ rerun: true })` can lead to
+   * multiple migrations running at the same time. When calling with this parameter, it's expected that the calling
+   * code should ensure that the initial call resolves before calling the function again.
+   *
    * @returns - A promise which resolves once all migrations have been applied.
    *    The promise resolves with an array of migration statuses, one for each
    *    elasticsearch index which was migrated.
    */
-  public runMigrations(): Promise<Array<{ status: string }>> {
-    if (this.migrationResult === undefined) {
+  public runMigrations({ rerun = false }: { rerun?: boolean } = {}): Promise<
+    Array<{ status: string }>
+  > {
+    if (this.migrationResult === undefined || rerun) {
       this.migrationResult = this.runMigrationsInternal();
     }
 

src/core/server/saved_objects/routes/index.ts

@@ -20,6 +20,7 @@
 import { InternalHttpServiceSetup } from '../../http';
 import { Logger } from '../../logging';
 import { SavedObjectConfig } from '../saved_objects_config';
+import { IKibanaMigrator } from '../migrations';
 import { registerGetRoute } from './get';
 import { registerCreateRoute } from './create';
 import { registerDeleteRoute } from './delete';
@@ -32,17 +33,20 @@ import { registerLogLegacyImportRoute } from './log_legacy_import';
 import { registerExportRoute } from './export';
 import { registerImportRoute } from './import';
 import { registerResolveImportErrorsRoute } from './resolve_import_errors';
+import { registerMigrateRoute } from './migrate';
 
 export function registerRoutes({
   http,
   logger,
   config,
   importableExportableTypes,
+  migratorPromise,
 }: {
   http: InternalHttpServiceSetup;
   logger: Logger;
   config: SavedObjectConfig;
   importableExportableTypes: string[];
+  migratorPromise: Promise<IKibanaMigrator>;
 }) {
   const router = http.createRouter('/api/saved_objects/');
 
@@ -58,4 +62,8 @@ export function registerRoutes({
   registerExportRoute(router, config, importableExportableTypes);
   registerImportRoute(router, config, importableExportableTypes);
   registerResolveImportErrorsRoute(router, config, importableExportableTypes);
+
+  const internalRouter = http.createRouter('/internal/saved_objects/');
+
+  registerMigrateRoute(internalRouter, migratorPromise);
 }

src/core/server/saved_objects/routes/integration_tests/import.test.ts

@@ -32,7 +32,7 @@ const config = {
   maxImportExportSize: 10000,
 } as SavedObjectConfig;
 
-describe('POST /api/saved_objects/_import', () => {
+describe('POST /internal/saved_objects/_import', () => {
   let server: setupServerReturn['server'];
   let httpSetup: setupServerReturn['httpSetup'];
   let handlerContext: setupServerReturn['handlerContext'];
@@ -51,7 +51,7 @@ describe('POST /api/saved_objects/_import', () => {
 
     savedObjectsClient.find.mockResolvedValue(emptyResponse);
 
-    const router = httpSetup.createRouter('/api/saved_objects/');
+    const router = httpSetup.createRouter('/internal/saved_objects/');
     registerImportRoute(router, config, allowedTypes);
 
     await server.start();
@@ -63,7 +63,7 @@ describe('POST /api/saved_objects/_import', () => {
 
   it('formats successful response', async () => {
     const result = await supertest(httpSetup.server.listener)
-      .post('/api/saved_objects/_import')
+      .post('/internal/saved_objects/_import')
       .set('content-Type', 'multipart/form-data; boundary=BOUNDARY')
       .send(
         [
@@ -99,7 +99,7 @@ describe('POST /api/saved_objects/_import', () => {
     });
 
     const result = await supertest(httpSetup.server.listener)
-      .post('/api/saved_objects/_import')
+      .post('/internal/saved_objects/_import')
       .set('content-Type', 'multipart/form-data; boundary=EXAMPLE')
       .send(
         [
@@ -148,7 +148,7 @@ describe('POST /api/saved_objects/_import', () => {
     });
 
     const result = await supertest(httpSetup.server.listener)
-      .post('/api/saved_objects/_import')
+      .post('/internal/saved_objects/_import')
       .set('content-Type', 'multipart/form-data; boundary=EXAMPLE')
       .send(
         [
@@ -199,7 +199,7 @@ describe('POST /api/saved_objects/_import', () => {
     });
 
     const result = await supertest(httpSetup.server.listener)
-      .post('/api/saved_objects/_import')
+      .post('/internal/saved_objects/_import')
       .set('content-Type', 'multipart/form-data; boundary=EXAMPLE')
       .send(
         [
@@ -249,7 +249,7 @@ describe('POST /api/saved_objects/_import', () => {
     });
 
     const result = await supertest(httpSetup.server.listener)
-      .post('/api/saved_objects/_import')
+      .post('/internal/saved_objects/_import')
       .set('content-Type', 'multipart/form-data; boundary=EXAMPLE')
       .send(
         [

src/core/server/saved_objects/routes/integration_tests/migrate.test.mocks.ts

@@ -0,0 +1,26 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { mockKibanaMigrator } from '../../migrations/kibana/kibana_migrator.mock';
+
+export const migratorInstanceMock = mockKibanaMigrator.create();
+export const KibanaMigratorMock = jest.fn().mockImplementation(() => migratorInstanceMock);
+jest.doMock('../../migrations/kibana/kibana_migrator', () => ({
+  KibanaMigrator: KibanaMigratorMock,
+}));

src/core/server/saved_objects/routes/integration_tests/migrate.test.ts

@@ -0,0 +1,62 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { migratorInstanceMock } from './migrate.test.mocks';
+import * as kbnTestServer from '../../../../../test_utils/kbn_server';
+
+describe('SavedObjects /_migrate endpoint', () => {
+  let root: ReturnType<typeof kbnTestServer.createRoot>;
+
+  beforeEach(async () => {
+    root = kbnTestServer.createRoot({ migrations: { skip: true } });
+    await root.setup();
+    await root.start();
+    migratorInstanceMock.runMigrations.mockClear();
+  }, 30000);
+
+  afterEach(async () => {
+    await root.shutdown();
+  });
+
+  it('calls runMigrations on the migrator with rerun=true when accessed', async () => {
+    await kbnTestServer.request
+      .post(root, '/internal/saved_objects/_migrate')
+      .send({})
+      .expect(200);
+
+    expect(migratorInstanceMock.runMigrations).toHaveBeenCalledTimes(1);
+    expect(migratorInstanceMock.runMigrations).toHaveBeenCalledWith({ rerun: true });
+  });
+
+  it('calls runMigrations multiple time when multiple access', async () => {
+    await kbnTestServer.request
+      .post(root, '/internal/saved_objects/_migrate')
+      .send({})
+      .expect(200);
+
+    expect(migratorInstanceMock.runMigrations).toHaveBeenCalledTimes(1);
+
+    await kbnTestServer.request
+      .post(root, '/internal/saved_objects/_migrate')
+      .send({})
+      .expect(200);
+
+    expect(migratorInstanceMock.runMigrations).toHaveBeenCalledTimes(2);
+  });
+});

src/core/server/saved_objects/routes/migrate.ts

@@ -0,0 +1,45 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { IRouter } from '../../http';
+import { IKibanaMigrator } from '../migrations';
+
+export const registerMigrateRoute = (
+  router: IRouter,
+  migratorPromise: Promise<IKibanaMigrator>
+) => {
+  router.post(
+    {
+      path: '/_migrate',
+      validate: false,
+      options: {
+        tags: ['access:migrateSavedObjects'],
+      },
+    },
+    router.handleLegacyErrors(async (context, req, res) => {
+      const migrator = await migratorPromise;
+      await migrator.runMigrations({ rerun: true });
+      return res.ok({
+        body: {
+          success: true,
+        },
+      });
+    })
+  );
+};

src/core/server/saved_objects/saved_objects_service.ts

@@ -17,8 +17,9 @@
  * under the License.
  */
 
-import { CoreService } from 'src/core/types';
+import { Subject } from 'rxjs';
 import { first, filter, take } from 'rxjs/operators';
+import { CoreService } from '../../types';
 import {
   SavedObjectsClient,
   SavedObjectsClientProvider,
@@ -36,7 +37,7 @@ import {
   SavedObjectsMigrationConfigType,
   SavedObjectConfig,
 } from './saved_objects_config';
-import { InternalHttpServiceSetup, KibanaRequest } from '../http';
+import { KibanaRequest, InternalHttpServiceSetup } from '../http';
 import { SavedObjectsClientContract, SavedObjectsType, SavedObjectsLegacyUiExports } from './types';
 import { ISavedObjectsRepository, SavedObjectsRepository } from './service/lib/repository';
 import {
@@ -47,8 +48,8 @@ import { Logger } from '../logging';
 import { convertLegacyTypes } from './utils';
 import { SavedObjectTypeRegistry, ISavedObjectTypeRegistry } from './saved_objects_type_registry';
 import { PropertyValidators } from './validation';
-import { registerRoutes } from './routes';
 import { SavedObjectsSerializer } from './serialization';
+import { registerRoutes } from './routes';
 
 /**
  * Saved Objects is Kibana's data persistence mechanism allowing plugins to
@@ -201,9 +202,9 @@ export interface SavedObjectsRepositoryFactory {
 
 /** @internal */
 export interface SavedObjectsSetupDeps {
+  http: InternalHttpServiceSetup;
   legacyPlugins: LegacyServiceDiscoverPlugins;
   elasticsearch: InternalElasticsearchServiceSetup;
-  http: InternalHttpServiceSetup;
 }
 
 interface WrappedClientFactoryWrapper {
@@ -225,6 +226,7 @@ export class SavedObjectsService
   private clientFactoryProvider?: SavedObjectsClientFactoryProvider;
   private clientFactoryWrappers: WrappedClientFactoryWrapper[] = [];
 
+  private migrator$ = new Subject<KibanaMigrator>();
   private typeRegistry = new SavedObjectTypeRegistry();
   private validations: PropertyValidators = {};
 
@@ -262,6 +264,7 @@ export class SavedObjectsService
       http: setupDeps.http,
       logger: this.logger,
       config: this.config,
+      migratorPromise: this.migrator$.pipe(first()).toPromise(),
       importableExportableTypes,
     });
 
@@ -302,6 +305,8 @@ export class SavedObjectsService
     const adminClient = this.setupDeps!.elasticsearch.adminClient;
     const migrator = this.createMigrator(kibanaConfig, this.config.migration, migrationsRetryDelay);
 
+    this.migrator$.next(migrator);
+
     /**
      * Note: We want to ensure that migrations have completed before
      * continuing with further Core start steps that might use SavedObjects

src/es_archiver/actions/empty_kibana_index.ts

@@ -32,9 +32,8 @@ export async function emptyKibanaIndexAction({
   kbnClient: KbnClient;
 }) {
   const stats = createStats('emptyKibanaIndex', log);
-  const kibanaPluginIds = await kbnClient.plugins.getEnabledIds();
 
   await deleteKibanaIndices({ client, stats, log });
-  await migrateKibanaIndex({ client, log, kibanaPluginIds });
+  await migrateKibanaIndex({ client, kbnClient });
   return stats;
 }

src/es_archiver/actions/load.ts

@@ -106,7 +106,7 @@ export async function loadAction({
 
   // If we affected the Kibana index, we need to ensure it's migrated...
   if (Object.keys(result).some(k => k.startsWith('.kibana'))) {
-    await migrateKibanaIndex({ client, log, kibanaPluginIds });
+    await migrateKibanaIndex({ client, kbnClient });
 
     if (kibanaPluginIds.includes('spaces')) {
       await createDefaultSpace({ client, index: '.kibana' });

src/es_archiver/lib/indices/kibana_index.ts

@@ -17,42 +17,10 @@
  * under the License.
  */
 
-import { get } from 'lodash';
-import fs from 'fs';
-import Path from 'path';
-import { promisify } from 'util';
-import { toArray } from 'rxjs/operators';
 import { Client, CreateDocumentParams } from 'elasticsearch';
-import { ToolingLog } from '@kbn/dev-utils';
-
+import { ToolingLog, KbnClient } from '@kbn/dev-utils';
 import { Stats } from '../stats';
 import { deleteIndex } from './delete_index';
-import { KibanaMigrator } from '../../../core/server/saved_objects/migrations';
-import { LegacyConfig } from '../../../core/server';
-import { convertLegacyTypes } from '../../../core/server/saved_objects/utils';
-import { SavedObjectTypeRegistry } from '../../../core/server/saved_objects';
-// @ts-ignore
-import { collectUiExports } from '../../../legacy/ui/ui_exports';
-// @ts-ignore
-import { findPluginSpecs } from '../../../legacy/plugin_discovery';
-
-/**
- * Load the uiExports for a Kibana instance, only load uiExports from xpack if
- * it is enabled in the Kibana server.
- */
-const getUiExports = async (kibanaPluginIds: string[]) => {
-  const xpackEnabled = kibanaPluginIds.includes('xpack_main');
-
-  const { spec$ } = await findPluginSpecs({
-    plugins: {
-      scanDirs: [Path.resolve(__dirname, '../../../legacy/core_plugins')],
-      paths: xpackEnabled ? [Path.resolve(__dirname, '../../../../x-pack')] : [],
-    },
-  });
-
-  const specs = await spec$.pipe(toArray()).toPromise();
-  return collectUiExports(specs);
-};
 
 /**
  * Deletes all indices that start with `.kibana`
@@ -93,61 +61,21 @@ export async function deleteKibanaIndices({
  */
 export async function migrateKibanaIndex({
   client,
-  log,
-  kibanaPluginIds,
+  kbnClient,
 }: {
   client: Client;
-  log: ToolingLog;
-  kibanaPluginIds: string[];
+  kbnClient: KbnClient;
 }) {
-  const uiExports = await getUiExports(kibanaPluginIds);
-  const kibanaVersion = await loadKibanaVersion();
-
-  const configKeys: Record<string, string> = {
-    'xpack.task_manager.index': '.kibana_task_manager',
-  };
-  const config = { get: (path: string) => configKeys[path] };
-
-  const savedObjectTypes = convertLegacyTypes(uiExports, config as LegacyConfig);
-  const typeRegistry = new SavedObjectTypeRegistry();
-  savedObjectTypes.forEach(type => typeRegistry.registerType(type));
-
-  const logger = {
-    trace: log.verbose.bind(log),
-    debug: log.debug.bind(log),
-    info: log.info.bind(log),
-    warn: log.warning.bind(log),
-    error: log.error.bind(log),
-    fatal: log.error.bind(log),
-    log: (entry: any) => log.info(entry.message),
-    get: () => logger,
-  };
-
-  const migratorOptions = {
-    savedObjectsConfig: {
-      scrollDuration: '5m',
-      batchSize: 100,
-      pollInterval: 100,
-      skip: false,
+  // we allow dynamic mappings on the index, as some interceptors are accessing documents before
+  // the migration is actually performed. The migrator will put the value back to `strict` after migration.
+  await client.indices.putMapping({
+    index: '.kibana',
+    body: {
+      dynamic: true,
     },
-    kibanaConfig: {
-      index: '.kibana',
-    } as any,
-    logger,
-    kibanaVersion,
-    typeRegistry,
-    savedObjectValidations: uiExports.savedObjectValidations,
-    callCluster: (path: string, ...args: any[]) =>
-      (get(client, path) as Function).call(client, ...args),
-  };
+  } as any);
 
-  return await new KibanaMigrator(migratorOptions).runMigrations();
-}
-
-async function loadKibanaVersion() {
-  const readFile = promisify(fs.readFile);
-  const packageJson = await readFile(Path.join(__dirname, '../../../../package.json'));
-  return JSON.parse(packageJson.toString('utf-8')).version;
+  return await kbnClient.savedObjects.migrate();
 }
 
 /**