[Security Solution] Allow rewriting rule create props in Cypress tests (

elastic#153474) **Relates to:** elastic#150553 ## Summary This PR is based on the review comments in elastic#150553. It allows to rewrite rule create properties. ## Details Rule create properties are returned by helper functions like `getNewRule()`, `getNewThresholdRule()` and so on. So instead of `createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' })` it allows to use a concise and a much more readable structure `createRule(getNewRule({ index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' ))`. ## Possible improvements The PR doesn't implement deep / nested fields merge. High level fields completely rewrite default values. Deep merge would allow to extend defaults with the provided rewrites. For example, overriding nested properties become tiresome quickly, as shown in the following code snippet: ```ts const rule = { ...getNewTermsRule(), rule_id: 'new_rule_id', runsEvery: { interval: '1', ...getNewTermsRule().runsEvery, }, }; ``` If we implement deep merge, the readability could be greatly improved: ```ts const rule = getNewTermsRule({ rule_id: 'new_rule_id', runsEvery: { interval: '1', }, }); ``` While it looks as a good idea we should take into consideration the fact that complete rewriting of default values can be a desired behavior. Engineers could tend to switch to `createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' })` to overcome deep merge. So it should be analysed carefully before implementing it.
maximpn · Mar 27, 2023 · ca696ac · ca696ac
1 parent 519185f
commit ca696ac
Show file tree

Hide file tree

Showing 33 changed files with 301 additions and 281 deletions.
diff --git a/x-pack/plugins/security_solution/cypress/e2e/dashboards/enable_risk_score.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/dashboards/enable_risk_score.cy.ts
@@ -36,7 +36,7 @@ describe('Enable risk scores', () => {
   before(() => {
     cleanKibana();
     login();
-    createRule({ ...getNewRule(), rule_id: 'rule1' });
+    createRule(getNewRule({ rule_id: 'rule1' }));
   });
 
   beforeEach(() => {

diff --git a/x-pack/plugins/security_solution/cypress/e2e/dashboards/upgrade_risk_score.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/dashboards/upgrade_risk_score.cy.ts
@@ -39,7 +39,7 @@ describe('Upgrade risk scores', () => {
   before(() => {
     cleanKibana();
     login();
-    createRule({ ...getNewRule(), rule_id: 'rule1' });
+    createRule(getNewRule({ rule_id: 'rule1' }));
   });
 
   beforeEach(() => {
@@ -88,7 +88,7 @@ versions.forEach((version) =>
     before(() => {
       cleanKibana();
       login();
-      createRule({ ...getNewRule(), rule_id: 'rule1' });
+      createRule(getNewRule({ rule_id: 'rule1' }));
     });
 
     beforeEach(() => {

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/alerts_charts.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/alerts_charts.cy.ts
@@ -30,7 +30,7 @@ describe('Histogram legend hover actions', { testIsolation: false }, () => {
   before(() => {
     cleanKibana();
     login();
-    createRule({ ...getNewRule(), rule_id: 'new custom rule' });
+    createRule(getNewRule({ rule_id: 'new custom rule' }));
     visit(ALERTS_URL);
     selectAlertsHistogram();
   });

diff --git a/...rity_solution/cypress/e2e/detection_alerts/alerts_detection_callouts_index_outdated.cy.ts b/...rity_solution/cypress/e2e/detection_alerts/alerts_detection_callouts_index_outdated.cy.ts
@@ -73,7 +73,7 @@ describe('Detections > Need Admin Callouts indicating an admin is needed to migr
 
       context('On Rule Details page', () => {
         beforeEach(() => {
-          createRule({ ...getNewRule(), rule_id: 'rule_testing' });
+          createRule(getNewRule({ rule_id: 'rule_testing' }));
           loadPageAsPlatformEngineerUser(DETECTIONS_RULE_MANAGEMENT_URL);
           waitForPageTitleToBeShown();
           goToRuleDetails();
@@ -123,7 +123,7 @@ describe('Detections > Need Admin Callouts indicating an admin is needed to migr
 
       context('On Rule Details page', () => {
         beforeEach(() => {
-          createRule({ ...getNewRule(), rule_id: 'rule_testing' });
+          createRule(getNewRule({ rule_id: 'rule_testing' }));
           loadPageAsPlatformEngineerUser(DETECTIONS_RULE_MANAGEMENT_URL);
           waitForPageTitleToBeShown();
           goToRuleDetails();
@@ -173,7 +173,7 @@ describe('Detections > Need Admin Callouts indicating an admin is needed to migr
 
       context('On Rule Details page', () => {
         beforeEach(() => {
-          createRule({ ...getNewRule(), rule_id: 'rule_testing' });
+          createRule(getNewRule({ rule_id: 'rule_testing' }));
           loadPageAsPlatformEngineerUser(DETECTIONS_RULE_MANAGEMENT_URL);
           waitForPageTitleToBeShown();
           goToRuleDetails();

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/changing_alert_status.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/changing_alert_status.cy.ts
@@ -157,7 +157,7 @@ describe('Changing alert status', () => {
   context('Closing alerts', () => {
     beforeEach(() => {
       deleteAlertsAndRules();
-      createRule({ ...getNewRule(), rule_id: '1', max_signals: 100 });
+      createRule(getNewRule({ rule_id: '1', max_signals: 100 }));
       visit(ALERTS_URL);
       waitForAlertsToPopulate();
       selectCountTable();

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/detection_page_filters.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/detection_page_filters.cy.ts
@@ -102,7 +102,7 @@ describe.skip('Detections : Page Filters', { testIsolation: false }, () => {
   before(() => {
     cleanKibana();
     login();
-    createRule({ ...getNewRule(), rule_id: 'custom_rule_filters' });
+    createRule(getNewRule({ rule_id: 'custom_rule_filters' }));
     visit(ALERTS_URL);
     waitForAlerts();
     waitForPageFilters();

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/enrichments.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/enrichments.cy.ts
@@ -45,7 +45,7 @@ describe('Enrichment', () => {
     beforeEach(() => {
       esArchiverLoad('risk_hosts');
       deleteAlertsAndRules();
-      createRule({ ...getNewRule(), rule_id: 'rule1' });
+      createRule(getNewRule({ rule_id: 'rule1' }));
       visit(ALERTS_URL);
       waitForAlertsToPopulate();
     });

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/all_rules_read_only.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/all_rules_read_only.cy.ts
@@ -25,7 +25,7 @@ const MISSING_PRIVILEGES_CALLOUT = 'missing-user-privileges';
 describe('All rules - read only', () => {
   before(() => {
     cleanKibana();
-    createRule({ ...getNewRule(), rule_id: '1' });
+    createRule(getNewRule({ rule_id: '1' }));
     login(ROLES.reader);
   });
 

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts
@@ -125,17 +125,12 @@ describe('Detection rules, bulk edit', () => {
     resetRulesTableState();
     deleteAlertsAndRules();
     esArchiverResetKibana();
-    createRule({
-      ...getNewRule(),
-      name: RULE_NAME,
-      ...defaultRuleData,
-      rule_id: '1',
-    });
-    createRule({ ...getEqlRule(), ...defaultRuleData, rule_id: '2' });
-    createRule({ ...getMachineLearningRule(), tags: ['test-default-tag-1', 'test-default-tag-2'] });
-    createRule({ ...getNewThreatIndicatorRule(), ...defaultRuleData, rule_id: '4' });
-    createRule({ ...getNewThresholdRule(), ...defaultRuleData, rule_id: '5' });
-    createRule({ ...getNewTermsRule(), ...defaultRuleData, rule_id: '6' });
+    createRule(getNewRule({ name: RULE_NAME, ...defaultRuleData, rule_id: '1' }));
+    createRule(getEqlRule({ ...defaultRuleData, rule_id: '2' }));
+    createRule(getMachineLearningRule({ tags: ['test-default-tag-1', 'test-default-tag-2'] }));
+    createRule(getNewThreatIndicatorRule({ ...defaultRuleData, rule_id: '4' }));
+    createRule(getNewThresholdRule({ ...defaultRuleData, rule_id: '5' }));
+    createRule(getNewTermsRule({ ...defaultRuleData, rule_id: '6' }));
 
     visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
 

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_actions.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_actions.cy.ts
@@ -85,21 +85,15 @@ describe('Detection rules, bulk edit of rule actions', () => {
         },
       ];
 
-      createRule({
-        ...getNewRule(),
-        name: ruleNameToAssert,
-        rule_id: '1',
-        max_signals: 500,
-        actions,
-      });
+      createRule(getNewRule({ name: ruleNameToAssert, rule_id: '1', max_signals: 500, actions }));
     });
 
-    createRule({ ...getEqlRule(), rule_id: '2' });
-    createRule({ ...getMachineLearningRule(), rule_id: '3' });
-    createRule({ ...getNewThreatIndicatorRule(), rule_id: '4' });
-    createRule({ ...getNewThresholdRule(), rule_id: '5' });
-    createRule({ ...getNewTermsRule(), rule_id: '6' });
-    createRule({ ...getNewRule(), saved_id: 'mocked', rule_id: '7' });
+    createRule(getEqlRule({ rule_id: '2' }));
+    createRule(getMachineLearningRule({ rule_id: '3' }));
+    createRule(getNewThreatIndicatorRule({ rule_id: '4' }));
+    createRule(getNewThresholdRule({ rule_id: '5' }));
+    createRule(getNewTermsRule({ rule_id: '6' }));
+    createRule(getNewRule({ saved_id: 'mocked', rule_id: '7' }));
 
     createSlackConnector();
   });

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_data_view.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_data_view.cy.ts
@@ -62,33 +62,16 @@ describe('Bulk editing index patterns of rules with a data view only', () => {
 
     postDataView(DATA_VIEW_ID);
 
-    createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' });
-    createRule({ ...getEqlRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '2' });
-    createRule({
-      ...getNewThreatIndicatorRule(),
-      index: undefined,
-      data_view_id: DATA_VIEW_ID,
-      rule_id: '3',
-    });
-    createRule({
-      ...getNewThresholdRule(),
-      index: undefined,
-      data_view_id: DATA_VIEW_ID,
-      rule_id: '4',
-    });
-    createRule({
-      ...getNewTermsRule(),
-      index: undefined,
-      data_view_id: DATA_VIEW_ID,
-      rule_id: '5',
-    });
-    createRule({
-      ...getNewRule(),
-      index: undefined,
-      data_view_id: DATA_VIEW_ID,
-      saved_id: 'mocked',
-      rule_id: '6',
-    });
+    createRule(getNewRule({ index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' }));
+    createRule(getEqlRule({ index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '2' }));
+    createRule(
+      getNewThreatIndicatorRule({ index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '3' })
+    );
+    createRule(getNewThresholdRule({ index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '4' }));
+    createRule(getNewTermsRule({ index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '5' }));
+    createRule(
+      getNewRule({ index: undefined, data_view_id: DATA_VIEW_ID, saved_id: 'mocked', rule_id: '6' })
+    );
 
     visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
 
@@ -205,12 +188,8 @@ describe('Bulk editing index patterns of rules with index patterns and rules wit
 
     postDataView(DATA_VIEW_ID);
 
-    createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' });
-    createRule({
-      ...getNewRule(),
-      index: ['test-index-1-*'],
-      rule_id: '2',
-    });
+    createRule(getNewRule({ index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' }));
+    createRule(getNewRule({ index: ['test-index-1-*'], rule_id: '2' }));
 
     visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
 

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/custom_query_rule.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/custom_query_rule.cy.ts
@@ -234,9 +234,9 @@ describe('Custom query rules', () => {
     context('Deletion', () => {
       beforeEach(() => {
         deleteAlertsAndRules();
-        createRule({ ...getNewRule(), rule_id: 'rule1', enabled: true, max_signals: 500 });
-        createRule({ ...getNewOverrideRule(), rule_id: 'rule2', enabled: true, max_signals: 500 });
-        createRule({ ...getExistingRule(), rule_id: 'rule3', enabled: true });
+        createRule(getNewRule({ rule_id: 'rule1', enabled: true, max_signals: 500 }));
+        createRule(getNewOverrideRule({ rule_id: 'rule2', enabled: true, max_signals: 500 }));
+        createRule(getExistingRule({ rule_id: 'rule3', enabled: true }));
         visit(DETECTIONS_RULE_MANAGEMENT_URL);
       });
 
@@ -346,7 +346,7 @@ describe('Custom query rules', () => {
       before(() => {
         deleteAlertsAndRules();
         deleteConnectors();
-        createRule({ ...getExistingRule(), rule_id: 'rule1', enabled: true });
+        createRule(getExistingRule({ rule_id: 'rule1', enabled: true }));
       });
       beforeEach(() => {
         visit(DETECTIONS_RULE_MANAGEMENT_URL);

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/custom_saved_query_rule.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/custom_saved_query_rule.cy.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { getNewRule } from '../../objects/rule';
+import { getNewRule, getSavedQueryRule } from '../../objects/rule';
 
 import {
   DEFINE_CONTINUE_BUTTON,
@@ -58,7 +58,7 @@ describe('Custom saved_query rules', () => {
     });
 
     it('Creates saved query rule', function () {
-      const rule = getNewRule();
+      const rule = getSavedQueryRule();
       createSavedQuery(savedQueryName, savedQueryQuery, savedQueryFilterKey);
       visit(RULE_CREATION);
 
@@ -101,12 +101,7 @@ describe('Custom saved_query rules', () => {
     context('Non existent saved query', () => {
       const FAILED_TO_LOAD_ERROR = 'Failed to load the saved query';
       beforeEach(() => {
-        createRule({
-          ...getNewRule(),
-          type: 'saved_query',
-          saved_id: 'non-existent',
-          query: undefined,
-        });
+        createRule(getSavedQueryRule({ saved_id: 'non-existent', query: undefined }));
         cy.visit(SECURITY_DETECTIONS_RULES_URL);
       });
       it('Shows error toast on details page when saved query can not be loaded', function () {
@@ -152,12 +147,7 @@ describe('Custom saved_query rules', () => {
         const expectedCustomTestQuery = 'random test query';
         createSavedQuery(savedQueryName, savedQueryQuery).then((response) => {
           cy.log(JSON.stringify(response.body, null, 2));
-          createRule({
-            ...getNewRule(),
-            type: 'saved_query',
-            saved_id: response.body.id,
-            query: undefined,
-          });
+          createRule(getSavedQueryRule({ saved_id: response.body.id, query: undefined }));
         });
 
         cy.visit(SECURITY_DETECTIONS_RULES_URL);
@@ -184,12 +174,7 @@ describe('Custom saved_query rules', () => {
 
       it('Allows to update saved_query rule with non-existent query by adding custom query', () => {
         const expectedCustomTestQuery = 'random test query';
-        createRule({
-          ...getNewRule(),
-          type: 'saved_query',
-          saved_id: 'non-existent',
-          query: undefined,
-        });
+        createRule(getSavedQueryRule({ saved_id: 'non-existent', query: undefined }));
 
         cy.visit(SECURITY_DETECTIONS_RULES_URL);
 
@@ -212,12 +197,7 @@ describe('Custom saved_query rules', () => {
 
       it('Allows to update saved_query rule with non-existent query by selecting another saved query', () => {
         createSavedQuery(savedQueryName, savedQueryQuery);
-        createRule({
-          ...getNewRule(),
-          type: 'saved_query',
-          saved_id: 'non-existent',
-          query: undefined,
-        });
+        createRule(getSavedQueryRule({ saved_id: 'non-existent', query: undefined }));
 
         cy.visit(SECURITY_DETECTIONS_RULES_URL);
 

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/export_rule.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/export_rule.cy.ts
@@ -52,7 +52,7 @@ describe('Export rules', () => {
     // Rules get exported via _bulk_action endpoint
     cy.intercept('POST', '/api/detection_engine/rules/_bulk_action').as('bulk_action');
     visitWithoutDateRange(DETECTIONS_RULE_MANAGEMENT_URL);
-    createRule({ ...getNewRule(), name: 'Rule to export' }).as('ruleResponse');
+    createRule(getNewRule({ name: 'Rule to export' })).as('ruleResponse');
   });
 
   it('exports a custom rule', function () {
@@ -65,7 +65,7 @@ describe('Export rules', () => {
 
   it('creates an importable file from executed rule', () => {
     // Rule needs to be enabled to make sure it has been executed so rule's SO contains runtime fields like `execution_summary`
-    createRule({ ...getNewRule(), name: 'Enabled rule to export', enabled: true });
+    createRule(getNewRule({ name: 'Enabled rule to export', enabled: true }));
     waitForRuleExecution('Enabled rule to export');
 
     exportRule('Enabled rule to export');
@@ -128,19 +128,20 @@ describe('Export rules', () => {
       deleteExceptionList(exceptionList.list_id, exceptionList.namespace_type);
       // create rule with exceptions
       createExceptionList(exceptionList, exceptionList.list_id).then((response) =>
-        createRule({
-          ...getNewRule(),
-          name: 'rule with exceptions',
-          exceptions_list: [
-            {
-              id: response.body.id,
-              list_id: exceptionList.list_id,
-              type: exceptionList.type,
-              namespace_type: exceptionList.namespace_type,
-            },
-          ],
-          rule_id: '2',
-        })
+        createRule(
+          getNewRule({
+            name: 'rule with exceptions',
+            exceptions_list: [
+              {
+                id: response.body.id,
+                list_id: exceptionList.list_id,
+                type: exceptionList.type,
+                namespace_type: exceptionList.namespace_type,
+              },
+            ],
+            rule_id: '2',
+          })
+        )
       );
     });
 

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/indicator_match_rule.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/indicator_match_rule.cy.ts
@@ -498,7 +498,7 @@ describe('indicator match', () => {
         const accessibilityText = `Press enter for options, or press space to begin dragging.`;
 
         loadPrepackagedTimelineTemplates();
-        createRule({ ...getNewThreatIndicatorRule(), rule_id: 'rule_testing', enabled: true });
+        createRule(getNewThreatIndicatorRule({ rule_id: 'rule_testing', enabled: true }));
         visit(DETECTIONS_RULE_MANAGEMENT_URL);
         goToRuleDetails();
         waitForAlertsToPopulate();
@@ -531,7 +531,7 @@ describe('indicator match', () => {
     describe('Duplicates the indicator rule', () => {
       beforeEach(() => {
         deleteAlertsAndRules();
-        createRule({ ...getNewThreatIndicatorRule(), rule_id: 'rule_testing', enabled: true });
+        createRule(getNewThreatIndicatorRule({ rule_id: 'rule_testing', enabled: true }));
         visitWithoutDateRange(DETECTIONS_RULE_MANAGEMENT_URL);
       });
 

diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/links.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/links.cy.ts
@@ -19,7 +19,7 @@ describe('Rules talbes links', () => {
   });
   beforeEach(() => {
     deleteAlertsAndRules();
-    createRule({ ...getNewRule(), rule_id: 'rule1' });
+    createRule(getNewRule({ rule_id: 'rule1' }));
     visitWithoutDateRange(DETECTIONS_RULE_MANAGEMENT_URL);
   });