[Security Solution] Fix not complete existing rule overwrite when importing rules#176166
Merged
maximpn merged 10 commits intoelastic:mainfrom Feb 19, 2024
Merged
[Security Solution] Fix not complete existing rule overwrite when importing rules#176166maximpn merged 10 commits intoelastic:mainfrom
maximpn merged 10 commits intoelastic:mainfrom
Conversation
maximpn
added
bug
maximpn
changed the title
Contributor
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Contributor
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
Contributor
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
maximpn
force-pushed
the
fix-rule-override-at-import
branch
from
42e5816 to
42e2dfb
Compare
maximpn
force-pushed
the
fix-rule-override-at-import
branch
from
42e2dfb to
60bb17c
Compare
Contributor
Author
|
/ci |
maximpn
force-pushed
the
fix-rule-override-at-import
branch
from
60bb17c to
92f3d75
Compare
Contributor
Author
|
/ci |
maximpn
force-pushed
the
fix-rule-override-at-import
branch
from
92f3d75 to
7760aac
Compare
Contributor
Author
|
/ci |
maximpn
force-pushed
the
fix-rule-override-at-import
branch
from
7760aac to
af64d04
Compare
maximpn
force-pushed
the
fix-rule-override-at-import
branch
from
af64d04 to
d6c5566
Compare
vitaliidm
approved these changes
Feb 19, 2024
Contributor
vitaliidm
left a comment
vitaliidm
left a comment
There was a problem hiding this comment.
detection engine area LGTM
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
History
To update your PR or re-run it, just comment with: cc @maximpn |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Feb 19, 2024
…orting rules (elastic#176166) **Fixes: elastic#93342 **Fixes: elastic#118166 ## Summary This PR fixes not complete existing rule overwrite when importing rules. ## Details When importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule. The fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause. ### Checklist - [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 - [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167)) (cherry picked from commit 53aaab4)
Contributor
💔 Some backports could not be created
Note: Successful backport PRs will be merged automatically after passing CI. Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Feb 19, 2024
…hen importing rules (#176166) (#177196) # Backport This will backport the following commits from `main` to `8.13`: - [[Security Solution] Fix not complete existing rule overwrite when importing rules (#176166)](#176166) <!--- Backport version: 9.4.3 --> ### 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":"2024-02-19T13:35:32Z","message":"[Security Solution] Fix not complete existing rule overwrite when importing rules (#176166)\n\n**Fixes: https://github.com/elastic/kibana/issues/93342**\r\n**Fixes: https://github.com/elastic/kibana/issues/118166**\r\n\r\n## Summary\r\n\r\nThis PR fixes not complete existing rule overwrite when importing rules.\r\n\r\n## Details\r\n\r\nWhen importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule.\r\n\r\nThe fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause.\r\n\r\n### Checklist\r\n\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\r\n- [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167))","sha":"53aaab47322fd15ad232d71a1749fd2df8a5dde4","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","impact:medium","Team:Detections and Resp","Team: SecuritySolution","Feature:Rule Management","Team:Detection Rule Management","Feature:Rule Import/Export","v8.13.0","v8.12.2","v8.14.0"],"title":"[Security Solution] Fix not complete existing rule overwrite when importing rules","number":176166,"url":"https://github.com/elastic/kibana/pull/176166","mergeCommit":{"message":"[Security Solution] Fix not complete existing rule overwrite when importing rules (#176166)\n\n**Fixes: https://github.com/elastic/kibana/issues/93342**\r\n**Fixes: https://github.com/elastic/kibana/issues/118166**\r\n\r\n## Summary\r\n\r\nThis PR fixes not complete existing rule overwrite when importing rules.\r\n\r\n## Details\r\n\r\nWhen importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule.\r\n\r\nThe fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause.\r\n\r\n### Checklist\r\n\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\r\n- [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167))","sha":"53aaab47322fd15ad232d71a1749fd2df8a5dde4"}},"sourceBranch":"main","suggestedTargetBranches":["8.13","8.12"],"targetPullRequestStates":[{"branch":"8.13","label":"v8.13.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.12","label":"v8.12.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/176166","number":176166,"mergeCommit":{"message":"[Security Solution] Fix not complete existing rule overwrite when importing rules (#176166)\n\n**Fixes: https://github.com/elastic/kibana/issues/93342**\r\n**Fixes: https://github.com/elastic/kibana/issues/118166**\r\n\r\n## Summary\r\n\r\nThis PR fixes not complete existing rule overwrite when importing rules.\r\n\r\n## Details\r\n\r\nWhen importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule.\r\n\r\nThe fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause.\r\n\r\n### Checklist\r\n\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\r\n- [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167))","sha":"53aaab47322fd15ad232d71a1749fd2df8a5dde4"}}]}] BACKPORT--> Co-authored-by: Maxim Palenov <maxim.palenov@elastic.co>
Contributor
Author
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
maximpn
added a commit
to maximpn/kibana
that referenced
this pull request
Feb 20, 2024
…orting rules (elastic#176166) **Fixes: elastic#93342 **Fixes: elastic#118166 ## Summary This PR fixes not complete existing rule overwrite when importing rules. ## Details When importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule. The fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause. ### Checklist - [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 - [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167)) (cherry picked from commit 53aaab4) # Conflicts: # x-pack/test/detection_engine_api_integration/basic/tests/export_rules.ts # x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts # x-pack/test/detection_engine_api_integration/basic/tests/import_rules_with_overwrite.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_connectors.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_export_rules.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules_with_overwrite.ts # x-pack/test/detection_engine_api_integration/utils/get_rules_as_ndjson.ts # x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/alerts/assignments/index.ts # x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_import_export/trial_license_complete_tier/export_rules.ts # x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_import_export/trial_license_complete_tier/import_rules_ess.ts # x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_import_export/trial_license_complete_tier/index.ts # x-pack/test/security_solution_api_integration/test_suites/detections_response/utils/rules/index.ts
maximpn
added a commit
that referenced
this pull request
Feb 20, 2024
…hen importing rules (#176166) (#177270) # Backport This will backport the following commits from `main` to `8.12`: - [[Security Solution] Fix not complete existing rule overwrite when importing rules (#176166)](#176166) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)
Contributor
|
This PR didn't landed on time to be included on v8.12.2. Updating the labels. |
fkanout
pushed a commit
to fkanout/kibana
that referenced
this pull request
Mar 4, 2024
…orting rules (elastic#176166) **Fixes: elastic#93342 **Fixes: elastic#118166 ## Summary This PR fixes not complete existing rule overwrite when importing rules. ## Details When importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule. The fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause. ### Checklist - [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 - [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167))
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: #93342
Fixes: #118166
Summary
This PR fixes not complete existing rule overwrite when importing rules.
Details
When importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule.
The fix simply swaps out the
patchRules()forupdateRules(). Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff inimport_rules_utils.test.tslooks bigger due to removing unnecessaryelseclause.Checklist