From 581d499143e9ec109803ea9e8b06a51332a24fae Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Fri, 10 May 2019 16:08:18 +0200 Subject: [PATCH] [New Platform] Validate config upfront (#35453) (#36439) * Introduce new convention for config definition. We need to define a way to acquire configuration schema as a part of plugin definition. Having schema we can split steps of config validation and plugin instantiation. * Discover plugins, read their schema and validate the config. Config validation finished before core services and plugins read from it. That allows us to fail fast and have predictable validation results. * Instantiate plugins using DiscoveredPluginsDefinitions. * Update tests for new API. * test server is not created if config validation fails * move plugin discovery to plugin service pre-setup stage. Set validation schemes in ConfigService.preSetup stage. * fix eslint problem * generate docs * address Rudolfs comments * separate core services and plugins validation * rename files for consistency * address comments for root.js * address comments #1 * useSchema everywhere for consistency. get rid of validateAll * plugin system runs plugin config validation * rename configDefinition * 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. * cleanup * update docs * address comments --- ...a-plugin-server.configservice.setschema.md | 25 ++++ .../server/__snapshots__/server.test.ts.snap | 17 --- .../__snapshots__/config_service.test.ts.snap | 4 - src/core/server/config/config_service.mock.ts | 5 +- src/core/server/config/config_service.test.ts | 118 +++++++++++------- src/core/server/config/config_service.ts | 64 ++++++---- src/core/server/dev/dev_config.ts | 10 +- src/core/server/dev/index.ts | 2 +- .../elasticsearch/elasticsearch_config.ts | 58 +++++---- .../elasticsearch_service.test.ts | 27 ++-- src/core/server/elasticsearch/index.ts | 1 + src/core/server/http/http_config.ts | 23 ++-- src/core/server/http/http_service.test.ts | 17 +-- src/core/server/http/index.ts | 2 +- src/core/server/index.test.mocks.ts | 9 +- src/core/server/logging/index.ts | 2 +- src/core/server/logging/logging_config.ts | 4 + ...cks.ts => plugins_discovery.test.mocks.ts} | 0 ...very.test.ts => plugins_discovery.test.ts} | 9 +- src/core/server/plugins/index.ts | 2 +- src/core/server/plugins/plugin.test.ts | 84 +++++++++++-- src/core/server/plugins/plugin.ts | 24 ++++ src/core/server/plugins/plugins_config.ts | 12 +- .../server/plugins/plugins_service.mock.ts | 34 +++++ .../server/plugins/plugins_service.test.ts | 51 +++++++- src/core/server/plugins/plugins_service.ts | 4 + .../server/plugins/plugins_system.test.ts | 15 +-- src/core/server/root/index.test.mocks.ts | 9 +- src/core/server/root/index.ts | 21 ++-- src/core/server/server.api.md | 7 +- src/core/server/server.test.ts | 54 ++++---- src/core/server/server.ts | 39 +++++- src/plugins/testbed/server/index.ts | 4 + 33 files changed, 524 insertions(+), 233 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-server.configservice.setschema.md delete mode 100644 src/core/server/__snapshots__/server.test.ts.snap rename src/core/server/plugins/discovery/{plugin_discovery.test.mocks.ts => plugins_discovery.test.mocks.ts} (100%) rename src/core/server/plugins/discovery/{plugin_discovery.test.ts => plugins_discovery.test.ts} (96%) create mode 100644 src/core/server/plugins/plugins_service.mock.ts diff --git a/docs/development/core/server/kibana-plugin-server.configservice.setschema.md b/docs/development/core/server/kibana-plugin-server.configservice.setschema.md new file mode 100644 index 00000000000000..3db6b071dbdad6 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.configservice.setschema.md @@ -0,0 +1,25 @@ + + +[Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [ConfigService](./kibana-plugin-server.configservice.md) > [setSchema](./kibana-plugin-server.configservice.setschema.md) + +## ConfigService.setSchema() method + +Set config schema for a path and performs its validation + +Signature: + +```typescript +setSchema(path: ConfigPath, schema: Type): Promise; +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| path | ConfigPath | | +| schema | Type<unknown> | | + +Returns: + +`Promise` + 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 f24b6d1168fd9d..9327b80dc79a0b 100644 --- a/src/core/server/config/__snapshots__/config_service.test.ts.snap +++ b/src/core/server/config/__snapshots__/config_service.test.ts.snap @@ -12,7 +12,3 @@ ExampleClassWithSchema { }, } `; - -exports[`throws error if config class does not implement 'schema' 1`] = `[Error: The config class [ExampleClass] did not contain a static 'schema' field, which is required when creating a config instance]`; - -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 92b0f117b1a02d..b9c4fa91ae7028 100644 --- a/src/core/server/config/config_service.mock.ts +++ b/src/core/server/config/config_service.mock.ts @@ -22,15 +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(), + 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 fd03554e9209d5..27c9473c163245 100644 --- a/src/core/server/config/config_service.test.ts +++ b/src/core/server/config/config_service.test.ts @@ -34,9 +34,16 @@ const emptyArgv = getEnvOptions(); const defaultEnv = new Env('/kibana', emptyArgv); const logger = loggingServiceMock.create(); +class ExampleClassWithStringSchema { + public static schema = schema.string(); + + constructor(readonly value: string) {} +} + test('returns config at path as observable', async () => { const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'foo' })); const configService = new ConfigService(config$, defaultEnv, logger); + await configService.setSchema('key', schema.string()); const configs = configService.atPath('key', ExampleClassWithStringSchema); const exampleConfig = await configs.pipe(first()).toPromise(); @@ -45,22 +52,45 @@ test('returns config at path as observable', async () => { }); test('throws if config at path does not match schema', async () => { - expect.assertions(1); - const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 123 })); const configService = new ConfigService(config$, defaultEnv, logger); - const configs = configService.atPath('key', ExampleClassWithStringSchema); - try { - await configs.pipe(first()).toPromise(); - } catch (e) { - expect(e.message).toMatchSnapshot(); - } + await expect( + configService.setSchema('key', schema.string()) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[key]: expected value of type [string] but got [number]"` + ); +}); + +test('re-validate config when updated', async () => { + const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'value' })); + + const configService = new ConfigService(config$, defaultEnv, logger); + configService.setSchema('key', schema.string()); + + const valuesReceived: any[] = []; + await configService.atPath('key', ExampleClassWithStringSchema).subscribe( + config => { + valuesReceived.push(config.value); + }, + error => { + valuesReceived.push(error); + } + ); + + config$.next(new ObjectToConfigAdapter({ key: 123 })); + + await expect(valuesReceived).toMatchInlineSnapshot(` +Array [ + "value", + [Error: [key]: expected value of type [string] but got [number]], +] +`); }); test("returns undefined if fetching optional config at a path that doesn't exist", async () => { - const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ foo: 'bar' })); + const config$ = new BehaviorSubject(new ObjectToConfigAdapter({})); const configService = new ConfigService(config$, defaultEnv, logger); const configs = configService.optionalAtPath('unique-name', ExampleClassWithStringSchema); @@ -72,6 +102,7 @@ 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); + await configService.setSchema('value', schema.string()); const configs = configService.optionalAtPath('value', ExampleClassWithStringSchema); const exampleConfig: any = await configs.pipe(first()).toPromise(); @@ -83,6 +114,7 @@ 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); + await configService.setSchema('key', schema.string()); const valuesReceived: any[] = []; configService.atPath('key', ExampleClassWithStringSchema).subscribe(config => { @@ -97,6 +129,7 @@ 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); + await configService.setSchema('key', schema.string()); const valuesReceived: any[] = []; configService.atPath('key', ExampleClassWithStringSchema).subscribe(config => { @@ -108,9 +141,7 @@ test('pushes new config when reloading and config at path has changed', async () expect(valuesReceived).toEqual(['value', 'new value']); }); -test("throws error if config class does not implement 'schema'", async () => { - expect.assertions(1); - +test("throws error if 'schema' is not defined for a key", async () => { class ExampleClass {} const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'value' })); @@ -118,11 +149,19 @@ test("throws error if config class does not implement 'schema'", async () => { const configs = configService.atPath('key', ExampleClass as any); - try { - await configs.pipe(first()).toPromise(); - } catch (e) { - expect(e).toMatchSnapshot(); - } + await expect(configs.pipe(first()).toPromise()).rejects.toMatchInlineSnapshot( + `[Error: No validation schema has been defined for key]` + ); +}); + +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(); + await expect(addSchema()).rejects.toMatchInlineSnapshot( + `[Error: Validation schema for key was already registered.]` + ); }); test('tracks unhandled paths', async () => { @@ -178,28 +217,25 @@ test('correctly passes context', async () => { const env = new Env('/kibana', getEnvOptions()); const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ foo: {} })); + const schemaDefinition = schema.object({ + branchRef: schema.string({ + defaultValue: schema.contextRef('branch'), + }), + buildNumRef: schema.number({ + defaultValue: schema.contextRef('buildNum'), + }), + buildShaRef: schema.string({ + defaultValue: schema.contextRef('buildSha'), + }), + devRef: schema.boolean({ defaultValue: schema.contextRef('dev') }), + prodRef: schema.boolean({ defaultValue: schema.contextRef('prod') }), + versionRef: schema.string({ + defaultValue: schema.contextRef('version'), + }), + }); const configService = new ConfigService(config$, env, logger); - const configs = configService.atPath( - 'foo', - createClassWithSchema( - schema.object({ - branchRef: schema.string({ - defaultValue: schema.contextRef('branch'), - }), - buildNumRef: schema.number({ - defaultValue: schema.contextRef('buildNum'), - }), - buildShaRef: schema.string({ - defaultValue: schema.contextRef('buildSha'), - }), - devRef: schema.boolean({ defaultValue: schema.contextRef('dev') }), - prodRef: schema.boolean({ defaultValue: schema.contextRef('prod') }), - versionRef: schema.string({ - defaultValue: schema.contextRef('version'), - }), - }) - ) - ); + await configService.setSchema('foo', schemaDefinition); + const configs = configService.atPath('foo', createClassWithSchema(schemaDefinition)); expect(await configs.pipe(first()).toPromise()).toMatchSnapshot(); }); @@ -278,9 +314,3 @@ function createClassWithSchema(s: Type) { constructor(readonly value: TypeOf) {} }; } - -class ExampleClassWithStringSchema { - public static schema = schema.string(); - - constructor(readonly value: string) {} -} diff --git a/src/core/server/config/config_service.ts b/src/core/server/config/config_service.ts index 346f3d4a2274d0..82b688e367884f 100644 --- a/src/core/server/config/config_service.ts +++ b/src/core/server/config/config_service.ts @@ -34,6 +34,7 @@ export class ConfigService { * then list all unhandled config paths when the startup process is completed. */ private readonly handledPaths: ConfigPath[] = []; + private readonly schemas = new Map>(); constructor( private readonly config$: Observable, @@ -43,6 +44,22 @@ export class ConfigService { this.log = logger.get('config'); } + /** + * Set config schema for a path and performs its validation + */ + 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(); + } + /** * Returns the full config object observable. This is not intended for * "normal use", but for features that _need_ access to the full object. @@ -59,13 +76,11 @@ 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 ) { - return this.getDistinctConfig(path).pipe( - map(config => this.createConfig(path, config, ConfigClass)) - ); + return this.validateConfig(path).pipe(map(config => this.createConfig(config, ConfigClass))); } /** @@ -79,9 +94,11 @@ export class ConfigService { ConfigClass: ConfigWithSchema ) { return this.getDistinctConfig(path).pipe( - map(config => - config === undefined ? undefined : this.createConfig(path, config, ConfigClass) - ) + map(config => { + if (config === undefined) return undefined; + const validatedConfig = this.validate(path, config); + return this.createConfig(validatedConfig, ConfigClass); + }) ); } @@ -122,24 +139,13 @@ export class ConfigService { return config.getFlattenedPaths().filter(path => isPathHandled(path, handledPaths)); } - private createConfig, TConfig>( - path: ConfigPath, - config: Record, - ConfigClass: ConfigWithSchema - ) { - const namespace = Array.isArray(path) ? path.join('.') : path; - - const configSchema = ConfigClass.schema; - - if (configSchema === undefined || typeof configSchema.validate !== 'function') { - throw new Error( - `The config class [${ - ConfigClass.name - }] did not contain a static 'schema' field, which is required when creating a config instance` - ); + private validate(path: ConfigPath, config: Record) { + const namespace = pathToString(path); + const schema = this.schemas.get(namespace); + if (!schema) { + throw new Error(`No validation schema has been defined for ${namespace}`); } - - const validatedConfig = ConfigClass.schema.validate( + return schema.validate( config, { dev: this.env.mode.dev, @@ -148,9 +154,19 @@ export class ConfigService { }, namespace ); + } + + private createConfig, TConfig>( + validatedConfig: unknown, + ConfigClass: ConfigWithSchema + ) { return new ConfigClass(validatedConfig, this.env); } + private validateConfig(path: ConfigPath) { + return this.getDistinctConfig(path).pipe(map(config => this.validate(path, config))); + } + private getDistinctConfig(path: ConfigPath) { this.markAsHandled(path); diff --git a/src/core/server/dev/dev_config.ts b/src/core/server/dev/dev_config.ts index 174b1088837920..6c99025da3fc0e 100644 --- a/src/core/server/dev/dev_config.ts +++ b/src/core/server/dev/dev_config.ts @@ -25,8 +25,12 @@ const createDevSchema = schema.object({ }), }); -type DevConfigType = TypeOf; +export const config = { + path: 'dev', + schema: createDevSchema, +}; +export type DevConfigType = TypeOf; export class DevConfig { /** * @internal @@ -38,7 +42,7 @@ export class DevConfig { /** * @internal */ - constructor(config: DevConfigType) { - this.basePathProxyTargetPort = config.basePathProxyTarget; + constructor(rawConfig: DevConfigType) { + this.basePathProxyTargetPort = rawConfig.basePathProxyTarget; } } diff --git a/src/core/server/dev/index.ts b/src/core/server/dev/index.ts index b3fa85892330e1..cccd6b1c5d07fd 100644 --- a/src/core/server/dev/index.ts +++ b/src/core/server/dev/index.ts @@ -17,4 +17,4 @@ * under the License. */ -export { DevConfig } from './dev_config'; +export { DevConfig, config } from './dev_config'; diff --git a/src/core/server/elasticsearch/elasticsearch_config.ts b/src/core/server/elasticsearch/elasticsearch_config.ts index deac4341f365b4..e3f2d46831bcf5 100644 --- a/src/core/server/elasticsearch/elasticsearch_config.ts +++ b/src/core/server/elasticsearch/elasticsearch_config.ts @@ -60,7 +60,13 @@ const configSchema = schema.object({ healthCheck: schema.object({ delay: schema.duration({ defaultValue: 2500 }) }), }); -type SslConfigSchema = TypeOf['ssl']; +export type ElasticsearchConfigType = TypeOf; +type SslConfigSchema = ElasticsearchConfigType['ssl']; + +export const config = { + path: 'elasticsearch', + schema: configSchema, +}; export class ElasticsearchConfig { public static schema = configSchema; @@ -154,34 +160,34 @@ export class ElasticsearchConfig { * headers cannot be overwritten by client-side headers and aren't affected by * `requestHeadersWhitelist` configuration. */ - public readonly customHeaders: TypeOf['customHeaders']; - - constructor(config: TypeOf) { - this.apiVersion = config.apiVersion; - this.logQueries = config.logQueries; - this.hosts = Array.isArray(config.hosts) ? config.hosts : [config.hosts]; - this.requestHeadersWhitelist = Array.isArray(config.requestHeadersWhitelist) - ? config.requestHeadersWhitelist - : [config.requestHeadersWhitelist]; - this.pingTimeout = config.pingTimeout; - this.requestTimeout = config.requestTimeout; - this.shardTimeout = config.shardTimeout; - this.sniffOnStart = config.sniffOnStart; - this.sniffOnConnectionFault = config.sniffOnConnectionFault; - this.sniffInterval = config.sniffInterval; - this.healthCheckDelay = config.healthCheck.delay; - this.username = config.username; - this.password = config.password; - this.customHeaders = config.customHeaders; - - const certificateAuthorities = Array.isArray(config.ssl.certificateAuthorities) - ? config.ssl.certificateAuthorities - : typeof config.ssl.certificateAuthorities === 'string' - ? [config.ssl.certificateAuthorities] + public readonly customHeaders: ElasticsearchConfigType['customHeaders']; + + constructor(rawConfig: ElasticsearchConfigType) { + this.apiVersion = rawConfig.apiVersion; + this.logQueries = rawConfig.logQueries; + this.hosts = Array.isArray(rawConfig.hosts) ? rawConfig.hosts : [rawConfig.hosts]; + this.requestHeadersWhitelist = Array.isArray(rawConfig.requestHeadersWhitelist) + ? rawConfig.requestHeadersWhitelist + : [rawConfig.requestHeadersWhitelist]; + this.pingTimeout = rawConfig.pingTimeout; + this.requestTimeout = rawConfig.requestTimeout; + this.shardTimeout = rawConfig.shardTimeout; + this.sniffOnStart = rawConfig.sniffOnStart; + this.sniffOnConnectionFault = rawConfig.sniffOnConnectionFault; + this.sniffInterval = rawConfig.sniffInterval; + this.healthCheckDelay = rawConfig.healthCheck.delay; + this.username = rawConfig.username; + this.password = rawConfig.password; + this.customHeaders = rawConfig.customHeaders; + + const certificateAuthorities = Array.isArray(rawConfig.ssl.certificateAuthorities) + ? rawConfig.ssl.certificateAuthorities + : typeof rawConfig.ssl.certificateAuthorities === 'string' + ? [rawConfig.ssl.certificateAuthorities] : undefined; this.ssl = { - ...config.ssl, + ...rawConfig.ssl, certificateAuthorities, }; } diff --git a/src/core/server/elasticsearch/elasticsearch_service.test.ts b/src/core/server/elasticsearch/elasticsearch_service.test.ts index 0697d9152f0a91..41e72a11a7ee1a 100644 --- a/src/core/server/elasticsearch/elasticsearch_service.test.ts +++ b/src/core/server/elasticsearch/elasticsearch_service.test.ts @@ -22,32 +22,33 @@ import { first } from 'rxjs/operators'; import { MockClusterClient } from './elasticsearch_service.test.mocks'; import { BehaviorSubject, combineLatest } from 'rxjs'; -import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config'; +import { Env } from '../config'; import { getEnvOptions } from '../config/__mocks__/env'; import { CoreContext } from '../core_context'; +import { configServiceMock } from '../config/config_service.mock'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { ElasticsearchConfig } from './elasticsearch_config'; import { ElasticsearchService } from './elasticsearch_service'; let elasticsearchService: ElasticsearchService; -let configService: ConfigService; +const configService = configServiceMock.create(); +configService.atPath.mockReturnValue( + new BehaviorSubject( + new ElasticsearchConfig({ + hosts: ['http://1.2.3.4'], + healthCheck: {}, + ssl: {}, + } as any) + ) +); + let env: Env; let coreContext: CoreContext; const logger = loggingServiceMock.create(); beforeEach(() => { env = Env.createDefault(getEnvOptions()); - configService = new ConfigService( - new BehaviorSubject( - new ObjectToConfigAdapter({ - elasticsearch: { hosts: ['http://1.2.3.4'], username: 'jest' }, - }) - ), - env, - logger - ); - - coreContext = { env, logger, configService }; + coreContext = { env, logger, configService: configService as any }; elasticsearchService = new ElasticsearchService(coreContext); }); diff --git a/src/core/server/elasticsearch/index.ts b/src/core/server/elasticsearch/index.ts index 0f60864dcc5d82..b9925e42d736a6 100644 --- a/src/core/server/elasticsearch/index.ts +++ b/src/core/server/elasticsearch/index.ts @@ -21,3 +21,4 @@ export { ElasticsearchServiceSetup, ElasticsearchService } from './elasticsearch export { CallAPIOptions, ClusterClient } from './cluster_client'; export { ScopedClusterClient, Headers, APICaller } from './scoped_cluster_client'; export { ElasticsearchClientConfig } from './elasticsearch_client_config'; +export { config } from './elasticsearch_config'; diff --git a/src/core/server/http/http_config.ts b/src/core/server/http/http_config.ts index ebf3bd8023f1a2..1848070b2a56f9 100644 --- a/src/core/server/http/http_config.ts +++ b/src/core/server/http/http_config.ts @@ -85,6 +85,11 @@ const createHttpSchema = schema.object( export type HttpConfigType = TypeOf; +export const config = { + path: 'server', + schema: createHttpSchema, +}; + export class HttpConfig { /** * @internal @@ -104,15 +109,15 @@ export class HttpConfig { /** * @internal */ - constructor(config: HttpConfigType, env: Env) { - this.autoListen = config.autoListen; - this.host = config.host; - this.port = config.port; - this.cors = config.cors; - this.maxPayload = config.maxPayload; - this.basePath = config.basePath; - this.rewriteBasePath = config.rewriteBasePath; + constructor(rawConfig: HttpConfigType, env: Env) { + this.autoListen = rawConfig.autoListen; + this.host = rawConfig.host; + this.port = rawConfig.port; + this.cors = rawConfig.cors; + this.maxPayload = rawConfig.maxPayload; + this.basePath = rawConfig.basePath; + this.rewriteBasePath = rawConfig.rewriteBasePath; this.publicDir = env.staticFilesDir; - this.ssl = new SslConfig(config.ssl); + this.ssl = new SslConfig(rawConfig.ssl); } } diff --git a/src/core/server/http/http_service.test.ts b/src/core/server/http/http_service.test.ts index dd66a050e3302c..7b3fd024b477c3 100644 --- a/src/core/server/http/http_service.test.ts +++ b/src/core/server/http/http_service.test.ts @@ -22,7 +22,7 @@ import { mockHttpServer } from './http_service.test.mocks'; import { noop } from 'lodash'; import { BehaviorSubject } from 'rxjs'; import { HttpService, Router } from '.'; -import { HttpConfigType } from './http_config'; +import { HttpConfigType, config } from './http_config'; import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { getEnvOptions } from '../config/__mocks__/env'; @@ -30,16 +30,19 @@ 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({ - http: value, + server: value, }) ), env, logger ); + configService.setSchema(config.path, config.schema); + return configService; +}; afterEach(() => { jest.clearAllMocks(); @@ -115,7 +118,7 @@ test('stops http server', async () => { }); test('register route handler', async () => { - const configService = createConfigService({}); + const configService = createConfigService(); const registerRouterMock = jest.fn(); const httpServer = { @@ -155,7 +158,7 @@ test('returns http server contract on setup', async () => { }); test('does not start http server if process is dev cluster master', async () => { - const configService = createConfigService({}); + const configService = createConfigService(); const httpServer = { isListening: () => false, setup: noop, @@ -190,7 +193,7 @@ test('does not start http server if configured with `autoListen:false`', async ( const service = new HttpService({ configService, - env: new Env('.', getEnvOptions({ isDevClusterMaster: true })), + env: new Env('.', getEnvOptions()), logger, }); diff --git a/src/core/server/http/index.ts b/src/core/server/http/index.ts index 633fcd98852120..465c5cb6a859b0 100644 --- a/src/core/server/http/index.ts +++ b/src/core/server/http/index.ts @@ -17,7 +17,7 @@ * under the License. */ -export { HttpConfig } from './http_config'; +export { config, HttpConfig, HttpConfigType } from './http_config'; export { HttpService, HttpServiceSetup, HttpServiceStart } from './http_service'; export { Router, KibanaRequest } from './router'; export { BasePathProxyServer } from './base_path_proxy_server'; 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/logging/index.ts b/src/core/server/logging/index.ts index a99ee9ec12e8b3..aa23a11e39b44f 100644 --- a/src/core/server/logging/index.ts +++ b/src/core/server/logging/index.ts @@ -22,6 +22,6 @@ export { LoggerFactory } from './logger_factory'; export { LogRecord } from './log_record'; export { LogLevel } from './log_level'; /** @internal */ -export { LoggingConfig } from './logging_config'; +export { LoggingConfig, config } from './logging_config'; /** @internal */ export { LoggingService } from './logging_service'; diff --git a/src/core/server/logging/logging_config.ts b/src/core/server/logging/logging_config.ts index 64de8ed1de2165..de85bde3959df9 100644 --- a/src/core/server/logging/logging_config.ts +++ b/src/core/server/logging/logging_config.ts @@ -79,6 +79,10 @@ const loggingSchema = schema.object({ /** @internal */ export type LoggerConfigType = TypeOf; +export const config = { + path: 'logging', + schema: loggingSchema, +}; type LoggingConfigType = TypeOf; diff --git a/src/core/server/plugins/discovery/plugin_discovery.test.mocks.ts b/src/core/server/plugins/discovery/plugins_discovery.test.mocks.ts similarity index 100% rename from src/core/server/plugins/discovery/plugin_discovery.test.mocks.ts rename to src/core/server/plugins/discovery/plugins_discovery.test.mocks.ts diff --git a/src/core/server/plugins/discovery/plugin_discovery.test.ts b/src/core/server/plugins/discovery/plugins_discovery.test.ts similarity index 96% rename from src/core/server/plugins/discovery/plugin_discovery.test.ts rename to src/core/server/plugins/discovery/plugins_discovery.test.ts index f897fe1527aa27..16fa4a18804d3f 100644 --- a/src/core/server/plugins/discovery/plugin_discovery.test.ts +++ b/src/core/server/plugins/discovery/plugins_discovery.test.ts @@ -17,7 +17,7 @@ * under the License. */ -import { mockPackage, mockReaddir, mockReadFile, mockStat } from './plugin_discovery.test.mocks'; +import { mockPackage, mockReaddir, mockReadFile, mockStat } from './plugins_discovery.test.mocks'; import { resolve } from 'path'; import { BehaviorSubject } from 'rxjs'; @@ -26,7 +26,7 @@ import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../../config' import { getEnvOptions } from '../../config/__mocks__/env'; import { loggingServiceMock } from '../../logging/logging_service.mock'; import { PluginWrapper } from '../plugin'; -import { PluginsConfig } from '../plugins_config'; +import { PluginsConfig, config } from '../plugins_config'; import { discover } from './plugins_discovery'; const TEST_PLUGIN_SEARCH_PATHS = { @@ -58,10 +58,6 @@ beforeEach(() => { } }); - mockStat.mockImplementation((path, cb) => - cb(null, { isDirectory: () => !path.includes('non-dir') }) - ); - mockStat.mockImplementation((path, cb) => { if (path.includes('9-inaccessible-dir')) { cb(new Error(`ENOENT (disappeared between "readdir" and "stat").`)); @@ -125,6 +121,7 @@ test('properly iterates through plugin search locations', async () => { env, logger ); + await configService.setSchema(config.path, config.schema); const pluginsConfig = await configService .atPath('plugins', PluginsConfig) diff --git a/src/core/server/plugins/index.ts b/src/core/server/plugins/index.ts index 6faef56a43ce71..9a1107b979fc5a 100644 --- a/src/core/server/plugins/index.ts +++ b/src/core/server/plugins/index.ts @@ -18,7 +18,7 @@ */ export { PluginsService, PluginsServiceSetup, PluginsServiceStart } from './plugins_service'; - +export { config } from './plugins_config'; /** @internal */ export { isNewPlatformPlugin } from './discovery'; /** @internal */ diff --git a/src/core/server/plugins/plugin.test.ts b/src/core/server/plugins/plugin.test.ts index cb4203244e86db..0ce4ba26661985 100644 --- a/src/core/server/plugins/plugin.test.ts +++ b/src/core/server/plugins/plugin.test.ts @@ -19,9 +19,12 @@ import { join } from 'path'; import { BehaviorSubject } from 'rxjs'; -import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config'; +import { schema } from '@kbn/config-schema'; + +import { Env } from '../config'; import { getEnvOptions } from '../config/__mocks__/env'; import { CoreContext } from '../core_context'; +import { configServiceMock } from '../config/config_service.mock'; import { elasticsearchServiceMock } from '../elasticsearch/elasticsearch_service.mock'; import { httpServiceMock } from '../http/http_service.mock'; import { loggingServiceMock } from '../logging/logging_service.mock'; @@ -57,7 +60,9 @@ function createPluginManifest(manifestProps: Partial = {}): Plug }; } -let configService: ConfigService; +const configService = configServiceMock.create(); +configService.atPath.mockReturnValue(new BehaviorSubject({ initialize: true })); + let env: Env; let coreContext: CoreContext; const setupDeps = { @@ -67,13 +72,7 @@ const setupDeps = { beforeEach(() => { env = Env.createDefault(getEnvOptions()); - configService = new ConfigService( - new BehaviorSubject(new ObjectToConfigAdapter({ plugins: { initialize: true } })), - env, - logger - ); - - coreContext = { env, logger, configService }; + coreContext = { env, logger, configService: configService as any }; }); afterEach(() => { @@ -263,3 +262,70 @@ test('`stop` calls `stop` defined by the plugin instance', async () => { await expect(plugin.stop()).resolves.toBeUndefined(); expect(mockPluginInstance.stop).toHaveBeenCalledTimes(1); }); + +describe('#getConfigSchema()', () => { + it('reads config schema from plugin', () => { + const pluginSchema = schema.any(); + jest.doMock( + 'plugin-with-schema/server', + () => ({ + config: { + schema: pluginSchema, + }, + }), + { virtual: true } + ); + const manifest = createPluginManifest(); + const plugin = new PluginWrapper( + 'plugin-with-schema', + manifest, + createPluginInitializerContext(coreContext, manifest) + ); + + expect(plugin.getConfigSchema()).toBe(pluginSchema); + }); + + it('returns null if config definition not specified', () => { + jest.doMock('plugin-with-no-definition/server', () => ({}), { virtual: true }); + const manifest = createPluginManifest(); + const plugin = new PluginWrapper( + 'plugin-with-no-definition', + manifest, + createPluginInitializerContext(coreContext, manifest) + ); + expect(plugin.getConfigSchema()).toBe(null); + }); + + it('returns null for plugins without a server part', () => { + const manifest = createPluginManifest({ server: false }); + const plugin = new PluginWrapper( + 'plugin-with-no-definition', + manifest, + createPluginInitializerContext(coreContext, manifest) + ); + expect(plugin.getConfigSchema()).toBe(null); + }); + + it('throws if plugin contains invalid schema', () => { + jest.doMock( + 'plugin-invalid-schema/server', + () => ({ + config: { + schema: { + validate: () => null, + }, + }, + }), + { virtual: true } + ); + const manifest = createPluginManifest(); + const plugin = new PluginWrapper( + 'plugin-invalid-schema', + manifest, + createPluginInitializerContext(coreContext, manifest) + ); + expect(() => plugin.getConfigSchema()).toThrowErrorMatchingInlineSnapshot( + `"Configuration schema expected to be an instance of Type"` + ); + }); +}); diff --git a/src/core/server/plugins/plugin.ts b/src/core/server/plugins/plugin.ts index 72981005968f37..1d4613375d95f1 100644 --- a/src/core/server/plugins/plugin.ts +++ b/src/core/server/plugins/plugin.ts @@ -19,10 +19,15 @@ import { join } from 'path'; import typeDetect from 'type-detect'; + +import { Type } from '@kbn/config-schema'; + import { ConfigPath } from '../config'; import { Logger } from '../logging'; import { PluginInitializerContext, PluginSetupContext, PluginStartContext } from './plugin_context'; +export type PluginConfigSchema = Type | null; + /** * Dedicated type for plugin name/id that is supposed to make Map/Set/Arrays * that use it as a key or value more obvious. @@ -237,6 +242,25 @@ export class PluginWrapper< this.instance = undefined; } + public getConfigSchema(): PluginConfigSchema { + if (!this.manifest.server) { + return null; + } + const pluginPathServer = join(this.path, 'server'); + // eslint-disable-next-line @typescript-eslint/no-var-requires + const pluginDefinition = require(pluginPathServer); + + if (!('config' in pluginDefinition)) { + this.log.debug(`"${pluginPathServer}" does not export "config".`); + return null; + } + + if (!(pluginDefinition.config.schema instanceof Type)) { + throw new Error('Configuration schema expected to be an instance of Type'); + } + return pluginDefinition.config.schema; + } + private createPluginInstance() { this.log.debug('Initializing plugin'); diff --git a/src/core/server/plugins/plugins_config.ts b/src/core/server/plugins/plugins_config.ts index 9e440748dff4af..d2c258faab3088 100644 --- a/src/core/server/plugins/plugins_config.ts +++ b/src/core/server/plugins/plugins_config.ts @@ -30,7 +30,11 @@ const pluginsSchema = schema.object({ paths: schema.arrayOf(schema.string(), { defaultValue: [] }), }); -type PluginsConfigType = TypeOf; +export type PluginsConfigType = TypeOf; +export const config = { + path: 'plugins', + schema: pluginsSchema, +}; /** @internal */ export class PluginsConfig { @@ -51,10 +55,10 @@ export class PluginsConfig { */ public readonly additionalPluginPaths: ReadonlyArray; - constructor(config: PluginsConfigType, env: Env) { - this.initialize = config.initialize; + constructor(rawConfig: PluginsConfigType, env: Env) { + this.initialize = rawConfig.initialize; this.pluginSearchPaths = env.pluginSearchPaths; // Only allow custom plugin paths in dev. - this.additionalPluginPaths = env.mode.dev ? config.paths : []; + this.additionalPluginPaths = env.mode.dev ? rawConfig.paths : []; } } 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..59b6f7fbd1026e --- /dev/null +++ b/src/core/server/plugins/plugins_service.mock.ts @@ -0,0 +1,34 @@ +/* + * 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 = { + setup: jest.fn(), + start: jest.fn(), + stop: jest.fn(), + }; + return mocked; +}; + +export const pluginServiceMock = { + create: createServiceMock, +}; diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts index 774639758738c3..07210003181333 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'; @@ -31,6 +32,7 @@ import { PluginDiscoveryError } from './discovery'; import { PluginWrapper } from './plugin'; import { PluginsService } from './plugins_service'; import { PluginsSystem } from './plugins_system'; +import { config } from './plugins_config'; const MockPluginsSystem: jest.Mock = PluginsSystem as any; @@ -43,7 +45,14 @@ const setupDeps = { http: httpServiceMock.createSetupContract(), }; const logger = loggingServiceMock.create(); -beforeEach(() => { + +['path-1', 'path-2', 'path-3', 'path-4', 'path-5'].forEach(path => { + jest.doMock(join(path, 'server'), () => ({}), { + virtual: true, + }); +}); + +beforeEach(async () => { mockPackage.raw = { branch: 'feature-v1', version: 'v1', @@ -61,6 +70,7 @@ beforeEach(() => { env, logger ); + await configService.setSchema(config.path, config.schema); pluginsService = new PluginsService({ env, logger, configService }); [mockPluginSystem] = MockPluginsSystem.mock.instances as any; @@ -326,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 2ae05556663f36..9754c1b03d0301 100644 --- a/src/core/server/plugins/plugins_system.test.ts +++ b/src/core/server/plugins/plugins_system.test.ts @@ -23,9 +23,11 @@ import { } from './plugins_system.test.mocks'; import { BehaviorSubject } from 'rxjs'; -import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config'; + +import { Env } from '../config'; import { getEnvOptions } from '../config/__mocks__/env'; import { CoreContext } from '../core_context'; +import { configServiceMock } from '../config/config_service.mock'; import { elasticsearchServiceMock } from '../elasticsearch/elasticsearch_service.mock'; import { httpServiceMock } from '../http/http_service.mock'; import { loggingServiceMock } from '../logging/logging_service.mock'; @@ -58,7 +60,8 @@ function createPlugin( } let pluginsSystem: PluginsSystem; -let configService: ConfigService; +const configService = configServiceMock.create(); +configService.atPath.mockReturnValue(new BehaviorSubject({ initialize: true })); let env: Env; let coreContext: CoreContext; const setupDeps = { @@ -68,13 +71,7 @@ const setupDeps = { beforeEach(() => { env = Env.createDefault(getEnvOptions()); - configService = new ConfigService( - new BehaviorSubject(new ObjectToConfigAdapter({ plugins: { initialize: true } })), - env, - logger - ); - - coreContext = { env, logger, configService }; + coreContext = { env, logger, configService: configService as any }; pluginsSystem = new PluginsSystem(coreContext); }); diff --git a/src/core/server/root/index.test.mocks.ts b/src/core/server/root/index.test.mocks.ts index 602fa2368a766b..5754e5a5b9321d 100644 --- a/src/core/server/root/index.test.mocks.ts +++ b/src/core/server/root/index.test.mocks.ts @@ -19,7 +19,7 @@ import { loggingServiceMock } from '../logging/logging_service.mock'; export const logger = loggingServiceMock.create(); -jest.doMock('../logging', () => ({ +jest.doMock('../logging/logging_service', () => ({ LoggingService: jest.fn(() => logger), })); @@ -29,5 +29,10 @@ jest.doMock('../config/config_service', () => ({ ConfigService: jest.fn(() => configService), })); -export const mockServer = { setup: jest.fn(), stop: jest.fn() }; +export const mockServer = { + setupConfigSchemas: 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 d3125543f4189c..ff4c9da4bcc9aa 100644 --- a/src/core/server/root/index.ts +++ b/src/core/server/root/index.ts @@ -20,7 +20,7 @@ import { ConnectableObservable, Observable, Subscription } from 'rxjs'; import { first, map, publishReplay, switchMap, tap } from 'rxjs/operators'; -import { Config, ConfigService, Env } from '../config'; +import { Config, Env } from '../config'; import { Logger, LoggerFactory, LoggingConfig, LoggingService } from '../logging'; import { Server } from '../server'; @@ -29,30 +29,27 @@ import { Server } from '../server'; */ export class Root { public readonly logger: LoggerFactory; - private readonly configService: ConfigService; private readonly log: Logger; - private readonly server: Server; private readonly loggingService: LoggingService; + private readonly server: Server; private loggingConfigSubscription?: Subscription; constructor( config$: Observable, - private readonly env: Env, + 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.configService = new ConfigService(config$, env, this.logger); - this.server = new Server(this.configService, this.logger, this.env); + this.server = new Server(config$, env, this.logger); } public async setup() { - this.log.debug('setting up root'); - try { + await this.server.setupConfigSchemas(); await this.setupLogging(); + this.log.debug('setting up root'); return await this.server.setup(); } catch (e) { await this.shutdown(e); @@ -62,7 +59,6 @@ export class Root { public async start() { this.log.debug('starting root'); - try { return await this.server.start(); } catch (e) { @@ -98,10 +94,11 @@ export class Root { } private async setupLogging() { + const { configService } = this.server; // 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.api.md b/src/core/server/server.api.md index fab525ac31718b..ca791f114f65c5 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -61,9 +61,8 @@ export class ConfigService { // Warning: (ae-forgotten-export) The symbol "Config" needs to be exported by the entry point index.d.ts // Warning: (ae-forgotten-export) The symbol "Env" needs to be exported by the entry point index.d.ts constructor(config$: Observable, env: Env, logger: LoggerFactory); - // Warning: (ae-forgotten-export) The symbol "ConfigPath" needs to be exported by the entry point index.d.ts // Warning: (ae-forgotten-export) The symbol "ConfigWithSchema" needs to be exported by the entry point index.d.ts - atPath, TConfig>(path: ConfigPath, ConfigClass: ConfigWithSchema): Observable; + atPath, TConfig>(path: ConfigPath, ConfigClass: ConfigWithSchema): Observable; getConfig$(): Observable; // (undocumented) getUnusedPaths(): Promise; @@ -72,7 +71,9 @@ export class ConfigService { // (undocumented) isEnabledAtPath(path: ConfigPath): Promise; optionalAtPath, TConfig>(path: ConfigPath, ConfigClass: ConfigWithSchema): Observable; -} + // Warning: (ae-forgotten-export) The symbol "ConfigPath" needs to be exported by the entry point index.d.ts + setSchema(path: ConfigPath, schema: Type): Promise; + } // @public (undocumented) export interface CoreSetup { diff --git a/src/core/server/server.test.ts b/src/core/server/server.test.ts index 6b673b7a68089a..257b9e72fd081a 100644 --- a/src/core/server/server.test.ts +++ b/src/core/server/server.test.ts @@ -22,17 +22,16 @@ 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(); @@ -42,48 +41,35 @@ beforeEach(() => { afterEach(() => { jest.clearAllMocks(); - - configService.atPath.mockReset(); - httpService.setup.mockClear(); - httpService.start.mockClear(); - httpService.stop.mockReset(); - elasticsearchService.setup.mockReset(); - elasticsearchService.stop.mockReset(); - mockPluginsService.setup.mockReset(); - mockPluginsService.stop.mockReset(); - mockLegacyService.setup.mockReset(); - mockLegacyService.start.mockReset(); - mockLegacyService.stop.mockReset(); }); +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); expect(httpService.setup).not.toHaveBeenCalled(); expect(elasticsearchService.setup).not.toHaveBeenCalled(); expect(mockPluginsService.setup).not.toHaveBeenCalled(); - expect(mockLegacyService.start).not.toHaveBeenCalled(); + expect(mockLegacyService.setup).not.toHaveBeenCalled(); await server.setup(); expect(httpService.setup).toHaveBeenCalledTimes(1); expect(elasticsearchService.setup).toHaveBeenCalledTimes(1); expect(mockPluginsService.setup).toHaveBeenCalledTimes(1); + expect(mockLegacyService.setup).toHaveBeenCalledTimes(1); }); 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); expect(httpService.setup).not.toHaveBeenCalled(); expect(mockLegacyService.start).not.toHaveBeenCalled(); await server.setup(); + + expect(httpService.start).not.toHaveBeenCalled(); + expect(mockLegacyService.start).not.toHaveBeenCalled(); await server.start(); expect(httpService.start).toHaveBeenCalledTimes(1); @@ -93,13 +79,13 @@ 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); + const server = new Server(config$, env, logger); + await expect(server.setup()).resolves.toBeDefined(); - expect(loggingServiceMock.collect(logger)).toMatchSnapshot('unused paths logs'); }); test('stops services on "stop"', async () => { - const server = new Server(configService as any, logger, env); + const server = new Server(config$, env, logger); await server.setup(); @@ -115,3 +101,17 @@ 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.setSchema.mockImplementation(() => { + throw new Error('invalid config'); + }); + const server = new Server(config$, env, logger); + await expect(server.setupConfigSchemas()).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 4d85c79b13e06a..9a416546d02e5e 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -16,24 +16,38 @@ * specific language governing permissions and limitations * under the License. */ +import { Observable } from 'rxjs'; +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 } from './plugins'; +import { PluginsService, config as pluginsConfig } from './plugins'; + +import { config as elasticsearchConfig } from './elasticsearch'; +import { config as httpConfig } from './http'; +import { config as loggingConfig } from './logging'; +import { config as devConfig } 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( + readonly config$: Observable, + 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); @@ -47,6 +61,7 @@ export class Server { this.registerDefaultRoute(httpSetup); const elasticsearchServiceSetup = await this.elasticsearch.setup(); + const pluginsSetup = await this.plugins.setup({ elasticsearch: elasticsearchServiceSetup, http: httpSetup, @@ -91,4 +106,18 @@ export class Server { router.get({ path: '/', validate: false }, async (req, res) => res.ok({ version: '0.0.1' })); httpSetup.registerRouter(router); } + + public async setupConfigSchemas() { + const schemas: Array<[ConfigPath, Type]> = [ + [elasticsearchConfig.path, elasticsearchConfig.schema], + [loggingConfig.path, loggingConfig.schema], + [httpConfig.path, httpConfig.schema], + [pluginsConfig.path, pluginsConfig.schema], + [devConfig.path, devConfig.schema], + ]; + + for (const [path, schema] of schemas) { + await this.configService.setSchema(path, schema); + } + } } diff --git a/src/plugins/testbed/server/index.ts b/src/plugins/testbed/server/index.ts index e430488dd268b1..538d3a3d07c9be 100644 --- a/src/plugins/testbed/server/index.ts +++ b/src/plugins/testbed/server/index.ts @@ -76,3 +76,7 @@ class Plugin { export const plugin = (initializerContext: PluginInitializerContext) => new Plugin(initializerContext); + +export const config = { + schema: TestBedConfig.schema, +};