[Security Solution] Adds basic test for rule data view #136822

MadameSheema · 2022-07-21T11:47:18Z

Summary

In this PR we are adding a basic rule data view test in order to use it as a template/inspiration to extend the current coverage.

The test and the code are similar to the current one for creating another type of rule. These are the modifications introduced:

To the CustomRule structure we have added dataView as optional and we've made index optional instead of mandatory, since a rule might have one thing or the other. Also, some adjustments to the current code have been done in order to deal with this new structure.
On the method that fills the define step when creating a new rule, we check if the rule we are passing has a dataview, if so, we click on the data view selector and we fill that value. If not, we proceed as usual since our tests does not fill the index patterns input and uses the default ones.
On the test that creates a rule using a data view, we don't call cleanKibana method on the before hook, instead we call esArchiverReseKibana on the before each one. This is because we are creating a data view we'll use after and cleanKibana does not delete all the data views created, esArchiverReseKibana does. We don't use esArchiverReseKibana in all the tests because is a time-consuming method and we don't need to perform an exhaustive cleaning in all the other tests.
The test is the same as the other Custom query rule test but: we are creating a rule with a data view and we check that the data view displayed on the rule details is the expected instead of checking for the index patterns.

dhurley14

LGTM and thanks a bunch!

elasticmachine · 2022-07-22T05:44:38Z

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

MadameSheema · 2022-07-22T05:45:23Z

@elasticmachine merge upstream

MadameSheema · 2022-07-22T07:33:23Z

@elasticmachine merge upstream

banderror

Just a few suggestions 🙂

...ns/security_solution/cypress/integration/detection_rules/custom_query_rule_data_view.spec.ts

x-pack/plugins/security_solution/cypress/objects/rule.ts

…-fix'

…na into cypress/data-view-rule

MadameSheema · 2022-07-25T05:22:45Z

Thanks for the review @banderror!! The PR is ready to be reviewed again 😊

x-pack/plugins/security_solution/cypress/tasks/common.ts

...ns/security_solution/cypress/integration/detection_rules/custom_query_rule_data_view.spec.ts

x-pack/plugins/security_solution/cypress/integration/detection_rules/custom_query_rule.spec.ts

banderror · 2022-07-25T08:59:26Z

x-pack/plugins/security_solution/cypress/tasks/api_calls/rules.ts

