Add e2e for Attack discovery #182918

patrykkopycinski · 2024-05-08T10:49:06Z

Summary

Adds e2e tests for Attack discovery. It takes list of preconfigured connectors from ./config/kibana.dev.yml and for each of them is generating Attack discovery and checks if the output is correct (currently just checking if the host provider doesn't contain {, as it was common issue with one of the connectors)

To test in open mode run:

yarn --cwd x-pack/test/security_solution_cypress cypress:open:ess --spec cypress/e2e/ai_assistant/attack_discovery.cy.ts

To test in run mode:

yarn --cwd x-pack/test/security_solution_cypress cypress:ai_assistant:run:ess --headed

To load Attack discovery data to the existing instance:

yarn --cwd x-pack/plugins/security_solution/scripts/load_attack_discovery_data.js

…covery-e2e # Conflicts: # yarn.lock

patrykkopycinski · 2024-05-08T10:54:07Z

/ci

…fix'

patrykkopycinski · 2024-05-08T11:21:45Z

/ci

…ki/kibana into feat/attack-discovery-e2e

patrykkopycinski · 2024-05-08T15:49:55Z

/ci

patrykkopycinski · 2024-05-10T09:09:04Z

/ci

…ki/kibana into feat/attack-discovery-e2e

patrykkopycinski · 2024-05-10T14:29:57Z

/ci

patrykkopycinski · 2024-05-14T16:00:41Z

/ci

patrykkopycinski · 2024-05-14T21:46:22Z

/ci

patrykkopycinski · 2024-05-15T06:30:05Z

/ci

patrykkopycinski · 2024-05-15T08:28:25Z

/ci

paul-tavares

Changes to files owned by the Defend Workflows team look good 👍

semd · 2024-05-16T07:38:22Z

x-pack/plugins/security_solution/scripts/assistant/attack_discovery/load.ts

+        // TODO: add deletion logic
+        // for (const index of existingIndices) {
+        //   await esClient.indices.delete({ index: pattern });
+
+        //   log.info(`Deleted index ${index}`);
+        // }


Could you explain a bit more why is the index delete logic commented out as a todo?

it was in the original script we have used locally, but I wasn't sure if it's something we will need in our tests, but just wanted to make sure we don't loose it

okay, if you know the author of the original script it would be great to find out why this was commented out.
If this is needed we should uncomment it, and if not there's no point in having this checkDeleteIndices function implemented without executing the actual deletion, right now it's called only to log stuff without doing anything.
Just a suggestion to keep the code clean.

…covery-e2e # Conflicts: # x-pack/test/security_solution_cypress/cypress/cypress.config.ts # x-pack/test/security_solution_cypress/cypress/cypress_ci.config.ts # x-pack/test/security_solution_cypress/cypress/cypress_ci_serverless.config.ts # x-pack/test/security_solution_cypress/cypress/cypress_ci_serverless_qa.config.ts # x-pack/test/security_solution_cypress/cypress/cypress_serverless.config.ts

semd · 2024-05-17T08:05:37Z

x-pack/test/security_solution_cypress/cypress/e2e/ai_assistant/attack_discovery.cy.ts

+      visitWithTimeRange(ATTACK_DISCOVERY_URL);
+    });
+
+    it.each(Object.keys(kibanaDevConfig['xpack.actions.preconfigured']))('%s', (connectorId) => {


If there's no connector configured in the kibana.yml the test crashes in this line with:

> Cannot convert undefined or null to object

Consider defaulting it (?? []) so cypress will tell there's no test to run instead, or maybe check this value and fail more gracefully if it's not defined. wdyt?

kibana-ci · 2024-05-17T15:31:09Z

💔 Build Failed

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #72 / Serverless observability API SLOs fetch historical summary computes the historical summary for a rolling occurrences SLO
[job] [logs] FTR Configs #72 / Serverless observability API SLOs fetch historical summary computes the historical summary for a rolling occurrences SLO
[job] [logs] FTR Configs #55 / SLO API Tests fetch historical summary computes the historical summary for a rolling occurrences SLO
[job] [logs] FTR Configs #55 / SLO API Tests fetch historical summary computes the historical summary for a rolling occurrences SLO

Metrics [docs]

✅ unchanged

History

💔 Build #210675 failed 7c04dd3
💔 Build #210582 failed ad70f8c
💔 Build #210078 failed 6d8f13a
💚 Build #210031 succeeded d31a8b4

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

PhilippeOberti · 2024-05-17T20:43:14Z

@patrykkopycinski looking at the code owners (running node ./code-owners.js 182918 in the kibana-operations repo) I see that a lot of the files in this x-pack/test/security_solution_cypress/cypress/fixtures/assistant/attack_discovery/ folder are owned by multiple teams.
Maybe it would be a good idea to be a bit more granular here?

PhilippeOberti

changes LGTM for the Threat Hunting Investigations team

ashokaditya

Security Solution changes LGTM. I've a couple of nits related to readability and TS but nothing that blocks this PR.

ashokaditya · 2024-05-23T08:10:07Z

x-pack/plugins/security_solution/scripts/run_cypress/utils.ts

+  filePath: string
+): {
+  ftrConfig: SecuritySolutionDescribeBlockFtrConfig;
+  devConfig: boolean;


nit: Consider naming this to something that answers a yes/no. devConfig seems like a collection/object but it is a boolean. isDevRun/isDevMode or something like that?

ashokaditya · 2024-05-23T08:13:26Z

x-pack/plugins/security_solution/scripts/run_cypress/utils.ts

@@ -90,25 +95,39 @@ export const parseTestFileConfig = (filePath: string): SecuritySolutionDescribeB
      'key.name',
      'ftrConfig',
    ]);
