[Security Solution] Unskip remaining Cypress tests from RAC rules migration #122661

madirey · 2022-01-11T14:42:07Z

Summary

This PR unskips the remaining timeline-related Cypress tests that were skipped as part of the RAC rules migration.

Indicator match: Fixed by subsequent PRs
Fields browser: A field was removed, so changed field count from 5 to 4.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

This reverts commit 05d549e.

kibana-ci · 2022-01-11T19:34:44Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💔 Build #16714 failed 4848c3d

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

elasticmachine · 2022-01-11T22:44:07Z

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

spong

LGTM! Thanks for addressing these skips and the added details in the description @madirey! 🙂

rylnd

💯 Thank you for closing the loop, here! It's nice to have these back. 🙌

…ration (elastic#122661) * Unskip indicator match timeline test * Unskip fields_browser tests * Enable alert_summary tests * add cti feed enrichment * Fix accessibility text in indicator match cypress test * Adjust fields_browser test to account for removed field * Correct indicator_match row renderer text in cypress test * Revert "Enable alert_summary tests" This reverts commit 05d549e. (cherry picked from commit 59be3ca)

kibanamachine · 2022-01-13T13:36:47Z

💚 All backports created successfully

Status	Branch	Result
✅	8.0

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

Questions ?

Please refer to the Backport tool documentation

…ration (#122661) (#122938) * Unskip indicator match timeline test * Unskip fields_browser tests * Enable alert_summary tests * add cti feed enrichment * Fix accessibility text in indicator match cypress test * Adjust fields_browser test to account for removed field * Correct indicator_match row renderer text in cypress test * Revert "Enable alert_summary tests" This reverts commit 05d549e. (cherry picked from commit 59be3ca) Co-authored-by: Madison Caldwell <madison.rey.caldwell@gmail.com>

MadameSheema · 2022-01-13T15:35:00Z

x-pack/plugins/security_solution/cypress/integration/timelines/fields_browser.spec.ts

      const filterInput = 'host.geo.c';

      filterFieldsBrowser(filterInput);

-      cy.get(FIELDS_BROWSER_SELECTED_CATEGORY_COUNT).should('have.text', '5');
+      cy.get(FIELDS_BROWSER_SELECTED_CATEGORY_COUNT).should('have.text', '4');


@madirey this change is not correct, we are still having 5 fields, this test was catching a legitimate bug.

The problem here is that when we are opening the timeline the new alert's index is not selected by default when it should, that index is the one that contains the missing field.

We should fix the issue and change the number of the test to 5 again, I'll open a ticket with the bug and assign to the alerts team since I'm assuming that as the issue was introduced when the rule registry changes were merged is the alerts team the one that should take care of it. Please note that this change impacts one of new explore team functionalities.

ping @spong @rylnd @stephmilovic

@MadameSheema I only see 4 fields in the ECS field mapping. Is there an additional non-ECS one?

https://www.elastic.co/guide/en/ecs/current/ecs-geo.html

As you can see, the test is looking at geo fields that start with c. there are 5

You are only looking at ecs mapping for Data View before it has alerts mappings because alerts index has not been initialized yet. Alerts index has extra field, if i recall correctly geo.continent_code

@stephmilovic @MadameSheema The new alerts mapping doesn't contain the geo.continent_code field so I believe it will only show up on upgrades (since the field exists in the legacy .siem-signals index, which is aliased). New installs will have only 4 fields. Our team will be updating the ECS mappings soon, which should result in that field being added. We should be able to update the test at that point.

@madirey

## Summary New ECS FieldMap was generated in #123012, however since it only contained changes to `Rule Registry` code the `Security Solution` Cypress tests were not run, and thus did not catch this field change. See #122661 (comment) for details. Confirmed w/ @madirey that expected value is indeed `5` now that `host.geo.continent_code` has been [added](https://github.com/elastic/kibana/pull/123012/files#diff-a1647ccb73ef26c8c8b6aefd87084504b146af72fcb088ccacad93fcaad15b69R1524-R1528). Some failing PR's from `main`: #123357 #121644 #123352 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

@madirey

…123429) ## Summary New ECS FieldMap was generated in elastic#123012, however since it only contained changes to `Rule Registry` code the `Security Solution` Cypress tests were not run, and thus did not catch this field change. See elastic#122661 (comment) for details. Confirmed w/ @madirey that expected value is indeed `5` now that `host.geo.continent_code` has been [added](https://github.com/elastic/kibana/pull/123012/files#diff-a1647ccb73ef26c8c8b6aefd87084504b146af72fcb088ccacad93fcaad15b69R1524-R1528). Some failing PR's from `main`: elastic#123357 elastic#121644 elastic#123352 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (cherry picked from commit d6917fc)

@madirey

…#123433) ## Summary New ECS FieldMap was generated in #123012, however since it only contained changes to `Rule Registry` code the `Security Solution` Cypress tests were not run, and thus did not catch this field change. See #122661 (comment) for details. Confirmed w/ @madirey that expected value is indeed `5` now that `host.geo.continent_code` has been [added](https://github.com/elastic/kibana/pull/123012/files#diff-a1647ccb73ef26c8c8b6aefd87084504b146af72fcb088ccacad93fcaad15b69R1524-R1528). Some failing PR's from `main`: #123357 #121644 #123352 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (cherry picked from commit d6917fc) Co-authored-by: Garrett Spong <spong@users.noreply.github.com>

@madirey

…123429) ## Summary New ECS FieldMap was generated in elastic#123012, however since it only contained changes to `Rule Registry` code the `Security Solution` Cypress tests were not run, and thus did not catch this field change. See elastic#122661 (comment) for details. Confirmed w/ @madirey that expected value is indeed `5` now that `host.geo.continent_code` has been [added](https://github.com/elastic/kibana/pull/123012/files#diff-a1647ccb73ef26c8c8b6aefd87084504b146af72fcb088ccacad93fcaad15b69R1524-R1528). Some failing PR's from `main`: elastic#123357 elastic#121644 elastic#123352 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

