Support more cases for detection; use code editor for condition (#783)

* support more cases for detection; use code editor for condition Signed-off-by: Amardeepsingh Siglani <[email protected]> * updated snapshots Signed-off-by: Amardeepsingh Siglani <[email protected]> * fixed cypress tests Signed-off-by: Amardeepsingh Siglani <[email protected]> --------- Signed-off-by: Amardeepsingh Siglani <[email protected]>
opensearch-project · Nov 21, 2023 · b3ddb58 · b3ddb58
1 parent b093c9d
commit b3ddb58
Show file tree

Hide file tree

Showing 11 changed files with 1,156 additions and 269 deletions.
diff --git a/babel.config.js b/babel.config.js
@@ -13,7 +13,7 @@ module.exports = {
   ],
   plugins: [
     [require('@babel/plugin-transform-runtime'), { regenerator: true }],
-    require('@babel/plugin-proposal-class-properties'),
-    require('@babel/plugin-proposal-object-rest-spread'),
+    require('@babel/plugin-transform-class-properties'),
+    require('@babel/plugin-transform-object-rest-spread'),
   ],
 };
diff --git a/cypress/integration/1_detectors.spec.js b/cypress/integration/1_detectors.spec.js
@@ -17,6 +17,7 @@ const cypressIndexWindows = 'cypress-index-windows';
 const detectorName = 'test detector';
 const cypressLogTypeDns = 'dns';
 const sampleNotificationChannel = 'sample_chime_channel';
+const creationFailedMessage = 'Create detector failed.';
 
 const cypressDNSRule = dns_name_rule_data.title;
 
