[release-12.0.2] Alerting: Fix group-level labels and query_offset in the import API (#106392)

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. (cherry picked from commit f7a52bc04e)
3 weeks ago · c53915cb87
parent d6b7bb6759
commit c53915cb87
11 changed files with 174 additions and 84 deletions
--- a/pkg/services/ngalert/api/api_convert_prometheus.go
+++ b/pkg/services/ngalert/api/api_convert_prometheus.go
@ -462,6 +462,9 @@ func (srv *ConvertPrometheusSrv) convertToGrafanaRuleGroup(
 		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
@ -1012,12 +1012,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},
+		QueryOffset: &queryOffset,
 	}

 	promGroup3 := apimodels.PrometheusRuleGroup{
@ -1053,10 +1058,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
@ -3030,31 +3030,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"
    }
   },
@ -3062,26 +3062,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
@ -222,23 +222,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
@ -3030,31 +3030,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"
    }
   },
@ -3062,26 +3062,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
@ -7131,31 +7131,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"
        }
      }
@ -7163,26 +7163,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
@ -235,6 +235,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)
+
 	originalRuleDefinition, err := yaml.Marshal(rule)
 	if err != nil {
 		return models.AlertRule{}, fmt.Errorf("failed to marshal original rule definition: %w", err)
--- a/pkg/services/ngalert/prom/convert_test.go
+++ b/pkg/services/ngalert/prom/convert_test.go
@ -346,6 +346,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"
@ -1195,3 +1196,69 @@ 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, env := testinfra.StartGrafanaEnv(t, dir, gpath)
+	createUser(t, env.SQLStore, env.Cfg, user.CreateUserCommand{
+		DefaultOrgRole: string(org.RoleAdmin),
+		Password:       "admin",
+		Login:          "admin",
+	})
+	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)
+
+	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
@ -18705,31 +18705,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"
        }
      }
@ -18737,26 +18737,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
@ -8766,31 +8766,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"
          }
        },
@ -8798,26 +8798,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"
            },