[Security Solution] Adds diff algorithm and unit tests for query fields#190179
[Security Solution] Adds diff algorithm and unit tests for query fields#190179dplumlee merged 12 commits intoelastic:mainfrom
query fields#190179Conversation
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
@dplumlee I'm gonna continue reviewing next week cause I gotta handle some SDHs today but:
So let's just use the simple diffing algorithm for the |
There was a problem hiding this comment.
Not sure I understand these two scenarios as described here:
returns target_version as merged output if all versions are the same: Do you mean hereif current and target versions have the same type?returns target_version as merged output if all versions are different: Did you mean hereif current and target versions have different type? In that case it doesn't match the input data, which are both ofsaved_querytype.
There was a problem hiding this comment.
Yeah I meant if both versions had the same/different type fields, I'll switch it over 👍
There was a problem hiding this comment.
This is unused now, right? Let's delete it for now then.
jpdjere
left a comment
There was a problem hiding this comment.
Leaving my approval because overall LGTM ✅ but please take a look at the two comments I left before merging.
eb1d612 to
8263b1a
Compare
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @dplumlee |
) ## Summary Related ticket: #187658 Adds test plan for diff algorithm for arrays of scalar values implemented here: #190179 ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
…tic#192529) ## Summary Related ticket: elastic#187658 Adds test plan for diff algorithm for arrays of scalar values implemented here: elastic#190179 ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) (cherry picked from commit 6680f35)
… algorithm (#192529) (#193062) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution] Test plan for `query` fields diff algorithm (#192529)](#192529) <!--- 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-09-16T17:19:39Z","message":"[Security Solution] Test plan for `query` fields diff algorithm (#192529)\n\n## Summary\r\n\r\nRelated ticket: https://github.com/elastic/kibana/issues/187658\r\n\r\nAdds test plan for diff algorithm for arrays of scalar values\r\nimplemented here: https://github.com/elastic/kibana/pull/190179\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#kibana-release-notes-process)","sha":"6680f35ef94286d34dd5c56e28575b31eab70836","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","test-plan","v9.0.0","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","v8.16.0"],"title":"[Security Solution] Test plan for `query` fields diff algorithm","number":192529,"url":"https://github.com/elastic/kibana/pull/192529","mergeCommit":{"message":"[Security Solution] Test plan for `query` fields diff algorithm (#192529)\n\n## Summary\r\n\r\nRelated ticket: https://github.com/elastic/kibana/issues/187658\r\n\r\nAdds test plan for diff algorithm for arrays of scalar values\r\nimplemented here: https://github.com/elastic/kibana/pull/190179\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#kibana-release-notes-process)","sha":"6680f35ef94286d34dd5c56e28575b31eab70836"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192529","number":192529,"mergeCommit":{"message":"[Security Solution] Test plan for `query` fields diff algorithm (#192529)\n\n## Summary\r\n\r\nRelated ticket: https://github.com/elastic/kibana/issues/187658\r\n\r\nAdds test plan for diff algorithm for arrays of scalar values\r\nimplemented here: https://github.com/elastic/kibana/pull/190179\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#kibana-release-notes-process)","sha":"6680f35ef94286d34dd5c56e28575b31eab70836"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Davis Plumlee <56367316+dplumlee@users.noreply.github.com>
…92655) ## Summary Completes #187658 Switches `kql_query`, `eql_query`, and `esql_query` fields to use the implemented diff algorithms assigned to them in #190179 Adds integration tests in accordance to #192529 for the `upgrade/_review` API endpoint for the `query` field diff algorithms. ### 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#kibana-release-notes-process)
…astic#192655) ## Summary Completes elastic#187658 Switches `kql_query`, `eql_query`, and `esql_query` fields to use the implemented diff algorithms assigned to them in elastic#190179 Adds integration tests in accordance to elastic#192529 for the `upgrade/_review` API endpoint for the `query` field diff algorithms. ### 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#kibana-release-notes-process) (cherry picked from commit ceb1b1a)
…f algorithms (#192655) (#193108) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution] Integration tests for `query` diff algorithms (#192655)](#192655) <!--- 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-09-16T23:58:55Z","message":"[Security Solution] Integration tests for `query` diff algorithms (#192655)\n\n## Summary\r\n\r\nCompletes https://github.com/elastic/kibana/issues/187658\r\n\r\n\r\nSwitches `kql_query`, `eql_query`, and `esql_query` fields to use the\r\nimplemented diff algorithms assigned to them in\r\nhttps://github.com//pull/190179\r\n\r\n\r\nAdds integration tests in accordance to\r\nhttps://github.com//pull/192529 for the `upgrade/_review`\r\nAPI endpoint for the `query` field diff algorithms.\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#kibana-release-notes-process)","sha":"ceb1b1a4bf253ac94f9ba0ba649e9a4908a76c51","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["test","release_note:skip","v9.0.0","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","v8.16.0"],"title":"[Security Solution] Integration tests for `query` diff algorithms","number":192655,"url":"https://github.com/elastic/kibana/pull/192655","mergeCommit":{"message":"[Security Solution] Integration tests for `query` diff algorithms (#192655)\n\n## Summary\r\n\r\nCompletes https://github.com/elastic/kibana/issues/187658\r\n\r\n\r\nSwitches `kql_query`, `eql_query`, and `esql_query` fields to use the\r\nimplemented diff algorithms assigned to them in\r\nhttps://github.com//pull/190179\r\n\r\n\r\nAdds integration tests in accordance to\r\nhttps://github.com//pull/192529 for the `upgrade/_review`\r\nAPI endpoint for the `query` field diff algorithms.\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#kibana-release-notes-process)","sha":"ceb1b1a4bf253ac94f9ba0ba649e9a4908a76c51"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192655","number":192655,"mergeCommit":{"message":"[Security Solution] Integration tests for `query` diff algorithms (#192655)\n\n## Summary\r\n\r\nCompletes https://github.com/elastic/kibana/issues/187658\r\n\r\n\r\nSwitches `kql_query`, `eql_query`, and `esql_query` fields to use the\r\nimplemented diff algorithms assigned to them in\r\nhttps://github.com//pull/190179\r\n\r\n\r\nAdds integration tests in accordance to\r\nhttps://github.com//pull/192529 for the `upgrade/_review`\r\nAPI endpoint for the `query` field diff algorithms.\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#kibana-release-notes-process)","sha":"ceb1b1a4bf253ac94f9ba0ba649e9a4908a76c51"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Davis Plumlee <56367316+dplumlee@users.noreply.github.com>
Summary
Related ticket: #187658
Adds diff algorithm for all the grouped
queryfields we have in theDiffableRuletype and prebuilt rule upgrade workflow. These includekql_query(both inline and saved query),eql_query, andesql_query. Also adds unit tests for all three algorithms.Checklist
Delete any items that are not applicable to this PR.
For maintainers