Skip to content

Commit

Permalink
Fix running Kibana Platform migrations in development (#61325)
Browse files Browse the repository at this point in the history
  • Loading branch information
joshdover authored Mar 25, 2020
1 parent 0d45d9b commit 4b012fb
Show file tree
Hide file tree
Showing 14 changed files with 71 additions and 117 deletions.
2 changes: 0 additions & 2 deletions docs/development/core/server/kibana-plugin-core-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,6 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
| [PluginConfigDescriptor](./kibana-plugin-core-server.pluginconfigdescriptor.md) | Describes a plugin configuration properties. |
| [PluginInitializerContext](./kibana-plugin-core-server.plugininitializercontext.md) | Context that's available to plugins during initialization stage. |
| [PluginManifest](./kibana-plugin-core-server.pluginmanifest.md) | Describes the set of required and optional properties plugin can define in its mandatory JSON manifest file. |
| [PluginsServiceSetup](./kibana-plugin-core-server.pluginsservicesetup.md) | |
| [PluginsServiceStart](./kibana-plugin-core-server.pluginsservicestart.md) | |
| [RequestHandlerContext](./kibana-plugin-core-server.requesthandlercontext.md) | Plugin specific context passed to a route handler.<!-- -->Provides the following clients and services: - [rendering](./kibana-plugin-core-server.iscopedrenderingclient.md) - Rendering client which uses the data of the incoming request - [savedObjects.client](./kibana-plugin-core-server.savedobjectsclient.md) - Saved Objects client which uses the credentials of the incoming request - [savedObjects.typeRegistry](./kibana-plugin-core-server.isavedobjecttyperegistry.md) - Type registry containing all the registered types. - [elasticsearch.dataClient](./kibana-plugin-core-server.scopedclusterclient.md) - Elasticsearch data client which uses the credentials of the incoming request - [elasticsearch.adminClient](./kibana-plugin-core-server.scopedclusterclient.md) - Elasticsearch admin client which uses the credentials of the incoming request - [uiSettings.client](./kibana-plugin-core-server.iuisettingsclient.md) - uiSettings client which uses the credentials of the incoming request |
| [RouteConfig](./kibana-plugin-core-server.routeconfig.md) | Route specific configuration. |
| [RouteConfigOptions](./kibana-plugin-core-server.routeconfigoptions.md) | Additional route options. |
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

1 change: 1 addition & 0 deletions src/core/server/legacy/legacy_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ beforeEach(() => {
},
savedObjects: savedObjectsServiceMock.createInternalSetupContract(),
plugins: {
initialized: true,
contracts: new Map([['plugin-id', 'plugin-value']]),
uiPlugins: {
public: new Map([['plugin-id', {} as DiscoveredPlugin]]),
Expand Down
1 change: 1 addition & 0 deletions src/core/server/plugins/plugins_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const createSetupContractMock = (): PluginsServiceSetup => ({
internal: new Map(),
public: new Map(),
},
initialized: true,
});
const createStartContractMock = () => ({ contracts: new Map() });
const createServiceMock = (): PluginsServiceMock => ({
Expand Down
57 changes: 38 additions & 19 deletions src/core/server/plugins/plugins_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -516,28 +516,29 @@ describe('PluginsService', () => {
});

describe('#setup()', () => {
describe('uiPlugins.internal', () => {
it('includes disabled plugins', async () => {
mockDiscover.mockReturnValue({
error$: from([]),
plugin$: from([
createPlugin('plugin-1', {
path: 'path-1',
version: 'some-version',
configPath: 'plugin1',
}),
createPlugin('plugin-2', {
path: 'path-2',
version: 'some-version',
configPath: 'plugin2',
}),
]),
});
beforeEach(() => {
mockDiscover.mockReturnValue({
error$: from([]),
plugin$: from([
createPlugin('plugin-1', {
path: 'path-1',
version: 'some-version',
configPath: 'plugin1',
}),
createPlugin('plugin-2', {
path: 'path-2',
version: 'some-version',
configPath: 'plugin2',
}),
]),
});

mockPluginSystem.uiPlugins.mockReturnValue(new Map());
mockPluginSystem.uiPlugins.mockReturnValue(new Map());
});

describe('uiPlugins.internal', () => {
it('includes disabled plugins', async () => {
config$.next({ plugins: { initialize: true }, plugin1: { enabled: false } });

await pluginsService.discover();
const { uiPlugins } = await pluginsService.setup({} as any);
expect(uiPlugins.internal).toMatchInlineSnapshot(`
Expand All @@ -552,6 +553,24 @@ describe('PluginsService', () => {
`);
});
});

describe('plugin initialization', () => {
it('does initialize if plugins.initialize is true', async () => {
config$.next({ plugins: { initialize: true } });
await pluginsService.discover();
const { initialized } = await pluginsService.setup({} as any);
expect(mockPluginSystem.setupPlugins).toHaveBeenCalled();
expect(initialized).toBe(true);
});

it('does not initialize if plugins.initialize is false', async () => {
config$.next({ plugins: { initialize: false } });
await pluginsService.discover();
const { initialized } = await pluginsService.setup({} as any);
expect(mockPluginSystem.setupPlugins).not.toHaveBeenCalled();
expect(initialized).toBe(false);
});
});
});

describe('#stop()', () => {
Expand Down
16 changes: 11 additions & 5 deletions src/core/server/plugins/plugins_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,11 @@ import { InternalCoreSetup, InternalCoreStart } from '../internal_types';
import { IConfigService } from '../config';
import { pick } from '../../utils';

/** @public */
/** @internal */
export interface PluginsServiceSetup {
/** Indicates whether or not plugins were initialized. */
initialized: boolean;
/** Setup contracts returned by plugins. */
contracts: Map<PluginName, unknown>;
uiPlugins: {
/**
Expand All @@ -55,8 +58,9 @@ export interface PluginsServiceSetup {
};
}

/** @public */
/** @internal */
export interface PluginsServiceStart {
/** Start contracts returned by plugins. */
contracts: Map<PluginName, unknown>;
}

Expand Down Expand Up @@ -103,14 +107,16 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
const config = await this.config$.pipe(first()).toPromise();

let contracts = new Map<PluginName, unknown>();
if (!config.initialize || this.coreContext.env.isDevClusterMaster) {
this.log.info('Plugin initialization disabled.');
} else {
const initialize = config.initialize && !this.coreContext.env.isDevClusterMaster;
if (initialize) {
contracts = await this.pluginsSystem.setupPlugins(deps);
} else {
this.log.info('Plugin initialization disabled.');
}

const uiPlugins = this.pluginsSystem.uiPlugins();
return {
initialized: initialize,
contracts,
uiPlugins: {
internal: this.uiPluginInternalInfo,
Expand Down
8 changes: 3 additions & 5 deletions src/core/server/saved_objects/saved_objects_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,12 @@ describe('SavedObjectsService', () => {
return expect(KibanaMigratorMock.mock.calls[0][0].callCluster()).resolves.toMatch('success');
});

it('skips KibanaMigrator migrations when --optimize=true', async () => {
const coreContext = createCoreContext({
env: ({ cliArgs: { optimize: true }, packageInfo: { version: 'x.x.x' } } as unknown) as Env,
});
it('skips KibanaMigrator migrations when pluginsInitialized=false', async () => {
const coreContext = createCoreContext({ skipMigration: false });
const soService = new SavedObjectsService(coreContext);

await soService.setup(createSetupDeps());
await soService.start({});
await soService.start({ pluginsInitialized: false });
expect(migratorInstanceMock.runMigrations).not.toHaveBeenCalled();
});

Expand Down
12 changes: 8 additions & 4 deletions src/core/server/saved_objects/saved_objects_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,9 @@ interface WrappedClientFactoryWrapper {

/** @internal */
// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface SavedObjectsStartDeps {}
export interface SavedObjectsStartDeps {
pluginsInitialized?: boolean;
}

export class SavedObjectsService
implements CoreService<InternalSavedObjectsServiceSetup, InternalSavedObjectsServiceStart> {
Expand Down Expand Up @@ -349,7 +351,7 @@ export class SavedObjectsService
}

public async start(
core: SavedObjectsStartDeps,
{ pluginsInitialized = true }: SavedObjectsStartDeps,
migrationsRetryDelay?: number
): Promise<InternalSavedObjectsServiceStart> {
if (!this.setupDeps || !this.config) {
Expand All @@ -376,9 +378,11 @@ export class SavedObjectsService
* However, our build system optimize step and some tests depend on the
* HTTP server running without an Elasticsearch server being available.
* So, when the `migrations.skip` is true, we skip migrations altogether.
*
* We also cannot safely run migrations if plugins are not initialized since
* not plugin migrations won't be registered.
*/
const cliArgs = this.coreContext.env.cliArgs;
const skipMigrations = cliArgs.optimize || this.config.migration.skip;
const skipMigrations = this.config.migration.skip || !pluginsInitialized;

if (skipMigrations) {
this.logger.warn(
Expand Down
9 changes: 4 additions & 5 deletions src/core/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1426,10 +1426,10 @@ export type PluginName = string;
// @public (undocumented)
export type PluginOpaqueId = symbol;

// @public (undocumented)
// @internal (undocumented)
export interface PluginsServiceSetup {
// (undocumented)
contracts: Map<PluginName, unknown>;
initialized: boolean;
// (undocumented)
uiPlugins: {
internal: Map<PluginName, InternalPluginInfo>;
Expand All @@ -1438,9 +1438,8 @@ export interface PluginsServiceSetup {
};
}

// @public (undocumented)
// @internal (undocumented)
export interface PluginsServiceStart {
// (undocumented)
contracts: Map<PluginName, unknown>;
}

Expand Down Expand Up @@ -2348,7 +2347,7 @@ export const validBodyOutput: readonly ["data", "stream"];
// src/core/server/legacy/types.ts:164:3 - (ae-forgotten-export) The symbol "LegacyNavLinkSpec" needs to be exported by the entry point index.d.ts
// src/core/server/legacy/types.ts:165:3 - (ae-forgotten-export) The symbol "LegacyAppSpec" needs to be exported by the entry point index.d.ts
// src/core/server/legacy/types.ts:166:16 - (ae-forgotten-export) The symbol "LegacyPluginSpec" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/plugins_service.ts:44:5 - (ae-forgotten-export) The symbol "InternalPluginInfo" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/plugins_service.ts:47:5 - (ae-forgotten-export) The symbol "InternalPluginInfo" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:226:3 - (ae-forgotten-export) The symbol "KibanaConfigType" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:226:3 - (ae-forgotten-export) The symbol "SharedGlobalConfigKeys" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:228:3 - (ae-forgotten-export) The symbol "PathConfigType" needs to be exported by the entry point index.d.ts
Expand Down
6 changes: 5 additions & 1 deletion src/core/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export class Server {
private readonly metrics: MetricsService;
private readonly coreApp: CoreApp;

private pluginsInitialized?: boolean;
private coreStart?: InternalCoreStart;

constructor(
Expand Down Expand Up @@ -156,6 +157,7 @@ export class Server {
};

const pluginsSetup = await this.plugins.setup(coreSetup);
this.pluginsInitialized = pluginsSetup.initialized;

const renderingSetup = await this.rendering.setup({
http: httpSetup,
Expand All @@ -176,7 +178,9 @@ export class Server {

public async start() {
this.log.debug('starting server');
const savedObjectsStart = await this.savedObjects.start({});
const savedObjectsStart = await this.savedObjects.start({
pluginsInitialized: this.pluginsInitialized,
});
const capabilitiesStart = this.capabilities.start();
const uiSettingsStart = await this.uiSettings.start();
const elasticsearchStart = await this.elasticsearch.start();
Expand Down

0 comments on commit 4b012fb

Please sign in to comment.