From 8d9dec69797b94c3adb8e58a5a09c6867534fa5a Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Tue, 24 Jun 2025 16:41:26 +0200 Subject: [PATCH] Alerting: Do not auto-populate target datasource if it's not selectable (#106912) --- .../unified/rule-editor/formDefaults.test.ts | 83 ++++++++++++++++++- .../unified/rule-editor/formDefaults.ts | 27 +++++- 2 files changed, 103 insertions(+), 7 deletions(-) diff --git a/public/app/features/alerting/unified/rule-editor/formDefaults.test.ts b/public/app/features/alerting/unified/rule-editor/formDefaults.test.ts index 3fe9e3d76cc..88b3fbed717 100644 --- a/public/app/features/alerting/unified/rule-editor/formDefaults.test.ts +++ b/public/app/features/alerting/unified/rule-editor/formDefaults.test.ts @@ -1,5 +1,5 @@ import { GrafanaConfig } from '@grafana/data'; -import { config } from '@grafana/runtime'; +import { config, getDataSourceSrv } from '@grafana/runtime'; import { mockAlertQuery, mockDataSource, mockReduceExpression, mockThresholdExpression } from '../mocks'; import { testWithFeatureToggles } from '../test/test-utils'; @@ -8,6 +8,11 @@ import { Annotation } from '../utils/constants'; import { DataSourceType, getDefaultOrFirstCompatibleDataSource } from '../utils/datasource'; import { MANUAL_ROUTING_KEY, getDefaultQueries } from '../utils/rule-form'; +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + getDataSourceSrv: jest.fn(), +})); + import { formValuesFromQueryParams, getDefaultFormValues, getDefautManualRouting } from './formDefaults'; import { isAlertQueryOfAlertData } from './formProcessing'; @@ -200,21 +205,91 @@ describe('getDefaultFormValues', () => { const grafanaConfig: GrafanaConfig = config; const uaConfig = grafanaConfig.unifiedAlerting; - afterEach(() => { - uaConfig.defaultRecordingRulesTargetDatasourceUID = undefined; + const mockGetInstanceSettings = jest.fn(); + + beforeEach(() => { + (getDataSourceSrv as jest.Mock).mockReturnValue({ + getInstanceSettings: mockGetInstanceSettings, + }); }); - it('should set targetDatasourceUid from config when defaultRecordingRulesTargetDatasourceUID is provided', () => { + afterEach(() => { + uaConfig.defaultRecordingRulesTargetDatasourceUID = undefined; + jest.clearAllMocks(); + }); + + it('should set targetDatasourceUid from config when datasource is valid for recording rules', () => { const expectedDatasourceUid = 'test-datasource-uid'; uaConfig.defaultRecordingRulesTargetDatasourceUID = expectedDatasourceUid; + const validDataSource = mockDataSource({ + uid: expectedDatasourceUid, + type: DataSourceType.Prometheus, + jsonData: { + allowAsRecordingRulesTarget: true, + }, + }); + mockGetInstanceSettings.mockReturnValue(validDataSource); + const result = getDefaultFormValues(); expect(result.targetDatasourceUid).toBe(expectedDatasourceUid); + expect(mockGetInstanceSettings).toHaveBeenCalledWith(expectedDatasourceUid); + }); + + it('should set targetDatasourceUid to undefined when datasource has allowAsRecordingRulesTarget disabled', () => { + const datasourceUid = 'test-datasource-uid'; + uaConfig.defaultRecordingRulesTargetDatasourceUID = datasourceUid; + + const invalidDataSource = mockDataSource({ + uid: datasourceUid, + type: DataSourceType.Prometheus, + jsonData: { + allowAsRecordingRulesTarget: false, + }, + }); + mockGetInstanceSettings.mockReturnValue(invalidDataSource); + + const result = getDefaultFormValues(); + + expect(result.targetDatasourceUid).toBeUndefined(); + expect(mockGetInstanceSettings).toHaveBeenCalledWith(datasourceUid); + }); + + it('should set targetDatasourceUid to undefined when datasource type is not supported', () => { + const datasourceUid = 'test-datasource-uid'; + uaConfig.defaultRecordingRulesTargetDatasourceUID = datasourceUid; + + const nonPrometheusDataSource = mockDataSource({ + uid: datasourceUid, + type: DataSourceType.Loki, + jsonData: { + allowAsRecordingRulesTarget: true, + }, + }); + mockGetInstanceSettings.mockReturnValue(nonPrometheusDataSource); + + const result = getDefaultFormValues(); + + expect(result.targetDatasourceUid).toBeUndefined(); + expect(mockGetInstanceSettings).toHaveBeenCalledWith(datasourceUid); + }); + + it('should set targetDatasourceUid to undefined when datasource does not exist', () => { + const datasourceUid = 'non-existent-datasource-uid'; + uaConfig.defaultRecordingRulesTargetDatasourceUID = datasourceUid; + + mockGetInstanceSettings.mockReturnValue(null); + + const result = getDefaultFormValues(); + + expect(result.targetDatasourceUid).toBeUndefined(); + expect(mockGetInstanceSettings).toHaveBeenCalledWith(datasourceUid); }); it('should set targetDatasourceUid to undefined when defaultRecordingRulesTargetDatasourceUID is not provided', () => { const result = getDefaultFormValues(); expect(result.targetDatasourceUid).toBeUndefined(); + expect(mockGetInstanceSettings).not.toHaveBeenCalled(); }); }); diff --git a/public/app/features/alerting/unified/rule-editor/formDefaults.ts b/public/app/features/alerting/unified/rule-editor/formDefaults.ts index cc271460651..e9eeeec8c71 100644 --- a/public/app/features/alerting/unified/rule-editor/formDefaults.ts +++ b/public/app/features/alerting/unified/rule-editor/formDefaults.ts @@ -1,7 +1,7 @@ import { clamp } from 'lodash'; import { z } from 'zod/v4'; -import { config } from '@grafana/runtime'; +import { config, getDataSourceSrv } from '@grafana/runtime'; import { RuleWithLocation } from 'app/types/unified-alerting'; import { GrafanaAlertStateDecision, RulerRuleDTO } from 'app/types/unified-alerting-dto'; @@ -9,7 +9,7 @@ import { RuleFormType, RuleFormValues } from '../types/rule-form'; // TODO Ideally all of these should be moved here import { getRulesAccess } from '../utils/access-control'; import { defaultAnnotations } from '../utils/constants'; -import { GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; +import { GRAFANA_RULES_SOURCE_NAME, isValidRecordingRulesTarget } from '../utils/datasource'; import { MANUAL_ROUTING_KEY, SIMPLIFIED_QUERY_EDITOR_KEY, @@ -35,6 +35,27 @@ const KEEP_FIRING_FOR_DEFAULT = '0s'; export const DEFAULT_GROUP_EVALUATION_INTERVAL = formatPrometheusDuration( clamp(GROUP_EVALUATION_MIN_INTERVAL_MS, GROUP_EVALUATION_INTERVAL_LOWER_BOUND, GROUP_EVALUATION_INTERVAL_UPPER_BOUND) ); + +function getValidDefaultTargetDatasourceUid(): string | undefined { + const configuredDefaultUid = config.unifiedAlerting?.defaultRecordingRulesTargetDatasourceUID; + + if (!configuredDefaultUid) { + return undefined; + } + + try { + const datasource = getDataSourceSrv().getInstanceSettings(configuredDefaultUid); + if (datasource && isValidRecordingRulesTarget(datasource)) { + return configuredDefaultUid; + } + } catch (error) { + // If datasource doesn't exist or can't be retrieved, + // just return undefined + } + + return undefined; +} + export const getDefaultFormValues = (ruleType?: RuleFormType): RuleFormValues => { const { canCreateGrafanaRules, canCreateCloudRules } = getRulesAccess(); const type = (() => { @@ -75,7 +96,7 @@ export const getDefaultFormValues = (ruleType?: RuleFormType): RuleFormValues => overrideTimings: false, muteTimeIntervals: [], editorSettings: getDefaultEditorSettings(ruleType), - targetDatasourceUid: config.unifiedAlerting?.defaultRecordingRulesTargetDatasourceUID, + targetDatasourceUid: getValidDefaultTargetDatasourceUid(), // cortex / loki namespace: '',