[RAM] Add error for action interval shorter than check interval (#148919

) ## Summary Closes #148569 Adds form error and API error for action intervals shorter than rule check intervals <img width="585" alt="Screen Shot 2023-01-13 at 1 46 55 PM" src="https://user-images.githubusercontent.com/1445834/212406181-059b53e4-a52d-44db-ba03-1a7285c676ce.png"> <img width="592" alt="Screen Shot 2023-01-13 at 1 46 42 PM" src="https://user-images.githubusercontent.com/1445834/212406188-9b59fc05-45c0-4f93-b68c-c1540d46cd2b.png"> ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Kibana Machine <[email protected]>
elastic · Jan 24, 2023 · d60d537 · d60d537
1 parent f3e96bf
commit d60d537
Show file tree

Hide file tree

Showing 8 changed files with 197 additions and 20 deletions.
diff --git a/x-pack/plugins/alerting/server/rules_client/lib/validate_actions.ts b/x-pack/plugins/alerting/server/rules_client/lib/validate_actions.ts
@@ -8,15 +8,16 @@
 import Boom from '@hapi/boom';
 import { map } from 'lodash';
 import { i18n } from '@kbn/i18n';
-import { RawRule } from '../../types';
+import { RawRule, RuleNotifyWhen } from '../../types';
 import { UntypedNormalizedRuleType } from '../../rule_type_registry';
 import { NormalizedAlertAction } from '../types';
 import { RulesClientContext } from '../types';
+import { parseDuration } from '../../lib';
 
 export async function validateActions(
   context: RulesClientContext,
   alertType: UntypedNormalizedRuleType,
-  data: Pick<RawRule, 'notifyWhen' | 'throttle'> & { actions: NormalizedAlertAction[] }
+  data: Pick<RawRule, 'notifyWhen' | 'throttle' | 'schedule'> & { actions: NormalizedAlertAction[] }
 ): Promise<void> {
   const { actions, notifyWhen, throttle } = data;
   const hasRuleLevelNotifyWhen = typeof notifyWhen !== 'undefined';
@@ -91,4 +92,26 @@ export async function validateActions(
       );
     }
   }
+
+  // check for actions throttled shorter than the rule schedule
+  const scheduleInterval = parseDuration(data.schedule.interval);
+  const actionsWithInvalidThrottles = actions.filter(
+    (action) =>
+      action.frequency?.notifyWhen === RuleNotifyWhen.THROTTLE &&
+      parseDuration(action.frequency.throttle!) < scheduleInterval
+  );
+  if (actionsWithInvalidThrottles.length) {
+    throw Boom.badRequest(
+      i18n.translate('xpack.alerting.rulesClient.validateActions.actionsWithInvalidThrottles', {
+        defaultMessage:
+          'Action throttle cannot be shorter than the schedule interval of {scheduleIntervalText}: {groups}',
+        values: {
+          scheduleIntervalText: data.schedule.interval,
+          groups: actionsWithInvalidThrottles
+            .map((a) => `${a.group} (${a.frequency?.throttle})`)
+            .join(', '),
+        },
+      })
+    );
+  }
 }
