diff --git a/x-pack/plugins/alerting/server/alerts_service/lib/set_alerts_to_untracked.test.ts b/x-pack/plugins/alerting/server/alerts_service/lib/set_alerts_to_untracked.test.ts index 691fc8548c098f..49ece72c42f955 100644 --- a/x-pack/plugins/alerting/server/alerts_service/lib/set_alerts_to_untracked.test.ts +++ b/x-pack/plugins/alerting/server/alerts_service/lib/set_alerts_to_untracked.test.ts @@ -426,6 +426,8 @@ describe('setAlertsToUntracked()', () => { }, }); + clusterClient.updateByQuery.mockResponseOnce({ total: 2, updated: 2 }); + const result = await setAlertsToUntracked({ isUsingQuery: true, query: [ @@ -523,4 +525,99 @@ describe('setAlertsToUntracked()', () => { }, ]); }); + + test('should return an empty array if the search returns zero results', async () => { + getAuthorizedRuleTypesMock.mockResolvedValue([ + { + id: 'test-rule-type', + }, + ]); + getAlertIndicesAliasMock.mockResolvedValue(['test-alert-index']); + + clusterClient.search.mockResponseOnce({ + took: 1, + timed_out: false, + _shards: { + total: 1, + successful: 1, + skipped: 0, + failed: 0, + }, + hits: { + hits: [], + }, + aggregations: { + ruleTypeIds: { + buckets: [ + { + key: 'some rule type', + consumers: { + buckets: [{ key: 'o11y' }], + }, + }, + ], + }, + }, + }); + + clusterClient.updateByQuery.mockResponseOnce({ total: 0, updated: 0 }); + + const result = await setAlertsToUntracked({ + isUsingQuery: true, + query: [ + { + bool: { + must: { + term: { + 'kibana.alert.rule.name': 'test', + }, + }, + }, + }, + ], + featureIds: ['o11y'], + spaceId: 'default', + getAuthorizedRuleTypes: getAuthorizedRuleTypesMock, + getAlertIndicesAlias: getAlertIndicesAliasMock, + ensureAuthorized: ensureAuthorizedMock, + logger, + esClient: clusterClient, + }); + + expect(getAlertIndicesAliasMock).lastCalledWith(['test-rule-type'], 'default'); + + expect(clusterClient.updateByQuery).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.objectContaining({ + query: { + bool: { + must: [ + { + term: { + 'kibana.alert.status': { + value: 'active', // This has to be active + }, + }, + }, + ], + filter: [ + { + bool: { + must: { + term: { + 'kibana.alert.rule.name': 'test', + }, + }, + }, + }, + ], + }, + }, + }), + }) + ); + + expect(clusterClient.search).not.toHaveBeenCalledWith(); + expect(result).toEqual([]); + }); }); diff --git a/x-pack/plugins/alerting/server/alerts_service/lib/set_alerts_to_untracked.ts b/x-pack/plugins/alerting/server/alerts_service/lib/set_alerts_to_untracked.ts index c28a760853ba62..ed0c2cb21e06b8 100644 --- a/x-pack/plugins/alerting/server/alerts_service/lib/set_alerts_to_untracked.ts +++ b/x-pack/plugins/alerting/server/alerts_service/lib/set_alerts_to_untracked.ts @@ -208,6 +208,7 @@ export async function setAlertsToUntracked( // Retry this updateByQuery up to 3 times to make sure the number of documents // updated equals the number of documents matched let total = 0; + for (let retryCount = 0; retryCount < 3; retryCount++) { const response = await esClient.updateByQuery({ index: indices, @@ -224,14 +225,18 @@ export async function setAlertsToUntracked( }); if (total === 0 && response.total === 0) { - throw new Error('No active alerts matched the query'); + logger.debug('No active alerts matched the query'); + break; } + if (response.total) { total = response.total; } + if (response.total === response.updated) { break; } + logger.warn( `Attempt ${retryCount + 1}: Failed to untrack ${ (response.total ?? 0) - (response.updated ?? 0) @@ -241,6 +246,10 @@ export async function setAlertsToUntracked( ); } + if (total === 0) { + return []; + } + // Fetch and return updated rule and alert instance UUIDs const searchResponse = await esClient.search({ index: indices, @@ -251,6 +260,7 @@ export async function setAlertsToUntracked( query: getUntrackQuery(params, ALERT_STATUS_UNTRACKED), }, }); + return searchResponse.hits.hits.map((hit) => hit._source) as UntrackedAlertsResult; } catch (err) { logger.error( diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_untrack/bulk_untrack_alerts.test.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_untrack/bulk_untrack_alerts.test.ts index 4e2077f15a899b..befd240286a061 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/bulk_untrack/bulk_untrack_alerts.test.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/bulk_untrack/bulk_untrack_alerts.test.ts @@ -70,7 +70,9 @@ const rulesClientParams: jest.Mocked = { describe('bulkUntrackAlerts()', () => { let rulesClient: RulesClient; + beforeEach(async () => { + jest.clearAllMocks(); rulesClient = new RulesClient(rulesClientParams); }); @@ -241,4 +243,61 @@ describe('bulkUntrackAlerts()', () => { } `); }); + + it('should not call bulkUpdateState with no taskIds', async () => { + alertsService.setAlertsToUntracked.mockResolvedValueOnce([]); + + await rulesClient.bulkUntrackAlerts({ + isUsingQuery: true, + indices: ['test-index'], + alertUuids: ['my-uuid'], + }); + + expect(alertsService.setAlertsToUntracked).toHaveBeenCalledTimes(1); + expect(taskManager.bulkUpdateState).not.toHaveBeenCalledWith(); + }); + + it('filters out undefined rule uuids', async () => { + alertsService.setAlertsToUntracked.mockResolvedValueOnce([{}, { foo: 'bar' }]); + + await rulesClient.bulkUntrackAlerts({ + isUsingQuery: true, + indices: ['test-index'], + alertUuids: ['my-uuid'], + }); + + expect(alertsService.setAlertsToUntracked).toHaveBeenCalledTimes(1); + expect(taskManager.bulkUpdateState).not.toHaveBeenCalledWith(); + }); + + it('should audit log success with no taskIds', async () => { + alertsService.setAlertsToUntracked.mockResolvedValueOnce([]); + + await rulesClient.bulkUntrackAlerts({ + isUsingQuery: true, + indices: ['test-index'], + alertUuids: ['my-uuid'], + }); + + expect(taskManager.bulkUpdateState).not.toHaveBeenCalledWith(); + expect(auditLogger.log.mock.calls[0][0]).toMatchInlineSnapshot(` + Object { + "error": undefined, + "event": Object { + "action": "rule_alert_untrack", + "category": Array [ + "database", + ], + "outcome": "success", + "type": Array [ + "change", + ], + }, + "kibana": Object { + "saved_object": undefined, + }, + "message": "User has untracked a rule", + } + `); + }); }); diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_untrack/bulk_untrack_alerts.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_untrack/bulk_untrack_alerts.ts index d3a1badd0ab330..304037782f6d39 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/bulk_untrack/bulk_untrack_alerts.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/bulk_untrack/bulk_untrack_alerts.ts @@ -9,6 +9,7 @@ import { omitBy } from 'lodash'; import Boom from '@hapi/boom'; import { withSpan } from '@kbn/apm-utils'; import { ALERT_RULE_UUID, ALERT_UUID } from '@kbn/rule-data-utils'; +import { AuditLogger } from '@kbn/core-security-server'; import { bulkUntrackBodySchema } from './schemas'; import type { BulkUntrackBody } from './types'; import { WriteOperations, AlertingAuthorizationEntity } from '../../../../authorization'; @@ -64,7 +65,13 @@ async function bulkUntrackAlertsWithOCC(context: RulesClientContext, params: Bul }); // Clear alert instances from their corresponding tasks so that they can remain untracked - const taskIds = [...new Set(result.map((doc) => doc[ALERT_RULE_UUID]))]; + const taskIds = [...new Set(result.map((doc) => doc[ALERT_RULE_UUID]).filter(Boolean))]; + + if (taskIds.length === 0) { + auditLogSuccess(context.auditLogger); + return; + } + await context.taskManager.bulkUpdateState(taskIds, (state, id) => { try { const uuidsToClear = result @@ -90,12 +97,7 @@ async function bulkUntrackAlertsWithOCC(context: RulesClientContext, params: Bul } }); - context.auditLogger?.log( - ruleAuditEvent({ - action: RuleAuditAction.UNTRACK_ALERT, - outcome: 'success', - }) - ); + auditLogSuccess(context.auditLogger); } catch (error) { context.auditLogger?.log( ruleAuditEvent({ @@ -106,3 +108,12 @@ async function bulkUntrackAlertsWithOCC(context: RulesClientContext, params: Bul throw error; } } + +const auditLogSuccess = (auditLogger?: AuditLogger) => { + auditLogger?.log( + ruleAuditEvent({ + action: RuleAuditAction.UNTRACK_ALERT, + outcome: 'success', + }) + ); +}; diff --git a/x-pack/plugins/alerting/server/rules_client/lib/untrack_rule_alerts.ts b/x-pack/plugins/alerting/server/rules_client/lib/untrack_rule_alerts.ts index d69d84c7fd8c73..76dd34a7de68a3 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/untrack_rule_alerts.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/untrack_rule_alerts.ts @@ -24,11 +24,13 @@ export const untrackRuleAlerts = async ( ) => { return withSpan({ name: 'untrackRuleAlerts', type: 'rules' }, async () => { if (!context.eventLogger || !attributes.scheduledTaskId) return; + try { const taskInstance = taskInstanceToAlertTaskInstance( await context.taskManager.get(attributes.scheduledTaskId), attributes as unknown as SanitizedRule ); + const { state } = taskInstance; const untrackedAlerts = mapValues, Alert>( diff --git a/x-pack/plugins/alerting/tsconfig.json b/x-pack/plugins/alerting/tsconfig.json index f76c6d781ad830..1bc43c34e5aa8e 100644 --- a/x-pack/plugins/alerting/tsconfig.json +++ b/x-pack/plugins/alerting/tsconfig.json @@ -70,6 +70,7 @@ "@kbn/core-execution-context-server-mocks", "@kbn/react-kibana-context-render", "@kbn/search-types", + "@kbn/core-security-server", ], "exclude": [ "target/**/*"