From 1badcf4b63da84d7085fd75409acd9e8c842ce97 Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Wed, 15 May 2024 09:35:54 -0500 Subject: [PATCH] Alerting: Allow NoData and ExecErrState to be fully blank on recording rules (#87868) * Allow empty NoData and ExecErrState on recording rules * remove TODO about this --- pkg/services/ngalert/models/alert_rule.go | 12 +++++++----- pkg/services/ngalert/store/alert_rule_test.go | 10 ++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index 19ff0cc32cf..bd9d0dec2cf 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -498,12 +498,14 @@ func (alertRule *AlertRule) ValidateAlertRule(cfg setting.UnifiedAlertingSetting return fmt.Errorf("%w: cannot have Panel ID without a Dashboard UID", ErrAlertRuleFailedValidation) } - if _, err := ErrStateFromString(string(alertRule.ExecErrState)); err != nil { - return err - } + if !alertRule.IsRecordingRule() { + if _, err := ErrStateFromString(string(alertRule.ExecErrState)); err != nil { + return err + } - if _, err := NoDataStateFromString(string(alertRule.NoDataState)); err != nil { - return err + if _, err := NoDataStateFromString(string(alertRule.NoDataState)); err != nil { + return err + } } if alertRule.For < 0 { diff --git a/pkg/services/ngalert/store/alert_rule_test.go b/pkg/services/ngalert/store/alert_rule_test.go index 7016aa9bedd..36ce548659f 100644 --- a/pkg/services/ngalert/store/alert_rule_test.go +++ b/pkg/services/ngalert/store/alert_rule_test.go @@ -619,10 +619,8 @@ func TestIntegrationInsertAlertRules(t *testing.T) { rrs := gen.GenerateMany(n) for i := range rrs { rrs[i].Condition = "" - // TODO: These fields do not apply to recording rules - for now, we just use the default values of them. This is validated at the storage level. - // TODO: Consider making it so recording rules allow for empty string here, or use some other sentinel value in the future. - rrs[i].NoDataState = models.NoData - rrs[i].ExecErrState = models.ErrorErrState + rrs[i].NoDataState = "" + rrs[i].ExecErrState = "" rrs[i].For = 0 rrs[i].NotificationSettings = nil rrs[i].Record = &models.Record{ @@ -679,8 +677,8 @@ func TestIntegrationInsertAlertRules(t *testing.T) { for _, rule := range dbRules { if rule.IsRecordingRule() { require.Empty(t, rule.Condition) - require.Equal(t, models.NoData, rule.NoDataState) - require.Equal(t, models.ErrorErrState, rule.ExecErrState) + require.Equal(t, models.NoDataState(""), rule.NoDataState) + require.Equal(t, models.ExecutionErrorState(""), rule.ExecErrState) require.Zero(t, rule.For) require.Nil(t, rule.NotificationSettings) }