[New Platform] Validate config upfront

docs/development/core/server/kibana-plugin-server.configservice.setschema.md

@@ -0,0 +1,25 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [ConfigService](./kibana-plugin-server.configservice.md) &gt; [setSchema](./kibana-plugin-server.configservice.setschema.md)
+
+## ConfigService.setSchema() method
+
+Set config schema for a path and performs its validation
+
+<b>Signature:</b>
+
+```typescript
+setSchema(path: ConfigPath, schema: Type<unknown>): Promise<void>;
+```
+
+## Parameters
+
+|  Parameter | Type | Description |
+|  --- | --- | --- |
+|  path | <code>ConfigPath</code> |  |
+|  schema | <code>Type&lt;unknown&gt;</code> |  |
+
+<b>Returns:</b>
+
+`Promise<void>`
+

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<ConfigService>;
+type ConfigServiceContract = PublicMethodsOf<ConfigService>;
 const createConfigServiceMock = () => {
-  const mocked: jest.Mocked<ConfigSericeContract> = {
+  const mocked: jest.Mocked<ConfigServiceContract> = {
     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({})));

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);
+  configService.setSchema('key', schema.string());
 
   const configs = configService.atPath('key', ExampleClassWithStringSchema);
   const exampleConfig = await configs.pipe(first()).toPromise();
@@ -50,6 +57,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);
+  configService.setSchema('key', schema.string());
+
   const configs = configService.atPath('key', ExampleClassWithStringSchema);
 
   try {
@@ -62,6 +71,7 @@ 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);
+  configService.setSchema('unique-name', schema.string());
 
   const configs = configService.optionalAtPath('unique-name', ExampleClassWithStringSchema);
   const exampleConfig = await configs.pipe(first()).toPromise();
@@ -72,6 +82,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);
+  configService.setSchema('value', schema.string());
 
   const configs = configService.optionalAtPath('value', ExampleClassWithStringSchema);
   const exampleConfig: any = await configs.pipe(first()).toPromise();
@@ -83,6 +94,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);
+  configService.setSchema('key', schema.string());
 
   const valuesReceived: any[] = [];
   configService.atPath('key', ExampleClassWithStringSchema).subscribe(config => {
@@ -97,6 +109,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);
+  configService.setSchema('key', schema.string());
 
   const valuesReceived: any[] = [];
   configService.atPath('key', ExampleClassWithStringSchema).subscribe(config => {
@@ -108,21 +121,28 @@ 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' }));
   const configService = new ConfigService(config$, defaultEnv, logger);
+  configService.setSchema('no-key', schema.string());
 
   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 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();
+  expect(addSchema()).rejects.toMatchInlineSnapshot(
+    `[Error: Validation schema for key was already registered.]`
+  );
 });
 
 test('tracks unhandled paths', async () => {
@@ -178,28 +198,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'),
-        }),
-      })
-    )
-  );
+  configService.setSchema('foo', schemaDefinition);
+  const configs = configService.atPath('foo', createClassWithSchema(schemaDefinition));
 
   expect(await configs.pipe(first()).toPromise()).toMatchSnapshot();
 });
@@ -278,9 +295,3 @@ function createClassWithSchema(s: Type<any>) {
     constructor(readonly value: TypeOf<typeof s>) {}
   };
 }
-
-class ExampleClassWithStringSchema {
-  public static schema = schema.string();
-
-  constructor(readonly value: string) {}
-}

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<string, Type<unknown>>();
 
   constructor(
     private readonly config$: Observable<Config>,
@@ -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<unknown>) {
+    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<TSchema extends Type<any>, TConfig>(
+  public atPath<TSchema extends Type<unknown>, TConfig>(
     path: ConfigPath,
     ConfigClass: ConfigWithSchema<TSchema, TConfig>
   ) {
-    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<TSchema, TConfig>
   ) {
     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<TSchema extends Type<any>, TConfig>(
-    path: ConfigPath,
-    config: Record<string, any>,
-    ConfigClass: ConfigWithSchema<TSchema, TConfig>
-  ) {
-    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<string, any>) {
+    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<TSchema extends Type<unknown>, TConfig>(
+    validatedConfig: unknown,
+    ConfigClass: ConfigWithSchema<TSchema, TConfig>
+  ) {
     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);
 

src/core/server/dev/dev_config.ts

@@ -25,8 +25,12 @@ const createDevSchema = schema.object({
   }),
 });
 
-type DevConfigType = TypeOf<typeof createDevSchema>;
+export const config = {
+  path: 'dev',
+  schema: createDevSchema,
+};
 
+export type DevConfigType = TypeOf<typeof createDevSchema>;
 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;
   }
 }

src/core/server/dev/index.ts

@@ -17,4 +17,4 @@
  * under the License.
  */
 
-export { DevConfig } from './dev_config';
+export { DevConfig, config } from './dev_config';