From b428e117a4921fc96d92efb06464c71d43869579 Mon Sep 17 00:00:00 2001 From: Ievgen Sorokopud Date: Wed, 24 Jan 2024 14:34:54 +0100 Subject: [PATCH 1/3] [Security Solution] "Data view" selector is shown in "Edit filter" view on the Rule Editing page (#174026) (#174922) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Addresses https://github.com/elastic/kibana/issues/174026 These changes fix the issue with filter editing on the rule's editing page when using index patterns instead of data view. **Steps to reproduce**: 1. Create a custom query rule and add a filter 2. Save the rule 3. Edit the rule 4. Edit the filter **Current behaviour**: Right now when user tries to edit the filter the data view picking UI appears even though index patterns were not modified. Screenshot 2024-01-16 at 15 14 23 **Expected behaviour**: Data view picking UI should not be present and previously set field and value options should be shown in the filter editing dialog. Screenshot 2024-01-16 at 15 16 07 **Cause**: The behaviour for the filter editing on rule’s editing page changed in `8.11` with these changes https://github.com/elastic/kibana/pull/166318. We convert `DataViewBase` object without ID set to a `DataView` object with auto-generated ID. This happens each time we try to edit the rule and leads to a different ID which is saved in `filter.meta.index`. Unified search internally checks those IDs to verify whether the filter belongs to provided data view. **Solution**: To solve this issue, we set the data view id explicitly on creating an in-memory data view that represents index patterns and update `filter.meta.index` to use the same ID. ~~**Known issue**: This does not resolve the issue for existing filters. In this case, user will need to update their filters manually.~~ (This was fixed by updating `filter.meta.index` field on rule editing) **Flaky test runner** [ESS 50 times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4927) [Serverless 50 times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4935) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit f0f6274b9563cc453a11a937fe66f3bfbe8311bf) --- .../common/components/query_bar/index.tsx | 10 +- .../rule_edit/custom_query_rule.cy.ts | 176 ++++++++++++------ .../cypress/screens/search_bar.ts | 2 + 3 files changed, 129 insertions(+), 59 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/query_bar/index.tsx b/x-pack/plugins/security_solution/public/common/components/query_bar/index.tsx index 08818172bca5a..0801c59e296fb 100644 --- a/x-pack/plugins/security_solution/public/common/components/query_bar/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/query_bar/index.tsx @@ -126,8 +126,14 @@ export const QueryBar = memo( setDataView(indexPattern); } else if (!isEsql) { const createDataView = async () => { - dv = await data.dataViews.create({ title: indexPattern.title }); + dv = await data.dataViews.create({ id: indexPattern.title, title: indexPattern.title }); setDataView(dv); + + /** + * We update filters and set new data view id to make sure that SearchBar does not show data view picker + * More details in https://github.com/elastic/kibana/issues/174026 + */ + filters.forEach((filter) => (filter.meta.index = indexPattern.title)); }; createDataView(); } @@ -136,7 +142,7 @@ export const QueryBar = memo( data.dataViews.clearInstanceCache(dv?.id); } }; - }, [data.dataViews, indexPattern, isEsql]); + }, [data.dataViews, filters, indexPattern, isEsql]); const timeHistory = useMemo(() => new TimeHistory(new Storage(localStorage)), []); const arrDataView = useMemo(() => (dataView != null ? [dataView] : []), [dataView]); diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts index 041ed42cd3de8..2da17854f4e65 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts @@ -5,6 +5,12 @@ * 2.0. */ +import { + ADD_FILTER_FORM_FIELD_INPUT, + ADD_FILTER_FORM_OPERATOR_FIELD, + GLOBAL_SEARCH_BAR_EDIT_FILTER_MENU_ITEM, + GLOBAL_SEARCH_BAR_FILTER_ITEM, +} from '../../../../screens/search_bar'; import { getExistingRule, getEditedRule } from '../../../../objects/rule'; import { @@ -63,81 +69,137 @@ describe('Custom query rules', { tags: ['@ess', '@serverless', '@brokenInServerl deleteConnectors(); deleteAlertsAndRules(); login(); - createRule(getExistingRule({ rule_id: 'rule1', enabled: true })).then((createdRule) => { - visitEditRulePage(createdRule.body.id); - }); }); - it('Allows a rule to be edited', () => { - const existingRule = getExistingRule(); + context('Basics', () => { + beforeEach(() => { + createRule(getExistingRule({ rule_id: 'rule1', enabled: true })).then((createdRule) => { + visitEditRulePage(createdRule.body.id); + }); + }); + + it('Allows a rule to be edited', () => { + const existingRule = getExistingRule(); + + // expect define step to populate + cy.get(CUSTOM_QUERY_INPUT).should('have.value', existingRule.query); - // expect define step to populate - cy.get(CUSTOM_QUERY_INPUT).should('have.value', existingRule.query); + cy.get(DEFINE_INDEX_INPUT).should('have.text', existingRule.index?.join('')); - cy.get(DEFINE_INDEX_INPUT).should('have.text', existingRule.index?.join('')); + goToAboutStepTab(); - goToAboutStepTab(); + // expect about step to populate + cy.get(RULE_NAME_INPUT).invoke('val').should('eql', existingRule.name); + cy.get(RULE_DESCRIPTION_INPUT).should('have.text', existingRule.description); + cy.get(TAGS_FIELD).should('have.text', existingRule.tags?.join('')); + cy.get(SEVERITY_DROPDOWN).should('have.text', 'High'); + cy.get(DEFAULT_RISK_SCORE_INPUT).invoke('val').should('eql', `${existingRule.risk_score}`); - // expect about step to populate - cy.get(RULE_NAME_INPUT).invoke('val').should('eql', existingRule.name); - cy.get(RULE_DESCRIPTION_INPUT).should('have.text', existingRule.description); - cy.get(TAGS_FIELD).should('have.text', existingRule.tags?.join('')); - cy.get(SEVERITY_DROPDOWN).should('have.text', 'High'); - cy.get(DEFAULT_RISK_SCORE_INPUT).invoke('val').should('eql', `${existingRule.risk_score}`); + goToScheduleStepTab(); - goToScheduleStepTab(); + // expect schedule step to populate + const interval = existingRule.interval; + const intervalParts = interval != null && interval.match(/[0-9]+|[a-zA-Z]+/g); + if (intervalParts) { + const [amount, unit] = intervalParts; + cy.get(SCHEDULE_INTERVAL_AMOUNT_INPUT).invoke('val').should('eql', amount); + cy.get(SCHEDULE_INTERVAL_UNITS_INPUT).invoke('val').should('eql', unit); + } else { + throw new Error('Cannot assert scheduling info on a rule without an interval'); + } - // expect schedule step to populate - const interval = existingRule.interval; - const intervalParts = interval != null && interval.match(/[0-9]+|[a-zA-Z]+/g); - if (intervalParts) { - const [amount, unit] = intervalParts; - cy.get(SCHEDULE_INTERVAL_AMOUNT_INPUT).invoke('val').should('eql', amount); - cy.get(SCHEDULE_INTERVAL_UNITS_INPUT).invoke('val').should('eql', unit); - } else { - throw new Error('Cannot assert scheduling info on a rule without an interval'); - } + goToActionsStepTab(); - goToActionsStepTab(); + addEmailConnectorAndRuleAction('test@example.com', 'Subject'); - addEmailConnectorAndRuleAction('test@example.com', 'Subject'); + cy.get(ACTIONS_SUMMARY_BUTTON).should('have.text', 'Summary of alerts'); + cy.get(ACTIONS_NOTIFY_WHEN_BUTTON).should('have.text', 'Per rule run'); - cy.get(ACTIONS_SUMMARY_BUTTON).should('have.text', 'Summary of alerts'); - cy.get(ACTIONS_NOTIFY_WHEN_BUTTON).should('have.text', 'Per rule run'); + goToAboutStepTab(); + cy.get(TAGS_CLEAR_BUTTON).click(); + fillAboutRule(getEditedRule()); - goToAboutStepTab(); - cy.get(TAGS_CLEAR_BUTTON).click(); - fillAboutRule(getEditedRule()); + cy.intercept('GET', '/api/detection_engine/rules?id*').as('getRule'); - cy.intercept('GET', '/api/detection_engine/rules?id*').as('getRule'); + saveEditedRule(); - saveEditedRule(); + cy.wait('@getRule').then(({ response }) => { + cy.wrap(response?.statusCode).should('eql', 200); + // ensure that editing rule does not modify max_signals + cy.wrap(response?.body.max_signals).should('eql', existingRule.max_signals); + }); - cy.wait('@getRule').then(({ response }) => { - cy.wrap(response?.statusCode).should('eql', 200); - // ensure that editing rule does not modify max_signals - cy.wrap(response?.body.max_signals).should('eql', existingRule.max_signals); + cy.get(RULE_NAME_HEADER).should('contain', `${getEditedRule().name}`); + cy.get(ABOUT_RULE_DESCRIPTION).should('have.text', getEditedRule().description); + cy.get(ABOUT_DETAILS).within(() => { + getDetails(SEVERITY_DETAILS).should('have.text', 'Medium'); + getDetails(RISK_SCORE_DETAILS).should('have.text', `${getEditedRule().risk_score}`); + getDetails(TAGS_DETAILS).should('have.text', expectedEditedtags); + }); + cy.get(INVESTIGATION_NOTES_TOGGLE).click(); + cy.get(ABOUT_INVESTIGATION_NOTES).should('have.text', getEditedRule().note); + cy.get(DEFINITION_DETAILS).within(() => { + getDetails(INDEX_PATTERNS_DETAILS).should( + 'have.text', + expectedEditedIndexPatterns?.join('') + ); + getDetails(CUSTOM_QUERY_DETAILS).should('have.text', getEditedRule().query); + getDetails(RULE_TYPE_DETAILS).should('have.text', 'Query'); + getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None'); + }); + if (getEditedRule().interval) { + cy.get(SCHEDULE_DETAILS).within(() => { + getDetails(RUNS_EVERY_DETAILS).should('have.text', getEditedRule().interval); + }); + } }); + }); - cy.get(RULE_NAME_HEADER).should('contain', `${getEditedRule().name}`); - cy.get(ABOUT_RULE_DESCRIPTION).should('have.text', getEditedRule().description); - cy.get(ABOUT_DETAILS).within(() => { - getDetails(SEVERITY_DETAILS).should('have.text', 'Medium'); - getDetails(RISK_SCORE_DETAILS).should('have.text', `${getEditedRule().risk_score}`); - getDetails(TAGS_DETAILS).should('have.text', expectedEditedtags); + context('With filters', () => { + beforeEach(() => { + createRule( + getExistingRule({ + rule_id: 'rule1', + enabled: true, + filters: [ + { + meta: { + disabled: false, + negate: false, + alias: null, + index: expectedEditedIndexPatterns?.join(','), + key: 'host.name', + field: 'host.name', + type: 'exists', + value: 'exists', + }, + query: { + exists: { + field: 'host.name', + }, + }, + $state: { + store: 'appState', + }, + }, + ], + }) + ).then((createdRule) => { + visitEditRulePage(createdRule.body.id); + }); }); - cy.get(INVESTIGATION_NOTES_TOGGLE).click(); - cy.get(ABOUT_INVESTIGATION_NOTES).should('have.text', getEditedRule().note); - cy.get(DEFINITION_DETAILS).within(() => { - getDetails(INDEX_PATTERNS_DETAILS).should('have.text', expectedEditedIndexPatterns?.join('')); - getDetails(CUSTOM_QUERY_DETAILS).should('have.text', getEditedRule().query); - getDetails(RULE_TYPE_DETAILS).should('have.text', 'Query'); - getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None'); + + it('Filter properly stores index information', () => { + // Check that filter exists on rule edit page + cy.get(GLOBAL_SEARCH_BAR_FILTER_ITEM).should('have.text', 'host.name: exists'); + + // Edit the filter + cy.get(GLOBAL_SEARCH_BAR_FILTER_ITEM).click(); + cy.get(GLOBAL_SEARCH_BAR_EDIT_FILTER_MENU_ITEM).click(); + + // Check that correct values are propagated in the filter editing dialog + cy.get(ADD_FILTER_FORM_FIELD_INPUT).should('have.value', 'host.name'); + cy.get(ADD_FILTER_FORM_OPERATOR_FIELD).should('have.value', 'exists'); }); - if (getEditedRule().interval) { - cy.get(SCHEDULE_DETAILS).within(() => { - getDetails(RUNS_EVERY_DETAILS).should('have.text', getEditedRule().interval); - }); - } }); }); diff --git a/x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts b/x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts index 36b3accb3e5a6..3e700b8207939 100644 --- a/x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts +++ b/x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts @@ -38,6 +38,8 @@ export const GLOBAL_SEARCH_BAR_FILTER_ITEM_DELETE = '#popoverFor_filter0 button[ export const GLOBAL_SEARCH_BAR_PINNED_FILTER = '.globalFilterItem-isPinned'; +export const GLOBAL_SEARCH_BAR_EDIT_FILTER_MENU_ITEM = '[data-test-subj="editFilter"]'; + export const LOCAL_KQL_INPUT = `[data-test-subj="unifiedQueryInput"] textarea`; export const GLOBAL_KQL_INPUT = `[data-test-subj="filters-global-container"] ${LOCAL_KQL_INPUT}`; From 532178bfae92822caa578d299026fdde28989e5e Mon Sep 17 00:00:00 2001 From: Ievgen Sorokopud Date: Thu, 25 Jan 2024 15:17:59 +0100 Subject: [PATCH 2/3] [Security Solution] Fix broken tests in 8.12 (#174026) --- .../public/common/components/query_bar/index.tsx | 7 +++++-- .../detection_engine/rule_edit/custom_query_rule.cy.ts | 4 ++-- .../cypress/screens/search_bar.ts | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/query_bar/index.tsx b/x-pack/plugins/security_solution/public/common/components/query_bar/index.tsx index 0801c59e296fb..c71abe9b7f2f0 100644 --- a/x-pack/plugins/security_solution/public/common/components/query_bar/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/query_bar/index.tsx @@ -63,6 +63,7 @@ export const QueryBar = memo( }) => { const { data } = useKibana().services; const [dataView, setDataView] = useState(); + const [searchBarFilters, setSearchBarFilters] = useState(filters); const onQuerySubmit = useCallback( (payload: { dateRange: TimeRange; query?: Query | AggregateQuery }) => { if (payload.query != null && !deepEqual(payload.query, filterQuery)) { @@ -133,7 +134,9 @@ export const QueryBar = memo( * We update filters and set new data view id to make sure that SearchBar does not show data view picker * More details in https://github.com/elastic/kibana/issues/174026 */ - filters.forEach((filter) => (filter.meta.index = indexPattern.title)); + const updatedFilters = [...filters]; + updatedFilters.forEach((filter) => (filter.meta.index = indexPattern.title)); + setSearchBarFilters(updatedFilters); }; createDataView(); } @@ -151,7 +154,7 @@ export const QueryBar = memo( showSubmitButton={false} dateRangeFrom={dateRangeFrom} dateRangeTo={dateRangeTo} - filters={filters} + filters={searchBarFilters} indexPatterns={arrDataView} isLoading={isLoading} isRefreshPaused={isRefreshPaused} diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts index 2da17854f4e65..f2b6d4c3440a7 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts @@ -198,8 +198,8 @@ describe('Custom query rules', { tags: ['@ess', '@serverless', '@brokenInServerl cy.get(GLOBAL_SEARCH_BAR_EDIT_FILTER_MENU_ITEM).click(); // Check that correct values are propagated in the filter editing dialog - cy.get(ADD_FILTER_FORM_FIELD_INPUT).should('have.value', 'host.name'); - cy.get(ADD_FILTER_FORM_OPERATOR_FIELD).should('have.value', 'exists'); + cy.get(ADD_FILTER_FORM_FIELD_INPUT).should('contain.text', 'host.name'); + cy.get(ADD_FILTER_FORM_OPERATOR_FIELD).should('contain.text', 'exists'); }); }); }); diff --git a/x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts b/x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts index 3e700b8207939..2af904a72f138 100644 --- a/x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts +++ b/x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts @@ -18,13 +18,13 @@ export const GET_LOCAL_SEARCH_BAR_SUBMIT_BUTTON = (localSearchBarSelector: strin `${localSearchBarSelector ?? ''} [data-test-subj="querySubmitButton"]`; export const ADD_FILTER_FORM_FIELD_INPUT = - '[data-test-subj="filterFieldSuggestionList"] input[data-test-subj="comboBoxSearchInput"]'; + '[data-test-subj="filterFieldSuggestionList"] [data-test-subj="euiComboBoxPill"]'; export const ADD_FILTER_FORM_FIELD_OPTION = (value: string) => `[data-test-subj="comboBoxOptionsList filterFieldSuggestionList-optionsList"] button[title="${value}"]`; export const ADD_FILTER_FORM_OPERATOR_FIELD = - '[data-test-subj="filterOperatorList"] input[data-test-subj="comboBoxSearchInput"]'; + '[data-test-subj="filterOperatorList"] [data-test-subj="euiComboBoxPill"]'; export const ADD_FILTER_FORM_FILTER_VALUE_INPUT = '[data-test-subj="filterParams"] input'; From 9a4702dedf50c784d49d00490daf63a80e2f6120 Mon Sep 17 00:00:00 2001 From: Ievgen Sorokopud Date: Thu, 25 Jan 2024 17:19:44 +0100 Subject: [PATCH 3/3] Fix broken tests --- .../detection_engine/rule_edit/custom_query_rule.cy.ts | 8 ++++---- .../cypress/screens/search_bar.ts | 6 ++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts index f2b6d4c3440a7..c3e18cf3a12e4 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts @@ -6,8 +6,8 @@ */ import { - ADD_FILTER_FORM_FIELD_INPUT, - ADD_FILTER_FORM_OPERATOR_FIELD, + ADD_FILTER_FORM_FIELD_SELECTED_VALUE, + ADD_FILTER_FORM_OPERATOR_FIELD_SELECTED_VALUE, GLOBAL_SEARCH_BAR_EDIT_FILTER_MENU_ITEM, GLOBAL_SEARCH_BAR_FILTER_ITEM, } from '../../../../screens/search_bar'; @@ -198,8 +198,8 @@ describe('Custom query rules', { tags: ['@ess', '@serverless', '@brokenInServerl cy.get(GLOBAL_SEARCH_BAR_EDIT_FILTER_MENU_ITEM).click(); // Check that correct values are propagated in the filter editing dialog - cy.get(ADD_FILTER_FORM_FIELD_INPUT).should('contain.text', 'host.name'); - cy.get(ADD_FILTER_FORM_OPERATOR_FIELD).should('contain.text', 'exists'); + cy.get(ADD_FILTER_FORM_FIELD_SELECTED_VALUE).should('contain.text', 'host.name'); + cy.get(ADD_FILTER_FORM_OPERATOR_FIELD_SELECTED_VALUE).should('contain.text', 'exists'); }); }); }); diff --git a/x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts b/x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts index 2af904a72f138..2e244c9e7764e 100644 --- a/x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts +++ b/x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts @@ -18,12 +18,18 @@ export const GET_LOCAL_SEARCH_BAR_SUBMIT_BUTTON = (localSearchBarSelector: strin `${localSearchBarSelector ?? ''} [data-test-subj="querySubmitButton"]`; export const ADD_FILTER_FORM_FIELD_INPUT = + '[data-test-subj="filterFieldSuggestionList"] input[data-test-subj="comboBoxSearchInput"]'; + +export const ADD_FILTER_FORM_FIELD_SELECTED_VALUE = '[data-test-subj="filterFieldSuggestionList"] [data-test-subj="euiComboBoxPill"]'; export const ADD_FILTER_FORM_FIELD_OPTION = (value: string) => `[data-test-subj="comboBoxOptionsList filterFieldSuggestionList-optionsList"] button[title="${value}"]`; export const ADD_FILTER_FORM_OPERATOR_FIELD = + '[data-test-subj="filterOperatorList"] input[data-test-subj="comboBoxSearchInput"]'; + +export const ADD_FILTER_FORM_OPERATOR_FIELD_SELECTED_VALUE = '[data-test-subj="filterOperatorList"] [data-test-subj="euiComboBoxPill"]'; export const ADD_FILTER_FORM_FILTER_VALUE_INPUT = '[data-test-subj="filterParams"] input';