-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Detection Engine] Adds 8.4 rules #138574
[Detection Engine] Adds 8.4 rules #138574
Conversation
@brokensound77 , I've prepared a PR (#138625), which once merged should allow tests in your PR to pass. |
@vitaliidm thank you so much for looking into this and finding a solution. Justin will be at DefCon this week and I will be handling the rest of the detection rules release. Please let me know if there is anything I can do. When your PR merges I will re-run the tests and hopefully they pass. Thanks again! |
While playing around with the updated prebuilt rules, noticed: when duplicating rules, duplicated rules are getting created without required fields, integrations and setup fields Screen.Recording.2022-08-12.at.12.05.27.movcc: @banderror , is this behaviour intentional or something we will need to fix later? |
@vitaliidm This behavior is expected. Users can't edit them (there's no UI for these fields on the Rule Creation and Editing pages), so the decision was to reset them to empty values in this case. Once we add support for editing them, we will need to remove this logic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@terrancedejesus , PR mentioned above is merged. Hopefully all tests will pass, I've triggered build to verify it.
Since @banderror addressed my comment before, I don't see any issues and approving changes
… update (elastic#138625) ## Summary fixes tests related to prebuilt rules update tests failures(elastic#138574 (comment)): - removed hardcoded rule version value - required_fields are not tested agains empty array, but agains actual immutable rule value (cherry picked from commit 800cc72)
… update (elastic#138625) ## Summary fixes tests related to prebuilt rules update tests failures(elastic#138574 (comment)): - removed hardcoded rule version value - required_fields are not tested agains empty array, but agains actual immutable rule value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hey @vitaliidm thank you so much for getting that PR merged and re-running the checks. The Kibana updates look great to see them in an actual stack with the rules! Waiting for these checks to pass and then I will go ahead and merge. Thanks everyone! |
@elasticmachine merge upstream |
@vitaliidm any additional information on the new errors for FTR configs #23? I believe this check passed before. Thank you in advance! |
another fix is on its way. This test didn't fail, as CI test job exits on first test failed, so it doesn't show the rest of failures |
…rules update elastic#138794 - fixes the rest of tests for elastic#138574 PR (cherry picked from commit 167bb8e)
…rules update elastic#138794 - fixes the rest of tests for elastic#138574 PR
Ah alright that makes sense, thanks for the insight. Is this the PR for that? I want to make sure I include this information in our overall tracking issue outside of this. |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
@terrancedejesus, correct Meanwhile all tests have passed and PR is ready to be merged |
Wow thanks for the fast turnaround and solution implementation on this! Merging now. |
Co-authored-by: Vitalii Dmyterko <92328789+vitaliidm@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com> (cherry picked from commit 3021824)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
Co-authored-by: Vitalii Dmyterko <92328789+vitaliidm@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com> (cherry picked from commit 3021824) Co-authored-by: Justin Ibarra <brokensound77@users.noreply.github.com>
… update (elastic#138625) ## Summary fixes tests related to prebuilt rules update tests failures(elastic#138574 (comment)): - removed hardcoded rule version value - required_fields are not tested agains empty array, but agains actual immutable rule value
…rules update elastic#138794 - fixes the rest of tests for elastic#138574 PR
Co-authored-by: Vitalii Dmyterko <92328789+vitaliidm@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com>
Summary
Pull updates to detection rules from https://github.com/elastic/detection-rules/tree/395ae2c1d1e1d93ea7520300d4fb5000c58ae870.
Checklist
Delete any items that are not applicable to this PR.
uses sentence case text and includes i18n support