+    // @ts-expect-error
+    const devConfig = _.find(callExpressionProperties?.value?.properties, [


nit: Maybe rename this local devConfig to avoid confusion and variable re-declaration TS error?

Suggested change

const devConfig = _.find(callExpressionProperties?.value?.properties, [

const _devConfig = _.find(callExpressionProperties?.value?.properties, [

ashokaditya · 2024-05-23T08:28:24Z

x-pack/plugins/security_solution/scripts/run_cypress/parallel_serverless.ts

@@ -437,7 +437,7 @@ ${JSON.stringify(cypressConfigFile, null, 2)}

              const productTypes = isOpen
                ? getProductTypes(tier, endpointAddon, cloudAddon)
-                : (parseTestFileConfig(filePath).productTypes as ProductType[]);
+                : (parseTestFileConfig(filePath).ftrConfig?.productTypes as ProductType[]);


Looks like ftrConfig is always present. The productTypes could be undefined which is why it is type casted here.

Suggested change

: (parseTestFileConfig(filePath).ftrConfig?.productTypes as ProductType[]);

: (parseTestFileConfig(filePath).ftrConfig.productTypes as ProductType[]);

patrykkopycinski added 2 commits May 8, 2024 12:41

Add e2e for Attack discovery

957e0b0

Merge branch 'main' of github.com:elastic/kibana into feat/attack-dis…

5b681fd

…covery-e2e # Conflicts: # yarn.lock

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

0110a1a

…fix'

patrykkopycinski added 2 commits May 8, 2024 13:43

fix

1e64c41

Merge branch 'feat/attack-discovery-e2e' of github.com:patrykkopycins…

a9d8dd9

…ki/kibana into feat/attack-discovery-e2e

stephmilovic mentioned this pull request May 9, 2024

Security solution sample data #182979

Draft

patrykkopycinski added 2 commits May 10, 2024 11:08

fix

049e172

Merge branch 'main' into feat/attack-discovery-e2e

dab9460

patrykkopycinski added 2 commits May 10, 2024 13:00

fix

758582b

Merge branch 'feat/attack-discovery-e2e' of github.com:patrykkopycins…

70fb039

…ki/kibana into feat/attack-discovery-e2e

Merge branch 'main' into feat/attack-discovery-e2e

2838945

patrykkopycinski added 2 commits May 14, 2024 23:44

fix

7548256

Merge branch 'main' into feat/attack-discovery-e2e

814b532

Merge branch 'main' into feat/attack-discovery-e2e

d31a8b4

retries

c0c821a

patrykkopycinski marked this pull request as ready for review May 15, 2024 09:29

patrykkopycinski requested review from a team, MadameSheema, oatkiller, maximpn and banderror as code owners May 15, 2024 09:29

patrykkopycinski requested review from a team as code owners May 15, 2024 09:29

patrykkopycinski requested review from pzl, paul-tavares and nkhristinin May 15, 2024 09:29

cleanup

6d8f13a

pzl requested review from ashokaditya and removed request for pzl May 15, 2024 11:30

jbudz approved these changes May 15, 2024

View reviewed changes

paul-tavares approved these changes May 15, 2024

View reviewed changes

semd reviewed May 16, 2024

View reviewed changes

semd reviewed May 17, 2024

View reviewed changes

patrykkopycinski added 2 commits May 17, 2024 11:24

Merge branch 'main' into feat/attack-discovery-e2e

7c04dd3

Merge branch 'main' into feat/attack-discovery-e2e

1186809

PhilippeOberti approved these changes May 17, 2024

View reviewed changes

ashokaditya approved these changes May 23, 2024

View reviewed changes

nkhristinin approved these changes May 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add e2e for Attack discovery #182918

Add e2e for Attack discovery #182918

patrykkopycinski commented May 8, 2024 •

edited

patrykkopycinski commented May 8, 2024

patrykkopycinski commented May 8, 2024

patrykkopycinski commented May 8, 2024

patrykkopycinski commented May 10, 2024

patrykkopycinski commented May 10, 2024

patrykkopycinski commented May 14, 2024

patrykkopycinski commented May 14, 2024

patrykkopycinski commented May 15, 2024

patrykkopycinski commented May 15, 2024

paul-tavares left a comment

semd May 16, 2024 •

edited

patrykkopycinski May 16, 2024

semd May 17, 2024

semd May 17, 2024

kibana-ci commented May 17, 2024 •

edited

PhilippeOberti commented May 17, 2024

PhilippeOberti left a comment

ashokaditya left a comment

ashokaditya May 23, 2024

ashokaditya May 23, 2024

ashokaditya May 23, 2024

	const devConfig = _.find(callExpressionProperties?.value?.properties, [
	const _devConfig = _.find(callExpressionProperties?.value?.properties, [

	: (parseTestFileConfig(filePath).ftrConfig?.productTypes as ProductType[]);
	: (parseTestFileConfig(filePath).ftrConfig.productTypes as ProductType[]);

Add e2e for Attack discovery #182918

Are you sure you want to change the base?

Add e2e for Attack discovery #182918

Conversation

patrykkopycinski commented May 8, 2024 • edited

Summary

patrykkopycinski commented May 8, 2024

patrykkopycinski commented May 8, 2024

patrykkopycinski commented May 8, 2024

patrykkopycinski commented May 10, 2024

patrykkopycinski commented May 10, 2024

patrykkopycinski commented May 14, 2024

patrykkopycinski commented May 14, 2024

patrykkopycinski commented May 15, 2024

patrykkopycinski commented May 15, 2024

paul-tavares left a comment

Choose a reason for hiding this comment

semd May 16, 2024 • edited

Choose a reason for hiding this comment

patrykkopycinski May 16, 2024

Choose a reason for hiding this comment

semd May 17, 2024

Choose a reason for hiding this comment

semd May 17, 2024

Choose a reason for hiding this comment

kibana-ci commented May 17, 2024 • edited

💔 Build Failed

Failed CI Steps

Test Failures

Metrics [docs]

History

PhilippeOberti commented May 17, 2024

PhilippeOberti left a comment

Choose a reason for hiding this comment

ashokaditya left a comment

Choose a reason for hiding this comment

ashokaditya May 23, 2024

Choose a reason for hiding this comment

ashokaditya May 23, 2024

Choose a reason for hiding this comment

ashokaditya May 23, 2024

Choose a reason for hiding this comment

patrykkopycinski commented May 8, 2024 •

edited

semd May 16, 2024 •

edited

kibana-ci commented May 17, 2024 •

edited