Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Alerting] Change logger level to debug when delete a rule without alerts produce an error or warn when untracking alerts #183207

Merged
merged 9 commits into from
May 15, 2024
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 });
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to mock the response of updateByQuery for the test to test the correct behavior of the code.


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