[Alert Summaries] [BE] Move “Notify When” and throttle from rule to a…

…ction (elastic#144130) * Add frequency props to actions and validate them on edit/create * Nullify undefined throttle * Update schema to allow for frequency param on actions * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * Commit missing file * Fix types * [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' * Fix types * Fix jest * Fix validating global freq params * Add tests for create and edit * Reset legacy api * Make notify and throttle optional in route schemas * Fix tests * Split missing frequency test cases Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit 0c864fe)
kibanamachine · Nov 16, 2022 · 205a43c · 205a43c
1 parent a9f7ba6
commit 205a43c
Show file tree

Hide file tree

Showing 19 changed files with 533 additions and 45 deletions.
diff --git a/x-pack/plugins/alerting/common/alert_summary.ts b/x-pack/plugins/alerting/common/alert_summary.ts
@@ -20,7 +20,7 @@ export interface AlertSummary {
   ruleTypeId: string;
   consumer: string;
   muteAll: boolean;
-  throttle: string | null;
+  throttle?: string | null;
   enabled: boolean;
   statusStartDate: string;
   statusEndDate: string;

diff --git a/x-pack/plugins/alerting/common/rule.ts b/x-pack/plugins/alerting/common/rule.ts
@@ -75,6 +75,11 @@ export interface RuleAction {
   id: string;
   actionTypeId: string;
   params: RuleActionParams;
+  frequency?: {
+    summary: boolean;
+    notifyWhen: RuleNotifyWhenType;
+    throttle: string | null;
+  };
 }
 
 export interface RuleAggregations {
@@ -123,9 +128,9 @@ export interface Rule<Params extends RuleTypeParams = never> {
   updatedAt: Date;
   apiKey: string | null;
   apiKeyOwner: string | null;
-  throttle: string | null;
+  throttle?: string | null;
   muteAll: boolean;
-  notifyWhen: RuleNotifyWhenType | null;
+  notifyWhen?: RuleNotifyWhenType | null;
   mutedInstanceIds: string[];
   executionStatus: RuleExecutionStatus;
   monitoring?: RuleMonitoring;

diff --git a/x-pack/plugins/alerting/common/rule_notify_when_type.ts b/x-pack/plugins/alerting/common/rule_notify_when_type.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-const RuleNotifyWhenTypeValues = [
+export const RuleNotifyWhenTypeValues = [
   'onActionGroupChange',
   'onActiveAlert',
   'onThrottleInterval',

diff --git a/x-pack/plugins/alerting/server/lib/alert_summary_from_event_log.ts b/x-pack/plugins/alerting/server/lib/alert_summary_from_event_log.ts
@@ -31,7 +31,7 @@ export function alertSummaryFromEventLog(params: AlertSummaryFromEventLogParams)
     statusEndDate: dateEnd,
     status: 'OK',
     muteAll: rule.muteAll,
-    throttle: rule.throttle,
+    throttle: rule.throttle ?? null,
     enabled: rule.enabled,
     lastRun: undefined,
     errorMessages: [],

diff --git a/x-pack/plugins/alerting/server/routes/create_rule.ts b/x-pack/plugins/alerting/server/routes/create_rule.ts
@@ -11,9 +11,11 @@ import { CreateOptions } from '../rules_client';
 import {
   RewriteRequestCase,
   RewriteResponseCase,
+  rewriteActions,
   handleDisabledApiKeysError,
   verifyAccessAndContext,
   countUsageOfPredefinedIds,
+  actionsSchema,
   rewriteRuleLastRun,
 } from './lib';
 import {
@@ -31,20 +33,13 @@ export const bodySchema = schema.object({
   enabled: schema.boolean({ defaultValue: true }),
   consumer: schema.string(),
   tags: schema.arrayOf(schema.string(), { defaultValue: [] }),
-  throttle: schema.nullable(schema.string({ validate: validateDurationSchema })),
+  throttle: schema.nullable(schema.maybe(schema.string({ validate: validateDurationSchema }))),
   params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
   schedule: schema.object({
     interval: schema.string({ validate: validateDurationSchema }),
   }),
-  actions: schema.arrayOf(
-    schema.object({
-      group: schema.string(),
-      id: schema.string(),
-      params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
-    }),
-    { defaultValue: [] }
-  ),
-  notify_when: schema.string({ validate: validateNotifyWhenType }),
+  actions: actionsSchema,
+  notify_when: schema.maybe(schema.string({ validate: validateNotifyWhenType })),
 });
 
 const rewriteBodyReq: RewriteRequestCase<CreateOptions<RuleTypeParams>['data']> = ({
@@ -56,6 +51,7 @@ const rewriteBodyReq: RewriteRequestCase<CreateOptions<RuleTypeParams>['data']>
   alertTypeId,
   notifyWhen,
 });
+
 const rewriteBodyRes: RewriteResponseCase<SanitizedRule<RuleTypeParams>> = ({
   actions,
   alertTypeId,
@@ -132,6 +128,7 @@ export const createRuleRoute = ({ router, licenseState, usageCounter }: RouteOpt
               await rulesClient.create<RuleTypeParams>({
                 data: rewriteBodyReq({
                   ...rule,
+                  actions: rewriteActions(rule.actions),
                   notify_when: rule.notify_when as RuleNotifyWhenType,
                 }),
                 options: { id: params?.id },

diff --git a/x-pack/plugins/alerting/server/routes/lib/actions_schema.ts b/x-pack/plugins/alerting/server/routes/lib/actions_schema.ts
@@ -0,0 +1,29 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { schema } from '@kbn/config-schema';
+import { validateDurationSchema } from '../../lib';
+
+export const actionsSchema = schema.arrayOf(
+  schema.object({
+    group: schema.string(),
+    id: schema.string(),
+    params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
+    frequency: schema.maybe(
+      schema.object({
+        summary: schema.boolean(),
+        notify_when: schema.oneOf([
+          schema.literal('onActionGroupChange'),
+          schema.literal('onActiveAlert'),
+          schema.literal('onThrottleInterval'),
+        ]),
+        throttle: schema.nullable(schema.string({ validate: validateDurationSchema })),
+      })
+    ),
+  }),
+  { defaultValue: [] }
+);
diff --git a/x-pack/plugins/alerting/server/routes/lib/index.ts b/x-pack/plugins/alerting/server/routes/lib/index.ts
@@ -18,5 +18,7 @@ export type {
 } from './rewrite_request_case';
 export { verifyAccessAndContext } from './verify_access_and_context';
 export { countUsageOfPredefinedIds } from './count_usage_of_predefined_ids';
+export { rewriteActions } from './rewrite_actions';
+export { actionsSchema } from './actions_schema';
 export { rewriteRule, rewriteRuleLastRun } from './rewrite_rule';
 export { rewriteNamespaces } from './rewrite_namespaces';
diff --git a/x-pack/plugins/alerting/server/routes/lib/rewrite_actions.ts b/x-pack/plugins/alerting/server/routes/lib/rewrite_actions.ts
@@ -0,0 +1,32 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+import { CamelToSnake, RewriteRequestCase } from './rewrite_request_case';
+import { RuleAction } from '../../types';
+
+type ReqRuleAction = Omit<RuleAction, 'actionTypeId' | 'frequency'> & {
+  frequency?: {
+    [K in keyof NonNullable<RuleAction['frequency']> as CamelToSnake<K>]: NonNullable<
+      RuleAction['frequency']
+    >[K];
+  };
+};
+export const rewriteActions: (
+  actions?: ReqRuleAction[]
+) => Array<Omit<RuleAction, 'actionTypeId'>> = (actions) => {
+  const rewriteFrequency: RewriteRequestCase<NonNullable<RuleAction['frequency']>> = ({
+    notify_when: notifyWhen,
+    ...rest
+  }) => ({ ...rest, notifyWhen });
+  if (!actions) return [];
+  return actions.map(
+    (action) =>
+      ({
+        ...action,
+        ...(action.frequency ? { frequency: rewriteFrequency(action.frequency) } : {}),
+      } as RuleAction)
+  );
+};
diff --git a/x-pack/plugins/alerting/server/routes/lib/rewrite_request_case.ts b/x-pack/plugins/alerting/server/routes/lib/rewrite_request_case.ts
@@ -71,7 +71,7 @@ export type RewriteResponseCase<T> = (
  *
  * For more details see this PR comment: https://github.com/microsoft/TypeScript/pull/40336#issuecomment-686723087
  */
-type CamelToSnake<T extends string> = string extends T
+export type CamelToSnake<T extends string> = string extends T
   ? string
   : T extends `${infer C0}${infer C1}${infer R}`
   ? `${C0 extends Uppercase<C0> ? '_' : ''}${Lowercase<C0>}${C1 extends Uppercase<C1>

diff --git a/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts b/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts
@@ -56,11 +56,19 @@ export const rewriteRule = ({
     last_execution_date: executionStatus.lastExecutionDate,
     last_duration: executionStatus.lastDuration,
   },
-  actions: actions.map(({ group, id, actionTypeId, params }) => ({
+  actions: actions.map(({ group, id, actionTypeId, params, frequency }) => ({
     group,
     id,
     params,
     connector_type_id: actionTypeId,
+    ...(frequency
+      ? {
+          frequency: {
+            ...frequency,
+            notify_when: frequency.notifyWhen,
+          },
+        }
+      : {}),
   })),
   ...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}),
   ...(nextRun ? { next_run: nextRun } : {}),

diff --git a/x-pack/plugins/alerting/server/routes/update_rule.ts b/x-pack/plugins/alerting/server/routes/update_rule.ts
@@ -15,6 +15,8 @@ import {
   RewriteResponseCase,
   RewriteRequestCase,
   handleDisabledApiKeysError,
+  rewriteActions,
+  actionsSchema,
   rewriteRuleLastRun,
 } from './lib';
 import {
@@ -35,17 +37,10 @@ const bodySchema = schema.object({
   schedule: schema.object({
     interval: schema.string({ validate: validateDurationSchema }),
   }),
-  throttle: schema.nullable(schema.string({ validate: validateDurationSchema })),
+  throttle: schema.nullable(schema.maybe(schema.string({ validate: validateDurationSchema }))),
   params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
-  actions: schema.arrayOf(
-    schema.object({
-      group: schema.string(),
-      id: schema.string(),
-      params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
-    }),
-    { defaultValue: [] }
-  ),
-  notify_when: schema.string({ validate: validateNotifyWhenType }),
+  actions: actionsSchema,
+  notify_when: schema.maybe(schema.string({ validate: validateNotifyWhenType })),
 });
 
 const rewriteBodyReq: RewriteRequestCase<UpdateOptions<RuleTypeParams>> = (result) => {
@@ -137,6 +132,7 @@ export const updateRuleRoute = (
                 id,
                 data: {
                   ...rule,
+                  actions: rewriteActions(rule.actions),
                   notify_when: rule.notify_when as RuleNotifyWhenType,
                 },
               })

diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts
@@ -396,8 +396,8 @@ export interface UpdateOptions<Params extends RuleTypeParams> {
     schedule: IntervalSchedule;
     actions: NormalizedAlertAction[];
     params: Params;
-    throttle: string | null;
-    notifyWhen: RuleNotifyWhenType | null;
+    throttle?: string | null;
+    notifyWhen?: RuleNotifyWhenType | null;
   };
 }
 
@@ -691,7 +691,7 @@ export class RulesClient {
       throw Boom.badRequest(`Error creating rule: could not create API key - ${error.message}`);
     }
 
-    await this.validateActions(ruleType, data.actions);
+    await this.validateActions(ruleType, data);
 
     // Throw error if schedule interval is less than the minimum and we are enforcing it
     const intervalInMs = parseDuration(data.schedule.interval);
@@ -711,7 +711,8 @@ export class RulesClient {
     const createTime = Date.now();
     const lastRunTimestamp = new Date();
     const legacyId = Semver.lt(this.kibanaVersion, '8.0.0') ? id : null;
-    const notifyWhen = getRuleNotifyWhenType(data.notifyWhen, data.throttle);
+    const notifyWhen = getRuleNotifyWhenType(data.notifyWhen ?? null, data.throttle ?? null);
+    const throttle = data.throttle ?? null;
 
     const rawRule: RawRule = {
       ...data,
@@ -727,6 +728,7 @@ export class RulesClient {
       muteAll: false,
       mutedInstanceIds: [],
       notifyWhen,
+      throttle,
       executionStatus: getRuleExecutionStatusPending(lastRunTimestamp.toISOString()),
       monitoring: getDefaultMonitoring(lastRunTimestamp.toISOString()),
     };
@@ -1870,7 +1872,7 @@ export class RulesClient {
 
     // Validate
     const validatedAlertTypeParams = validateRuleTypeParams(data.params, ruleType.validate?.params);
-    await this.validateActions(ruleType, data.actions);
+    await this.validateActions(ruleType, data);
 
     // Throw error if schedule interval is less than the minimum and we are enforcing it
     const intervalInMs = parseDuration(data.schedule.interval);
@@ -1899,7 +1901,7 @@ export class RulesClient {
     }
 
     const apiKeyAttributes = this.apiKeyAsAlertAttributes(createdAPIKey, username);
-    const notifyWhen = getRuleNotifyWhenType(data.notifyWhen, data.throttle);
+    const notifyWhen = getRuleNotifyWhenType(data.notifyWhen ?? null, data.throttle ?? null);
 
     let updatedObject: SavedObject<RawRule>;
     const createAttributes = this.updateMeta({
@@ -2429,7 +2431,7 @@ export class RulesClient {
             for (const operation of operations) {
               switch (operation.field) {
                 case 'actions':
-                  await this.validateActions(ruleType, operation.value);
+                  await this.validateActions(ruleType, { ...attributes, actions: operation.value });
                   ruleActions = applyBulkEditOperation(operation, ruleActions);
                   break;
                 case 'snoozeSchedule':
@@ -2539,7 +2541,7 @@ export class RulesClient {
 
             // get notifyWhen
             const notifyWhen = getRuleNotifyWhenType(
-              attributes.notifyWhen,
+              attributes.notifyWhen ?? null,
               attributes.throttle ?? null
             );
 
@@ -3904,8 +3906,23 @@ export class RulesClient {
 
   private async validateActions(
     alertType: UntypedNormalizedRuleType,
-    actions: NormalizedAlertAction[]
+    data: Pick<RawRule, 'notifyWhen' | 'throttle'> & { actions: NormalizedAlertAction[] }
   ): Promise<void> {
+    const { actions, notifyWhen, throttle } = data;
+    const hasNotifyWhen = typeof notifyWhen !== 'undefined';
+    const hasThrottle = typeof throttle !== 'undefined';
+    let usesRuleLevelFreqParams;
+    if (hasNotifyWhen && hasThrottle) usesRuleLevelFreqParams = true;
+    else if (!hasNotifyWhen && !hasThrottle) usesRuleLevelFreqParams = false;
+    else {
+      throw Boom.badRequest(
+        i18n.translate('xpack.alerting.rulesClient.usesValidGlobalFreqParams.oneUndefined', {
+          defaultMessage:
+            'Rule-level notifyWhen and throttle must both be defined or both be undefined',
+        })
+      );
+    }
+
     if (actions.length === 0) {
       return;
     }
@@ -3948,6 +3965,34 @@ export class RulesClient {
         })
       );
     }
+
+    // check for actions using frequency params if the rule has rule-level frequency params defined
+    if (usesRuleLevelFreqParams) {
+      const actionsWithFrequency = actions.filter((action) => Boolean(action.frequency));
+      if (actionsWithFrequency.length) {
+        throw Boom.badRequest(
+          i18n.translate('xpack.alerting.rulesClient.validateActions.mixAndMatchFreqParams', {
+            defaultMessage:
+              'Cannot specify per-action frequency params when notify_when and throttle are defined at the rule level: {groups}',
+            values: {
+              groups: actionsWithFrequency.map((a) => a.group).join(', '),
+            },
+          })
+        );
+      }
+    } else {
+      const actionsWithoutFrequency = actions.filter((action) => !action.frequency);
+      if (actionsWithoutFrequency.length) {
+        throw Boom.badRequest(
+          i18n.translate('xpack.alerting.rulesClient.validateActions.notAllActionsWithFreq', {
+            defaultMessage: 'Actions missing frequency parameters: {groups}',
+            values: {
+              groups: actionsWithoutFrequency.map((a) => a.group).join(', '),
+            },
+          })
+        );
+      }
+    }
   }
 
   private async extractReferences<