Skip to content

Commit

Permalink
[Alerting] Change logger level to debug when delete a rule without al…
Browse files Browse the repository at this point in the history
…erts produce an error or warn when untracking alerts (#183207)

## Summary

Fixes: #182754

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
cnasikas and kibanamachine committed May 15, 2024
1 parent 19818f2 commit d1806c9
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,8 @@ describe('setAlertsToUntracked()', () => {
},
});

clusterClient.updateByQuery.mockResponseOnce({ total: 2, updated: 2 });

const result = await setAlertsToUntracked({
isUsingQuery: true,
query: [
Expand Down Expand Up @@ -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([]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand All @@ -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,
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ const rulesClientParams: jest.Mocked<ConstructorOptions> = {

describe('bulkUntrackAlerts()', () => {
let rulesClient: RulesClient;

beforeEach(async () => {
jest.clearAllMocks();
rulesClient = new RulesClient(rulesClientParams);
});

Expand Down Expand Up @@ -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",
}
`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand All @@ -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({
Expand All @@ -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',
})
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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<Record<string, RawAlert>, Alert>(
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerting/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
"@kbn/core-execution-context-server-mocks",
"@kbn/react-kibana-context-render",
"@kbn/search-types",
"@kbn/core-security-server",
],
"exclude": [
"target/**/*"
Expand Down

0 comments on commit d1806c9

Please sign in to comment.