[Security Solution] Add extra rules export tests #150553
Closed
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
maximpn
added
release_note:skip
maximpn
commented
Feb 8, 2023
x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts
Outdated
Show resolved
Hide resolved
maximpn
force-pushed
the
rules-export-import-tests
branch
from
442abeb to
41684fb
Compare
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
maximpn
force-pushed
the
rules-export-import-tests
branch
from
371cf94 to
6bd20ab
Compare
banderror
requested review from
WafaaNasr,
a team and
xcrzx
and removed request for
a team
maximpn
added
backport:prev-minor
maximpn
force-pushed
the
rules-export-import-tests
branch
from
6bd20ab to
0104bf5
Compare
xcrzx
reviewed
Feb 23, 2023
x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/cypress/e2e/detection_rules/export_rule.cy.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/cypress/e2e/detection_rules/export_rule.cy.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/cypress/tasks/api_calls/rules.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/cypress/e2e/detection_rules/export_rule.cy.ts
Outdated
Show resolved
Hide resolved
x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts
Outdated
Show resolved
Hide resolved
Comment on lines
+99
to
+100
| id: rule.id, | ||
| created_at: rule.created_at, | ||
| updated_at: rule.updated_at, |
There was a problem hiding this comment.
Can we use expect.objectContaining here to verify only relevant properties?
There was a problem hiding this comment.
What's wrong in checking that we export specific fields only? The rule's export format shouldn't change easily. If some new fields appear it must be prominent.
There was a problem hiding this comment.
We have discussed the drawbacks of using snapshot tests multiple times, and it is worth summarizing them for clarity. Firstly, snapshot tests tend to capture irrelevant information, creating unnecessary noise in the output. Secondly, they hide the intent of the test, making it difficult for developers to determine precisely what matters just by looking at the output. Lastly, snapshot tests are often fragile and require frequent updates, leading to false positives.
To avoid these issues, it's important to make the intent of your test explicit. For instance, if your goal is to verify that dynamic fields do not get exported, it is best to focus on checking that a specific rule was successfully exported and that the output does not contain any dynamic fields. On the other hand, if you want to verify the correctness of the entire exported rule schema, it is better to create a separate test for that purpose. Mixing different cases in one test can lead to confusion and complicate the debugging process.
By explicitly stating the intent of your test and creating separate tests for different cases, you can improve the quality and reliability of your codebase. This approach reduces the likelihood of false positives and makes it easier for developers to understand the test results and debug issues quickly.
x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts
Outdated
Show resolved
Hide resolved
Comment on lines
124
to
143
| expect(bodySplitAndParsed).to.eql({ | ||
| ...getSimpleRuleOutput(ruleId, true), | ||
| id: rule.id, | ||
| created_at: rule.created_at, | ||
| updated_at: rule.updated_at, | ||
| }); |
There was a problem hiding this comment.
Same here. Can we use expect.objectContaining here to verify only relevant properties?
There was a problem hiding this comment.
The same as above. It looks right to check the whole rule's object.
maximpn
force-pushed
the
rules-export-import-tests
branch
from
0104bf5 to
fbcde61
Compare
maximpn
force-pushed
the
rules-export-import-tests
branch
from
c095567 to
b4e974e
Compare
💔 Build FailedFailed CI StepsTest Failures
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @maximpn |
|
After appearing of the huge number of conflicts it's simpler to split this PR into two parts. |
4 tasks
maximpn
added a commit
that referenced
this pull request
Mar 10, 2023
**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
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Mar 10, 2023
**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)
kibanamachine
referenced
this pull request
Mar 10, 2023
# 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>
bmorelli25
pushed a commit
to bmorelli25/kibana
that referenced
this pull request
Mar 10, 2023
**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
maximpn
added a commit
that referenced
this pull request
Mar 27, 2023
…153410) **Relates to:** #152900 ## Summary This PR adds an ability to wait for rule status by its rule id in functional tests. It is a result of splitting of #150553 into isolated parts. ## Details Based on what kind of id is used (SO id or rule id) it leads to different behaviour under the hood. SO id related functionality consumes ES Get API while rule id related functionality consumes ES Search API. This way it may require to add some delay to let ES to refresh the data if the testing logic consumes ES Search API while rule status was awaited via SO id so that handled by ES Get API. This PR removes such a delay at rule exporting functional tests.
maximpn
added a commit
that referenced
this pull request
Mar 27, 2023
#153474) **Relates to:** #150553 ## Summary This PR is based on the review comments in #150553. It allows to rewrite rule create properties. ## Details Rule create properties are returned by helper functions like `getNewRule()`, `getNewThresholdRule()` and so on. So instead of `createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' })` it allows to use a concise and a much more readable structure `createRule(getNewRule({ index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' ))`. ## Possible improvements The PR doesn't implement deep / nested fields merge. High level fields completely rewrite default values. Deep merge would allow to extend defaults with the provided rewrites. For example, overriding nested properties become tiresome quickly, as shown in the following code snippet: ```ts const rule = { ...getNewTermsRule(), rule_id: 'new_rule_id', runsEvery: { interval: '1', ...getNewTermsRule().runsEvery, }, }; ``` If we implement deep merge, the readability could be greatly improved: ```ts const rule = getNewTermsRule({ rule_id: 'new_rule_id', runsEvery: { interval: '1', }, }); ``` While it looks as a good idea we should take into consideration the fact that complete rewriting of default values can be a desired behavior. Engineers could tend to switch to `createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' })` to overcome deep merge. So it should be analysed carefully before implementing it.
maximpn
added a commit
to maximpn/kibana
that referenced
this pull request
Apr 4, 2023
…lastic#153410) **Relates to:** elastic#152900 ## Summary This PR adds an ability to wait for rule status by its rule id in functional tests. It is a result of splitting of elastic#150553 into isolated parts. ## Details Based on what kind of id is used (SO id or rule id) it leads to different behaviour under the hood. SO id related functionality consumes ES Get API while rule id related functionality consumes ES Search API. This way it may require to add some delay to let ES to refresh the data if the testing logic consumes ES Search API while rule status was awaited via SO id so that handled by ES Get API. This PR removes such a delay at rule exporting functional tests. (cherry picked from commit 519185f) # Conflicts: # x-pack/test/cases_api_integration/common/lib/alerts.ts
maximpn
added a commit
to maximpn/kibana
that referenced
this pull request
Apr 4, 2023
elastic#153474) **Relates to:** elastic#150553 ## Summary This PR is based on the review comments in elastic#150553. It allows to rewrite rule create properties. ## Details Rule create properties are returned by helper functions like `getNewRule()`, `getNewThresholdRule()` and so on. So instead of `createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' })` it allows to use a concise and a much more readable structure `createRule(getNewRule({ index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' ))`. ## Possible improvements The PR doesn't implement deep / nested fields merge. High level fields completely rewrite default values. Deep merge would allow to extend defaults with the provided rewrites. For example, overriding nested properties become tiresome quickly, as shown in the following code snippet: ```ts const rule = { ...getNewTermsRule(), rule_id: 'new_rule_id', runsEvery: { interval: '1', ...getNewTermsRule().runsEvery, }, }; ``` If we implement deep merge, the readability could be greatly improved: ```ts const rule = getNewTermsRule({ rule_id: 'new_rule_id', runsEvery: { interval: '1', }, }); ``` While it looks as a good idea we should take into consideration the fact that complete rewriting of default values can be a desired behavior. Engineers could tend to switch to `createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' })` to overcome deep merge. So it should be analysed carefully before implementing it. (cherry picked from commit ca696ac) # Conflicts: # x-pack/plugins/security_solution/cypress/e2e/detection_alerts/alerts_charts.cy.ts # x-pack/plugins/security_solution/cypress/e2e/detection_alerts/detection_page_filters.cy.ts # x-pack/plugins/security_solution/cypress/e2e/exceptions/add_edit_flyout/flyout_validation.cy.ts
maximpn
added a commit
that referenced
this pull request
Apr 4, 2023
…s tests (#153474) (#154332) # Backport This will backport the following commits from `main` to `8.7`: - [[Security Solution] Allow rewriting rule create props in Cypress tests (#153474)](#153474) <!--- 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-27T09:08:38Z","message":"[Security Solution] Allow rewriting rule create props in Cypress tests (#153474)\n\n**Relates to:** https://github.com/elastic/kibana/pull/150553\r\n\r\n## Summary\r\n\r\nThis PR is based on the review comments in #150553. It allows to rewrite rule create properties.\r\n\r\n## Details\r\n\r\nRule create properties are returned by helper functions like `getNewRule()`, `getNewThresholdRule()` and so on. So instead of `createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' })` it allows to use a concise and a much more readable structure `createRule(getNewRule({ index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' ))`.\r\n\r\n## Possible improvements\r\n\r\nThe PR doesn't implement deep / nested fields merge. High level fields completely rewrite default values. Deep merge would allow to extend defaults with the provided rewrites. For example, overriding nested properties become tiresome quickly, as shown in the following code snippet:\r\n\r\n```ts\r\nconst rule = {\r\n ...getNewTermsRule(),\r\n rule_id: 'new_rule_id',\r\n runsEvery: {\r\n interval: '1',\r\n ...getNewTermsRule().runsEvery,\r\n },\r\n};\r\n```\r\n\r\nIf we implement deep merge, the readability could be greatly improved:\r\n\r\n```ts\r\nconst rule = getNewTermsRule({\r\n rule_id: 'new_rule_id',\r\n runsEvery: {\r\n interval: '1',\r\n },\r\n});\r\n```\r\n\r\nWhile it looks as a good idea we should take into consideration the fact that complete rewriting of default values can be a desired behavior. Engineers could tend to switch to `createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' })` to overcome deep merge. So it should be analysed carefully before implementing it.","sha":"ca696ac50c0591acf6723e130d2f9278c2d6ef65","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["test","refactoring","test_ui_functional","technical debt","release_note:skip","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rules","backport:prev-minor","v8.8.0"],"number":153474,"url":"https://github.com/elastic/kibana/pull/153474","mergeCommit":{"message":"[Security Solution] Allow rewriting rule create props in Cypress tests (#153474)\n\n**Relates to:** https://github.com/elastic/kibana/pull/150553\r\n\r\n## Summary\r\n\r\nThis PR is based on the review comments in #150553. It allows to rewrite rule create properties.\r\n\r\n## Details\r\n\r\nRule create properties are returned by helper functions like `getNewRule()`, `getNewThresholdRule()` and so on. So instead of `createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' })` it allows to use a concise and a much more readable structure `createRule(getNewRule({ index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' ))`.\r\n\r\n## Possible improvements\r\n\r\nThe PR doesn't implement deep / nested fields merge. High level fields completely rewrite default values. Deep merge would allow to extend defaults with the provided rewrites. For example, overriding nested properties become tiresome quickly, as shown in the following code snippet:\r\n\r\n```ts\r\nconst rule = {\r\n ...getNewTermsRule(),\r\n rule_id: 'new_rule_id',\r\n runsEvery: {\r\n interval: '1',\r\n ...getNewTermsRule().runsEvery,\r\n },\r\n};\r\n```\r\n\r\nIf we implement deep merge, the readability could be greatly improved:\r\n\r\n```ts\r\nconst rule = getNewTermsRule({\r\n rule_id: 'new_rule_id',\r\n runsEvery: {\r\n interval: '1',\r\n },\r\n});\r\n```\r\n\r\nWhile it looks as a good idea we should take into consideration the fact that complete rewriting of default values can be a desired behavior. Engineers could tend to switch to `createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' })` to overcome deep merge. So it should be analysed carefully before implementing it.","sha":"ca696ac50c0591acf6723e130d2f9278c2d6ef65"}},"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/153474","number":153474,"mergeCommit":{"message":"[Security Solution] Allow rewriting rule create props in Cypress tests (#153474)\n\n**Relates to:** https://github.com/elastic/kibana/pull/150553\r\n\r\n## Summary\r\n\r\nThis PR is based on the review comments in #150553. It allows to rewrite rule create properties.\r\n\r\n## Details\r\n\r\nRule create properties are returned by helper functions like `getNewRule()`, `getNewThresholdRule()` and so on. So instead of `createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' })` it allows to use a concise and a much more readable structure `createRule(getNewRule({ index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' ))`.\r\n\r\n## Possible improvements\r\n\r\nThe PR doesn't implement deep / nested fields merge. High level fields completely rewrite default values. Deep merge would allow to extend defaults with the provided rewrites. For example, overriding nested properties become tiresome quickly, as shown in the following code snippet:\r\n\r\n```ts\r\nconst rule = {\r\n ...getNewTermsRule(),\r\n rule_id: 'new_rule_id',\r\n runsEvery: {\r\n interval: '1',\r\n ...getNewTermsRule().runsEvery,\r\n },\r\n};\r\n```\r\n\r\nIf we implement deep merge, the readability could be greatly improved:\r\n\r\n```ts\r\nconst rule = getNewTermsRule({\r\n rule_id: 'new_rule_id',\r\n runsEvery: {\r\n interval: '1',\r\n },\r\n});\r\n```\r\n\r\nWhile it looks as a good idea we should take into consideration the fact that complete rewriting of default values can be a desired behavior. Engineers could tend to switch to `createRule({ ...getNewRule(), index: undefined, data_view_id: DATA_VIEW_ID, rule_id: '1' })` to overcome deep merge. So it should be analysed carefully before implementing it.","sha":"ca696ac50c0591acf6723e130d2f9278c2d6ef65"}}]}] BACKPORT-->
maximpn
added a commit
that referenced
this pull request
Apr 4, 2023
…per (#153410) (#154328) # Backport This will backport the following commits from `main` to `8.7`: - [[Security Solution] Support rule id in wait for rule status helper (#153410)](#153410) <!--- 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-27T09:03:15Z","message":"[Security Solution] Support rule id in wait for rule status helper (#153410)\n\n**Relates to:** https://github.com/elastic/kibana/pull/152900\r\n\r\n## Summary\r\n\r\nThis PR adds an ability to wait for rule status by its rule id in functional tests. It is a result of splitting of #150553 into isolated parts.\r\n\r\n## Details\r\n\r\nBased on what kind of id is used (SO id or rule id) it leads to different behaviour under the hood. SO id related functionality consumes ES Get API while rule id related functionality consumes ES Search API. This way it may require to add some delay to let ES to refresh the data if the testing logic consumes ES Search API while rule status was awaited via SO id so that handled by ES Get API. This PR removes such a delay at rule exporting functional tests.","sha":"519185ffa88aeea05626f71b303a7daf1ce9d14b","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["refactoring","technical debt","release_note:skip","test-api-integration","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rules","backport:prev-minor","v8.8.0"],"number":153410,"url":"https://github.com/elastic/kibana/pull/153410","mergeCommit":{"message":"[Security Solution] Support rule id in wait for rule status helper (#153410)\n\n**Relates to:** https://github.com/elastic/kibana/pull/152900\r\n\r\n## Summary\r\n\r\nThis PR adds an ability to wait for rule status by its rule id in functional tests. It is a result of splitting of #150553 into isolated parts.\r\n\r\n## Details\r\n\r\nBased on what kind of id is used (SO id or rule id) it leads to different behaviour under the hood. SO id related functionality consumes ES Get API while rule id related functionality consumes ES Search API. This way it may require to add some delay to let ES to refresh the data if the testing logic consumes ES Search API while rule status was awaited via SO id so that handled by ES Get API. This PR removes such a delay at rule exporting functional tests.","sha":"519185ffa88aeea05626f71b303a7daf1ce9d14b"}},"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/153410","number":153410,"mergeCommit":{"message":"[Security Solution] Support rule id in wait for rule status helper (#153410)\n\n**Relates to:** https://github.com/elastic/kibana/pull/152900\r\n\r\n## Summary\r\n\r\nThis PR adds an ability to wait for rule status by its rule id in functional tests. It is a result of splitting of #150553 into isolated parts.\r\n\r\n## Details\r\n\r\nBased on what kind of id is used (SO id or rule id) it leads to different behaviour under the hood. SO id related functionality consumes ES Get API while rule id related functionality consumes ES Search API. This way it may require to add some delay to let ES to refresh the data if the testing logic consumes ES Search API while rule status was awaited via SO id so that handled by ES Get API. This PR removes such a delay at rule exporting functional tests.","sha":"519185ffa88aeea05626f71b303a7daf1ce9d14b"}}]}] 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.
Relates to: https://github.com/elastic/security-team/issues/5339, #150097
Summary
This PR covers rules exporting functionality by extra functional and e2e tests to make sure
execution_summaryand the other dynamic fields aren't exported. This problem was introduced during8.7dev cycle.Details
While working on this PR some changes were made to achieve the goal
execution_summaryfield while exporting all rulesChecklist
Delete any items that are not applicable to this PR.