Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Security Solution] Revisit prebuilt detection rules tests #149502

Merged
merged 1 commit into from
Feb 23, 2023
Merged

[Security Solution] Revisit prebuilt detection rules tests #149502

merged 1 commit into from
Feb 23, 2023

Conversation

xcrzx
Copy link
Contributor

@xcrzx xcrzx commented Jan 25, 2023 •
edited by kibanamachine
Loading

Partially addresses: #148176

Summary

Improved test coverage of the functionality related to the status and installation of the prebuilt rules:

  • Added missing integrational test for the get_prebuilt_rules_status and install_prebuilt_rules endpoints
  • Added tests for two detection engine package types 1) with historical rules version and 2) with the latest version only.
  • Added integrational tests that use actual Fleet packages in addition to tests that use mocks

See test scenarios here: #148176 (comment)

Not covered by this PR

  • E2E tests to ensure that detection rules package installation doesn't fail with OOM
  • Ability to test a prebuilt rule package of a specific version; the latest version is always used as of now
  • Cypress tests

@xcrzx xcrzx self-assigned this Jan 25, 2023
@xcrzx xcrzx force-pushed the prebuilt-rules-tests branch 2 times, most recently from f2c4f52 to 65d7195 Compare January 25, 2023 12:43
@xcrzx xcrzx added 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 technical debt Improvement of the software architecture and operational architecture release_note:skip Skip the PR/issue when compiling release notes Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area v8.7.0 labels Jan 25, 2023
@xcrzx xcrzx force-pushed the prebuilt-rules-tests branch 8 times, most recently from e50822d to f6a34b7 Compare January 27, 2023 15:33
@xcrzx xcrzx marked this pull request as ready for review January 27, 2023 16:10
@xcrzx xcrzx requested review from a team as code owners January 27, 2023 16:10
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@xcrzx xcrzx force-pushed the prebuilt-rules-tests branch from f6a34b7 to 205ec9a Compare January 27, 2023 16:14
@banderror banderror requested review from a team, spong and banderror and removed request for a team and spong January 27, 2023 16:37
@xcrzx xcrzx force-pushed the prebuilt-rules-tests branch from 205ec9a to 1460266 Compare January 30, 2023 10:22
@xcrzx xcrzx force-pushed the prebuilt-rules-tests branch from 1460266 to d65e9f6 Compare February 15, 2023 10:23
@xcrzx xcrzx added the v8.8.0 label Feb 15, 2023
banderror
banderror reviewed Feb 20, 2023
View reviewed changes
Copy link
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.

@xcrzx Fantastic PR! I ended up posting quite a bunch of comments, although non of them are critical. Let's go over them together when you read them, and also sync on the test plan and the next steps.

x-pack/test/detection_engine_api_integration/basic/tests/index.ts Show resolved Hide resolved
x-pack/test/detection_engine_api_integration/security_and_spaces/group1/index.ts Outdated Show resolved Hide resolved
x-pack/test/detection_engine_api_integration/utils/index.ts Outdated Show resolved Hide resolved
x-pack/test/detection_engine_api_integration/utils/increment_prebuilt_rule_version.ts Outdated Show resolved Hide resolved
x-pack/test/detection_engine_api_integration/utils/increment_prebuilt_rule_version.ts Outdated Show resolved Hide resolved
...k/test/detection_engine_api_integration/security_and_spaces/group1/install_prebuilt_rules.ts Outdated Show resolved Hide resolved
x-pack/test/detection_engine_api_integration/security_and_spaces/group1/fleet_integration.ts Outdated Show resolved Hide resolved
x-pack/test/detection_engine_api_integration/security_and_spaces/group1/fleet_integration.ts Show resolved Hide resolved
x-pack/test/detection_engine_api_integration/security_and_spaces/group1/fleet_integration.ts Outdated Show resolved Hide resolved
x-pack/test/detection_engine_api_integration/security_and_spaces/group1/fleet_integration.ts Outdated Show resolved Hide resolved
@xcrzx xcrzx force-pushed the prebuilt-rules-tests branch 3 times, most recently from 053f92c to f4b686c Compare February 22, 2023 10:07
@xcrzx xcrzx requested a review from banderror February 22, 2023 11:47
@xcrzx xcrzx force-pushed the prebuilt-rules-tests branch from f4b686c to 146874c Compare February 22, 2023 11:47
pmuellr
pmuellr approved these changes Feb 22, 2023
View reviewed changes
Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

