From 1a75c3703fdcf54a3163b060b700918c395aac41 Mon Sep 17 00:00:00 2001 From: Davis Plumlee <56367316+dplumlee@users.noreply.github.com> Date: Thu, 22 Feb 2024 14:57:41 -0500 Subject: [PATCH] [Security Solution] Per-field diffs test coverage (#177399) ## Summary Addresses test coverage acceptance criteria for https://github.com/elastic/kibana/issues/166489 Adds test coverage in accordance to the recently merged [test plan](https://github.com/elastic/kibana/pull/176474) [Flaky test runner](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5279) (cherry picked from commit 3c34b535ceac5a5c869719998d9e03a0d44ce21a) --- .../installation_and_upgrade.md | 3 +- .../diff_components/field_diff.tsx | 2 +- .../diff_components/panel_wrapper.tsx | 8 +- .../diff_components/rule_diff_section.tsx | 6 +- .../per_field_rule_diff_tab.test.tsx | 172 ++++++++++++++++++ .../rule_details/per_field_rule_diff_tab.tsx | 24 ++- .../prebuilt_rules_preview.cy.ts | 115 +++++++++++- .../cypress/screens/alerts_detection_rules.ts | 3 + 8 files changed, 318 insertions(+), 15 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_rule_diff_tab.test.tsx diff --git a/x-pack/plugins/security_solution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md b/x-pack/plugins/security_solution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md index 41e379906eb42..24aaf34764034 100644 --- a/x-pack/plugins/security_solution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md +++ b/x-pack/plugins/security_solution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md @@ -978,13 +978,14 @@ Then the preview should display the changes for the newly selected rule #### **Scenario: User can see changes when updated rule is a different rule type** -**Automation**: 1 UI integration test +**Automation**: 1 e2e test ```Gherkin Given a prebuilt rule is installed in Kibana And this rule has an update available that changes the rule type When user opens the upgrade preview Then the rule type changes should be displayed in grouped field diffs with corresponding query fields +# When tooltip enhancement is added, this step needs to be added to the corresponding test scenario And a tooltip is displayed with information about changing rule types ``` diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/diff_components/field_diff.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/diff_components/field_diff.tsx index b817d6ab3882c..86c85b7b7a89b 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/diff_components/field_diff.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/diff_components/field_diff.tsx @@ -26,7 +26,7 @@ const SubFieldComponent = ({ {shouldShowSubtitles ? ( - +

{fieldToDisplayNameMap[fieldName] ?? startCase(camelCase(fieldName))}

 ) : null} diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/diff_components/panel_wrapper.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/diff_components/panel_wrapper.tsx index 789e9abf43395..f0c86a68cafaf 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/diff_components/panel_wrapper.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/diff_components/panel_wrapper.tsx @@ -20,7 +20,7 @@ export const RuleDiffPanelWrapper = ({ fieldName, children }: RuleDiffPanelWrapp const { euiTheme } = useEuiTheme(); return ( - + +
{fieldToDisplayNameMap[fieldName] ?? startCase(camelCase(fieldName))}
 } > - {children} + + {children} + ); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/diff_components/rule_diff_section.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/diff_components/rule_diff_section.tsx index fcfdf930fbd0f..a75a8db426f06 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/diff_components/rule_diff_section.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/diff_components/rule_diff_section.tsx @@ -13,19 +13,21 @@ import { FieldGroupDiffComponent } from './field_diff'; interface RuleDiffSectionProps { title: string; fieldGroups: FieldsGroupDiff[]; + dataTestSubj?: string; } -export const RuleDiffSection = ({ title, fieldGroups }: RuleDiffSectionProps) => ( +export const RuleDiffSection = ({ title, fieldGroups, dataTestSubj }: RuleDiffSectionProps) => ( <> +

{title}

 } diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_rule_diff_tab.test.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_rule_diff_tab.test.tsx new file mode 100644 index 0000000000000..3b24b7022acdd --- /dev/null +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_rule_diff_tab.test.tsx @@ -0,0 +1,172 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { + KqlQueryType, + ThreeWayDiffOutcome, + ThreeWayMergeOutcome, +} from '../../../../../common/api/detection_engine'; +import type { PartialRuleDiff } from '../../../../../common/api/detection_engine'; +import { TestProviders } from '../../../../common/mock'; +import { render } from '@testing-library/react'; +import React from 'react'; +import { PerFieldRuleDiffTab } from './per_field_rule_diff_tab'; + +const ruleFieldsDiffBaseFieldsMock = { + diff_outcome: ThreeWayDiffOutcome.StockValueCanUpdate, + has_conflict: false, + has_update: true, + merge_outcome: ThreeWayMergeOutcome.Target, +}; + +const ruleFieldsDiffMock: PartialRuleDiff = { + fields: { + version: { + ...ruleFieldsDiffBaseFieldsMock, + base_version: 1, + current_version: 1, + merged_version: 2, + target_version: 2, + }, + }, + has_conflict: false, +}; + +const renderPerFieldRuleDiffTab = (ruleDiff: PartialRuleDiff) => { + return render( + + + + ); +}; + +describe('PerFieldRuleDiffTab', () => { + test('Field groupings should be rendered together in the same accordion panel', () => { + const mockData: PartialRuleDiff = { + ...ruleFieldsDiffMock, + fields: { + kql_query: { + ...ruleFieldsDiffBaseFieldsMock, + base_version: { + filters: [], + language: 'lucene', + query: 'old query', + type: KqlQueryType.inline_query, + }, + current_version: { + filters: [], + language: 'lucene', + query: 'old query', + type: KqlQueryType.inline_query, + }, + merged_version: { + filters: [], + language: 'kuery', + query: 'new query', + type: KqlQueryType.inline_query, + }, + target_version: { + filters: [], + language: 'kuery', + query: 'new query', + type: KqlQueryType.inline_query, + }, + }, + }, + }; + const wrapper = renderPerFieldRuleDiffTab(mockData); + + const matchedSubtitleElements = wrapper.queryAllByTestId('ruleUpgradePerFieldDiffSubtitle'); + const subtitles = matchedSubtitleElements.map((element) => element.textContent); + + // `filters` and `type` have not changed between versions so shouldn't be displayed + expect(subtitles).toEqual(['Query', 'Language']); + }); + + describe('Undefined values are displayed with empty diffs', () => { + test('Displays only an updated field value when changed from undefined', () => { + const mockData: PartialRuleDiff = { + ...ruleFieldsDiffMock, + fields: { + timestamp_field: { + ...ruleFieldsDiffBaseFieldsMock, + base_version: undefined, + current_version: undefined, + merged_version: 'new timestamp field', + target_version: 'new timestamp field', + }, + }, + }; + const wrapper = renderPerFieldRuleDiffTab(mockData); + const diffContent = wrapper.getByTestId('ruleUpgradePerFieldDiffContent').textContent; + + // Only the new timestamp field should be displayed + expect(diffContent).toEqual('+new timestamp field'); + }); + + test('Displays only an outdated field value when incoming update is undefined', () => { + const mockData: PartialRuleDiff = { + ...ruleFieldsDiffMock, + fields: { + timestamp_field: { + ...ruleFieldsDiffBaseFieldsMock, + base_version: 'old timestamp field', + current_version: 'old timestamp field', + merged_version: undefined, + target_version: undefined, + }, + }, + }; + const wrapper = renderPerFieldRuleDiffTab(mockData); + const diffContent = wrapper.getByTestId('ruleUpgradePerFieldDiffContent').textContent; + + // Only the old timestamp_field should be displayed + expect(diffContent).toEqual('-old timestamp field'); + }); + }); + + test('Field diff components have the same grouping and order as in rule details overview', () => { + const mockData: PartialRuleDiff = { + ...ruleFieldsDiffMock, + fields: { + setup: { + ...ruleFieldsDiffBaseFieldsMock, + base_version: 'old setup', + current_version: 'old setup', + merged_version: 'new setup', + target_version: 'new setup', + }, + timestamp_field: { + ...ruleFieldsDiffBaseFieldsMock, + base_version: undefined, + current_version: undefined, + merged_version: 'new timestamp', + target_version: 'new timestamp', + }, + name: { + ...ruleFieldsDiffBaseFieldsMock, + base_version: 'old name', + current_version: 'old name', + merged_version: 'new name', + target_version: 'new name', + }, + }, + }; + const wrapper = renderPerFieldRuleDiffTab(mockData); + + const matchedSectionElements = wrapper.queryAllByTestId('ruleUpgradePerFieldDiffSectionHeader'); + const sectionLabels = matchedSectionElements.map((element) => element.textContent); + + // Schedule doesn't have any fields in the diff and shouldn't be displayed + expect(sectionLabels).toEqual(['About', 'Definition', 'Setup guide']); + + const matchedFieldElements = wrapper.queryAllByTestId('ruleUpgradePerFieldDiffLabel'); + const fieldLabels = matchedFieldElements.map((element) => element.textContent); + + expect(fieldLabels).toEqual(['Name', 'Timestamp Field', 'Setup']); + }); +}); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_rule_diff_tab.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_rule_diff_tab.tsx index 2e39428a0dc12..4a90f8624d21e 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_rule_diff_tab.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_rule_diff_tab.tsx @@ -43,16 +43,32 @@ export const PerFieldRuleDiffTab = ({ ruleDiff }: PerFieldRuleDiffTabProps) => { <> {aboutFields.length !== 0 && ( - + )} {definitionFields.length !== 0 && ( - + )} {scheduleFields.length !== 0 && ( - + )} {setupFields.length !== 0 && ( - + )} ); diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management/prebuilt_rules/prebuilt_rules_preview.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management/prebuilt_rules/prebuilt_rules_preview.cy.ts index 0cd75c6162be3..db39136abc963 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management/prebuilt_rules/prebuilt_rules_preview.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management/prebuilt_rules/prebuilt_rules_preview.cy.ts @@ -19,6 +19,8 @@ import { INSTALL_PREBUILT_RULE_PREVIEW, UPDATE_PREBUILT_RULE_PREVIEW, UPDATE_PREBUILT_RULE_BUTTON, + PER_FIELD_DIFF_WRAPPER, + PER_FIELD_DIFF_DEFINITION_SECTION, } from '../../../../screens/alerts_detection_rules'; import { RULE_MANAGEMENT_PAGE_BREADCRUMB } from '../../../../screens/breadcrumbs'; import { @@ -72,7 +74,7 @@ const TEST_ENV_TAGS = ['@ess', '@serverless']; const PREVIEW_TABS = { OVERVIEW: 'Overview', JSON_VIEW: 'JSON view', - UPDATES: 'Updates', + UPDATES: 'Updates', // Currently open by default on upgrade }; describe('Detection rules, Prebuilt Rules Installation and Update workflow', () => { @@ -851,7 +853,6 @@ describe('Detection rules, Prebuilt Rules Installation and Update workflow', () clickRuleUpdatesTab(); openRuleUpdatePreview(UPDATED_CUSTOM_QUERY_INDEX_PATTERN_RULE['security-rule'].name); - assertSelectedPreviewTab(PREVIEW_TABS.JSON_VIEW); selectPreviewTab(PREVIEW_TABS.OVERVIEW); const { index } = UPDATED_CUSTOM_QUERY_INDEX_PATTERN_RULE['security-rule'] as { @@ -879,7 +880,6 @@ describe('Detection rules, Prebuilt Rules Installation and Update workflow', () closeRulePreview(); openRuleUpdatePreview(UPDATED_SAVED_QUERY_DATA_VIEW_RULE['security-rule'].name); - assertSelectedPreviewTab(PREVIEW_TABS.JSON_VIEW); selectPreviewTab(PREVIEW_TABS.OVERVIEW); const { data_view_id: dataViewId } = UPDATED_SAVED_QUERY_DATA_VIEW_RULE[ @@ -1074,7 +1074,7 @@ describe('Detection rules, Prebuilt Rules Installation and Update workflow', () clickRuleUpdatesTab(); openRuleUpdatePreview(OUTDATED_RULE_1['security-rule'].name); - assertSelectedPreviewTab(PREVIEW_TABS.JSON_VIEW); + selectPreviewTab(PREVIEW_TABS.JSON_VIEW); cy.get(UPDATE_PREBUILT_RULE_PREVIEW).contains('Current rule').should('be.visible'); cy.get(UPDATE_PREBUILT_RULE_PREVIEW).contains('Elastic update').should('be.visible'); @@ -1136,5 +1136,112 @@ describe('Detection rules, Prebuilt Rules Installation and Update workflow', () cy.get(UPDATE_PREBUILT_RULE_PREVIEW).contains('execution_summary').should('not.exist'); }); }); + + describe( + 'Viewing rule changes in per-field diff view', + { + tags: TEST_ENV_TAGS, + env: { + ftrConfig: { + kbnServerArgs: [ + `--xpack.securitySolution.enableExperimental=${JSON.stringify([ + 'perFieldPrebuiltRulesDiffingEnabled', + ])}`, + ], + }, + }, + }, + () => { + it('User can see changes in a side-by-side per-field diff view', () => { + clickRuleUpdatesTab(); + + openRuleUpdatePreview(OUTDATED_RULE_1['security-rule'].name); + assertSelectedPreviewTab(PREVIEW_TABS.UPDATES); // Should be open by default + + cy.get(UPDATE_PREBUILT_RULE_PREVIEW).contains('Current rule').should('be.visible'); + cy.get(UPDATE_PREBUILT_RULE_PREVIEW).contains('Elastic update').should('be.visible'); + + cy.get(PER_FIELD_DIFF_WRAPPER).should('have.length', 2); + + /* Version should be the first field in the order */ + cy.get(PER_FIELD_DIFF_WRAPPER).first().contains('Version').should('be.visible'); + cy.get(PER_FIELD_DIFF_WRAPPER).first().contains('1').should('be.visible'); + cy.get(PER_FIELD_DIFF_WRAPPER).first().contains('2').should('be.visible'); + + cy.get(PER_FIELD_DIFF_WRAPPER).last().contains('Name').should('be.visible'); + cy.get(PER_FIELD_DIFF_WRAPPER).last().contains('Outdated rule 1').should('be.visible'); + cy.get(PER_FIELD_DIFF_WRAPPER).last().contains('Updated rule 1').should('be.visible'); + }); + + it('User can switch between rules upgrades without closing flyout', () => { + clickRuleUpdatesTab(); + + openRuleUpdatePreview(OUTDATED_RULE_1['security-rule'].name); + assertSelectedPreviewTab(PREVIEW_TABS.UPDATES); // Should be open by default + + /* Version should be the first field in the order */ + cy.get(PER_FIELD_DIFF_WRAPPER).first().contains('Version').should('be.visible'); + cy.get(PER_FIELD_DIFF_WRAPPER).first().contains('1').should('be.visible'); + cy.get(PER_FIELD_DIFF_WRAPPER).first().contains('2').should('be.visible'); + + cy.get(PER_FIELD_DIFF_WRAPPER).last().contains('Name').should('be.visible'); + cy.get(PER_FIELD_DIFF_WRAPPER).last().contains('Outdated rule 1').should('be.visible'); + cy.get(PER_FIELD_DIFF_WRAPPER).last().contains('Updated rule 1').should('be.visible'); + + /* Select another rule without closing the preview for the current rule */ + openRuleUpdatePreview(OUTDATED_RULE_2['security-rule'].name); + + /* Make sure the per-field diff is displayed for the newly selected rule */ + cy.get(PER_FIELD_DIFF_WRAPPER).last().contains('Name').should('be.visible'); + cy.get(UPDATE_PREBUILT_RULE_PREVIEW).contains('Outdated rule 2').should('be.visible'); + cy.get(UPDATE_PREBUILT_RULE_PREVIEW).contains('Updated rule 2').should('be.visible'); + cy.get(UPDATE_PREBUILT_RULE_PREVIEW).contains('Outdated rule 1').should('not.exist'); + cy.get(UPDATE_PREBUILT_RULE_PREVIEW).contains('Updated rule 1').should('not.exist'); + + cy.get(PER_FIELD_DIFF_WRAPPER).first().contains('Version').should('be.visible'); + cy.get(PER_FIELD_DIFF_WRAPPER).first().contains('1').should('be.visible'); + cy.get(PER_FIELD_DIFF_WRAPPER).first().contains('2').should('be.visible'); + }); + + it('User can see changes when updated rule is a different rule type', () => { + const OUTDATED_RULE_WITH_QUERY_TYPE = createRuleAssetSavedObject({ + name: 'Query rule', + rule_id: 'rule_id', + version: 1, + type: 'query', + language: 'kuery', + }); + const UPDATED_RULE_WITH_EQL_TYPE = createRuleAssetSavedObject({ + language: 'eql', + name: 'EQL rule', + rule_id: 'rule_id', + version: 2, + type: 'eql', + }); + /* Create a new rule and install it */ + createAndInstallMockedPrebuiltRules([OUTDATED_RULE_WITH_QUERY_TYPE]); + /* Create a second version of the rule, making it available for update */ + installPrebuiltRuleAssets([UPDATED_RULE_WITH_EQL_TYPE]); + + cy.reload(); + clickRuleUpdatesTab(); + + openRuleUpdatePreview(OUTDATED_RULE_WITH_QUERY_TYPE['security-rule'].name); + assertSelectedPreviewTab(PREVIEW_TABS.UPDATES); // Should be open by default + + cy.get(UPDATE_PREBUILT_RULE_PREVIEW).contains('Current rule').should('be.visible'); + cy.get(UPDATE_PREBUILT_RULE_PREVIEW).contains('Elastic update').should('be.visible'); + + cy.get(PER_FIELD_DIFF_WRAPPER).should('have.length', 5); + + cy.get(PER_FIELD_DIFF_DEFINITION_SECTION).contains('Type').should('be.visible'); + cy.get(PER_FIELD_DIFF_DEFINITION_SECTION).contains('query').should('be.visible'); + cy.get(PER_FIELD_DIFF_DEFINITION_SECTION).contains('eql').should('be.visible'); + + cy.get(PER_FIELD_DIFF_DEFINITION_SECTION).contains('KQL query').should('exist'); + cy.get(PER_FIELD_DIFF_DEFINITION_SECTION).contains('EQL query').should('exist'); + }); + } + ); }); }); diff --git a/x-pack/test/security_solution_cypress/cypress/screens/alerts_detection_rules.ts b/x-pack/test/security_solution_cypress/cypress/screens/alerts_detection_rules.ts index 72767e9f137b4..d3fcfdf4c1cf1 100644 --- a/x-pack/test/security_solution_cypress/cypress/screens/alerts_detection_rules.ts +++ b/x-pack/test/security_solution_cypress/cypress/screens/alerts_detection_rules.ts @@ -339,3 +339,6 @@ export const NEW_TERMS_WINDOW_SIZE_VALUE = '[data-test-subj^="newTermsWindowSize export const ESQL_QUERY_TITLE = '[data-test-subj="esqlQueryPropertyTitle"]'; export const ESQL_QUERY_VALUE = '[data-test-subj="esqlQueryPropertyValue"]'; + +export const PER_FIELD_DIFF_WRAPPER = '[data-test-subj="ruleUpgradePerFieldDiffWrapper"]'; +export const PER_FIELD_DIFF_DEFINITION_SECTION = '[data-test-subj="perFieldDiffDefinitionSection"]';