Skip to content

Commit

Permalink
[Fleet] Fix validation errors in KQL queries (elastic#168329)
Browse files Browse the repository at this point in the history 
Related to elastic#167139

## Summary

[Fleet] Fix validation errors in KQL searchboxes

### Testing
- In the agent list searchbox, type the following query:

```
  agent.version : 8.10.0
```

The UI shouldn't display any error

### 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]>
  • Loading branch information
@criamico @kibanamachine
criamico and kibanamachine authored Oct 10, 2023
1 parent aa48e31 commit 7839d51
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 45 deletions.
9 changes: 6 additions & 3 deletions x-pack/plugins/fleet/server/constants/mappings.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
*/

/**
* The mappings declared closely mirror the ones declared in indices and SOs
* But they are only used to perform validation on those endpoints using ListWithKuery
* Whenever a field is added on any of these mappings, make sure to add it here as well
* ATTENTION: New mappings for Fleet are defined in the ElasticSearch repo.
*
* The following mappings declared here closely mirror them
* But they are only used to perform validation on the endpoints using ListWithKuery
* They're needed to perform searches on these mapping trough the KQL searchboxes in the UI.
* Whenever a field is added on any of these mappings in ES, make sure to add it here as well
*/

export const AGENT_POLICY_MAPPINGS = {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/fleet/server/routes/utils/filter_utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ describe('Filter Utils', () => {
},
{
astPath: 'arguments.1',
error: 'The key is empty and needs to be wrapped by a saved object type like foo',
error: 'Invalid key',
isSavedObjectAttr: false,
key: null,
type: null,
Expand Down
54 changes: 27 additions & 27 deletions x-pack/plugins/fleet/server/routes/utils/filter_utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ export const hasFilterKeyError = (
indexMapping: IndexMapping,
skipNormalization?: boolean
): string | null => {
if (key == null) {
return `The key is empty and needs to be wrapped by a saved object type like ${types.join()}`;
if (!key) {
return `Invalid key`;
}
if (!key.includes('.')) {
if (allowedTerms.some((term) => term === key) || fieldDefined(indexMapping, key)) {
Expand All @@ -141,12 +141,11 @@ export const hasFilterKeyError = (
return `This type '${key}' is not allowed`;
} else if (key.includes('.')) {
const keySplit = key.split('.');
if (
keySplit.length <= 1 &&
!fieldDefined(indexMapping, keySplit[0]) &&
!types.includes(keySplit[0])
) {
return `This type '${keySplit[0]}' is not allowed`;
const firstField = keySplit[0];
const hasIndexWrap = types.includes(firstField);

if (keySplit.length <= 1 && !fieldDefined(indexMapping, firstField) && !hasIndexWrap) {
return `This type '${firstField}' is not allowed`;
}
// In some cases we don't want to check about the `attributes` presence
// In that case pass the `skipNormalization` parameter
Expand All @@ -157,34 +156,35 @@ export const hasFilterKeyError = (
return `This key '${key}' does NOT match the filter proposition SavedObjectType.attributes.key`;
}
// Check that the key exists in the mappings
const searchKey =
skipNormalization || keySplit[1] !== 'attributes'
? `${keySplit[0]}.${keySplit.slice(1, keySplit.length).join('.')}`
: `${keySplit[0]}.${keySplit.slice(2, keySplit.length).join('.')}`;
if (
(keySplit.length === 2 && !fieldDefined(indexMapping, keySplit[1])) ||
(keySplit.length === 2 &&
!types.includes(keySplit[0]) &&
!fieldDefined(indexMapping, searchKey)) ||
(keySplit.length > 2 && !fieldDefined(indexMapping, searchKey))
) {
let searchKey = '';
if (keySplit.length === 2) {
searchKey = hasIndexWrap ? keySplit[1] : key;
} else if (keySplit.length > 2) {
searchKey =
skipNormalization || keySplit[1] !== 'attributes'
? `${firstField}.${keySplit.slice(1, keySplit.length).join('.')}`
: `${firstField}.${keySplit.slice(2, keySplit.length).join('.')}`;
}
if (!fieldDefined(indexMapping, searchKey)) {
return `This key '${key}' does NOT exist in ${types.join()} saved object index patterns`;
}
}
return null;
};

const getMappingKey = (key?: string) =>
!!key ? 'properties.' + key.split('.').join('.properties.') : '';

export const fieldDefined = (indexMappings: IndexMapping, key: string): boolean => {
const keySplit = key.split('.');
const shortenedKey = `${keySplit[1]}.${keySplit.slice(2, keySplit.length).join('.')}`;
const mappingKey = 'properties.' + key.split('.').join('.properties.');
const shortenedMappingKey = 'properties.' + shortenedKey.split('.').join('.properties.');

if (get(indexMappings, mappingKey) != null || get(indexMappings, shortenedMappingKey) != null) {
return true;
}
const mappingKey = getMappingKey(key);

if (mappingKey === 'properties.id') {
if (
!!get(indexMappings, mappingKey) ||
!!get(indexMappings, getMappingKey(shortenedKey)) ||
mappingKey === 'properties.id'
) {
return true;
}

Expand All @@ -199,7 +199,7 @@ export const fieldDefined = (indexMappings: IndexMapping, key: string): boolean
mappingKey.lastIndexOf(propertiesAttribute) + `${propertiesAttribute}.`.length
);
const mapping = `${fieldMapping}fields.${fieldType}`;
if (get(indexMappings, mapping) != null) {
if (!!get(indexMappings, mapping)) {
return true;
}

Expand Down
108 changes: 94 additions & 14 deletions x-pack/plugins/fleet/server/routes/utils/filter_utils_real_queries.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { validateFilterKueryNode, validateKuery } from './filter_utils';

describe('ValidateFilterKueryNode validates real kueries through KueryNode', () => {
describe('Agent policies', () => {
it('Test 1 - search by data_output_id', async () => {
it('Search by data_output_id', async () => {
const astFilter = esKuery.fromKueryExpression(
`${AGENT_POLICY_SAVED_OBJECT_TYPE}.data_output_id: test_id`
);
Expand All @@ -44,7 +44,7 @@ describe('ValidateFilterKueryNode validates real kueries through KueryNode', ()
]);
});

it('Test 2 - search by inactivity timeout', async () => {
it('Search by inactivity timeout', async () => {
const astFilter = esKuery.fromKueryExpression(
`${AGENT_POLICY_SAVED_OBJECT_TYPE}.inactivity_timeout:*`
);
Expand All @@ -65,7 +65,7 @@ describe('ValidateFilterKueryNode validates real kueries through KueryNode', ()
]);
});

it('Test 3 - complex query', async () => {
it('Complex query', async () => {
const validationObject = validateFilterKueryNode({
astFilter: esKuery.fromKueryExpression(
`${AGENT_POLICY_SAVED_OBJECT_TYPE}.download_source_id:some_id or (not ${AGENT_POLICY_SAVED_OBJECT_TYPE}.download_source_id:*)`
Expand Down Expand Up @@ -93,7 +93,7 @@ describe('ValidateFilterKueryNode validates real kueries through KueryNode', ()
]);
});

it('Test 4', async () => {
it('Test another complex query', async () => {
const astFilter = esKuery.fromKueryExpression(
`${AGENT_POLICY_SAVED_OBJECT_TYPE}.data_output_id: test_id or ${AGENT_POLICY_SAVED_OBJECT_TYPE}.monitoring_output_id: test_id or (not ${AGENT_POLICY_SAVED_OBJECT_TYPE}.data_output_id:*)`
);
Expand Down Expand Up @@ -129,7 +129,7 @@ describe('ValidateFilterKueryNode validates real kueries through KueryNode', ()
]);
});

it('Test 5 - returns error if the attribute does not exist', async () => {
it('Returns error if the attribute does not exist', async () => {
const astFilter = esKuery.fromKueryExpression(
`${AGENT_POLICY_SAVED_OBJECT_TYPE}.package_policies:test_id_1 or ${AGENT_POLICY_SAVED_OBJECT_TYPE}.package_policies:test_id_2`
);
Expand Down Expand Up @@ -442,9 +442,47 @@ describe('ValidateFilterKueryNode validates real kueries through KueryNode', ()
},
]);
});
it('Search by version', async () => {
const astFilter = esKuery.fromKueryExpression(`fleet-agents.agent.version: 8.10.0`);
const validationObj = validateFilterKueryNode({
astFilter,
types: [AGENTS_PREFIX],
indexMapping: AGENT_MAPPINGS,
storeValue: true,
skipNormalization: true,
});
expect(validationObj).toEqual([
{
astPath: 'arguments.0',
error: null,
isSavedObjectAttr: false,
key: 'fleet-agents.agent.version',
type: 'fleet-agents',
},
]);
});
it('Search by version without SO wrapping', async () => {
const astFilter = esKuery.fromKueryExpression(`agent.version: 8.10.0`);
const validationObj = validateFilterKueryNode({
astFilter,
types: [AGENTS_PREFIX],
indexMapping: AGENT_MAPPINGS,
storeValue: true,
skipNormalization: true,
});
expect(validationObj).toEqual([
{
astPath: 'arguments.0',
error: null,
isSavedObjectAttr: false,
key: 'agent.version',
type: 'agent',
},
]);
});
});

describe('Enrollment Api keys', () => {
describe('Enrollment API keys', () => {
it('Search by policy id', async () => {
const astFilter = esKuery.fromKueryExpression(
`${FLEET_ENROLLMENT_API_PREFIX}.policy_id: policyId1`
Expand Down Expand Up @@ -508,9 +546,7 @@ describe('validateKuery validates real kueries', () => {
true
);
expect(validationObj?.isValid).toEqual(false);
expect(validationObj?.error).toContain(
`KQLSyntaxError: The key is empty and needs to be wrapped by a saved object type like ingest-agent-policies`
);
expect(validationObj?.error).toContain(`KQLSyntaxError: Invalid key`);
});

it('Kuery with non existent parameter wrapped by SO', async () => {
Expand All @@ -526,7 +562,7 @@ describe('validateKuery validates real kueries', () => {
);
});

it('Kuery with non existent parameter', async () => {
it('Invalid search by non existent parameter', async () => {
const validationObj = validateKuery(
`non_existent_parameter: 'test_id'`,
[AGENT_POLICY_SAVED_OBJECT_TYPE],
Expand All @@ -541,7 +577,7 @@ describe('validateKuery validates real kueries', () => {
});

describe('Agents', () => {
it('Test 1 - search policy id', async () => {
it('Search policy id', async () => {
const validationObj = validateKuery(
`${AGENTS_PREFIX}.policy_id: "policy_id"`,
[AGENTS_PREFIX],
Expand All @@ -551,7 +587,7 @@ describe('validateKuery validates real kueries', () => {
expect(validationObj?.isValid).toEqual(true);
});

it('Test 2 - status kuery without SO wrapping', async () => {
it('Status kuery without SO wrapping', async () => {
const validationObj = validateKuery(
`status:online or (status:updating or status:unenrolling or status:enrolling)`,
[AGENTS_PREFIX],
Expand All @@ -561,7 +597,7 @@ describe('validateKuery validates real kueries', () => {
expect(validationObj?.isValid).toEqual(true);
});

it('Test 3 - status kuery with SO wrapping', async () => {
it('Status kuery with SO wrapping', async () => {
const validationObj = validateKuery(
`${AGENTS_PREFIX}.status:online or (${AGENTS_PREFIX}.status:updating or ${AGENTS_PREFIX}.status:unenrolling or ${AGENTS_PREFIX}.status:enrolling)`,
[AGENTS_PREFIX],
Expand All @@ -571,7 +607,7 @@ describe('validateKuery validates real kueries', () => {
expect(validationObj?.isValid).toEqual(true);
});

it('Test 4 - valid kuery without SO wrapping', async () => {
it('Valid kuery without SO wrapping', async () => {
const validationObj = validateKuery(
`local_metadata.elastic.agent.version : "8.6.0"`,
[AGENTS_PREFIX],
Expand Down Expand Up @@ -650,6 +686,39 @@ describe('validateKuery validates real kueries', () => {
);
expect(validationObj?.isValid).toEqual(true);
});

it('Search by version', async () => {
const validationObj = validateKuery(
`agent.version: "8.10.0"`,
[AGENTS_PREFIX],
AGENT_MAPPINGS,
true
);
expect(validationObj?.isValid).toEqual(true);
});

it('Search by activity', async () => {
const validationObj = validateKuery(`active: true`, [AGENTS_PREFIX], AGENT_MAPPINGS, true);
expect(validationObj?.isValid).toEqual(true);
});

it('Search by agent.id', async () => {
const validationObj = validateKuery(`agent.id: id1`, [AGENTS_PREFIX], AGENT_MAPPINGS, true);
expect(validationObj?.isValid).toEqual(true);
});

it('Invalid search by non existent parameter', async () => {
const validationObj = validateKuery(
`non_existent_parameter: 'test_id'`,
[AGENTS_PREFIX],
AGENT_MAPPINGS,
true
);
expect(validationObj?.isValid).toEqual(false);
expect(validationObj?.error).toContain(
`KQLSyntaxError: This type 'non_existent_parameter' is not allowed`
);
});
});

describe('Package policies', () => {
Expand Down Expand Up @@ -751,5 +820,16 @@ describe('validateKuery validates real kueries', () => {
);
expect(validationObj?.isValid).toEqual(true);
});

it('Invalid search by non existent parameter', async () => {
const validationObj = validateKuery(
`policyId1`,
[FLEET_ENROLLMENT_API_PREFIX],
ENROLLMENT_API_KEY_MAPPINGS,
true
);
expect(validationObj?.isValid).toEqual(false);
expect(validationObj?.error).toEqual(`KQLSyntaxError: Invalid key`);
});
});
});

0 comments on commit 7839d51

Please sign in to comment.