[Detection Engine][MKI] Audit alerts FTRs in prep for MKI #179211

yctercero · 2024-03-22T02:50:07Z

Summary

Begins work on #169185 and #151877 .

Related to #151877 this PR:

Moves FTR tests under the /alerts folder that do not require Platinum license into the basics folder. Tests in /alerts/trial_license_complete_tier folder should now relate to functionality that requires the higher license tier.
Rearranged some of the folder structure so that it was clear what the intent of the tests is
Makes note of any issues in tickets that we will need to follow up on

Related to #169185 this PR:

Ensures that tests are properly tagged for ESS & serverless
Ensures none of the tests that are critical contain the @skipInQA tag

Issues to follow up on:

Old structure

New structure

elasticmachine · 2024-03-22T02:50:10Z

Pinging @elastic/security-detection-engine (Team:Detection Engine)

…_mki

yctercero · 2024-03-22T02:55:08Z

..._response/detection_engine/alerts/basic_license_essentials_tier/alert_status/alert_status.ts

@@ -142,41 +136,13 @@ export default ({ getService }: FtrProviderContext) => {
          await waitForAlertsToBePresent(supertest, log, 10, [id]);
          const alertsOpen = await getAlertsByIds(supertest, log, [id]);
          expect(alertsOpen.hits.hits.length).equal(10);
-        });
-
-        it('should be have set the alerts in an open state initially', async () => {


Duplicate. This was not testing anything that the test above it was doing.

Great catch!

yctercero · 2024-03-22T02:56:52Z

..._response/detection_engine/alerts/basic_license_essentials_tier/alert_status/alert_status.ts

          const everyAlertOpen = alertsOpen.hits.hits.every(
            (hit) => hit._source?.[ALERT_WORKFLOW_STATUS] === 'open'
          );
          expect(everyAlertOpen).to.eql(true);
        });

-        it('should be able to close alerts while logged in and populate workflow_user', async () => {


Moved to ESS specific folder due to the user we are using to test. Will need to match the test for serverless (see issues created and linked in description).

yctercero · 2024-03-22T02:58:26Z

..._response/detection_engine/alerts/basic_license_essentials_tier/alert_status/alert_status.ts

@@ -262,75 +214,6 @@ export default ({ getService }: FtrProviderContext) => {
          );
          expect(everyAlertWorkflowStatusUpdatedAtExists).to.eql(true);
        });
