Skip to content

[SIEM] Fixes Modification of ML Rules#60662

Merged
rylnd merged 6 commits intoelastic:masterfrom
rylnd:fix_rule_modifications
Mar 20, 2020
Merged

[SIEM] Fixes Modification of ML Rules#60662
rylnd merged 6 commits intoelastic:masterfrom
rylnd:fix_rule_modifications

Conversation

@rylnd
Copy link
Copy Markdown
Contributor

@rylnd rylnd commented Mar 19, 2020 •
edited
Loading

Summary

  • Fixes updating, patching, and importing of ML Rules on the server
  • Restricts Rule Edit view to prevent users from changing an existing Rule's type
  • Backfills some regression unit tests for functions that were not being tested

A few of these issues were not caught by Typescript because while we require all these rule fields to be passed into e.g. updateRules, we do not type the parameters that we pass to the relevant ES client(s).

@rylnd rylnd added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Mar 19, 2020
@rylnd rylnd self-assigned this Mar 19, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Mar 19, 2020

Pinging @elastic/siem (Team:SIEM)

rylnd
rylnd commented Mar 19, 2020
View reviewed changes
x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/import_rules_route.ts
@@ -228,6 +228,8 @@ export const importRulesRoute = (router: IRouter, config: LegacyServices['config
references,
note,
version,
Copy link
Copy Markdown
Contributor Author

@rylnd rylnd Mar 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FrankHassanabad I noticed that lists is absent from this logic branch, meaning that we wouldn't be able to, on import, overwrite a rule's lists. Happy to add that here if you want.

Copy link
Copy Markdown
Contributor

@FrankHassanabad FrankHassanabad Mar 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you can here if you want or we can do another PR for it. But yes, it is missing here.

Copy link
Copy Markdown
Contributor

@FrankHassanabad FrankHassanabad Mar 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for finding it.

@rylnd rylnd mentioned this pull request Mar 19, 2020
Closed
11 tasks
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Mar 19, 2020

💚 Build Succeeded

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

FrankHassanabad
FrankHassanabad approved these changes Mar 20, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small, bug fixes, and tests, what more could you ask for?

LGTM

@rylnd rylnd merged commit 182acdb into elastic:master Mar 20, 2020
@rylnd rylnd deleted the fix_rule_modifications branch March 20, 2020 00:33
@rylnd rylnd mentioned this pull request Mar 20, 2020
Merged
patrykkopycinski pushed a commit that referenced this pull request Mar 20, 2020
@rylnd
[SIEM] Fixes Modification of ML Rules (#60662) (#60715)
41f2a9c
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 20, 2020
@gmmorris
Merge branch 'master' into alerting/api-constant
377acfc 
* master: (52 commits)
  [SIEM] Fix types in rules tests (elastic#60736)
  [Alerting] prevent flickering when fields are updated in an alert (elastic#60666)
  License checks for actions plugin (elastic#59070)
  Implemented ability to clear and properly validate alert interval (elastic#60571)
  WebElementWrapper: add findByTestSubject/findAllByTestSubject to search with data-test-subj (elastic#60568)
  [Maps] Update layer dependencies to NP (elastic#59585)
  [Discover] Remove StateManagementConfigProvider (elastic#60221)
  [ML] Listing all categorization wizard checks (elastic#60502)
  [Upgrade Assistant] First iteration of batch reindex docs (elastic#59887)
  [SIEM] Export timeline (elastic#58368)
  [SIEM] Add support for actions and throttle in Rules (elastic#59641)
  Fix ace a11y listener (elastic#60639)
  Add addInfo toast to core notifications service (elastic#60574)
  fix test description (elastic#60638)
  [SIEM] Cypress screenshots upload to google cloud (elastic#60556)
  [canvas/shareable_runtime] sync sass loaders with kbn/optimizer (elastic#60653)
  [SIEM] Fixes Modification of ML Rules (elastic#60662)
  [SIEM] [Case] Bulk status update, add comment avatar, id => title in breadcrumbs (elastic#60410)
  [Alerting] add functional tests for index threshold alertType (elastic#60597)
  [Ingest]EMT-248: add post action request handler and resources (elastic#60581)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 20, 2020
@gmmorris
Merge branch 'master' into alerting/empty-list
e8c7be8 
* master: (55 commits)
  Update dependency @elastic/charts to v18.1.0 (elastic#60578)
  Only set timezone when user setting is a valid timezone (elastic#57850)
  [NP] Remove `ui/agg_types` dependencies and move paginated table to kibana_legacy (elastic#60276)
  [SIEM] Fix types in rules tests (elastic#60736)
  [Alerting] prevent flickering when fields are updated in an alert (elastic#60666)
  License checks for actions plugin (elastic#59070)
  Implemented ability to clear and properly validate alert interval (elastic#60571)
  WebElementWrapper: add findByTestSubject/findAllByTestSubject to search with data-test-subj (elastic#60568)
  [Maps] Update layer dependencies to NP (elastic#59585)
  [Discover] Remove StateManagementConfigProvider (elastic#60221)
  [ML] Listing all categorization wizard checks (elastic#60502)
  [Upgrade Assistant] First iteration of batch reindex docs (elastic#59887)
  [SIEM] Export timeline (elastic#58368)
  [SIEM] Add support for actions and throttle in Rules (elastic#59641)
  Fix ace a11y listener (elastic#60639)
  Add addInfo toast to core notifications service (elastic#60574)
  fix test description (elastic#60638)
  [SIEM] Cypress screenshots upload to google cloud (elastic#60556)
  [canvas/shareable_runtime] sync sass loaders with kbn/optimizer (elastic#60653)
  [SIEM] Fixes Modification of ML Rules (elastic#60662)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 20, 2020
@gmmorris
Merge branch 'alerting/tls-warning' of github.com:gmmorris/kibana int…
f04878a 
…o alerting/tls-warning

* 'alerting/tls-warning' of github.com:gmmorris/kibana: (32 commits)
  [ML] Listing all categorization wizard checks (elastic#60502)
  [Upgrade Assistant] First iteration of batch reindex docs (elastic#59887)
  [SIEM] Export timeline (elastic#58368)
  [SIEM] Add support for actions and throttle in Rules (elastic#59641)
  Fix ace a11y listener (elastic#60639)
  Add addInfo toast to core notifications service (elastic#60574)
  fix test description (elastic#60638)
  [SIEM] Cypress screenshots upload to google cloud (elastic#60556)
  [canvas/shareable_runtime] sync sass loaders with kbn/optimizer (elastic#60653)
  [SIEM] Fixes Modification of ML Rules (elastic#60662)
  [SIEM] [Case] Bulk status update, add comment avatar, id => title in breadcrumbs (elastic#60410)
  [Alerting] add functional tests for index threshold alertType (elastic#60597)
  [Ingest]EMT-248: add post action request handler and resources (elastic#60581)
  Return incident's url (elastic#60617)
  [Endpoint] TEST: GET alert details - boundary test for first alert retrieval (elastic#60320)
  [ML] Transforms: Fix pivot preview table mapping. (elastic#60609)
  [Endpoint] Log random seed for sample data CLI to console (elastic#60646)
  Use common event model for determining if event is v0 or v1 (elastic#60667)
  Disables PR Project Assigner workflow
  [Reporting] Allow reports to be deleted in Management > Kibana > Reporting (elastic#60077)
  ...
@rylnd rylnd mentioned this pull request Mar 20, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yctercero yctercero Awaiting requested review from yctercero

@dhurley14 dhurley14 Awaiting requested review from dhurley14

1 more reviewer

@FrankHassanabad FrankHassanabad FrankHassanabad approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rylnd rylnd

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:SIEM v7.7.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rylnd @elasticmachine @kibanamachine @FrankHassanabad