From 80283761ade730971432858bda8501d3087436a9 Mon Sep 17 00:00:00 2001 From: Moustafa Baiou Date: Fri, 20 Dec 2024 17:57:37 -0500 Subject: [PATCH] Fix make alert rule uid settable to avoid accidental overwriting of existing rule history Unfortunately this is the only viable workaround. It will require users to manually set the uid for alert rules similar to how it is done in file provisioning. Addresses #1928 --- docs/resources/rule_group.md | 3 - .../grafana/resource_alerting_rule_group.go | 6 +- .../resource_alerting_rule_group_test.go | 81 +++++++++++++++++++ pkg/generate/generate_test.go | 13 ++- .../{resources.tf => resources.tf.tmpl} | 3 +- 5 files changed, 99 insertions(+), 7 deletions(-) rename pkg/generate/testdata/generate/alerting-in-org/{resources.tf => resources.tf.tmpl} (97%) diff --git a/docs/resources/rule_group.md b/docs/resources/rule_group.md index 7a5454a1c..5f98a2a8c 100644 --- a/docs/resources/rule_group.md +++ b/docs/resources/rule_group.md @@ -146,9 +146,6 @@ Optional: - `no_data_state` (String) Describes what state to enter when the rule's query returns No Data. Options are OK, NoData, KeepLast, and Alerting. Defaults to `NoData`. - `notification_settings` (Block List, Max: 1) Notification settings for the rule. If specified, it overrides the notification policies. Available since Grafana 10.4, requires feature flag 'alertingSimplifiedRouting' to be enabled. (see [below for nested schema](#nestedblock--rule--notification_settings)) - `record` (Block List, Max: 1) Settings for a recording rule. Available since Grafana 11.2, requires feature flag 'grafanaManagedRecordingRules' to be enabled. (see [below for nested schema](#nestedblock--rule--record)) - -Read-Only: - - `uid` (String) The unique identifier of the alert rule. diff --git a/internal/resources/grafana/resource_alerting_rule_group.go b/internal/resources/grafana/resource_alerting_rule_group.go index d665e5412..a027fae09 100644 --- a/internal/resources/grafana/resource_alerting_rule_group.go +++ b/internal/resources/grafana/resource_alerting_rule_group.go @@ -83,6 +83,7 @@ This resource requires Grafana 9.1.0 or later. Schema: map[string]*schema.Schema{ "uid": { Type: schema.TypeString, + Optional: true, Computed: true, Description: "The unique identifier of the alert rule.", }, @@ -383,11 +384,14 @@ func putAlertRuleGroup(ctx context.Context, data *schema.ResourceData, meta inte return retry.NonRetryableError(err) } - // Check if a rule with the same name already exists within the same rule group + // Check if a rule with the same name or uid already exists within the same rule group for _, r := range rules { if *r.Title == *ruleToApply.Title { return retry.NonRetryableError(fmt.Errorf("rule with name %q is defined more than once", *ruleToApply.Title)) } + if ruleToApply.UID != "" && r.UID == ruleToApply.UID { + return retry.NonRetryableError(fmt.Errorf("rule with UID %q is defined more than once. Rules with name %q and %q have the same uid", ruleToApply.UID, *r.Title, *ruleToApply.Title)) + } } // Check if a rule with the same name already exists within the same folder (changing the ordering is allowed within the same rule group) diff --git a/internal/resources/grafana/resource_alerting_rule_group_test.go b/internal/resources/grafana/resource_alerting_rule_group_test.go index 5052e8f4a..57bb8b487 100644 --- a/internal/resources/grafana/resource_alerting_rule_group_test.go +++ b/internal/resources/grafana/resource_alerting_rule_group_test.go @@ -458,6 +458,87 @@ resource "grafana_rule_group" "first" { }) } +func TestAccAlertRule_ruleUIDConflict(t *testing.T) { + testutils.CheckOSSTestsEnabled(t, ">=9.1.0") + + name := acctest.RandString(10) + uid := acctest.RandString(10) + + resource.ParallelTest(t, resource.TestCase{ + ProtoV5ProviderFactories: testutils.ProtoV5ProviderFactories, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(` +resource "grafana_organization" "test" { + name = "%[1]s" +} + +resource "grafana_folder" "first" { + org_id = grafana_organization.test.id + title = "%[1]s-first" +} + +resource "grafana_rule_group" "first" { + org_id = grafana_organization.test.id + name = "alert rule group" + folder_uid = grafana_folder.first.uid + interval_seconds = 60 + rule { + name = "%[1]s" + uid = "%[2]s" + for = "2m" + condition = "B" + no_data_state = "NoData" + exec_err_state = "Alerting" + is_paused = false + data { + ref_id = "A" + query_type = "" + relative_time_range { + from = 600 + to = 0 + } + datasource_uid = "PD8C576611E62080A" + model = jsonencode({ + hide = false + intervalMs = 1000 + maxDataPoints = 43200 + refId = "A" + }) + } + } + rule { + name = "%[1]s 2" + uid = "%[2]s" + for = "2m" + condition = "B" + no_data_state = "NoData" + exec_err_state = "Alerting" + is_paused = false + data { + ref_id = "A" + query_type = "" + relative_time_range { + from = 600 + to = 0 + } + datasource_uid = "PD8C576611E62080A" + model = jsonencode({ + hide = false + intervalMs = 1000 + maxDataPoints = 43200 + refId = "A" + }) + } + } +} + `, name, uid), + ExpectError: regexp.MustCompile(`rule with UID "` + uid + `" is defined more than once. Rules with name "` + name + `" and "` + name + ` 2" have the same uid`), + }, + }, + }) +} + func TestAccAlertRule_moveRules(t *testing.T) { testutils.CheckOSSTestsEnabled(t, ">=9.1.0") diff --git a/pkg/generate/generate_test.go b/pkg/generate/generate_test.go index e41c893b9..417634dfc 100644 --- a/pkg/generate/generate_test.go +++ b/pkg/generate/generate_test.go @@ -96,6 +96,7 @@ func TestAccGenerate(t *testing.T) { // Install Terraform to a temporary directory to avoid reinstalling it for each test case. installDir := t.TempDir() + var AlertRule1ID string cases := []generateTestCase{ { name: "dashboard", @@ -241,10 +242,18 @@ func TestAccGenerate(t *testing.T) { } }, check: func(t *testing.T, tempDir string) { - assertFiles(t, tempDir, "testdata/generate/alerting-in-org", []string{ + templateAttrs := map[string]string{ + "AlertRule1ID": AlertRule1ID, + } + assertFilesWithTemplating(t, tempDir, "testdata/generate/alerting-in-org", []string{ ".terraform", ".terraform.lock.hcl", - }) + }, templateAttrs) + }, + stateCheck: func(s *terraform.State) error { + // Save the ID of the first alert rule for later use + AlertRule1ID = s.RootModule().Resources["grafana_rule_group.my_alert_rule"].Primary.Attributes["rule[0].uid"] + return nil }, }, { diff --git a/pkg/generate/testdata/generate/alerting-in-org/resources.tf b/pkg/generate/testdata/generate/alerting-in-org/resources.tf.tmpl similarity index 97% rename from pkg/generate/testdata/generate/alerting-in-org/resources.tf rename to pkg/generate/testdata/generate/alerting-in-org/resources.tf.tmpl index 4ebbe2970..8e34de30c 100644 --- a/pkg/generate/testdata/generate/alerting-in-org/resources.tf +++ b/pkg/generate/testdata/generate/alerting-in-org/resources.tf.tmpl @@ -47,7 +47,7 @@ resource "grafana_folder" "_2_alert-rule-folder" { resource "grafana_message_template" "_2_My_Reusable_Template" { name = "My Reusable Template" org_id = grafana_organization.alerting-org.id - template = "{{define \"My Reusable Template\" }}\n template content\n{{ end }}" + template = "\{\{define \"My Reusable Template\" \}\}\n template content\n\{\{ end \}\}" } # __generated__ by Terraform from "2:My Mute Timing" @@ -110,6 +110,7 @@ resource "grafana_rule_group" "_2_alert-rule-folder_My_Rule_Group" { } name = "My Alert Rule 1" no_data_state = "NoData" + uid = "{{ .AlertRule1ID }}" data { datasource_uid = "PD8C576611E62080A" model = jsonencode({