ResponseOps changes LGTM

@xcrzx xcrzx force-pushed the prebuilt-rules-tests branch from 146874c to 4c7285c Compare February 22, 2023 17:01
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

  • 💚 Build #109583 succeeded 146874c88a1d199d42154bc37d58d6f4688a455d
  • 💚 Build #109543 succeeded f4b686c72b3aab5274e332514a5fad16cb740d17
  • 💔 Build #109349 failed 053f92c744002b0314450eaf6fe58a4d8ae7382b
  • 💚 Build #109143 succeeded 008215f4479d6f389468300208306640c0ed33dc
  • 💚 Build #108145 succeeded d65e9f6e97d990597ebf058379561da622f45b5d
  • 💛 Build #103676 was flaky 1460266196b16b07ac33a1bf4c066faed82be3e6

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @xcrzx

banderror
banderror approved these changes Feb 23, 2023
View reviewed changes
Copy link
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.

@xcrzx Thank you for addressing all the comments. Tests started to become much better thanks to your leadership on their quality 🙌

@xcrzx xcrzx merged commit a2bba15 into elastic:main Feb 23, 2023
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
❌ 8.7 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 149502

Questions ?

Please refer to the Backport tool documentation

@xcrzx xcrzx mentioned this pull request Feb 23, 2023
Merged
@xcrzx
Copy link
Contributor Author

xcrzx commented Feb 23, 2023

💚 All backports created successfully

Status Branch Result
✅ 8.7

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

Questions ?

Please refer to the Backport tool documentation

xcrzx added a commit to xcrzx/kibana that referenced this pull request Feb 23, 2023
@xcrzx
[Security Solution] Revisit prebuilt detection rules tests (elastic#1…
0671989 
…49502)

(cherry picked from commit a2bba15)

# Conflicts:
#	x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts
#	x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts
xcrzx added a commit that referenced this pull request Feb 23, 2023
@xcrzx
[8.7] [Security Solution] Revisit prebuilt detection rules tests (#14…
7a91663 
…9502) (#151999)

# Backport

This will backport the following commits from `main` to `8.7`:
- [[Security Solution] Revisit prebuilt detection rules tests
(#149502)](#149502)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Dmitrii
Shevchenko","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-02-23T12:36:57Z","message":"[Security
Solution] Revisit prebuilt detection rules tests
(#149502)","sha":"a2bba156e23bfa1cb57f58c5f6e864b00d3d6203","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["technical
debt","release_note:skip","Team:Detections and Resp","Team:
SecuritySolution","Team:Detection Rules","Feature:Prebuilt Detection
Rules","v8.7.0","v8.8.0"],"number":149502,"url":"https://github.com/elastic/kibana/pull/149502","mergeCommit":{"message":"[Security
Solution] Revisit prebuilt detection rules tests
(#149502)","sha":"a2bba156e23bfa1cb57f58c5f6e864b00d3d6203"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/149502","number":149502,"mergeCommit":{"message":"[Security
Solution] Revisit prebuilt detection rules tests
(#149502)","sha":"a2bba156e23bfa1cb57f58c5f6e864b00d3d6203"}}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@banderror banderror banderror approved these changes

@pmuellr pmuellr pmuellr approved these changes

Assignees

@xcrzx xcrzx

Labels
Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area release_note:skip Skip the PR/issue when compiling release notes 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. technical debt Improvement of the software architecture and operational architecture v8.7.0 v8.8.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@xcrzx @elasticmachine @kibana-ci @kibanamachine @pmuellr @banderror