diff --git a/src/core/server/config/config_service.mock.ts b/src/core/server/config/config_service.mock.ts index d43130b8396213..b9c4fa91ae7028 100644 --- a/src/core/server/config/config_service.mock.ts +++ b/src/core/server/config/config_service.mock.ts @@ -22,17 +22,16 @@ import { ObjectToConfigAdapter } from './object_to_config_adapter'; import { ConfigService } from './config_service'; -type ConfigSericeContract = PublicMethodsOf; +type ConfigServiceContract = PublicMethodsOf; const createConfigServiceMock = () => { - const mocked: jest.Mocked = { + const mocked: jest.Mocked = { atPath: jest.fn(), getConfig$: jest.fn(), optionalAtPath: jest.fn(), getUsedPaths: jest.fn(), getUnusedPaths: jest.fn(), isEnabledAtPath: jest.fn(), - validateAll: jest.fn(), - setSchemaFor: jest.fn(), + setSchema: 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 aa307c3b09dc5c..9bcbde7fbcfd8e 100644 --- a/src/core/server/config/config_service.test.ts +++ b/src/core/server/config/config_service.test.ts @@ -40,11 +40,10 @@ class ExampleClassWithStringSchema { constructor(readonly value: string) {} } -const stringSchemaFor = (key: string): Array<[ConfigPath, Type]> => [[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.setSchema('key', schema.string()); const configs = configService.atPath('key', ExampleClassWithStringSchema); const exampleConfig = await configs.pipe(first()).toPromise(); @@ -57,7 +56,8 @@ 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.setSchema('key', schema.string()); const configs = configService.atPath('key', ExampleClassWithStringSchema); @@ -70,12 +70,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.setSchema('unique-name', schema.string()); const configs = configService.optionalAtPath('unique-name', ExampleClassWithStringSchema); const exampleConfig = await configs.pipe(first()).toPromise(); @@ -85,7 +81,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.setSchema('value', schema.string()); const configs = configService.optionalAtPath('value', ExampleClassWithStringSchema); const exampleConfig: any = await configs.pipe(first()).toPromise(); @@ -96,7 +93,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.setSchema('key', schema.string()); const valuesReceived: any[] = []; configService.atPath('key', ExampleClassWithStringSchema).subscribe(config => { @@ -110,7 +108,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.setSchema('key', schema.string()); const valuesReceived: any[] = []; configService.atPath('key', ExampleClassWithStringSchema).subscribe(config => { @@ -126,7 +125,8 @@ 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, stringSchemaFor('no-key')); + const configService = new ConfigService(config$, defaultEnv, logger); + configService.setSchema('no-key', schema.string()); const configs = configService.atPath('key', ExampleClass as any); @@ -135,6 +135,16 @@ test("throws error if 'schema' is not defined for a key", async () => { ); }); +test("throws error if 'setSchema' called several times for the same key", async () => { + const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'value' })); + const configService = new ConfigService(config$, defaultEnv, logger); + const addSchema = async () => await configService.setSchema('key', schema.string()); + await addSchema(); + expect(addSchema()).rejects.toMatchInlineSnapshot( + `[Error: Validation schema for key was already registered.]` + ); +}); + test('tracks unhandled paths', async () => { const initialConfig = { bar: { @@ -204,8 +214,8 @@ test('correctly passes context', async () => { defaultValue: schema.contextRef('version'), }), }); - const configService = new ConfigService(config$, env, logger, [['foo', schemaDefinition]]); - + const configService = new ConfigService(config$, env, logger); + configService.setSchema('foo', schemaDefinition); const configs = configService.atPath('foo', createClassWithSchema(schemaDefinition)); expect(await configs.pipe(first()).toPromise()).toMatchSnapshot(); diff --git a/src/core/server/config/config_service.ts b/src/core/server/config/config_service.ts index ef2534109b9d5f..fbe4f7ac70472f 100644 --- a/src/core/server/config/config_service.ts +++ b/src/core/server/config/config_service.ts @@ -34,41 +34,32 @@ 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 readonly schemas = new Map>(); constructor( private readonly config$: Observable, private readonly env: Env, - logger: LoggerFactory, - coreServiceSchemas: Array<[ConfigPath, Type]> = [] + logger: LoggerFactory ) { this.log = logger.get('config'); - for (const [path, schema] of coreServiceSchemas) { - this.setSchema(path, schema); - } } /** * Set config schema for a path and performs its validation */ - public async setSchemaFor(path: ConfigPath, schema: Type) { - this.setSchema(path, schema); + public async setSchema(path: ConfigPath, schema: Type) { + const namespace = pathToString(path); + if (this.schemas.has(namespace)) { + throw new Error(`Validation schema for ${path} was already registered.`); + } + + this.schemas.set(namespace, schema); + await this.validateConfig(path) .pipe(first()) .toPromise(); } - /** - * Performs validation for all known validation schemas - */ - public async validateAll() { - for (const namespace of this.schemas.keys()) { - await this.validateConfig(namespace) - .pipe(first()) - .toPromise(); - } - } - /** * Returns the full config object observable. This is not intended for * "normal use", but for features that _need_ access to the full object. @@ -85,7 +76,7 @@ export class ConfigService { * @param ConfigClass - A class (not an instance of a class) that contains a * static `schema` that we validate the config at the given `path` against. */ - public atPath, TConfig>( + public atPath, TConfig>( path: ConfigPath, ConfigClass: ConfigWithSchema ) { @@ -165,8 +156,8 @@ export class ConfigService { ); } - private createConfig, TConfig>( - validatedConfig: Record, + private createConfig, TConfig>( + validatedConfig: unknown, ConfigClass: ConfigWithSchema ) { return new ConfigClass(validatedConfig, this.env); @@ -189,15 +180,6 @@ export class ConfigService { this.log.debug(`Marking config path as handled: ${path}`); this.handledPaths.push(path); } - - /** - * Defines a validation schema for an appropriate path in config object. - * @internal - */ - private setSchema(path: ConfigPath, schema: Type) { - const namespace = pathToString(path); - this.schemas.set(namespace, schema); - } } const createPluginEnabledPath = (configPath: string | string[]) => { diff --git a/src/core/server/http/http_service.test.ts b/src/core/server/http/http_service.test.ts index 91f6f802c1c5b4..6f1dae9249c017 100644 --- a/src/core/server/http/http_service.test.ts +++ b/src/core/server/http/http_service.test.ts @@ -38,10 +38,9 @@ const createConfigService = (value: Partial = {}) => { }) ), env, - logger, - [[configDefinition.configPath, configDefinition.schema]] + logger ); - + configService.setSchema(configDefinition.configPath, configDefinition.schema); return configService; }; diff --git a/src/core/server/plugins/discovery/plugins_discovery.test.ts b/src/core/server/plugins/discovery/plugins_discovery.test.ts index c54455fcdf1853..bd718cb382e0d4 100644 --- a/src/core/server/plugins/discovery/plugins_discovery.test.ts +++ b/src/core/server/plugins/discovery/plugins_discovery.test.ts @@ -144,9 +144,9 @@ test('properly iterates through plugin search locations', async () => { new ObjectToConfigAdapter({ plugins: { paths: [TEST_EXTRA_PLUGIN_PATH] } }) ), env, - logger, - [[configDefinition.configPath, configDefinition.schema]] + logger ); + configService.setSchema(configDefinition.configPath, configDefinition.schema); const pluginsConfig = await configService .atPath('plugins', PluginsConfig) diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts index 0f798f1959cf42..da21fcad5d2708 100644 --- a/src/core/server/plugins/plugins_service.test.ts +++ b/src/core/server/plugins/plugins_service.test.ts @@ -61,9 +61,9 @@ beforeEach(() => { configService = new ConfigService( new BehaviorSubject(new ObjectToConfigAdapter({ plugins: { initialize: true } })), env, - logger, - [[configDefinition.configPath, configDefinition.schema]] + logger ); + configService.setSchema(configDefinition.configPath, configDefinition.schema); pluginsService = new PluginsService({ env, logger, configService }); [mockPluginSystem] = MockPluginsSystem.mock.instances as any; diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts index 510c3f1d57e727..3cb71221087348 100644 --- a/src/core/server/plugins/plugins_service.ts +++ b/src/core/server/plugins/plugins_service.ts @@ -132,7 +132,7 @@ export class PluginsService implements CoreService { const isEnabled = await this.coreContext.configService.isEnabledAtPath(plugin.configPath); if (plugin.schema) { - await this.coreContext.configService.setSchemaFor(plugin.configPath, plugin.schema); + await this.coreContext.configService.setSchema(plugin.configPath, plugin.schema); } if (pluginEnableStatuses.has(plugin.name)) { diff --git a/src/core/server/root/index.test.mocks.ts b/src/core/server/root/index.test.mocks.ts index d5f0fdcef65e82..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(), configService }; +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.ts b/src/core/server/root/index.ts index 4add4d6916baaa..a9c8b3143cf276 100644 --- a/src/core/server/root/index.ts +++ b/src/core/server/root/index.ts @@ -47,8 +47,9 @@ export class Root { public async setup() { try { - this.log.debug('setting up root'); + await this.server.preSetup(); await this.setupLogging(); + this.log.debug('setting up root'); return await this.server.setup(); } catch (e) { await this.shutdown(e); diff --git a/src/core/server/server.test.ts b/src/core/server/server.test.ts index ace1ed43099bc2..8847b9d09e17da 100644 --- a/src/core/server/server.test.ts +++ b/src/core/server/server.test.ts @@ -103,11 +103,11 @@ test('stops services on "stop"', async () => { }); test(`doesn't setup core services if config validation fails`, async () => { - configService.validateAll.mockImplementation(() => { + configService.setSchema.mockImplementation(() => { throw new Error('invalid config'); }); const server = new Server(config$, env, logger); - await expect(server.setup()).rejects.toThrowErrorMatchingInlineSnapshot(`"invalid config"`); + await expect(server.preSetup()).rejects.toThrowErrorMatchingInlineSnapshot(`"invalid config"`); expect(httpService.setup).not.toHaveBeenCalled(); expect(elasticsearchService.setup).not.toHaveBeenCalled(); expect(mockPluginsService.setup).not.toHaveBeenCalled(); diff --git a/src/core/server/server.ts b/src/core/server/server.ts index f1204f277a9634..cc62a9ea5675ed 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -31,14 +31,6 @@ import { configDefinition as httpConfigDefinition } from './http'; import { configDefinition as loggingConfigDefinition } from './logging'; import { configDefinition as devConfigDefinition } from './dev'; -const schemas: Array<[ConfigPath, Type]> = [ - [elasticsearchConfigDefinition.configPath, elasticsearchConfigDefinition.schema], - [loggingConfigDefinition.configPath, loggingConfigDefinition.schema], - [httpConfigDefinition.configPath, httpConfigDefinition.schema], - [pluginsConfigDefinition.configPath, pluginsConfigDefinition.schema], - [devConfigDefinition.configPath, devConfigDefinition.schema], -]; - export class Server { public readonly configService: ConfigService; private readonly elasticsearch: ElasticsearchService; @@ -53,7 +45,7 @@ export class Server { private readonly logger: LoggerFactory ) { this.log = this.logger.get('server'); - this.configService = new ConfigService(config$, env, logger, schemas); + this.configService = new ConfigService(config$, env, logger); const core = { configService: this.configService, env, logger }; this.http = new HttpService(core); @@ -62,8 +54,11 @@ export class Server { this.elasticsearch = new ElasticsearchService(core); } + public async preSetup() { + await this.setupConfigSchemas(); + } + public async setup() { - await this.configService.validateAll(); this.log.debug('setting up server'); const httpSetup = await this.http.setup(); @@ -115,4 +110,18 @@ export class Server { router.get({ path: '/', validate: false }, async (req, res) => res.ok({ version: '0.0.1' })); httpSetup.registerRouter(router); } + + private async setupConfigSchemas() { + const schemas: Array<[ConfigPath, Type]> = [ + [elasticsearchConfigDefinition.configPath, elasticsearchConfigDefinition.schema], + [loggingConfigDefinition.configPath, loggingConfigDefinition.schema], + [httpConfigDefinition.configPath, httpConfigDefinition.schema], + [pluginsConfigDefinition.configPath, pluginsConfigDefinition.schema], + [devConfigDefinition.configPath, devConfigDefinition.schema], + ]; + + for (const [path, schema] of schemas) { + await this.configService.setSchema(path, schema); + } + } }