-        // This fails and should be investigated or removed if it no longer applies
-        it.skip('should be able to close alerts with t1 analyst user', async () => {


Was skipped because it was failing. Moved to ESS specific file and updated roles used for testing. Now unskipped. Need matching tests for prebuilt serverless roles (see issues I created to track in description).

yctercero · 2024-03-22T03:00:08Z

...ections_response/detection_engine/alerts/basic_license_essentials_tier/alert_status/index.ts

+import { FtrProviderContext } from '../../../../../../ftr_provider_context';
+
+export default function ({ loadTestFile }: FtrProviderContext) {
+  describe('Alert status APIs', function () {


Changed language for test names from open_close_alert to change_status as we have more statuses than just open/close and added a test for the acknowledged status.

That's a nice improvement, open_close_alert has never made much sense to me 👍

yctercero · 2024-03-22T03:04:02Z

...ions_response/detection_engine/alerts/trial_license_complete_tier/document_level_security.ts

@@ -74,7 +74,7 @@ export default ({ getService }: FtrProviderContext) => {
  const esArchiver = getService('esArchiver');
  const security = getService('security');

-  describe('find alert with/without doc level security', () => {


This test has likely not been picked up as it did not have a tag. I think this test should apply to serverless as well.

@paulewing can you confirm my thinking that this is just a Platinum feature? If so we should test that it is blocked in lower tier.

…_mki

rylnd

The new file naming/locations make much more sense; this is a great improvement.

I had one nit about keeping a no-op test skipped, and there were a few outdated comments that should be deleted. I'm gonna approve anyway.

Thanks!

rylnd · 2024-03-25T19:35:48Z

..._response/detection_engine/alerts/basic_license_essentials_tier/alert_status/alert_status.ts

@@ -142,41 +136,13 @@ export default ({ getService }: FtrProviderContext) => {
          await waitForAlertsToBePresent(supertest, log, 10, [id]);
          const alertsOpen = await getAlertsByIds(supertest, log, [id]);
          expect(alertsOpen.hits.hits.length).equal(10);
-        });
-
-        it('should be have set the alerts in an open state initially', async () => {


Great catch!

rylnd · 2024-03-25T19:38:47Z

...ections_response/detection_engine/alerts/basic_license_essentials_tier/alert_status/index.ts

+import { FtrProviderContext } from '../../../../../../ftr_provider_context';
+
+export default function ({ loadTestFile }: FtrProviderContext) {
+  describe('Alert status APIs', function () {


That's a nice improvement, open_close_alert has never made much sense to me 👍

rylnd · 2024-03-25T19:39:16Z

...engine/alerts/basic_license_essentials_tier/ess_specific_index_logic/alerts_compatibility.ts


 export default ({ getService }: FtrProviderContext) => {
  const esArchiver = getService('esArchiver');
  const log = getService('log');
  const supertest = getService('supertest');
  const es = getService('es');

-  describe('@ess Alerts Compatibility', function () {
+  describe('@ess alerts compatability with legacy siem signals index', function () {


nit:

Suggested change

describe('@ess alerts compatability with legacy siem signals index', function () {

describe('@ess alerts compatibility with legacy siem signals index', function () {

rylnd · 2024-03-25T19:39:38Z

...tection_engine/alerts/basic_license_essentials_tier/ess_specific_index_logic/create_index.ts

@@ -38,7 +38,7 @@ export default ({ getService }: FtrProviderContext) => {
          await esArchiver.unload('x-pack/test/functional/es_archives/signals/index_alias_clash');
        });

-        // This fails and should be investigated or removed if it no longer applies
+        // Skipped: see https://github.com/elastic/kibana/issues/179208


rylnd · 2024-03-25T21:09:16Z

...asic_license_essentials_tier/ess_specific_index_logic/migrations/delete_alerts_migrations.ts

@@ -83,6 +84,10 @@ export default ({ getService }: FtrProviderContext): void => {

    afterEach(async () => {
      await esArchiver.unload('x-pack/test/functional/es_archives/signals/outdated_signals_index');
+      await deleteMigrations({


rylnd · 2024-03-25T21:22:17Z

...ic_license_essentials_tier/ess_specific_index_logic/migrations/finalize_alerts_migrations.ts

@@ -190,7 +190,7 @@ export default ({ getService }: FtrProviderContext): void => {
    });

    // This fails and should be investigated or removed if it no longer applies
-    it.skip('deletes the underlying migration task', async () => {


This looked familiar: I wrote this test, and it's been skipped since it was originally introduced. I don't recall exactly why it was skipped, and it doesn't look like I left any clues for us 😢 .

However, I was surprised to see the build was green after unskipping this. It looks to be a false positive, as the actual assertions of this test are commented out below (and most obviously, no getMigration helper currently exists).

I would say: just keep this skipped for now, and (maybe) create a ticket to unskip it in the future.

Tracking #179593

rylnd · 2024-03-25T21:23:50Z

...es/detections_response/detection_engine/alerts/basic_license_essentials_tier/query_alerts.ts

@@ -96,12 +104,16 @@ export default ({ getService }: FtrProviderContext) => {
    describe('find_alerts_route', () => {
      describe('validation checks', () => {
        // This fails and should be investigated or removed if it no longer applies


Suggested change

// This fails and should be investigated or removed if it no longer applies

…_mki

kibana-ci · 2024-03-28T16:22:14Z

💚 Build Succeeded

Buildkite Build
Commit: 1d0e4f1

Metrics [docs]

✅ unchanged

History

💛 Build #199889 was flaky e1ae686
💚 Build #198869 succeeded 26d90a1

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

cc @yctercero

rearranged alerts tests to prepare for mki

a40d708

yctercero added release_note:skip Skip the PR/issue when compiling release notes Team:Detection Engine Security Solution Detection Engine Area v8.14.0 labels Mar 22, 2024

yctercero self-assigned this Mar 22, 2024

yctercero requested review from a team as code owners March 22, 2024 02:50

yctercero requested a review from rylnd March 22, 2024 02:50

yctercero added 2 commits March 21, 2024 19:51

updates with link to track skipped test

fac0f60

Merge branch 'main' of github.com:elastic/kibana into ftr_alerts_prep…

5cbdd2c

…_mki

yctercero commented Mar 22, 2024

View reviewed changes

yctercero added 2 commits March 21, 2024 20:04

some cleanup

26d90a1

Merge branch 'main' of github.com:elastic/kibana into ftr_alerts_prep…

a479150

…_mki

rylnd approved these changes Mar 25, 2024

View reviewed changes

yctercero mentioned this pull request Mar 28, 2024

[Detection Engine][FTR] Unskip alerts migration test #179593

Open

yctercero added 3 commits March 28, 2024 00:39

adding pr feedback

2b42266

Merge branch 'main' of github.com:elastic/kibana into ftr_alerts_prep…

e1ae686

…_mki

uncomment suite

1d0e4f1

yctercero merged commit 7698d9c into elastic:main Mar 28, 2024
16 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Mar 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Detection Engine][MKI] Audit alerts FTRs in prep for MKI #179211

[Detection Engine][MKI] Audit alerts FTRs in prep for MKI #179211

yctercero commented Mar 22, 2024 •

edited

Loading

elasticmachine commented Mar 22, 2024

yctercero Mar 22, 2024

rylnd Mar 25, 2024

yctercero Mar 22, 2024

yctercero Mar 22, 2024

yctercero Mar 22, 2024

rylnd Mar 25, 2024

yctercero Mar 22, 2024

rylnd left a comment

rylnd Mar 25, 2024

rylnd Mar 25, 2024

rylnd Mar 25, 2024

rylnd Mar 25, 2024

rylnd Mar 25, 2024

rylnd Mar 25, 2024

yctercero Mar 28, 2024

rylnd Mar 25, 2024

kibana-ci commented Mar 28, 2024

	describe('@ess alerts compatability with legacy siem signals index', function () {
	describe('@ess alerts compatibility with legacy siem signals index', function () {

[Detection Engine][MKI] Audit alerts FTRs in prep for MKI #179211

[Detection Engine][MKI] Audit alerts FTRs in prep for MKI #179211

Conversation

yctercero commented Mar 22, 2024 • edited Loading

Summary

Old structure

New structure

elasticmachine commented Mar 22, 2024

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

rylnd 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

kibana-ci commented Mar 28, 2024

💚 Build Succeeded

Metrics [docs]

History

yctercero commented Mar 22, 2024 •

edited

Loading