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.

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

[SIEM] Fix patching of ML Rules #60830

Merged
merged 4 commits into from Mar 21, 2020
Merged

[SIEM] Fix patching of ML Rules #60830

merged 4 commits into from Mar 21, 2020

Conversation

rylnd
Copy link
Contributor

@rylnd rylnd commented Mar 20, 2020

Summary

As uncovered by #60713 , the patch route (and its bulk variant) were not passing down the new ML params to the patchRules helper. Consequently, ML rules could not patch-update those fields. This fixes that and adds regression tests.

Also addresses the issue discussed in this comment

Since patchRules accepts a partial there's no way to verify this in
typescript, we need regression tests instead.
This was simply missed earlier.
@rylnd rylnd added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Mar 20, 2020
@rylnd rylnd requested a review from a team as a code owner March 20, 2020 22:06
@rylnd rylnd self-assigned this Mar 20, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

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.

This looks great!

LGTM

@rylnd
Copy link
Contributor Author

rylnd commented Mar 20, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@rylnd rylnd merged commit 9e91146 into elastic:master Mar 21, 2020
@rylnd rylnd deleted the fix_patch_rules branch March 21, 2020 02:32
rylnd added a commit that referenced this pull request Mar 22, 2020
* Allow ML Rules to be patched

* Test passing of params from our patch routes to our helpers

Since patchRules accepts a partial there's no way to verify this in
typescript, we need regression tests instead.

* Update lists when importing with overwrite

This was simply missed earlier.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 23, 2020
* master: (39 commits)
  [APM]Create custom link from Trace summary (elastic#59648)
  [ML] Fixing app clean up (elastic#60853)
  [SIEM] Use ECS categorisation for Authentication widgets (elastic#60734)
  [NP] Remove kbnUrl usage in discover/dashboard/visualize (elastic#60016)
  Skip failing test
  [Uptime]Update fetch effect failed action handling (elastic#60742)
  [npm] upgrade elastic/maki (elastic#60829)
  [Uptime] Add Settings Page (elastic#53550)
  [APM] service maps: avoid unnecesary `useDeepObjectIdentity` (elastic#60836)
  [Index management] Re-enable index template tests (elastic#60780)
  Fixed UI/UX issues: alerts delete confirmation, combobox behaviors (elastic#60703)
  [SIEM] Fix patching of ML Rules (elastic#60830)
  [APM] Service Map - Separate overlapping edges by rotating nodes (elastic#60477)
  [Alerting] fix flaky test for index threshold grouping (elastic#60792)
  [SIEM][Detection Engine] Adds test scripts for machine learning feature
  Flatten child api response for resolver (elastic#60810)
  Change "url" to "urls" in APM agent instructions (elastic#60790)
  [DOCS] Updates API requests and examples (elastic#60695)
  [SIEM] [Cases] Create case from timeline (elastic#60711)
  [Lens] Resetting a layer generates new suggestions (elastic#60674)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:SIEM v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants