Skip to content

[Security Solution] Fixes required_fields being removed after rule PATCH calls#199901

Merged
banderror merged 5 commits intoelastic:mainfrom
dplumlee:rule-patch-required-fields-fix
Nov 15, 2024
Merged

[Security Solution] Fixes required_fields being removed after rule PATCH calls#199901
banderror merged 5 commits intoelastic:mainfrom
dplumlee:rule-patch-required-fields-fix

Conversation

@dplumlee
Copy link
Copy Markdown
Contributor

@dplumlee dplumlee commented Nov 13, 2024

Fixes #199665

Summary

Fixes the required_fields field being removed from the existing rule when not present in the rule PATCH API call.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@dplumlee dplumlee added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes v9.0.0 Feature:Detection Rules Security Solution rules and Detection Engine Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team v8.17.0 v8.16.1 labels Nov 13, 2024
@dplumlee dplumlee self-assigned this Nov 13, 2024
@dplumlee dplumlee requested a review from a team as a code owner November 13, 2024 03:11
@dplumlee dplumlee requested a review from nikitaindik November 13, 2024 03:11
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@dplumlee dplumlee added the backport:version Backport to applied version labels label Nov 13, 2024
@banderror banderror added impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. and removed impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. labels Nov 13, 2024
@banderror banderror requested review from banderror and xcrzx and removed request for nikitaindik and xcrzx November 13, 2024 11:24
@kibanamachine
Copy link
Copy Markdown
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#7397

[✅] x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_patch/basic_license_essentials_tier/configs/ess.config.ts: 100/100 tests passed.
[✅] x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_patch/basic_license_essentials_tier/configs/serverless.config.ts: 100/100 tests passed.

see run history

Copy link
Copy Markdown
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the diff and tested the fix locally in main with the feature flag turned OFF.

I installed a prebuilt rule and PATCHed its name. Everything worked as expected:

  • The name field was updated
  • The required_fields field remained unchanged
  • No other rule parameters were changed by mistake
  • A few technical fields were updated:
    • updated_at
    • revision was updated from 0 to 1
    • rule_source.is_customized was updated from false to true

Here's a diff between two revisions of the rule and a screenshot of the updated rule's Details page:

Screenshot 2024-11-14 at 15 53 51

So the bug has been fixed and we don't have any other similar bugs in the PATCH endpoint.

@dplumlee Thank you for the fix and adding that integration test! 🚀

@banderror banderror added release_note:fix and removed release_note:skip Skip the PR/issue when compiling release notes labels Nov 14, 2024
@banderror banderror enabled auto-merge (squash) November 14, 2024 14:56
@dplumlee
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #12 / discover/group5 discover tab field data shows top-level object keys

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [53c05a3]

History

cc @dplumlee

@dplumlee dplumlee force-pushed the rule-patch-required-fields-fix branch from d965d89 to aea65f4 Compare November 14, 2024 18:53
@elasticmachine
Copy link
Copy Markdown
Contributor

⏳ Build in-progress, with failures

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #32 / serverless search UI Elasticsearch Start [Onboarding Empty State] developer should show the api key in code view

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [4d1cb0b]

History

cc @dplumlee

@banderror
Copy link
Copy Markdown
Contributor

@elasticmachine merge upstream

@banderror banderror merged commit f716948 into elastic:main Nov 15, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.16, 8.x

https://github.com/elastic/kibana/actions/runs/11858792233

kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Nov 15, 2024
…`PATCH` calls (elastic#199901)

**Fixes elastic#199665

## Summary

Fixes the `required_fields` field being removed from the existing rule
when not present in the rule `PATCH` API call.

### Checklist

Delete any items that are not applicable to this PR.

- [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] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
- [ ] This will appear in the **Release Notes** and follow the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Georgii Gorbachev <georgii.gorbachev@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit f716948)
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Nov 15, 2024
…`PATCH` calls (elastic#199901)

**Fixes elastic#199665

## Summary

Fixes the `required_fields` field being removed from the existing rule
when not present in the rule `PATCH` API call.

### Checklist

Delete any items that are not applicable to this PR.

- [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] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
- [ ] This will appear in the **Release Notes** and follow the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Georgii Gorbachev <georgii.gorbachev@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit f716948)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.16
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@dplumlee dplumlee deleted the rule-patch-required-fields-fix branch November 15, 2024 15:23
kibanamachine added a commit that referenced this pull request Nov 15, 2024
…moved after rule &#x60;PATCH&#x60; calls (#199901) (#200306)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Security Solution] Fixes &#x60;required_fields&#x60; being removed
after rule &#x60;PATCH&#x60; calls
(#199901)](#199901)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Davis
Plumlee","email":"56367316+dplumlee@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-11-15T15:17:37Z","message":"[Security
Solution] Fixes `required_fields` being removed after rule `PATCH` calls
(#199901)\n\n**Fixes
https://github.com/elastic/kibana/issues/199665**\r\n\r\n##
Summary\r\n\r\nFixes the `required_fields` field being removed from the
existing rule\r\nwhen not present in the rule `PATCH` API
call.\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n\r\n### For maintainers\r\n\r\n-
[ ] This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Georgii Gorbachev <georgii.gorbachev@elastic.co>\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"f716948053c1b6f4a9f1dda27d4f5a501631b692","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","impact:medium","v9.0.0","Feature:Detection
Rules","Team:Detections and Resp","Team:
SecuritySolution","Team:Detection Rule
Management","backport:version","v8.17.0","v8.16.1"],"title":"[Security
Solution] Fixes `required_fields` being removed after rule `PATCH`
calls","number":199901,"url":"https://github.com/elastic/kibana/pull/199901","mergeCommit":{"message":"[Security
Solution] Fixes `required_fields` being removed after rule `PATCH` calls
(#199901)\n\n**Fixes
https://github.com/elastic/kibana/issues/199665**\r\n\r\n##
Summary\r\n\r\nFixes the `required_fields` field being removed from the
existing rule\r\nwhen not present in the rule `PATCH` API
call.\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n\r\n### For maintainers\r\n\r\n-
[ ] This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Georgii Gorbachev <georgii.gorbachev@elastic.co>\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"f716948053c1b6f4a9f1dda27d4f5a501631b692"}},"sourceBranch":"main","suggestedTargetBranches":["8.x","8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199901","number":199901,"mergeCommit":{"message":"[Security
Solution] Fixes `required_fields` being removed after rule `PATCH` calls
(#199901)\n\n**Fixes
https://github.com/elastic/kibana/issues/199665**\r\n\r\n##
Summary\r\n\r\nFixes the `required_fields` field being removed from the
existing rule\r\nwhen not present in the rule `PATCH` API
call.\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n\r\n### For maintainers\r\n\r\n-
[ ] This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Georgii Gorbachev <georgii.gorbachev@elastic.co>\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"f716948053c1b6f4a9f1dda27d4f5a501631b692"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.16","label":"v8.16.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Davis Plumlee <56367316+dplumlee@users.noreply.github.com>
kibanamachine added a commit that referenced this pull request Nov 15, 2024
…oved after rule &#x60;PATCH&#x60; calls (#199901) (#200307)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution] Fixes &#x60;required_fields&#x60; being removed
after rule &#x60;PATCH&#x60; calls
(#199901)](#199901)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Davis
Plumlee","email":"56367316+dplumlee@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-11-15T15:17:37Z","message":"[Security
Solution] Fixes `required_fields` being removed after rule `PATCH` calls
(#199901)\n\n**Fixes
https://github.com/elastic/kibana/issues/199665**\r\n\r\n##
Summary\r\n\r\nFixes the `required_fields` field being removed from the
existing rule\r\nwhen not present in the rule `PATCH` API
call.\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n\r\n### For maintainers\r\n\r\n-
[ ] This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Georgii Gorbachev <georgii.gorbachev@elastic.co>\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"f716948053c1b6f4a9f1dda27d4f5a501631b692","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","impact:medium","v9.0.0","Feature:Detection
Rules","Team:Detections and Resp","Team:
SecuritySolution","Team:Detection Rule
Management","backport:version","v8.17.0","v8.16.1"],"title":"[Security
Solution] Fixes `required_fields` being removed after rule `PATCH`
calls","number":199901,"url":"https://github.com/elastic/kibana/pull/199901","mergeCommit":{"message":"[Security
Solution] Fixes `required_fields` being removed after rule `PATCH` calls
(#199901)\n\n**Fixes
https://github.com/elastic/kibana/issues/199665**\r\n\r\n##
Summary\r\n\r\nFixes the `required_fields` field being removed from the
existing rule\r\nwhen not present in the rule `PATCH` API
call.\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n\r\n### For maintainers\r\n\r\n-
[ ] This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Georgii Gorbachev <georgii.gorbachev@elastic.co>\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"f716948053c1b6f4a9f1dda27d4f5a501631b692"}},"sourceBranch":"main","suggestedTargetBranches":["8.x","8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199901","number":199901,"mergeCommit":{"message":"[Security
Solution] Fixes `required_fields` being removed after rule `PATCH` calls
(#199901)\n\n**Fixes
https://github.com/elastic/kibana/issues/199665**\r\n\r\n##
Summary\r\n\r\nFixes the `required_fields` field being removed from the
existing rule\r\nwhen not present in the rule `PATCH` API
call.\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n\r\n### For maintainers\r\n\r\n-
[ ] This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Georgii Gorbachev <georgii.gorbachev@elastic.co>\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"f716948053c1b6f4a9f1dda27d4f5a501631b692"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.16","label":"v8.16.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Davis Plumlee <56367316+dplumlee@users.noreply.github.com>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024
…`PATCH` calls (elastic#199901)

**Fixes elastic#199665

## Summary

Fixes the `required_fields` field being removed from the existing rule
when not present in the rule `PATCH` API call.


### Checklist

Delete any items that are not applicable to this PR.

- [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] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
- [ ] This will appear in the **Release Notes** and follow the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Georgii Gorbachev <georgii.gorbachev@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024
…`PATCH` calls (elastic#199901)

**Fixes elastic#199665

## Summary

Fixes the `required_fields` field being removed from the existing rule
when not present in the rule `PATCH` API call.


### Checklist

Delete any items that are not applicable to this PR.

- [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] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
- [ ] This will appear in the **Release Notes** and follow the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Georgii Gorbachev <georgii.gorbachev@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels bug Fixes for quality problems that affect the customer experience Feature:Detection Rules Security Solution rules and Detection Engine impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. release_note:fix Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.16.1 v8.17.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security Solution] Required fields are getting erased on rule PATCH

4 participants