Allow kbn-config-schema to ignore unknown keys (elastic#59560)

* allow kbn-config-schema to ignore unknown keys * Consolidate unknown key configuration * updates following merge Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
legrego · Mar 17, 2020 · afe55b9 · afe55b9
1 parent 6d10ca8
commit afe55b9
Show file tree

Hide file tree

Showing 52 changed files with 130 additions and 94 deletions.
diff --git a/packages/kbn-config-schema/README.md b/packages/kbn-config-schema/README.md
@@ -239,7 +239,7 @@ __Output type:__ `{ [K in keyof TProps]: TypeOf<TProps[K]> } as TObject`
 __Options:__
   * `defaultValue: TObject | Reference<TObject> | (() => TObject)` - defines a default value, see [Default values](#default-values) section for more details.
   * `validate: (value: TObject) => string | void` - defines a custom validator function, see [Custom validation](#custom-validation) section for more details.
-  * `allowUnknowns: boolean` - indicates whether unknown object properties should be allowed. It's `false` by default.
+  * `unknowns: 'allow' | 'ignore' | 'forbid'` - indicates whether unknown object properties should be allowed, ignored, or forbidden. It's `forbid` by default.
 
 __Usage:__
 ```typescript
@@ -250,7 +250,7 @@ const valueSchema = schema.object({
 ```
 
 __Notes:__
-* Using `allowUnknowns` is discouraged and should only be used in exceptional circumstances. Consider using `schema.recordOf()` instead.
+* Using `unknowns: 'allow'` is discouraged and should only be used in exceptional circumstances. Consider using `schema.recordOf()` instead.
 * Currently `schema.object()` always has a default value of `{}`, but this may change in the near future. Try to not rely on this behaviour and specify default value explicitly or use `schema.maybe()` if the value is optional.
 * `schema.object()` also supports a json string as input if it can be safely parsed using `JSON.parse` and if the resulting value is a plain object.
 

diff --git a/packages/kbn-config-schema/src/types/object_type.test.ts b/packages/kbn-config-schema/src/types/object_type.test.ts
@@ -276,10 +276,10 @@ test('individual keys can validated', () => {
   );
 });
 
-test('allow unknown keys when allowUnknowns = true', () => {
+test('allow unknown keys when unknowns = `allow`', () => {
   const type = schema.object(
     { foo: schema.string({ defaultValue: 'test' }) },
-    { allowUnknowns: true }
+    { unknowns: 'allow' }
   );
 
   expect(
@@ -292,10 +292,10 @@ test('allow unknown keys when allowUnknowns = true', () => {
   });
 });
 
-test('allowUnknowns = true affects only own keys', () => {
+test('unknowns = `allow` affects only own keys', () => {
   const type = schema.object(
     { foo: schema.object({ bar: schema.string() }) },
-    { allowUnknowns: true }
+    { unknowns: 'allow' }
   );
 
   expect(() =>
@@ -308,14 +308,45 @@ test('allowUnknowns = true affects only own keys', () => {
   ).toThrowErrorMatchingInlineSnapshot(`"[foo.baz]: definition for this key is missing"`);
 });
 
-test('does not allow unknown keys when allowUnknowns = false', () => {
+test('does not allow unknown keys when unknowns = `forbid`', () => {
   const type = schema.object(
     { foo: schema.string({ defaultValue: 'test' }) },
-    { allowUnknowns: false }
+    { unknowns: 'forbid' }
   );
   expect(() =>
     type.validate({
       bar: 'baz',
     })
   ).toThrowErrorMatchingInlineSnapshot(`"[bar]: definition for this key is missing"`);
 });
+
+test('allow and remove unknown keys when unknowns = `ignore`', () => {
+  const type = schema.object(
+    { foo: schema.string({ defaultValue: 'test' }) },
+    { unknowns: 'ignore' }
+  );
+
+  expect(
+    type.validate({
+      bar: 'baz',
+    })
+  ).toEqual({
+    foo: 'test',
+  });
+});
+
+test('unknowns = `ignore` affects only own keys', () => {
+  const type = schema.object(
+    { foo: schema.object({ bar: schema.string() }) },
+    { unknowns: 'ignore' }
+  );
+
+  expect(() =>
+    type.validate({
+      foo: {
+        bar: 'bar',
+        baz: 'baz',
+      },
+    })
+  ).toThrowErrorMatchingInlineSnapshot(`"[foo.baz]: definition for this key is missing"`);
+});
diff --git a/packages/kbn-config-schema/src/types/object_type.ts b/packages/kbn-config-schema/src/types/object_type.ts
@@ -30,17 +30,25 @@ export type TypeOf<RT extends Type<any>> = RT['type'];
 // this might not have perfect _rendering_ output, but it will be typed.
 export type ObjectResultType<P extends Props> = Readonly<{ [K in keyof P]: TypeOf<P[K]> }>;
 
+interface UnknownOptions {
+  /**
+   * Options for dealing with unknown keys:
+   * - allow: unknown keys will be permitted
+   * - ignore: unknown keys will not fail validation, but will be stripped out
+   * - forbid (default): unknown keys will fail validation
+   */
+  unknowns?: 'allow' | 'ignore' | 'forbid';
+}
+
 export type ObjectTypeOptions<P extends Props = any> = TypeOptions<
   { [K in keyof P]: TypeOf<P[K]> }
-> & {
-  /** Should uknown keys not be defined in the schema be allowed. Defaults to `false` */
-  allowUnknowns?: boolean;
-};
+> &
+  UnknownOptions;
 
 export class ObjectType<P extends Props = any> extends Type<ObjectResultType<P>> {
   private props: Record<string, AnySchema>;
 
-  constructor(props: P, { allowUnknowns = false, ...typeOptions }: ObjectTypeOptions<P> = {}) {
+  constructor(props: P, { unknowns = 'forbid', ...typeOptions }: ObjectTypeOptions<P> = {}) {
     const schemaKeys = {} as Record<string, AnySchema>;
     for (const [key, value] of Object.entries(props)) {
       schemaKeys[key] = value.getSchema();
@@ -50,7 +58,8 @@ export class ObjectType<P extends Props = any> extends Type<ObjectResultType<P>>
       .keys(schemaKeys)
       .default()
       .optional()
-      .unknown(Boolean(allowUnknowns));
+      .unknown(unknowns === 'allow')
+      .options({ stripUnknown: { objects: unknowns === 'ignore' } });
 
     super(schema, typeOptions);
     this.props = schemaKeys;

diff --git a/src/core/server/http/router/route.ts b/src/core/server/http/router/route.ts
@@ -179,7 +179,7 @@ export interface RouteConfig<P, Q, B, Method extends RouteMethod> {
    * access to raw values.
    * In some cases you may want to use another validation library. To do this, you need to
    * instruct the `@kbn/config-schema` library to output **non-validated values** with
-   * setting schema as `schema.object({}, { allowUnknowns: true })`;
+   * setting schema as `schema.object({}, { unknowns: 'allow' })`;
    *
    * @example
    * ```ts
@@ -212,7 +212,7 @@ export interface RouteConfig<P, Q, B, Method extends RouteMethod> {
    *   path: 'path/{id}',
    *   validate: {
    *     // handler has access to raw non-validated params in runtime
-   *     params: schema.object({}, { allowUnknowns: true })
+   *     params: schema.object({}, { unknowns: 'allow' })
    *   },
    * },
    * (context, req, res,) {

diff --git a/src/core/server/http/router/router.test.ts b/src/core/server/http/router/router.test.ts
@@ -59,7 +59,7 @@ describe('Router', () => {
           {
             path: '/',
             options: { body: { output: 'file' } } as any, // We explicitly don't support 'file'
-            validate: { body: schema.object({}, { allowUnknowns: true }) },
+            validate: { body: schema.object({}, { unknowns: 'allow' }) },
           },
           (context, req, res) => res.ok({})
         )

diff --git a/src/core/server/ui_settings/routes/set_many.ts b/src/core/server/ui_settings/routes/set_many.ts
@@ -24,7 +24,7 @@ import { CannotOverrideError } from '../ui_settings_errors';
 
 const validate = {
   body: schema.object({
-    changes: schema.object({}, { allowUnknowns: true }),
+    changes: schema.object({}, { unknowns: 'allow' }),
   }),
 };
 

diff --git a/src/core/server/ui_settings/ui_settings_config.ts b/src/core/server/ui_settings/ui_settings_config.ts
@@ -39,7 +39,7 @@ const configSchema = schema.object({
         })
       ),
     },
-    { allowUnknowns: true }
+    { unknowns: 'allow' }
   ),
 });
 

diff --git a/src/plugins/data/server/autocomplete/value_suggestions_route.ts b/src/plugins/data/server/autocomplete/value_suggestions_route.ts
@@ -39,15 +39,15 @@ export function registerValueSuggestionsRoute(
           {
             index: schema.string(),
           },
-          { allowUnknowns: false }
+          { unknowns: 'allow' }
         ),
         body: schema.object(
           {
             field: schema.string(),
             query: schema.string(),
             boolFilter: schema.maybe(schema.any()),
           },
-          { allowUnknowns: false }
+          { unknowns: 'allow' }
         ),
       },
     },

diff --git a/src/plugins/data/server/search/routes.ts b/src/plugins/data/server/search/routes.ts
@@ -28,9 +28,9 @@ export function registerSearchRoute(router: IRouter): void {
       validate: {
         params: schema.object({ strategy: schema.string() }),
 
-        query: schema.object({}, { allowUnknowns: true }),
+        query: schema.object({}, { unknowns: 'allow' }),
 
-        body: schema.object({}, { allowUnknowns: true }),
+        body: schema.object({}, { unknowns: 'allow' }),
       },
     },
     async (context, request, res) => {
@@ -64,7 +64,7 @@ export function registerSearchRoute(router: IRouter): void {
           id: schema.string(),
         }),
 
-        query: schema.object({}, { allowUnknowns: true }),
+        query: schema.object({}, { unknowns: 'allow' }),
       },
     },
     async (context, request, res) => {

diff --git a/src/plugins/timelion/config.ts b/src/plugins/timelion/config.ts
@@ -25,7 +25,7 @@ export const configSchema = schema.object(
     graphiteUrls: schema.maybe(schema.arrayOf(schema.string())),
   },
   // This option should be removed as soon as we entirely migrate config from legacy Timelion plugin.
-  { allowUnknowns: true }
+  { unknowns: 'allow' }
 );
 
 export type ConfigSchema = TypeOf<typeof configSchema>;
diff --git a/src/plugins/timelion/server/routes/run.ts b/src/plugins/timelion/server/routes/run.ts
@@ -78,15 +78,11 @@ export function runRoute(
               es: schema.object({
                 filter: schema.object({
                   bool: schema.object({
-                    filter: schema.maybe(
-                      schema.arrayOf(schema.object({}, { allowUnknowns: true }))
-                    ),
-                    must: schema.maybe(schema.arrayOf(schema.object({}, { allowUnknowns: true }))),
-                    should: schema.maybe(
-                      schema.arrayOf(schema.object({}, { allowUnknowns: true }))
-                    ),
+                    filter: schema.maybe(schema.arrayOf(schema.object({}, { unknowns: 'allow' }))),
+                    must: schema.maybe(schema.arrayOf(schema.object({}, { unknowns: 'allow' }))),
+                    should: schema.maybe(schema.arrayOf(schema.object({}, { unknowns: 'allow' }))),
                     must_not: schema.maybe(
-                      schema.arrayOf(schema.object({}, { allowUnknowns: true }))
+                      schema.arrayOf(schema.object({}, { unknowns: 'allow' }))
                     ),
                   }),
                 }),

diff --git a/src/plugins/vis_type_timeseries/server/routes/vis.ts b/src/plugins/vis_type_timeseries/server/routes/vis.ts
@@ -23,7 +23,7 @@ import { getVisData, GetVisDataOptions } from '../lib/get_vis_data';
 import { visPayloadSchema } from './post_vis_schema';
 import { Framework, ValidationTelemetryServiceSetup } from '../index';
 
-const escapeHatch = schema.object({}, { allowUnknowns: true });
+const escapeHatch = schema.object({}, { unknowns: 'allow' });
 
 export const visDataRoutes = (
   router: IRouter,

diff --git a/test/plugin_functional/plugins/rendering_plugin/server/plugin.ts b/test/plugin_functional/plugins/rendering_plugin/server/plugin.ts
@@ -33,7 +33,7 @@ export class RenderingPlugin implements Plugin {
             {
               includeUserSettings: schema.boolean({ defaultValue: true }),
             },
-            { allowUnknowns: true }
+            { unknowns: 'allow' }
           ),
           params: schema.object({
             id: schema.maybe(schema.string()),

diff --git a/...y/plugins/license_management/server/np_ready/routes/api/license/register_license_route.ts b/...y/plugins/license_management/server/np_ready/routes/api/license/register_license_route.ts
@@ -15,7 +15,7 @@ export function registerLicenseRoute(server: Server, legacy: Legacy, xpackInfo:
       validate: {
         query: schema.object({ acknowledge: schema.string() }),
         body: schema.object({
-          license: schema.object({}, { allowUnknowns: true }),
+          license: schema.object({}, { unknowns: 'allow' }),
         }),
       },
     },

diff --git a/x-pack/legacy/plugins/rollup/server/routes/api/jobs.ts b/x-pack/legacy/plugins/rollup/server/routes/api/jobs.ts
@@ -137,7 +137,7 @@ export function registerJobsRoute(deps: RouteDependencies, legacy: ServerShim) {
             {
               id: schema.string(),
             },
-            { allowUnknowns: true }
+            { unknowns: 'allow' }
           ),
         }),
       },

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/signal_params_schema.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/signal_params_schema.ts
@@ -26,13 +26,13 @@ export const signalParamsSchema = () =>
     savedId: schema.nullable(schema.string()),
     timelineId: schema.nullable(schema.string()),
     timelineTitle: schema.nullable(schema.string()),
-    meta: schema.nullable(schema.object({}, { allowUnknowns: true })),
+    meta: schema.nullable(schema.object({}, { unknowns: 'allow' })),
     query: schema.nullable(schema.string()),
-    filters: schema.nullable(schema.arrayOf(schema.object({}, { allowUnknowns: true }))),
+    filters: schema.nullable(schema.arrayOf(schema.object({}, { unknowns: 'allow' }))),
     maxSignals: schema.number({ defaultValue: DEFAULT_MAX_SIGNALS }),
     riskScore: schema.number(),
     severity: schema.string(),
-    threat: schema.nullable(schema.arrayOf(schema.object({}, { allowUnknowns: true }))),
+    threat: schema.nullable(schema.arrayOf(schema.object({}, { unknowns: 'allow' }))),
     to: schema.string(),
     type: schema.string(),
     references: schema.arrayOf(schema.string(), { defaultValue: [] }),

diff --git a/x-pack/legacy/plugins/siem/server/lib/framework/kibana_framework_adapter.ts b/x-pack/legacy/plugins/siem/server/lib/framework/kibana_framework_adapter.ts
@@ -61,7 +61,7 @@ export class KibanaBackendFrameworkAdapter implements FrameworkAdapter {
     this.router.post(
       {
         path: routePath,
-        validate: { body: configSchema.object({}, { allowUnknowns: true }) },
+        validate: { body: configSchema.object({}, { unknowns: 'allow' }) },
         options: {
           tags: ['access:siem'],
         },

diff --git a/x-pack/plugins/apm/server/routes/create_api/index.ts b/x-pack/plugins/apm/server/routes/create_api/index.ts
@@ -71,7 +71,7 @@ export function createApi() {
           body: bodyRt || t.null
         };
 
-        const anyObject = schema.object({}, { allowUnknowns: true });
+        const anyObject = schema.object({}, { unknowns: 'allow' });
 
         (router[routerMethod] as RouteRegistrar<typeof routerMethod>)(
           {

diff --git a/x-pack/plugins/canvas/server/routes/workpad/update.ts b/x-pack/plugins/canvas/server/routes/workpad/update.ts
@@ -120,7 +120,7 @@ export function initializeUpdateWorkpadAssetsRoute(deps: RouteInitializerDeps) {
         // ToDo: Currently the validation must be a schema.object
         // Because we don't know what keys the assets will have, we have to allow
         // unknowns and then validate in the handler
-        body: schema.object({}, { allowUnknowns: true }),
+        body: schema.object({}, { unknowns: 'allow' }),
       },
       options: {
         body: {

diff --git a/x-pack/plugins/case/server/routes/api/utils.ts b/x-pack/plugins/case/server/routes/api/utils.ts
@@ -141,4 +141,4 @@ export const sortToSnake = (sortField: string): SortFieldCase => {
   }
 };
 
-export const escapeHatch = schema.object({}, { allowUnknowns: true });
+export const escapeHatch = schema.object({}, { unknowns: 'allow' });
diff --git a/x-pack/plugins/file_upload/server/routes/file_upload.js b/x-pack/plugins/file_upload/server/routes/file_upload.js
@@ -28,12 +28,12 @@ export const bodySchema = schema.object(
         {},
         {
           defaultValue: {},
-          allowUnknowns: true,
+          unknowns: 'allow',
         }
       )
     ),
   },
-  { allowUnknowns: true }
+  { unknowns: 'allow' }
 );
 
 const options = {
@@ -48,7 +48,7 @@ export const idConditionalValidation = (body, boolHasId) =>
     .object(
       {
         data: boolHasId
-          ? schema.arrayOf(schema.object({}, { allowUnknowns: true }), { minSize: 1 })
+          ? schema.arrayOf(schema.object({}, { unknowns: 'allow' }), { minSize: 1 })
           : schema.any(),
         settings: boolHasId
           ? schema.any()
@@ -58,7 +58,7 @@ export const idConditionalValidation = (body, boolHasId) =>
                 defaultValue: {
                   number_of_shards: 1,
                 },
-                allowUnknowns: true,
+                unknowns: 'allow',
               }
             ),
         mappings: boolHasId
@@ -67,11 +67,11 @@ export const idConditionalValidation = (body, boolHasId) =>
               {},
               {
                 defaultValue: {},
-                allowUnknowns: true,
+                unknowns: 'allow',
               }
             ),
       },
-      { allowUnknowns: true }
+      { unknowns: 'allow' }
     )
     .validate(body);
 

diff --git a/x-pack/plugins/graph/server/routes/explore.ts b/x-pack/plugins/graph/server/routes/explore.ts
@@ -23,7 +23,7 @@ export function registerExploreRoute({
       validate: {
         body: schema.object({
           index: schema.string(),
-          query: schema.object({}, { allowUnknowns: true }),
+          query: schema.object({}, { unknowns: 'allow' }),
         }),
       },
     },

diff --git a/x-pack/plugins/graph/server/routes/search.ts b/x-pack/plugins/graph/server/routes/search.ts
@@ -21,7 +21,7 @@ export function registerSearchRoute({
       validate: {
         body: schema.object({
           index: schema.string(),
-          body: schema.object({}, { allowUnknowns: true }),
+          body: schema.object({}, { unknowns: 'allow' }),
         }),
       },
     },