From e39658097f123f87fcbcff51e9018505e1fbf96e Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Tue, 14 May 2024 07:44:14 -0500 Subject: [PATCH] Alerting: Wire recording rules feature toggle into limits struct (#87778) Wire toggle into limits --- pkg/services/ngalert/api/api_ruler.go | 2 +- pkg/services/ngalert/api/api_ruler_export.go | 2 +- .../ngalert/api/api_ruler_validation.go | 6 ++- .../ngalert/api/api_ruler_validation_test.go | 41 +++++++++++++------ pkg/services/ngalert/api/api_testing.go | 2 +- 5 files changed, 36 insertions(+), 17 deletions(-) diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index ee0bfa0db76..1ab6d12295d 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -296,7 +296,7 @@ func (srv RulerSrv) RoutePostNameRulesConfig(c *contextmodel.ReqContext, ruleGro return ErrResp(http.StatusBadRequest, err, "") } - rules, err := ValidateRuleGroup(&ruleGroupConfig, c.SignedInUser.GetOrgID(), namespace.UID, RuleLimitsFromConfig(srv.cfg)) + rules, err := ValidateRuleGroup(&ruleGroupConfig, c.SignedInUser.GetOrgID(), namespace.UID, RuleLimitsFromConfig(srv.cfg, srv.featureManager)) if err != nil { return ErrResp(http.StatusBadRequest, err, "") } diff --git a/pkg/services/ngalert/api/api_ruler_export.go b/pkg/services/ngalert/api/api_ruler_export.go index f925a4d4a8a..12a916ad627 100644 --- a/pkg/services/ngalert/api/api_ruler_export.go +++ b/pkg/services/ngalert/api/api_ruler_export.go @@ -20,7 +20,7 @@ func (srv RulerSrv) ExportFromPayload(c *contextmodel.ReqContext, ruleGroupConfi return toNamespaceErrorResponse(err) } - rulesWithOptionals, err := ValidateRuleGroup(&ruleGroupConfig, c.SignedInUser.GetOrgID(), namespace.UID, RuleLimitsFromConfig(srv.cfg)) + rulesWithOptionals, err := ValidateRuleGroup(&ruleGroupConfig, c.SignedInUser.GetOrgID(), namespace.UID, RuleLimitsFromConfig(srv.cfg, srv.featureManager)) if err != nil { return ErrResp(http.StatusBadRequest, err, "") } diff --git a/pkg/services/ngalert/api/api_ruler_validation.go b/pkg/services/ngalert/api/api_ruler_validation.go index 2fa3100ac58..203c4930458 100644 --- a/pkg/services/ngalert/api/api_ruler_validation.go +++ b/pkg/services/ngalert/api/api_ruler_validation.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/grafana/grafana/pkg/services/featuremgmt" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/store" @@ -18,12 +19,15 @@ type RuleLimits struct { DefaultRuleEvaluationInterval time.Duration // All intervals must be an integer multiple of this duration. BaseInterval time.Duration + // Whether recording rules are allowed. + RecordingRulesAllowed bool } -func RuleLimitsFromConfig(cfg *setting.UnifiedAlertingSettings) RuleLimits { +func RuleLimitsFromConfig(cfg *setting.UnifiedAlertingSettings, toggles featuremgmt.FeatureToggles) RuleLimits { return RuleLimits{ DefaultRuleEvaluationInterval: cfg.DefaultRuleEvaluationInterval, BaseInterval: cfg.BaseInterval, + RecordingRulesAllowed: toggles.IsEnabledGlobally(featuremgmt.FlagGrafanaManagedRecordingRules), } } diff --git a/pkg/services/ngalert/api/api_ruler_validation_test.go b/pkg/services/ngalert/api/api_ruler_validation_test.go index 46d6406bb17..728fab3e568 100644 --- a/pkg/services/ngalert/api/api_ruler_validation_test.go +++ b/pkg/services/ngalert/api/api_ruler_validation_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/exp/rand" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/models" @@ -42,6 +43,11 @@ func config(t *testing.T) *setting.UnifiedAlertingSettings { return result } +func makeLimits(cfg *setting.UnifiedAlertingSettings) RuleLimits { + baseToggles := featuremgmt.WithFeatures() + return RuleLimitsFromConfig(cfg, baseToggles) +} + func validRule() apimodels.PostableExtendedRuleNode { forDuration := model.Duration(rand.Int63n(1000)) uid := util.GenerateShortUID() @@ -195,10 +201,11 @@ func TestValidateRuleGroup(t *testing.T) { rules = append(rules, validRule()) } cfg := config(t) + limits := makeLimits(cfg) t.Run("should validate struct and rules", func(t *testing.T) { g := validGroup(cfg, rules...) - alerts, err := ValidateRuleGroup(&g, orgId, folder.UID, RuleLimitsFromConfig(cfg)) + alerts, err := ValidateRuleGroup(&g, orgId, folder.UID, limits) require.NoError(t, err) require.Len(t, alerts, len(rules)) }) @@ -206,7 +213,7 @@ func TestValidateRuleGroup(t *testing.T) { t.Run("should default to default interval from config if group interval is 0", func(t *testing.T) { g := validGroup(cfg, rules...) g.Interval = 0 - alerts, err := ValidateRuleGroup(&g, orgId, folder.UID, RuleLimitsFromConfig(cfg)) + alerts, err := ValidateRuleGroup(&g, orgId, folder.UID, limits) require.NoError(t, err) for _, alert := range alerts { require.Equal(t, int64(cfg.DefaultRuleEvaluationInterval.Seconds()), alert.IntervalSeconds) @@ -221,7 +228,7 @@ func TestValidateRuleGroup(t *testing.T) { isPaused = !(isPaused) } g := validGroup(cfg, rules...) - alerts, err := ValidateRuleGroup(&g, orgId, folder.UID, RuleLimitsFromConfig(cfg)) + alerts, err := ValidateRuleGroup(&g, orgId, folder.UID, limits) require.NoError(t, err) for _, alert := range alerts { require.True(t, alert.HasPause) @@ -233,6 +240,7 @@ func TestValidateRuleGroupFailures(t *testing.T) { orgId := rand.Int63() folder := randFolder() cfg := config(t) + limits := makeLimits(cfg) testCases := []struct { name string @@ -293,7 +301,7 @@ func TestValidateRuleGroupFailures(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { g := testCase.group() - _, err := ValidateRuleGroup(g, orgId, folder.UID, RuleLimitsFromConfig(cfg)) + _, err := ValidateRuleGroup(g, orgId, folder.UID, limits) require.Error(t, err) if testCase.assert != nil { testCase.assert(t, g, err) @@ -307,6 +315,7 @@ func TestValidateRuleNode_NoUID(t *testing.T) { folder := randFolder() name := util.GenerateShortUID() var cfg = config(t) + limits := makeLimits(cfg) interval := cfg.BaseInterval * time.Duration(rand.Int63n(10)+1) testCases := []struct { @@ -401,7 +410,7 @@ func TestValidateRuleNode_NoUID(t *testing.T) { r := testCase.rule() r.GrafanaManagedAlert.UID = "" - alert, err := validateRuleNode(r, name, interval, orgId, folder.UID, RuleLimitsFromConfig(cfg)) + alert, err := validateRuleNode(r, name, interval, orgId, folder.UID, limits) require.NoError(t, err) testCase.assert(t, r, alert) }) @@ -409,7 +418,7 @@ func TestValidateRuleNode_NoUID(t *testing.T) { t.Run("accepts empty group name", func(t *testing.T) { r := validRule() - alert, err := validateRuleNode(&r, "", interval, orgId, folder.UID, RuleLimitsFromConfig(cfg)) + alert, err := validateRuleNode(&r, "", interval, orgId, folder.UID, limits) require.NoError(t, err) require.Equal(t, "", alert.RuleGroup) }) @@ -419,6 +428,7 @@ func TestValidateRuleNodeFailures_NoUID(t *testing.T) { orgId := rand.Int63() folder := randFolder() cfg := config(t) + limits := makeLimits(cfg) testCases := []struct { name string @@ -562,7 +572,7 @@ func TestValidateRuleNodeFailures_NoUID(t *testing.T) { interval = *testCase.interval } - _, err := validateRuleNode(r, "", interval, orgId, folder.UID, RuleLimitsFromConfig(cfg)) + _, err := validateRuleNode(r, "", interval, orgId, folder.UID, limits) require.Error(t, err) if testCase.assert != nil { testCase.assert(t, r, err) @@ -576,6 +586,7 @@ func TestValidateRuleNode_UID(t *testing.T) { folder := randFolder() name := util.GenerateShortUID() var cfg = config(t) + limits := makeLimits(cfg) interval := cfg.BaseInterval * time.Duration(rand.Int63n(10)+1) testCases := []struct { @@ -654,7 +665,7 @@ func TestValidateRuleNode_UID(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { r := testCase.rule() - alert, err := validateRuleNode(r, name, interval, orgId, folder.UID, RuleLimitsFromConfig(cfg)) + alert, err := validateRuleNode(r, name, interval, orgId, folder.UID, limits) require.NoError(t, err) testCase.assert(t, r, alert) }) @@ -662,7 +673,7 @@ func TestValidateRuleNode_UID(t *testing.T) { t.Run("accepts empty group name", func(t *testing.T) { r := validRule() - alert, err := validateRuleNode(&r, "", interval, orgId, folder.UID, RuleLimitsFromConfig(cfg)) + alert, err := validateRuleNode(&r, "", interval, orgId, folder.UID, limits) require.NoError(t, err) require.Equal(t, "", alert.RuleGroup) }) @@ -672,6 +683,7 @@ func TestValidateRuleNodeFailures_UID(t *testing.T) { orgId := rand.Int63() folder := randFolder() cfg := config(t) + limits := makeLimits(cfg) testCases := []struct { name string @@ -757,7 +769,7 @@ func TestValidateRuleNodeFailures_UID(t *testing.T) { interval = *testCase.interval } - _, err := validateRuleNode(r, "", interval, orgId, folder.UID, RuleLimitsFromConfig(cfg)) + _, err := validateRuleNode(r, "", interval, orgId, folder.UID, limits) require.Error(t, err) if testCase.assert != nil { testCase.assert(t, r, err) @@ -768,6 +780,7 @@ func TestValidateRuleNodeFailures_UID(t *testing.T) { func TestValidateRuleNodeIntervalFailures(t *testing.T) { cfg := config(t) + limits := makeLimits(cfg) testCases := []struct { name string @@ -790,7 +803,7 @@ func TestValidateRuleNodeIntervalFailures(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { r := validRule() - _, err := validateRuleNode(&r, util.GenerateShortUID(), testCase.interval, rand.Int63(), randFolder().UID, RuleLimitsFromConfig(cfg)) + _, err := validateRuleNode(&r, util.GenerateShortUID(), testCase.interval, rand.Int63(), randFolder().UID, limits) require.Error(t, err) }) } @@ -798,6 +811,7 @@ func TestValidateRuleNodeIntervalFailures(t *testing.T) { func TestValidateRuleNodeNotificationSettings(t *testing.T) { cfg := config(t) + limits := makeLimits(cfg) validNotificationSettings := models.NotificationSettingsGen(models.NSMuts.WithGroupBy(model.AlertNameLabel, models.FolderTitleLabel)) @@ -880,7 +894,7 @@ func TestValidateRuleNodeNotificationSettings(t *testing.T) { t.Run(tt.name, func(t *testing.T) { r := validRule() r.GrafanaManagedAlert.NotificationSettings = AlertRuleNotificationSettingsFromNotificationSettings([]models.NotificationSettings{tt.notificationSettings}) - _, err := validateRuleNode(&r, util.GenerateShortUID(), cfg.BaseInterval*time.Duration(rand.Int63n(10)+1), rand.Int63(), randFolder().UID, RuleLimitsFromConfig(cfg)) + _, err := validateRuleNode(&r, util.GenerateShortUID(), cfg.BaseInterval*time.Duration(rand.Int63n(10)+1), rand.Int63(), randFolder().UID, limits) if tt.expErrorContains != "" { require.Error(t, err) @@ -894,6 +908,7 @@ func TestValidateRuleNodeNotificationSettings(t *testing.T) { func TestValidateRuleNodeReservedLabels(t *testing.T) { cfg := config(t) + limits := makeLimits(cfg) for label := range models.LabelsUserCannotSpecify { t.Run(label, func(t *testing.T) { @@ -901,7 +916,7 @@ func TestValidateRuleNodeReservedLabels(t *testing.T) { r.ApiRuleNode.Labels = map[string]string{ label: "true", } - _, err := validateRuleNode(&r, util.GenerateShortUID(), cfg.BaseInterval*time.Duration(rand.Int63n(10)+1), rand.Int63(), randFolder().UID, RuleLimitsFromConfig(cfg)) + _, err := validateRuleNode(&r, util.GenerateShortUID(), cfg.BaseInterval*time.Duration(rand.Int63n(10)+1), rand.Int63(), randFolder().UID, limits) require.Error(t, err) require.ErrorContains(t, err, label) }) diff --git a/pkg/services/ngalert/api/api_testing.go b/pkg/services/ngalert/api/api_testing.go index 8be9845eba5..ad31e426fa1 100644 --- a/pkg/services/ngalert/api/api_testing.go +++ b/pkg/services/ngalert/api/api_testing.go @@ -67,7 +67,7 @@ func (srv TestingApiSrv) RouteTestGrafanaRuleConfig(c *contextmodel.ReqContext, srv.cfg.BaseInterval, c.SignedInUser.GetOrgID(), folder.UID, - RuleLimitsFromConfig(srv.cfg), + RuleLimitsFromConfig(srv.cfg, srv.featureManager), ) if err != nil { return ErrResp(http.StatusBadRequest, err, "")