[Config Service] Use stripUnknownKeys when checking enabled flags (#201579)

## Summary

Resolves #201442.

The underlying issue is that `isEnabledAtPath` validates the entire
config object when it only cares about `.enabled`. This PR performs that
check using `stripUnknownKeys: true`, as we'll perform the actual
validation later on.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
This commit is contained in:
Alejandro Fernández Haro 2024-12-02 17:30:09 +01:00 committed by GitHub
parent 5731fb3ca4
commit 3e1d62ebc4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 103 additions and 7 deletions

View file

@ -517,6 +517,41 @@ describe('nested unknowns', () => {
},
});
});
describe(`stripUnknownKeys: true in validate`, () => {
test('should strip unknown keys', () => {
const type = schema.object({
myObj: schema.object({
foo: schema.string({ defaultValue: 'test' }),
nested: schema.object({
a: schema.number(),
}),
}),
});
expect(
type.validate(
{
myObj: {
bar: 'baz',
nested: {
a: 1,
b: 2,
},
},
},
void 0,
void 0,
{ stripUnknownKeys: true }
)
).toStrictEqual({
myObj: {
foo: 'test',
nested: { a: 1 },
},
});
});
});
});
test('handles optional properties', () => {

View file

@ -313,6 +313,37 @@ test('handles disabled path and marks config as used', async () => {
expect(unusedPaths).toEqual([]);
});
test('does not throw if extra options are provided', async () => {
const initialConfig = {
pid: {
enabled: false,
file: '/some/file.pid',
extraUnknownOption: 1,
extraNestedUnknownOptions: {
anOption: true,
anotherOption: 'something',
},
},
};
const rawConfigProvider = createRawConfigServiceMock({ rawConfig: initialConfig });
const configService = new ConfigService(rawConfigProvider, defaultEnv, logger);
configService.setSchema(
'pid',
schema.object({
enabled: schema.boolean({ defaultValue: false }),
file: schema.string(),
})
);
const isEnabled = await configService.isEnabledAtPath('pid');
expect(isEnabled).toBe(false);
const unusedPaths = await configService.getUnusedPaths();
expect(unusedPaths).toEqual([]);
});
test('does not throw if schema does not define "enabled" schema', async () => {
const initialConfig = {
pid: {

View file

@ -12,7 +12,7 @@ import { SchemaTypeError, Type, ValidationError } from '@kbn/config-schema';
import { cloneDeep, isEqual, merge, unset } from 'lodash';
import { set } from '@kbn/safer-lodash-set';
import { BehaviorSubject, combineLatest, firstValueFrom, Observable, identity } from 'rxjs';
import { distinctUntilChanged, first, map, shareReplay, tap } from 'rxjs';
import { distinctUntilChanged, map, shareReplay, tap } from 'rxjs';
import { Logger, LoggerFactory } from '@kbn/logging';
import { getDocLinks, DocLinks } from '@kbn/doc-links';
@ -209,10 +209,31 @@ export class ConfigService {
throw new Error(`No validation schema has been defined for [${namespace}]`);
}
const validatedConfig = hasSchema
? await this.atPath<{ enabled?: boolean }>(path).pipe(first()).toPromise()
let validatedConfig = hasSchema
? await firstValueFrom(
this.getValidatedConfigAtPath$(
path,
// At this point we don't care about how valid the config is: we just want to read `enabled`
{ stripUnknownKeys: true }
) as Observable<{ enabled?: boolean }>,
{ defaultValue: undefined }
)
: undefined;
// Special use case: when the provided config includes `enabled` and the validated config doesn't,
// it's quite likely that's not an allowed config and it should fail.
// Applying "normal" validation (not stripping unknowns) in that case.
if (
hasSchema &&
typeof config.get(path)?.enabled !== 'undefined' &&
typeof validatedConfig?.enabled === 'undefined'
) {
validatedConfig = await firstValueFrom(
this.getValidatedConfigAtPath$(path) as Observable<{ enabled?: boolean }>,
{ defaultValue: undefined }
);
}
const isDisabled = validatedConfig?.enabled === false;
if (isDisabled) {
// If the plugin is explicitly disabled, we mark the entire plugin
@ -325,7 +346,13 @@ export class ConfigService {
});
}
private validateAtPath(path: ConfigPath, config: Record<string, unknown>) {
private validateAtPath(
path: ConfigPath,
config: Record<string, unknown>,
validateOptions?: { stripUnknownKeys?: boolean }
) {
const stripUnknownKeys = validateOptions?.stripUnknownKeys || this.stripUnknownKeys;
const namespace = pathToString(path);
const schema = this.schemas.get(namespace);
if (!schema) {
@ -340,18 +367,21 @@ export class ConfigService {
...this.env.packageInfo,
},
`config validation of [${namespace}]`,
this.stripUnknownKeys ? { stripUnknownKeys: this.stripUnknownKeys } : {}
stripUnknownKeys ? { stripUnknownKeys } : {}
);
}
private getValidatedConfigAtPath$(
path: ConfigPath,
{ ignoreUnchanged = true }: { ignoreUnchanged?: boolean } = {}
{
ignoreUnchanged = true,
stripUnknownKeys,
}: { ignoreUnchanged?: boolean; stripUnknownKeys?: boolean } = {}
) {
return this.config$.pipe(
map((config) => config.get(path)),
ignoreUnchanged ? distinctUntilChanged(isEqual) : identity,
map((config) => this.validateAtPath(path, config))
map((config) => this.validateAtPath(path, config, { stripUnknownKeys }))
);
}