[Security Solution] Removes `esArchiverResetKibana` from our Cypress tests #169563

MadameSheema · 2023-10-23T18:41:03Z

Context

We are working to execute our Cypress tests in the second Quality Gate (QA environment, deployed projects on MKI). To do so we are mimicking the orchestration we currently have thanks to the parallel.ts script on our ESS and serverless FTR environment, where each spec file is executed in its own project and each spec file can set the PLI that needs to be executed.

While testing the changes, we saw the following error when esArchiverResetKibana was executed:

cy.task('esArchiverResetKibana') failed with the following error:

> security_exception

Root causes:

security_exception: action [indices:data/write/delete/byquery] is unauthorized for user [elastic] with effective roles [superuser] on restricted indices [.kibana_task_manager,.kibana,.kibana_ingest,.kibana_alerting_cases,.kibana_security_solution,.kibana_analytics], this action is granted by the index privileges [delete,write,all]

This is because in a real serverless environment, it is not allowed to access restricted / system indices directly and esArchiverResetKibana is deleting a bunch of those.

In this PR

In this PR we are getting rid of esArchiverResetKibana method, in order to do so, we are following different strategies depending on the requirements of the tests:

(1) Removing the method directly from the test. This is because each spec file is executed in a clean instance and there is no need to remove/clean anything between tests.

(2) Cleaning just the indices that are modified during the spec execution and may impact other tests inside the same spec.

(3) Different approach.

Approaches followed for each test:

detection_response/detection_alerts/alert_tags.cy.ts $\rightarrow$ (1)
detection_response/prebuilt_rules_install_update_workflows.cy.ts $\rightarrow$ (3)
- In this situation, we have split the original spec in 3 different ones.
detection_response/rule_creation/custom_query_rule_data_view.cy.ts $\rightarrow$ (2)
ddetection_response/rule_management/rule_actions/bulk_actions/bulk_duplicate_rules.cy.ts $\rightarrow$ (1)
detection_response/rule_management/rule_actions/bulk_actions/bulk_edit_rules.cy.ts $\rightarrow$ (1)
detection_response/rule_management/rule_actions/bulk_actions/bulk_edit_rules_actions.cy.ts $\rightarrow$ (1)
detection_response/rule_management/rule_actions/bulk_actions/bulk_edit_rules_data_view.cy.ts $\rightarrow$ (2)
detection_response/rule_management/rules_table/rules_table_filtering.cy.ts $\rightarrow$ (1)
detection_response/sourcerer/sourcerer.cy.ts $\rightarrow$ (1)
detection_response/sourcerer/sourcerer_permissions.cy.ts $\rightarrow$ (1)
exceptions/alerts_table_flow/endpoint_exceptions.cy.ts $\rightarrow$ (1)
exceptions/entry/flyout_validation.cy.ts $\rightarrow$ (1)
exceptions/entry/multiple_conditions.cy.ts $\rightarrow$ (1)
exceptions/rule_details_flow/add_edit_endpoint_exception.cy.ts $\rightarrow$ (2)
exceptions/rule_details_flow/add_edit_exception.cy.ts $\rightarrow$ (2)
exceptions/rule_details_flow/add_edit_exception_data_view.cy.ts $\rightarrow$ (1)
exceptions/shared_exception_lists_management/list_detail_page/list_details.cy.ts $\rightarrow$ (1)
exceptions/shared_exception_lists_management/manage_exceptions.cy.ts $\rightarrow$ (2)
exceptions/shared_exception_lists_management/manage_exceptions.cy.ts $\rightarrow$ (2)
exceptions/shared_exception_lists_management/filter_table.cy.ts $\rightarrow$ (1)
exceptions/shared_exception_lists_management/import_lists.cy.ts $\rightarrow$ (1)
exceptions/shared_exception_lists_management/manage_lists.cy.ts $\rightarrow$ (1)
exceptions/shared_exception_lists_management/read_only.cy.ts $\rightarrow$ (1)

Things to take into consideration

These changes would help to make most of the above tests pass in MKI environment. We need to take into consideration that some might still fail if we continue accessing restricted indices.

Next steps

Once the PR is merged, execute all the tests again on MKI.
Identify tests that still fail due to the access of restricted indices.
Notify each responsible team about it.

elasticmachine · 2023-10-25T18:43:54Z

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

janmonschke

Threat Hunting changes lgtm 🎉

vitaliidm

Detection Engine area LGTM

maximpn

@MadameSheema thank you for addressing MKI related issues in the tests 🙏

The approach looks good to me but some details require clarification so I left comments.

maximpn · 2023-10-27T07:35:28Z

