From 39de18f5e921f0e8abc2cd11dea36e685dd1283a Mon Sep 17 00:00:00 2001 From: Claudio Procida Date: Mon, 29 Nov 2021 15:56:22 +0100 Subject: [PATCH 01/11] Replaces multiple find requests with aggregations --- .../server/rules_client/rules_client.ts | 79 +++++++++++++------ 1 file changed, 54 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts index b190737a157ea..2b0182a9bc851 100644 --- a/x-pack/plugins/alerting/server/rules_client/rules_client.ts +++ b/x-pack/plugins/alerting/server/rules_client/rules_client.ts @@ -93,6 +93,15 @@ export type InvalidateAPIKeyResult = | { apiKeysEnabled: false } | { apiKeysEnabled: true; result: SecurityPluginInvalidateAPIKeyResult }; +export interface RuleAggregation { + status: { + buckets: Array<{ + key: string; + doc_count: number; + }>; + }; +} + export interface ConstructorOptions { logger: Logger; taskManager: TaskManagerStartContract; @@ -646,40 +655,60 @@ export class RulesClient { public async aggregate({ options: { fields, ...options } = {}, }: { options?: AggregateOptions } = {}): Promise { - // Replace this when saved objects supports aggregations https://github.com/elastic/kibana/pull/64002 - const alertExecutionStatus = await Promise.all( - AlertExecutionStatusValues.map(async (status: string) => { - const { filter: authorizationFilter } = await this.authorization.getFindAuthorizationFilter( - AlertingAuthorizationEntity.Rule, - alertingAuthorizationFilterOpts - ); - const filter = options.filter - ? `${options.filter} and alert.attributes.executionStatus.status:(${status})` - : `alert.attributes.executionStatus.status:(${status})`; - const { total } = await this.unsecuredSavedObjectsClient.find({ - ...options, - filter: - (authorizationFilter && filter - ? nodeBuilder.and([ - esKuery.fromKueryExpression(filter), - authorizationFilter as KueryNode, - ]) - : authorizationFilter) ?? filter, - page: 1, - perPage: 0, - type: 'alert', - }); + const { filter: authorizationFilter } = await this.authorization.getFindAuthorizationFilter( + AlertingAuthorizationEntity.Rule, + alertingAuthorizationFilterOpts + ); + const filter = ''; + // const filter = options.filter + // ? `${options.filter} and alert.attributes.executionStatus.status:(${status})` + // : `alert.attributes.executionStatus.status:(${status})`; + const resp = await this.unsecuredSavedObjectsClient.find({ + ...options, + filter: + (authorizationFilter && filter + ? nodeBuilder.and([esKuery.fromKueryExpression(filter), authorizationFilter as KueryNode]) + : authorizationFilter) ?? filter, + page: 1, + perPage: 0, + type: 'alert', + aggs: { + status: { + terms: { field: 'alert.attributes.executionStatus.status' }, + }, + // ruleName: { + // terms: { field: 'alert.attributes.rule.name' }, + // }, + enabled: { + terms: { field: 'alert.attributes.enabled' }, + }, + muted: { + terms: { field: 'alert.attributes.muteAll' }, + }, + }, + }); - return { [status]: total }; + const alertExecutionStatus = resp.aggregations!.status.buckets.map( + ({ key, doc_count: docCount }) => ({ + [key]: docCount, }) ); - return { + const ret = { alertExecutionStatus: alertExecutionStatus.reduce( (acc, curr: { [status: string]: number }) => Object.assign(acc, curr), {} ), }; + + // Fill missing keys + for (const key of AlertExecutionStatusValues) { + if (!ret.alertExecutionStatus.hasOwnProperty(key)) { + ret.alertExecutionStatus[key] = 0; + } + } + + return ret; } public async delete({ id }: { id: string }) { From 757aaed1f83a3d8ca5f2f1714f7feebb93358cbd Mon Sep 17 00:00:00 2001 From: Claudio Procida Date: Mon, 29 Nov 2021 16:17:30 +0100 Subject: [PATCH 02/11] Updates unit tests --- .../server/rules_client/rules_client.ts | 6 +- .../rules_client/tests/aggregate.test.ts | 121 +++++++++--------- 2 files changed, 65 insertions(+), 62 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts index 2b0182a9bc851..1d73786f4493c 100644 --- a/x-pack/plugins/alerting/server/rules_client/rules_client.ts +++ b/x-pack/plugins/alerting/server/rules_client/rules_client.ts @@ -653,16 +653,12 @@ export class RulesClient { } public async aggregate({ - options: { fields, ...options } = {}, + options: { fields, filter, ...options } = {}, }: { options?: AggregateOptions } = {}): Promise { const { filter: authorizationFilter } = await this.authorization.getFindAuthorizationFilter( AlertingAuthorizationEntity.Rule, alertingAuthorizationFilterOpts ); - const filter = ''; - // const filter = options.filter - // ? `${options.filter} and alert.attributes.executionStatus.status:(${status})` - // : `alert.attributes.executionStatus.status:(${status})`; const resp = await this.unsecuredSavedObjectsClient.find({ ...options, filter: diff --git a/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts index faea609d39ffe..cb157514b0c3e 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts @@ -70,37 +70,24 @@ describe('aggregate()', () => { authorization.getFindAuthorizationFilter.mockResolvedValue({ ensureRuleTypeIsAuthorized() {}, }); - unsecuredSavedObjectsClient.find - .mockResolvedValueOnce({ - total: 10, - per_page: 0, - page: 1, - saved_objects: [], - }) - .mockResolvedValueOnce({ - total: 8, - per_page: 0, - page: 1, - saved_objects: [], - }) - .mockResolvedValueOnce({ - total: 6, - per_page: 0, - page: 1, - saved_objects: [], - }) - .mockResolvedValueOnce({ - total: 4, - per_page: 0, - page: 1, - saved_objects: [], - }) - .mockResolvedValueOnce({ - total: 2, - per_page: 0, - page: 1, - saved_objects: [], - }); + unsecuredSavedObjectsClient.find.mockResolvedValueOnce({ + total: 30, + per_page: 0, + page: 1, + saved_objects: [], + aggregations: { + status: { + buckets: [ + { key: 'active', doc_count: 8 }, + { key: 'error', doc_count: 6 }, + { key: 'ok', doc_count: 10 }, + { key: 'pending', doc_count: 4 }, + { key: 'unknown', doc_count: 2 }, + ], + }, + }, + }); + ruleTypeRegistry.list.mockReturnValue(listedTypes); authorization.filterByRuleTypeAuthorization.mockResolvedValue( new Set([ @@ -136,39 +123,59 @@ describe('aggregate()', () => { }, } `); - expect(unsecuredSavedObjectsClient.find).toHaveBeenCalledTimes( - AlertExecutionStatusValues.length - ); - AlertExecutionStatusValues.forEach((status: string, ndx: number) => { - expect(unsecuredSavedObjectsClient.find.mock.calls[ndx]).toEqual([ - { - fields: undefined, - filter: `alert.attributes.executionStatus.status:(${status})`, - page: 1, - perPage: 0, - type: 'alert', + expect(unsecuredSavedObjectsClient.find).toHaveBeenCalledTimes(1); + + expect(unsecuredSavedObjectsClient.find.mock.calls[0]).toEqual([ + { + filter: undefined, + page: 1, + perPage: 0, + type: 'alert', + aggs: { + status: { + terms: { field: 'alert.attributes.executionStatus.status' }, + }, + // ruleName: { + // terms: { field: 'alert.attributes.rule.name' }, + // }, + enabled: { + terms: { field: 'alert.attributes.enabled' }, + }, + muted: { + terms: { field: 'alert.attributes.muteAll' }, + }, }, - ]); - }); + }, + ]); }); test('supports filters when aggregating', async () => { const rulesClient = new RulesClient(rulesClientParams); await rulesClient.aggregate({ options: { filter: 'someTerm' } }); - expect(unsecuredSavedObjectsClient.find).toHaveBeenCalledTimes( - AlertExecutionStatusValues.length - ); - AlertExecutionStatusValues.forEach((status: string, ndx: number) => { - expect(unsecuredSavedObjectsClient.find.mock.calls[ndx]).toEqual([ - { - fields: undefined, - filter: `someTerm and alert.attributes.executionStatus.status:(${status})`, - page: 1, - perPage: 0, - type: 'alert', + expect(unsecuredSavedObjectsClient.find).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.find.mock.calls[0]).toEqual([ + { + fields: undefined, + filter: 'someTerm', + page: 1, + perPage: 0, + type: 'alert', + aggs: { + status: { + terms: { field: 'alert.attributes.executionStatus.status' }, + }, + // ruleName: { + // terms: { field: 'alert.attributes.rule.name' }, + // }, + enabled: { + terms: { field: 'alert.attributes.enabled' }, + }, + muted: { + terms: { field: 'alert.attributes.muteAll' }, + }, }, - ]); - }); + }, + ]); }); }); From 4072ceaa901911d452732d440b3d2293c4983fec Mon Sep 17 00:00:00 2001 From: Claudio Procida Date: Mon, 29 Nov 2021 16:25:13 +0100 Subject: [PATCH 03/11] Removes commented out code --- x-pack/plugins/alerting/server/rules_client/rules_client.ts | 3 --- .../alerting/server/rules_client/tests/aggregate.test.ts | 6 ------ 2 files changed, 9 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts index 1d73786f4493c..f2aee53561fd6 100644 --- a/x-pack/plugins/alerting/server/rules_client/rules_client.ts +++ b/x-pack/plugins/alerting/server/rules_client/rules_client.ts @@ -672,9 +672,6 @@ export class RulesClient { status: { terms: { field: 'alert.attributes.executionStatus.status' }, }, - // ruleName: { - // terms: { field: 'alert.attributes.rule.name' }, - // }, enabled: { terms: { field: 'alert.attributes.enabled' }, }, diff --git a/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts index cb157514b0c3e..97444b2609066 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts @@ -135,9 +135,6 @@ describe('aggregate()', () => { status: { terms: { field: 'alert.attributes.executionStatus.status' }, }, - // ruleName: { - // terms: { field: 'alert.attributes.rule.name' }, - // }, enabled: { terms: { field: 'alert.attributes.enabled' }, }, @@ -165,9 +162,6 @@ describe('aggregate()', () => { status: { terms: { field: 'alert.attributes.executionStatus.status' }, }, - // ruleName: { - // terms: { field: 'alert.attributes.rule.name' }, - // }, enabled: { terms: { field: 'alert.attributes.enabled' }, }, From 86e3bdc8b0dd348ea4257b43f66183eb8af6348a Mon Sep 17 00:00:00 2001 From: Claudio Procida Date: Mon, 29 Nov 2021 17:26:51 +0100 Subject: [PATCH 04/11] Removes unused import --- .../plugins/alerting/server/rules_client/tests/aggregate.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts index 97444b2609066..aa7386baa6363 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts @@ -15,7 +15,6 @@ import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertingAuthorization } from '../../authorization/alerting_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; import { getBeforeSetup, setGlobalDate } from './lib'; -import { AlertExecutionStatusValues } from '../../types'; import { RecoveredActionGroup } from '../../../common'; import { RegistryRuleType } from '../../rule_type_registry'; From a1fbd5e0bebd6d7952c2804cee149ee26d29b520 Mon Sep 17 00:00:00 2001 From: Claudio Procida Date: Mon, 29 Nov 2021 18:30:06 +0100 Subject: [PATCH 05/11] Adds muted and enabled aggregations --- x-pack/plugins/alerting/common/alert.ts | 2 ++ .../alerting/server/routes/aggregate_rules.ts | 4 +++ .../server/rules_client/rules_client.ts | 30 ++++++++++++++++++- .../application/lib/alert_api/aggregate.ts | 4 +++ 4 files changed, 39 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/alerting/common/alert.ts b/x-pack/plugins/alerting/common/alert.ts index 4431f185ac9ca..829388a68cc0a 100644 --- a/x-pack/plugins/alerting/common/alert.ts +++ b/x-pack/plugins/alerting/common/alert.ts @@ -55,6 +55,8 @@ export interface AlertAction { export interface AlertAggregations { alertExecutionStatus: { [status: string]: number }; + ruleEnabledStatus: { enabled: number; disabled: number }; + ruleMutedStatus: { muted: number; unmuted: number }; } export interface Alert { diff --git a/x-pack/plugins/alerting/server/routes/aggregate_rules.ts b/x-pack/plugins/alerting/server/routes/aggregate_rules.ts index 84c03e21ff36e..ee05897848ecf 100644 --- a/x-pack/plugins/alerting/server/routes/aggregate_rules.ts +++ b/x-pack/plugins/alerting/server/routes/aggregate_rules.ts @@ -47,10 +47,14 @@ const rewriteQueryReq: RewriteRequestCase = ({ }); const rewriteBodyRes: RewriteResponseCase = ({ alertExecutionStatus, + ruleEnabledStatus, + ruleMutedStatus, ...rest }) => ({ ...rest, rule_execution_status: alertExecutionStatus, + rule_enabled_status: ruleEnabledStatus, + rule_muted_status: ruleMutedStatus, }); export const aggregateRulesRoute = ( diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts index f2aee53561fd6..25024c1e18951 100644 --- a/x-pack/plugins/alerting/server/rules_client/rules_client.ts +++ b/x-pack/plugins/alerting/server/rules_client/rules_client.ts @@ -100,6 +100,20 @@ export interface RuleAggregation { doc_count: number; }>; }; + muted: { + buckets: Array<{ + key: number; + key_as_string: string; + doc_count: number; + }>; + }; + enabled: { + buckets: Array<{ + key: number; + key_as_string: string; + doc_count: number; + }>; + }; } export interface ConstructorOptions { @@ -159,6 +173,8 @@ interface IndexType { export interface AggregateResult { alertExecutionStatus: { [status: string]: number }; + ruleEnabledStatus?: { enabled: number; disabled: number }; + ruleMutedStatus?: { muted: number; unmuted: number }; } export interface FindResult { @@ -687,7 +703,7 @@ export class RulesClient { }) ); - const ret = { + const ret: AggregateResult = { alertExecutionStatus: alertExecutionStatus.reduce( (acc, curr: { [status: string]: number }) => Object.assign(acc, curr), {} @@ -701,6 +717,18 @@ export class RulesClient { } } + const enabledBuckets = resp.aggregations!.enabled.buckets; + ret.ruleEnabledStatus = { + enabled: enabledBuckets.find((bucket) => bucket.key === 1)?.doc_count ?? 0, + disabled: enabledBuckets.find((bucket) => bucket.key === 0)?.doc_count ?? 0, + }; + + const mutedBuckets = resp.aggregations!.muted.buckets; + ret.ruleMutedStatus = { + muted: mutedBuckets.find((bucket) => bucket.key === 1)?.doc_count ?? 0, + unmuted: mutedBuckets.find((bucket) => bucket.key === 0)?.doc_count ?? 0, + }; + return ret; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/aggregate.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/aggregate.ts index 917a491586b36..60482b26b7f25 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/aggregate.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/aggregate.ts @@ -12,10 +12,14 @@ import { AsApiContract, RewriteRequestCase } from '../../../../../actions/common const rewriteBodyRes: RewriteRequestCase = ({ rule_execution_status: alertExecutionStatus, + rule_enabled_status: ruleEnabledStatus, + rule_muted_status: ruleMutedStatus, ...rest }: any) => ({ ...rest, alertExecutionStatus, + ruleEnabledStatus, + ruleMutedStatus, }); export async function loadAlertAggregations({ From 31a4fe4772695cbaca9f2b875adedcdaff210797 Mon Sep 17 00:00:00 2001 From: Claudio Procida Date: Mon, 29 Nov 2021 18:37:07 +0100 Subject: [PATCH 06/11] Updates tests --- .../rules_client/tests/aggregate.test.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts index aa7386baa6363..0c64907d29a8a 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts @@ -84,6 +84,18 @@ describe('aggregate()', () => { { key: 'unknown', doc_count: 2 }, ], }, + enabled: { + buckets: [ + { key: 0, key_as_string: '0', doc_count: 2 }, + { key: 1, key_as_string: '1', doc_count: 28 }, + ], + }, + muted: { + buckets: [ + { key: 0, key_as_string: '0', doc_count: 27 }, + { key: 1, key_as_string: '1', doc_count: 3 }, + ], + }, }, }); @@ -120,6 +132,14 @@ describe('aggregate()', () => { "pending": 4, "unknown": 2, }, + "ruleEnabledStatus": Object { + "disabled": 2, + "enabled": 28, + }, + "ruleMutedStatus": Object { + "muted": 3, + "unmuted": 27, + }, } `); expect(unsecuredSavedObjectsClient.find).toHaveBeenCalledTimes(1); From ce9a5e85fd11a3e616cd2467cffeb3724cbf7ce3 Mon Sep 17 00:00:00 2001 From: Claudio Procida Date: Mon, 29 Nov 2021 19:59:06 +0100 Subject: [PATCH 07/11] Updates snapshot --- .../server/routes/aggregate_rules.test.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/x-pack/plugins/alerting/server/routes/aggregate_rules.test.ts b/x-pack/plugins/alerting/server/routes/aggregate_rules.test.ts index c8d4372fb72cf..81fb66ef5cf55 100644 --- a/x-pack/plugins/alerting/server/routes/aggregate_rules.test.ts +++ b/x-pack/plugins/alerting/server/routes/aggregate_rules.test.ts @@ -49,6 +49,14 @@ describe('aggregateRulesRoute', () => { pending: 1, unknown: 0, }, + ruleEnabledStatus: { + disabled: 1, + enabled: 40, + }, + ruleMutedStatus: { + muted: 2, + unmuted: 39, + }, }; rulesClient.aggregate.mockResolvedValueOnce(aggregateResult); @@ -65,6 +73,10 @@ describe('aggregateRulesRoute', () => { expect(await handler(context, req, res)).toMatchInlineSnapshot(` Object { "body": Object { + "rule_enabled_status": Object { + "disabled": 1, + "enabled": 40, + }, "rule_execution_status": Object { "active": 23, "error": 2, @@ -72,6 +84,10 @@ describe('aggregateRulesRoute', () => { "pending": 1, "unknown": 0, }, + "rule_muted_status": Object { + "muted": 2, + "unmuted": 39, + }, }, } `); @@ -89,6 +105,10 @@ describe('aggregateRulesRoute', () => { expect(res.ok).toHaveBeenCalledWith({ body: { + rule_enabled_status: { + disabled: 1, + enabled: 40, + }, rule_execution_status: { ok: 15, error: 2, @@ -96,6 +116,10 @@ describe('aggregateRulesRoute', () => { pending: 1, unknown: 0, }, + rule_muted_status: { + muted: 2, + unmuted: 39, + }, }, }); }); From c37e03965ab3c2f1250b8b911e443a965c18da82 Mon Sep 17 00:00:00 2001 From: Claudio Procida Date: Mon, 29 Nov 2021 21:21:19 +0100 Subject: [PATCH 08/11] Fixes functional test --- .../spaces_only/tests/alerting/aggregate.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts index 65aa38cb23c24..106918066f915 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts @@ -26,6 +26,10 @@ export default function createAggregateTests({ getService }: FtrProviderContext) expect(response.status).to.eql(200); expect(response.body).to.eql({ + rule_enabled_status: { + disabled: 0, + enabled: 0, + }, rule_execution_status: { ok: 0, active: 0, @@ -33,6 +37,10 @@ export default function createAggregateTests({ getService }: FtrProviderContext) pending: 0, unknown: 0, }, + rule_muted_status: { + muted: 0, + unmuted: 0, + }, }); }); From 31c50403caa094ad40683397873f55635ec576f8 Mon Sep 17 00:00:00 2001 From: Claudio Procida Date: Tue, 30 Nov 2021 00:05:31 +0100 Subject: [PATCH 09/11] Fixes functional test --- .../spaces_only/tests/alerting/aggregate.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts index 106918066f915..ea1b7aba18d29 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts @@ -101,6 +101,10 @@ export default function createAggregateTests({ getService }: FtrProviderContext) expect(reponse.status).to.eql(200); expect(reponse.body).to.eql({ + rule_enabled_status: { + disabled: 0, + enabled: 7, + }, rule_execution_status: { ok: NumOkAlerts, active: NumActiveAlerts, @@ -108,6 +112,10 @@ export default function createAggregateTests({ getService }: FtrProviderContext) pending: 0, unknown: 0, }, + rule_muted_status: { + muted: 0, + unmuted: 7, + }, }); }); From 8c25f00fa44eac0d40d5839898de47d26e4b11df Mon Sep 17 00:00:00 2001 From: Claudio Procida Date: Tue, 30 Nov 2021 15:18:09 +0100 Subject: [PATCH 10/11] Review feedback, fixes API tests --- .../server/rules_client/audit_events.ts | 3 +++ .../server/rules_client/rules_client.ts | 20 +++++++++++++++---- .../spaces_only/tests/alerting/aggregate.ts | 8 ++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/audit_events.ts b/x-pack/plugins/alerting/server/rules_client/audit_events.ts index 5f6122458ddaf..487de19691503 100644 --- a/x-pack/plugins/alerting/server/rules_client/audit_events.ts +++ b/x-pack/plugins/alerting/server/rules_client/audit_events.ts @@ -22,6 +22,7 @@ export enum RuleAuditAction { UNMUTE = 'rule_unmute', MUTE_ALERT = 'rule_alert_mute', UNMUTE_ALERT = 'rule_alert_unmute', + AGGREGATE = 'rule_aggregate', } type VerbsTuple = [string, string, string]; @@ -40,6 +41,7 @@ const eventVerbs: Record = { rule_unmute: ['unmute', 'unmuting', 'unmuted'], rule_alert_mute: ['mute alert of', 'muting alert of', 'muted alert of'], rule_alert_unmute: ['unmute alert of', 'unmuting alert of', 'unmuted alert of'], + rule_aggregate: ['access', 'accessing', 'accessed'], }; const eventTypes: Record = { @@ -56,6 +58,7 @@ const eventTypes: Record = { rule_unmute: 'change', rule_alert_mute: 'change', rule_alert_unmute: 'change', + rule_aggregate: 'access', }; export interface RuleAuditEventParams { diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts index 25024c1e18951..00c9c02c86127 100644 --- a/x-pack/plugins/alerting/server/rules_client/rules_client.ts +++ b/x-pack/plugins/alerting/server/rules_client/rules_client.ts @@ -671,10 +671,22 @@ export class RulesClient { public async aggregate({ options: { fields, filter, ...options } = {}, }: { options?: AggregateOptions } = {}): Promise { - const { filter: authorizationFilter } = await this.authorization.getFindAuthorizationFilter( - AlertingAuthorizationEntity.Rule, - alertingAuthorizationFilterOpts - ); + let authorizationTuple; + try { + authorizationTuple = await this.authorization.getFindAuthorizationFilter( + AlertingAuthorizationEntity.Rule, + alertingAuthorizationFilterOpts + ); + } catch (error) { + this.auditLogger?.log( + ruleAuditEvent({ + action: RuleAuditAction.AGGREGATE, + error, + }) + ); + throw error; + } + const { filter: authorizationFilter } = authorizationTuple; const resp = await this.unsecuredSavedObjectsClient.find({ ...options, filter: diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts index ea1b7aba18d29..cf7ebffef85a2 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/aggregate.ts @@ -184,6 +184,14 @@ export default function createAggregateTests({ getService }: FtrProviderContext) pending: 0, unknown: 0, }, + ruleEnabledStatus: { + disabled: 0, + enabled: 7, + }, + ruleMutedStatus: { + muted: 0, + unmuted: 7, + }, }); }); }); From 1e6e3ab4c34350ada05fe09b11aa7161c5df2365 Mon Sep 17 00:00:00 2001 From: Claudio Procida Date: Tue, 30 Nov 2021 15:42:55 +0100 Subject: [PATCH 11/11] Logs audit event and updates tests --- .../server/rules_client/rules_client.ts | 29 ++++++++++++++++--- .../rules_client/tests/aggregate.test.ts | 23 +++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts index 00c9c02c86127..674f659ba6a87 100644 --- a/x-pack/plugins/alerting/server/rules_client/rules_client.ts +++ b/x-pack/plugins/alerting/server/rules_client/rules_client.ts @@ -709,7 +709,28 @@ export class RulesClient { }, }); - const alertExecutionStatus = resp.aggregations!.status.buckets.map( + if (!resp.aggregations) { + // Return a placeholder with all zeroes + const placeholder: AggregateResult = { + alertExecutionStatus: {}, + ruleEnabledStatus: { + enabled: 0, + disabled: 0, + }, + ruleMutedStatus: { + muted: 0, + unmuted: 0, + }, + }; + + for (const key of AlertExecutionStatusValues) { + placeholder.alertExecutionStatus[key] = 0; + } + + return placeholder; + } + + const alertExecutionStatus = resp.aggregations.status.buckets.map( ({ key, doc_count: docCount }) => ({ [key]: docCount, }) @@ -722,20 +743,20 @@ export class RulesClient { ), }; - // Fill missing keys + // Fill missing keys with zeroes for (const key of AlertExecutionStatusValues) { if (!ret.alertExecutionStatus.hasOwnProperty(key)) { ret.alertExecutionStatus[key] = 0; } } - const enabledBuckets = resp.aggregations!.enabled.buckets; + const enabledBuckets = resp.aggregations.enabled.buckets; ret.ruleEnabledStatus = { enabled: enabledBuckets.find((bucket) => bucket.key === 1)?.doc_count ?? 0, disabled: enabledBuckets.find((bucket) => bucket.key === 0)?.doc_count ?? 0, }; - const mutedBuckets = resp.aggregations!.muted.buckets; + const mutedBuckets = resp.aggregations.muted.buckets; ret.ruleMutedStatus = { muted: mutedBuckets.find((bucket) => bucket.key === 1)?.doc_count ?? 0, unmuted: mutedBuckets.find((bucket) => bucket.key === 0)?.doc_count ?? 0, diff --git a/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts index 0c64907d29a8a..cd452ce6df571 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts @@ -14,6 +14,8 @@ import { encryptedSavedObjectsMock } from '../../../../encrypted_saved_objects/s import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertingAuthorization } from '../../authorization/alerting_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; +import { httpServerMock } from '../../../../../../src/core/server/mocks'; +import { auditServiceMock } from '../../../../security/server/audit/index.mock'; import { getBeforeSetup, setGlobalDate } from './lib'; import { RecoveredActionGroup } from '../../../common'; import { RegistryRuleType } from '../../rule_type_registry'; @@ -25,6 +27,7 @@ const unsecuredSavedObjectsClient = savedObjectsClientMock.create(); const encryptedSavedObjects = encryptedSavedObjectsMock.createClient(); const authorization = alertingAuthorizationMock.create(); const actionsAuthorization = actionsAuthorizationMock.create(); +const auditLogger = auditServiceMock.create().asScoped(httpServerMock.createKibanaRequest()); const kibanaVersion = 'v7.10.0'; const rulesClientParams: jest.Mocked = { @@ -46,6 +49,7 @@ const rulesClientParams: jest.Mocked = { beforeEach(() => { getBeforeSetup(rulesClientParams, taskManager, ruleTypeRegistry); + (auditLogger.log as jest.Mock).mockClear(); }); setGlobalDate(); @@ -191,4 +195,23 @@ describe('aggregate()', () => { }, ]); }); + + test('logs audit event when not authorized to aggregate rules', async () => { + const rulesClient = new RulesClient({ ...rulesClientParams, auditLogger }); + authorization.getFindAuthorizationFilter.mockRejectedValue(new Error('Unauthorized')); + + await expect(rulesClient.aggregate()).rejects.toThrow(); + expect(auditLogger.log).toHaveBeenCalledWith( + expect.objectContaining({ + event: expect.objectContaining({ + action: 'rule_aggregate', + outcome: 'failure', + }), + error: { + code: 'Error', + message: 'Unauthorized', + }, + }) + ); + }); });