-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Security Solution] Fix DataSource
payload creation during rule upgrade with MERGED
pick_version
#197262
[Security Solution] Fix DataSource
payload creation during rule upgrade with MERGED
pick_version
#197262
Changes from all commits
9c15bf2
d4fe04a
4e13441
5e5912b
fb5f822
d4b0c73
46e0ca7
ca6dd5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
Comment on lines
+259
to
+260
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these fields relevant for the test? |
||
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. | ||
Comment on lines
+280
to
+281
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you expand on in which cases the |
||
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', () => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could use a short comment as to why we do this similar to the one on line
198