From 7347083f9890b4166d6039890ee0040bbb1d1983 Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Tue, 1 Apr 2025 20:28:10 +0200 Subject: [PATCH] Alerting: Validate target datasource type only when importing recording rules (#103214) --- pkg/services/ngalert/prom/convert.go | 17 ++++-- pkg/services/ngalert/prom/convert_test.go | 72 +++++++++++++++++++++++ 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/pkg/services/ngalert/prom/convert.go b/pkg/services/ngalert/prom/convert.go index 149d9b9eda0..2027dcd1f99 100644 --- a/pkg/services/ngalert/prom/convert.go +++ b/pkg/services/ngalert/prom/convert.go @@ -26,7 +26,14 @@ const ( ) var ( - ErrInvalidDatasourceType = errutil.ValidationFailed("alerting.invalidDatasourceType") + ErrInvalidDatasourceType = errutil.ValidationFailed( + "alerting.invalidDatasourceType", + errutil.WithPublicMessage("Datasource type must be Prometheus or Loki to import rules."), + ) + ErrInvalidTargetDatasourceType = errutil.ValidationFailed( + "alerting.invalidTargetDatasourceType", + errutil.WithPublicMessage("Target datasource type must be Prometheus for recording rules."), + ) ) // Config defines the configuration options for the Prometheus to Grafana rules converter. @@ -110,9 +117,6 @@ func NewConverter(cfg Config) (*Converter, error) { if cfg.DatasourceType != datasources.DS_PROMETHEUS && cfg.DatasourceType != datasources.DS_LOKI { return nil, ErrInvalidDatasourceType.Errorf("invalid datasource type: %s, must be prometheus or loki", cfg.DatasourceType) } - if cfg.TargetDatasourceType != datasources.DS_PROMETHEUS { - return nil, ErrInvalidDatasourceType.Errorf("invalid target datasource type: %s, must be prometheus", cfg.TargetDatasourceType) - } return &Converter{ cfg: cfg, @@ -203,7 +207,12 @@ func (p *Converter) convertRule(orgID int64, namespaceUID string, promGroup Prom if err != nil { return models.AlertRule{}, err } + if isRecordingRule { + if p.cfg.TargetDatasourceType != datasources.DS_PROMETHEUS { + return models.AlertRule{}, ErrInvalidTargetDatasourceType.Errorf("invalid target datasource type: %s, must be prometheus", p.cfg.TargetDatasourceType) + } + record = &models.Record{ From: queryRefID, Metric: rule.Record, diff --git a/pkg/services/ngalert/prom/convert_test.go b/pkg/services/ngalert/prom/convert_test.go index d5e7aae559b..ffa655ef650 100644 --- a/pkg/services/ngalert/prom/convert_test.go +++ b/pkg/services/ngalert/prom/convert_test.go @@ -55,6 +55,78 @@ func TestPrometheusRulesToGrafana(t *testing.T) { }, expectError: false, }, + { + // If the rule group has no recording rules, the target datasource + // can be anything and should not be validated. + name: "alert rules with non-prometheus target datasource", + orgID: 1, + namespace: "namespaceUID", + promGroup: PrometheusRuleGroup{ + Name: "test-group-1", + Interval: prommodel.Duration(10 * time.Second), + Rules: []PrometheusRule{ + { + Alert: "alert-1", + Expr: "up == 0", + }, + }, + }, + config: Config{ + TargetDatasourceUID: "target-datasource-uid", + TargetDatasourceType: "non-prometheus-datasource", + }, + expectError: false, + }, + { + // If the rule group has recording rules and a non-prometheus target datasource, + // we should return an error + name: "recording rules with non-prometheus target datasource", + orgID: 1, + namespace: "namespaceUID", + promGroup: PrometheusRuleGroup{ + Name: "test-group-1", + Interval: prommodel.Duration(10 * time.Second), + Rules: []PrometheusRule{ + { + Record: "some_metric", + Expr: "sum(rate(http_requests_total[5m]))", + }, + }, + }, + config: Config{ + TargetDatasourceUID: "target-datasource-uid", + TargetDatasourceType: "non-prometheus-datasource", + }, + expectError: true, + errorMsg: "invalid target datasource type: non-prometheus-datasource, must be prometheus", + }, + { + // If the rule group has recording rules and a non-prometheus target datasource, + // we should return an error + name: "mixed group with both alert and recording rules requires prometheus target datasource", + orgID: 1, + namespace: "namespaceUID", + promGroup: PrometheusRuleGroup{ + Name: "mixed-rules-group", + Interval: prommodel.Duration(10 * time.Second), + Rules: []PrometheusRule{ + { + Alert: "alert-1", + Expr: "up == 0", + }, + { + Record: "some_metric", + Expr: "sum(rate(http_requests_total[5m]))", + }, + }, + }, + config: Config{ + TargetDatasourceUID: "target-datasource-uid", + TargetDatasourceType: "non-prometheus-datasource", + }, + expectError: true, + errorMsg: "invalid target datasource type: non-prometheus-datasource, must be prometheus", + }, { name: "rules with keep_firing_for are not supported", orgID: 1,