diff --git a/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts
@@ -2712,7 +2712,10 @@ describe('create()', () => {
     ruleTypeRegistry.get.mockImplementation(() => ({
       id: '123',
       name: 'Test',
-      actionGroups: [{ id: 'default', name: 'Default' }],
+      actionGroups: [
+        { id: 'default', name: 'Default' },
+        { id: 'group2', name: 'Action Group 2' },
+      ],
       recoveryActionGroup: RecoveredActionGroup,
       defaultActionGroupId: 'default',
       minimumLicenseRequired: 'basic',
@@ -2743,7 +2746,7 @@ describe('create()', () => {
           },
         },
         {
-          group: 'default',
+          group: 'group2',
           id: '2',
           params: {
             foo: true,
@@ -2757,7 +2760,7 @@ describe('create()', () => {
       ],
     });
     await expect(rulesClient.create({ data })).rejects.toThrowErrorMatchingInlineSnapshot(
-      `"Cannot specify per-action frequency params when notify_when or throttle are defined at the rule level: default, default"`
+      `"Cannot specify per-action frequency params when notify_when or throttle are defined at the rule level: default, group2"`
     );
     expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled();
     expect(taskManager.schedule).not.toHaveBeenCalled();
@@ -2778,7 +2781,7 @@ describe('create()', () => {
           },
         },
         {
-          group: 'default',
+          group: 'group2',
           id: '2',
           params: {
             foo: true,
@@ -2835,6 +2838,7 @@ describe('create()', () => {
     expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled();
     expect(taskManager.schedule).not.toHaveBeenCalled();
   });
+
   test('throws error when some actions are missing frequency params', async () => {
     rulesClient = new RulesClient({
       ...rulesClientParams,
@@ -2843,7 +2847,10 @@ describe('create()', () => {
     ruleTypeRegistry.get.mockImplementation(() => ({
       id: '123',
       name: 'Test',
-      actionGroups: [{ id: 'default', name: 'Default' }],
+      actionGroups: [
+        { id: 'default', name: 'Default' },
+        { id: 'group2', name: 'Action Group 2' },
+      ],
       recoveryActionGroup: RecoveredActionGroup,
       defaultActionGroupId: 'default',
       minimumLicenseRequired: 'basic',
@@ -2874,17 +2881,94 @@ describe('create()', () => {
             throttle: null,
           },
         },
+        {
+          group: 'group2',
+          id: '2',
+          params: {
+            foo: true,
+          },
+        },
+      ],
+    });
+    await expect(rulesClient.create({ data })).rejects.toThrowErrorMatchingInlineSnapshot(
+      `"Actions missing frequency parameters: group2"`
+    );
+    expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled();
+    expect(taskManager.schedule).not.toHaveBeenCalled();
+  });
+
+  test('throws error when some actions have throttle intervals shorter than the check interval', async () => {
+    rulesClient = new RulesClient({
+      ...rulesClientParams,
+      minimumScheduleInterval: { value: '1m', enforce: true },
+    });
+    ruleTypeRegistry.get.mockImplementation(() => ({
+      id: '123',
+      name: 'Test',
+      actionGroups: [
+        { id: 'default', name: 'Default' },
+        { id: 'group2', name: 'Action Group 2' },
+        { id: 'group3', name: 'Action Group 3' },
+      ],
+      recoveryActionGroup: RecoveredActionGroup,
+      defaultActionGroupId: 'default',
+      minimumLicenseRequired: 'basic',
+      isExportable: true,
+      async executor() {
+        return { state: {} };
+      },
+      producer: 'alerts',
+      useSavedObjectReferences: {
+        extractReferences: jest.fn(),
+        injectReferences: jest.fn(),
+      },
+    }));
+
+    const data = getMockData({
+      notifyWhen: undefined,
+      throttle: undefined,
+      schedule: { interval: '3h' },
+      actions: [
         {
           group: 'default',
+          id: '1',
+          params: {
+            foo: true,
+          },
+          frequency: {
+            summary: false,
+            notifyWhen: 'onThrottleInterval',
+            throttle: '1h',
+          },
+        },
+        {
+          group: 'group2',
           id: '2',
           params: {
             foo: true,
           },
+          frequency: {
+            summary: false,
+            notifyWhen: 'onThrottleInterval',
+            throttle: '3m',
+          },
+        },
+        {
+          group: 'group3',
+          id: '3',
+          params: {
+            foo: true,
+          },
+          frequency: {
+            summary: false,
+            notifyWhen: 'onThrottleInterval',
+            throttle: '240m',
+          },
         },
       ],
     });
     await expect(rulesClient.create({ data })).rejects.toThrowErrorMatchingInlineSnapshot(
-      `"Actions missing frequency parameters: default"`
+      `"Action throttle cannot be shorter than the schedule interval of 3h: default (1h), group2 (3m)"`
     );
     expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled();
     expect(taskManager.schedule).not.toHaveBeenCalled();

diff --git a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts
@@ -1576,7 +1576,10 @@ describe('update()', () => {
       ruleTypeRegistry.get.mockReturnValueOnce({
         id: '123',
         name: 'Test',
-        actionGroups: [{ id: 'default', name: 'Default' }],
+        actionGroups: [
+          { id: 'default', name: 'Default' },
+          { id: 'group2', name: 'Action Group 2' },
+        ],
         defaultActionGroupId: 'default',
         minimumLicenseRequired: 'basic',
         isExportable: true,
@@ -1749,7 +1752,7 @@ describe('update()', () => {
                 },
               },
               {
-                group: 'default',
+                group: 'group2',
                 id: '2',
                 params: {
                   foo: true,
@@ -1764,7 +1767,7 @@ describe('update()', () => {
           },
         })
       ).rejects.toThrowErrorMatchingInlineSnapshot(
-        `"Cannot specify per-action frequency params when notify_when or throttle are defined at the rule level: default, default"`
+        `"Cannot specify per-action frequency params when notify_when or throttle are defined at the rule level: default, group2"`
       );
       expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled();
       expect(taskManager.schedule).not.toHaveBeenCalled();

diff --git a/...ins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx b/...ins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx
@@ -65,6 +65,7 @@ export interface ActionAccordionFormProps {
   isActionGroupDisabledForActionType?: (actionGroupId: string, actionTypeId: string) => boolean;
   hideActionHeader?: boolean;
   hideNotifyWhen?: boolean;
+  minimumThrottleInterval?: [number | undefined, string];
 }
 
 interface ActiveActionConnectorState {
@@ -91,6 +92,7 @@ export const ActionForm = ({
   isActionGroupDisabledForActionType,
   hideActionHeader,
   hideNotifyWhen,
+  minimumThrottleInterval,
 }: ActionAccordionFormProps) => {
   const {
     http,
@@ -396,6 +398,7 @@ export const ActionForm = ({
                 setActiveActionItem(undefined);
               }}
               hideNotifyWhen={hideNotifyWhen}
+              minimumThrottleInterval={minimumThrottleInterval}
             />
           );
         })}

diff --git a/...ggers_actions_ui/public/application/sections/action_connector_form/action_notify_when.tsx b/...ggers_actions_ui/public/application/sections/action_connector_form/action_notify_when.tsx
@@ -122,6 +122,8 @@ interface RuleNotifyWhenProps {
   throttleUnit: string;
   onNotifyWhenChange: (notifyWhen: RuleNotifyWhenType) => void;
   onThrottleChange: (throttle: number | null, throttleUnit: string) => void;
+  showMinimumThrottleWarning?: boolean;
+  showMinimumThrottleUnitWarning?: boolean;
 }
 
 export const ActionNotifyWhen = ({
@@ -130,6 +132,8 @@ export const ActionNotifyWhen = ({
   throttleUnit,
   onNotifyWhenChange,
   onThrottleChange,
+  showMinimumThrottleWarning,
+  showMinimumThrottleUnitWarning,
 }: RuleNotifyWhenProps) => {
   const [showCustomThrottleOpts, setShowCustomThrottleOpts] = useState<boolean>(false);
   const [notifyWhenValue, setNotifyWhenValue] =
@@ -195,6 +199,7 @@ export const ActionNotifyWhen = ({
                 <EuiFlexGroup gutterSize="s">
                   <EuiFlexItem grow={2}>
                     <EuiFieldNumber
+                      isInvalid={showMinimumThrottleWarning}
                       min={1}
                       value={throttle ?? 1}
                       name="throttle"
@@ -220,6 +225,7 @@ export const ActionNotifyWhen = ({
                   </EuiFlexItem>
                   <EuiFlexItem grow={3}>
                     <EuiSelect
+                      isInvalid={showMinimumThrottleUnitWarning}
                       data-test-subj="throttleUnitInput"
                       value={throttleUnit}
                       options={getTimeOptions(throttle ?? 1)}
@@ -230,6 +236,20 @@ export const ActionNotifyWhen = ({
                   </EuiFlexItem>
                 </EuiFlexGroup>
               </EuiFormRow>
+              {(showMinimumThrottleWarning || showMinimumThrottleUnitWarning) && (
+                <>
+                  <EuiSpacer size="xs" />
+                  <EuiText size="xs" color="danger">
+                    {i18n.translate(
+                      'xpack.triggersActionsUI.sections.actionTypeForm.notifyWhenThrottleWarning',
+                      {
+                        defaultMessage:
+                          "Custom action intervals cannot be shorter than the rule's check interval",
+                      }
+                    )}
+                  </EuiText>
+                </>
+              )}
             </>
           )}
         </EuiFlexItem>