[Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748) by yctercero · Pull Request #92748 · elastic/kibana

yctercero · 2021-02-25T02:34:51Z

Summary

Fixes a bug where max_signals was being reverted to it's default value when the rule was edited via the UI.

This PR fixes that and adds tests around the various scenarios.

To test, follow the below steps:

cd x-pack/plugins/security_solution/server/lib/detection_engine/scripts
./post_rule.sh ./rules/queries/query_with_max_signals.json (creates a rule with max_signals of 500)
Via UI, enable/disable rule

./get_rules.sh - check to see that rule continues to have max_signals: 500

Via UI, add an action to prepackaged rule

./get_rules.sh - check to see that rule continues to have max_signals: 500

Via UI, edit rule

./get_rules.sh - check to see that rule continues to have max_signals: 500

Via UI, add an exception

./get_rules.sh - check to see that rule continues to have max_signals: 500

NOTE: The exceptions flow was not previously breaking max_signals but I added it as something to test as it had previously caused some issues on the PATCH.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

yctercero · 2021-03-01T23:06:13Z

x-pack/plugins/security_solution/cypress/tasks/rule_details.ts

  });
 };

-export const deactivatesRule = () => {


not being used anywhere.

elasticmachine · 2021-03-01T23:30:35Z

Pinging @elastic/security-solution (Team: SecuritySolution)

elasticmachine · 2021-03-01T23:30:36Z

Pinging @elastic/security-detections-response (Team:Detections and Resp)

spong · 2021-03-02T01:08:56Z

x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/edit/index.tsx

          rule
        ),
        ...(ruleId ? { id: ruleId } : {}),
+        ...(rule != null ? { max_signals: rule.max_signals } : {}),


spong

Checked out, tested locally and all scenarios worked! Thanks for the fix and test coverage to boot! 🙂 LGTM! 👍

kibanamachine · 2021-03-02T01:35:18Z

💛 Build succeeded, but was flaky

continuous-integration/kibana-ci/pull-request
Commit: 4fecd10
Storybooks Preview
Flaky suites:
- xpack-securitySolutionCypressChrome

Test Failures

Kibana Pipeline / general / "before all" hook for "should contain notes".Timeline notes tab "before all" hook for "should contain notes"

Link to Jenkins

Stack Trace

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

AssertionError: Timed out retrying after 60000ms: Expected to find element: `[data-test-subj="title-999e5df0-7aee-11eb-9ab8-09b681cd86fc"]`, but never found it.

Because this error occurred during a `before all` hook we are skipping the remaining tests in the current suite: `Timeline notes tab`

