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] Migrate Prebuilt rules API integration tests to security_solution_api_integration folder #169951

Merged
merged 17 commits into from
Nov 10, 2023
Merged

[Security Solution] Migrate Prebuilt rules API integration tests to security_solution_api_integration folder #169951

merged 17 commits into from
Nov 10, 2023

Conversation

jpdjere
Copy link
Contributor

@jpdjere jpdjere commented Oct 26, 2023 •
edited
Loading

Addresses partially: #151902

Summary

  • Migrates Prebuilt rules-related API integration tests to security_solution_api_integration folder.
    • Moves tests from x-pack/test/detection_engine_api_integration/security_and_spaces into x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license.
    • PR moves following folders:
      • /prebuilt_rules
      • /bundled_prebuilt_rules_package
      • /large_prebuilt_rules_package
      • /update_prebuilt_rules_package
  • Duplicates or completely moves needed utils into:
    - x-pack/test/security_solution_api_integration/test_suites/detections_response/utils/rules/prebuilt_rules, depending if they are no longer used in the original folder or they still are in the remaining test (should be moved shortly as well)
  • All tests run on both ESS and Serverless

Flaky test runner

Link to all for PR

For maintainers

@jpdjere jpdjere force-pushed the migrate-prebuilt-rules-integration-tests branch 2 times, most recently from 2190d36 to 159b54a Compare October 26, 2023 16:03
@jpdjere jpdjere mentioned this pull request Oct 27, 2023
Closed
@jpdjere jpdjere force-pushed the migrate-prebuilt-rules-integration-tests branch from 67f287c to 143e248 Compare November 1, 2023 10:53
@jpdjere jpdjere marked this pull request as ready for review November 2, 2023 15:04
@jpdjere jpdjere requested review from a team as code owners November 2, 2023 15:04
@jpdjere jpdjere self-assigned this Nov 2, 2023
@jpdjere jpdjere added test release_note:skip Skip the PR/issue when compiling release notes 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 Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area v8.12.0 labels Nov 2, 2023
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

jbudz
jbudz approved these changes Nov 2, 2023
View reviewed changes
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

ftr_configs.yml

@jpdjere jpdjere requested a review from maximpn November 3, 2023 09:04
@jpdjere jpdjere mentioned this pull request Nov 3, 2023
Closed
maximpn
maximpn reviewed Nov 3, 2023
View reviewed changes
.buildkite/ftr_configs.yml Outdated
@@ -461,7 +457,15 @@ enabled:
- x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_creation/configs/ess.config.ts
- x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/actions/configs/serverless.config.ts
- x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/actions/configs/ess.config.ts

- x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/prebuilt_rules/configs/serverless.config.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we just have prebuilt_rules group?

Copy link
Contributor Author

@jpdjere jpdjere Nov 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Because all of these folders have different configs which are specific to the tests, within them. For example, for large_prebuilt_rules_package, we need to configure the env to use a max of 800mb to tests possible OOM issues. Similarly with the others; the specific configs are passed to the ess.config.ts and serveres.config.ts files

WafaaNasr
WafaaNasr reviewed Nov 6, 2023
View reviewed changes
Copy link
Contributor

@WafaaNasr WafaaNasr left a comment

Choose a reason for hiding this comment

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

Could you please update the Codeowner file for the newly added files, thanks!

@jpdjere jpdjere force-pushed the migrate-prebuilt-rules-integration-tests branch from 7b32e6d to 46f3894 Compare November 6, 2023 17:09
jpdjere
jpdjere commented Nov 6, 2023
View reviewed changes
.github/CODEOWNERS Outdated
@@ -1314,6 +1314,10 @@ x-pack/plugins/cloud_integrations/cloud_full_story/server/config.ts @elastic/kib
/x-pack/test/security_solution_cypress/cypress/e2e/overview @elastic/security-detection-engine
x-pack/test/security_solution_api_integration/test_suites/detections_response/exceptions @elastic/security-detection-engine
x-pack/test/security_solution_api_integration/test_suites/detections_response/rule_creation @elastic/security-detection-engine
/x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/bundled_prebuilt_rules_package @elastic/security-detection-rule-management
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@WafaaNasr Thanks for the heads up on updating CODEOWNERS.

Just pinging you here cause the routes that you added for your folders look non-existent, missing the /default_license dir. Not sure if it works anyways, but worth a check.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, Juan!! Yes correct I updated them in this PR

@banderror banderror self-requested a review November 6, 2023 18:33
banderror
banderror reviewed Nov 7, 2023
View reviewed changes
Copy link
Contributor

@banderror banderror left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The changes LGTM, thank you @jpdjere 👍 I left a few questions and suggestions.

.github/CODEOWNERS Outdated Show resolved Hide resolved
...uites/detections_response/default_license/large_prebuilt_rules_package/configs/ess.config.ts Outdated Show resolved Hide resolved
...egration/test_suites/detections_response/default_license/prebuilt_rules/fleet_integration.ts Outdated Show resolved Hide resolved
...tion_api_integration/test_suites/detections_response/default_license/prebuilt_rules/index.ts Outdated Show resolved Hide resolved
...est_suites/detections_response/utils/rules/prebuilt_rules/delete_all_prebuilt_rule_assets.ts Show resolved Hide resolved
@jpdjere jpdjere force-pushed the migrate-prebuilt-rules-integration-tests branch from 5257c29 to 74b7c48 Compare November 7, 2023 23:36
@jpdjere jpdjere requested a review from banderror November 8, 2023 13:15
banderror
banderror approved these changes Nov 9, 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.

Thanks for addressing my comments @jpdjere, the result looks awesome! All the flaky test runs also seem to have succeeded without a single error, so let's rebase the PR and merge it 🚀

@jpdjere jpdjere force-pushed the migrate-prebuilt-rules-integration-tests branch from fdba032 to 765f986 Compare November 9, 2023 18:45
@jpdjere jpdjere force-pushed the migrate-prebuilt-rules-integration-tests branch from 765f986 to 75ad811 Compare November 9, 2023 18:50
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

  • 💚 Build #174101 succeeded fdba0322c4ff9a16e9ec59d0c1cc817bdce58e8b
  • 💔 Build #174061 failed 5928e1ab4f46bdd52e2f6864d04ff28da4be22bb
  • 💚 Build #173580 succeeded 46f38949001b23d28a9f465ce71f0dcd3d004943
  • 💛 Build #173081 was flaky 3f6d86de2e54619d08456495e1e9d1ac0aace7d1
  • 💛 Build #172842 was flaky 97496ec9d8885f68ff6bbebdf70aeb7d41a702f3
  • 💔 Build #172799 failed 8d39a28c9205c2d50469047ddde3d9891cbf3765

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

cc @jpdjere

@jpdjere jpdjere merged commit 4b28ec4 into elastic:main Nov 10, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Nov 10, 2023
@jpdjere jpdjere deleted the migrate-prebuilt-rules-integration-tests branch November 10, 2023 10:05
@jpdjere jpdjere mentioned this pull request Nov 10, 2023
Merged
3 tasks
@banderror banderror mentioned this pull request Nov 15, 2023
Closed
3 tasks
@logeekal logeekal mentioned this pull request Nov 16, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WafaaNasr WafaaNasr WafaaNasr left review comments

@maximpn maximpn maximpn left review comments

@banderror banderror banderror approved these changes

@jbudz jbudz jbudz approved these changes

Assignees

@jpdjere jpdjere

Labels
backport:skip This commit does not require backporting 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. test v8.12.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@jpdjere @elasticmachine @kibana-ci @jbudz @maximpn @banderror @WafaaNasr @kibanamachine