-
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
[Security Solution] Avoid exporting execution_summary field #150097
Conversation
70697ff
to
065ec29
Compare
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
History
To update your PR or re-run it, just comment with: cc @maximpn |
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
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, thanks! 👍
const rule = internalRuleToAPIResponse(matchingRule, legacyActions[matchingRule.id]); | ||
|
||
// Fields containing runtime information shouldn't be exported. It causes import failures. | ||
delete rule.execution_summary; |
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.
Nit: Let's extract this into a function with a clear name, e.g. transformRuleToExportableFormat
or something like that
**Relates to:** elastic/security-team#5339, #150097, #150553 ## Summary This PR fixes all rules exporting functionality which started exporting unintentionally runtime fields like `execution_summary`. This way it lead to inability to import just exported rules if as minimum one of them executed just once. On top of this the PR contains functional and Cypress tests to cover the fix. ## TODO - [ ] get rid of `await waitForEventLogExecuteComplete()` in functional tests - [ ] allow `getNewRule()` to rewrite its defaults ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
**Relates to:** elastic/security-team#5339, elastic#150097, elastic#150553 ## Summary This PR fixes all rules exporting functionality which started exporting unintentionally runtime fields like `execution_summary`. This way it lead to inability to import just exported rules if as minimum one of them executed just once. On top of this the PR contains functional and Cypress tests to cover the fix. ## TODO - [ ] get rid of `await waitForEventLogExecuteComplete()` in functional tests - [ ] allow `getNewRule()` to rewrite its defaults ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (cherry picked from commit 6b62ae2)
# Backport This will backport the following commits from `main` to `8.7`: - [[Security Solution] Fix exporting all rules (#152900)](#152900) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Maxim Palenov","email":"maxim.palenov@elastic.co"},"sourceCommit":{"committedDate":"2023-03-10T17:23:48Z","message":"[Security Solution] Fix exporting all rules (#152900)\n\n**Relates to:** elastic/security-team#5339, #150097, https://github.com/elastic/kibana/pull/150553\r\n\r\n## Summary\r\n\r\nThis PR fixes all rules exporting functionality which started exporting unintentionally runtime fields like `execution_summary`. This way it lead to inability to import just exported rules if as minimum one of them executed just once.\r\n\r\nOn top of this the PR contains functional and Cypress tests to cover the fix.\r\n\r\n## TODO\r\n\r\n- [ ] get rid of `await waitForEventLogExecuteComplete()` in functional tests\r\n- [ ] allow `getNewRule()` to rewrite its defaults\r\n\r\n### Checklist\r\n\r\n- [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials\r\n- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios","sha":"6b62ae2adfead5ece8b47c0909ab58c67f3f1adb","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rules","backport:prev-minor","Feature:Rule Import/Export","v8.8.0"],"number":152900,"url":"https://github.com/elastic/kibana/pull/152900","mergeCommit":{"message":"[Security Solution] Fix exporting all rules (#152900)\n\n**Relates to:** elastic/security-team#5339, #150097, https://github.com/elastic/kibana/pull/150553\r\n\r\n## Summary\r\n\r\nThis PR fixes all rules exporting functionality which started exporting unintentionally runtime fields like `execution_summary`. This way it lead to inability to import just exported rules if as minimum one of them executed just once.\r\n\r\nOn top of this the PR contains functional and Cypress tests to cover the fix.\r\n\r\n## TODO\r\n\r\n- [ ] get rid of `await waitForEventLogExecuteComplete()` in functional tests\r\n- [ ] allow `getNewRule()` to rewrite its defaults\r\n\r\n### Checklist\r\n\r\n- [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials\r\n- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios","sha":"6b62ae2adfead5ece8b47c0909ab58c67f3f1adb"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/152900","number":152900,"mergeCommit":{"message":"[Security Solution] Fix exporting all rules (#152900)\n\n**Relates to:** elastic/security-team#5339, #150097, https://github.com/elastic/kibana/pull/150553\r\n\r\n## Summary\r\n\r\nThis PR fixes all rules exporting functionality which started exporting unintentionally runtime fields like `execution_summary`. This way it lead to inability to import just exported rules if as minimum one of them executed just once.\r\n\r\nOn top of this the PR contains functional and Cypress tests to cover the fix.\r\n\r\n## TODO\r\n\r\n- [ ] get rid of `await waitForEventLogExecuteComplete()` in functional tests\r\n- [ ] allow `getNewRule()` to rewrite its defaults\r\n\r\n### Checklist\r\n\r\n- [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials\r\n- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios","sha":"6b62ae2adfead5ece8b47c0909ab58c67f3f1adb"}}]}] BACKPORT--> Co-authored-by: Maxim Palenov <maxim.palenov@elastic.co>
**Relates to:** elastic/security-team#5339, elastic#150097, elastic#150553 ## Summary This PR fixes all rules exporting functionality which started exporting unintentionally runtime fields like `execution_summary`. This way it lead to inability to import just exported rules if as minimum one of them executed just once. On top of this the PR contains functional and Cypress tests to cover the fix. ## TODO - [ ] get rid of `await waitForEventLogExecuteComplete()` in functional tests - [ ] allow `getNewRule()` to rewrite its defaults ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
Summary
It fixes a problem of exporting
execution_summary
field while exporting detection rules which was introduce in #147035. Presence of that field make importing of just exported rule failing.Tests to cover this fix will come in a separate PR.