@@ -41,7 +45,7 @@ export const createCustomRule = (rule: CustomRule, ruleId = 'rule_testing', inte
      severity: rule.severity.toLocaleLowerCase(),
      type: 'query',
      from: 'now-50000h',
-      index: rule.index,
+      index: rule.dataSource.type === 'indexPatterns' ? rule.dataSource.index : '',


Hmm, why default value for the index is an empty string and why don't we support posting a dataView if it's in the test data?

index: rule.dataSource.type === 'indexPatterns' ? rule.dataSource.index : undefined, dataView: rule.dataSource.type === 'dataView' ? rule.dataSource.dataView : undefined,

I would follow up this in a different/follow up PR, adding the capability of creating also a rule with data view at first glance is more complex, since first, we need to get the data view id.

banderror · 2022-07-25T09:00:31Z

x-pack/plugins/security_solution/cypress/tasks/api_calls/rules.ts

-    headers: { 'kbn-xsrf': 'cypress-creds' },
-  });
+export const createEventCorrelationRule = (rule: CustomRule, ruleId = 'rule_testing') => {
+  if (rule.dataSource.type === 'indexPatterns') {


Instead of having this condition, let's support both index patterns and data views here as in createCustomRule.

I would prefer to do this on a different PR, since it could be a bit complex and at least we will have a test for data views already in main.

That sounds good to me, thanks @MadameSheema 👍

banderror · 2022-07-25T09:00:37Z

x-pack/plugins/security_solution/cypress/tasks/api_calls/rules.ts

-    failOnStatusCode: false,
-  });
+export const createCustomIndicatorRule = (rule: ThreatIndicatorRule, ruleId = 'rule_testing') => {
+  if (rule.dataSource.type === 'indexPatterns') {


Instead of having this condition, let's support both index patterns and data views here as in createCustomRule.

Same as above.

banderror · 2022-07-25T09:00:46Z

x-pack/plugins/security_solution/cypress/tasks/api_calls/rules.ts

-    failOnStatusCode: false,
-  });
+) => {
+  if (rule.dataSource.type === 'indexPatterns') {


Instead of having this condition, let's support both index patterns and data views here as in createCustomRule.

Same as above.

kibana-ci · 2022-07-25T17:27:34Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💚 Build #60137 succeeded 74d2bc5
💔 Build #60132 failed ddd0a00
💔 Build #60121 failed 3feb50f
💛 Build #59770 was flaky c5584d5
💔 Build #59756 failed d3864ec
💛 Build #59504 was flaky 48ff966

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

cc @MadameSheema

banderror

Thank you for addressing my annoying comments @MadameSheema ❤️
🚀

banderror · 2022-07-25T17:39:15Z

x-pack/plugins/security_solution/cypress/tasks/common.ts

  cy.request({
    method: 'POST',
    url: `/api/index_patterns/index_pattern`,
    body: {
      index_pattern: {
        fieldAttrs: '{}',
-        title: indexPattern,
+        title: dataSource,


Nit: dataViewTitle or just leave it as it was before - indexPattern?

MadameSheema · 2022-07-26T05:15:57Z

@banderror your comments were not annoying! Lots of thanks for helping to deliver the best solution :)

MadameSheema added 3 commits July 21, 2022 13:46

adds basic test for rule data view

6f72eca

fixes

5f33f1f

refactor

48ff966

MadameSheema marked this pull request as ready for review July 21, 2022 14:49

MadameSheema requested review from a team as code owners July 21, 2022 14:49

MadameSheema requested a review from banderror July 21, 2022 14:49

MadameSheema self-assigned this Jul 21, 2022

dhurley14 approved these changes Jul 21, 2022

View reviewed changes

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. Team:Security Solution Platform Security Solution Platform Team v8.4.0 labels Jul 22, 2022

Merge branch 'main' into cypress/data-view-rule

d3864ec

Merge branch 'main' into cypress/data-view-rule

c5584d5

logeekal approved these changes Jul 22, 2022

View reviewed changes

semd approved these changes Jul 22, 2022

View reviewed changes

MadameSheema enabled auto-merge (squash) July 22, 2022 14:13

banderror reviewed Jul 22, 2022

View reviewed changes

...ns/security_solution/cypress/integration/detection_rules/custom_query_rule_data_view.spec.ts Show resolved Hide resolved

x-pack/plugins/security_solution/cypress/objects/rule.ts Outdated Show resolved Hide resolved

MadameSheema and others added 5 commits July 24, 2022 08:47

Merge branch 'main' into cypress/data-view-rule

8780c96

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

ff51f10

…-fix'

please review

4a27232

Merge branch 'cypress/data-view-rule' of github.com:MadameSheema/kiba…

3feb50f

…na into cypress/data-view-rule

please review

ddd0a00

MadameSheema requested a review from a team as a code owner July 24, 2022 19:47

fixes

74d2bc5

banderror reviewed Jul 25, 2022

View reviewed changes

please latest review

8699790

dhurley14 approved these changes Jul 25, 2022

View reviewed changes

banderror approved these changes Jul 25, 2022

View reviewed changes

MadameSheema merged commit 691e21e into elastic:main Jul 25, 2022

kibanamachine added the backport:skip This commit does not require backporting label Jul 25, 2022

banderror mentioned this pull request Jul 25, 2022

[Security Solution] [Platform] Adds state to remember what was in data view or index pattern selection when switching between the two #136448

Merged

MadameSheema deleted the cypress/data-view-rule branch August 31, 2023 09:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Adds basic test for rule data view #136822

[Security Solution] Adds basic test for rule data view #136822

MadameSheema commented Jul 21, 2022 •

edited

Loading

dhurley14 left a comment

elasticmachine commented Jul 22, 2022

MadameSheema commented Jul 22, 2022

MadameSheema commented Jul 22, 2022

banderror left a comment

MadameSheema commented Jul 25, 2022

banderror Jul 25, 2022

MadameSheema Jul 25, 2022

banderror Jul 25, 2022

MadameSheema Jul 25, 2022

banderror Jul 25, 2022

banderror Jul 25, 2022

MadameSheema Jul 25, 2022

banderror Jul 25, 2022

MadameSheema Jul 25, 2022

kibana-ci commented Jul 25, 2022

banderror left a comment

banderror Jul 25, 2022

MadameSheema commented Jul 26, 2022

[Security Solution] Adds basic test for rule data view #136822

[Security Solution] Adds basic test for rule data view #136822

Conversation

MadameSheema commented Jul 21, 2022 • edited Loading

Summary

dhurley14 left a comment

Choose a reason for hiding this comment

elasticmachine commented Jul 22, 2022

MadameSheema commented Jul 22, 2022

MadameSheema commented Jul 22, 2022

banderror left a comment

Choose a reason for hiding this comment

MadameSheema commented Jul 25, 2022

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

kibana-ci commented Jul 25, 2022

💚 Build Succeeded

Metrics [docs]

History

banderror left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MadameSheema commented Jul 26, 2022

MadameSheema commented Jul 21, 2022 •

edited

Loading