From 687d07f0383362cb705331b30916fece5fa0ad90 Mon Sep 17 00:00:00 2001 From: Davis Plumlee Date: Tue, 30 Jul 2024 14:46:07 -0400 Subject: [PATCH] addresses comments --- .../three_way_diff/three_way_diff_outcome.ts | 34 ++++++++ .../data_source_diff_algorithm.test.ts | 84 ++++++++++++------- .../algorithms/data_source_diff_algorithm.ts | 69 +++++---------- .../diff/calculation/algorithms/helpers.ts | 14 +++- 4 files changed, 123 insertions(+), 78 deletions(-) diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff_outcome.ts b/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff_outcome.ts index 44885d1932070..d5d656f3bb385 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff_outcome.ts +++ b/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff_outcome.ts @@ -7,6 +7,8 @@ import { isEqual } from 'lodash'; import { MissingVersion } from './three_way_diff'; +import type { RuleDataSource } from '../diffable_rule/diffable_field_types'; +import { DataSourceType } from '../diffable_rule/diffable_field_types'; /** * Result of comparing three versions of a value against each other. @@ -75,6 +77,33 @@ export const determineOrderAgnosticDiffOutcome = ( }); }; +/** + * Determines diff outcome for `data_source` field + * + * NOTE: uses order agnostic comparison for nested array fields (e.g. `index`) + */ +export const determineDiffOutcomeForDataSource = ( + baseVersion: RuleDataSource | MissingVersion, + currentVersion: RuleDataSource, + targetVersion: RuleDataSource +): ThreeWayDiffOutcome => { + const isBaseVersionMissing = baseVersion === MissingVersion; + + if ( + (isBaseVersionMissing || isIndexPatternDataSourceType(baseVersion)) && + isIndexPatternDataSourceType(currentVersion) && + isIndexPatternDataSourceType(targetVersion) + ) { + return determineOrderAgnosticDiffOutcome( + isBaseVersionMissing ? MissingVersion : baseVersion.index_patterns, + currentVersion.index_patterns, + targetVersion.index_patterns + ); + } + + return determineDiffOutcome(baseVersion, currentVersion, targetVersion); +}; + interface DetermineDiffOutcomeProps { baseEqlCurrent: boolean; baseEqlTarget: boolean; @@ -121,3 +150,8 @@ export const determineIfValueCanUpdate = (diffCase: ThreeWayDiffOutcome): boolea diffCase === ThreeWayDiffOutcome.MissingBaseCanUpdate ); }; + +const isIndexPatternDataSourceType = ( + version: RuleDataSource +): version is Extract => + version.type === DataSourceType.index_patterns; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/data_source_diff_algorithm.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/data_source_diff_algorithm.test.ts index 6c7309bd4444e..480c8c64f2d1d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/data_source_diff_algorithm.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/data_source_diff_algorithm.test.ts @@ -166,32 +166,53 @@ describe('dataSourceDiffAlgorithm', () => { }); }); - it('returns current_version as merged output if current version is different but it matches the update - scenario ABB', () => { - const mockVersions: ThreeVersionsOf = { - base_version: { - type: DataSourceType.index_patterns, - index_patterns: ['one', 'two', 'three'], - }, - current_version: { - type: DataSourceType.index_patterns, - index_patterns: ['one', 'three', 'four'], - }, - target_version: { - type: DataSourceType.index_patterns, - index_patterns: ['one', 'three', 'four'], - }, - }; - - const result = dataSourceDiffAlgorithm(mockVersions); - - expect(result).toEqual( - expect.objectContaining({ - merged_version: mockVersions.current_version, - diff_outcome: ThreeWayDiffOutcome.CustomizedValueSameUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, - conflict: ThreeWayDiffConflict.NONE, - }) - ); + describe('returns current_version as merged output if current version is different but it matches the update - scenario ABB', () => { + it('if all versions are index patterns', () => { + const mockVersions: ThreeVersionsOf = { + base_version: { + type: DataSourceType.index_patterns, + index_patterns: ['one', 'two', 'three'], + }, + current_version: { + type: DataSourceType.index_patterns, + index_patterns: ['one', 'three', 'four'], + }, + target_version: { + type: DataSourceType.index_patterns, + index_patterns: ['one', 'three', 'four'], + }, + }; + + const result = dataSourceDiffAlgorithm(mockVersions); + + expect(result).toEqual( + expect.objectContaining({ + merged_version: mockVersions.current_version, + diff_outcome: ThreeWayDiffOutcome.CustomizedValueSameUpdate, + merge_outcome: ThreeWayMergeOutcome.Current, + conflict: ThreeWayDiffConflict.NONE, + }) + ); + }); + + it('if all versions are data views', () => { + const mockVersions: ThreeVersionsOf = { + base_version: { type: DataSourceType.data_view, data_view_id: '123' }, + current_version: { type: DataSourceType.data_view, data_view_id: '456' }, + target_version: { type: DataSourceType.data_view, data_view_id: '456' }, + }; + + const result = dataSourceDiffAlgorithm(mockVersions); + + expect(result).toEqual( + expect.objectContaining({ + merged_version: mockVersions.current_version, + diff_outcome: ThreeWayDiffOutcome.CustomizedValueSameUpdate, + merge_outcome: ThreeWayMergeOutcome.Current, + conflict: ThreeWayDiffConflict.NONE, + }) + ); + }); }); describe('returns current_version as merged output if all three versions are different - scenario ABC', () => { @@ -260,14 +281,19 @@ describe('dataSourceDiffAlgorithm', () => { }, }; + const expectedMergedVersion: RuleDataSource = { + type: DataSourceType.index_patterns, + index_patterns: ['one', 'three', 'four', 'two', 'five'], + }; + const result = dataSourceDiffAlgorithm(mockVersions); expect(result).toEqual( expect.objectContaining({ - merged_version: mockVersions.current_version, + merged_version: expectedMergedVersion, diff_outcome: ThreeWayDiffOutcome.CustomizedValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, - conflict: ThreeWayDiffConflict.NON_SOLVABLE, + merge_outcome: ThreeWayMergeOutcome.Merged, + conflict: ThreeWayDiffConflict.SOLVABLE, }) ); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/data_source_diff_algorithm.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/data_source_diff_algorithm.ts index 635ea020c73ce..9068b215b11b2 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/data_source_diff_algorithm.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/data_source_diff_algorithm.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { uniq } from 'lodash'; +import { union } from 'lodash'; import { assertUnreachable } from '../../../../../../../../common/utility_types'; import type { RuleDataSource, @@ -13,16 +13,15 @@ import type { ThreeWayDiff, } from '../../../../../../../../common/api/detection_engine/prebuilt_rules'; import { - determineDiffOutcome, determineIfValueCanUpdate, ThreeWayDiffOutcome, ThreeWayMergeOutcome, MissingVersion, DataSourceType, - determineOrderAgnosticDiffOutcome, ThreeWayDiffConflict, + determineDiffOutcomeForDataSource, } from '../../../../../../../../common/api/detection_engine/prebuilt_rules'; -import { mergeDedupedArrays } from './helpers'; +import { getDedupedDataSourceVersion, mergeDedupedArrays } from './helpers'; export const dataSourceDiffAlgorithm = ( versions: ThreeVersionsOf @@ -33,36 +32,7 @@ export const dataSourceDiffAlgorithm = ( target_version: targetVersion, } = versions; - let diffOutcome: ThreeWayDiffOutcome; - - if (baseVersion === MissingVersion) { - if ( - currentVersion.type === DataSourceType.index_patterns && - targetVersion.type === DataSourceType.index_patterns - ) { - diffOutcome = determineOrderAgnosticDiffOutcome( - MissingVersion, - currentVersion.index_patterns, - targetVersion.index_patterns - ); - } else { - diffOutcome = determineDiffOutcome(baseVersion, currentVersion, targetVersion); - } - } else { - if ( - baseVersion.type === DataSourceType.index_patterns && - currentVersion.type === DataSourceType.index_patterns && - targetVersion.type === DataSourceType.index_patterns - ) { - diffOutcome = determineOrderAgnosticDiffOutcome( - baseVersion.index_patterns, - currentVersion.index_patterns, - targetVersion.index_patterns - ); - } else { - diffOutcome = determineDiffOutcome(baseVersion, currentVersion, targetVersion); - } - } + const diffOutcome = determineDiffOutcomeForDataSource(baseVersion, currentVersion, targetVersion); const valueCanUpdate = determineIfValueCanUpdate(diffOutcome); @@ -134,18 +104,31 @@ const mergeVersions = ({ case ThreeWayDiffOutcome.CustomizedValueCanUpdate: { if ( - dedupedBaseVersion && - dedupedBaseVersion.type === DataSourceType.index_patterns && dedupedCurrentVersion.type === DataSourceType.index_patterns && dedupedTargetVersion.type === DataSourceType.index_patterns ) { + if (dedupedBaseVersion && dedupedBaseVersion.type === DataSourceType.index_patterns) { + // If all versions are index pattern types, merge all arrays + return { + conflict: ThreeWayDiffConflict.SOLVABLE, + mergeOutcome: ThreeWayMergeOutcome.Merged, + mergedVersion: { + type: DataSourceType.index_patterns, + index_patterns: mergeDedupedArrays( + dedupedBaseVersion.index_patterns, + dedupedCurrentVersion.index_patterns, + dedupedTargetVersion.index_patterns + ), + }, + }; + } + // If just the current and target versions are index pattern types, return the union of the arrays as the merged version return { conflict: ThreeWayDiffConflict.SOLVABLE, mergeOutcome: ThreeWayMergeOutcome.Merged, mergedVersion: { type: DataSourceType.index_patterns, - index_patterns: mergeDedupedArrays( - dedupedBaseVersion.index_patterns, + index_patterns: union( dedupedCurrentVersion.index_patterns, dedupedTargetVersion.index_patterns ), @@ -173,13 +156,3 @@ const mergeVersions = ({ return assertUnreachable(diffOutcome); } }; - -const getDedupedDataSourceVersion = (version: RuleDataSource): RuleDataSource => { - if (version.type === DataSourceType.index_patterns) { - return { - ...version, - index_patterns: uniq(version.index_patterns), - }; - } - return version; -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/helpers.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/helpers.ts index 9db1d0040085e..498b857eb0428 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/helpers.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/helpers.ts @@ -5,7 +5,9 @@ * 2.0. */ -import { difference, union } from 'lodash'; +import { difference, union, uniq } from 'lodash'; +import type { RuleDataSource } from '../../../../../../../../common/api/detection_engine'; +import { DataSourceType } from '../../../../../../../../common/api/detection_engine'; export const mergeDedupedArrays = ( dedupedBaseVersion: T[], @@ -23,3 +25,13 @@ export const mergeDedupedArrays = ( return difference(union(dedupedBaseVersion, bothAdded), bothRemoved); }; + +export const getDedupedDataSourceVersion = (version: RuleDataSource): RuleDataSource => { + if (version.type === DataSourceType.index_patterns) { + return { + ...version, + index_patterns: uniq(version.index_patterns), + }; + } + return version; +};