Skip to content

Commit

Permalink
Adapt es-archiver to call _migrate endpoint instead of creating a m…
Browse files Browse the repository at this point in the history
…igrator (elastic#56971)

* 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 <[email protected]>

* 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 <[email protected]>
Co-authored-by: Josh Dover <[email protected]>
Co-authored-by: Aleh Zasypkin <[email protected]>
  • Loading branch information
4 people committed Feb 26, 2020
1 parent f17dd96 commit 812596e
Show file tree
Hide file tree
Showing 16 changed files with 221 additions and 100 deletions.
19 changes: 19 additions & 0 deletions packages/kbn-dev-utils/src/kbn_client/kbn_client_saved_objects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
12 changes: 12 additions & 0 deletions packages/kbn-pm/dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Expand Down
18 changes: 15 additions & 3 deletions src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
8 changes: 8 additions & 0 deletions src/core/server/saved_objects/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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/');

Expand All @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand All @@ -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();
Expand All @@ -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(
[
Expand Down Expand Up @@ -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(
[
Expand Down Expand Up @@ -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(
[
Expand Down Expand Up @@ -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(
[
Expand Down Expand Up @@ -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(
[
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
}));
Original file line number Diff line number Diff line change
@@ -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);
});
});
45 changes: 45 additions & 0 deletions src/core/server/saved_objects/routes/migrate.ts
Original file line number Diff line number Diff line change
@@ -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,
},
});
})
);
};
13 changes: 9 additions & 4 deletions src/core/server/saved_objects/saved_objects_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -201,9 +202,9 @@ export interface SavedObjectsRepositoryFactory {

/** @internal */
export interface SavedObjectsSetupDeps {
http: InternalHttpServiceSetup;
legacyPlugins: LegacyServiceDiscoverPlugins;
elasticsearch: InternalElasticsearchServiceSetup;
http: InternalHttpServiceSetup;
}

interface WrappedClientFactoryWrapper {
Expand All @@ -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 = {};

Expand Down Expand Up @@ -262,6 +264,7 @@ export class SavedObjectsService
http: setupDeps.http,
logger: this.logger,
config: this.config,
migratorPromise: this.migrator$.pipe(first()).toPromise(),
importableExportableTypes,
});

Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 812596e

Please sign in to comment.