Skip to content

Commit

Permalink
[Fleet] Introduce feature flag disabling KQL validation in search box…
Browse files Browse the repository at this point in the history
…es (#176806)

Closes #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 <[email protected]>
Co-authored-by: Julia Bardi <[email protected]>
  • Loading branch information
3 people authored Feb 19, 2024
1 parent 582de67 commit 6cf5918
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 12 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/common/experimental_features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export const allowedExperimentalValues = Object.freeze<Record<string, boolean>>(
outputSecretsStorage: true,
remoteESOutput: true,
agentless: false,
enableStrictKQLValidation: false,
});

type ExperimentalConfigKeys = Array<keyof ExperimentalFeatures>;
Expand Down
7 changes: 7 additions & 0 deletions x-pack/plugins/fleet/server/routes/utils/filter_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof appContextService>;

describe('ValidateFilterKueryNode validates real kueries through KueryNode', () => {
describe('Agent policies', () => {
it('Search by data_output_id', async () => {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
4 changes: 2 additions & 2 deletions x-pack/test/fleet_api_integration/apis/agents/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
4 changes: 2 additions & 2 deletions x-pack/test/fleet_api_integration/apis/agents/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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')
Expand Down
4 changes: 2 additions & 2 deletions x-pack/test/fleet_api_integration/apis/package_policy/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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')
Expand Down
1 change: 1 addition & 0 deletions x-pack/test/fleet_api_integration/config.base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')),
Expand Down

0 comments on commit 6cf5918

Please sign in to comment.