[Security Solution] Adds normalization for query fields before diff algorithm comparison
#203482
Conversation
This file contains 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
dplumlee
added
bug
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Flaky Test Runner Stats🟠 Some tests failed. - kibana-flaky-test-suite-runner#7570[✅] x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/configs/ess.config.ts: 100/100 tests passed. |
jpdjere
approved these changes
Dec 11, 2024
There was a problem hiding this comment.
Looking good, tested and behaving properly. LGTM!
My only question is: do you think we should do the same for other string fields like name and description? Not sure if the newline at the end of the queries comes directly from the rule assets, or where it is introduced, but I guess exactly the same could theoretically happen for other string fields in the future.
|
@elasticmachine merge upstream |
|
@jpdjere that's not a bad question, after thinking it over I think I'll add it to all the fields that we don't allow new line formatting to be added or removed via the current rule edit UI. Namely not fields like The fields that will be trimmed now, including the ones in this PR are:
|
dplumlee
force-pushed
the
query-trimming-is-customized
branch
from
c9cf4f6 to
7a6a7f7
Compare
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Async chunks
History
cc @dplumlee |
|
Starting backport for target branches: 8.17, 8.x |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Dec 13, 2024
… algorithm comparison (elastic#203482) ## Summary Fixes elastic#203151 Adds a normalization for the `kql_query`, `eql_query`, and `esql_query` fields that trims the whitespace from the beginning and end of query strings for a more robust comparison in the diff algorithms. Since whitespace before or after the query string is purely a formatting choice and doesn't impact the query itself, we discard the excess whitespace characters before the direct string comparison. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [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 (cherry picked from commit 0294838)
💔 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 |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Dec 13, 2024
…elds before diff algorithm comparison (#203482) (#204159) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution] Adds normalization for `query` fields before diff algorithm comparison (#203482)](#203482) <!--- 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-12-13T03:58:50Z","message":"[Security Solution] Adds normalization for `query` fields before diff algorithm comparison (#203482)\n\n## Summary\r\n\r\nFixes https://github.com/elastic/kibana/issues/203151\r\n\r\nAdds a normalization for the `kql_query`, `eql_query`, and `esql_query`\r\nfields that trims the whitespace from the beginning and end of query\r\nstrings for a more robust comparison in the diff algorithms. Since\r\nwhitespace before or after the query string is purely a formatting\r\nchoice and doesn't impact the query itself, we discard the excess\r\nwhitespace characters before the direct string comparison.\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\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","sha":"0294838a95975ac6b5ee37a94ecacfe2e9a19955","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v9.0.0","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","backport:version","v8.18.0","v8.17.1"],"title":"[Security Solution] Adds normalization for `query` fields before diff algorithm comparison","number":203482,"url":"https://github.com/elastic/kibana/pull/203482","mergeCommit":{"message":"[Security Solution] Adds normalization for `query` fields before diff algorithm comparison (#203482)\n\n## Summary\r\n\r\nFixes https://github.com/elastic/kibana/issues/203151\r\n\r\nAdds a normalization for the `kql_query`, `eql_query`, and `esql_query`\r\nfields that trims the whitespace from the beginning and end of query\r\nstrings for a more robust comparison in the diff algorithms. Since\r\nwhitespace before or after the query string is purely a formatting\r\nchoice and doesn't impact the query itself, we discard the excess\r\nwhitespace characters before the direct string comparison.\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\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","sha":"0294838a95975ac6b5ee37a94ecacfe2e9a19955"}},"sourceBranch":"main","suggestedTargetBranches":["8.x","8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203482","number":203482,"mergeCommit":{"message":"[Security Solution] Adds normalization for `query` fields before diff algorithm comparison (#203482)\n\n## Summary\r\n\r\nFixes https://github.com/elastic/kibana/issues/203151\r\n\r\nAdds a normalization for the `kql_query`, `eql_query`, and `esql_query`\r\nfields that trims the whitespace from the beginning and end of query\r\nstrings for a more robust comparison in the diff algorithms. Since\r\nwhitespace before or after the query string is purely a formatting\r\nchoice and doesn't impact the query itself, we discard the excess\r\nwhitespace characters before the direct string comparison.\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\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","sha":"0294838a95975ac6b5ee37a94ecacfe2e9a19955"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Davis Plumlee <56367316+dplumlee@users.noreply.github.com>
dplumlee
added a commit
that referenced
this pull request
Dec 13, 2024
…re diff algorithm comparison (#203482) (#204160) # Backport This will backport the following commits from `main` to `8.17`: - [[Security Solution] Adds normalization for `query` fields before diff algorithm comparison (#203482)](#203482) <!--- Backport version: 8.9.8 --> ### 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-12-13T03:58:50Z","message":"[Security Solution] Adds normalization for `query` fields before diff algorithm comparison (#203482)\n\n## Summary\r\n\r\nFixes https://github.com/elastic/kibana/issues/203151\r\n\r\nAdds a normalization for the `kql_query`, `eql_query`, and `esql_query`\r\nfields that trims the whitespace from the beginning and end of query\r\nstrings for a more robust comparison in the diff algorithms. Since\r\nwhitespace before or after the query string is purely a formatting\r\nchoice and doesn't impact the query itself, we discard the excess\r\nwhitespace characters before the direct string comparison.\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\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","sha":"0294838a95975ac6b5ee37a94ecacfe2e9a19955","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v9.0.0","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","backport:version","v8.18.0","v8.17.1"],"number":203482,"url":"https://github.com/elastic/kibana/pull/203482","mergeCommit":{"message":"[Security Solution] Adds normalization for `query` fields before diff algorithm comparison (#203482)\n\n## Summary\r\n\r\nFixes https://github.com/elastic/kibana/issues/203151\r\n\r\nAdds a normalization for the `kql_query`, `eql_query`, and `esql_query`\r\nfields that trims the whitespace from the beginning and end of query\r\nstrings for a more robust comparison in the diff algorithms. Since\r\nwhitespace before or after the query string is purely a formatting\r\nchoice and doesn't impact the query itself, we discard the excess\r\nwhitespace characters before the direct string comparison.\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\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","sha":"0294838a95975ac6b5ee37a94ecacfe2e9a19955"}},"sourceBranch":"main","suggestedTargetBranches":["8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203482","number":203482,"mergeCommit":{"message":"[Security Solution] Adds normalization for `query` fields before diff algorithm comparison (#203482)\n\n## Summary\r\n\r\nFixes https://github.com/elastic/kibana/issues/203151\r\n\r\nAdds a normalization for the `kql_query`, `eql_query`, and `esql_query`\r\nfields that trims the whitespace from the beginning and end of query\r\nstrings for a more robust comparison in the diff algorithms. Since\r\nwhitespace before or after the query string is purely a formatting\r\nchoice and doesn't impact the query itself, we discard the excess\r\nwhitespace characters before the direct string comparison.\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\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","sha":"0294838a95975ac6b5ee37a94ecacfe2e9a19955"}},{"branch":"8.x","label":"v8.18.0","labelRegex":"^v8.18.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/204159","number":204159,"state":"OPEN"},{"branch":"8.17","label":"v8.17.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fixes #203151
Adds a normalization for the
kql_query,eql_query, andesql_queryfields that trims the whitespace from the beginning and end of query strings for a more robust comparison in the diff algorithms. Since whitespace before or after the query string is purely a formatting choice and doesn't impact the query itself, we discard the excess whitespace characters before the direct string comparison.Checklist
Check the PR satisfies following conditions.
Reviewers should verify this PR satisfies this list as well.