[8.x] [Config Service] Use stripUnknownKeys when checking `enabl…

…ed` flags (#201579) (#202531) # Backport This will backport the following commits from `main` to `8.x`: - [[Config Service] Use stripUnknownKeys when checking `enabled` flags (#201579)](#201579)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Alejandro Fernández Haro <[email protected]>
elastic · Dec 2, 2024 · 6123855 · 6123855
1 parent 91fb172
commit 6123855
Show file tree

Hide file tree

Showing 3 changed files with 103 additions and 7 deletions.
diff --git a/packages/kbn-config-schema/src/types/object_type.test.ts b/packages/kbn-config-schema/src/types/object_type.test.ts
@@ -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', () => {

diff --git a/packages/kbn-config/src/config_service.test.ts b/packages/kbn-config/src/config_service.test.ts
@@ -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: {

diff --git a/packages/kbn-config/src/config_service.ts b/packages/kbn-config/src/config_service.ts
@@ -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 }))
     );
   }