[9.0] Alerts created within a Maintenance Windows trigger actions after the MW expires (#219797) (#220935)

ersin-erdal · web-flow · commit 01973c0886e4 · 2025-05-20T09:13:00.000-04:00
# Backport This will backport the following commits from `main` to `9.0`: - [Alerts created within a Maintenance Windows trigger actions after the MW expires (#219797)](#219797)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)
diff --git a/x-pack/platform/plugins/shared/alerting/server/alerts_client/legacy_alerts_client.test.ts b/x-pack/platform/plugins/shared/alerting/server/alerts_client/legacy_alerts_client.test.ts
@@ -371,7 +371,7 @@ describe('Legacy Alerts Client', () => {
     });
   });
 
-  test('processAlerts() should set maintenance windows IDs on new alerts', async () => {
+  test('processAlerts() should set maintenance windows IDs on new alerts and remove the expired maintenance windows from the active and recovered alerts', async () => {
     maintenanceWindowsService.getMaintenanceWindows.mockReturnValue({
       maintenanceWindows: [
         {
@@ -386,37 +386,57 @@ describe('Legacy Alerts Client', () => {
           eventStartTime: new Date().toISOString(),
           eventEndTime: new Date().toISOString(),
           status: MaintenanceWindowStatus.Running,
-          id: 'test-id2',
+          id: 'test-id5',
         },
       ],
-      maintenanceWindowsWithoutScopedQueryIds: ['test-id1', 'test-id2'],
+      maintenanceWindowsWithoutScopedQueryIds: ['test-id1', 'test-id5'],
     });
+
+    const activeAlert = {
+      state: {},
+      meta: {
+        uuid: 'bar',
+        maintenanceWindowIds: ['test-id1', 'test-id2'],
+      },
+    };
+
+    const recoveredAlert = {
+      state: {},
+      meta: {
+        uuid: 'ghi',
+        maintenanceWindowIds: ['test-id1', `test-id3`],
+      },
+    };
+
     (processAlerts as jest.Mock).mockReturnValue({
       newAlerts: {
         '1': new Alert<AlertInstanceContext, AlertInstanceContext>('1', testAlert1),
       },
       activeAlerts: {
-        '2': new Alert<AlertInstanceContext, AlertInstanceContext>('2', testAlert2),
+        '2': new Alert<AlertInstanceContext, AlertInstanceContext>('2', activeAlert),
+      },
+      recoveredAlerts: {
+        '3': new Alert<AlertInstanceContext, AlertInstanceContext>('3', recoveredAlert),
       },
       currentRecoveredAlerts: {},
-      recoveredAlerts: {},
     });
+
     (trimRecoveredAlerts as jest.Mock).mockReturnValue({
       trimmedAlertsRecovered: {},
       earlyRecoveredAlerts: {},
     });
+
     (getAlertsForNotification as jest.Mock).mockReturnValue({
       newAlerts: {
         '1': new Alert<AlertInstanceContext, AlertInstanceContext>('1', testAlert1),
       },
       activeAlerts: {
-        '2': new Alert<AlertInstanceContext, AlertInstanceContext>('2', testAlert2),
+        '2': new Alert<AlertInstanceContext, AlertInstanceContext>('2', activeAlert),
       },
-      currentActiveAlerts: {
-        '2': new Alert<AlertInstanceContext, AlertInstanceContext>('2', testAlert2),
+      recoveredAlerts: {
+        '3': new Alert<AlertInstanceContext, AlertInstanceContext>('3', recoveredAlert),
       },
       currentRecoveredAlerts: {},
-      recoveredAlerts: {},
     });
     const alertsClient = new LegacyAlertsClient({
       alertingEventLogger,
@@ -430,7 +450,8 @@ describe('Legacy Alerts Client', () => {
     await alertsClient.initializeExecution({
       ...defaultExecutionOpts,
       activeAlertsFromState: {
-        '2': testAlert2,
+        '2': activeAlert,
+        '3': recoveredAlert,
       },
     });
 
@@ -447,27 +468,16 @@ describe('Legacy Alerts Client', () => {
       spaceId: 'space1',
     });
 
-    expect(getAlertsForNotification).toHaveBeenCalledWith(
-      {
-        enabled: true,
-        lookBackWindow: 20,
-        statusChangeThreshold: 4,
-      },
-      'default',
-      5,
-      {
-        '1': new Alert<AlertInstanceContext, AlertInstanceContext>('1', {
-          ...testAlert1,
-          meta: { ...testAlert1.meta, maintenanceWindowIds: ['test-id1', 'test-id2'] },
-        }),
-      },
-      {
-        '2': new Alert<AlertInstanceContext, AlertInstanceContext>('2', testAlert2),
-      },
-      {},
-      {},
-      null
-    );
+    expect(alertsClient.getProcessedAlerts('new')['1'].getMaintenanceWindowIds()).toEqual([
+      'test-id1',
+      'test-id5',
+    ]);
+    expect(alertsClient.getProcessedAlerts('active')['2'].getMaintenanceWindowIds()).toEqual([
+      'test-id1',
+    ]);
+    expect(alertsClient.getProcessedAlerts('recovered')['3'].getMaintenanceWindowIds()).toEqual([
+      'test-id1',
+    ]);
   });
 
   test('isTrackedAlert() should return true if alert was active in a previous execution, false otherwise', async () => {
diff --git a/x-pack/platform/plugins/shared/alerting/server/alerts_client/legacy_alerts_client.ts b/x-pack/platform/plugins/shared/alerting/server/alerts_client/legacy_alerts_client.ts
@@ -33,9 +33,10 @@ import {
   TrackedAlerts,
 } from './types';
 import { DEFAULT_MAX_ALERTS } from '../config';
-import { UntypedNormalizedRuleType } from '../rule_type_registry';
-import { MaintenanceWindowsService } from '../task_runner/maintenance_windows';
-import { AlertingEventLogger } from '../lib/alerting_event_logger/alerting_event_logger';
+import type { UntypedNormalizedRuleType } from '../rule_type_registry';
+import type { MaintenanceWindowsService } from '../task_runner/maintenance_windows';
+import type { MaintenanceWindow } from '../application/maintenance_window/types';
+import type { AlertingEventLogger } from '../lib/alerting_event_logger/alerting_event_logger';
 
 export interface LegacyAlertsClientParams {
   alertingEventLogger: AlertingEventLogger;
@@ -175,14 +176,20 @@ export class LegacyAlertsClient<
         keys(processedAlertsActive).length > 0 ||
         keys(processedAlertsRecovered).length > 0
       ) {
-        const { maintenanceWindowsWithoutScopedQueryIds } =
+        const { maintenanceWindowsWithoutScopedQueryIds, maintenanceWindows } =
           await this.options.maintenanceWindowsService.getMaintenanceWindows({
             eventLogger: this.options.alertingEventLogger,
             request: this.options.request,
             ruleTypeCategory: this.options.ruleType.category,
             spaceId: this.options.spaceId,
           });
 
+        this.removeExpiredMaintenanceWindows({
+          processedAlertsActive,
+          processedAlertsRecovered,
+          maintenanceWindows,
+        });
+
         for (const id in processedAlertsNew) {
           if (Object.hasOwn(processedAlertsNew, id)) {
             processedAlertsNew[id].setMaintenanceWindowIds(maintenanceWindowsWithoutScopedQueryIds);
@@ -283,4 +290,35 @@ export class LegacyAlertsClient<
   public async setAlertStatusToUntracked() {
     return;
   }
+  public getTrackedExecutions() {
+    return new Set([]);
+  }
+
+  private removeExpiredMaintenanceWindows({
+    processedAlertsActive,
+    processedAlertsRecovered,
+    maintenanceWindows,
+  }: {
+    processedAlertsActive: Record<string, Alert<State, Context, ActionGroupIds>>;
+    processedAlertsRecovered: Record<string, Alert<State, Context, RecoveryActionGroupId>>;
+    maintenanceWindows: MaintenanceWindow[];
+  }) {
+    const maintenanceWindowIds = maintenanceWindows.map((mw) => mw.id);
+
+    const clearMws = (
+      alerts: Record<string, Alert<State, Context, ActionGroupIds | RecoveryActionGroupId>>
+    ) => {
+      for (const id in alerts) {
+        if (Object.hasOwn(alerts, id)) {
+          const existingMaintenanceWindowIds = alerts[id].getMaintenanceWindowIds();
+          const activeMaintenanceWindowIds = existingMaintenanceWindowIds.filter((mw) => {
+            return maintenanceWindowIds.includes(mw);
+          });
+          alerts[id].setMaintenanceWindowIds(activeMaintenanceWindowIds);
+        }
+      }
+    };
+    clearMws(processedAlertsActive);
+    clearMws(processedAlertsRecovered);
+  }
 }
diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group3/maintenance_window_flows.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group3/maintenance_window_flows.ts
@@ -17,6 +17,7 @@ import {
   getRuleEvents,
   expectNoActionsFired,
   runSoon,
+  expectActionsFired,
 } from './test_helpers';
 
 // eslint-disable-next-line import/no-default-export
@@ -214,7 +215,7 @@ export default function maintenanceWindowFlowsTests({ getService }: FtrProviderC
       });
     });
 
-    it('alerts triggered within a MW should not fire actions if active or recovered outside a MW', async () => {
+    it('alerts triggered within a MW should fire actions if still active or recoveres after the MW expired', async () => {
       const pattern = {
         instance: [true, true, false, true],
       };
@@ -278,10 +279,11 @@ export default function maintenanceWindowFlowsTests({ getService }: FtrProviderC
         getService,
       });
 
-      await expectNoActionsFired({
+      await expectActionsFired({
         id: rule.id,
         supertest,
         retry,
+        expectedNumberOfActions: 1,
       });
 
       // Run again - recovered
@@ -298,10 +300,11 @@ export default function maintenanceWindowFlowsTests({ getService }: FtrProviderC
         getService,
       });
 
-      await expectNoActionsFired({
+      await expectActionsFired({
         id: rule.id,
         supertest,
         retry,
+        expectedNumberOfActions: 2,
       });
 
       // Run again - active again, this time fire the action since its a new alert instance
@@ -312,7 +315,7 @@ export default function maintenanceWindowFlowsTests({ getService }: FtrProviderC
       });
       await getRuleEvents({
         id: rule.id,
-        action: 1,
+        action: 3,
         activeInstance: 3,
         recoveredInstance: 1,
         retry,
diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group3/test_helpers.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group3/test_helpers.ts
@@ -213,6 +213,35 @@ export const expectNoActionsFired = async ({
   expect(actionEvents.length).eql(0);
 };
 
+export const expectActionsFired = async ({
+  id,
+  supertest,
+  retry,
+  expectedNumberOfActions,
+}: {
+  id: string;
+  supertest: SuperTestAgent;
+  retry: RetryService;
+  expectedNumberOfActions: number;
+}) => {
+  const events = await retry.try(async () => {
+    const { body: result } = await supertest
+      .get(`${getUrlPrefix(Spaces.space1.id)}/_test/event_log/alert/${id}/_find?per_page=5000`)
+      .expect(200);
+
+    if (!result.total) {
+      throw new Error('no events found yet');
+    }
+    return result.data as IValidatedEvent[];
+  });
+
+  const actionEvents = events.filter((event) => {
+    return event?.event?.action === 'execute-action';
+  });
+
+  expect(actionEvents.length).eql(expectedNumberOfActions);
+};
+
 export const runSoon = async ({
   id,
   supertest,
