handle inconsistent rule schedule

elastic · Dec 19, 2024 · 1a0d56f · 1a0d56f
1 parent 07c85dc
commit 1a0d56f
Show file tree

Hide file tree

Showing 10 changed files with 124 additions and 32 deletions.
diff --git a/x-pack/solutions/security/plugins/security_solution/common/detection_engine/constants.ts b/x-pack/solutions/security/plugins/security_solution/common/detection_engine/constants.ts
@@ -7,6 +7,8 @@
 
 import type { Type } from '@kbn/securitysolution-io-ts-alerting-types';
 
+export const DEFAULT_RULE_EXECUTION_LOOKBACK = '1m';
+
 export enum RULE_PREVIEW_INVOCATION_COUNT {
   HOUR = 12,
   DAY = 24,

diff --git a/...olution/public/detection_engine/rule_management/components/rule_details/rule_diff_tab.tsx b/...olution/public/detection_engine/rule_management/components/rule_details/rule_diff_tab.tsx
@@ -86,12 +86,17 @@ const normalizeRule = (originalRule: RuleResponse): RuleResponse => {
     indicate a one-hour duration.
     The same helper is used in the rule editing UI to format "from" before submitting the edits.
     So, after the rule is saved, the "from" property unit/value might differ from what's in the package.
+
+    Skips conversion when `to` isn't equal to "now". Conversion may be not valid in that case.
   */
-  rule.from = formatScheduleStepData({
-    interval: rule.interval,
-    from: getHumanizedDuration(rule.from, rule.interval),
-    to: rule.to,
-  }).from;
+  rule.from =
+    rule.to !== 'now'
+      ? rule.from
+      : formatScheduleStepData({
+          interval: rule.interval,
+          from: getHumanizedDuration(rule.from, rule.interval),
+          to: rule.to,
+        }).from;
 
   /*
     Default "note" to an empty string if it's not present.

diff --git a/...public/detection_engine/rule_management/components/rule_details/rule_schedule_section.tsx b/...public/detection_engine/rule_management/components/rule_details/rule_schedule_section.tsx
@@ -6,7 +6,7 @@
  */
 
 import React from 'react';
-import { EuiDescriptionList, EuiText } from '@elastic/eui';
+import { EuiDescriptionList, EuiFlexGroup, EuiIcon, EuiText, EuiToolTip } from '@elastic/eui';
 import type { EuiDescriptionListProps } from '@elastic/eui';
 import { IntervalAbbrScreenReader } from '../../../../common/components/accessibility';
 import type { RuleResponse } from '../../../../../common/api/detection_engine/model/rule_schema';
@@ -62,18 +62,28 @@ export const RuleScheduleSection = ({
     return null;
   }
 
-  const ruleSectionListItems = [];
-
-  ruleSectionListItems.push(
+  const ruleSectionListItems = [
     {
       title: <span data-test-subj="intervalPropertyTitle">{i18n.INTERVAL_FIELD_LABEL}</span>,
       description: <Interval interval={rule.interval} />,
     },
     {
       title: <span data-test-subj="fromPropertyTitle">{i18n.FROM_FIELD_LABEL}</span>,
-      description: <From from={rule.from} interval={rule.interval} />,
-    }
-  );
+      description:
+        rule.to === 'now' ? (
+          <From from={rule.from} interval={rule.interval} />
+        ) : (
+          <EuiToolTip content={i18n.RULE_LOOKBACK_INCONSISTENCY_WARNING}>
+            <EuiText color="warning">
+              <EuiFlexGroup alignItems="center" gutterSize="s">
+                <From from={rule.from} interval={rule.interval} />
+                <EuiIcon type="warning" />
+              </EuiFlexGroup>
+            </EuiText>
+          </EuiToolTip>
+        ),
+    },
+  ];
 
   return (
     <div data-test-subj="listItemColumnStepRuleDescription">

diff --git a/...ule_management/components/rule_details/three_way_diff/final_edit/fields/rule_schedule.tsx b/...ule_management/components/rule_details/three_way_diff/final_edit/fields/rule_schedule.tsx
@@ -6,12 +6,12 @@
  */
 
 import React from 'react';
-import { parseDuration } from '@kbn/alerting-plugin/common';
+import { DEFAULT_RULE_EXECUTION_LOOKBACK } from '../../../../../../../../common/detection_engine/constants';
 import { type FormSchema, type FormData, UseField } from '../../../../../../../shared_imports';
 import { schema } from '../../../../../../rule_creation_ui/components/step_schedule_rule/schema';
 import type { RuleSchedule } from '../../../../../../../../common/api/detection_engine';
-import { secondsToDurationString } from '../../../../../../../detections/pages/detection_engine/rules/helpers';
 import { ScheduleItemField } from '../../../../../../rule_creation/components/schedule_item_field';
+import { safeHumanizeLookbackDuration } from '../../utils/safe_humanize_lookback';
 
 export const ruleScheduleSchema = {
   interval: schema.interval,
@@ -35,12 +35,12 @@ export function RuleScheduleEdit(): JSX.Element {
 }
 
 export function ruleScheduleDeserializer(defaultValue: FormData) {
-  const lookbackSeconds = parseDuration(defaultValue.rule_schedule.lookback) / 1000;
-  const lookbackHumanized = secondsToDurationString(lookbackSeconds);
-
   return {
     interval: defaultValue.rule_schedule.interval,
-    from: lookbackHumanized,
+    from: safeHumanizeLookbackDuration(
+      defaultValue.rule_schedule.lookback,
+      DEFAULT_RULE_EXECUTION_LOOKBACK
+    ),
   };
 }
 

diff --git a/...ponents/rule_details/three_way_diff/final_readonly/fields/rule_schedule/rule_schedule.tsx b/...ponents/rule_details/three_way_diff/final_readonly/fields/rule_schedule/rule_schedule.tsx
@@ -6,20 +6,23 @@
  */
 
 import React from 'react';
-import { EuiDescriptionList } from '@elastic/eui';
-import { parseDuration } from '@kbn/alerting-plugin/common';
+import { EuiDescriptionList, EuiFlexGroup, EuiIcon, EuiText, EuiToolTip } from '@elastic/eui';
+import { DEFAULT_RULE_EXECUTION_LOOKBACK } from '../../../../../../../../../common/detection_engine/constants';
 import * as i18n from '../../../../translations';
 import type { RuleSchedule } from '../../../../../../../../../common/api/detection_engine';
 import { AccessibleTimeValue } from '../../../../rule_schedule_section';
-import { secondsToDurationString } from '../../../../../../../../detections/pages/detection_engine/rules/helpers';
+import { safeHumanizeLookbackDuration } from '../../../utils/safe_humanize_lookback';
+import { RULE_LOOKBACK_INCONSISTENCY_WARNING } from './translations';
 
 interface RuleScheduleReadOnlyProps {
   ruleSchedule: RuleSchedule;
 }
 
 export function RuleScheduleReadOnly({ ruleSchedule }: RuleScheduleReadOnlyProps) {
-  const lookbackSeconds = parseDuration(ruleSchedule.lookback) / 1000;
-  const lookbackHumanized = secondsToDurationString(lookbackSeconds);
+  const lookbackHumanized = safeHumanizeLookbackDuration(
+    ruleSchedule.lookback,
+    LOOKBACK_FALLBACK_VALUE
+  );
 
   return (
     <EuiDescriptionList
@@ -30,9 +33,25 @@ export function RuleScheduleReadOnly({ ruleSchedule }: RuleScheduleReadOnlyProps
         },
         {
           title: i18n.FROM_FIELD_LABEL,
-          description: <AccessibleTimeValue timeValue={lookbackHumanized} />,
+          description:
+            lookbackHumanized === LOOKBACK_FALLBACK_VALUE ? (
+              <EuiToolTip
+                content={RULE_LOOKBACK_INCONSISTENCY_WARNING(DEFAULT_RULE_EXECUTION_LOOKBACK)}
+              >
+                <EuiText color="warning">
+                  <EuiFlexGroup alignItems="center" gutterSize="s">
+                    <AccessibleTimeValue timeValue={ruleSchedule.lookback} />
+                    <EuiIcon type="warning" />
+                  </EuiFlexGroup>
+                </EuiText>
+              </EuiToolTip>
+            ) : (
+              <AccessibleTimeValue timeValue={lookbackHumanized} />
+            ),
         },
       ]}
     />
   );
 }
+
+const LOOKBACK_FALLBACK_VALUE = '-';
diff --git a/...omponents/rule_details/three_way_diff/final_readonly/fields/rule_schedule/translations.ts b/...omponents/rule_details/three_way_diff/final_readonly/fields/rule_schedule/translations.ts
@@ -0,0 +1,18 @@
+/*
+ * 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 { i18n } from '@kbn/i18n';
+
+export const RULE_LOOKBACK_INCONSISTENCY_WARNING = (defaultValue: string) =>
+  i18n.translate(
+    'xpack.securitySolution.detectionEngine.rules.upgradeRules.ruleSchedule.lookbackInconsistencyWarning',
+    {
+      defaultMessage:
+        'There is an inconsistency in rule schedule configuration. Rule may run with gaps. Default value "{defaultValue}" is suggested for upgrade.',
+      values: { defaultValue },
+    }
+  );
diff --git a/...ne/rule_management/components/rule_details/three_way_diff/utils/safe_humanize_lookback.ts b/...ne/rule_management/components/rule_details/three_way_diff/utils/safe_humanize_lookback.ts
@@ -0,0 +1,28 @@
+/*
+ * 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 { parseDuration } from '@kbn/alerting-plugin/common';
+import { secondsToDurationString } from '../../../../../../detections/pages/detection_engine/rules/helpers';
+
+/**
+ * Tries to transform rule's `lookback` duration into a human normalized form. Returns `fallback`
+ * when transformation fails.
+ *
+ * `lookback` is expected in duration format `{value: number}(s|m|h)` e.g. `1m`, `50s`
+ *
+ * Human normaized form represents a the same value with less numbers when possible.
+ * A number of seconds equal whole minutes or hours get transformed to the biggest unit.
+ */
+export function safeHumanizeLookbackDuration(lookbackDuration: string, fallback: string): string {
+  try {
+    const lookbackSeconds = parseDuration(lookbackDuration) / 1000;
+
+    return secondsToDurationString(lookbackSeconds);
+  } catch {
+    return fallback;
+  }
+}
diff --git a/..._solution/public/detection_engine/rule_management/components/rule_details/translations.ts b/..._solution/public/detection_engine/rule_management/components/rule_details/translations.ts
@@ -419,3 +419,11 @@ export const LUCENE_LANGUAGE_LABEL = i18n.translate(
     defaultMessage: 'Lucene',
   }
 );
+
+export const RULE_LOOKBACK_INCONSISTENCY_WARNING = i18n.translate(
+  'xpack.securitySolution.detectionEngine.ruleDetails.lookbackInconsistencyWarning',
+  {
+    defaultMessage:
+      'There is an inconsistency in rule schedule configuration. Rule may run with gaps. Please edit the rule to resolve it.',
+  }
+);
diff --git a/...rity/plugins/security_solution/server/lib/detection_engine/rule_types/utils/utils.test.ts b/...rity/plugins/security_solution/server/lib/detection_engine/rule_types/utils/utils.test.ts
@@ -593,14 +593,16 @@ describe('utils', () => {
       expect(from).toEqual('now-540s');
     });
 
-    test('should return formatted `from` value from rule schedule fields with no lookback', () => {
-      const from = calculateFromValue('5m', '0m');
-      expect(from).toEqual('now-300s');
-    });
+    describe('when lookback is invalid', () => {
+      test('should return formatted `from` value from rule schedule fields with default lookback', () => {
+        const from = calculateFromValue('5m', '0m');
+        expect(from).toEqual('now-360s');
+      });
 
-    test('should return formatted `from` value from rule schedule fields with invalid moment fields', () => {
-      const from = calculateFromValue('5', '5');
-      expect(from).toEqual('now-0s');
+      test('should return formatted `from` value from rule schedule fields with invalid moment fields with default lookback', () => {
+        const from = calculateFromValue('5', '5');
+        expect(from).toEqual('now-60s');
+      });
     });
   });
 

diff --git a/.../security/plugins/security_solution/server/lib/detection_engine/rule_types/utils/utils.ts b/.../security/plugins/security_solution/server/lib/detection_engine/rule_types/utils/utils.ts
@@ -577,7 +577,7 @@ export const getCatchupTuples = ({
  */
 export const calculateFromValue = (interval: string, lookback: string) => {
   const parsedInterval = parseInterval(interval) ?? moment.duration(0);
-  const parsedFrom = parseInterval(lookback) ?? moment.duration(0);
+  const parsedFrom = parseInterval(lookback) ?? moment.duration(1, 'm');
   const duration = parsedFrom.asSeconds() + parsedInterval.asSeconds();
   return `now-${duration}s`;
 };