madirey added 7 commits January 11, 2022 09:35

Unskip indicator match timeline test

7bef661

Unskip fields_browser tests

65978da

Enable alert_summary tests

05d549e

add cti feed enrichment

20b6ed6

Fix accessibility text in indicator match cypress test

0f1013e

Adjust fields_browser test to account for removed field

3cd4944

Correct indicator_match row renderer text in cypress test

e7e67cb

madirey added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Detection Alerts Security Solution Detection Alerts Feature v8.1.0 labels Jan 11, 2022

madirey requested a review from a team as a code owner January 11, 2022 14:42

This was referenced Jan 11, 2022

[Security Solution] Unskip remaining Cypress tests from RAC rules migration #119997

Closed

Unskip tests for RAC rule migration #116460

Closed

madirey added 2 commits January 11, 2022 10:18

Merge branch 'main' of github.com:elastic/kibana into unskip-rac-cypress

4848c3d

Revert "Enable alert_summary tests"

b3847fb

This reverts commit 05d549e.

madirey added the auto-backport Deprecated - use backport:version if exact versions are needed label Jan 11, 2022

madirey requested a review from MadameSheema January 11, 2022 19:54

madirey self-assigned this Jan 11, 2022

madirey added the Team:Detections and Resp Security Detection Response Team label Jan 11, 2022

spong approved these changes Jan 12, 2022

View reviewed changes

rylnd approved these changes Jan 12, 2022

View reviewed changes

madirey merged commit 59be3ca into elastic:main Jan 13, 2022

madirey deleted the unskip-rac-cypress branch January 13, 2022 13:32

kibanamachine mentioned this pull request Jan 13, 2022

[8.0] [Security Solution] Unskip remaining Cypress tests from RAC rules migration (#122661) #122938

Merged

MadameSheema reviewed Jan 13, 2022

View reviewed changes

This was referenced Jan 19, 2022

[Security Solution][Detections] Rule execution logging overhaul #121644

Merged

[Security Solution] Fixes broken cypress test after 8.0 ECS update to Rule Registry #123429

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Unskip remaining Cypress tests from RAC rules migration #122661

[Security Solution] Unskip remaining Cypress tests from RAC rules migration #122661

madirey commented Jan 11, 2022 •

edited

Loading

kibana-ci commented Jan 11, 2022

elasticmachine commented Jan 11, 2022

spong left a comment

rylnd left a comment

kibanamachine commented Jan 13, 2022

MadameSheema Jan 13, 2022

madirey Jan 13, 2022

stephmilovic Jan 13, 2022

madirey Jan 13, 2022

[Security Solution] Unskip remaining Cypress tests from RAC rules migration #122661

[Security Solution] Unskip remaining Cypress tests from RAC rules migration #122661

Conversation

madirey commented Jan 11, 2022 • edited Loading

Summary

Checklist

For maintainers

kibana-ci commented Jan 11, 2022

💚 Build Succeeded

Metrics [docs]

History

elasticmachine commented Jan 11, 2022

spong left a comment

Choose a reason for hiding this comment

rylnd left a comment

Choose a reason for hiding this comment

kibanamachine commented Jan 13, 2022

💚 All backports created successfully

Questions ?

MadameSheema Jan 13, 2022

Choose a reason for hiding this comment

madirey Jan 13, 2022

Choose a reason for hiding this comment

stephmilovic Jan 13, 2022

Choose a reason for hiding this comment

madirey Jan 13, 2022

Choose a reason for hiding this comment

madirey commented Jan 11, 2022 •

edited

Loading