Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix running Kibana Platform migrations in development #61325

Merged
merged 3 commits into from
Mar 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 });
Comment on lines +174 to +179
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably low impact, but we now have the config SO type that is registered by core, not plugins. The type would be usable but not migrated with PR changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably the correct way to solve is to have the processes that need to run Core only run setup and extract whatever information they need.

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