maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

fixes shared types across alerting plugins (#55824)

Browse source 
This addresses two issues that have come up:

Alerting and Actions have TypeScript types that are needed across server and public plugins, and need to be extracted to a common path (thanks @chrisronline for bringing this to our attention)
Due to the above, types have been duplicated between the alerting and actions when needed in the Alerting UI, which has led to them diverging. This forces the UI to type check against the API, which will help reduce these errors in the future.
This commit is contained in:
Gidi Meir Morris 2020-01-24 17:06:59 +00:00 committed by GitHub
parent 943e450ead
commit bb29cec20b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 186 additions and 146 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
x-pack/legacy/plugins/alerting/common/index.ts Normal file
View file

@ -0,0 +1,7 @@
/*
* 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.
*/
export * from './types';

43
x-pack/legacy/plugins/alerting/common/types.ts Normal file
View file

@ -0,0 +1,43 @@
/*
* 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 { SavedObjectAttributes } from 'kibana/server';
import { AlertActionParams } from '../server/types';
export interface IntervalSchedule extends SavedObjectAttributes {
interval: string;
}
export interface AlertAction {
group: string;
id: string;
actionTypeId: string;
params: AlertActionParams;
}
export interface Alert {
id: string;
enabled: boolean;
name: string;
tags: string[];
alertTypeId: string;
consumer: string;
schedule: IntervalSchedule;
actions: AlertAction[];
params: Record<string, any>;
scheduledTaskId?: string;
createdBy: string | null;
updatedBy: string | null;
createdAt: Date;
updatedAt: Date;
apiKey: string | null;
apiKeyOwner: string | null;
throttle: string | null;
muteAll: boolean;
mutedInstanceIds: string[];
}
export type SanitizedAlert = Omit<Alert, 'apiKey'>;

5
x-pack/legacy/plugins/alerting/server/alerts_client.ts
View file

@ -21,6 +21,7 @@ import {
AlertAction,
AlertType,
IntervalSchedule,
SanitizedAlert,
} from './types';
import { validateAlertTypeParams } from './lib';
import {
@ -74,7 +75,7 @@ export interface FindResult {
page: number;
perPage: number;
total: number;
data: Alert[];
data: SanitizedAlert[];
}
interface CreateOptions {
@ -198,7 +199,7 @@ export class AlertsClient {
);
}
public async get({ id }: { id: string }): Promise<Alert> {
public async get({ id }: { id: string }): Promise<SanitizedAlert> {
const result = await this.savedObjectsClient.get('alert', id);
return this.getAlertFromRaw(result.id, result.attributes, result.updated_at, result.references);
}

36
x-pack/legacy/plugins/alerting/server/types.ts
View file

@ -8,6 +8,9 @@ import { AlertInstance } from './alert_instance';
import { AlertTypeRegistry as OrigAlertTypeRegistry } from './alert_type_registry';
import { PluginSetupContract, PluginStartContract } from './plugin';
import { SavedObjectAttributes, SavedObjectsClientContract } from '../../../../../src/core/server';
import { Alert } from '../common';
export * from '../common';
export type State = Record<string, any>;
export type Context = Record<string, any>;
@ -52,13 +55,6 @@ export interface AlertType {
export type AlertActionParams = SavedObjectAttributes;
export interface AlertAction {
group: string;
id: string;
actionTypeId: string;
params: AlertActionParams;
}
export interface RawAlertAction extends SavedObjectAttributes {
group: string;
actionRef: string;
@ -66,32 +62,6 @@ export interface RawAlertAction extends SavedObjectAttributes {
params: AlertActionParams;
}
export interface IntervalSchedule extends SavedObjectAttributes {
interval: string;
}
export interface Alert {
id: string;
enabled: boolean;
name: string;
tags: string[];
alertTypeId: string;
consumer: string;
schedule: IntervalSchedule;
actions: AlertAction[];
params: Record<string, any>;
scheduledTaskId?: string;
createdBy: string | null;
updatedBy: string | null;
createdAt: Date;
updatedAt: Date;
apiKey: string | null;
apiKeyOwner: string | null;
throttle: string | null;
muteAll: boolean;
mutedInstanceIds: string[];
}
export type PartialAlert = Pick<Alert, 'id'> & Partial<Omit<Alert, 'id'>>;
export interface RawAlert extends SavedObjectAttributes {

4
x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/read_rules.ts
View file

@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { Alert } from '../../../../../alerting/server/types';
import { SanitizedAlert } from '../../../../../alerting/common';
import { INTERNAL_RULE_ID_KEY } from '../../../../common/constants';
import { findRules } from './find_rules';
import { ReadRuleParams, isAlertType } from './types';
@ -21,7 +21,7 @@ export const readRules = async ({
alertsClient,
id,
ruleId,
}: ReadRuleParams): Promise<Alert | null> => {
}: ReadRuleParams): Promise<SanitizedAlert | null> => {
if (id != null) {
try {
const rule = await alertsClient.get({ id });

162
x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/lib/alert_api.test.ts
View file

@ -83,20 +83,20 @@ describe('loadAlerts', () => {
const result = await loadAlerts({ http, searchText: 'apples', page: { index: 0, size: 10 } });
expect(result).toEqual(resolvedValue);
expect(http.get.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"/api/alert/_find",
Object {
"query": Object {
"default_search_operator": "AND",
"filter": undefined,
"page": 1,
"per_page": 10,
"search": "apples",
"search_fields": "[\\"name\\",\\"tags\\"]",
},
},
]
`);
Array [
"/api/alert/_find",
Object {
"query": Object {
"default_search_operator": "AND",
"filter": undefined,
"page": 1,
"per_page": 10,
"search": "apples",
"search_fields": "[\\"name\\",\\"tags\\"]",
},
},
]
`);
});
test('should call find API with actionTypesFilter', async () => {
@ -115,20 +115,20 @@ describe('loadAlerts', () => {
});
expect(result).toEqual(resolvedValue);
expect(http.get.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"/api/alert/_find",
Object {
"query": Object {
"default_search_operator": "AND",
"filter": undefined,
"page": 1,
"per_page": 10,
"search": "foo",
"search_fields": "[\\"name\\",\\"tags\\"]",
},
},
]
`);
Array [
"/api/alert/_find",
Object {
"query": Object {
"default_search_operator": "AND",
"filter": undefined,
"page": 1,
"per_page": 10,
"search": "foo",
"search_fields": "[\\"name\\",\\"tags\\"]",
},
},
]
`);
});
test('should call find API with typesFilter', async () => {
@ -147,20 +147,20 @@ describe('loadAlerts', () => {
});
expect(result).toEqual(resolvedValue);
expect(http.get.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"/api/alert/_find",
Object {
"query": Object {
"default_search_operator": "AND",
"filter": "alert.attributes.alertTypeId:(foo or bar)",
"page": 1,
"per_page": 10,
"search": undefined,
"search_fields": undefined,
},
},
]
`);
Array [
"/api/alert/_find",
Object {
"query": Object {
"default_search_operator": "AND",
"filter": "alert.attributes.alertTypeId:(foo or bar)",
"page": 1,
"per_page": 10,
"search": undefined,
"search_fields": undefined,
},
},
]
`);
});
test('should call find API with actionTypesFilter and typesFilter', async () => {
@ -180,20 +180,20 @@ describe('loadAlerts', () => {
});
expect(result).toEqual(resolvedValue);
expect(http.get.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"/api/alert/_find",
Object {
"query": Object {
"default_search_operator": "AND",
"filter": "alert.attributes.alertTypeId:(foo or bar)",
"page": 1,
"per_page": 10,
"search": "baz",
"search_fields": "[\\"name\\",\\"tags\\"]",
},
},
]
`);
Array [
"/api/alert/_find",
Object {
"query": Object {
"default_search_operator": "AND",
"filter": "alert.attributes.alertTypeId:(foo or bar)",
"page": 1,
"per_page": 10,
"search": "baz",
"search_fields": "[\\"name\\",\\"tags\\"]",
},
},
]
`);
});
test('should call find API with searchText and tagsFilter and typesFilter', async () => {
@ -213,20 +213,20 @@ describe('loadAlerts', () => {
});
expect(result).toEqual(resolvedValue);
expect(http.get.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"/api/alert/_find",
Object {
"query": Object {
"default_search_operator": "AND",
"filter": "alert.attributes.alertTypeId:(foo or bar)",
"page": 1,
"per_page": 10,
"search": "apples, foo, baz",
"search_fields": "[\\"name\\",\\"tags\\"]",
},
},
]
`);
Array [
"/api/alert/_find",
Object {
"query": Object {
"default_search_operator": "AND",
"filter": "alert.attributes.alertTypeId:(foo or bar)",
"page": 1,
"per_page": 10,
"search": "apples, foo, baz",
"search_fields": "[\\"name\\",\\"tags\\"]",
},
},
]
`);
});
});
@ -258,10 +258,17 @@ describe('createAlert', () => {
tags: ['foo'],
enabled: true,
alertTypeId: 'test',
interval: '1m',
schedule: {
interval: '1m',
},
actions: [],
params: {},
throttle: null,
consumer: '',
createdAt: new Date('1970-01-01T00:00:00.000Z'),
updatedAt: new Date('1970-01-01T00:00:00.000Z'),
apiKey: null,
apiKeyOwner: null,
};
const resolvedValue: Alert = {
...alertToCreate,
@ -279,7 +286,7 @@ describe('createAlert', () => {
Array [
"/api/alert",
Object {
"body": "{\\"name\\":\\"test\\",\\"tags\\":[\\"foo\\"],\\"enabled\\":true,\\"alertTypeId\\":\\"test\\",\\"interval\\":\\"1m\\",\\"actions\\":[],\\"params\\":{},\\"throttle\\":null}",
"body": "{\\"name\\":\\"test\\",\\"tags\\":[\\"foo\\"],\\"enabled\\":true,\\"alertTypeId\\":\\"test\\",\\"schedule\\":{\\"interval\\":\\"1m\\"},\\"actions\\":[],\\"params\\":{},\\"throttle\\":null,\\"consumer\\":\\"\\",\\"createdAt\\":\\"1970-01-01T00:00:00.000Z\\",\\"updatedAt\\":\\"1970-01-01T00:00:00.000Z\\",\\"apiKey\\":null,\\"apiKeyOwner\\":null}",
},
]
`);
@ -292,9 +299,16 @@ describe('updateAlert', () => {
throttle: '1m',
name: 'test',
tags: ['foo'],
interval: '1m',
schedule: {
interval: '1m',
},
params: {},
actions: [],
consumer: '',
createdAt: new Date('1970-01-01T00:00:00.000Z'),
updatedAt: new Date('1970-01-01T00:00:00.000Z'),
apiKey: null,
apiKeyOwner: null,
};
const resolvedValue: Alert = {
...alertToUpdate,
@ -314,7 +328,7 @@ describe('updateAlert', () => {
Array [
"/api/alert/123",
Object {
"body": "{\\"throttle\\":\\"1m\\",\\"name\\":\\"test\\",\\"tags\\":[\\"foo\\"],\\"interval\\":\\"1m\\",\\"params\\":{},\\"actions\\":[]}",
"body": "{\\"throttle\\":\\"1m\\",\\"name\\":\\"test\\",\\"tags\\":[\\"foo\\"],\\"schedule\\":{\\"interval\\":\\"1m\\"},\\"params\\":{},\\"actions\\":[],\\"consumer\\":\\"\\",\\"createdAt\\":\\"1970-01-01T00:00:00.000Z\\",\\"updatedAt\\":\\"1970-01-01T00:00:00.000Z\\",\\"apiKey\\":null,\\"apiKeyOwner\\":null}",
},
]
`);

2
x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/lib/alert_api.ts
View file

@ -83,7 +83,7 @@ export async function updateAlert({
id,
}: {
http: HttpSetup;
alert: Pick<AlertWithoutId, 'throttle' | 'name' | 'tags' | 'interval' | 'params' | 'actions'>;
alert: Pick<AlertWithoutId, 'throttle' | 'name' | 'tags' | 'schedule' | 'params' | 'actions'>;
id: string;
}): Promise<Alert> {
return await http.put(`${BASE_ALERT_API_PATH}/${id}`, {

18
x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/sections/alert_add/alert_add.tsx
View file

@ -72,7 +72,7 @@ function validateBaseProperties(alertObject: Alert) {
})
);
}
if (!alertObject.interval) {
if (!(alertObject.schedule && alertObject.schedule.interval)) {
errors.interval.push(
i18n.translate('xpack.triggersActionsUI.sections.alertAdd.error.requiredIntervalText', {
defaultMessage: 'Check interval is required.',
@ -94,7 +94,9 @@ export const AlertAdd = ({ refreshList }: Props) => {
const initialAlert = {
params: {},
alertTypeId: null,
interval: '1m',
schedule: {
interval: '1m',
},
actions: [],
tags: [],
};
@ -148,7 +150,9 @@ export const AlertAdd = ({ refreshList }: Props) => {
value: {
params: {},
alertTypeId: null,
interval: '1m',
schedule: {
interval: '1m',
},
actions: [],
tags: [],
},
@ -674,7 +678,9 @@ export const AlertAdd = ({ refreshList }: Props) => {
const interval =
e.target.value !== '' ? parseInt(e.target.value, 10) : null;
setAlertInterval(interval);
setAlertProperty('interval', `${e.target.value}${alertIntervalUnit}`);
setAlertProperty('schedule', {
interval: `${e.target.value}${alertIntervalUnit}`,
});
}}
/>
</EuiFlexItem>
@ -686,7 +692,9 @@ export const AlertAdd = ({ refreshList }: Props) => {
options={getTimeOptions((alertInterval ? alertInterval : 1).toString())}
onChange={(e: any) => {
setAlertIntervalUnit(e.target.value);
setAlertProperty('interval', `${alertInterval}${e.target.value}`);
setAlertProperty('schedule', {
interval: `${alertInterval}${e.target.value}`,
});
}}
/>
</EuiFlexItem>

8
x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/sections/alerts_list/components/alerts_list.test.tsx
View file

@ -145,7 +145,7 @@ describe('alerts_list component with items', () => {
tags: ['tag1'],
enabled: true,
alertTypeId: 'test_alert_type',
interval: '5d',
schedule: { interval: '5d' },
actions: [],
params: { name: 'test alert type name' },
scheduledTaskId: null,
@ -162,7 +162,7 @@ describe('alerts_list component with items', () => {
tags: ['tag1'],
enabled: true,
alertTypeId: 'test_alert_type',
interval: '5d',
schedule: { interval: '5d' },
actions: [{ id: 'test', group: 'alert', params: { message: 'test' } }],
params: { name: 'test alert type name' },
scheduledTaskId: null,
@ -348,7 +348,7 @@ describe('alerts_list with show only capability', () => {
tags: ['tag1'],
enabled: true,
alertTypeId: 'test_alert_type',
interval: '5d',
schedule: { interval: '5d' },
actions: [],
params: { name: 'test alert type name' },
scheduledTaskId: null,
@ -365,7 +365,7 @@ describe('alerts_list with show only capability', () => {
tags: ['tag1'],
enabled: true,
alertTypeId: 'test_alert_type',
interval: '5d',
schedule: { interval: '5d' },
actions: [{ id: 'test', group: 'alert', params: { message: 'test' } }],
params: { name: 'test alert type name' },
scheduledTaskId: null,

26
x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/types.ts
View file

@ -5,6 +5,8 @@
*/
import { capabilities } from 'ui/capabilities';
import { TypeRegistry } from './application/type_registry';
import { SanitizedAlert as Alert } from '../../../alerting/common';
export { SanitizedAlert as Alert, AlertAction } from '../../../alerting/common';
export type ActionTypeIndex = Record<string, ActionType>;
export type AlertTypeIndex = Record<string, AlertType>;
@ -71,30 +73,6 @@ export interface AlertType {
name: string;
}
export interface AlertAction {
group: string;
id: string;
params: Record<string, any>;
}
export interface Alert {
id: string;
name: string;
tags: string[];
enabled: boolean;
alertTypeId: string;
interval: string;
actions: AlertAction[];
params: Record<string, any>;
scheduledTaskId?: string;
createdBy: string | null;
updatedBy: string | null;
apiKeyOwner?: string;
throttle: string | null;
muteAll: boolean;
mutedInstanceIds: string[];
}
export type AlertWithoutId = Omit<Alert, 'id'>;
export interface AlertTableItem extends Alert {

7
x-pack/plugins/actions/common/index.ts Normal file
View file

@ -0,0 +1,7 @@
/*
* 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.
*/
export * from './types';

11
x-pack/plugins/actions/common/types.ts Normal file
View file

@ -0,0 +1,11 @@
/*
* 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.
*/
export interface ActionType {
id: string;
name: string;
enabled: boolean;
}

3
x-pack/plugins/actions/server/action_type_registry.ts
View file

@ -9,6 +9,7 @@ import { i18n } from '@kbn/i18n';
import { RunContext, TaskManagerSetupContract } from '../../task_manager/server';
import { ExecutorError, TaskRunnerFactory } from './lib';
import { ActionType } from './types';
import { ActionType as CommonActionType } from '../common';
import { ActionsConfigurationUtilities } from './actions_config';
interface ConstructorOptions {
@ -98,7 +99,7 @@ export class ActionTypeRegistry {
/**
* Returns a list of registered action types [{ id, name, enabled }]
*/
public list() {
public list(): CommonActionType[] {
return Array.from(this.actionTypes).map(([actionTypeId, actionType]) => ({
id: actionTypeId,
name: actionType.name,