public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[release-12.0.1] Alerting: Ensure field validators return the proper type (#104243)

Browse Source 
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 820c338414)

Co-authored-by: William Wernert <william.wernert@grafana.com>
This commit is contained in:
grafana-delivery-bot[bot]
2025-04-21 21:17:40 +01:00
committed by GitHub
co-authored by William Wernert
parent 341a885a38
commit b9ee6bae38
2 changed files with 138 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/ngalert/models/alert_rule.go
+4 -4
View File
@@ -642,7 +642,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 {
@@ -682,7 +682,7 @@ func validateAlertRuleFields(rule *AlertRule) error {
}
if rule.MissingSeriesEvalsToResolve != nil && *rule.MissingSeriesEvalsToResolve <= 0 {
return fmt.Errorf("%w: field `missing_series_evals_to_resolve` must be greater than 0", ErrAlertRuleFailedValidation)
return errors.New("field `missing_series_evals_to_resolve` must be greater than 0")
}
return nil
@@ -691,10 +691,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)
pkg/services/ngalert/models/alert_rule_test.go
+134 -87
View File
@@ -1052,45 +1052,143 @@ func TestGeneratorFillsAllFields(t *testing.T) {
}
func TestValidateAlertRule(t *testing.T) {
testCases := []struct {
name string
keepFiringFor time.Duration
expectedErr error
}{
{
name: "should accept zero keep firing for",
keepFiringFor: 0,
expectedErr: nil,
},
{
name: "should accept positive keep firing for",
keepFiringFor: 1 * time.Minute,
expectedErr: nil,
},
{
name: "should reject negative keep firing for",
keepFiringFor: -1 * time.Minute,
expectedErr: fmt.Errorf("%w: field `keep_firing_for` cannot be negative", ErrAlertRuleFailedValidation),
},
}
t.Run("keepFiringFor", func(t *testing.T) {
testCases := []struct {
name string
keepFiringFor time.Duration
expectedErr error
}{
{
name: "should accept zero keep firing for",
keepFiringFor: 0,
expectedErr: nil,
},
{
name: "should accept positive keep firing for",
keepFiringFor: 1 * time.Minute,
expectedErr: nil,
},
{
name: "should reject negative keep firing for",
keepFiringFor: -1 * time.Minute,
expectedErr: fmt.Errorf("%w: field `keep_firing_for` cannot be negative", ErrAlertRuleFailedValidation),
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
rule := RuleGen.With(
RuleGen.WithKeepFiringFor(tc.keepFiringFor),
RuleGen.WithIntervalSeconds(10),
).GenerateRef()
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
rule := RuleGen.With(
RuleGen.WithKeepFiringFor(tc.keepFiringFor),
RuleGen.WithIntervalSeconds(10),
).GenerateRef()
err := rule.ValidateAlertRule(setting.UnifiedAlertingSettings{BaseInterval: 10 * time.Second})
err := rule.ValidateAlertRule(setting.UnifiedAlertingSettings{BaseInterval: 10 * time.Second})
if tc.expectedErr == nil {
require.NoError(t, err)
} else {
require.Error(t, err)
require.Equal(t, tc.expectedErr.Error(), err.Error())
}
})
}
if tc.expectedErr == nil {
require.NoError(t, err)
} else {
require.Error(t, err)
require.Equal(t, tc.expectedErr.Error(), err.Error())
}
})
}
})
t.Run("missingSeriesEvalsToResolve", func(t *testing.T) {
testCases := []struct {
name string
missingSeriesEvalsToResolve *int
expectedErrorContains string
}{
{
name: "should allow nil value",
missingSeriesEvalsToResolve: nil,
},
{
name: "should reject negative value",
missingSeriesEvalsToResolve: util.Pointer(-1),
expectedErrorContains: "field `missing_series_evals_to_resolve` must be greater than 0",
},
{
name: "should reject 0",
missingSeriesEvalsToResolve: util.Pointer(0),
expectedErrorContains: "field `missing_series_evals_to_resolve` must be greater than 0",
},
{
name: "should accept positive value",
missingSeriesEvalsToResolve: util.Pointer(2),
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
baseIntervalSeconds := int64(10)
cfg := setting.UnifiedAlertingSettings{
BaseInterval: time.Duration(baseIntervalSeconds) * time.Second,
}
rule := RuleGen.With(
RuleMuts.WithIntervalSeconds(baseIntervalSeconds * 2),
).Generate()
rule.MissingSeriesEvalsToResolve = tc.missingSeriesEvalsToResolve
err := rule.ValidateAlertRule(cfg)
if tc.expectedErrorContains != "" {
require.Error(t, err)
require.ErrorIs(t, err, ErrAlertRuleFailedValidation)
require.Contains(t, err.Error(), tc.expectedErrorContains)
} else {
require.NoError(t, err)
}
})
}
})
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)
}
})
}
})
}
func TestAlertRule_PrometheusRuleDefinition(t *testing.T) {
@@ -1159,54 +1257,3 @@ func TestAlertRule_PrometheusRuleDefinition(t *testing.T) {
})
}
}
func TestMissingSeriesEvalsToResolveValidation(t *testing.T) {
testCases := []struct {
name string
missingSeriesEvalsToResolve *int
expectedErrorContains string
}{
{
name: "should allow nil value",
missingSeriesEvalsToResolve: nil,
},
{
name: "should reject negative value",
missingSeriesEvalsToResolve: util.Pointer(-1),
expectedErrorContains: "field `missing_series_evals_to_resolve` must be greater than 0",
},
{
name: "should reject 0",
missingSeriesEvalsToResolve: util.Pointer(0),
expectedErrorContains: "field `missing_series_evals_to_resolve` must be greater than 0",
},
{
name: "should accept positive value",
missingSeriesEvalsToResolve: util.Pointer(2),
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
baseIntervalSeconds := int64(10)
cfg := setting.UnifiedAlertingSettings{
BaseInterval: time.Duration(baseIntervalSeconds) * time.Second,
}
rule := RuleGen.With(
RuleMuts.WithIntervalSeconds(baseIntervalSeconds * 2),
).Generate()
rule.MissingSeriesEvalsToResolve = tc.missingSeriesEvalsToResolve
err := rule.ValidateAlertRule(cfg)
if tc.expectedErrorContains != "" {
require.Error(t, err)
require.ErrorIs(t, err, ErrAlertRuleFailedValidation)
require.Contains(t, err.Error(), tc.expectedErrorContains)
} else {
require.NoError(t, err)
}
})
}
}