From e74c2390de9081304f7a5e77dcb36b86ecc4a0d8 Mon Sep 17 00:00:00 2001
From: Peter Holmberg <peterholmberg@users.noreply.github.com>
Date: Tue, 19 Jul 2022 10:30:26 +0200
Subject: [PATCH] Alerting: Prevent evaluation if "for" shorter than "evaluate"
 (#51797)

Co-authored-by: Armand Grillet <armand.grillet@outlook.com>
---
 pkg/services/ngalert/CHANGELOG.md             |  1 +
 .../GrafanaConditionEvalWarning.tsx           | 34 -------------------
 .../rule-editor/GrafanaEvaluationBehavior.tsx | 21 +++++++++---
 3 files changed, 17 insertions(+), 39 deletions(-)
 delete mode 100644 public/app/features/alerting/unified/components/rule-editor/GrafanaConditionEvalWarning.tsx

diff --git a/pkg/services/ngalert/CHANGELOG.md b/pkg/services/ngalert/CHANGELOG.md
index a94b240f666..7ec0abe243b 100644
--- a/pkg/services/ngalert/CHANGELOG.md
+++ b/pkg/services/ngalert/CHANGELOG.md
@@ -52,6 +52,7 @@ Scopes must have an order to ensure consistency and ease of search, this helps u
 - [ENHANCEMENT] Scheduler: Drop ticks if rule evaluation is too slow and adds a metric grafana_alerting_schedule_rule_evaluations_missed_total to track missed evaluations per rule #48885
 - [ENHANCEMENT] Ticker to tick at predictable time #50197
 - [ENHANCEMENT] Migration: Don't stop the migration when failing to parse alert rule tags #51253
+- [ENHANCEMENT] Prevent evaluation if "for" shorter than "evaluate" #51797
 
 ## 9.0.0
 
diff --git a/public/app/features/alerting/unified/components/rule-editor/GrafanaConditionEvalWarning.tsx b/public/app/features/alerting/unified/components/rule-editor/GrafanaConditionEvalWarning.tsx
deleted file mode 100644
index 0de1c1d6122..00000000000
--- a/public/app/features/alerting/unified/components/rule-editor/GrafanaConditionEvalWarning.tsx
+++ /dev/null
@@ -1,34 +0,0 @@
-import { isEmpty } from 'lodash';
-import React, { FC } from 'react';
-import { useFormContext } from 'react-hook-form';
-
-import { durationToMilliseconds, parseDuration } from '@grafana/data';
-import { Alert } from '@grafana/ui';
-
-import { RuleFormValues } from '../../types/rule-form';
-
-// a warning that will be shown if a problematic yet technically valid combination of "evaluate every" and "evaluate for" is enetered
-export const GrafanaConditionEvalWarning: FC = () => {
-  const { watch } = useFormContext<RuleFormValues>();
-  const evaluateFor = watch('evaluateFor');
-  const evaluateEvery = watch('evaluateEvery');
-  if (evaluateFor === '0') {
-    return null;
-  }
-  const durationFor = parseDuration(evaluateFor);
-  const durationEvery = parseDuration(evaluateEvery);
-  if (isEmpty(durationFor) || isEmpty(durationEvery)) {
-    return null;
-  }
-  const millisFor = durationToMilliseconds(durationFor);
-  const millisEvery = durationToMilliseconds(durationEvery);
-  if (millisFor && millisEvery && millisFor <= millisEvery) {
-    return (
-      <Alert severity="warning" title="">
-        Setting a &quot;for&quot; duration that is less than or equal to the evaluation interval will result in the
-        evaluation interval being used to calculate when an alert that has stopped receiving data will be closed.
-      </Alert>
-    );
-  }
-  return null;
-};
diff --git a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx
index 056c6f7fcb6..d696ca6a97b 100644
--- a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx
@@ -10,19 +10,26 @@ import { positiveDurationValidationPattern, durationValidationPattern } from '..
 import { CollapseToggle } from '../CollapseToggle';
 
 import { GrafanaAlertStatePicker } from './GrafanaAlertStatePicker';
-import { GrafanaConditionEvalWarning } from './GrafanaConditionEvalWarning';
 import { PreviewRule } from './PreviewRule';
 import { RuleEditorSection } from './RuleEditorSection';
 
 const MIN_TIME_RANGE_STEP_S = 10; // 10 seconds
 
-const forValidationOptions: RegisterOptions = {
+const forValidationOptions = (evaluateEvery: string): RegisterOptions => ({
   required: {
     value: true,
     message: 'Required.',
   },
   pattern: durationValidationPattern,
-};
+  validate: (value) => {
+    const evaluateEveryDuration = parseDuration(evaluateEvery);
+    const forDuration = parseDuration(value);
+    const millisFor = durationToMilliseconds(forDuration);
+    const millisEvery = durationToMilliseconds(evaluateEveryDuration);
+
+    return millisFor >= millisEvery ? true : 'For must be greater than or equal to evaluate every.';
+  },
+});
 
 const evaluateEveryValidationOptions: RegisterOptions = {
   required: {
@@ -51,6 +58,7 @@ export const GrafanaEvaluationBehavior: FC = () => {
   const {
     register,
     formState: { errors },
+    watch,
   } = useFormContext<RuleFormValues>();
 
   const evaluateEveryId = 'eval-every-input';
@@ -85,11 +93,14 @@ export const GrafanaEvaluationBehavior: FC = () => {
             invalid={!!errors.evaluateFor?.message}
             validationMessageHorizontalOverflow={true}
           >
-            <Input id={evaluateForId} width={8} {...register('evaluateFor', forValidationOptions)} />
+            <Input
+              id={evaluateForId}
+              width={8}
+              {...register('evaluateFor', forValidationOptions(watch('evaluateEvery')))}
+            />
           </Field>
         </div>
       </Field>
-      <GrafanaConditionEvalWarning />
       <CollapseToggle
         isCollapsed={!showErrorHandling}
         onToggle={(collapsed) => setShowErrorHandling(!collapsed)}