From 6cf59182d16e02f10b64a6905791e4d7e80ad2ac Mon Sep 17 00:00:00 2001 From: Cristina Amico Date: Mon, 19 Feb 2024 11:29:13 +0100 Subject: [PATCH] [Fleet] Introduce feature flag disabling KQL validation in search boxes (#176806) Closes https://github.com/elastic/kibana/issues/176760 ## Summary Introduce a feature flag `enableStrictKQLValidation: false` to disable KQL validation in search boxes. This will allow free text queries in KQL searchboxes until it's decided what to do with the validation. ### Checklist - [ ] [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 --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Julia Bardi <90178898+juliaElastic@users.noreply.github.com> --- .../fleet/common/experimental_features.ts | 1 + .../fleet/server/routes/utils/filter_utils.ts | 7 +++ .../utils/filter_utils_real_queries.test.ts | 43 +++++++++++++++++++ .../apis/agent_policy/agent_policy.ts | 4 +- .../fleet_api_integration/apis/agents/list.ts | 4 +- .../apis/agents/status.ts | 4 +- .../apis/agents/update_agent_tags.ts | 4 +- .../apis/enrollment_api_keys/crud.ts | 4 +- .../apis/package_policy/get.ts | 4 +- .../test/fleet_api_integration/config.base.ts | 1 + 10 files changed, 64 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/fleet/common/experimental_features.ts b/x-pack/plugins/fleet/common/experimental_features.ts index 0c4a6b91fdb83..12fd089636926 100644 --- a/x-pack/plugins/fleet/common/experimental_features.ts +++ b/x-pack/plugins/fleet/common/experimental_features.ts @@ -26,6 +26,7 @@ export const allowedExperimentalValues = Object.freeze>( outputSecretsStorage: true, remoteESOutput: true, agentless: false, + enableStrictKQLValidation: false, }); type ExperimentalConfigKeys = Array; diff --git a/x-pack/plugins/fleet/server/routes/utils/filter_utils.ts b/x-pack/plugins/fleet/server/routes/utils/filter_utils.ts index 4b34bd788d29a..fac6e40024e60 100644 --- a/x-pack/plugins/fleet/server/routes/utils/filter_utils.ts +++ b/x-pack/plugins/fleet/server/routes/utils/filter_utils.ts @@ -9,6 +9,8 @@ import { get } from 'lodash'; import * as esKuery from '@kbn/es-query'; import type { IndexMapping } from '@kbn/core-saved-objects-base-server-internal'; +import { appContextService } from '../../services/app_context'; + type KueryNode = any; const astFunctionType = ['is', 'range', 'nested']; @@ -223,7 +225,12 @@ export const validateKuery = ( ) => { let isValid = true; let error: string | undefined; + const { enableStrictKQLValidation } = appContextService.getExperimentalFeatures(); + // Skip validation when enableStrictKQLValidation is disabled + if (!enableStrictKQLValidation) { + return { isValid, error }; + } if (!kuery) { isValid = true; } diff --git a/x-pack/plugins/fleet/server/routes/utils/filter_utils_real_queries.test.ts b/x-pack/plugins/fleet/server/routes/utils/filter_utils_real_queries.test.ts index 73712ebb72750..122a8d7a7ddc7 100644 --- a/x-pack/plugins/fleet/server/routes/utils/filter_utils_real_queries.test.ts +++ b/x-pack/plugins/fleet/server/routes/utils/filter_utils_real_queries.test.ts @@ -19,8 +19,13 @@ import { import { FLEET_ENROLLMENT_API_PREFIX } from '../../../common/constants'; +import { appContextService } from '../../services/app_context'; + import { validateFilterKueryNode, validateKuery } from './filter_utils'; +jest.mock('../../services/app_context'); +const mockedAppContextService = appContextService as jest.Mocked; + describe('ValidateFilterKueryNode validates real kueries through KueryNode', () => { describe('Agent policies', () => { it('Search by data_output_id', async () => { @@ -507,6 +512,14 @@ describe('ValidateFilterKueryNode validates real kueries through KueryNode', () }); describe('validateKuery validates real kueries', () => { + beforeEach(() => { + mockedAppContextService.getExperimentalFeatures.mockReturnValue({ + enableStrictKQLValidation: true, + }); + }); + afterEach(() => { + mockedAppContextService.getExperimentalFeatures.mockReset(); + }); describe('Agent policies', () => { it('Search by data_output_id', async () => { const validationObj = validateKuery( @@ -832,4 +845,34 @@ describe('validateKuery validates real kueries', () => { expect(validationObj?.error).toEqual(`KQLSyntaxError: Invalid key`); }); }); + describe('Feature flag enableStrictKQLValidation', () => { + beforeEach(() => { + mockedAppContextService.getExperimentalFeatures.mockReturnValue({ + enableStrictKQLValidation: false, + }); + }); + + it('Allows to skip validation for a free text query', async () => { + const validationObj = validateKuery(`test`, [AGENTS_PREFIX], AGENT_MAPPINGS, true); + expect(validationObj?.isValid).toEqual(true); + expect(validationObj?.error).toEqual(undefined); + }); + + it('Allows to skip validation for a catch all query', async () => { + const validationObj = validateKuery(`*`, [AGENTS_PREFIX], AGENT_MAPPINGS, true); + expect(validationObj?.isValid).toEqual(true); + expect(validationObj?.error).toEqual(undefined); + }); + + it('Allows to skip validation for a disallowed query too', async () => { + const validationObj = validateKuery( + `non_existent_parameter: 'test_id'`, + [AGENTS_PREFIX], + AGENT_MAPPINGS, + true + ); + expect(validationObj?.isValid).toEqual(true); + expect(validationObj?.error).toEqual(undefined); + }); + }); }); diff --git a/x-pack/test/fleet_api_integration/apis/agent_policy/agent_policy.ts b/x-pack/test/fleet_api_integration/apis/agent_policy/agent_policy.ts index a8a22bd66ed0c..b537f836acfe7 100644 --- a/x-pack/test/fleet_api_integration/apis/agent_policy/agent_policy.ts +++ b/x-pack/test/fleet_api_integration/apis/agent_policy/agent_policy.ts @@ -80,14 +80,14 @@ export default function (providerContext: FtrProviderContext) { .expect(200); }); - it('should return 400 if passed kuery is not correct', async () => { + it('with enableStrictKQLValidation should return 400 if passed kuery is not correct', async () => { await supertest .get(`/api/fleet/agent_policies?kuery=ingest-agent-policies.non_existent_parameter:test`) .set('kbn-xsrf', 'xxxx') .expect(400); }); - it('should return 400 if passed kuery is invalid', async () => { + it('with enableStrictKQLValidation should return 400 if passed kuery is invalid', async () => { await supertest .get(`/api/fleet/agent_policies?kuery='test%3A'`) .set('kbn-xsrf', 'xxxx') diff --git a/x-pack/test/fleet_api_integration/apis/agents/list.ts b/x-pack/test/fleet_api_integration/apis/agents/list.ts index 91796d5a9b9dd..55d9373c0bd95 100644 --- a/x-pack/test/fleet_api_integration/apis/agents/list.ts +++ b/x-pack/test/fleet_api_integration/apis/agents/list.ts @@ -87,11 +87,11 @@ export default function ({ getService }: FtrProviderContext) { .expect(200); }); - it('should return a 400 when given an invalid "kuery" value', async () => { + it('with enableStrictKQLValidation should return a 400 when given an invalid "kuery" value', async () => { await supertest.get(`/api/fleet/agents?kuery='test%3A'`).expect(400); }); - it('should return 400 if passed kuery has non existing parameters', async () => { + it('with enableStrictKQLValidation should return 400 if passed kuery has non existing parameters', async () => { await supertest .get(`/api/fleet/agents?kuery=fleet-agents.non_existent_parameter:healthy`) .set('kbn-xsrf', 'xxxx') diff --git a/x-pack/test/fleet_api_integration/apis/agents/status.ts b/x-pack/test/fleet_api_integration/apis/agents/status.ts index 742d5d6d3be7d..7c1134d055bc8 100644 --- a/x-pack/test/fleet_api_integration/apis/agents/status.ts +++ b/x-pack/test/fleet_api_integration/apis/agents/status.ts @@ -327,14 +327,14 @@ export default function ({ getService }: FtrProviderContext) { .expect(200); }); - it('should return 400 if passed kuery has non existing parameters', async () => { + it('with enableStrictKQLValidation should return 400 if passed kuery has non existing parameters', async () => { await supertest .get(`/api/fleet/agent_status?kuery=fleet-agents.non_existent_parameter:healthy`) .set('kbn-xsrf', 'xxxx') .expect(400); }); - it('should return 400 if passed kuery is not correct', async () => { + it('with enableStrictKQLValidation should return 400 if passed kuery is not correct', async () => { await supertest .get(`/api/fleet/agent_status?kuery='test%3A'`) .set('kbn-xsrf', 'xxxx') diff --git a/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts b/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts index 0292e62f60896..592f2d964b748 100644 --- a/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts +++ b/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts @@ -156,14 +156,14 @@ export default function (providerContext: FtrProviderContext) { .expect(200); }); - it('should return 400 if the passed kuery is not correct', async () => { + it('with enableStrictKQLValidation should return 400 if the passed kuery is not correct ', async () => { await supertest .get(`/api/fleet/agents?kuery=fleet-agents.non_existent_parameter:existingTag`) .set('kbn-xsrf', 'xxxx') .expect(400); }); - it('should return 400 if the passed kuery is invalid', async () => { + it('with enableStrictKQLValidation should return 400 if the passed kuery is invalid', async () => { await supertest .get(`/api/fleet/agents?kuery='test%3A'`) .set('kbn-xsrf', 'xxxx') diff --git a/x-pack/test/fleet_api_integration/apis/enrollment_api_keys/crud.ts b/x-pack/test/fleet_api_integration/apis/enrollment_api_keys/crud.ts index 087218bf4727e..9597c3ac38e68 100644 --- a/x-pack/test/fleet_api_integration/apis/enrollment_api_keys/crud.ts +++ b/x-pack/test/fleet_api_integration/apis/enrollment_api_keys/crud.ts @@ -73,7 +73,7 @@ export default function (providerContext: FtrProviderContext) { .expect(200); }); - it('should return 400 if the passed kuery is not correct', async () => { + it('with enableStrictKQLValidation should return 400 if the passed kuery is not correct', async () => { await supertest .get( `/api/fleet/enrollment_api_keys?kuery=fleet-enrollment-api-keys.non_existent_parameter:test` @@ -82,7 +82,7 @@ export default function (providerContext: FtrProviderContext) { .expect(400); }); - it('should return 400 if the passed kuery is invalid', async () => { + it('with enableStrictKQLValidation should return 400 if the passed kuery is invalid', async () => { await supertest .get(`/api/fleet/enrollment_api_keys?kuery='test%3A'`) .set('kbn-xsrf', 'xxxx') diff --git a/x-pack/test/fleet_api_integration/apis/package_policy/get.ts b/x-pack/test/fleet_api_integration/apis/package_policy/get.ts index 6801c3908e658..b5010543dad38 100644 --- a/x-pack/test/fleet_api_integration/apis/package_policy/get.ts +++ b/x-pack/test/fleet_api_integration/apis/package_policy/get.ts @@ -492,7 +492,7 @@ export default function (providerContext: FtrProviderContext) { .expect(400); }); - it('should return 400 if the passed kuery is not correct', async () => { + it('with enableStrictKQLValidation should return 400 if the passed kuery is not correct', async () => { await supertest .get( `/api/fleet/package_policies?kuery=ingest-package-policies.non_existent_parameter:test` @@ -501,7 +501,7 @@ export default function (providerContext: FtrProviderContext) { .expect(400); }); - it('should return 400 if the passed kuery is invalid', async () => { + it('with enableStrictKQLValidation should return 400 if the passed kuery is invalid', async () => { await supertest .get(`/api/fleet/package_policies?kuery='test%3A'`) .set('kbn-xsrf', 'xxxx') diff --git a/x-pack/test/fleet_api_integration/config.base.ts b/x-pack/test/fleet_api_integration/config.base.ts index 5dfea767f98da..ccdf9d03ba3ae 100644 --- a/x-pack/test/fleet_api_integration/config.base.ts +++ b/x-pack/test/fleet_api_integration/config.base.ts @@ -74,6 +74,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) { `--xpack.fleet.enableExperimental=${JSON.stringify([ 'outputSecretsStorage', 'agentTamperProtectionEnabled', + 'enableStrictKQLValidation', ])}`, `--logging.loggers=${JSON.stringify([ ...getKibanaCliLoggers(xPackAPITestsConfig.get('kbnTestServer.serverArgs')),