.../prebuilt_rules/prebuilt_rules_install_update_workflows/installation_of_prebuilt_rules.cy.ts

+} from '../../../../tasks/prebuilt_rules';
+import { visitRulesManagementTable } from '../../../../tasks/rules_management';
+
+describe.skip(


Why this test suite is skipped?

I splitted prebuilt_rules_install_update_workflows.cy.ts in different spec files like this one, at some point my PR started to fail and I saw that was skipped in main, so I skipped this test as was part of the original skipped spec file.

Now that the issue as been addressed I'll make sure it us unskipped and all the changes made are reflected as well.

@jpdjere Could you please review the changes to tests for prebuilt rules in this PR?

maximpn · 2023-10-27T07:35:33Z

...response/prebuilt_rules/prebuilt_rules_install_update_workflows/installation_via_fleet.cy.ts

+import { addElasticRulesButtonClick } from '../../../../tasks/prebuilt_rules';
+import { visitRulesManagementTable } from '../../../../tasks/rules_management';
+
+describe.skip(


Why this test suite is skipped?

I splitted prebuilt_rules_install_update_workflows.cy.ts in different spec files like this one, at some point my PR started to fail and I saw that was skipped in main, so I skipped this test as was part of the original skipped spec file.

Now that the issue as been addressed I'll make sure it us unskipped and all the changes made are reflected as well.

maximpn · 2023-10-27T07:36:00Z

...ponse/prebuilt_rules/prebuilt_rules_install_update_workflows/upgrade_of_prebuilt_rules.cy.ts

+} from '../../../../tasks/prebuilt_rules';
+import { visitRulesManagementTable } from '../../../../tasks/rules_management';
+
+describe.skip(


Why this test suite is skipped?

I splitted prebuilt_rules_install_update_workflows.cy.ts in different spec files like this one, at some point my PR started to fail and I saw that was skipped in main, so I skipped this test as was part of the original skipped spec file.

Now that the issue as been addressed I'll make sure it us unskipped and all the changes made are reflected as well.

maximpn · 2023-10-27T07:37:25Z

...response/prebuilt_rules/prebuilt_rules_install_update_workflows/installation_via_fleet.cy.ts

nit: Maybe rename file to install_via_fleet.cy.ts?

maximpn · 2023-10-27T07:41:38Z

.../prebuilt_rules/prebuilt_rules_install_update_workflows/installation_of_prebuilt_rules.cy.ts

nit: Maybe rename to install_workflow? We could reduce occurrences of prebuilt_rules in the path to make it shorter but it still will be clear what group these tests belong.

I'd also move prebuilt rules tests one directory up to remove prebuilt_rules_install_update_workflows folder. So we'd have all prebuilt rules test suites under prebuilt_rules folder.

maximpn · 2023-10-27T08:03:48Z

...detection_response/rule_management/rule_actions/bulk_actions/bulk_edit_rules_data_view.cy.ts

@@ -132,6 +127,10 @@ describe(
      ]);
    });

