From 4cd937642248311d4dd41ec78b3f48d41a0213ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Mon, 14 Apr 2025 17:52:51 +0200 Subject: [PATCH] [Config stripUnknowns] Skip compatible mode when running in CI (#217536) --- .buildkite/ftr_chat_serverless_configs.yml | 1 + .buildkite/ftr_oblt_serverless_configs.yml | 1 + .buildkite/ftr_search_serverless_configs.yml | 1 + .../ftr_security_serverless_configs.yml | 1 + .github/CODEOWNERS | 6 ++ config/serverless.chat.yml | 5 +- .../root/server-internal/src/core_config.ts | 10 +++ .../root/server-internal/src/server.test.ts | 70 +++++++++++++++++++ .../root/server-internal/src/server.ts | 5 +- .../plugins/search_connectors/kibana.jsonc | 5 +- .../plugins/search_homepage/kibana.jsonc | 3 +- .../config.config_compat_mode.ts | 33 +++++++++ .../home_page.ts | 26 +++++++ .../strip_unknown_config_workaround/index.ts | 19 +++++ .../config.config_compat_mode.ts | 33 +++++++++ .../config.config_compat_mode.ts | 33 +++++++++ .../config.config_compat_mode.ts | 33 +++++++++ 17 files changed, 276 insertions(+), 9 deletions(-) create mode 100644 x-pack/test_serverless/functional/test_suites/chat/common_configs/config.config_compat_mode.ts create mode 100644 x-pack/test_serverless/functional/test_suites/common/strip_unknown_config_workaround/home_page.ts create mode 100644 x-pack/test_serverless/functional/test_suites/common/strip_unknown_config_workaround/index.ts create mode 100644 x-pack/test_serverless/functional/test_suites/observability/common_configs/config.config_compat_mode.ts create mode 100644 x-pack/test_serverless/functional/test_suites/search/common_configs/config.config_compat_mode.ts create mode 100644 x-pack/test_serverless/functional/test_suites/security/common_configs/config.config_compat_mode.ts diff --git a/.buildkite/ftr_chat_serverless_configs.yml b/.buildkite/ftr_chat_serverless_configs.yml index 1b4ba1f66fe7..e4033908ba6b 100644 --- a/.buildkite/ftr_chat_serverless_configs.yml +++ b/.buildkite/ftr_chat_serverless_configs.yml @@ -8,4 +8,5 @@ enabled: - x-pack/test_serverless/api_integration/test_suites/chat/common_configs/config.group1.ts - x-pack/test_serverless/functional/test_suites/chat/config.ts - x-pack/test_serverless/functional/test_suites/chat/config.feature_flags.ts + - x-pack/test_serverless/functional/test_suites/chat/common_configs/config.config_compat_mode.ts - x-pack/test_serverless/functional/test_suites/chat/common_configs/config.group1.ts diff --git a/.buildkite/ftr_oblt_serverless_configs.yml b/.buildkite/ftr_oblt_serverless_configs.yml index 2f5cd2f16747..8910813c63cf 100644 --- a/.buildkite/ftr_oblt_serverless_configs.yml +++ b/.buildkite/ftr_oblt_serverless_configs.yml @@ -18,6 +18,7 @@ enabled: - x-pack/test_serverless/functional/test_suites/observability/config.feature_flags.ts - x-pack/test_serverless/functional/test_suites/observability/config.saved_objects_management.ts - x-pack/test_serverless/functional/test_suites/observability/config.context_awareness.ts + - x-pack/test_serverless/functional/test_suites/observability/common_configs/config.config_compat_mode.ts - x-pack/test_serverless/functional/test_suites/observability/common_configs/config.group1.ts - x-pack/test_serverless/functional/test_suites/observability/common_configs/config.group2.ts - x-pack/test_serverless/functional/test_suites/observability/common_configs/config.group3.ts diff --git a/.buildkite/ftr_search_serverless_configs.yml b/.buildkite/ftr_search_serverless_configs.yml index 20f3e8165e1e..0447650a9cda 100644 --- a/.buildkite/ftr_search_serverless_configs.yml +++ b/.buildkite/ftr_search_serverless_configs.yml @@ -12,6 +12,7 @@ enabled: - x-pack/test_serverless/functional/test_suites/search/config.screenshots.ts - x-pack/test_serverless/functional/test_suites/search/config.saved_objects_management.ts - x-pack/test_serverless/functional/test_suites/search/config.context_awareness.ts + - x-pack/test_serverless/functional/test_suites/search/common_configs/config.config_compat_mode.ts - x-pack/test_serverless/functional/test_suites/search/common_configs/config.group1.ts - x-pack/test_serverless/functional/test_suites/search/common_configs/config.group2.ts - x-pack/test_serverless/functional/test_suites/search/common_configs/config.group3.ts diff --git a/.buildkite/ftr_security_serverless_configs.yml b/.buildkite/ftr_security_serverless_configs.yml index 84f3c29ccfda..db5d959d007f 100644 --- a/.buildkite/ftr_security_serverless_configs.yml +++ b/.buildkite/ftr_security_serverless_configs.yml @@ -40,6 +40,7 @@ enabled: - x-pack/test_serverless/functional/test_suites/security/config.saved_objects_management.ts - x-pack/test_serverless/functional/test_suites/security/config.context_awareness.ts - x-pack/test_serverless/functional/test_suites/security/config.examples.context_awareness.ts + - x-pack/test_serverless/functional/test_suites/security/common_configs/config.config_compat_mode.ts - x-pack/test_serverless/functional/test_suites/security/common_configs/config.group1.ts - x-pack/test_serverless/functional/test_suites/security/common_configs/config.group2.ts - x-pack/test_serverless/functional/test_suites/security/common_configs/config.group3.ts diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index cffe7caea27a..6208441cf7f6 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -2760,10 +2760,16 @@ docs/settings-gen @elastic/platform-docs /x-pack/plugins/**/kibana.jsonc @elastic/kibana-core /x-pack/platform/plugins/shared/**/kibana.jsonc @elastic/kibana-core /x-pack/platform/plugins/private/**/kibana.jsonc @elastic/kibana-core +/x-pack//solutions/observability/plugins/**/kibana.jsonc @elastic/kibana-core +/x-pack//solutions/search/plugins/**/kibana.jsonc @elastic/kibana-core +/x-pack//solutions/security/plugins/**/kibana.jsonc @elastic/kibana-core + /x-pack/solutions/observability/plugins/**/kibana.jsonc @elastic/kibana-core /x-pack/solutions/search/plugins/**/kibana.jsonc @elastic/kibana-core /x-pack/solutions/security/plugins/**/kibana.jsonc @elastic/kibana-core +/x-pack/test_serverless/functional/test_suites/common/strip_unknown_config_workaround @elastic/kibana-core + # Plugin manifests for test plugins /src/platform/test/**/kibana.jsonc @elastic/appex-qa @elastic/kibana-core /x-pack/test/**/kibana.jsonc @elastic/appex-qa @elastic/kibana-core diff --git a/config/serverless.chat.yml b/config/serverless.chat.yml index 7bf8860f1ee6..32d20099890b 100644 --- a/config/serverless.chat.yml +++ b/config/serverless.chat.yml @@ -21,7 +21,6 @@ xpack.spaces.maxSpaces: 1 ## Disable plugins that belong to other solutions xpack.apm.enabled: false -xpack.entities_data_access.enabled: false xpack.infra.enabled: false xpack.inventory.enabled: false xpack.observability.enabled: false @@ -35,8 +34,8 @@ xpack.legacy_uptime.enabled: false xpack.ux.enabled: false xpack.search.enabled: false xpack.searchAssistant.enabled: false -xpack.search.connectors.enabled: false -xpack.search.homepage.enabled: false +xpack.searchConnectors.enabled: false +xpack.searchHomepage.enabled: false xpack.searchIndices.enabled: false xpack.searchInferenceEndpoints.enabled: false xpack.searchNotebooks.enabled: false diff --git a/src/core/packages/root/server-internal/src/core_config.ts b/src/core/packages/root/server-internal/src/core_config.ts index 1290f6e342b0..f018f0f92d4c 100644 --- a/src/core/packages/root/server-internal/src/core_config.ts +++ b/src/core/packages/root/server-internal/src/core_config.ts @@ -14,6 +14,16 @@ const coreConfigSchema = schema.object({ lifecycle: schema.object({ disablePreboot: schema.boolean({ defaultValue: false }), }), + /** + * If the config validation fails, this setting allows retrying it with `stripUnknownKeys: true`, which removes any + * unknown config keys from the resulting validated config object. + * + * This is an escape hatch that should be used only + * if necessary. The setting is expected to be false in the classic offering and during dev and CI times. + * However, on Serverless, we'd like to set it to true to avoid bootlooping in case of any temporary misalignment + * between our kibana-controller and the Kibana versions. + */ + enableStripUnknownConfigWorkaround: schema.boolean({ defaultValue: false }), }); export type CoreConfigType = TypeOf; diff --git a/src/core/packages/root/server-internal/src/server.test.ts b/src/core/packages/root/server-internal/src/server.test.ts index 49328931e181..6d60ae47cab5 100644 --- a/src/core/packages/root/server-internal/src/server.test.ts +++ b/src/core/packages/root/server-internal/src/server.test.ts @@ -289,6 +289,76 @@ test(`doesn't preboot core services if config validation fails`, async () => { expect(mockPrebootService.preboot).not.toHaveBeenCalled(); }); +describe('stripUnknownsWorkaround', () => { + beforeEach(async () => { + mockEnsureValidConfiguration.mockImplementation(() => { + throw new Error('Unknown configuration keys'); + }); + }); + + test(`aborts on the first validation attempt if enableStripUnknownConfigWorkaround is not true`, async () => { + mockConfigService.atPath.mockReturnValue( + new BehaviorSubject({ lifecycle: { disablePreboot: true } }) + ); + const server = new Server(rawConfigService, env, logger); + await server.preboot(); + await expect(server.setup()).rejects.toThrowErrorMatchingInlineSnapshot( + `"Unknown configuration keys"` + ); + expect(mockEnsureValidConfiguration).toHaveBeenCalledTimes(1); + }); + + test(`tries again with stripUnknownKeys: true when enableStripUnknownConfigWorkaround is true`, async () => { + mockConfigService.atPath.mockReturnValue( + new BehaviorSubject({ + lifecycle: { disablePreboot: true }, + enableStripUnknownConfigWorkaround: true, + }) + ); + + const server = new Server(rawConfigService, env, logger); + await server.preboot(); + await expect(server.setup()).rejects.toThrowErrorMatchingInlineSnapshot( + `"Unknown configuration keys"` + ); + expect(mockEnsureValidConfiguration).toHaveBeenCalledTimes(2); + expect(mockEnsureValidConfiguration).toHaveBeenNthCalledWith( + 2, + expect.anything(), + expect.objectContaining({ stripUnknownKeys: true }) + ); + expect(logger.get('config-validation').error).toHaveBeenCalledTimes(0); // No error logged as it lets the validation fail + }); + + test(`if the 2nd validation succeeds, it logs an error to highlight that it's running in compat mode`, async () => { + mockEnsureValidConfiguration.mockImplementation(() => ({})); // Success by default + mockEnsureValidConfiguration.mockRejectedValueOnce(new Error('Unknown configuration keys')); // Fail the first time + + mockConfigService.atPath.mockReturnValue( + new BehaviorSubject({ + lifecycle: { disablePreboot: true }, + enableStripUnknownConfigWorkaround: true, + }) + ); + + const server = new Server(rawConfigService, env, logger); + await server.preboot(); + await expect(server.setup()).resolves.toBeDefined(); + expect(mockEnsureValidConfiguration).toHaveBeenCalledTimes(2); + expect(mockEnsureValidConfiguration).toHaveBeenNthCalledWith( + 2, + expect.anything(), + expect.objectContaining({ stripUnknownKeys: true }) + ); + expect(logger.get('config-validation').error).toHaveBeenCalledTimes(1); + expect(logger.get('config-validation').error).toHaveBeenCalledWith( + expect.stringContaining( + 'Strict config validation failed! Extra unknown keys removed in Serverless-compatible mode. Original error' + ) + ); + }); +}); + test('migrator-only node throws exception during start', async () => { rawConfigService.getConfig$.mockReturnValue( new BehaviorSubject({ node: { roles: ['migrator'] } }) diff --git a/src/core/packages/root/server-internal/src/server.ts b/src/core/packages/root/server-internal/src/server.ts index 432a75535088..d96a0de4c0db 100644 --- a/src/core/packages/root/server-internal/src/server.ts +++ b/src/core/packages/root/server-internal/src/server.ts @@ -521,7 +521,10 @@ export class Server { try { await ensureValidConfiguration(this.configService); } catch (validationError) { - if (this.env.packageInfo.buildFlavor !== 'serverless') { + const config = await firstValueFrom( + this.configService.atPath(coreConfig.path) + ); + if (!config.enableStripUnknownConfigWorkaround) { throw validationError; } // When running on serverless, we may allow unknown keys, but stripping them from the final config object. diff --git a/x-pack/solutions/search/plugins/search_connectors/kibana.jsonc b/x-pack/solutions/search/plugins/search_connectors/kibana.jsonc index 06ed791f2831..66fb694d720a 100644 --- a/x-pack/solutions/search/plugins/search_connectors/kibana.jsonc +++ b/x-pack/solutions/search/plugins/search_connectors/kibana.jsonc @@ -14,8 +14,7 @@ "browser": true, "configPath": [ "xpack", - "search", - "connectors" + "searchConnectors" ], "requiredPlugins": [ "licensing", @@ -26,4 +25,4 @@ "cloud" ] } -} \ No newline at end of file +} diff --git a/x-pack/solutions/search/plugins/search_homepage/kibana.jsonc b/x-pack/solutions/search/plugins/search_homepage/kibana.jsonc index 7120432e1ef7..163092d7b454 100644 --- a/x-pack/solutions/search/plugins/search_homepage/kibana.jsonc +++ b/x-pack/solutions/search/plugins/search_homepage/kibana.jsonc @@ -10,8 +10,7 @@ "browser": true, "configPath": [ "xpack", - "search", - "homepage" + "searchHomepage" ], "requiredPlugins": [ "share", diff --git a/x-pack/test_serverless/functional/test_suites/chat/common_configs/config.config_compat_mode.ts b/x-pack/test_serverless/functional/test_suites/chat/common_configs/config.config_compat_mode.ts new file mode 100644 index 000000000000..40d80af40a8b --- /dev/null +++ b/x-pack/test_serverless/functional/test_suites/chat/common_configs/config.config_compat_mode.ts @@ -0,0 +1,33 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { FtrConfigProviderContext } from '@kbn/test'; + +export default async function ({ readConfigFile }: FtrConfigProviderContext) { + const baseTestConfig = await readConfigFile(require.resolve('../config.ts')); + + return { + ...baseTestConfig.getAll(), + testFiles: [require.resolve('../../common/strip_unknown_config_workaround')], + kbnTestServer: { + ...baseTestConfig.get('kbnTestServer'), + serverArgs: [ + ...baseTestConfig.get('kbnTestServer.serverArgs'), + + // Enable config compatibility mode. This is how it is run in the actual serverless deployments. + // However, in dev mode and CI, we run Kibana with this flag disabled to catch unintended misconfigurations. + '--core.enableStripUnknownConfigWorkaround=true', + + // Intentionally invalid setting, setting a key that doesn't exist + '--elasticsearch.unknown.key=1', + ], + }, + junit: { + reportName: 'Serverless Chat Functional Tests - Common Config Compat Mode', + }, + }; +} diff --git a/x-pack/test_serverless/functional/test_suites/common/strip_unknown_config_workaround/home_page.ts b/x-pack/test_serverless/functional/test_suites/common/strip_unknown_config_workaround/home_page.ts new file mode 100644 index 000000000000..377cd486d359 --- /dev/null +++ b/x-pack/test_serverless/functional/test_suites/common/strip_unknown_config_workaround/home_page.ts @@ -0,0 +1,26 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { FtrProviderContext } from '../../../ftr_provider_context'; + +export default function ({ getPageObject, getService }: FtrProviderContext) { + const svlCommonPage = getPageObject('svlCommonPage'); + + describe('can browse Kibana', function () { + before(async () => { + await svlCommonPage.loginAsViewer(); + }); + + it('has project header', async () => { + await svlCommonPage.assertProjectHeaderExists(); + }); + + // We don't have a logger service to check the logs at this stage. + // However, it's already tested in the unit tests. + // it('Kibana logs "Strict config validation failed!"'); + }); +} diff --git a/x-pack/test_serverless/functional/test_suites/common/strip_unknown_config_workaround/index.ts b/x-pack/test_serverless/functional/test_suites/common/strip_unknown_config_workaround/index.ts new file mode 100644 index 000000000000..0ca5583baf0c --- /dev/null +++ b/x-pack/test_serverless/functional/test_suites/common/strip_unknown_config_workaround/index.ts @@ -0,0 +1,19 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { FtrProviderContext } from '../../../ftr_provider_context'; + +export default function ({ loadTestFile }: FtrProviderContext) { + // this is a super smoke test just to validate that Kibana successfully starts even when + // the configuration includes invalid config (excess config keys). + // Use in combination of FTR configs that inject such excess keys. + describe('Serverless Common UI - Strip Unknown Config Workaround', function () { + this.tags(['skipMKI', 'esGate']); + + loadTestFile(require.resolve('./home_page')); + }); +} diff --git a/x-pack/test_serverless/functional/test_suites/observability/common_configs/config.config_compat_mode.ts b/x-pack/test_serverless/functional/test_suites/observability/common_configs/config.config_compat_mode.ts new file mode 100644 index 000000000000..63ae8875b488 --- /dev/null +++ b/x-pack/test_serverless/functional/test_suites/observability/common_configs/config.config_compat_mode.ts @@ -0,0 +1,33 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { FtrConfigProviderContext } from '@kbn/test'; + +export default async function ({ readConfigFile }: FtrConfigProviderContext) { + const baseTestConfig = await readConfigFile(require.resolve('../config.ts')); + + return { + ...baseTestConfig.getAll(), + testFiles: [require.resolve('../../common/strip_unknown_config_workaround')], + kbnTestServer: { + ...baseTestConfig.get('kbnTestServer'), + serverArgs: [ + ...baseTestConfig.get('kbnTestServer.serverArgs'), + + // Enable config compatibility mode. This is how it is run in the actual serverless deployments. + // However, in dev mode and CI, we run Kibana with this flag disabled to catch unintended misconfigurations. + '--core.enableStripUnknownConfigWorkaround=true', + + // Intentionally invalid setting, setting a key that doesn't exist + '--elasticsearch.unknown.key=1', + ], + }, + junit: { + reportName: 'Serverless Observability Functional Tests - Common Config Compat Mode', + }, + }; +} diff --git a/x-pack/test_serverless/functional/test_suites/search/common_configs/config.config_compat_mode.ts b/x-pack/test_serverless/functional/test_suites/search/common_configs/config.config_compat_mode.ts new file mode 100644 index 000000000000..bf3690276bd5 --- /dev/null +++ b/x-pack/test_serverless/functional/test_suites/search/common_configs/config.config_compat_mode.ts @@ -0,0 +1,33 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { FtrConfigProviderContext } from '@kbn/test'; + +export default async function ({ readConfigFile }: FtrConfigProviderContext) { + const baseTestConfig = await readConfigFile(require.resolve('../config.ts')); + + return { + ...baseTestConfig.getAll(), + testFiles: [require.resolve('../../common/strip_unknown_config_workaround')], + kbnTestServer: { + ...baseTestConfig.get('kbnTestServer'), + serverArgs: [ + ...baseTestConfig.get('kbnTestServer.serverArgs'), + + // Enable config compatibility mode. This is how it is run in the actual serverless deployments. + // However, in dev mode and CI, we run Kibana with this flag disabled to catch unintended misconfigurations. + '--core.enableStripUnknownConfigWorkaround=true', + + // Intentionally invalid setting, setting a key that doesn't exist + '--elasticsearch.unknown.key=1', + ], + }, + junit: { + reportName: 'Serverless Search Functional Tests - Common Config Compat Mode', + }, + }; +} diff --git a/x-pack/test_serverless/functional/test_suites/security/common_configs/config.config_compat_mode.ts b/x-pack/test_serverless/functional/test_suites/security/common_configs/config.config_compat_mode.ts new file mode 100644 index 000000000000..d637b93a6f5c --- /dev/null +++ b/x-pack/test_serverless/functional/test_suites/security/common_configs/config.config_compat_mode.ts @@ -0,0 +1,33 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { FtrConfigProviderContext } from '@kbn/test'; + +export default async function ({ readConfigFile }: FtrConfigProviderContext) { + const baseTestConfig = await readConfigFile(require.resolve('../config.ts')); + + return { + ...baseTestConfig.getAll(), + testFiles: [require.resolve('../../common/strip_unknown_config_workaround')], + kbnTestServer: { + ...baseTestConfig.get('kbnTestServer'), + serverArgs: [ + ...baseTestConfig.get('kbnTestServer.serverArgs'), + + // Enable config compatibility mode. This is how it is run in the actual serverless deployments. + // However, in dev mode and CI, we run Kibana with this flag disabled to catch unintended misconfigurations. + '--core.enableStripUnknownConfigWorkaround=true', + + // Intentionally invalid setting, setting a key that doesn't exist + '--elasticsearch.unknown.key=1', + ], + }, + junit: { + reportName: 'Serverless Security Functional Tests - Common Config Compat Mode', + }, + }; +}