Although you have test retries enabled, we do not retry tests when `before all` or `after all` hooks fail
    at Object.openTimelineById (http://localhost:6121/__cypress/tests?p=cypress/integration/timelines/notes_tab.spec.ts:16007:15)
    at Context.eval (http://localhost:6121/__cypress/tests?p=cypress/integration/timelines/notes_tab.spec.ts:15041:24)

Kibana Pipeline / general / "after all" hook for "should contain notes".Timeline notes tab "after all" hook for "should contain notes"

Link to Jenkins

Stack Trace

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

CypressError: `cy.filter()` failed because it requires a DOM element.

The subject received was:

  > `undefined`

The previous command that ran was:

  > `cy.get()`

All 2 subject validations failed on this subject.

Because this error occurred during a `after all` hook we are skipping the remaining tests in the current suite: `Timeline notes tab`

Although you have test retries enabled, we do not retry tests when `before all` or `after all` hooks fail
    at ensureElement (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:161322:24)
    at validateType (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:161159:16)
    at Object.ensureSubjectByType (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:161195:9)
    at pushSubjectAndValidate (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:169888:15)
    at Context.<anonymous> (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:170225:18)
From Your Spec Code:
    at Object.closeTimeline (http://localhost:6121/__cypress/tests?p=cypress/integration/timelines/notes_tab.spec.ts:15959:43)
    at Context.eval (http://localhost:6121/__cypress/tests?p=cypress/integration/timelines/notes_tab.spec.ts:15051:20)

Kibana Pipeline / general / "before all" hook for "should contain the right query".Timeline query tab Query tab "before all" hook for "should contain the right query"

Link to Jenkins

Stack Trace

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

AssertionError: Timed out retrying after 60000ms: Expected to find element: `[data-test-subj="title-e1699410-7aee-11eb-9ab8-09b681cd86fc"]`, but never found it.

Because this error occurred during a `before all` hook we are skipping the remaining tests in the current suite: `Timeline query tab`

Although you have test retries enabled, we do not retry tests when `before all` or `after all` hooks fail
    at Object.openTimelineById (http://localhost:6121/__cypress/tests?p=cypress/integration/timelines/query_tab.spec.ts:16058:15)
    at Context.eval (http://localhost:6121/__cypress/tests?p=cypress/integration/timelines/query_tab.spec.ts:15045:28)

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	7.8MB	7.8MB	+48.0B
`triggersActionsUi`	1.6MB	1.5MB	-23.9KB
total			-23.9KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`triggersActionsUi`	104.0KB	104.1KB	+82.0B

Unknown metric groups

async chunk count

id	before	after	diff
`triggersActionsUi`	41	42	+1

History

💔 Build #110016 failed 0cce882
💔 Build #109289 failed f9bb686

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @yctercero

…gnals (elastic#92748) ### Summary Fixes a bug where max_signals was being reverted to it's default value when the rule was edited via the UI.

kibanamachine · 2021-03-02T01:37:52Z

💔 Backport failed

❌ 7.11: Commit could not be cherrypicked due to conflicts
✅ 7.12 / #93157
✅ 7.x / #93158

Successful backport PRs will be merged automatically after passing CI.

To backport manually, check out the target branch and run:
node scripts/backport --pr 92748

…gnals (#92748) (#93157) ### Summary Fixes a bug where max_signals was being reverted to it's default value when the rule was edited via the UI. Co-authored-by: Yara Tercero <yctercero@users.noreply.github.com>

…gnals (elastic#92748) ### Summary Fixes a bug where max_signals was being reverted to it's default value when the rule was edited via the UI. # Conflicts: # x-pack/plugins/security_solution/cypress/integration/detection_rules/custom_query_rule.spec.ts

* master: (42 commits) [Lens] Introduces new chart switcher (elastic#91844) [Lens] fix selection when dragging (elastic#93034) Converts usage collection README to .mdx (elastic#92982) Fix expanding document when using saved search data grid (elastic#92999) [SECURITY SOLUTIONS] Bug case connector (elastic#93104) [Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON (elastic#92025) [Alerting][Docs] Changed alerting documentation to point to a single source of explaining the configurations. (elastic#92942) [APM] Fix hidden search bar in error pages while loading (elastic#84476) (elastic#93139) [DOCS] Fixes links for machine learning alerts (elastic#92744) [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (elastic#92748) [SecuritySolution][Case] Disable cases on detections in read-only mode (elastic#93010) [Security Solution][Case][Bug] Prevent closing collection when pushing (elastic#93095) [Security Solution][Detections][7.12] Critical Threshold Rule Fixes (elastic#92667) Bump ems landing page to 7.12 (elastic#93065) [App Search] Implement various Relevance Tuning states and form actions (elastic#92644) [actions] for simplistic email servers, set rejectUnauthorized to false (elastic#91760) [Security Solution][Case] Migrate category & subcategory fields of ServiceNow ITSM connector (elastic#93092) Hide instances latency distribution chart (elastic#92869) [Maps] fix MapboxDraw import from pointing to dist just pointing to folder (elastic#93087) [Maps] fix results trimmed tooltip message doubles feature count for line and polygon features (elastic#92932) ...

… playwright-ftr-e2e * 'playwright-ftr-e2e' of github.com:shahzad31/kibana: (38 commits) [chore] Enable core's eslint rule: `@ts-expect-error` (elastic#93086) [Lens] Introduces new chart switcher (elastic#91844) [Lens] fix selection when dragging (elastic#93034) Converts usage collection README to .mdx (elastic#92982) Fix expanding document when using saved search data grid (elastic#92999) [SECURITY SOLUTIONS] Bug case connector (elastic#93104) [Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON (elastic#92025) [Alerting][Docs] Changed alerting documentation to point to a single source of explaining the configurations. (elastic#92942) [APM] Fix hidden search bar in error pages while loading (elastic#84476) (elastic#93139) [DOCS] Fixes links for machine learning alerts (elastic#92744) [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (elastic#92748) [SecuritySolution][Case] Disable cases on detections in read-only mode (elastic#93010) [Security Solution][Case][Bug] Prevent closing collection when pushing (elastic#93095) [Security Solution][Detections][7.12] Critical Threshold Rule Fixes (elastic#92667) Bump ems landing page to 7.12 (elastic#93065) [App Search] Implement various Relevance Tuning states and form actions (elastic#92644) [actions] for simplistic email servers, set rejectUnauthorized to false (elastic#91760) [Security Solution][Case] Migrate category & subcategory fields of ServiceNow ITSM connector (elastic#93092) Hide instances latency distribution chart (elastic#92869) [Maps] fix MapboxDraw import from pointing to dist just pointing to folder (elastic#93087) ...

… max_signals (#92748) (#93169) * [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748) ### Summary Fixes a bug where max_signals was being reverted to it's default value when the rule was edited via the UI. # Conflicts: # x-pack/plugins/security_solution/cypress/integration/detection_rules/custom_query_rule.spec.ts * fixing lint issue * Delete bin * Delete kibana * Delete out * Delete testlogs

…gnals (#92748) (#93158) ### Summary Fixes a bug where max_signals was being reverted to it's default value when the rule was edited via the UI. Co-authored-by: Yara Tercero <yctercero@users.noreply.github.com>

… ilm/rollup-v2-action * 'ilm/rollup-v2-action' of github.com:elastic/kibana: (30 commits) Fix expanding document when using saved search data grid (#92999) [SECURITY SOLUTIONS] Bug case connector (#93104) [Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON (#92025) [Alerting][Docs] Changed alerting documentation to point to a single source of explaining the configurations. (#92942) [APM] Fix hidden search bar in error pages while loading (#84476) (#93139) [DOCS] Fixes links for machine learning alerts (#92744) [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748) [SecuritySolution][Case] Disable cases on detections in read-only mode (#93010) [Security Solution][Case][Bug] Prevent closing collection when pushing (#93095) [Security Solution][Detections][7.12] Critical Threshold Rule Fixes (#92667) Bump ems landing page to 7.12 (#93065) [App Search] Implement various Relevance Tuning states and form actions (#92644) [actions] for simplistic email servers, set rejectUnauthorized to false (#91760) [Security Solution][Case] Migrate category & subcategory fields of ServiceNow ITSM connector (#93092) Hide instances latency distribution chart (#92869) [Maps] fix MapboxDraw import from pointing to dist just pointing to folder (#93087) [Maps] fix results trimmed tooltip message doubles feature count for line and polygon features (#92932) [Security Solution][Detecttions] Indicator enrichment tweaks (#92989) [Maps] fix fit to data on heatmap not working (#92697) [Security Solution][Endpoint][Admin] Fixes policy sticky footer save test (#92919) ...

fixing rule edit rule format

f9bb686

yctercero self-assigned this Feb 25, 2021

yctercero added bug Fixes for quality problems that affect the customer experience Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detections and Resp Security Detection Response Team v7.12.0 v7.13.0 v8.0.0 labels Feb 25, 2021

yctercero added 5 commits February 24, 2021 19:10

Merge branch 'master' of github.com:elastic/kibana into max_signals

569b8ed

Merge branch 'master' of github.com:elastic/kibana into max_signals

9caf252

simplify solution and add query

0cce882

Merge branch 'master' of github.com:elastic/kibana into max_signals

02b5e91

adding more tests

4fecd10

yctercero commented Mar 1, 2021

View reviewed changes

yctercero marked this pull request as ready for review March 1, 2021 23:30

yctercero requested a review from a team as a code owner March 1, 2021 23:30

yctercero added the impact:critical This issue should be addressed immediately due to a critical level of impact on the product. label Mar 1, 2021

spong reviewed Mar 2, 2021

View reviewed changes

yctercero added the v7.11.2 label Mar 2, 2021

spong approved these changes Mar 2, 2021

View reviewed changes

yctercero added release_note:fix auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 2, 2021

yctercero enabled auto-merge (squash) March 2, 2021 01:27

yctercero changed the title ~~fixing rule edit rule format~~ [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748) Mar 2, 2021

yctercero merged commit fb13948 into elastic:master Mar 2, 2021

kibanamachine mentioned this pull request Mar 2, 2021

[7.12] [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748) #93157

Merged

kibanamachine mentioned this pull request Mar 2, 2021

[7.x] [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748) #93158

Merged

yctercero mentioned this pull request Mar 2, 2021

[7.11] [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748) #93169

Merged

yctercero deleted the max_signals branch October 13, 2021 06:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748)#92748

[Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748)#92748
yctercero merged 6 commits intoelastic:masterfrom
yctercero:max_signals

yctercero commented Feb 25, 2021 •

edited

Loading

Uh oh!

yctercero Mar 1, 2021

Uh oh!

elasticmachine commented Mar 1, 2021

Uh oh!

elasticmachine commented Mar 1, 2021

Uh oh!

spong Mar 2, 2021

Uh oh!

spong left a comment

Uh oh!

kibanamachine commented Mar 2, 2021

Stack Trace

Stack Trace

Stack Trace

async chunk count

Uh oh!

kibanamachine commented Mar 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

yctercero commented Feb 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

yctercero Mar 1, 2021

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Mar 1, 2021

Uh oh!

elasticmachine commented Mar 1, 2021

Uh oh!

spong Mar 2, 2021

Choose a reason for hiding this comment

Uh oh!

spong left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Mar 2, 2021

💛 Build succeeded, but was flaky

Test Failures

Stack Trace

Stack Trace

Stack Trace

Metrics [docs]

Async chunks

Page load bundle

async chunk count

History

Uh oh!

kibanamachine commented Mar 2, 2021

💔 Backport failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yctercero commented Feb 25, 2021 •

edited

Loading