[Security Solution] Fix DataSource payload creation during rule upgrade with MERGED pick_version (#197262)

jpdjere · jpdjere · commit 7a73df9b72cd · 2024-10-23T14:48:55.000Z
## Summary The PR #191439 enhanced the `/upgrade/_perform` API contract and functionality to allow the users of the endpoint to upgrade rules to their `MERGED` version. However, a bug slipped in, where the two different types of `DataSource` (`type: index_patterns` or `type: data_view_id`) weren't properly handled and would cause, in some cases, a rule payload to be created having both an `index` and `data_view` field, causing upgrade to fail. This PR fixes the issue by handling these two field in a specific way, checking what the `DataSource` diffable field's type is, and setting the other field to `undefined`. ### Checklist Delete any items that are not applicable to this PR. - [ ] [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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels) - [ ] This will appear in the **Release Notes** and follow the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) (cherry picked from commit 9656621)
diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api/perform_rule_upgrade/diffable_rule_fields_mappings.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api/perform_rule_upgrade/diffable_rule_fields_mappings.ts
@@ -7,6 +7,8 @@
 import { get } from 'lodash';
 import type {
   RuleSchedule,
+  DataSourceIndexPatterns,
+  DataSourceDataView,
   InlineKqlQuery,
   ThreeWayDiff,
   DiffableRuleTypes,
@@ -195,6 +197,10 @@ export const transformDiffableFieldValues = (
   } else if (fieldName === 'saved_id' && isInlineQuery(diffableFieldValue)) {
     // saved_id should be set only for rules with SavedKqlQuery, undefined otherwise
     return { type: 'TRANSFORMED_FIELD', value: undefined };
+  } else if (fieldName === 'data_view_id' && isDataSourceIndexPatterns(diffableFieldValue)) {
+    return { type: 'TRANSFORMED_FIELD', value: undefined };
+  } else if (fieldName === 'index' && isDataSourceDataView(diffableFieldValue)) {
+    return { type: 'TRANSFORMED_FIELD', value: undefined };
   }
 
   return { type: 'NON_TRANSFORMED_FIELD' };
@@ -209,3 +215,18 @@ function isInlineQuery(value: unknown): value is InlineKqlQuery {
     typeof value === 'object' && value !== null && 'type' in value && value.type === 'inline_query'
   );
 }
+
+function isDataSourceIndexPatterns(value: unknown): value is DataSourceIndexPatterns {
+  return (
+    typeof value === 'object' &&
+    value !== null &&
+    'type' in value &&
+    value.type === 'index_patterns'
+  );
+}
+
+function isDataSourceDataView(value: unknown): value is DataSourceDataView {
+  return (
+    typeof value === 'object' && value !== null && 'type' in value && value.type === 'data_view'
+  );
+}
diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/upgrade_perform_prebuilt_rules.all_rules_mode.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/upgrade_perform_prebuilt_rules.all_rules_mode.ts
@@ -17,6 +17,9 @@ import {
   ThreatMatchRule,
   FIELDS_TO_UPGRADE_TO_CURRENT_VERSION,
   ModeEnum,
+  AllFieldsDiff,
+  DataSourceIndexPatterns,
+  QueryRule,
 } from '@kbn/security-solution-plugin/common/api/detection_engine';
 import { PrebuiltRuleAsset } from '@kbn/security-solution-plugin/server/lib/detection_engine/prebuilt_rules';
 import { FtrProviderContext } from '../../../../../../ftr_provider_context';
@@ -246,6 +249,41 @@ export default ({ getService }: FtrProviderContext): void => {
           expect(installedRule.tags).toEqual(reviewRuleResponseMap.get(ruleId)?.tags);
         }
       });
+
+      it('correctly upgrades rules with DataSource diffs to their MERGED versions', async () => {
+        await createHistoricalPrebuiltRuleAssetSavedObjects(es, [queryRule]);
+        await installPrebuiltRules(es, supertest);
+
+        const targetObject = cloneDeep(queryRule);
+        targetObject['security-rule'].version += 1;
+        targetObject['security-rule'].name = TARGET_NAME;
+        targetObject['security-rule'].tags = TARGET_TAGS;
+        targetObject['security-rule'].index = ['auditbeat-*'];
+        await createHistoricalPrebuiltRuleAssetSavedObjects(es, [targetObject]);
+
+        const reviewResponse = await reviewPrebuiltRulesToUpgrade(supertest);
+        const ruleDiffFields = reviewResponse.rules[0].diff.fields as AllFieldsDiff;
+
+        const performUpgradeResponse = await performUpgradePrebuiltRules(es, supertest, {
+          mode: ModeEnum.ALL_RULES,
+          pick_version: 'MERGED',
+        });
+
+        expect(performUpgradeResponse.summary.succeeded).toEqual(1);
+
+        const installedRules = await getInstalledRules(supertest);
+        const installedRule = installedRules.data[0] as QueryRule;
+
+        expect(installedRule.name).toEqual(ruleDiffFields.name.merged_version);
+        expect(installedRule.tags).toEqual(ruleDiffFields.tags.merged_version);
+
+        // Check that the updated rules has an `index` field which equals the output of the diff algorithm
+        // for the DataSource diffable field, and that the data_view_id is correspondingly set to undefined.
+        expect(installedRule.index).toEqual(
+          (ruleDiffFields.data_source.merged_version as DataSourceIndexPatterns).index_patterns
+        );
+        expect(installedRule.data_view_id).toBe(undefined);
+      });
     });
 
     describe('edge cases and unhappy paths', () => {
