[Security Solution][API testing][Rule Management] Move and restructures remaining rule managements trail-tests #172173
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1329,6 +1329,14 @@ x-pack/test/security_solution_cypress/cypress/tasks/expandable_flyout @elastic/ | |
| /x-pack/plugins/security_solution/docs/testing/test_plans/detection_response/prebuilt_rules @elastic/security-detection-rule-management | ||
| /x-pack/plugins/security_solution/docs/testing/test_plans/detection_response/rule_management @elastic/security-detection-rule-management | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/prebuilt_rules @elastic/security-detection-rule-management | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_delete @elastic/security-detection-rule-management | ||
| x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_update @elastic/security-detection-rule-management | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_patch @elastic/security-detection-rule-management | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_import_export @elastic/security-detection-rule-management | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_management @elastic/security-detection-rule-management | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_bulk_actions @elastic/security-detection-rule-management | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_read @elastic/security-detection-rule-management | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_creation/create_rules_bulk.ts @elastic/security-detection-rule-management | ||
|
There was a problem hiding this comment. Why do we add a single file to CODEOWNERS? We shouldn't target individual files in CODEOWNERS because this can get out of sync with actual files pretty quickly. There was a problem hiding this comment. Copying from other comment: @banderror let's definitely chat! I plan to address the comments that don't deal with restructuring first, and then open a PR dedicated to any structure updates. For the ownership on the endpoints we largely followed what we saw in the folders structure in the code itself (these routes are under rule_management, but it definitely felt a bit confusing. I'll find a time to sync! |
||
|
|
||
| /x-pack/plugins/security_solution/public/common/components/health_truncate_text @elastic/security-detection-rule-management | ||
| /x-pack/plugins/security_solution/public/common/components/links_to_docs @elastic/security-detection-rule-management | ||
|
|
@@ -1393,14 +1401,24 @@ x-pack/test/security_solution_cypress/cypress/tasks/expandable_flyout @elastic/ | |
| /x-pack/test/security_solution_cypress/cypress/e2e/detection_response/value_lists @elastic/security-detection-engine | ||
| /x-pack/test/security_solution_cypress/cypress/e2e/exceptions @elastic/security-detection-engine | ||
| /x-pack/test/security_solution_cypress/cypress/e2e/overview @elastic/security-detection-engine | ||
|
|
||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/exceptions @elastic/security-detection-engine | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_creation/create_new_terms @elastic/security-detection-engine | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_creation/create_rules @elastic/security-detection-engine | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_creation/preview_rules @elastic/security-detection-engine | ||
|
Comment on lines
+1414
to
+1416
There was a problem hiding this comment. We shouldn't target individual files in CODEOWNERS because this can get out of sync with actual files pretty quickly. There was a problem hiding this comment. Copying from other comment: @banderror let's definitely chat! I plan to address the comments that don't deal with restructuring first, and then open a PR dedicated to any structure updates. For the ownership on the endpoints we largely followed what we saw in the folders structure in the code itself (these routes are under rule_management, but it definitely felt a bit confusing. I'll find a time to sync! |
||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/actions @elastic/security-detection-engine | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/alerts @elastic/security-detection-engine | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/user_roles @elastic/security-detection-engine | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/basic_essentials_license/detection_engine @elastic/security-detection-engine | ||
| /x-pack/plugins/security_solution/server/lib/detection_engine/scripts/roles_users @elastic/security-detection-engine | ||
| /x-pack/test/security_solution_api_integration/test_suites/lists_and_exception_lists @elastic/security-detection-engine | ||
| x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_update/update_rules.ts @elastic/security-detection-engine | ||
| x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_update/update_rules_ess.ts @elastic/security-detection-engine | ||
| x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_patch/patch_rules.ts @elastic/security-detection-engine | ||
| x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_patch/patch_rules_ess.ts @elastic/security-detection-engine | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_delete/delete_rules.ts @elastic/security-detection-engine | ||
| /x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_delete/delete_rules_ess.ts @elastic/security-detection-engine | ||
|
Comment on lines
+1423
to
+1428
There was a problem hiding this comment. Same here regarding targeting individual files. Also, I'd like to chat with you @yctercero about which team should own which rules CRUD tests. I don't understand the proposed separation between our two teams, and I have some thoughts about how we should do it. There was a problem hiding this comment. @banderror let's definitely chat! I plan to address the comments that don't deal with restructuring first, and then open a PR dedicated to any structure updates. For the ownership on the endpoints we largely followed what we saw in the folders structure in the code itself (these routes are under rule_management, but it definitely felt a bit confusing. I'll find a time to sync! |
||
|
|
||
|
|
||
| ## Security Threat Intelligence - Under Security Platform | ||
| /x-pack/plugins/security_solution/public/common/components/threat_match @elastic/security-detection-engine | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a big fan of the folder structure proposed in this PR.
I think these are too many folders to track them all separately in CODEOWNERS. It would make it much easier for both code navigation and CODEOWNERS if we grouped these test groups into higher-level folders reflecting our subdomains. In this case, all these folders could be under a
rule_managementfolder so we'd only need to add a single line to this file here:/x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/rule_management @elastic/security-detection-rule-managementFor the sake of having less pain I'd suggest to merge this PR as is and restructure these folders in a follow-up one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copying from other comment:
@banderror let's definitely chat! I plan to address the comments that don't deal with restructuring first, and then open a PR dedicated to any structure updates. For the ownership on the endpoints we largely followed what we saw in the folders structure in the code itself (these routes are under rule_management, but it definitely felt a bit confusing.
I'll find a time to sync!