[8.10] [Fleet] Fix validation errors in KQL queries (#168329) (#168436)

# Backport This will backport the following commits from `main` to `8.10`: - [[Fleet] Fix validation errors in KQL queries (#168329)](#168329)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Cristina Amico <[email protected]>
elastic · Oct 10, 2023 · b3cfe48 · b3cfe48
1 parent c0b8235
commit b3cfe48
Show file tree

Hide file tree

Showing 4 changed files with 128 additions and 45 deletions.
diff --git a/x-pack/plugins/fleet/server/constants/mappings.ts b/x-pack/plugins/fleet/server/constants/mappings.ts
@@ -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 = {

diff --git a/x-pack/plugins/fleet/server/routes/utils/filter_utils.test.ts b/x-pack/plugins/fleet/server/routes/utils/filter_utils.test.ts
@@ -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,

diff --git a/x-pack/plugins/fleet/server/routes/utils/filter_utils.ts b/x-pack/plugins/fleet/server/routes/utils/filter_utils.ts
@@ -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)) {
@@ -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
@@ -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;
   }
 
@@ -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;
   }
 

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
@@ -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`
       );
@@ -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:*`
       );
@@ -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:*)`
@@ -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:*)`
       );
@@ -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`
       );
@@ -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`
@@ -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 () => {
@@ -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],
@@ -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],
@@ -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],
@@ -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],
@@ -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],
@@ -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', () => {
@@ -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`);
+    });
   });
 });