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

[7.x] [Actions] Notify only on action group change (#82969) #85642

Merged
merged 1 commit into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions x-pack/plugins/alerts/common/alert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import { SavedObjectAttribute, SavedObjectAttributes } from 'kibana/server';
import { AlertNotifyWhenType } from './alert_notify_when_type';

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type AlertTypeState = Record<string, any>;
Expand Down Expand Up @@ -68,6 +69,7 @@ export interface Alert {
apiKey: string | null;
apiKeyOwner: string | null;
throttle: string | null;
notifyWhen: AlertNotifyWhenType | null;
muteAll: boolean;
mutedInstanceIds: string[];
executionStatus: AlertExecutionStatus;
Expand Down
18 changes: 18 additions & 0 deletions x-pack/plugins/alerts/common/alert_notify_when_type.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { validateNotifyWhenType } from './alert_notify_when_type';

test('validates valid notify when type', () => {
expect(validateNotifyWhenType('onActionGroupChange')).toBeUndefined();
expect(validateNotifyWhenType('onActiveAlert')).toBeUndefined();
expect(validateNotifyWhenType('onThrottleInterval')).toBeUndefined();
});
test('returns error string if input is not valid notify when type', () => {
expect(validateNotifyWhenType('randomString')).toEqual(
`string is not a valid AlertNotifyWhenType: randomString`
);
});
19 changes: 19 additions & 0 deletions x-pack/plugins/alerts/common/alert_notify_when_type.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

const AlertNotifyWhenTypeValues = [
'onActionGroupChange',
'onActiveAlert',
'onThrottleInterval',
] as const;
export type AlertNotifyWhenType = typeof AlertNotifyWhenTypeValues[number];

export function validateNotifyWhenType(notifyWhen: string) {
if (AlertNotifyWhenTypeValues.includes(notifyWhen as AlertNotifyWhenType)) {
return;
}
return `string is not a valid AlertNotifyWhenType: ${notifyWhen}`;
}
1 change: 1 addition & 0 deletions x-pack/plugins/alerts/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export * from './alert_navigation';
export * from './alert_instance_summary';
export * from './builtin_action_groups';
export * from './disabled_action_groups';
export * from './alert_notify_when_type';

export interface AlertingFrameworkHealth {
isSufficientlySecure: boolean;
Expand Down
108 changes: 108 additions & 0 deletions x-pack/plugins/alerts/server/alert_instance/alert_instance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,114 @@ describe('isThrottled', () => {
});
});

describe('scheduledActionGroupOrSubgroupHasChanged()', () => {
test('should be false if no last scheduled and nothing scheduled', () => {
const alertInstance = new AlertInstance();
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(false);
});

test('should be false if group does not change', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
},
},
});
alertInstance.scheduleActions('default');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(false);
});

test('should be false if group and subgroup does not change', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
subgroup: 'subgroup',
},
},
});
alertInstance.scheduleActionsWithSubGroup('default', 'subgroup');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(false);
});

test('should be false if group does not change and subgroup goes from undefined to defined', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
},
},
});
alertInstance.scheduleActionsWithSubGroup('default', 'subgroup');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(false);
});

test('should be false if group does not change and subgroup goes from defined to undefined', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
subgroup: 'subgroup',
},
},
});
alertInstance.scheduleActions('default');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(false);
});

test('should be true if no last scheduled and has scheduled action', () => {
const alertInstance = new AlertInstance();
alertInstance.scheduleActions('default');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(true);
});

test('should be true if group does change', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
},
},
});
alertInstance.scheduleActions('penguin');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(true);
});

test('should be true if group does change and subgroup does change', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
subgroup: 'subgroup',
},
},
});
alertInstance.scheduleActionsWithSubGroup('penguin', 'fish');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(true);
});

test('should be true if group does not change and subgroup does change', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
subgroup: 'subgroup',
},
},
});
alertInstance.scheduleActionsWithSubGroup('default', 'fish');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(true);
});
});

