[Security Solution] Unskip related integrations tests #164933

maximpn · 2023-08-27T10:39:46Z

Fixes: #154663
Fixes: #153684

Summary

This PR unskips Cypress rule related integration tests (related_integrations.cy.ts).

Details

Testing approach has changed. Instead of importing a rule and installing agent and package policies via Fleet UI the rule is created by mocking a prebuilt rule asset and Fleet API is used to install required integrations. Along the way it required to add and update some testing selectors as UI had changed while the tests were skipped.

Flaky test runner

related_integrations.cy.ts (150 runs) 🟢

elasticmachine · 2023-08-28T06:16:37Z

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

elasticmachine · 2023-08-28T06:16:38Z

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

jpdjere · 2023-08-29T08:46:35Z

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

+ *
+ * - open `app/integrations/detail/aws-1.17.0/overview?integration=cloudfront`
+ * - click the button `Add Amazon CloudFront`
+ * - fill in `Query URL`


Suggested change

* - fill in `Query URL`

* - fill in `Queue URL`

jpdjere · 2023-08-29T09:03:28Z

...press/e2e/detection_response/rule_management/related_integrations/related_integrations.cy.ts

+/**
+ * Agent policy and AWS package policy can be generated by Kibana by opening a page
+ * `app/integrations/detail/aws-1.17.0/overview?integration=cloudfront` then
+ * click the button `Add Amazon CloudFront` -> fill in `Query URL` -> press `Preview API request` at the bottom.


Suggested change

* click the button `Add Amazon CloudFront` -> fill in `Query URL` -> press `Preview API request` at the bottom.

* click the button `Add Amazon CloudFront` -> fill in `Queue URL` -> press `Preview API request` at the bottom.

jpdjere · 2023-08-29T09:08:07Z

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

@@ -195,6 +198,18 @@ export const preventPrebuiltRulesPackageInstallation = () => {
  cy.intercept('POST', '/api/fleet/epm/packages/security_detection_engine/*', {});
 };

+export const addAvailableToInstallPrebuiltRules = (


I think the name of the method is a little confusing, if you don't understand the whole idea of first installing the assets, and then installing those assets as prebuilt rules to Kibana.

What do you think about makeRulesAvailableForInstall?

I ended up with installPrebuiltRuleAssets(). It better reflects the process under the hood and an explanation comment should give some info. Generally speaking we can change business logic later on so installed assets aren't available to be installed without a flag or something and the function names makeRulesAvailableForInstall won't do it its name says.

jpdjere · 2023-08-29T09:10:39Z

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


  if (installToKibana) {
+    cy.log('Install prebuilt rules', rules.length);


Now that we have addAvailableToInstallPrebuiltRules, or however we want to call it, I think createAndInstallMockedPrebuiltRules should always install the rules that were created as assets, without having the optional installToKibana flag as param. So if you don't actually want to install the rules, you should just use addAvailableToInstallPrebuiltRules.

Anyways, that is maybe out of scope of this PR, we can follow up on it.

You are right, this is what we want eventually but it's out of scope of this PR. I'll come up with a refactoring PR to get rid of installToKibana param.

jpdjere · 2023-08-29T09:21:59Z

...press/e2e/detection_response/rule_management/related_integrations/related_integrations.cy.ts

+ * The contents below is a shrunk copy from the flyout with the only required inputs.
+ */
+const AWS_PACKAGE_POLICY: PackagePolicyWithoutAgentPolicyId = {
+  package: {
+    name: 'aws',
+    version: '1.17.0',
+  },
+  name: 'aws-1',
+  namespace: 'default',
+  inputs: {
+    'cloudtrail-aws-s3': {
+      enabled: false,
+      streams: {
+        'aws.cloudtrail': {
+          enabled: true,
+          vars: {
+            fips_enabled: false,
+            tags: ['forwarded', 'aws-cloudtrail'],
+            preserve_original_event: false,
+            cloudtrail_regex: '/CloudTrail/',
+            cloudtrail_digest_regex: '/CloudTrail-Digest/',
+            cloudtrail_insight_regex: '/CloudTrail-Insight/',
+            max_number_of_messages: 5,
+          },
+        },
+      },
+    },
+    'elb-aws-s3': {
+      enabled: false,
+      streams: {
+        'aws.elb_logs': {
+          enabled: true,
+          vars: {
+            fips_enabled: false,
+            tags: ['forwarded', 'aws-elb-logs'],
+            preserve_original_event: false,
+            max_number_of_messages: 5,
+          },
+        },
+      },
+    },
+    'firewall-aws-s3': {
+      enabled: false,
+      streams: {
+        'aws.firewall_logs': {
+          enabled: true,
+          vars: {
+            fips_enabled: false,
+            tags: ['forwarded', 'aws-firewall-logs'],
+            preserve_original_event: false,
+            max_number_of_messages: 5,
+          },
+        },
+      },
+    },
+    's3-aws-s3': {
+      enabled: false,
+      streams: {
+        'aws.s3access': {
+          enabled: true,
+          vars: {
+            fips_enabled: false,
+            tags: ['forwarded', 'aws-s3access'],
+            preserve_original_event: false,
+            max_number_of_messages: 5,
+          },
+        },
+      },
+    },
+    'waf-aws-s3': {
+      enabled: false,
+      streams: {
+        'aws.waf': {
+          enabled: true,
+          vars: {
+            fips_enabled: false,
+            tags: ['forwarded', 'aws-waf'],
+            preserve_original_event: false,
+            max_number_of_messages: 5,
+          },
+        },
+      },
+    },
+    'cloudfront-aws-s3': {
+      enabled: true,
+      streams: {
+        'aws.cloudfront_logs': {
+          enabled: true,
+          vars: {
+            queue_url: 'https://example.com',
+            fips_enabled: false,
+            tags: ['forwarded', 'aws-cloudfront'],
+            preserve_original_event: false,
+            max_number_of_messages: 5,
+          },
+        },
+      },
+    },
+  },
+};


