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] Enables AlertTypes to define the custom recovery action groups #84408

Merged
merged 29 commits into from
Dec 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8716b86
renamed Resolved to Recovered
gmmorris Nov 23, 2020
6dd664d
fixed missing import
gmmorris Nov 23, 2020
00f7ef3
fixed buggy default message behaviour
gmmorris Nov 24, 2020
f9c7560
added missing test
gmmorris Nov 24, 2020
b1eb47b
fixed typing
gmmorris Nov 26, 2020
2c5348f
Merge branch 'master' into alerts/fix-buggy-default-message
gmmorris Nov 26, 2020
0d57716
fixed resolved in tests
gmmorris Nov 26, 2020
f2bd0e0
Merge branch 'master' into alerts/rename-resolved-to-recovered
gmmorris Nov 26, 2020
50e612e
allows alert types to specify their own custom recovery group name
gmmorris Nov 26, 2020
095159a
removed unnecesery field on always fires
gmmorris Nov 26, 2020
0ea276f
allows alert types to specify their own custom recovery group
gmmorris Nov 26, 2020
c040c6f
fixed mock alert types throughout unit tests
gmmorris Nov 26, 2020
455d295
Merge remote-tracking branch 'origin/alerts/fix-buggy-default-message…
gmmorris Nov 26, 2020
2699660
fixed typing issues
gmmorris Nov 30, 2020
23f1433
reduce repetition of mock data
gmmorris Nov 30, 2020
09732bd
Merge branch 'master' into alerts/custom-recovery-action-group
gmmorris Nov 30, 2020
4c20c3b
fixed alert type list test
gmmorris Nov 30, 2020
a4f8a9c
support legacy event log alert recovery syntax
gmmorris Nov 30, 2020
3421a0f
Merge branch 'master' into alerts/custom-recovery-action-group
gmmorris Dec 1, 2020
94a30d5
added doc
gmmorris Dec 2, 2020
b92d4b8
Merge branch 'master' into alerts/custom-recovery-action-group
gmmorris Dec 2, 2020
0010947
removed unneeded change in jira
gmmorris Dec 2, 2020
124d002
correct callback name in siem
gmmorris Dec 2, 2020
c690e67
renamed resolved to recovered
gmmorris Dec 2, 2020
d7b3b34
fixed mistaken rename
gmmorris Dec 2, 2020
7762864
Merge remote-tracking branch 'upstream/master' into alerts/custom-rec…
gmmorris Dec 3, 2020
ca54013
elvated default params to alert concern instead of actions concern
gmmorris Dec 3, 2020
69fa7af
made default params optional
gmmorris Dec 3, 2020
8940592
Merge branch 'master' into alerts/custom-recovery-action-group
gmmorris Dec 4, 2020
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
4 changes: 4 additions & 0 deletions x-pack/examples/alerting_example/server/alert_types/astros.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ export const alertType: AlertType = {
name: 'People In Space Right Now',
actionGroups: [{ id: 'default', name: 'default' }],
defaultActionGroupId: 'default',
recoveryActionGroup: {
id: 'hasLandedBackOnEarth',
name: 'Has landed back on Earth',
},
async executor({ services, params }) {
const { outerSpaceCapacity, craft: craftToTriggerBy, op } = params;

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ The following table describes the properties of the `options` object.
|name|A user-friendly name for the alert type. These will be displayed in dropdowns when choosing alert types.|string|
|actionGroups|An explicit list of groups the alert type may schedule actions for, each specifying the ActionGroup's unique ID and human readable name. Alert `actions` validation will use this configuartion to ensure groups are valid. We highly encourage using `kbn-i18n` to translate the names of actionGroup when registering the AlertType. |Array<{id:string, name:string}>|
|defaultActionGroupId|Default ID value for the group of the alert type.|string|
|recoveryActionGroup|An action group to use when an alert instance goes from an active state, to an inactive one. This action group should not be specified under the `actionGroups` property. If no recoveryActionGroup is specified, the default `recovered` action group will be used. |{id:string, name:string}|
|actionVariables|An explicit list of action variables the alert type makes available via context and state in action parameter templates, and a short human readable description. Alert UI will use this to display prompts for the users for these variables, in action parameter editors. We highly encourage using `kbn-i18n` to translate the descriptions. |{ context: Array<{name:string, description:string}, state: Array<{name:string, description:string}>|
|validate.params|When developing an alert type, you can choose to accept a series of parameters. You may also have the parameters validated before they are passed to the `executor` function or created as an alert saved object. In order to do this, provide a `@kbn/config-schema` schema that we will use to validate the `params` attribute.|@kbn/config-schema|
|executor|This is where the code of the alert type lives. This is a function to be called when executing an alert on an interval basis. For full details, see executor section below.|Function|
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerts/common/alert_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export interface AlertType {
id: string;
name: string;
actionGroups: ActionGroup[];
recoveryActionGroup: ActionGroup;
actionVariables: string[];
defaultActionGroupId: ActionGroup['id'];
producer: string;
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/alerts/common/builtin_action_groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
import { i18n } from '@kbn/i18n';
import { ActionGroup } from './alert_type';

export const RecoveredActionGroup: ActionGroup = {
export const RecoveredActionGroup: Readonly<ActionGroup> = {
Copy link
Contributor

Choose a reason for hiding this comment

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

naming nit: It seems like we use recovered everywhere except here where it is recovery. What if we renamed the built-in action group DefaultRecoveredActionGroup and then named the field on the alert recoveredActionGroup

Copy link
Contributor

Choose a reason for hiding this comment

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

I realize I put my comment on the wrong line where it doesn't actually say recovery 🤦‍♀️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha interesting, I just thought of recovery as the verb for becoming Recovered ...which is why the default AG for is Recovered. The term recoveredActionGroup could work.... but the past tense feels wrong to me 🤔

I don't know... Having two first languages makes these weird i my head at times.
Any thoughts from someone else in @elastic/kibana-alerting-services ?

id: 'recovered',
name: i18n.translate('xpack.alerts.builtinActionGroups.recovered', {
defaultMessage: 'Recovered',
}),
};

export function getBuiltinActionGroups(): ActionGroup[] {
return [RecoveredActionGroup];
export function getBuiltinActionGroups(customRecoveryGroup?: ActionGroup): ActionGroup[] {
return [customRecoveryGroup ?? Object.freeze(RecoveredActionGroup)];
}
6 changes: 5 additions & 1 deletion x-pack/plugins/alerts/public/alert_api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { AlertType } from '../common';
import { AlertType, RecoveredActionGroup } from '../common';
import { httpServiceMock } from '../../../../src/core/public/mocks';
import { loadAlert, loadAlertType, loadAlertTypes } from './alert_api';
import uuid from 'uuid';
Expand All @@ -22,6 +22,7 @@ describe('loadAlertTypes', () => {
actionVariables: ['var1'],
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
recoveryActionGroup: RecoveredActionGroup,
producer: 'alerts',
},
];
Expand All @@ -45,6 +46,7 @@ describe('loadAlertType', () => {
actionVariables: ['var1'],
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
recoveryActionGroup: RecoveredActionGroup,
producer: 'alerts',
};
http.get.mockResolvedValueOnce([alertType]);
Expand All @@ -65,6 +67,7 @@ describe('loadAlertType', () => {
actionVariables: [],
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
recoveryActionGroup: RecoveredActionGroup,
producer: 'alerts',
};
http.get.mockResolvedValueOnce([alertType]);
Expand All @@ -80,6 +83,7 @@ describe('loadAlertType', () => {
actionVariables: [],
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
recoveryActionGroup: RecoveredActionGroup,
producer: 'alerts',
},
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { AlertNavigationRegistry } from './alert_navigation_registry';
import { AlertType, SanitizedAlert } from '../../common';
import { AlertType, RecoveredActionGroup, SanitizedAlert } from '../../common';
import uuid from 'uuid';

beforeEach(() => jest.resetAllMocks());
Expand All @@ -14,6 +14,7 @@ const mockAlertType = (id: string): AlertType => ({
id,
name: id,
actionGroups: [],
recoveryActionGroup: RecoveredActionGroup,
actionVariables: [],
defaultActionGroupId: 'default',
producer: 'alerts',
Expand Down
73 changes: 73 additions & 0 deletions x-pack/plugins/alerts/server/alert_type_registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,71 @@ describe('register()', () => {
);
});

test('allows an AlertType to specify a custom recovery group', () => {
const alertType = {
id: 'test',
name: 'Test',
actionGroups: [
{
id: 'default',
name: 'Default',
},
],
defaultActionGroupId: 'default',
recoveryActionGroup: {
id: 'backToAwesome',
name: 'Back To Awesome',
},
executor: jest.fn(),
producer: 'alerts',
};
const registry = new AlertTypeRegistry(alertTypeRegistryParams);
registry.register(alertType);
expect(registry.get('test').actionGroups).toMatchInlineSnapshot(`
Array [
Object {
"id": "default",
"name": "Default",
},
Object {
"id": "backToAwesome",
"name": "Back To Awesome",
},
]
`);
});

test('throws if the custom recovery group is contained in the AlertType action groups', () => {
const alertType = {
id: 'test',
name: 'Test',
actionGroups: [
{
id: 'default',
name: 'Default',
},
{
id: 'backToAwesome',
name: 'Back To Awesome',
},
],
recoveryActionGroup: {
id: 'backToAwesome',
name: 'Back To Awesome',
},
defaultActionGroupId: 'default',
executor: jest.fn(),
producer: 'alerts',
};
const registry = new AlertTypeRegistry(alertTypeRegistryParams);

expect(() => registry.register(alertType)).toThrowError(
new Error(
`Alert type [id="${alertType.id}"] cannot be registered. Action group [backToAwesome] cannot be used as both a recovery and an active action group.`
)
);
});

test('registers the executor with the task manager', () => {
const alertType = {
id: 'test',
Expand Down Expand Up @@ -243,6 +308,10 @@ describe('get()', () => {
"id": "test",
"name": "Test",
"producer": "alerts",
"recoveryActionGroup": Object {
"id": "recovered",
"name": "Recovered",
},
}
`);
});
Expand Down Expand Up @@ -300,6 +369,10 @@ describe('list()', () => {
"id": "test",
"name": "Test",
"producer": "alerts",
"recoveryActionGroup": Object {
"id": "recovered",
"name": "Recovered",
},
},
}
`);
Expand Down
104 changes: 80 additions & 24 deletions x-pack/plugins/alerts/server/alert_type_registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { i18n } from '@kbn/i18n';
import { schema } from '@kbn/config-schema';
import typeDetect from 'type-detect';
import { intersection } from 'lodash';
import _ from 'lodash';
import { RunContext, TaskManagerSetupContract } from '../../task_manager/server';
import { TaskRunnerFactory } from './task_runner';
import {
Expand All @@ -18,9 +17,8 @@ import {
AlertTypeState,
AlertInstanceState,
AlertInstanceContext,
ActionGroup,
} from './types';
import { getBuiltinActionGroups } from '../common';
import { RecoveredActionGroup, getBuiltinActionGroups } from '../common';

interface ConstructorOptions {
taskManager: TaskManagerSetupContract;
Expand All @@ -29,8 +27,13 @@ interface ConstructorOptions {

export interface RegistryAlertType
extends Pick<
AlertType,
'name' | 'actionGroups' | 'defaultActionGroupId' | 'actionVariables' | 'producer'
NormalizedAlertType,
| 'name'
| 'actionGroups'
| 'recoveryActionGroup'
| 'defaultActionGroupId'
| 'actionVariables'
| 'producer'
> {
id: string;
}
Expand All @@ -55,9 +58,17 @@ const alertIdSchema = schema.string({
},
});

export type NormalizedAlertType<
Params extends AlertTypeParams = AlertTypeParams,
State extends AlertTypeState = AlertTypeState,
InstanceState extends AlertInstanceState = AlertInstanceState,
InstanceContext extends AlertInstanceContext = AlertInstanceContext
> = Omit<AlertType<Params, State, InstanceState, InstanceContext>, 'recoveryActionGroup'> &
Pick<Required<AlertType<Params, State, InstanceState, InstanceContext>>, 'recoveryActionGroup'>;

export class AlertTypeRegistry {
private readonly taskManager: TaskManagerSetupContract;
private readonly alertTypes: Map<string, AlertType> = new Map();
private readonly alertTypes: Map<string, NormalizedAlertType> = new Map();
private readonly taskRunnerFactory: TaskRunnerFactory;

constructor({ taskManager, taskRunnerFactory }: ConstructorOptions) {
Expand Down Expand Up @@ -86,14 +97,15 @@ export class AlertTypeRegistry {
);
}
alertType.actionVariables = normalizedActionVariables(alertType.actionVariables);
validateActionGroups(alertType.id, alertType.actionGroups);
alertType.actionGroups = [...alertType.actionGroups, ..._.cloneDeep(getBuiltinActionGroups())];
this.alertTypes.set(alertIdSchema.validate(alertType.id), { ...alertType } as AlertType);

const normalizedAlertType = augmentActionGroupsWithReserved(alertType as AlertType);

this.alertTypes.set(alertIdSchema.validate(alertType.id), normalizedAlertType);
this.taskManager.registerTaskDefinitions({
[`alerting:${alertType.id}`]: {
title: alertType.name,
createTaskRunner: (context: RunContext) =>
this.taskRunnerFactory.create({ ...alertType } as AlertType, context),
this.taskRunnerFactory.create(normalizedAlertType, context),
},
});
}
Expand All @@ -103,7 +115,7 @@ export class AlertTypeRegistry {
State extends AlertTypeState = AlertTypeState,
InstanceState extends AlertInstanceState = AlertInstanceState,
InstanceContext extends AlertInstanceContext = AlertInstanceContext
>(id: string): AlertType<Params, State, InstanceState, InstanceContext> {
>(id: string): NormalizedAlertType<Params, State, InstanceState, InstanceContext> {
if (!this.has(id)) {
throw Boom.badRequest(
i18n.translate('xpack.alerts.alertTypeRegistry.get.missingAlertTypeError', {
Expand All @@ -114,19 +126,32 @@ export class AlertTypeRegistry {
})
);
}
return this.alertTypes.get(id)! as AlertType<Params, State, InstanceState, InstanceContext>;
return this.alertTypes.get(id)! as NormalizedAlertType<
Params,
State,
InstanceState,
InstanceContext
>;
}

public list(): Set<RegistryAlertType> {
return new Set(
Array.from(this.alertTypes).map(
([id, { name, actionGroups, defaultActionGroupId, actionVariables, producer }]: [
string,
AlertType
]) => ({
([
id,
{
name,
actionGroups,
recoveryActionGroup,
defaultActionGroupId,
actionVariables,
producer,
},
]: [string, NormalizedAlertType]) => ({
id,
name,
actionGroups,
recoveryActionGroup,
defaultActionGroupId,
actionVariables,
producer,
Expand All @@ -144,21 +169,52 @@ function normalizedActionVariables(actionVariables: AlertType['actionVariables']
};
}

function validateActionGroups(alertTypeId: string, actionGroups: ActionGroup[]) {
const reservedActionGroups = intersection(
actionGroups.map((item) => item.id),
getBuiltinActionGroups().map((item) => item.id)
function augmentActionGroupsWithReserved<
Params extends AlertTypeParams,
State extends AlertTypeState,
InstanceState extends AlertInstanceState,
InstanceContext extends AlertInstanceContext
>(
alertType: AlertType<Params, State, InstanceState, InstanceContext>
): NormalizedAlertType<Params, State, InstanceState, InstanceContext> {
const reservedActionGroups = getBuiltinActionGroups(alertType.recoveryActionGroup);
const { id, actionGroups, recoveryActionGroup } = alertType;

const activeActionGroups = new Set(actionGroups.map((item) => item.id));
const intersectingReservedActionGroups = intersection(
[...activeActionGroups.values()],
reservedActionGroups.map((item) => item.id)
);
if (reservedActionGroups.length > 0) {
if (recoveryActionGroup && activeActionGroups.has(recoveryActionGroup.id)) {
throw new Error(
i18n.translate(
'xpack.alerts.alertTypeRegistry.register.customRecoveryActionGroupUsageError',
{
defaultMessage:
'Alert type [id="{id}"] cannot be registered. Action group [{actionGroup}] cannot be used as both a recovery and an active action group.',
values: {
actionGroup: recoveryActionGroup.id,
id,
},
}
)
);
} else if (intersectingReservedActionGroups.length > 0) {
throw new Error(
i18n.translate('xpack.alerts.alertTypeRegistry.register.reservedActionGroupUsageError', {
defaultMessage:
'Alert type [id="{alertTypeId}"] cannot be registered. Action groups [{actionGroups}] are reserved by the framework.',
'Alert type [id="{id}"] cannot be registered. Action groups [{actionGroups}] are reserved by the framework.',
values: {
actionGroups: reservedActionGroups.join(', '),
alertTypeId,
actionGroups: intersectingReservedActionGroups.join(', '),
id,
},
})
);
}

return {
...alertType,
actionGroups: [...actionGroups, ...reservedActionGroups],
recoveryActionGroup: recoveryActionGroup ?? RecoveredActionGroup,
};
}
Loading