Skip to content

Commit

Permalink
Allow kbn-config-schema to ignore unknown keys (#59560)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
legrego and elasticmachine committed Mar 17, 2020
1 parent 6a70d21 commit 9318862
Show file tree
Hide file tree
Showing 53 changed files with 132 additions and 96 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ validate: RouteValidatorFullConfig<P, Q, B> | false;

## Remarks

You \*must\* specify a validation schema to be able to read: - url path segments - request query - request body To opt out of validating the request, specify `validate: false`<!-- -->. In this case request params, query, and body will be \*\*empty\*\* objects and have no 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 })`<!-- -->;
You \*must\* specify a validation schema to be able to read: - url path segments - request query - request body To opt out of validating the request, specify `validate: false`<!-- -->. In this case request params, query, and body will be \*\*empty\*\* objects and have no 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({}, { unknowns: 'allow' })`<!-- -->;

## Example

Expand Down Expand Up @@ -49,7 +49,7 @@ router.get({
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,) {
Expand Down
4 changes: 2 additions & 2 deletions packages/kbn-config-schema/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.

Expand Down
43 changes: 37 additions & 6 deletions packages/kbn-config-schema/src/types/object_type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(() =>
Expand All @@ -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"`);
});
21 changes: 15 additions & 6 deletions packages/kbn-config-schema/src/types/object_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/core/server/http/router/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,) {
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/http/router/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({})
)
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/ui_settings/routes/set_many.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' }),
}),
};

Expand Down
2 changes: 1 addition & 1 deletion src/core/server/ui_settings/ui_settings_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const configSchema = schema.object({
})
),
},
{ allowUnknowns: true }
{ unknowns: 'allow' }
),
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' }
),
},
},
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/data/server/search/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/timelion/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
12 changes: 4 additions & 8 deletions src/plugins/timelion/server/routes/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' }))
),
}),
}),
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_type_timeseries/server/routes/vis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' }),
}),
},
},
Expand Down
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/rollup/server/routes/api/jobs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export function registerJobsRoute(deps: RouteDependencies, legacy: ServerShim) {
{
id: schema.string(),
},
{ allowUnknowns: true }
{ unknowns: 'allow' }
),
}),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: [] }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
},
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/apm/server/routes/create_api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>)(
{
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/canvas/server/routes/workpad/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/case/server/routes/api/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,4 @@ export const sortToSnake = (sortField: string): SortFieldCase => {
}
};

export const escapeHatch = schema.object({}, { allowUnknowns: true });
export const escapeHatch = schema.object({}, { unknowns: 'allow' });
12 changes: 6 additions & 6 deletions x-pack/plugins/file_upload/server/routes/file_upload.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ export const bodySchema = schema.object(
{},
{
defaultValue: {},
allowUnknowns: true,
unknowns: 'allow',
}
)
),
},
{ allowUnknowns: true }
{ unknowns: 'allow' }
);

const options = {
Expand All @@ -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()
Expand All @@ -58,7 +58,7 @@ export const idConditionalValidation = (body, boolHasId) =>
defaultValue: {
number_of_shards: 1,
},
allowUnknowns: true,
unknowns: 'allow',
}
),
mappings: boolHasId
Expand All @@ -67,11 +67,11 @@ export const idConditionalValidation = (body, boolHasId) =>
{},
{
defaultValue: {},
allowUnknowns: true,
unknowns: 'allow',
}
),
},
{ allowUnknowns: true }
{ unknowns: 'allow' }
)
.validate(body);

Expand Down
Loading

0 comments on commit 9318862

Please sign in to comment.