diff --git a/src/core/server/__snapshots__/server.test.ts.snap b/src/core/server/__snapshots__/server.test.ts.snap deleted file mode 100644 index 19a3d188e1b485..00000000000000 --- a/src/core/server/__snapshots__/server.test.ts.snap +++ /dev/null @@ -1,17 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`does not fail on "setup" if there are unused paths detected: unused paths logs 1`] = ` -Object { - "debug": Array [ - Array [ - "setting up server", - ], - ], - "error": Array [], - "fatal": Array [], - "info": Array [], - "log": Array [], - "trace": Array [], - "warn": Array [], -} -`; diff --git a/src/core/server/config/__snapshots__/config_service.test.ts.snap b/src/core/server/config/__snapshots__/config_service.test.ts.snap index dc2546c6d8cb86..1e58438f58560e 100644 --- a/src/core/server/config/__snapshots__/config_service.test.ts.snap +++ b/src/core/server/config/__snapshots__/config_service.test.ts.snap @@ -13,6 +13,4 @@ ExampleClassWithSchema { } `; -exports[`throws error if 'schema' is not defined for a key 1`] = `[Error: No config validator defined for key]`; - exports[`throws if config at path does not match schema 1`] = `"[key]: expected value of type [string] but got [number]"`; diff --git a/src/core/server/config/config_service.mock.ts b/src/core/server/config/config_service.mock.ts index 9a32a36e2c12e9..daf6bb28c1e4c7 100644 --- a/src/core/server/config/config_service.mock.ts +++ b/src/core/server/config/config_service.mock.ts @@ -25,13 +25,13 @@ import { ConfigService } from './config_service'; type ConfigSericeContract = PublicMethodsOf; const createConfigServiceMock = () => { const mocked: jest.Mocked = { - validateAll: jest.fn(), atPath: jest.fn(), getConfig$: jest.fn(), optionalAtPath: jest.fn(), getUsedPaths: jest.fn(), getUnusedPaths: jest.fn(), isEnabledAtPath: jest.fn(), + preSetup: jest.fn(), }; mocked.atPath.mockReturnValue(new BehaviorSubject({})); mocked.getConfig$.mockReturnValue(new BehaviorSubject(new ObjectToConfigAdapter({}))); diff --git a/src/core/server/config/config_service.test.ts b/src/core/server/config/config_service.test.ts index 28a8f32ffba671..b4a51a0f5bb3b7 100644 --- a/src/core/server/config/config_service.test.ts +++ b/src/core/server/config/config_service.test.ts @@ -40,11 +40,12 @@ class ExampleClassWithStringSchema { constructor(readonly value: string) {} } -const stringSchemaFor = (key: string) => new Map([[key, ExampleClassWithStringSchema.schema]]); +const stringSchemaFor = (key: string) => new Map([[key, schema.string()]]); test('returns config at path as observable', async () => { const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'foo' })); - const configService = new ConfigService(config$, defaultEnv, logger, stringSchemaFor('key')); + const configService = new ConfigService(config$, defaultEnv, logger); + configService.preSetup(stringSchemaFor('key')); const configs = configService.atPath('key', ExampleClassWithStringSchema); const exampleConfig = await configs.pipe(first()).toPromise(); @@ -57,7 +58,9 @@ test('throws if config at path does not match schema', async () => { const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 123 })); - const configService = new ConfigService(config$, defaultEnv, logger, stringSchemaFor('key')); + const configService = new ConfigService(config$, defaultEnv, logger); + configService.preSetup(stringSchemaFor('key')); + const configs = configService.atPath('key', ExampleClassWithStringSchema); try { @@ -69,12 +72,8 @@ test('throws if config at path does not match schema', async () => { test("returns undefined if fetching optional config at a path that doesn't exist", async () => { const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ foo: 'bar' })); - const configService = new ConfigService( - config$, - defaultEnv, - logger, - stringSchemaFor('unique-name') - ); + const configService = new ConfigService(config$, defaultEnv, logger); + configService.preSetup(stringSchemaFor('unique-name')); const configs = configService.optionalAtPath('unique-name', ExampleClassWithStringSchema); const exampleConfig = await configs.pipe(first()).toPromise(); @@ -84,7 +83,8 @@ test("returns undefined if fetching optional config at a path that doesn't exist test('returns observable config at optional path if it exists', async () => { const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ value: 'bar' })); - const configService = new ConfigService(config$, defaultEnv, logger, stringSchemaFor('value')); + const configService = new ConfigService(config$, defaultEnv, logger); + configService.preSetup(stringSchemaFor('value')); const configs = configService.optionalAtPath('value', ExampleClassWithStringSchema); const exampleConfig: any = await configs.pipe(first()).toPromise(); @@ -95,7 +95,8 @@ test('returns observable config at optional path if it exists', async () => { test("does not push new configs when reloading if config at path hasn't changed", async () => { const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'value' })); - const configService = new ConfigService(config$, defaultEnv, logger, stringSchemaFor('key')); + const configService = new ConfigService(config$, defaultEnv, logger); + configService.preSetup(stringSchemaFor('key')); const valuesReceived: any[] = []; configService.atPath('key', ExampleClassWithStringSchema).subscribe(config => { @@ -109,7 +110,8 @@ test("does not push new configs when reloading if config at path hasn't changed" test('pushes new config when reloading and config at path has changed', async () => { const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'value' })); - const configService = new ConfigService(config$, defaultEnv, logger, stringSchemaFor('key')); + const configService = new ConfigService(config$, defaultEnv, logger); + configService.preSetup(stringSchemaFor('key')); const valuesReceived: any[] = []; configService.atPath('key', ExampleClassWithStringSchema).subscribe(config => { @@ -121,21 +123,29 @@ test('pushes new config when reloading and config at path has changed', async () expect(valuesReceived).toEqual(['value', 'new value']); }); -test("throws error if 'schema' is not defined for a key", async () => { - expect.assertions(1); - +test('throws error if reads config before schema is set', async () => { class ExampleClass {} const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'value' })); - const configService = new ConfigService(config$, defaultEnv, logger, new Map()); + const configService = new ConfigService(config$, defaultEnv, logger); + const configs = configService.atPath('key', ExampleClass as any); + expect(configs.pipe(first()).toPromise()).rejects.toMatchInlineSnapshot( + `[Error: No validation schema has been defined]` + ); +}); + +test("throws error if 'schema' is not defined for a key", async () => { + class ExampleClass {} + + const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'value' })); + const configService = new ConfigService(config$, defaultEnv, logger); + configService.preSetup(stringSchemaFor('no-key')); const configs = configService.atPath('key', ExampleClass as any); - try { - await configs.pipe(first()).toPromise(); - } catch (e) { - expect(e).toMatchSnapshot(); - } + expect(configs.pipe(first()).toPromise()).rejects.toMatchInlineSnapshot( + `[Error: No config validator defined for key]` + ); }); test('tracks unhandled paths', async () => { @@ -160,7 +170,7 @@ test('tracks unhandled paths', async () => { }; const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig)); - const configService = new ConfigService(config$, defaultEnv, logger, new Map()); + const configService = new ConfigService(config$, defaultEnv, logger); configService.atPath('foo', createClassWithSchema(schema.string())); configService.atPath( @@ -207,12 +217,8 @@ test('correctly passes context', async () => { defaultValue: schema.contextRef('version'), }), }); - const configService = new ConfigService( - config$, - env, - logger, - new Map([['foo', schemaDefinition]]) - ); + const configService = new ConfigService(config$, env, logger); + configService.preSetup(new Map([['foo', schemaDefinition]])); const configs = configService.atPath('foo', createClassWithSchema(schemaDefinition)); @@ -228,7 +234,7 @@ test('handles enabled path, but only marks the enabled path as used', async () = }; const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig)); - const configService = new ConfigService(config$, defaultEnv, logger, new Map()); + const configService = new ConfigService(config$, defaultEnv, logger); const isEnabled = await configService.isEnabledAtPath('pid'); expect(isEnabled).toBe(true); @@ -246,7 +252,7 @@ test('handles enabled path when path is array', async () => { }; const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig)); - const configService = new ConfigService(config$, defaultEnv, logger, new Map()); + const configService = new ConfigService(config$, defaultEnv, logger); const isEnabled = await configService.isEnabledAtPath(['pid']); expect(isEnabled).toBe(true); @@ -264,7 +270,7 @@ test('handles disabled path and marks config as used', async () => { }; const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig)); - const configService = new ConfigService(config$, defaultEnv, logger, new Map()); + const configService = new ConfigService(config$, defaultEnv, logger); const isEnabled = await configService.isEnabledAtPath('pid'); expect(isEnabled).toBe(false); @@ -277,7 +283,7 @@ test('treats config as enabled if config path is not present in config', async ( const initialConfig = {}; const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig)); - const configService = new ConfigService(config$, defaultEnv, logger, new Map()); + const configService = new ConfigService(config$, defaultEnv, logger); const isEnabled = await configService.isEnabledAtPath('pid'); expect(isEnabled).toBe(true); diff --git a/src/core/server/config/config_service.ts b/src/core/server/config/config_service.ts index d34caf6c351fec..7c9b554877b295 100644 --- a/src/core/server/config/config_service.ts +++ b/src/core/server/config/config_service.ts @@ -34,17 +34,24 @@ export class ConfigService { * then list all unhandled config paths when the startup process is completed. */ private readonly handledPaths: ConfigPath[] = []; - private readonly schemas = new Map>(); + private schemas?: Map>; constructor( private readonly config$: Observable, private readonly env: Env, - logger: LoggerFactory, - schemas: Map> = new Map() + logger: LoggerFactory ) { this.log = logger.get('config'); + } + + public async preSetup(schemas: Map>) { + this.schemas = new Map(); for (const [path, schema] of schemas) { - this.schemas.set(pathToString(path), schema); + const namespace = pathToString(path); + this.schemas.set(namespace, schema); + await this.getValidatedConfig(namespace) + .pipe(first()) + .toPromise(); } } @@ -125,15 +132,10 @@ export class ConfigService { return config.getFlattenedPaths().filter(path => isPathHandled(path, handledPaths)); } - public async validateAll() { - for (const namespace of this.schemas.keys()) { - await this.getValidatedConfig(namespace) - .pipe(first()) - .toPromise(); - } - } - private validateConfig(path: ConfigPath, config: Record) { + if (!this.schemas) { + throw new Error('No validation schema has been defined'); + } const namespace = pathToString(path); const schema = this.schemas.get(namespace); if (!schema) { diff --git a/src/core/server/http/http_service.test.ts b/src/core/server/http/http_service.test.ts index b649b8c4b10f34..7799052a74fb69 100644 --- a/src/core/server/http/http_service.test.ts +++ b/src/core/server/http/http_service.test.ts @@ -30,18 +30,21 @@ import { getEnvOptions } from '../config/__mocks__/env'; const logger = loggingServiceMock.create(); const env = Env.createDefault(getEnvOptions()); -const createConfigService = (value: Partial = {}) => - new ConfigService( +const createConfigService = (value: Partial = {}) => { + const configService = new ConfigService( new BehaviorSubject( new ObjectToConfigAdapter({ server: value, }) ), env, - logger, - new Map([['server', configDefinition.schema]]) + logger ); + configService.preSetup(new Map([['server', configDefinition.schema]])) + return configService; +}; + afterEach(() => { jest.clearAllMocks(); }); diff --git a/src/core/server/index.test.mocks.ts b/src/core/server/index.test.mocks.ts index a3bcaa56da975b..4e61316fcff940 100644 --- a/src/core/server/index.test.mocks.ts +++ b/src/core/server/index.test.mocks.ts @@ -23,7 +23,8 @@ jest.doMock('./http/http_service', () => ({ HttpService: jest.fn(() => httpService), })); -export const mockPluginsService = { setup: jest.fn(), start: jest.fn(), stop: jest.fn() }; +import { pluginServiceMock } from './plugins/plugins_service.mock'; +export const mockPluginsService = pluginServiceMock.create(); jest.doMock('./plugins/plugins_service', () => ({ PluginsService: jest.fn(() => mockPluginsService), })); @@ -38,3 +39,9 @@ export const mockLegacyService = { setup: jest.fn(), start: jest.fn(), stop: jes jest.mock('./legacy/legacy_service', () => ({ LegacyService: jest.fn(() => mockLegacyService), })); + +import { configServiceMock } from './config/config_service.mock'; +export const configService = configServiceMock.create(); +jest.doMock('./config/config_service', () => ({ + ConfigService: jest.fn(() => configService), +})); diff --git a/src/core/server/plugins/discovery/plugins_discovery.mock.ts b/src/core/server/plugins/discovery/plugins_discovery.mock.ts new file mode 100644 index 00000000000000..ab3c65edbf3c5c --- /dev/null +++ b/src/core/server/plugins/discovery/plugins_discovery.mock.ts @@ -0,0 +1,20 @@ +/* + * 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. + */ +export const mockDiscover = jest.fn(); +jest.mock('./plugins_discovery', () => ({ discover: mockDiscover })); diff --git a/src/core/server/plugins/plugins_service.mock.ts b/src/core/server/plugins/plugins_service.mock.ts new file mode 100644 index 00000000000000..40eeb258af2d6f --- /dev/null +++ b/src/core/server/plugins/plugins_service.mock.ts @@ -0,0 +1,41 @@ +/* + * 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 { PluginsService } from './plugins_service'; + +type ServiceContract = PublicMethodsOf; +const createServiceMock = () => { + const mocked: jest.Mocked = { + preSetup: jest.fn(), + setup: jest.fn(), + start: jest.fn(), + stop: jest.fn(), + }; + mocked.preSetup.mockResolvedValue({ + pluginDefinitions: [], + errors: [], + searchPaths: [], + devPluginPaths: [], + }); + return mocked; +}; + +export const pluginServiceMock = { + create: createServiceMock, +}; diff --git a/src/core/server/plugins/plugins_service.test.mocks.ts b/src/core/server/plugins/plugins_service.test.mocks.ts index 13b492e382d676..334a03a278b5a6 100644 --- a/src/core/server/plugins/plugins_service.test.mocks.ts +++ b/src/core/server/plugins/plugins_service.test.mocks.ts @@ -20,7 +20,4 @@ export const mockPackage = new Proxy({ raw: {} as any }, { get: (obj, prop) => obj.raw[prop] }); jest.mock('../../../legacy/utils/package_json', () => ({ pkg: mockPackage })); -export const mockDiscover = jest.fn(); -jest.mock('./discovery/plugins_discovery', () => ({ discover: mockDiscover })); - jest.mock('./plugins_system'); diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts index 7b7e4476681b34..6f77aaaa593d36 100644 --- a/src/core/server/plugins/plugins_service.test.ts +++ b/src/core/server/plugins/plugins_service.test.ts @@ -19,6 +19,7 @@ import { BehaviorSubject } from 'rxjs'; import { mockPackage } from './plugins_service.test.mocks'; +import { mockDiscover } from './discovery/plugins_discovery.mock'; import { Env } from '../config'; import { configServiceMock } from '../config/config_service.mock'; @@ -66,14 +67,21 @@ afterEach(() => { jest.clearAllMocks(); }); +test('`setup` throws if `preSetup` has not been called before', async () => { + await expect(pluginsService.setup(setupDeps)).rejects.toMatchInlineSnapshot( + `[Error: pre-setup has not been run]` + ); +}); + test('`setup` throws if plugin has an invalid manifest', async () => { - const plugins = { + mockDiscover.mockResolvedValue({ pluginDefinitions: [], errors: [PluginDiscoveryError.invalidManifest('path-1', new Error('Invalid JSON'))], searchPaths: [], devPluginPaths: [], - }; - await expect(pluginsService.setup(setupDeps, plugins)).rejects.toMatchInlineSnapshot(` + }); + await pluginsService.preSetup(); + await expect(pluginsService.setup(setupDeps)).rejects.toMatchInlineSnapshot(` [Error: Failed to initialize plugins: Invalid JSON (invalid-manifest, path-1)] `); @@ -87,13 +95,14 @@ Array [ }); test('`setup` throws if plugin required Kibana version is incompatible with the current version', async () => { - const plugins = { + mockDiscover.mockResolvedValue({ pluginDefinitions: [], errors: [PluginDiscoveryError.incompatibleVersion('path-3', new Error('Incompatible version'))], searchPaths: [], devPluginPaths: [], - }; - await expect(pluginsService.setup(setupDeps, plugins)).rejects.toMatchInlineSnapshot(` + }); + await pluginsService.preSetup(); + await expect(pluginsService.setup(setupDeps)).rejects.toMatchInlineSnapshot(` [Error: Failed to initialize plugins: Incompatible version (incompatible-version, path-3)] `); @@ -141,8 +150,10 @@ test('`setup` throws if discovered plugins with conflicting names', async () => searchPaths: [], devPluginPaths: [], }; + mockDiscover.mockResolvedValue(plugins); - await expect(pluginsService.setup(setupDeps, plugins)).rejects.toMatchInlineSnapshot( + await pluginsService.preSetup(); + await expect(pluginsService.setup(setupDeps)).rejects.toMatchInlineSnapshot( `[Error: Plugin with id "conflicting-id" is already registered!]` ); @@ -217,8 +228,10 @@ test('`setup` properly detects plugins that should be disabled.', async () => { searchPaths: [], devPluginPaths: [], }; + mockDiscover.mockResolvedValue(plugins); - const start = await pluginsService.setup(setupDeps, plugins); + await pluginsService.preSetup(); + const start = await pluginsService.setup(setupDeps); expect(start.contracts).toBeInstanceOf(Map); expect(start.uiPlugins.public).toBeInstanceOf(Map); @@ -288,7 +301,10 @@ test('`setup` properly invokes `discover` and ignores non-critical errors.', asy mockPluginSystem.setupPlugins.mockResolvedValue(contracts); mockPluginSystem.uiPlugins.mockReturnValue(discoveredPlugins); - const setup = await pluginsService.setup(setupDeps, plugins); + mockDiscover.mockResolvedValue(plugins); + + await pluginsService.preSetup(); + const setup = await pluginsService.setup(setupDeps); expect(setup.contracts).toBe(contracts); expect(setup.uiPlugins).toBe(discoveredPlugins); diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts index 15579dd9c4126e..c937e696b08fc2 100644 --- a/src/core/server/plugins/plugins_service.ts +++ b/src/core/server/plugins/plugins_service.ts @@ -32,6 +32,7 @@ import { DiscoveredPlugin, DiscoveredPluginInternal, PluginWrapper, PluginName } import { createPluginInitializerContext } from './plugin_context'; import { PluginsConfig } from './plugins_config'; import { PluginsSystem } from './plugins_system'; +import { discover } from './discovery'; /** @internal */ export interface PluginsServiceSetup { @@ -60,20 +61,27 @@ export interface PluginsServiceStartDeps {} // eslint-disable-line @typescript-e export class PluginsService implements CoreService { private readonly log: Logger; private readonly pluginsSystem: PluginsSystem; + private pluginDefinitions?: DiscoveredPluginsDefinitions; constructor(private readonly coreContext: CoreContext) { this.log = coreContext.logger.get('plugins-service'); this.pluginsSystem = new PluginsSystem(coreContext); } - public async setup( - deps: PluginsServiceSetupDeps, - newPlatformPluginDefinitions: DiscoveredPluginsDefinitions - ) { + public async preSetup(devPluginPaths: ReadonlyArray = []) { + const { env, logger } = this.coreContext; + this.pluginDefinitions = await discover(env.pluginSearchPaths, devPluginPaths, env, logger); + return this.pluginDefinitions; + } + + public async setup(deps: PluginsServiceSetupDeps) { + if (!this.pluginDefinitions) { + throw new Error('pre-setup has not been run'); + } this.log.debug('Setting up plugins service'); - this.handleDiscoveryErrors(newPlatformPluginDefinitions.errors); + this.handleDiscoveryErrors(this.pluginDefinitions.errors); - const plugins = newPlatformPluginDefinitions.pluginDefinitions.map( + const plugins = this.pluginDefinitions.pluginDefinitions.map( ({ path, manifest }) => new PluginWrapper( path, diff --git a/src/core/server/root/index.test.mocks.ts b/src/core/server/root/index.test.mocks.ts index 68b014a4a96192..5e5cebee6691d2 100644 --- a/src/core/server/root/index.test.mocks.ts +++ b/src/core/server/root/index.test.mocks.ts @@ -29,5 +29,5 @@ jest.doMock('../config/config_service', () => ({ ConfigService: jest.fn(() => configService), })); -export const mockServer = { setup: jest.fn(), stop: jest.fn() }; +export const mockServer = { preSetup: jest.fn(), setup: jest.fn(), stop: jest.fn(), configService }; jest.mock('../server', () => ({ Server: jest.fn(() => mockServer) })); diff --git a/src/core/server/root/index.test.ts b/src/core/server/root/index.test.ts index 2a31c32f3a42cf..fcd8f84ed31a70 100644 --- a/src/core/server/root/index.test.ts +++ b/src/core/server/root/index.test.ts @@ -145,13 +145,11 @@ test('fails and stops services if initial logger upgrade fails', async () => { expect(mockOnShutdown).not.toHaveBeenCalled(); expect(logger.stop).not.toHaveBeenCalled(); - expect(mockServer.setup).not.toHaveBeenCalled(); await expect(root.setup()).rejects.toThrowError('logging config upgrade failed'); expect(mockOnShutdown).toHaveBeenCalledTimes(1); expect(mockOnShutdown).toHaveBeenCalledWith(loggingUpgradeError); - expect(mockServer.setup).not.toHaveBeenCalled(); expect(logger.stop).toHaveBeenCalledTimes(1); expect(mockConsoleError.mock.calls).toMatchSnapshot(); @@ -195,19 +193,3 @@ test('stops services if consequent logger upgrade fails', async () => { expect(mockConsoleError.mock.calls).toMatchSnapshot(); }); - -test(`doesn't create server if config validation fails`, async () => { - configService.validateAll.mockImplementation(() => { - throw new Error('invalid config'); - }); - const ServerMock = jest.fn(); - jest.doMock('../server', () => ({ - Server: ServerMock, - })); - const onShutdown = jest.fn(); - const root = new Root(config$, env, onShutdown); - - await expect(root.setup()).rejects.toThrowErrorMatchingInlineSnapshot(`"invalid config"`); - expect(ServerMock).not.toHaveBeenCalled(); - expect(onShutdown).toHaveBeenCalledTimes(1); -}); diff --git a/src/core/server/root/index.ts b/src/core/server/root/index.ts index d05db6f82c33f1..7f227ecacfaa54 100644 --- a/src/core/server/root/index.ts +++ b/src/core/server/root/index.ts @@ -19,22 +19,11 @@ import { ConnectableObservable, Observable, Subscription } from 'rxjs'; import { first, map, publishReplay, switchMap, tap } from 'rxjs/operators'; -import { Type } from '@kbn/config-schema'; -import { Config, ConfigPath, ConfigService, Env } from '../config'; +import { Config, ConfigService, Env } from '../config'; import { Logger, LoggerFactory, LoggingConfig, LoggingService } from '../logging'; import { Server } from '../server'; -import { configDefinition as elasticsearchConfigDefinition } from '../elasticsearch'; -import { configDefinition as httpConfigDefinition } from '../http'; -import { configDefinition as loggingConfigDefinition } from '../logging'; -import { - configDefinition as pluginsConfigDefinition, - discover, - DiscoveredPluginsDefinitions, -} from '../plugins'; -import { configDefinition as devConfigDefinition } from '../dev'; - /** * Top-level entry point to kick off the app and start the Kibana server. */ @@ -42,35 +31,26 @@ export class Root { public readonly logger: LoggerFactory; private readonly log: Logger; private readonly loggingService: LoggingService; + private readonly server: Server; private loggingConfigSubscription?: Subscription; - private server?: Server; - private configService?: ConfigService; constructor( - private readonly config$: Observable, - private readonly env: Env, + config$: Observable, + env: Env, private readonly onShutdown?: (reason?: Error | string) => void ) { this.loggingService = new LoggingService(); this.logger = this.loggingService.asLoggerFactory(); this.log = this.logger.get('root'); + this.server = new Server(config$, env, this.logger); } public async setup() { try { - const newPlatformPluginDefinitions = await this.discoverPlugins(); - const schemas = this.getSchemas(newPlatformPluginDefinitions); - - this.configService = new ConfigService(this.config$, this.env, this.logger, schemas); - - await this.configService.validateAll(); - - this.server = new Server(this.configService, this.logger, this.env); - this.log.debug('setting up root'); - - await this.setupLogging(); - return await this.server.setup(newPlatformPluginDefinitions); + await this.server.preSetup(); + await this.setupLogging(this.server.configService); + return await this.server.setup(); } catch (e) { await this.shutdown(e); throw e; @@ -79,10 +59,6 @@ export class Root { public async start() { this.log.debug('starting root'); - if (!this.server) { - throw new Error('server is not set up yet'); - } - try { return await this.server.start(); } catch (e) { @@ -119,44 +95,11 @@ export class Root { } } - private async discoverPlugins() { - const config = await this.config$.pipe(first()).toPromise(); - const isDev = this.env.mode.dev; - const hasDevPaths = Boolean(config.get('plugins') && config.get('plugins').paths); - const devPluginPaths = isDev && hasDevPaths ? config.get('plugins').paths : []; - - return await discover(this.env.pluginSearchPaths, devPluginPaths, this.env, this.logger); - } - - private getSchemas(pluginDefinitions: DiscoveredPluginsDefinitions) { - const pluginConfigSchemas = new Map( - pluginDefinitions.pluginDefinitions - .filter(pluginDef => Boolean(pluginDef.schema)) - .map( - pluginDef => - [pluginDef.manifest.configPath, pluginDef.schema!] as [ConfigPath, Type] - ) - ); - - const coreConfigSchemas = new Map>([ - [elasticsearchConfigDefinition.configPath, elasticsearchConfigDefinition.schema], - [loggingConfigDefinition.configPath, loggingConfigDefinition.schema], - [httpConfigDefinition.configPath, httpConfigDefinition.schema], - [pluginsConfigDefinition.configPath, pluginsConfigDefinition.schema], - [devConfigDefinition.configPath, devConfigDefinition.schema], - ]); - - return new Map([...pluginConfigSchemas, ...coreConfigSchemas]); - } - - private async setupLogging() { - if (this.configService === undefined) { - throw new Error('config server is not created yet'); - } + private async setupLogging(configService: ConfigService) { // Stream that maps config updates to logger updates, including update failures. - const update$ = this.configService.getConfig$().pipe( + const update$ = configService.getConfig$().pipe( // always read the logging config when the underlying config object is re-read - switchMap(() => this.configService!.atPath('logging', LoggingConfig)), + switchMap(() => configService.atPath('logging', LoggingConfig)), map(config => this.loggingService.upgrade(config)), // This specifically console.logs because we were not able to configure the logger. // eslint-disable-next-line no-console diff --git a/src/core/server/server.test.ts b/src/core/server/server.test.ts index b6114bc3e7a7dd..8c4931a9a043d9 100644 --- a/src/core/server/server.test.ts +++ b/src/core/server/server.test.ts @@ -22,55 +22,38 @@ import { httpService, mockLegacyService, mockPluginsService, + configService, } from './index.test.mocks'; import { BehaviorSubject } from 'rxjs'; -import { Env } from './config'; +import { Env, Config, ObjectToConfigAdapter } from './config'; import { Server } from './server'; import { getEnvOptions } from './config/__mocks__/env'; -import { configServiceMock } from './config/config_service.mock'; import { loggingServiceMock } from './logging/logging_service.mock'; -const configService = configServiceMock.create(); const env = new Env('.', getEnvOptions()); const logger = loggingServiceMock.create(); -const newPlatformPluginDefinitions = { - pluginDefinitions: [], - errors: [], - searchPaths: [], - devPluginPaths: [], -}; + beforeEach(() => { configService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: true })); }); afterEach(() => { jest.clearAllMocks(); - - configService.atPath.mockClear(); - httpService.setup.mockClear(); - httpService.stop.mockClear(); - elasticsearchService.setup.mockClear(); - elasticsearchService.stop.mockClear(); - mockPluginsService.setup.mockClear(); - mockPluginsService.stop.mockClear(); - mockLegacyService.setup.mockClear(); - mockLegacyService.stop.mockClear(); }); +const config$ = new BehaviorSubject(new ObjectToConfigAdapter({})); test('sets up services on "setup"', async () => { - const mockPluginsServiceSetup = new Map([['some-plugin', 'some-value']]); - mockPluginsService.setup.mockReturnValue(Promise.resolve(mockPluginsServiceSetup)); - - const server = new Server(configService as any, logger, env); + const server = new Server(config$, env, logger); + await server.preSetup(); expect(httpService.setup).not.toHaveBeenCalled(); expect(elasticsearchService.setup).not.toHaveBeenCalled(); expect(mockPluginsService.setup).not.toHaveBeenCalled(); expect(mockLegacyService.setup).not.toHaveBeenCalled(); - await server.setup(newPlatformPluginDefinitions); + await server.setup(); expect(httpService.setup).toHaveBeenCalledTimes(1); expect(elasticsearchService.setup).toHaveBeenCalledTimes(1); @@ -79,15 +62,13 @@ test('sets up services on "setup"', async () => { }); test('runs services on "start"', async () => { - const mockPluginsServiceSetup = new Map([['some-plugin', 'some-value']]); - mockPluginsService.setup.mockReturnValue(Promise.resolve(mockPluginsServiceSetup)); - - const server = new Server(configService as any, logger, env); + const server = new Server(config$, env, logger); + await server.preSetup(); expect(httpService.setup).not.toHaveBeenCalled(); expect(mockLegacyService.start).not.toHaveBeenCalled(); - await server.setup(newPlatformPluginDefinitions); + await server.setup(); expect(httpService.start).not.toHaveBeenCalled(); expect(mockLegacyService.start).not.toHaveBeenCalled(); @@ -100,15 +81,17 @@ test('runs services on "start"', async () => { test('does not fail on "setup" if there are unused paths detected', async () => { configService.getUnusedPaths.mockResolvedValue(['some.path', 'another.path']); - const server = new Server(configService as any, logger, env); - await expect(server.setup(newPlatformPluginDefinitions)).resolves.toBeDefined(); - expect(loggingServiceMock.collect(logger)).toMatchSnapshot('unused paths logs'); + const server = new Server(config$, env, logger); + await server.preSetup(); + + await expect(server.setup()).resolves.toBeDefined(); }); test('stops services on "stop"', async () => { - const server = new Server(configService as any, logger, env); + const server = new Server(config$, env, logger); + await server.preSetup(); - await server.setup(newPlatformPluginDefinitions); + await server.setup(); expect(httpService.stop).not.toHaveBeenCalled(); expect(elasticsearchService.stop).not.toHaveBeenCalled(); @@ -122,3 +105,15 @@ test('stops services on "stop"', async () => { expect(mockPluginsService.stop).toHaveBeenCalledTimes(1); expect(mockLegacyService.stop).toHaveBeenCalledTimes(1); }); + +test(`doesn't setup core services if config validation fails`, async () => { + configService.preSetup.mockImplementation(() => { + throw new Error('invalid config'); + }); + const server = new Server(config$, env, logger); + await expect(server.preSetup()).rejects.toThrowErrorMatchingInlineSnapshot(`"invalid config"`); + expect(httpService.setup).not.toHaveBeenCalled(); + expect(elasticsearchService.setup).not.toHaveBeenCalled(); + expect(mockPluginsService.setup).not.toHaveBeenCalled(); + expect(mockLegacyService.setup).not.toHaveBeenCalled(); +}); diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 5480dc4f4e62fa..fc34068e01128f 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -16,31 +16,62 @@ * specific language governing permissions and limitations * under the License. */ +import { Observable } from 'rxjs'; +import { first } from 'rxjs/operators'; +import { Type } from '@kbn/config-schema'; -import { ConfigService, Env } from './config'; +import { ConfigService, Env, Config, ConfigPath } from './config'; import { ElasticsearchService } from './elasticsearch'; import { HttpService, HttpServiceSetup, Router } from './http'; import { LegacyService } from './legacy'; import { Logger, LoggerFactory } from './logging'; -import { PluginsService, DiscoveredPluginsDefinitions } from './plugins'; +import { + PluginsService, + DiscoveredPluginsDefinitions, + configDefinition as pluginsConfigDefinition, +} from './plugins'; + +import { configDefinition as elasticsearchConfigDefinition } from './elasticsearch'; +import { configDefinition as httpConfigDefinition } from './http'; +import { configDefinition as loggingConfigDefinition } from './logging'; +import { configDefinition as devConfigDefinition } from './dev'; export class Server { + public readonly configService: ConfigService; private readonly elasticsearch: ElasticsearchService; private readonly http: HttpService; private readonly plugins: PluginsService; private readonly legacy: LegacyService; private readonly log: Logger; - constructor(configService: ConfigService, logger: LoggerFactory, env: Env) { - const core = { env, configService, logger }; - this.log = logger.get('server'); + constructor( + private readonly config$: Observable, + private readonly env: Env, + private readonly logger: LoggerFactory + ) { + this.log = this.logger.get('server'); + this.configService = new ConfigService(config$, env, logger); + + const core = { configService: this.configService, env, logger }; this.http = new HttpService(core); this.plugins = new PluginsService(core); this.legacy = new LegacyService(core); this.elasticsearch = new ElasticsearchService(core); } - public async setup(newPlatformPluginDefinitions: DiscoveredPluginsDefinitions) { + public async preSetup() { + this.log.debug('pre-setup server'); + const config = await this.config$.pipe(first()).toPromise(); + const hasDevPaths = Boolean(config.get('plugins') && config.get('plugins').paths); + const devPluginPaths = this.env.mode.dev && hasDevPaths ? config.get('plugins').paths : []; + + const pluginDefinitions = await this.plugins.preSetup(devPluginPaths); + const schemas = this.getSchemas(pluginDefinitions); + + await this.configService.preSetup(schemas); + } + + public async setup() { this.log.debug('setting up server'); const httpSetup = await this.http.setup(); @@ -48,13 +79,10 @@ export class Server { const elasticsearchServiceSetup = await this.elasticsearch.setup(); - const pluginsSetup = await this.plugins.setup( - { - elasticsearch: elasticsearchServiceSetup, - http: httpSetup, - }, - newPlatformPluginDefinitions - ); + const pluginsSetup = await this.plugins.setup({ + elasticsearch: elasticsearchServiceSetup, + http: httpSetup, + }); const coreSetup = { elasticsearch: elasticsearchServiceSetup, @@ -90,6 +118,27 @@ export class Server { await this.http.stop(); } + private getSchemas(pluginDefinitions: DiscoveredPluginsDefinitions) { + const pluginConfigSchemas = new Map( + pluginDefinitions.pluginDefinitions + .filter(pluginDef => Boolean(pluginDef.schema)) + .map( + pluginDef => + [pluginDef.manifest.configPath, pluginDef.schema!] as [ConfigPath, Type] + ) + ); + + const coreConfigSchemas = new Map>([ + [elasticsearchConfigDefinition.configPath, elasticsearchConfigDefinition.schema], + [loggingConfigDefinition.configPath, loggingConfigDefinition.schema], + [httpConfigDefinition.configPath, httpConfigDefinition.schema], + [pluginsConfigDefinition.configPath, pluginsConfigDefinition.schema], + [devConfigDefinition.configPath, devConfigDefinition.schema], + ]); + + return new Map([...pluginConfigSchemas, ...coreConfigSchemas]); + } + private registerDefaultRoute(httpSetup: HttpServiceSetup) { const router = new Router('/core'); router.get({ path: '/', validate: false }, async (req, res) => res.ok({ version: '0.0.1' }));