apitech
/
grafana
mirror of https://github.com/grafana/grafana
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Alerting: Ensure field validators return the proper type (#104050)

Browse Source 
* 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
pull/102780/head
William Wernert 2 months ago committed by GitHub
parent 3dda7ccc30
commit 820c338414
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 138 additions and 91 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 8
      pkg/services/ngalert/models/alert_rule.go
  2. 221
      pkg/services/ngalert/models/alert_rule_test.go

8
pkg/services/ngalert/models/alert_rule.go
Unescape 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)

221
pkg/services/ngalert/models/alert_rule_test.go
Unescape 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)
}
})
}
}

Write Preview
Loading…
Cancel
Save