describe('getScheduledActionOptions()', () => {
test('defaults to undefined', () => {
const alertInstance = new AlertInstance();
Expand Down
25 changes: 25 additions & 0 deletions x-pack/plugins/alerts/server/alert_instance/alert_instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,31 @@ export class AlertInstance<
return false;
}

scheduledActionGroupOrSubgroupHasChanged(): boolean {
if (!this.meta.lastScheduledActions && this.scheduledExecutionOptions) {
// it is considered a change when there are no previous scheduled actions
// and new scheduled actions
return true;
}

if (this.meta.lastScheduledActions && this.scheduledExecutionOptions) {
// compare previous and new scheduled actions if both exist
return (
!this.scheduledActionGroupIsUnchanged(
this.meta.lastScheduledActions,
this.scheduledExecutionOptions
) ||
!this.scheduledActionSubgroupIsUnchanged(
this.meta.lastScheduledActions,
this.scheduledExecutionOptions
)
);
}

// no previous and no new scheduled actions
return false;
}

private scheduledActionGroupIsUnchanged(
lastScheduledActions: NonNullable<AlertInstanceMeta['lastScheduledActions']>,
scheduledExecutionOptions: ScheduledExecutionOptions<State, Context>
Expand Down
16 changes: 14 additions & 2 deletions x-pack/plugins/alerts/server/alerts_client/alerts_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,13 @@ import {
AlertTaskState,
AlertInstanceSummary,
AlertExecutionStatusValues,
AlertNotifyWhenType,
} from '../types';
import { validateAlertTypeParams, alertExecutionStatusFromRaw } from '../lib';
import {
validateAlertTypeParams,
alertExecutionStatusFromRaw,
getAlertNotifyWhenType,
} from '../lib';
import {
GrantAPIKeyResult as SecurityPluginGrantAPIKeyResult,
InvalidateAPIKeyResult as SecurityPluginInvalidateAPIKeyResult,
Expand Down Expand Up @@ -157,6 +162,7 @@ interface UpdateOptions {
actions: NormalizedAlertAction[];
params: Record<string, unknown>;
throttle: string | null;
notifyWhen: AlertNotifyWhenType | null;
};
}

Expand Down Expand Up @@ -251,6 +257,8 @@ export class AlertsClient {
const createTime = Date.now();
const { references, actions } = await this.denormalizeActions(data.actions);

const notifyWhen = getAlertNotifyWhenType(data.notifyWhen, data.throttle);

const rawAlert: RawAlert = {
...data,
...this.apiKeyAsAlertAttributes(createdAPIKey, username),
Expand All @@ -262,6 +270,7 @@ export class AlertsClient {
params: validatedAlertTypeParams as RawAlert['params'],
muteAll: false,
mutedInstanceIds: [],
notifyWhen,
executionStatus: {
status: 'pending',
lastExecutionDate: new Date().toISOString(),
Expand Down Expand Up @@ -694,6 +703,7 @@ export class AlertsClient {
? await this.createAPIKey(this.generateAPIKeyName(alertType.id, data.name))
: null;
const apiKeyAttributes = this.apiKeyAsAlertAttributes(createdAPIKey, username);
const notifyWhen = getAlertNotifyWhenType(data.notifyWhen, data.throttle);

let updatedObject: SavedObject<RawAlert>;
const createAttributes = this.updateMeta({
Expand All @@ -702,6 +712,7 @@ export class AlertsClient {
...apiKeyAttributes,
params: validatedAlertTypeParams as RawAlert['params'],
actions,
notifyWhen,
updatedBy: username,
updatedAt: new Date().toISOString(),
});
Expand Down Expand Up @@ -1326,7 +1337,7 @@ export class AlertsClient {

private getPartialAlertFromRaw(
id: string,
{ createdAt, updatedAt, meta, scheduledTaskId, ...rawAlert }: Partial<RawAlert>,
{ createdAt, updatedAt, meta, notifyWhen, scheduledTaskId, ...rawAlert }: Partial<RawAlert>,
references: SavedObjectReference[] | undefined
): PartialAlert {
// Not the prettiest code here, but if we want to use most of the
Expand All @@ -1341,6 +1352,7 @@ export class AlertsClient {
const executionStatus = alertExecutionStatusFromRaw(this.logger, id, rawAlert.executionStatus);
return {
id,
notifyWhen,
...rawAlertWithoutExecutionStatus,
// we currently only support the Interval Schedule type
// Once we support additional types, this type signature will likely change
Expand Down
Loading