From 172662af58839d319c9e7cc9f2f09c868facc8db Mon Sep 17 00:00:00 2001
From: Tom Ratcliffe <tom.ratcliffe@grafana.com>
Date: Fri, 30 Aug 2024 14:47:28 +0100
Subject: [PATCH] Alerting: Stop rule state view from crashing when prom rule
 state is missing (#91483)

---
 .../alerting/unified/api/prometheus.ts        | 18 +++++-
 .../rules/RuleListStateView.test.tsx          | 59 +++++++++++++++++++
 .../components/rules/RuleListStateView.tsx    |  5 +-
 3 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100644 public/app/features/alerting/unified/components/rules/RuleListStateView.test.tsx

diff --git a/public/app/features/alerting/unified/api/prometheus.ts b/public/app/features/alerting/unified/api/prometheus.ts
index a18e1aa9294..a2c7faa9382 100644
--- a/public/app/features/alerting/unified/api/prometheus.ts
+++ b/public/app/features/alerting/unified/api/prometheus.ts
@@ -1,9 +1,15 @@
 import { lastValueFrom } from 'rxjs';
 
 import { getBackendSrv } from '@grafana/runtime';
+import { logInfo } from 'app/features/alerting/unified/Analytics';
 import { Matcher } from 'app/plugins/datasource/alertmanager/types';
 import { RuleGroup, RuleIdentifier, RuleNamespace } from 'app/types/unified-alerting';
-import { PromRuleGroupDTO, PromRulesResponse } from 'app/types/unified-alerting-dto';
+import {
+  PromAlertingRuleState,
+  PromRuleGroupDTO,
+  PromRulesResponse,
+  PromRuleType,
+} from 'app/types/unified-alerting-dto';
 
 import { getDatasourceAPIUid, GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource';
 import { isCloudRuleIdentifier, isPrometheusRuleIdentifier } from '../utils/rules';
@@ -88,6 +94,16 @@ export const groupRulesByFileName = (groups: PromRuleGroupDTO[], dataSourceName:
   groups.forEach((group) => {
     group.rules.forEach((rule) => {
       rule.query = rule.query || '';
+      if (rule.type === PromRuleType.Alerting) {
+        // There's a possibility that a custom/unexpected datasource might response with
+        // `type: alerting` but no state
+        // In this case, we fall back to `Inactive` state so that elsewhere in the UI we don't fail/have to handle the edge case
+        // and log a message so we can identify how frequently this might be happening
+        if (!rule.state) {
+          logInfo('prom rule with type=alerting is missing a state', { dataSourceName, ruleName: rule.name });
+          rule.state = PromAlertingRuleState.Inactive;
+        }
+      }
     });
     if (!nsMap[group.file]) {
       nsMap[group.file] = {
diff --git a/public/app/features/alerting/unified/components/rules/RuleListStateView.test.tsx b/public/app/features/alerting/unified/components/rules/RuleListStateView.test.tsx
new file mode 100644
index 00000000000..6aa79128c34
--- /dev/null
+++ b/public/app/features/alerting/unified/components/rules/RuleListStateView.test.tsx
@@ -0,0 +1,59 @@
+import { render, screen } from 'test/test-utils';
+
+import { setPluginExtensionsHook } from '@grafana/runtime';
+import { RuleListStateView } from 'app/features/alerting/unified/components/rules/RuleListStateView';
+import {
+  mockCombinedRule,
+  mockCombinedRuleGroup,
+  mockCombinedRuleNamespace,
+  mockPromAlertingRule,
+} from 'app/features/alerting/unified/mocks';
+import { PromAlertingRuleState } from 'app/types/unified-alerting-dto';
+
+setPluginExtensionsHook(() => ({
+  extensions: [],
+  isLoading: false,
+}));
+
+const namespaces = [
+  mockCombinedRuleNamespace({
+    groups: [
+      mockCombinedRuleGroup('Group with a missing state', [
+        mockCombinedRule({
+          name: 'Rule in firing state',
+          promRule: mockPromAlertingRule({
+            state: PromAlertingRuleState.Firing,
+          }),
+        }),
+        mockCombinedRule({
+          name: 'Rule in pending state',
+          promRule: mockPromAlertingRule({
+            state: PromAlertingRuleState.Pending,
+          }),
+        }),
+        mockCombinedRule({
+          name: 'Rule in inactive state',
+          promRule: mockPromAlertingRule({
+            state: PromAlertingRuleState.Inactive,
+          }),
+        }),
+        mockCombinedRule({
+          name: 'Rule with missing prom state',
+          promRule: mockPromAlertingRule({
+            state: undefined,
+          }),
+        }),
+      ]),
+    ],
+  }),
+];
+
+describe('RuleListStateView', () => {
+  it('renders differing prom rule states correctly and does not crash with missing state', () => {
+    render(<RuleListStateView namespaces={namespaces} />);
+
+    expect(screen.getByText(/firing \(1\)/i)).toBeInTheDocument();
+    expect(screen.getByText(/pending \(1\)/i)).toBeInTheDocument();
+    expect(screen.getByText(/normal \(1\)/i)).toBeInTheDocument();
+  });
+});
diff --git a/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx b/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx
index 04c16b82a06..d09f998d1a7 100644
--- a/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx
+++ b/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx
@@ -28,7 +28,10 @@ export const RuleListStateView = ({ namespaces }: Props) => {
     namespaces.forEach((namespace) =>
       namespace.groups.forEach((group) =>
         group.rules.forEach((rule) => {
-          if (rule.promRule && isAlertingRule(rule.promRule)) {
+          // We might hit edge cases where there type = alerting, but there is no state.
+          // In this case, we shouldn't try to group these alerts in the state view
+          // Even though we handle this at the API layer, this is a last catch point for any edge cases
+          if (rule.promRule && isAlertingRule(rule.promRule) && rule.promRule.state) {
             result[rule.promRule.state].push(rule);
           }
         })