From 1fe3bb35153b3903ef20c32524825a8546c3b997 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 7 Apr 2021 22:38:37 +0200 Subject: [PATCH] fix config validation --- .../config/ensure_valid_configuration.test.ts | 46 +++++++++++++++++-- .../config/ensure_valid_configuration.ts | 25 ++++++---- src/core/server/dev/dev_config.ts | 16 ------- src/core/server/dev/index.ts | 9 ---- src/core/server/server.ts | 2 - 5 files changed, 58 insertions(+), 40 deletions(-) delete mode 100644 src/core/server/dev/dev_config.ts delete mode 100644 src/core/server/dev/index.ts diff --git a/src/core/server/config/ensure_valid_configuration.test.ts b/src/core/server/config/ensure_valid_configuration.test.ts index 474e8dd59b4c4..f1006f93dbc2d 100644 --- a/src/core/server/config/ensure_valid_configuration.test.ts +++ b/src/core/server/config/ensure_valid_configuration.test.ts @@ -16,14 +16,40 @@ describe('ensureValidConfiguration', () => { beforeEach(() => { jest.clearAllMocks(); configService = configServiceMock.create(); - configService.getUsedPaths.mockReturnValue(Promise.resolve(['core', 'elastic'])); + + configService.validate.mockResolvedValue(); + configService.getUsedPaths.mockReturnValue(Promise.resolve([])); }); - it('returns normally when there is no unused keys', async () => { - configService.getUnusedPaths.mockResolvedValue([]); + it('returns normally when there is no unused keys and when the config validates', async () => { await expect(ensureValidConfiguration(configService as any)).resolves.toBeUndefined(); }); + it('throws when config validation fails', async () => { + configService.validate.mockImplementation(() => { + throw new Error('some message'); + }); + + await expect(ensureValidConfiguration(configService as any)).rejects.toMatchInlineSnapshot( + `[Error: some message]` + ); + }); + + it('throws a `CriticalError` with the correct processExitCode value when config validation fails', async () => { + expect.assertions(2); + + configService.validate.mockImplementation(() => { + throw new Error('some message'); + }); + + try { + await ensureValidConfiguration(configService as any); + } catch (e) { + expect(e).toBeInstanceOf(CriticalError); + expect(e.processExitCode).toEqual(78); + } + }); + it('throws when there are some unused keys', async () => { configService.getUnusedPaths.mockResolvedValue(['some.key', 'some.other.key']); @@ -44,4 +70,18 @@ describe('ensureValidConfiguration', () => { expect(e.processExitCode).toEqual(64); } }); + + it('does not throw when all unused keys are included in the ignored paths', async () => { + configService.getUnusedPaths.mockResolvedValue(['dev.someDevKey', 'elastic.apm.enabled']); + + await expect(ensureValidConfiguration(configService as any)).resolves.toBeUndefined(); + }); + + it('throws when only some keys are included in the ignored paths', async () => { + configService.getUnusedPaths.mockResolvedValue(['dev.someDevKey', 'some.key']); + + await expect(ensureValidConfiguration(configService as any)).rejects.toMatchInlineSnapshot( + `[Error: Unknown configuration key(s): "some.key". Check for spelling errors and ensure that expected plugins are installed.]` + ); + }); }); diff --git a/src/core/server/config/ensure_valid_configuration.ts b/src/core/server/config/ensure_valid_configuration.ts index a33625cc0841d..c7a4721b7d2ae 100644 --- a/src/core/server/config/ensure_valid_configuration.ts +++ b/src/core/server/config/ensure_valid_configuration.ts @@ -9,22 +9,27 @@ import { ConfigService } from '@kbn/config'; import { CriticalError } from '../errors'; +const ignoredPaths = ['dev.', 'elastic.apm.']; + +const invalidConfigExitCode = 78; +const legacyInvalidConfigExitCode = 64; + export async function ensureValidConfiguration(configService: ConfigService) { - await configService.validate(); + try { + await configService.validate(); + } catch (e) { + throw new CriticalError(e.message, 'InvalidConfig', invalidConfigExitCode, e); + } - const unusedConfigKeys = await configService.getUnusedPaths(); + const unusedPaths = await configService.getUnusedPaths(); + const unusedConfigKeys = unusedPaths.filter((unusedPath) => { + return !ignoredPaths.some((ignoredPath) => unusedPath.startsWith(ignoredPath)); + }); if (unusedConfigKeys.length > 0) { const message = `Unknown configuration key(s): ${unusedConfigKeys .map((key) => `"${key}"`) .join(', ')}. Check for spelling errors and ensure that expected plugins are installed.`; - throw new InvalidConfigurationError(message); - } -} - -class InvalidConfigurationError extends CriticalError { - constructor(message: string) { - super(message, 'InvalidConfig', 64); - Object.setPrototypeOf(this, InvalidConfigurationError.prototype); + throw new CriticalError(message, 'InvalidConfig', legacyInvalidConfigExitCode); } } diff --git a/src/core/server/dev/dev_config.ts b/src/core/server/dev/dev_config.ts deleted file mode 100644 index 2fec778d85713..0000000000000 --- a/src/core/server/dev/dev_config.ts +++ /dev/null @@ -1,16 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { schema } from '@kbn/config-schema'; - -export const config = { - path: 'dev', - // dev configuration is validated by the dev cli. - // we only need to register the `dev` schema to avoid failing core's config validation - schema: schema.object({}, { unknowns: 'ignore' }), -}; diff --git a/src/core/server/dev/index.ts b/src/core/server/dev/index.ts deleted file mode 100644 index 70257d2a5e6c5..0000000000000 --- a/src/core/server/dev/index.ts +++ /dev/null @@ -1,9 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -export { config } from './dev_config'; diff --git a/src/core/server/server.ts b/src/core/server/server.ts index b34d7fec3dcbf..45d11f9013fed 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -36,7 +36,6 @@ import { config as cspConfig } from './csp'; import { config as elasticsearchConfig } from './elasticsearch'; import { config as httpConfig } from './http'; import { config as loggingConfig } from './logging'; -import { config as devConfig } from './dev'; import { config as kibanaConfig } from './kibana_config'; import { savedObjectsConfig, savedObjectsMigrationConfig } from './saved_objects'; import { config as uiSettingsConfig } from './ui_settings'; @@ -303,7 +302,6 @@ export class Server { loggingConfig, httpConfig, pluginsConfig, - devConfig, kibanaConfig, savedObjectsConfig, savedObjectsMigrationConfig,