Can we move these types, utils and mocks to separate files from the main tests? Probably their own integrations.ts files in the /helpers and /tasks dir?

It's related to this test only. Moving it somewhere will reduce test's transparency. It makes sense if there is another test reusing the same policy.

jpdjere · 2023-08-29T09:25:43Z

...press/e2e/detection_response/rule_management/related_integrations/related_integrations.cy.ts

        cy.get(INTEGRATIONS_POPOVER).should(
          'have.text',
-          `${rule.enabledIntegrations}/${rule.integrations.length} integrations`
+          `${RULE_RELATED_INTEGRATIONS.filter((x) => x.enabled).length}/${
+            RULE_RELATED_INTEGRATIONS.length
+          } integrations`


For readability:

Suggested change

cy.get(INTEGRATIONS_POPOVER).should(

'have.text',

`${rule.enabledIntegrations}/${rule.integrations.length} integrations`

`${RULE_RELATED_INTEGRATIONS.filter((x) => x.enabled).length}/${

RULE_RELATED_INTEGRATIONS.length

} integrations`

const enabledIntegrations = RULE_RELATED_INTEGRATIONS.filter((x) => x.enabled).length;

const totalIntegrations = RULE_RELATED_INTEGRATIONS.length;

cy.get(INTEGRATIONS_POPOVER).should(

'have.text',

`${enabledIntegrations}/${totalIntegrations} integrations`

jpdjere · 2023-08-29T09:27:57Z

...press/e2e/detection_response/rule_management/related_integrations/related_integrations.cy.ts

+        cy.get(INTEGRATION_STATUS).should('have.length', RULE_RELATED_INTEGRATIONS.length);
+
+        RULE_RELATED_INTEGRATIONS.forEach((integration, index) => {
+          cy.get(INTEGRATION_LINK).eq(index).contains(getIntegrationName(integration), {


Wondering about the use of eq(i) here (and in the rest of this file)

Can we guarantee that the order of the integrations links is always the same as defined in RULE_RELATED_INTEGRATIONS? Might it be a cause of flakiness in the future?

As you can see prebuilt rule asset and tests reuse the same constant RULE_RELATED_INTEGRATIONS. Related integrations rendered in the popover are rendered based on rule.related_integrations array. This means the order is stable now and we catch any changes here if some refactoring happens in the future.

jpdjere

LGTM

christineweng

Investigations changes LGTM!

kibana-ci · 2023-08-31T06:06:56Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 7f84a6a

Failed CI Steps

Serverless Security Cypress Tests #10

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	12.6MB	12.6MB	+55.0B

History

💛 Build #154430 was flaky 44fe5247a3bfb4bb486b1df4518a31d248acd2e9
💛 Build #153960 was flaky 6718a670be10389825ba8e64b0e0c63aebaca14f
💔 Build #153956 failed 31abd6d57cca8def342a8d534f8140a987518f5c
💛 Build #153947 was flaky 24a71e503de563a88c2922651347a51d41ae7d42
💔 Build #153945 failed e9d679aaa7233c93a5f2a0dbea6aac7454bbaeec
💔 Build #153939 failed 53cf2bdee4bdaf94dfba5a97bd2c4ad48da020e3

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

cc @maximpn

e40pud

DE changes LGTM

kibanamachine · 2023-08-31T10:33:59Z

💔 All backports failed

Status	Branch	Result
❌	8.10	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 164933

Questions ?

Please refer to the Backport tool documentation

maximpn · 2023-08-31T10:59:21Z

💚 All backports created successfully

Status	Branch	Result
✅	8.10

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

Questions ?

Please refer to the Backport tool documentation

**Fixes: elastic#154663 **Fixes: elastic#153684 ## Summary This PR unskips Cypress rule related integration tests ([related_integrations.cy.ts](https://github.com/elastic/kibana/blob/main/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management/related_integrations/related_integrations.cy.ts)). ## Details Testing approach has changed. Instead of importing a rule and installing agent and package policies via Fleet UI the rule is created by mocking a prebuilt rule asset and Fleet API is used to install required integrations. Along the way it required to add and update some testing selectors as UI had changed while the tests were skipped. ## Flaky test runner [related_integrations.cy.ts (150 runs)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2995) 🟢 (cherry picked from commit 2c3464c) # Conflicts: # x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management/related_integrations/related_integrations.cy.ts # x-pack/test/security_solution_cypress/cypress/tasks/api_calls/prebuilt_rules.ts

#165310) # Backport This will backport the following commits from `main` to `8.10`: - [[Security Solution] Unskip related integrations tests (#164933)](#164933)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)

maximpn self-assigned this Aug 27, 2023

maximpn requested a review from jpdjere August 27, 2023 10:41

maximpn force-pushed the unskip-related-integrations-tests branch 4 times, most recently from 31abd6d to 6718a67 Compare August 28, 2023 06:16

maximpn marked this pull request as ready for review August 28, 2023 06:16

maximpn requested review from a team as code owners August 28, 2023 06:16

jpdjere reviewed Aug 29, 2023

View reviewed changes

maximpn force-pushed the unskip-related-integrations-tests branch from 6718a67 to a767094 Compare August 29, 2023 09:41

maximpn requested a review from jpdjere August 29, 2023 09:53

jpdjere approved these changes Aug 29, 2023

View reviewed changes

christineweng approved these changes Aug 29, 2023

View reviewed changes

jpdjere added the flake-docs Temp label to gather PRs used to create dev docs label Aug 30, 2023

maximpn added 8 commits August 31, 2023 07:04

unskip rule related integrations tests

96a486f

use the correct waitForTheRuleToBeExecuted function

f0b914b

prepare tests's state

604627c

fix selectors issue

fe42d3d

delete data stream before the test

b2fcebc

fix misprints

3cf94e4

rename a helper function

d7d7b6b

improve test's readability

7f84a6a

maximpn force-pushed the unskip-related-integrations-tests branch from 44fe524 to 7f84a6a Compare August 31, 2023 05:04

e40pud approved these changes Aug 31, 2023

View reviewed changes

maximpn merged commit 2c3464c into elastic:main Aug 31, 2023

maximpn deleted the unskip-related-integrations-tests branch August 31, 2023 10:28

maximpn mentioned this pull request Aug 31, 2023

[8.10] [Security Solution] Unskip related integrations tests (#164933) #165310

Merged

This was referenced Oct 1, 2023

[8.10] fix(slo): Handle partial indicator url state (#167247) #167722

Merged

[8.10] backport SLO fixes #168231

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Unskip related integrations tests #164933

[Security Solution] Unskip related integrations tests #164933

maximpn commented Aug 27, 2023 •

edited by kibanamachine

Loading

elasticmachine commented Aug 28, 2023

elasticmachine commented Aug 28, 2023

jpdjere Aug 29, 2023 •

edited

Loading

jpdjere Aug 29, 2023

jpdjere Aug 29, 2023

maximpn Aug 29, 2023

jpdjere Aug 29, 2023

maximpn Aug 29, 2023

jpdjere Aug 29, 2023

maximpn Aug 29, 2023

jpdjere Aug 29, 2023

jpdjere Aug 29, 2023 •

edited

Loading

maximpn Aug 29, 2023 •

edited

Loading

jpdjere left a comment

christineweng left a comment

kibana-ci commented Aug 31, 2023

e40pud left a comment

kibanamachine commented Aug 31, 2023

maximpn commented Aug 31, 2023

	* click the button `Add Amazon CloudFront` -> fill in `Query URL` -> press `Preview API request` at the bottom.
	* click the button `Add Amazon CloudFront` -> fill in `Queue URL` -> press `Preview API request` at the bottom.


		if (installToKibana) {
		cy.log('Install prebuilt rules', rules.length);

[Security Solution] Unskip related integrations tests #164933

[Security Solution] Unskip related integrations tests #164933

Conversation

maximpn commented Aug 27, 2023 • edited by kibanamachine Loading

Summary

Details

Flaky test runner

elasticmachine commented Aug 28, 2023

elasticmachine commented Aug 28, 2023

jpdjere Aug 29, 2023 • edited Loading

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

jpdjere Aug 29, 2023 • edited Loading

Choose a reason for hiding this comment

maximpn Aug 29, 2023 • edited Loading

Choose a reason for hiding this comment

jpdjere left a comment

Choose a reason for hiding this comment

christineweng left a comment

Choose a reason for hiding this comment

kibana-ci commented Aug 31, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

History

e40pud left a comment

Choose a reason for hiding this comment

kibanamachine commented Aug 31, 2023

💔 All backports failed

Manual backport

Questions ?

maximpn commented Aug 31, 2023

💚 All backports created successfully

Questions ?

maximpn commented Aug 27, 2023 •

edited by kibanamachine

Loading

jpdjere Aug 29, 2023 •

edited

Loading

jpdjere Aug 29, 2023 •

edited

Loading

maximpn Aug 29, 2023 •

edited

Loading