From a1151599accfa80c596d56b31b3193ee7b439ab6 Mon Sep 17 00:00:00 2001 From: Juan Pablo Djeredjian Date: Wed, 23 Oct 2024 16:44:13 +0200 Subject: [PATCH] [Security Solution] Fix `DataSource` payload creation during rule upgrade with `MERGED` pick_version (#197262) ## Summary The PR https://github.com/elastic/kibana/pull/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 9656621fcc8f6f9a615b0a27d45db9722e047a10) --- .../diffable_rule_fields_mappings.ts | 21 ++++++++++ ...e_perform_prebuilt_rules.all_rules_mode.ts | 38 +++++++++++++++++++ 2 files changed, 59 insertions(+) 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 index d56747f9db264..8a796b5db1e28 100644 --- 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 index 2d0fe71e7d5d4..e24e517c93459 100644 --- 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', () => {