Alerting: Fix group-level labels and query_offset in the import API (#106379)

What is this feature? Fixes a bug when group-level query_offset and labels parameters are ignored and not saved Why do we need this feature? In the import API Prometheus YAML rule definitions are supported: groups: - name: group-1 interval: 1m query_offset: 10m labels: severity: "warning" rules: - alert: Alert 0 > 0 expr: vector(0) > 0 But applying group-level labels and query_offset is broken and they are not saved right now because during the conversion of the API model to PrometheusRuleGroup they aren't saved to the new structure.
3 weeks ago · f7a52bc04e
parent 10fb03f97a
commit f7a52bc04e
11 changed files with 171 additions and 84 deletions
--- a/pkg/services/ngalert/api/api_convert_prometheus.go
+++ b/pkg/services/ngalert/api/api_convert_prometheus.go
@ -473,9 +473,12 @@ func (srv *ConvertPrometheusSrv) convertToGrafanaRuleGroup(
 		}
 	}
 	group := prom.PrometheusRuleGroup{
-		Name:     promGroup.Name,
-		Interval: promGroup.Interval,
-		Rules:    rules,
+		Name:        promGroup.Name,
+		Interval:    promGroup.Interval,
+		Rules:       rules,
+		QueryOffset: promGroup.QueryOffset,
+		Limit:       promGroup.Limit,
+		Labels:      promGroup.Labels,
 	}

 	converter, err := prom.NewConverter(
--- a/pkg/services/ngalert/api/api_convert_prometheus_test.go
+++ b/pkg/services/ngalert/api/api_convert_prometheus_test.go
@ -1084,12 +1084,17 @@ func TestRouteConvertPrometheusPostRuleGroups(t *testing.T) {
 		Name:     "TestGroup1",
 		Interval: prommodel.Duration(1 * time.Minute),
 		Rules:    []apimodels.PrometheusRule{promAlertRule},
+		Labels: map[string]string{
+			"group_label": "group_value",
+		},
 	}

+	queryOffset := prommodel.Duration(5 * time.Minute)
 	promGroup2 := apimodels.PrometheusRuleGroup{
-		Name:     "TestGroup2",
-		Interval: prommodel.Duration(1 * time.Minute),
-		Rules:    []apimodels.PrometheusRule{promAlertRule},
+		Name:        "TestGroup2",
+		Interval:    prommodel.Duration(1 * time.Minute),
+		Rules:       []apimodels.PrometheusRule{promAlertRule},
+		QueryOffset: &queryOffset,
 	}

 	promGroup3 := apimodels.PrometheusRuleGroup{
@ -1125,10 +1130,12 @@ func TestRouteConvertPrometheusPostRuleGroups(t *testing.T) {
 				require.Equal(t, "TestAlert", rule.Title)
 				require.Equal(t, "critical", rule.Labels["severity"])
 				require.Equal(t, 5*time.Minute, rule.For)
+				require.Equal(t, "group_value", rule.Labels["group_label"])
 			case "TestGroup2":
 				require.Equal(t, "TestAlert", rule.Title)
 				require.Equal(t, "critical", rule.Labels["severity"])
 				require.Equal(t, 5*time.Minute, rule.For)
+				require.Equal(t, models.Duration(queryOffset), rule.Data[0].RelativeTimeRange.To)
 			case "TestGroup3":
 				switch rule.Title {
 				case "TestAlert":
--- a/pkg/services/ngalert/api/tooling/api.json
+++ b/pkg/services/ngalert/api/tooling/api.json
@ -3062,31 +3062,31 @@
  },
  "PrometheusRule": {
   "properties": {
-    "Alert": {
+    "alert": {
     "type": "string"
    },
-    "Annotations": {
+    "annotations": {
     "additionalProperties": {
      "type": "string"
     },
     "type": "object"
    },
-    "Expr": {
+    "expr": {
     "type": "string"
    },
-    "For": {
+    "for": {
     "type": "string"
    },
-    "KeepFiringFor": {
+    "keep_firing_for": {
     "type": "string"
    },
-    "Labels": {
+    "labels": {
     "additionalProperties": {
      "type": "string"
     },
     "type": "object"
    },
-    "Record": {
+    "record": {
     "type": "string"
    }
   },
@ -3094,26 +3094,26 @@
  },
  "PrometheusRuleGroup": {
   "properties": {
-    "Interval": {
+    "interval": {
     "$ref": "#/definitions/Duration"
    },
-    "Labels": {
+    "labels": {
     "additionalProperties": {
      "type": "string"
     },
     "type": "object"
    },
-    "Limit": {
+    "limit": {
     "format": "int64",
     "type": "integer"
    },
-    "Name": {
+    "name": {
     "type": "string"
    },
-    "QueryOffset": {
+    "query_offset": {
     "type": "string"
    },
-    "Rules": {
+    "rules": {
     "items": {
      "$ref": "#/definitions/PrometheusRule"
     },
--- a/pkg/services/ngalert/api/tooling/definitions/convert_prometheus_api.go
+++ b/pkg/services/ngalert/api/tooling/definitions/convert_prometheus_api.go
@ -228,23 +228,23 @@ type PrometheusNamespace struct {

 // swagger:model
 type PrometheusRuleGroup struct {
-	Name        string            `yaml:"name"`
-	Interval    model.Duration    `yaml:"interval"`
-	QueryOffset *model.Duration   `yaml:"query_offset,omitempty"`
-	Limit       int               `yaml:"limit,omitempty"`
-	Rules       []PrometheusRule  `yaml:"rules"`
-	Labels      map[string]string `yaml:"labels,omitempty"`
+	Name        string            `yaml:"name" json:"name"`
+	Interval    model.Duration    `yaml:"interval" json:"interval"`
+	QueryOffset *model.Duration   `yaml:"query_offset,omitempty" json:"query_offset,omitempty"`
+	Limit       int               `yaml:"limit,omitempty" json:"limit,omitempty"`
+	Rules       []PrometheusRule  `yaml:"rules" json:"rules"`
+	Labels      map[string]string `yaml:"labels,omitempty" json:"labels,omitempty"`
 }

 // swagger:model
 type PrometheusRule struct {
-	Alert         string            `yaml:"alert,omitempty"`
-	Expr          string            `yaml:"expr"`
-	For           *model.Duration   `yaml:"for,omitempty"`
-	KeepFiringFor *model.Duration   `yaml:"keep_firing_for,omitempty"`
-	Labels        map[string]string `yaml:"labels,omitempty"`
-	Annotations   map[string]string `yaml:"annotations,omitempty"`
-	Record        string            `yaml:"record,omitempty"`
+	Alert         string            `yaml:"alert,omitempty" json:"alert,omitempty"`
+	Expr          string            `yaml:"expr" json:"expr"`
+	For           *model.Duration   `yaml:"for,omitempty" json:"for,omitempty"`
+	KeepFiringFor *model.Duration   `yaml:"keep_firing_for,omitempty" json:"keep_firing_for,omitempty"`
+	Labels        map[string]string `yaml:"labels,omitempty" json:"labels,omitempty"`
+	Annotations   map[string]string `yaml:"annotations,omitempty" json:"annotations,omitempty"`
+	Record        string            `yaml:"record,omitempty" json:"record,omitempty"`
 }

 // swagger:parameters RouteConvertPrometheusDeleteRuleGroup RouteConvertPrometheusCortexDeleteRuleGroup RouteConvertPrometheusGetRuleGroup RouteConvertPrometheusCortexGetRuleGroup
--- a/pkg/services/ngalert/api/tooling/post.json
+++ b/pkg/services/ngalert/api/tooling/post.json
@ -3062,31 +3062,31 @@
  },
  "PrometheusRule": {
   "properties": {
-    "Alert": {
+    "alert": {
     "type": "string"
    },
-    "Annotations": {
+    "annotations": {
     "additionalProperties": {
      "type": "string"
     },
     "type": "object"
    },
-    "Expr": {
+    "expr": {
     "type": "string"
    },
-    "For": {
+    "for": {
     "type": "string"
    },
-    "KeepFiringFor": {
+    "keep_firing_for": {
     "type": "string"
    },
-    "Labels": {
+    "labels": {
     "additionalProperties": {
      "type": "string"
     },
     "type": "object"
    },
-    "Record": {
+    "record": {
     "type": "string"
    }
   },
@ -3094,26 +3094,26 @@
  },
  "PrometheusRuleGroup": {
   "properties": {
-    "Interval": {
+    "interval": {
     "$ref": "#/definitions/Duration"
    },
-    "Labels": {
+    "labels": {
     "additionalProperties": {
      "type": "string"
     },
     "type": "object"
    },
-    "Limit": {
+    "limit": {
     "format": "int64",
     "type": "integer"
    },
-    "Name": {
+    "name": {
     "type": "string"
    },
-    "QueryOffset": {
+    "query_offset": {
     "type": "string"
    },
-    "Rules": {
+    "rules": {
     "items": {
      "$ref": "#/definitions/PrometheusRule"
     },
--- a/pkg/services/ngalert/api/tooling/spec.json
+++ b/pkg/services/ngalert/api/tooling/spec.json
@ -7236,31 +7236,31 @@
    "PrometheusRule": {
      "type": "object",
      "properties": {
-        "Alert": {
+        "alert": {
          "type": "string"
        },
-        "Annotations": {
+        "annotations": {
          "type": "object",
          "additionalProperties": {
            "type": "string"
          }
        },
-        "Expr": {
+        "expr": {
          "type": "string"
        },
-        "For": {
+        "for": {
          "type": "string"
        },
-        "KeepFiringFor": {
+        "keep_firing_for": {
          "type": "string"
        },
-        "Labels": {
+        "labels": {
          "type": "object",
          "additionalProperties": {
            "type": "string"
          }
        },
-        "Record": {
+        "record": {
          "type": "string"
        }
      }
@ -7268,26 +7268,26 @@
    "PrometheusRuleGroup": {
      "type": "object",
      "properties": {
-        "Interval": {
+        "interval": {
          "$ref": "#/definitions/Duration"
        },
-        "Labels": {
+        "labels": {
          "type": "object",
          "additionalProperties": {
            "type": "string"
          }
        },
-        "Limit": {
+        "limit": {
          "type": "integer",
          "format": "int64"
        },
-        "Name": {
+        "name": {
          "type": "string"
        },
-        "QueryOffset": {
+        "query_offset": {
          "type": "string"
        },
-        "Rules": {
+        "rules": {
          "type": "array",
          "items": {
            "$ref": "#/definitions/PrometheusRule"
--- a/pkg/services/ngalert/prom/convert.go
+++ b/pkg/services/ngalert/prom/convert.go
@ -236,6 +236,13 @@ func (p *Converter) convertRule(orgID int64, namespaceUID string, promGroup Prom
 	maps.Copy(labels, promGroup.Labels)
 	maps.Copy(labels, rule.Labels)

+	// Save the merged group-level + rule-level labels to the original rule,
+	// to ensure that they are saved to the original YAML rule definition.
+	if rule.Labels == nil {
+		rule.Labels = make(map[string]string)
+	}
+	maps.Copy(rule.Labels, labels)
+
 	// Add a special label to indicate that this rule was converted from a Prometheus rule.
 	labels[models.ConvertedPrometheusRuleLabel] = "true"

--- a/pkg/services/ngalert/prom/convert_test.go
+++ b/pkg/services/ngalert/prom/convert_test.go
@ -347,6 +347,12 @@ func TestPrometheusRulesToGrafana(t *testing.T) {
 				require.Equal(t, models.OkErrState, grafanaRule.ExecErrState)
 				require.Equal(t, models.OK, grafanaRule.NoDataState)

+				// Update the rule with the group-level labels,
+				// to test that they are saved to the rule definition.
+				mergedLabels := make(map[string]string)
+				maps.Copy(mergedLabels, tc.promGroup.Labels)
+				maps.Copy(mergedLabels, promRule.Labels)
+				promRule.Labels = mergedLabels
 				originalRuleDefinition, err := yaml.Marshal(promRule)
 				require.NoError(t, err)
 				require.Equal(t, string(originalRuleDefinition), grafanaRule.Metadata.PrometheusStyleRule.OriginalRuleDefinition)
--- a/pkg/tests/api/alerting/api_convert_prometheus_test.go
+++ b/pkg/tests/api/alerting/api_convert_prometheus_test.go
@ -2,6 +2,7 @@ package alerting

 import (
 	"encoding/json"
+	"maps"
 	"net/http"
 	"testing"
 	"time"
@ -1141,3 +1142,66 @@ func TestIntegrationConvertPrometheusEndpoints_Delete(t *testing.T) {
 		runTest(t, true)
 	})
 }
+
+func TestIntegrationConvertPrometheusEndpoints_GroupLabels(t *testing.T) {
+	testinfra.SQLiteIntegrationTest(t)
+
+	dir, gpath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
+		DisableLegacyAlerting: true,
+		EnableUnifiedAlerting: true,
+		DisableAnonymous:      true,
+		AppModeProduction:     true,
+		EnableRecordingRules:  true,
+	})
+
+	grafanaListedAddr, _ := testinfra.StartGrafanaEnv(t, dir, gpath)
+	apiClient := newAlertingApiClient(grafanaListedAddr, "admin", "admin")
+
+	ds := apiClient.CreateDatasource(t, datasources.DS_PROMETHEUS)
+
+	testGroup := apimodels.PrometheusRuleGroup{
+		Name:     "test-group-with-labels",
+		Interval: prommodel.Duration(60 * time.Second),
+		Labels: map[string]string{
+			"group_label": "value-1",
+		},
+		Rules: []apimodels.PrometheusRule{
+			{
+				Alert: "TestAlert",
+				Expr:  "up == 0",
+				For:   util.Pointer(prommodel.Duration(2 * time.Minute)),
+				Labels: map[string]string{
+					"rule_label": "value-2",
+				},
+				Annotations: map[string]string{
+					"annotation-1": "annotation-value",
+				},
+			},
+		},
+	}
+
+	namespace := "test-namespace-1"
+	namespaceUID := util.GenerateShortUID()
+	apiClient.CreateFolder(t, namespaceUID, namespace)
+
+	apiClient.ConvertPrometheusPostRuleGroup(t, namespace, ds.Body.Datasource.UID, testGroup, nil)
+
+	expectedLabels := make(map[string]string)
+	maps.Copy(expectedLabels, testGroup.Labels)
+	maps.Copy(expectedLabels, testGroup.Rules[0].Labels)
+
+	// Verify the Import API returns the expected merged format
+	group := apiClient.ConvertPrometheusGetRuleGroupRules(t, namespace, testGroup.Name, nil)
+	testGroup.Labels = nil
+	testGroup.Rules[0].Labels = expectedLabels
+	require.Equal(t, testGroup, group)
+
+	// Grafana should return the additional internal label
+	expectedLabels[models.ConvertedPrometheusRuleLabel] = "true"
+	ruleGroup, _, _ := apiClient.GetRulesGroupWithStatus(t, namespaceUID, testGroup.Name)
+	require.Len(t, ruleGroup.Rules, 1)
+
+	rule := ruleGroup.Rules[0]
+
+	require.Equal(t, expectedLabels, rule.Labels)
+}
--- a/public/api-merged.json
+++ b/public/api-merged.json
@ -18714,31 +18714,31 @@
    "PrometheusRule": {
      "type": "object",
      "properties": {
-        "Alert": {
+        "alert": {
          "type": "string"
        },
-        "Annotations": {
+        "annotations": {
          "type": "object",
          "additionalProperties": {
            "type": "string"
          }
        },
-        "Expr": {
+        "expr": {
          "type": "string"
        },
-        "For": {
+        "for": {
          "type": "string"
        },
-        "KeepFiringFor": {
+        "keep_firing_for": {
          "type": "string"
        },
-        "Labels": {
+        "labels": {
          "type": "object",
          "additionalProperties": {
            "type": "string"
          }
        },
-        "Record": {
+        "record": {
          "type": "string"
        }
      }
@ -18746,26 +18746,26 @@
    "PrometheusRuleGroup": {
      "type": "object",
      "properties": {
-        "Interval": {
+        "interval": {
          "$ref": "#/definitions/Duration"
        },
-        "Labels": {
+        "labels": {
          "type": "object",
          "additionalProperties": {
            "type": "string"
          }
        },
-        "Limit": {
+        "limit": {
          "type": "integer",
          "format": "int64"
        },
-        "Name": {
+        "name": {
          "type": "string"
        },
-        "QueryOffset": {
+        "query_offset": {
          "type": "string"
        },
-        "Rules": {
+        "rules": {
          "type": "array",
          "items": {
            "$ref": "#/definitions/PrometheusRule"
--- a/public/openapi3.json
+++ b/public/openapi3.json
@ -8763,31 +8763,31 @@
      },
      "PrometheusRule": {
        "properties": {
-          "Alert": {
+          "alert": {
            "type": "string"
          },
-          "Annotations": {
+          "annotations": {
            "additionalProperties": {
              "type": "string"
            },
            "type": "object"
          },
-          "Expr": {
+          "expr": {
            "type": "string"
          },
-          "For": {
+          "for": {
            "type": "string"
          },
-          "KeepFiringFor": {
+          "keep_firing_for": {
            "type": "string"
          },
-          "Labels": {
+          "labels": {
            "additionalProperties": {
              "type": "string"
            },
            "type": "object"
          },
-          "Record": {
+          "record": {
            "type": "string"
          }
        },
@ -8795,26 +8795,26 @@
      },
      "PrometheusRuleGroup": {
        "properties": {
-          "Interval": {
+          "interval": {
            "$ref": "#/components/schemas/Duration"
          },
-          "Labels": {
+          "labels": {
            "additionalProperties": {
              "type": "string"
            },
            "type": "object"
          },
-          "Limit": {
+          "limit": {
            "format": "int64",
            "type": "integer"
          },
-          "Name": {
+          "name": {
            "type": "string"
          },
-          "QueryOffset": {
+          "query_offset": {
            "type": "string"
          },
-          "Rules": {
+          "rules": {
            "items": {
              "$ref": "#/components/schemas/PrometheusRule"
            },