From 6746207c367f35b673c33593a8285f2cad65ec0e Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Thu, 6 Nov 2025 16:43:42 +0100 Subject: [PATCH] Alerting: Fix url rule form parsing (#113513) Fix alertRuleFormSchema to properly handle query model objects --- .../unified/rule-editor/formDefaults.test.ts | 284 +++++++++++++++++- .../alerting/alertRuleFormSchema.test.ts | 168 +++++++++++ .../alerting/alertRuleFormSchema.ts | 70 +---- 3 files changed, 457 insertions(+), 65 deletions(-) create mode 100644 public/app/features/plugins/components/restrictedGrafanaApis/alerting/alertRuleFormSchema.test.ts 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 9bfe9d4e593..9c86955bbfb 100644 --- a/public/app/features/alerting/unified/rule-editor/formDefaults.test.ts +++ b/public/app/features/alerting/unified/rule-editor/formDefaults.test.ts @@ -13,8 +13,13 @@ jest.mock('@grafana/runtime', () => ({ getDataSourceSrv: jest.fn(), })); -import { formValuesFromQueryParams, getDefaultFormValues, getDefautManualRouting } from './formDefaults'; -import { isAlertQueryOfAlertData } from './formProcessing'; +import { + formValuesFromPrefill, + formValuesFromQueryParams, + getDefaultFormValues, + getDefautManualRouting, +} from './formDefaults'; +import { isAlertQueryOfAlertData, isExpressionQueryInAlert } from './formProcessing'; jest.mock('../utils/datasource', () => ({ ...jest.requireActual('../utils/datasource'), @@ -292,3 +297,278 @@ describe('getDefaultFormValues', () => { expect(mockGetInstanceSettings).not.toHaveBeenCalled(); }); }); + +describe('formValuesFromPrefill', () => { + it('should preserve threshold expression query structure', () => { + const prefillData = { + folder: { uid: 'test-folder', title: 'Test Folder' }, + group: 'test-group', + queries: [ + { + refId: 'A', + datasourceUid: 'gdev-prometheus', + queryType: '', + relativeTimeRange: { from: 600, to: 0 }, + model: { + datasource: { type: 'prometheus', uid: 'gdev-prometheus' }, + editorMode: 'code', + exemplar: false, + expr: 'sum by (handler) (rate(grafana_http_request_duration_seconds_count[5h]))', + format: 'time_series', + instant: true, + intervalMs: 1000, + legendFormat: '__auto', + maxDataPoints: 43200, + range: false, + refId: 'A', + }, + }, + { + refId: 'C', + datasourceUid: '__expr__', + queryType: '', + relativeTimeRange: { from: 600, to: 0 }, + model: { + conditions: [ + { + evaluator: { params: [0.0001, 0], type: 'gt' }, + operator: { type: 'and' }, + query: { params: [] }, + reducer: { params: [], type: 'avg' }, + type: 'query', + }, + ], + datasource: { name: 'Expression', type: '__expr__', uid: '__expr__' }, + expression: 'A', + intervalMs: 1000, + maxDataPoints: 43200, + refId: 'C', + type: 'threshold', + }, + }, + ], + }; + + const result = formValuesFromPrefill(prefillData); + + const queryC = result.queries.find((q) => q.refId === 'C'); + expect(queryC?.model).toHaveProperty('type', 'threshold'); + expect(queryC?.model).toHaveProperty('conditions'); + expect(queryC?.model).toHaveProperty('expression', 'A'); + expect(queryC?.model.datasource).toEqual({ name: 'Expression', type: '__expr__', uid: '__expr__' }); + + // Should NOT have these defaults added + expect(queryC?.model).not.toHaveProperty('instant'); + expect(queryC?.model).not.toHaveProperty('range'); + }); + + it('should preserve reduce expression query structure', () => { + const prefillData = { + queries: [ + { + refId: 'B', + datasourceUid: '-100', + queryType: '', + relativeTimeRange: { from: 0, to: 0 }, + model: { + expression: 'A', + intervalMs: 1000, + maxDataPoints: 100, + reducer: 'mean', + refId: 'B', + type: 'reduce', + }, + }, + ], + }; + + const result = formValuesFromPrefill(prefillData); + const query = result.queries[0]; + + expect(query.model).toHaveProperty('type', 'reduce'); + expect(query.model).toHaveProperty('reducer', 'mean'); + expect(query.model).toHaveProperty('expression', 'A'); + expect(query.model).not.toHaveProperty('instant'); + expect(query.model).not.toHaveProperty('range'); + }); + + it('should preserve math expression query structure', () => { + const prefillData = { + queries: [ + { + refId: 'C', + datasourceUid: '-100', + queryType: '', + relativeTimeRange: { from: 0, to: 0 }, + model: { + conditions: [ + { + evaluator: { params: [0, 0], type: 'gt' }, + operator: { type: 'and' }, + query: { params: ['B'] }, + reducer: { params: [], type: 'avg' }, + type: 'query', + }, + ], + datasource: { name: 'Expression', type: '__expr__', uid: '__expr__' }, + expression: '$B > 0.4', + intervalMs: 1000, + maxDataPoints: 43200, + refId: 'C', + type: 'math', + }, + }, + ], + }; + + const result = formValuesFromPrefill(prefillData); + const query = result.queries[0]; + + expect(query.model).toHaveProperty('type', 'math'); + expect(query.model).toHaveProperty('expression', '$B > 0.4'); + expect(query.model).toHaveProperty('conditions'); + }); + + it('should preserve classic_conditions expression query structure', () => { + const prefillData = { + queries: [ + { + refId: 'B', + datasourceUid: '-100', + queryType: '', + relativeTimeRange: { from: 0, to: 0 }, + model: { + conditions: [ + { + evaluator: { params: [10], type: 'gt' }, + operator: { type: 'and' }, + query: { params: ['A'] }, + reducer: { params: [], type: 'last' }, + type: 'query', + }, + { + evaluator: { params: [5, 15], type: 'within_range' }, + operator: { type: 'and' }, + query: { params: ['A'] }, + reducer: { params: [], type: 'avg' }, + type: 'query', + }, + ], + datasource: { type: '__expr__', uid: '-100' }, + expression: 'A', + intervalMs: 1000, + maxDataPoints: 43200, + refId: 'B', + type: 'classic_conditions', + }, + }, + ], + }; + + const result = formValuesFromPrefill(prefillData); + const [query] = result.queries.filter(isExpressionQueryInAlert); + + expect(query.model).toHaveProperty('type', 'classic_conditions'); + expect(query.model.conditions).toHaveLength(2); + expect(query.model.conditions?.[0].evaluator.type).toBe('gt'); + expect(query.model.conditions?.[1].evaluator.type).toBe('within_range'); + }); + + it('should preserve resample expression query structure', () => { + const prefillData = { + queries: [ + { + refId: 'D', + datasourceUid: '-100', + queryType: '', + relativeTimeRange: { from: 600, to: 0 }, + model: { + conditions: [ + { + evaluator: { params: [0, 0], type: 'gt' }, + operator: { type: 'and' }, + query: { params: [] }, + reducer: { params: [], type: 'avg' }, + type: 'query', + }, + ], + datasource: { name: 'Expression', type: '__expr__', uid: '__expr__' }, + downsampler: 'min', + expression: 'A', + intervalMs: 1000, + maxDataPoints: 43200, + refId: 'D', + type: 'resample', + upsampler: 'backfilling', + window: '2m', + }, + }, + ], + }; + + const result = formValuesFromPrefill(prefillData); + const query = result.queries[0]; + + expect(query.model).toHaveProperty('type', 'resample'); + expect(query.model).toHaveProperty('downsampler', 'min'); + expect(query.model).toHaveProperty('upsampler', 'backfilling'); + expect(query.model).toHaveProperty('window', '2m'); + }); + + it('should preserve Prometheus query fields', () => { + const prefillData = { + queries: [ + { + refId: 'A', + datasourceUid: 'gdev-prometheus', + queryType: '', + relativeTimeRange: { from: 600, to: 0 }, + model: { + datasource: { type: 'prometheus', uid: 'gdev-prometheus' }, + editorMode: 'code', + exemplar: false, + expr: 'rate(promhttp_metric_handler_requests_total{}[15m])', + instant: true, + intervalMs: 1000, + legendFormat: '__auto', + maxDataPoints: 43200, + range: false, + refId: 'A', + }, + }, + ], + }; + + const result = formValuesFromPrefill(prefillData); + const [query] = result.queries.filter(isAlertQueryOfAlertData); + + expect(query.model).toHaveProperty('expr', 'rate(promhttp_metric_handler_requests_total{}[15m])'); + expect(query.model).toHaveProperty('editorMode', 'code'); + expect(query.model).toHaveProperty('exemplar', false); + expect(query.model).toHaveProperty('legendFormat', '__auto'); + expect(query.model.instant).toBe(true); + expect(query.model.range).toBe(false); + }); + + it('should not add default values to query models', () => { + const prefillData = { + queries: [ + { + refId: 'A', + datasourceUid: 'test-uid', + queryType: '', + model: { refId: 'A' }, + }, + ], + }; + + const result = formValuesFromPrefill(prefillData); + const query = result.queries[0]; + + // Should NOT have defaults added + expect(query.model).not.toHaveProperty('instant'); + expect(query.model).not.toHaveProperty('range'); + expect(query.model).not.toHaveProperty('expression'); + expect(query.model).not.toHaveProperty('queryType'); + }); +}); diff --git a/public/app/features/plugins/components/restrictedGrafanaApis/alerting/alertRuleFormSchema.test.ts b/public/app/features/plugins/components/restrictedGrafanaApis/alerting/alertRuleFormSchema.test.ts new file mode 100644 index 00000000000..59e02a1ab92 --- /dev/null +++ b/public/app/features/plugins/components/restrictedGrafanaApis/alerting/alertRuleFormSchema.test.ts @@ -0,0 +1,168 @@ +import { alertingAlertRuleFormSchema, alertingModelSchema } from './alertRuleFormSchema'; + +describe('alertingModelSchema', () => { + it('should allow threshold expression model', () => { + const model = { + conditions: [ + { + evaluator: { params: [0.0001, 0], type: 'gt' }, + operator: { type: 'and' }, + query: { params: [] }, + reducer: { params: [], type: 'avg' }, + type: 'query', + }, + ], + datasource: { name: 'Expression', type: '__expr__', uid: '__expr__' }, + expression: 'A', + intervalMs: 1000, + maxDataPoints: 43200, + refId: 'C', + type: 'threshold', + }; + + const result = alertingModelSchema.parse(model); + expect(result.type).toBe('threshold'); + expect(result.conditions).toBeDefined(); + }); + + it('should allow reduce expression model', () => { + const model = { + expression: 'A', + intervalMs: 1000, + maxDataPoints: 100, + reducer: 'mean', + refId: 'B', + type: 'reduce', + }; + + const result = alertingModelSchema.parse(model); + expect(result.type).toBe('reduce'); + expect(result.reducer).toBe('mean'); + }); + + it('should allow math expression model', () => { + const model = { + expression: '$B > 0.4', + intervalMs: 1000, + maxDataPoints: 43200, + refId: 'C', + type: 'math', + }; + + const result = alertingModelSchema.parse(model); + expect(result.type).toBe('math'); + expect(result.expression).toBe('$B > 0.4'); + }); + + it('should allow Prometheus query model', () => { + const model = { + datasource: { type: 'prometheus', uid: 'gdev-prometheus' }, + editorMode: 'code', + expr: 'up', + instant: true, + range: false, + refId: 'A', + }; + + const result = alertingModelSchema.parse(model); + expect(result.expr).toBe('up'); + expect(result.instant).toBe(true); + }); + + it('should not add default values', () => { + const model = { refId: 'A' }; + + const result = alertingModelSchema.parse(model); + expect(result).not.toHaveProperty('instant'); + expect(result).not.toHaveProperty('range'); + expect(result).not.toHaveProperty('queryType'); + expect(result).not.toHaveProperty('expression'); + }); +}); + +describe('alertingAlertRuleFormSchema', () => { + it('should validate alert rule with mixed query types', () => { + const alertRule = { + folder: { uid: 'folder-uid', title: 'Test Folder' }, + group: 'test-group', + queries: [ + { + refId: 'A', + datasourceUid: 'gdev-prometheus', + queryType: '', + relativeTimeRange: { from: 600, to: 0 }, + model: { + datasource: { type: 'prometheus', uid: 'gdev-prometheus' }, + expr: 'up', + instant: true, + range: false, + refId: 'A', + }, + }, + { + refId: 'B', + datasourceUid: '__expr__', + queryType: '', + model: { + expression: 'A', + reducer: 'mean', + refId: 'B', + type: 'reduce', + }, + }, + { + refId: 'C', + datasourceUid: '__expr__', + queryType: '', + model: { + conditions: [ + { + evaluator: { params: [0], type: 'gt' }, + operator: { type: 'and' }, + query: { params: [] }, + reducer: { params: [], type: 'avg' }, + type: 'query', + }, + ], + datasource: { type: '__expr__', uid: '__expr__' }, + expression: 'B', + refId: 'C', + type: 'threshold', + }, + }, + ], + condition: 'C', + }; + + const result = alertingAlertRuleFormSchema.parse(alertRule); + const queries = result.queries || []; + expect(queries[0].model.expr).toBe('up'); + expect(queries[1].model.type).toBe('reduce'); + expect(queries[2].model.type).toBe('threshold'); + }); + + it('should handle minimal payload from group details page', () => { + const minimalPayload = { + folder: { uid: 'folder-uid', title: 'Alpha squad' }, + group: 'alpha_squad_api_service_rules', + }; + + const result = alertingAlertRuleFormSchema.parse(minimalPayload); + expect(result.folder?.uid).toBe('folder-uid'); + expect(result.folder?.title).toBe('Alpha squad'); + expect(result.group).toBe('alpha_squad_api_service_rules'); + + // Verify no defaults are added for optional fields that weren't provided + expect(result.name).toBeUndefined(); + expect(result.condition).toBeUndefined(); + expect(result.noDataState).toBeUndefined(); + expect(result.execErrState).toBeUndefined(); + expect(result.evaluateEvery).toBeUndefined(); + expect(result.evaluateFor).toBeUndefined(); + expect(result.keepFiringFor).toBeUndefined(); + expect(result.metric).toBeUndefined(); + expect(result.targetDatasourceUid).toBeUndefined(); + expect(result.returnTo).toBeUndefined(); + expect(result.annotations).toBeUndefined(); + }); +}); diff --git a/public/app/features/plugins/components/restrictedGrafanaApis/alerting/alertRuleFormSchema.ts b/public/app/features/plugins/components/restrictedGrafanaApis/alerting/alertRuleFormSchema.ts index 4470c195544..bcfe4c7b8d1 100644 --- a/public/app/features/plugins/components/restrictedGrafanaApis/alerting/alertRuleFormSchema.ts +++ b/public/app/features/plugins/components/restrictedGrafanaApis/alerting/alertRuleFormSchema.ts @@ -1,68 +1,14 @@ import { z } from 'zod'; -import alertDef from 'app/features/alerting/state/alertDef'; import { RuleFormType } from 'app/features/alerting/unified/types/rule-form'; -import { ExpressionQueryType } from 'app/features/expressions/types'; import { GrafanaAlertStateDecision } from 'app/types/unified-alerting-dto'; -// Schema for __expr__ type queries (reduce, threshold, etc.) -export const exprQuerySchema = z.object({ - refId: z.string().describe('Reference ID for the query, e.g., "B", "C", etc.'), - type: z.enum(ExpressionQueryType).describe('Expression type'), - datasource: z.object({ - uid: z.literal('__expr__').describe('Must be "__expr__" for expression queries'), - type: z.literal('__expr__').describe('Must be "__expr__" for expression queries'), - }), - conditions: z - .array( - z.object({ - type: z.string().describe('Condition type, e.g., "query"'), - evaluator: z.object({ - params: z.array(z.any()).describe('Parameters for the evaluator'), - type: z.enum(alertDef.evalFunctions.map((ef) => ef.value)).describe('Evaluator type'), - }), - operator: z.object({ - type: z.enum(alertDef.evalOperators.map((eo) => eo.value)).describe('Operator type'), - }), - query: z.object({ - params: z.array(z.string()).describe('Query parameters, typically the refId to evaluate'), - }), - reducer: z.object({ - params: z.array(z.any()).describe('Parameters for the reducer'), - type: z.string().describe('Reducer type, e.g., "last", "avg", "sum", "count", "min", "max"'), - }), - }) - ) - .optional() - .describe('Conditions for the expression query'), - reducer: z.string().optional().describe('Reducer function, e.g., "last", "avg", "sum"'), - expression: z.string().optional().describe('Expression referencing other queries, e.g., "A"'), - math: z.string().optional().describe('Math expression for math type queries'), -}); - -// Schema for regular datasource queries -export const alertingQuerySchema = z.object({ - refId: z.string().describe('Reference ID for the query, e.g., "A", "B", etc.'), - queryType: z.string().optional().default('alerting').describe('Type of query (e.g., "alerting", "recording")'), - expression: z - .string() - .optional() - .default('') - .describe('Query expression to be executed. This can not include variables (e.g. $var).'), - instant: z.boolean().optional().default(true).describe('Whether the query is an instant query'), - range: z - .boolean() - .optional() - .default(false) - .describe('Whether the query is a range query, should be false if instant is true'), - datasource: z.object({ - type: z.string().optional().describe('Datasource type or "__expr__" when it is an expression query'), - uid: z.string().optional().describe('Datasource UID'), - }), -}); - // Combined schema that supports both regular and expression queries -export const alertingModelSchema = z.union([alertingQuerySchema, exprQuerySchema]); +export const alertingModelSchema = z.looseObject({ + refId: z.string(), + maxDataPoints: z.number().optional().describe('Maximum number of data points to return'), + intervalMs: z.number().optional().describe('Interval in milliseconds'), +}); // Main navigate to alert form schema - merged from both alertingSchemaApi and formDefaults export const alertingAlertRuleFormSchema = z.object({ @@ -112,11 +58,11 @@ export const alertingAlertRuleFormSchema = z.object({ .array( z.object({ refId: z.string().describe('Reference ID for the query (e.g., "A", "B", "C")'), - queryType: z.string().optional().default('instant').describe('Type of query (e.g., "instant")'), + queryType: z.string().default('').describe('Datasource-specific'), relativeTimeRange: z .object({ from: z.number().describe('Relative time from in seconds (e.g., 3600 for 1 hour)'), - to: z.number().default(0).describe('Relative time to in seconds (usually 0 for "now")'), + to: z.number().describe('Relative time to in seconds (usually 0 for "now")'), }) .optional(), datasourceUid: z.string().describe('Datasource UID for the query'), @@ -193,8 +139,6 @@ export const alertingAlertRuleFormSchema = z.object({ // Export types for use in plugins export type AlertingAlertRuleFormSchemaType = z.infer; -export type AlertingQuerySchemaType = z.infer; -export type ExprQuerySchemaType = z.infer; export type AlertingModelSchemaType = z.infer; // Simple API that only exposes the navigate to alert rule form schema