+    afterEach(() => {


Let's delete data view in before each.

maximpn · 2023-10-27T08:07:48Z

...ion_cypress/cypress/e2e/exceptions/shared_exception_lists_management/manage_exceptions.cy.ts

@@ -46,13 +46,12 @@ describe(
  { tags: ['@ess', '@serverless', '@skipInServerless'] },
  () => {
    beforeEach(() => {
-      cy.task('esArchiverResetKibana');
+      login();


Interesting that login() isn't required to be done before ES data manipulation helpers as rootRequest() helper under the hood uses ELASTICSEARCH_USERNAME and ELASTICSEARCH_PASSWORD for auth.

maximpn · 2023-10-27T08:09:05Z

x-pack/test/security_solution_cypress/cypress/tasks/api_calls/exceptions.ts

@@ -19,15 +19,23 @@ export const createEndpointExceptionList = () =>
  rootRequest<ExceptionListSchema>({
    method: 'POST',
    url: ENDPOINT_LIST_URL,
-    headers: { 'kbn-xsrf': 'cypress-creds', 'x-elastic-internal-origin': 'security-solution' },
+    headers: {
+      'kbn-xsrf': 'cypress-creds',


rootRequet already has this header. Ideally 'x-elastic-internal-origin': 'security-solution' should be inside rootRequest as well.

@maximpn I see that this is a broader problem across other API calls referencing rootRequest. It is if the cleanup is done in a follow-up PR?

maximpn · 2023-10-27T08:10:11Z

x-pack/test/security_solution_cypress/cypress/tasks/exceptions.ts

@@ -183,6 +185,12 @@ export const validateEmptyExceptionConditionField = () => {
 };
 export const submitNewExceptionItem = () => {
  cy.get(CONFIRM_BTN).should('exist');
+  cy.root().then(($page) => {


Why was it added?

When doing some pair programming and testing with @WafaaNasr to check which indices we should delete for the exceptions-specific tests, sometimes we saw displayed an unrelated toaster error message unrelated with the testing we were performing. That toaster was blocking the confirm button we had to click, using force true would solve the issue, but I'm against the use of it.

If you check the current code on tasks/exceptions_table.ts, there are some tests that use the closeErrorToast() method to close error toasters before continuing with the interactions with the page. That is why I decided to add a smaller piece of code that checks if a toaster is displayed and if so, close it to continue with the test.

@MadameSheema thank you for the explanation. Did I get it right the toaster doesn't always appear? May it's some UI related issue.

Could you add a comment in. the code to clarify your changes?

banderror · 2023-10-27T12:48:53Z

@MadameSheema Thank you for taking care of splitting tests for prebuilt rules into multiple files, the one we had got too big already. @jpdjere Please review these changes since you're the main author and the owner of these tests.

@MadameSheema Once all the adjustments are done in this PR, before merging it, please run the changed tests in the flaky test runner in both ESS and Serverless. @jpdjere and @maximpn have been doing this for all their PRs for tests for the last several weeks and it's a good way to make sure PRs don't introduce new flakes.

jpdjere · 2023-10-27T14:01:43Z

...lution_cypress/cypress/e2e/detection_response/prebuilt_rules/upgrade_of_prebuilt_rules.cy.ts

To keep consistency with install_workflow let's rename this file to update_workflow, as they were both two subsections of the same test suite.

jpdjere

Thanks for the splitting up of the /prebuilt_rules dir. Left only one comment to rename a file for consistency.

MadameSheema · 2023-10-30T10:35:29Z

@elasticmachine merge upstream

MadameSheema · 2023-10-30T15:42:12Z

150 executions for the tests inside the exceptions folder and all passed :)

maximpn

@MadameSheema thank you addressing the comments 👍

MadameSheema · 2023-10-31T08:21:09Z

150 executions for the tests inside the detections_response folders. The only test failing is a known flaky test and is unrelated to the changes I introduced, so if after merging with main al the tests continue passing, I'll merge the changes. Thanks!! :)

kibana-ci · 2023-10-31T09:29:34Z

💚 Build Succeeded

Buildkite Build
Commit: 008f253

Metrics [docs]

✅ unchanged

History

💚 Build #171809 succeeded f351876
💔 Build #171792 failed 022b19a
💚 Build #171558 succeeded 21d31f8
💔 Build #171479 failed 8f2521c
💚 Build #170839 succeeded 31a22d8
💔 Build #170688 failed f4b4f5e

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

cc @MadameSheema

## Summary After the merge of #169563 seems that there is a test still using resetKibana (I don't know the reason). In this PR we are fixing that. Execution in ESS: <img width="2560" alt="Screenshot 2023-10-31 at 12 46 55" src="https://github.com/elastic/kibana/assets/17427073/dc14eb9a-9ef5-4c35-be83-9f76d9812f8e"> Execution in Serverless: <img width="2552" alt="Screenshot 2023-10-31 at 12 49 14" src="https://github.com/elastic/kibana/assets/17427073/28594862-1c0d-40f5-9b9e-dfe7f97584cc">

## Summary After the merge of elastic#169563 seems that there is a test still using resetKibana (I don't know the reason). In this PR we are fixing that. Execution in ESS: <img width="2560" alt="Screenshot 2023-10-31 at 12 46 55" src="https://github.com/elastic/kibana/assets/17427073/dc14eb9a-9ef5-4c35-be83-9f76d9812f8e"> Execution in Serverless: <img width="2552" alt="Screenshot 2023-10-31 at 12 49 14" src="https://github.com/elastic/kibana/assets/17427073/28594862-1c0d-40f5-9b9e-dfe7f97584cc">

MadameSheema added 11 commits October 23, 2023 18:24

removes reset kibana from alert_tags

0f9882e

removing reset kibana from shared exception list

02ad7a6

removing reset kibana from several places

1a8e41e

removing more methods

aa5198e

deleting deleting more reset kibana methods

cfc17c6

splits prebuilt_rules_install_update_workflows tests

bab9949

removing more reset-kibana methods

78e2edd

removing remaining reset kibana methods

3970e41

merging main

e66b59e

adjustments

f4b4f5e

skipping same tests we currently have skipped in main

31a22d8

MadameSheema changed the title ~~removes reset kibana from alert_tags~~ [Security Solution] Removes esArchiverResetKibana from our Cypress tests Oct 25, 2023

MadameSheema self-assigned this Oct 25, 2023

MadameSheema added release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.12.0 labels Oct 25, 2023

MadameSheema marked this pull request as ready for review October 25, 2023 18:43

MadameSheema requested review from a team as code owners October 25, 2023 18:43

MadameSheema requested a review from banderror October 25, 2023 18:43

janmonschke approved these changes Oct 26, 2023

View reviewed changes

maximpn requested review from maximpn and removed request for banderror October 26, 2023 10:42

angorayc approved these changes Oct 26, 2023

View reviewed changes

vitaliidm approved these changes Oct 26, 2023

View reviewed changes

dkirchan approved these changes Oct 26, 2023

View reviewed changes

maximpn reviewed Oct 27, 2023

View reviewed changes

addressing comments

8f2521c

banderror requested a review from jpdjere October 27, 2023 12:32

merging main

21d31f8

jpdjere reviewed Oct 27, 2023

View reviewed changes

jpdjere approved these changes Oct 27, 2023

View reviewed changes

MadameSheema added 2 commits October 30, 2023 10:35

addressing comments

baa048d

Merge branch 'main' into cypress/clean-up-reset-kibana

022b19a

MadameSheema added the ci:skip-cypress-osquery Skips osquery cypress checks label Oct 30, 2023

Merge branch 'main' into cypress/clean-up-reset-kibana

f351876

maximpn approved these changes Oct 30, 2023

View reviewed changes

Merge branch 'main' into cypress/clean-up-reset-kibana

008f253

MadameSheema enabled auto-merge (squash) October 31, 2023 08:21

MadameSheema merged commit 0f3382f into elastic:main Oct 31, 2023

kibanamachine added the backport:skip This commit does not require backporting label Oct 31, 2023

MadameSheema deleted the cypress/clean-up-reset-kibana branch October 31, 2023 10:32

MadameSheema mentioned this pull request Oct 31, 2023

[Security Solution] Fixes problematic test #170222

Merged

MadameSheema mentioned this pull request Dec 13, 2023

[Security Solution] Preparing Cypress for the second quality gate - latest steps #173327

Open

43 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Removes `esArchiverResetKibana` from our Cypress tests #169563

[Security Solution] Removes `esArchiverResetKibana` from our Cypress tests #169563

MadameSheema commented Oct 23, 2023 •

edited

Loading

elasticmachine commented Oct 25, 2023

janmonschke left a comment

vitaliidm left a comment

maximpn left a comment

maximpn Oct 27, 2023

MadameSheema Oct 27, 2023

banderror Oct 27, 2023

maximpn Oct 27, 2023

MadameSheema Oct 27, 2023

maximpn Oct 27, 2023

MadameSheema Oct 27, 2023

maximpn Oct 27, 2023

maximpn Oct 27, 2023

maximpn Oct 27, 2023

maximpn Oct 27, 2023

maximpn Oct 27, 2023

MadameSheema Oct 27, 2023

maximpn Oct 27, 2023

MadameSheema Oct 27, 2023 •

edited

Loading

maximpn Oct 27, 2023

banderror commented Oct 27, 2023

jpdjere Oct 27, 2023

jpdjere left a comment

MadameSheema commented Oct 30, 2023

MadameSheema commented Oct 30, 2023

maximpn left a comment

MadameSheema commented Oct 31, 2023

kibana-ci commented Oct 31, 2023

[Security Solution] Removes esArchiverResetKibana from our Cypress tests #169563

[Security Solution] Removes esArchiverResetKibana from our Cypress tests #169563

Conversation

MadameSheema commented Oct 23, 2023 • edited Loading

Context

In this PR

Approaches followed for each test:

Things to take into consideration

Next steps

elasticmachine commented Oct 25, 2023

janmonschke left a comment

Choose a reason for hiding this comment

vitaliidm left a comment

Choose a reason for hiding this comment

maximpn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MadameSheema Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

banderror commented Oct 27, 2023

Choose a reason for hiding this comment

jpdjere left a comment

Choose a reason for hiding this comment

MadameSheema commented Oct 30, 2023

MadameSheema commented Oct 30, 2023

maximpn left a comment

Choose a reason for hiding this comment

MadameSheema commented Oct 31, 2023

kibana-ci commented Oct 31, 2023

💚 Build Succeeded

Metrics [docs]

History

[Security Solution] Removes `esArchiverResetKibana` from our Cypress tests #169563

[Security Solution] Removes `esArchiverResetKibana` from our Cypress tests #169563

MadameSheema commented Oct 23, 2023 •

edited

Loading

MadameSheema Oct 27, 2023 •

edited

Loading