From 2ba6cf30cd867d578399aa1e1393c89a5290f0fb Mon Sep 17 00:00:00 2001 From: William Wernert Date: Tue, 13 May 2025 08:49:40 -0400 Subject: [PATCH] [release-11.5.5] Alerting: Ensure field validators return the proper type (#105288) Alerting: Ensure field validators return the proper type (#104050) * Ensure field validators return the proper type This ensures correct error propagation through services up to the API layer. * Move error wrapping up to call site (cherry picked from commit 820c33841434513dae2359ec4e923a2f878b3abd) --- pkg/services/ngalert/models/alert_rule.go | 6 +-- .../ngalert/models/alert_rule_test.go | 47 +++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index 5b4a85dca10..3a9be94adbc 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -580,7 +580,7 @@ func (alertRule *AlertRule) ValidateAlertRule(cfg setting.UnifiedAlertingSetting err = validateAlertRuleFields(alertRule) } if err != nil { - return err + return fmt.Errorf("%w: %s", ErrAlertRuleFailedValidation, err) } if alertRule.For < 0 { @@ -621,10 +621,10 @@ func validateAlertRuleFields(rule *AlertRule) error { func validateRecordingRuleFields(rule *AlertRule) error { metricName := prommodels.LabelValue(rule.Record.Metric) if !metricName.IsValid() { - return fmt.Errorf("%w: %s", ErrAlertRuleFailedValidation, "metric name for recording rule must be a valid utf8 string") + return errors.New("metric name for recording rule must be a valid utf8 string") } if !prommodels.IsValidMetricName(metricName) { - return fmt.Errorf("%w: %s", ErrAlertRuleFailedValidation, "metric name for recording rule must be a valid Prometheus metric name") + return errors.New("metric name for recording rule must be a valid Prometheus metric name") } ClearRecordingRuleIgnoredFields(rule) diff --git a/pkg/services/ngalert/models/alert_rule_test.go b/pkg/services/ngalert/models/alert_rule_test.go index bb7bfdadf33..ee0f751eb69 100644 --- a/pkg/services/ngalert/models/alert_rule_test.go +++ b/pkg/services/ngalert/models/alert_rule_test.go @@ -17,6 +17,7 @@ import ( "github.com/stretchr/testify/require" "gopkg.in/yaml.v3" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/util/cmputil" ) @@ -914,3 +915,49 @@ func TestAlertRuleGetKeyWithGroup(t *testing.T) { require.Equal(t, expected, rule.GetKeyWithGroup()) }) } +func TestValidateAlertRule(t *testing.T) { + t.Run("ExecErrState & NoDataState", func(t *testing.T) { + testCases := []struct { + name string + execErrState string + noDataState string + error bool + }{ + { + name: "invalid error state", + execErrState: "invalid", + error: true, + }, + { + name: "invalid no data state", + noDataState: "invalid", + error: true, + }, + { + name: "valid states", + error: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + rule := RuleGen.With( + RuleMuts.WithIntervalSeconds(10), + ).Generate() + if tc.execErrState != "" { + rule.ExecErrState = ExecutionErrorState(tc.execErrState) + } + if tc.noDataState != "" { + rule.NoDataState = NoDataState(tc.noDataState) + } + + err := rule.ValidateAlertRule(setting.UnifiedAlertingSettings{BaseInterval: 10 * time.Second}) + if tc.error { + require.Error(t, err) + require.ErrorIs(t, err, ErrAlertRuleFailedValidation) + } else { + require.NoError(t, err) + } + }) + } + }) +}