@@ -381,12 +382,12 @@ describe('Detectors', () => {
 
     it('...can fail creation', () => {
       createDetector(`${detectorName}_fail`, '.kibana_1', true);
-      cy.getElementByText('.euiCallOut', 'Create detector failed.');
+      cy.getElementByText('.euiCallOut', creationFailedMessage);
     });
 
     it('...can be created', () => {
       createDetector(detectorName, cypressIndexDns, false);
-      cy.getElementByText('.euiCallOut', 'Detector created successfully');
+      cy.contains(creationFailedMessage).should('not.exist');
     });
 
     it('...basic details can be edited', () => {

diff --git a/cypress/integration/2_rules.spec.js b/cypress/integration/2_rules.spec.js
@@ -176,8 +176,6 @@ const fillCreateForm = () => {
     getMapValueField().type('FieldValue');
   });
 
-  getConditionAddButton().click({ force: true });
-
   // rule additional details
   SAMPLE_RULE.tags.forEach((tag, idx) => {
     getTagField(idx).type(tag);
@@ -336,7 +334,7 @@ describe('Rules', () => {
         getMapKeyField()
           .parentsUntil('.euiFormRow__fieldWrapper')
           .siblings()
-          .contains('Key name is required');
+          .contains('Invalid key name');
 
         getMapKeyField().type('FieldKey');
         getMapKeyField()
@@ -388,13 +386,10 @@ describe('Rules', () => {
       getConditionField().scrollIntoView();
       getConditionField().find('.euiFormErrorText').should('not.exist');
       getRuleSubmitButton().click({ force: true });
-      getConditionField().parents('.euiFormRow__fieldWrapper').contains('Condition is required');
-
-      getConditionAddButton().click({ force: true });
-      getConditionField().find('.euiFormErrorText').should('not.exist');
-
-      getConditionRemoveButton(0).click({ force: true });
-      getConditionField().parents('.euiFormRow__fieldWrapper').contains('Condition is required');
+      getConditionField()
+        .parents('.euiFormRow__fieldWrapper')
+        .contains('Condition is required')
+        .should('not.exist');
     });
 
     it('...should validate tag field', () => {
@@ -472,11 +467,6 @@ describe('Rules', () => {
         getMapListField().type('FieldValue');
       });
 
-      // condition field
-      getConditionRemoveButton(0).click({ force: true });
-      toastShouldExist();
-      getConditionAddButton().click({ force: true });
-
       // tags field
       getTagField(0).clearValue().type('wrong.tag');
       toastShouldExist();

diff --git a/cypress/integration/3_alerts.spec.js b/cypress/integration/3_alerts.spec.js
@@ -112,12 +112,12 @@ describe('Alerts', () => {
 
       // Wait for the findings table to finish loading
       cy.contains('Findings (1)');
-      cy.contains('Cypress USB Rule');
+      cy.contains('Detection rules');
 
       // Confirm alert findings contain expected values
       cy.get('tbody > tr').should(($tr) => {
         expect($tr, `timestamp`).to.contain(date);
-        expect($tr, `rule name`).to.contain('Cypress USB Rule');
+        expect($tr, `detection`).to.contain('Detection rules');
         expect($tr, `detector name`).to.contain(testDetector.name);
         expect($tr, `log type`).to.contain(
           `System Activity: ${getLogTypeLabel(testDetector.detector_type)}`
@@ -143,7 +143,8 @@ describe('Alerts', () => {
 
     cy.get('[data-test-subj="alert-details-flyout"]').within(() => {
       // Wait for findings table to finish loading
-      cy.contains('Cypress USB Rule');
+      cy.wait(3000);
+      cy.contains('Detection rules');
 
       // Click the details button for the first finding
       cy.get('tbody > tr')

diff --git a/package.json b/package.json
@@ -57,6 +57,8 @@
     "@cypress/request": "^3.0.0"
   },
   "devDependencies": {
+    "@babel/plugin-transform-class-properties": "^7.22.9",
+    "@babel/plugin-transform-object-rest-spread": "^7.22.9",
     "@elastic/elastic-eslint-config-kibana": "link:../../packages/opensearch-eslint-config-opensearch-dashboards",
     "@elastic/eslint-import-resolver-kibana": "link:../../packages/osd-eslint-import-resolver-opensearch-dashboards",
     "@testing-library/dom": "^8.11.3",

diff --git a/public/pages/Rules/components/RuleEditor/DetectionVisualEditor.tsx b/public/pages/Rules/components/RuleEditor/DetectionVisualEditor.tsx
@@ -29,10 +29,10 @@ import {
   EuiFilePicker,
   EuiButtonEmpty,
   EuiCallOut,
+  EuiCodeEditor,
 } from '@elastic/eui';
 import _ from 'lodash';
 import { validateCondition, validateDetectionFieldName } from '../../../../utils/validation';
-import { SelectionExpField } from './components/SelectionExpField';
 
 export interface DetectionVisualEditorProps {
   detectionYml: string;
@@ -90,7 +90,7 @@ const detectionModifierOptions = [
 ];
 
 const defaultDetectionObj: DetectionObject = {
-  condition: '',
+  condition: 'Selection_1',
   selections: [
     {
       name: 'Selection_1',
@@ -178,7 +178,18 @@ export class DetectionVisualEditor extends React.Component<
       const selectionMapJSON = detectionJSON[selectionKey];
       const selectionDataEntries: SelectionData[] = [];
 
-      if (typeof selectionMapJSON === 'object') {
+      if (Array.isArray(selectionMapJSON)) {
+        selectionDataEntries.push({
+          field: '',
+          modifier: 'all',
+          values: selectionMapJSON,
+          selectedRadioId: `${
+            selectionMapJSON.length <= 1
+              ? SelectionMapValueRadioId.VALUE
+              : SelectionMapValueRadioId.LIST
+          }-${selectionIdx}-0`,
+        });
+      } else if (typeof selectionMapJSON === 'object') {
         Object.keys(selectionMapJSON).forEach((fieldKey, dataIdx) => {
           const [field, modifier] = fieldKey.split('|');
           const val = selectionMapJSON[fieldKey];
@@ -212,11 +223,15 @@ export class DetectionVisualEditor extends React.Component<
     };
 
     selections.forEach((selection) => {
-      const selectionMaps: any = {};
+      let selectionMaps: any = {};
 
       selection.data.forEach((datum) => {
-        const key = `${datum.field}${datum.modifier ? `|${datum.modifier}` : ''}`;
-        selectionMaps[key] = datum.values;
+        if (datum.field) {
+          const key = `${datum.field}${datum.modifier ? `|${datum.modifier}` : ''}`;
+          selectionMaps[key] = datum.values;
+        } else {
+          selectionMaps = datum.values;
+        }
       });
 
       compiledDetection[selection.name] = selectionMaps;
@@ -228,21 +243,16 @@ export class DetectionVisualEditor extends React.Component<
   private validateData = (selections: Selection[]) => {
     const { errors } = this.state;
     selections.map((selection, selIdx) => {
-      const fieldNames = new Set<string>();
       selection.data.map((data, idx) => {
         if ('field' in data) {
           const fieldName = `field_${selIdx}_${idx}`;
           delete errors.fields[fieldName];
-          if (!data.field) {
-            errors.fields[fieldName] = 'Key name is required';
-          } else if (fieldNames.has(data.field)) {
-            errors.fields[fieldName] = 'Key name already used';
-          } else {
-            fieldNames.add(data.field);
-            if (!validateDetectionFieldName(data.field)) {
-              errors.fields[fieldName] = 'Invalid key name.';
-            }
+
+          if (!validateDetectionFieldName(data.field)) {
+            errors.fields[fieldName] =
+              'Invalid key name. Valid characters are a-z, A-Z, 0-9, hyphens, dots, and underscores';
           }
+
           errors.touched[fieldName] = true;
         }
 
@@ -343,29 +353,14 @@ export class DetectionVisualEditor extends React.Component<
   };
 
   private validateCondition = (value: string) => {
-    const {
-      errors,
-      detectionObj: { selections },
-    } = this.state;
+    const { errors } = this.state;
     value = value.trim();
     delete errors.fields['condition'];
     if (!value) {
       errors.fields['condition'] = 'Condition is required';
     } else {
       if (!validateCondition(value)) {
         errors.fields['condition'] = 'Invalid condition.';
-      } else {
-        const selectionNames = _.map(selections, 'name');
-        const conditions = _.pull(value.split(' '), ...['and', 'or', 'not']);
-        conditions.map((selection) => {
-          if (_.indexOf(selectionNames, selection) === -1) {
-            errors.fields[
-              'condition'
-            ] = `Invalid selection name ${selection}. Allowed names: "${selectionNames.join(
-              ', '
-            )}"`;
-          }
-        });
       }
     }
 
@@ -376,8 +371,6 @@ export class DetectionVisualEditor extends React.Component<
   };
 
   private updateCondition = (value: string) => {
-    value = value.trim();
-
     const detectionObj: DetectionObject = { ...this.state.detectionObj, condition: value };
     this.setState(
       {
@@ -729,6 +722,7 @@ export class DetectionVisualEditor extends React.Component<
                 <EuiButton
                   style={{ width: '70%' }}
                   iconType="plusInCircle"
+                  disabled={!selection.data.at(-1)?.field}
                   onClick={() => {
                     const newData = [
                       ...selection.data,
@@ -754,14 +748,15 @@ export class DetectionVisualEditor extends React.Component<
           fullWidth
           iconType={'plusInCircle'}
           onClick={() => {
+            const selectionName = `Selection_${selections.length + 1}`;
             this.setState({
               detectionObj: {
-                condition,
+                condition: `${condition} and ${selectionName}`,
                 selections: [
                   ...selections,
                   {
                     ...defaultDetectionObj.selections[0],
-                    name: `Selection_${selections.length + 1}`,
+                    name: selectionName,
                   },
                 ],
               },
@@ -796,11 +791,16 @@ export class DetectionVisualEditor extends React.Component<
             </>
           }
         >
-          <SelectionExpField
-            selections={this.state.detectionObj.selections}
+          <EuiCodeEditor
+            mode="yaml"
+            width="600px"
+            height="50px"
             value={this.state.detectionObj.condition}
-            onChange={this.updateCondition}
-            dataTestSubj={'rule_detection_field'}
+            onChange={(value) => this.updateCondition(value)}
+            onBlur={(e) => {
+              this.updateCondition(this.state.detectionObj.condition);
+            }}
+            data-test-subj={'rule_detection_field'}
           />
         </EuiFormRow>