Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[SIEM] Fixes Modification of ML Rules #60662

Merged
merged 6 commits into from
Mar 20, 2020
Merged

[SIEM] Fixes Modification of ML Rules #60662

merged 6 commits into from
Mar 20, 2020

Conversation

rylnd
Copy link
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
Contributor

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
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
Contributor

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
Contributor

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
Contributor

💚 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
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

@FrankHassanabad FrankHassanabad FrankHassanabad approved these changes

@yctercero yctercero Awaiting requested review from yctercero

@dhurley14 dhurley14 Awaiting requested review from dhurley14

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