From 2a01ed74198c61ac126252a8d846136ed1406b96 Mon Sep 17 00:00:00 2001 From: restrry Date: Thu, 9 May 2019 14:34:28 +0200 Subject: [PATCH] move plugin schema registration in plugins plugins service plugins system is not setup when kibana is run in optimizer mode, so config keys aren't marked as applied. --- .../server/plugins/plugins_service.test.ts | 47 ++++++++++++++++++- src/core/server/plugins/plugins_service.ts | 4 ++ .../server/plugins/plugins_system.test.ts | 15 ------ src/core/server/plugins/plugins_system.ts | 6 --- 4 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts index 8d1a57234e7517..d0c9f52a01bf41 100644 --- a/src/core/server/plugins/plugins_service.test.ts +++ b/src/core/server/plugins/plugins_service.test.ts @@ -19,8 +19,9 @@ import { mockDiscover, mockPackage } from './plugins_service.test.mocks'; -import { resolve } from 'path'; +import { resolve, join } from 'path'; import { BehaviorSubject, from } from 'rxjs'; +import { schema } from '@kbn/config-schema'; import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config'; import { getEnvOptions } from '../config/__mocks__/env'; @@ -44,6 +45,13 @@ const setupDeps = { http: httpServiceMock.createSetupContract(), }; const logger = loggingServiceMock.create(); + +['path-1', 'path-2', 'path-3', 'path-4', 'path-5'].forEach(path => { + jest.doMock(join(path, 'server'), () => ({}), { + virtual: true, + }); +}); + beforeEach(() => { mockPackage.raw = { branch: 'feature-v1', @@ -328,3 +336,40 @@ test('`stop` stops plugins system', async () => { await pluginsService.stop(); expect(mockPluginSystem.stopPlugins).toHaveBeenCalledTimes(1); }); + +test('`setup` registers plugin config schema in config service', async () => { + const configSchema = schema.string(); + jest.spyOn(configService, 'setSchema').mockImplementation(() => Promise.resolve()); + jest.doMock( + join('path-with-schema', 'server'), + () => ({ + config: { + schema: configSchema, + }, + }), + { + virtual: true, + } + ); + mockDiscover.mockReturnValue({ + error$: from([]), + plugin$: from([ + new PluginWrapper( + 'path-with-schema', + { + id: 'some-id', + version: 'some-version', + configPath: 'path', + kibanaVersion: '7.0.0', + requiredPlugins: [], + optionalPlugins: [], + server: true, + ui: true, + }, + { logger } as any + ), + ]), + }); + await pluginsService.setup(setupDeps); + expect(configService.setSchema).toBeCalledWith('path', configSchema); +}); diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts index f243754db84154..9d52f7f364a5a5 100644 --- a/src/core/server/plugins/plugins_service.ts +++ b/src/core/server/plugins/plugins_service.ts @@ -130,6 +130,10 @@ export class PluginsService implements CoreService { + const schema = plugin.getConfigSchema(); + if (schema) { + await this.coreContext.configService.setSchema(plugin.configPath, schema); + } const isEnabled = await this.coreContext.configService.isEnabledAtPath(plugin.configPath); if (pluginEnableStatuses.has(plugin.name)) { diff --git a/src/core/server/plugins/plugins_system.test.ts b/src/core/server/plugins/plugins_system.test.ts index 7bc242b924c153..9754c1b03d0301 100644 --- a/src/core/server/plugins/plugins_system.test.ts +++ b/src/core/server/plugins/plugins_system.test.ts @@ -23,7 +23,6 @@ import { } from './plugins_system.test.mocks'; import { BehaviorSubject } from 'rxjs'; -import { schema } from '@kbn/config-schema'; import { Env } from '../config'; import { getEnvOptions } from '../config/__mocks__/env'; @@ -119,7 +118,6 @@ test('`setupPlugins` throws if plugins have circular optional dependency', async test('`setupPlugins` ignores missing optional dependency', async () => { const plugin = createPlugin('some-id', { optional: ['missing-dep'] }); jest.spyOn(plugin, 'setup').mockResolvedValue('test'); - jest.spyOn(plugin, 'getConfigSchema').mockReturnValue(null); pluginsSystem.addPlugin(plugin); @@ -182,7 +180,6 @@ test('correctly orders plugins and returns exposed values for "setup" and "start [...plugins.keys()].forEach((plugin, index) => { jest.spyOn(plugin, 'setup').mockResolvedValue(`added-as-${index}`); jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`); - jest.spyOn(plugin, 'getConfigSchema').mockReturnValue(null); setupContextMap.set(plugin.name, `setup-for-${plugin.name}`); startContextMap.set(plugin.name, `start-for-${plugin.name}`); @@ -269,7 +266,6 @@ test('`setupPlugins` only setups plugins that have server side', async () => { [firstPluginToRun, secondPluginNotToRun, thirdPluginToRun].forEach((plugin, index) => { jest.spyOn(plugin, 'setup').mockResolvedValue(`added-as-${index}`); - jest.spyOn(plugin, 'getConfigSchema').mockReturnValue(null); pluginsSystem.addPlugin(plugin); }); @@ -359,7 +355,6 @@ test('`startPlugins` only starts plugins that were setup', async () => { [firstPluginToRun, secondPluginNotToRun, thirdPluginToRun].forEach((plugin, index) => { jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`); jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`); - jest.spyOn(plugin, 'getConfigSchema').mockReturnValue(null); pluginsSystem.addPlugin(plugin); }); @@ -378,13 +373,3 @@ Array [ ] `); }); - -test('`setup` calls configService.setSchema if plugin specifies config schema', async () => { - const pluginSchema = schema.string(); - const plugin = createPlugin('plugin-1'); - jest.spyOn(plugin, 'setup').mockResolvedValue('test'); - jest.spyOn(plugin, 'getConfigSchema').mockReturnValue(pluginSchema); - pluginsSystem.addPlugin(plugin); - await pluginsSystem.setupPlugins(setupDeps); - await expect(configService.setSchema).toBeCalledWith('path', pluginSchema); -}); diff --git a/src/core/server/plugins/plugins_system.ts b/src/core/server/plugins/plugins_system.ts index abf8344dece8a1..37eab8226af728 100644 --- a/src/core/server/plugins/plugins_system.ts +++ b/src/core/server/plugins/plugins_system.ts @@ -70,12 +70,6 @@ export class PluginsSystem { {} as Record ); - const schema = plugin.getConfigSchema(); - - if (schema) { - await this.coreContext.configService.setSchema(plugin.configPath, schema); - } - contracts.set( pluginName, await plugin.setup(