public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Alerting: Fix updating Prometheus definition in the metadata (#101440)

Browse Source 
Initially, Metadata had only the EditorSettings, and HasMetadata was used to understand if the incoming update request had metadata in the body because it could be omitted if it was empty. For example, when the rule is updated via the provisioning API or has only false values. If it was in the request, we used that; if not, we used the metadata from the existing rule from the database. If the rule was updated via the AlertRuleService, we didn't change Metadata at all if the rule already existed.

But now, Metadata also has the Prometheus rule definition, and we always need to update it with the new version of the AlertRuleService when the rule exists in the DB and has the same UID. HasMetadata is renamed to HasEditorSettings to keep the old behaviour only for EditorSettings.

Now, the provisioning API and the conversion API will overwrite everything except EditorSettings with the new data.
This commit is contained in:
Alexander Akhmetov
2025-02-28 13:11:49 +02:00
committed by GitHub
parent c6c5f44eeb
commit ae2074ef55
6 changed files with 227 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/ngalert/api/api_ruler_validation.go
+3 -3
View File
@@ -314,7 +314,7 @@ func ValidateRuleGroup(
uids[rule.UID] = idx
}
var hasPause, isPaused, hasMetadata bool
var hasPause, isPaused, hasEditorSettings bool
original := ruleGroupConfig.Rules[idx]
if alert := original.GrafanaManagedAlert; alert != nil {
if alert.IsPaused != nil {
@@ -322,7 +322,7 @@ func ValidateRuleGroup(
hasPause = true
}
if alert.Metadata != nil {
hasMetadata = true
hasEditorSettings = true
}
}
@@ -331,7 +331,7 @@ func ValidateRuleGroup(
rule.RuleGroupIndex = idx + 1
ruleWithOptionals.AlertRule = *rule
ruleWithOptionals.HasPause = hasPause
ruleWithOptionals.HasMetadata = hasMetadata
ruleWithOptionals.HasEditorSettings = hasEditorSettings
result = append(result, &ruleWithOptionals)
}
pkg/services/ngalert/models/alert_rule.go
+8 -11
View File
@@ -327,8 +327,8 @@ type AlertRuleWithOptionals struct {
AlertRule
// This parameter is to know if an optional API field was sent and, therefore, patch it with the current field from
// DB in case it was not sent.
HasPause bool
HasMetadata bool
HasPause bool
HasEditorSettings bool
}
// AlertsRulesBy is a function that defines the ordering of alert rules.
@@ -902,9 +902,10 @@ func (c Condition) IsValid() bool {
// PatchPartialAlertRule patches `ruleToPatch` by `existingRule` following the rule that if a field of `ruleToPatch` is empty or has the default value, it is populated by the value of the corresponding field from `existingRule`.
// There are several exceptions:
// 1. Following fields are not patched and therefore will be ignored: AlertRule.ID, AlertRule.OrgID, AlertRule.Updated, AlertRule.Version, AlertRule.UID, AlertRule.DashboardUID, AlertRule.PanelID, AlertRule.Annotations and AlertRule.Labels
// 2. There are fields that are patched together:
// - AlertRule.Condition and AlertRule.Data
// 1. Following fields are not patched and therefore will be ignored: AlertRule.ID, AlertRule.OrgID, AlertRule.Updated, AlertRule.Version,
// AlertRule.UID, AlertRule.DashboardUID, AlertRule.PanelID, AlertRule.Annotations, AlertRule.Labels, AlertRule.Metadata (except for EditorSettings)
// 2. There are fields that are patched together:
// - AlertRule.Condition and AlertRule.Data
//
// If either of the pair is specified, neither is patched.
func PatchPartialAlertRule(existingRule *AlertRule, ruleToPatch *AlertRuleWithOptionals) {
@@ -936,12 +937,8 @@ func PatchPartialAlertRule(existingRule *AlertRule, ruleToPatch *AlertRuleWithOp
if !ruleToPatch.HasPause {
ruleToPatch.IsPaused = existingRule.IsPaused
}
// Currently metadata contains only editor settings, so we can just copy it.
// If we add more fields to metadata, we might need to handle them separately,
// and/or merge or update their values.
if !ruleToPatch.HasMetadata {
ruleToPatch.Metadata = existingRule.Metadata
if !ruleToPatch.HasEditorSettings {
ruleToPatch.Metadata.EditorSettings = existingRule.Metadata.EditorSettings
}
}
pkg/services/ngalert/models/alert_rule_test.go
+1 -1
View File
@@ -263,7 +263,7 @@ func TestPatchPartialAlertRule(t *testing.T) {
name: "No metadata",
mutator: func(r *AlertRuleWithOptionals) {
r.Metadata = AlertRuleMetadata{}
r.HasMetadata = false
r.HasEditorSettings = false
},
},
}
pkg/services/ngalert/provisioning/alert_rules_test.go
+125
View File
@@ -229,6 +229,97 @@ func TestAlertRuleService(t *testing.T) {
require.Equal(t, ruleMetadata, readGroup.Rules[0].Metadata)
})
t.Run("updating a group with editor settings should override its prometheus rule definition", func(t *testing.T) {
namespaceUID := "my-namespace"
groupTitle := "test-group-123"
// create the rule group via the rule store, to persist the editor settings
rule := createTestRule(util.GenerateShortUID(), groupTitle, orgID, namespaceUID)
ruleMetadata := models.AlertRuleMetadata{
EditorSettings: models.EditorSettings{
SimplifiedQueryAndExpressionsSection: true,
},
PrometheusStyleRule: &models.PrometheusStyleRule{
OriginalRuleDefinition: "old",
},
}
rule.Metadata = ruleMetadata
r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{rule})
require.NoError(t, err)
require.Len(t, r, 1)
// Set the UID for the rule to update it
rule.UID = r[0].UID
// clear the editor settings in the metadata to check that the existing setting is not overridden
rule.Metadata = models.AlertRuleMetadata{
PrometheusStyleRule: &models.PrometheusStyleRule{
OriginalRuleDefinition: "new",
},
}
// Now update the rule group with the rule to update its metadata
group := models.AlertRuleGroup{
Title: groupTitle,
Interval: 60,
FolderUID: namespaceUID,
Rules: []models.AlertRule{rule},
}
err = ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI)
require.NoError(t, err)
readGroup, err := ruleService.GetRuleGroup(context.Background(), u, namespaceUID, groupTitle)
require.NoError(t, err)
require.NotEmpty(t, readGroup.Rules)
require.Len(t, readGroup.Rules, 1)
// check that the editor settings are still there
require.True(t, readGroup.Rules[0].Metadata.EditorSettings.SimplifiedQueryAndExpressionsSection)
// check the new prometheus rule definition
require.Equal(t, "new", readGroup.Rules[0].Metadata.PrometheusStyleRule.OriginalRuleDefinition)
})
t.Run("updating a group should override its prometheus rule definition", func(t *testing.T) {
namespaceUID := "my-namespace"
groupTitle := "test-group-123"
// create the rule group via the rule store, to persist the editor settings
rule := createTestRule(util.GenerateShortUID(), groupTitle, orgID, namespaceUID)
ruleMetadata := models.AlertRuleMetadata{
PrometheusStyleRule: &models.PrometheusStyleRule{
OriginalRuleDefinition: "old",
},
}
rule.Metadata = ruleMetadata
r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{rule})
require.NoError(t, err)
require.Len(t, r, 1)
// Set the UID for the rule to update it
rule.UID = r[0].UID
// make the metadata empty
rule.Metadata = models.AlertRuleMetadata{}
// Now update the rule group with the rule to update its metadata
group := models.AlertRuleGroup{
Title: groupTitle,
Interval: 60,
FolderUID: namespaceUID,
Rules: []models.AlertRule{rule},
}
err = ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI)
require.NoError(t, err)
readGroup, err := ruleService.GetRuleGroup(context.Background(), u, namespaceUID, groupTitle)
require.NoError(t, err)
require.NotEmpty(t, readGroup.Rules)
require.Len(t, readGroup.Rules, 1)
// check that the prometheus rule definition is empty
require.Nil(t, readGroup.Rules[0].Metadata.PrometheusStyleRule)
})
t.Run("updating a rule should not override its editor settings", func(t *testing.T) {
rule := createTestRule(util.GenerateShortUID(), "my-group", orgID, "my-folder")
ruleMetadata := models.AlertRuleMetadata{
@@ -1508,6 +1599,40 @@ func TestReplaceGroup(t *testing.T) {
require.Len(t, updates, 1)
})
})
t.Run("alert rule metadata should be updated correctly", func(t *testing.T) {
service, _, _, _ := initServiceWithData(t)
rule := dummyRule("test#3", orgID)
// the rule must have a UID to be updated, otherwise it will be created as new
// and the previous version will be deleted
rule.UID = util.GenerateShortUID()
rule.Metadata = models.AlertRuleMetadata{
EditorSettings: models.EditorSettings{
SimplifiedQueryAndExpressionsSection: true,
},
PrometheusStyleRule: &models.PrometheusStyleRule{
OriginalRuleDefinition: "old",
},
}
group := models.AlertRuleGroup{
Title: rule.RuleGroup,
Interval: rule.IntervalSeconds,
FolderUID: rule.NamespaceUID,
Rules: []models.AlertRule{rule},
}
err := service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceNone)
require.NoError(t, err)
rule.Metadata.PrometheusStyleRule.OriginalRuleDefinition = "new"
err = service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceNone)
require.NoError(t, err)
rule, _, err = service.GetAlertRule(context.Background(), u, rule.UID)
require.NoError(t, err)
require.Equal(t, "new", rule.Metadata.PrometheusStyleRule.OriginalRuleDefinition)
})
}
func TestDeleteRuleGroup(t *testing.T) {
pkg/services/ngalert/store/deltas_test.go
+2 -2
View File
@@ -81,7 +81,7 @@ func TestCalculateChanges(t *testing.T) {
submittedMap := groupByUID(t, rules)
submitted := make([]*models.AlertRuleWithOptionals, 0, len(rules))
for _, rule := range rules {
submitted = append(submitted, &models.AlertRuleWithOptionals{AlertRule: *rule, HasMetadata: true})
submitted = append(submitted, &models.AlertRuleWithOptionals{AlertRule: *rule, HasEditorSettings: true})
}
fakeStore := fakes.NewRuleStore(t)
@@ -216,7 +216,7 @@ func TestCalculateChanges(t *testing.T) {
submittedMap := groupByUID(t, rules)
submitted := make([]*models.AlertRuleWithOptionals, 0, len(rules))
for _, rule := range rules {
submitted = append(submitted, &models.AlertRuleWithOptionals{AlertRule: *rule, HasMetadata: true})
submitted = append(submitted, &models.AlertRuleWithOptionals{AlertRule: *rule, HasEditorSettings: true})
}
changes, err := CalculateChanges(context.Background(), fakeStore, groupKey, submitted)
pkg/tests/api/alerting/api_convert_prometheus_test.go
+88
View File
@@ -213,6 +213,94 @@ func TestIntegrationConvertPrometheusEndpoints(t *testing.T) {
})
}
func TestIntegrationConvertPrometheusEndpoints_UpdateRule(t *testing.T) {
runTest := func(t *testing.T, enableLokiPaths bool) {
testinfra.SQLiteIntegrationTest(t)
// Setup Grafana and its Database
dir, gpath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
DisableLegacyAlerting: true,
EnableUnifiedAlerting: true,
DisableAnonymous: true,
AppModeProduction: true,
EnableFeatureToggles: []string{"alertingConversionAPI"},
})
grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, gpath)
// Create a user to make authenticated requests
createUser(t, env.SQLStore, env.Cfg, user.CreateUserCommand{
DefaultOrgRole: string(org.RoleAdmin),
Password: "password",
Login: "admin",
})
apiClient := newAlertingApiClient(grafanaListedAddr, "admin", "password")
apiClient.prometheusConversionUseLokiPaths = enableLokiPaths
createUser(t, env.SQLStore, env.Cfg, user.CreateUserCommand{
DefaultOrgRole: string(org.RoleViewer),
Password: "password",
Login: "viewer",
})
namespace1 := "test-namespace-1"
ds := apiClient.CreateDatasource(t, datasources.DS_PROMETHEUS)
promGroup := apimodels.PrometheusRuleGroup{
Name: "test-group-for-an-update",
Interval: prommodel.Duration(60 * time.Second),
Rules: []apimodels.PrometheusRule{
{
Alert: "HighDiskUsage",
Expr: "disk_usage > 80",
For: util.Pointer(prommodel.Duration(1 * time.Minute)),
Labels: map[string]string{
"severity": "low",
"team": "alerting",
},
Annotations: map[string]string{
"annotation-5": "value-5",
},
},
},
}
t.Run("update a rule", func(t *testing.T) {
// Create the rule group
_, status, body := apiClient.ConvertPrometheusPostRuleGroup(t, namespace1, ds.Body.Datasource.UID, promGroup, nil)
requireStatusCode(t, http.StatusAccepted, status, body)
// Now get the group
group1 := apiClient.ConvertPrometheusGetRuleGroupRules(t, namespace1, promGroup.Name)
require.Equal(t, promGroup, group1)
// Update the rule group interval
promGroup.Interval = prommodel.Duration(30 * time.Second)
// Update the query
promGroup.Rules[0].Expr = "disk_usage > 90"
// Labels, and annotations too
promGroup.Rules[0].Labels["another-label"] = "something"
promGroup.Rules[0].Annotations["another-annotation"] = "also-something"
// Update the group
_, status, body = apiClient.ConvertPrometheusPostRuleGroup(t, namespace1, ds.Body.Datasource.UID, promGroup, nil)
requireStatusCode(t, http.StatusAccepted, status, body)
// Now get the group again and check that the rule group has been updated
group1 = apiClient.ConvertPrometheusGetRuleGroupRules(t, namespace1, promGroup.Name)
require.Equal(t, promGroup, group1)
})
}
t.Run("with the mimirtool paths", func(t *testing.T) {
runTest(t, false)
})
t.Run("with the cortextool Loki paths", func(t *testing.T) {
runTest(t, true)
})
}
func TestIntegrationConvertPrometheusEndpoints_Conflict(t *testing.T) {
runTest := func(t *testing.T, enableLokiPaths bool) {
testinfra.SQLiteIntegrationTest(t)