Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow kbn-config-schema to ignore unknown keys #59560

Merged
merged 7 commits into from
Mar 17, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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' })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow, the corresponding Markdown API doc (at docs/development/core/server/kibana-plugin-server.routeconfig.validate.md, line 52) doesn't reflect this change.

* },
* },
* (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 @@ -65,13 +65,13 @@ export const signalRulesAlertType = ({
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 @@ -134,4 +134,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
2 changes: 1 addition & 1 deletion x-pack/plugins/graph/server/routes/explore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function registerExploreRoute({
validate: {
body: schema.object({
index: schema.string(),
query: schema.object({}, { allowUnknowns: true }),
query: schema.object({}, { unknowns: 'allow' }),
}),
},
},
Expand Down
Loading