From b8d885c6b8e37829a2463c1197a7a7a7a5e9c8fb Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Thu, 12 Oct 2023 08:37:18 -0400 Subject: [PATCH 1/3] relax constraints for nested policies --- .../resources/grafana_notification_policy/resource.tf | 1 - .../grafana/resource_alerting_notification_policy.go | 8 +++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/examples/resources/grafana_notification_policy/resource.tf b/examples/resources/grafana_notification_policy/resource.tf index 7a4f37a4e..b7a600622 100644 --- a/examples/resources/grafana_notification_policy/resource.tf +++ b/examples/resources/grafana_notification_policy/resource.tf @@ -31,7 +31,6 @@ resource "grafana_notification_policy" "my_notification_policy" { value = "myvalue" } contact_point = grafana_contact_point.a_contact_point.name - group_by = ["alertname"] continue = true mute_timings = [grafana_mute_timing.a_mute_timing.name] diff --git a/internal/resources/grafana/resource_alerting_notification_policy.go b/internal/resources/grafana/resource_alerting_notification_policy.go index 6595fce06..67c4ccf8f 100644 --- a/internal/resources/grafana/resource_alerting_notification_policy.go +++ b/internal/resources/grafana/resource_alerting_notification_policy.go @@ -5,10 +5,11 @@ import ( "fmt" gapi "github.com/grafana/grafana-api-golang-client" - "github.com/grafana/terraform-provider-grafana/internal/common" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + + "github.com/grafana/terraform-provider-grafana/internal/common" ) func ResourceNotificationPolicy() *schema.Resource { @@ -96,8 +97,9 @@ func policySchema(depth uint) *schema.Resource { }, "group_by": { Type: schema.TypeList, - Required: true, - Description: "A list of alert labels to group alerts into notifications by. Use the special label `...` to group alerts by all labels, effectively disabling grouping.", + Required: depth == 1, + Optional: depth > 1, + Description: "A list of alert labels to group alerts into notifications by. Use the special label `...` to group alerts by all labels, effectively disabling grouping. Required for root policy only. If empty, the parent grouping is used.", Elem: &schema.Schema{ Type: schema.TypeString, }, From 89263ce09325d9eb7f13992277d1b6e71c2cd472 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Thu, 12 Oct 2023 08:43:06 -0400 Subject: [PATCH 2/3] docs --- docs/resources/notification_policy.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/resources/notification_policy.md b/docs/resources/notification_policy.md index 81237a113..e0a8be400 100644 --- a/docs/resources/notification_policy.md +++ b/docs/resources/notification_policy.md @@ -56,7 +56,6 @@ resource "grafana_notification_policy" "my_notification_policy" { value = "myvalue" } contact_point = grafana_contact_point.a_contact_point.name - group_by = ["alertname"] continue = true mute_timings = [grafana_mute_timing.a_mute_timing.name] @@ -112,11 +111,11 @@ resource "grafana_notification_policy" "my_notification_policy" { Required: - `contact_point` (String) The contact point to route notifications that match this rule to. -- `group_by` (List of String) A list of alert labels to group alerts into notifications by. Use the special label `...` to group alerts by all labels, effectively disabling grouping. Optional: - `continue` (Boolean) Whether to continue matching subsequent rules if an alert matches the current rule. Otherwise, the rule will be 'consumed' by the first policy to match it. +- `group_by` (List of String) A list of alert labels to group alerts into notifications by. Use the special label `...` to group alerts by all labels, effectively disabling grouping. Required for root policy only. If empty, the parent grouping is used. - `group_interval` (String) Minimum time interval between two notifications for the same group. Default is 5 minutes. - `group_wait` (String) Time to wait to buffer alerts of the same group before sending a notification. Default is 30 seconds. - `matcher` (Block List) Describes which labels this rule should match. When multiple matchers are supplied, an alert must match ALL matchers to be accepted by this policy. When no matchers are supplied, the rule will match all alert instances. (see [below for nested schema](#nestedblock--policy--matcher)) @@ -140,11 +139,11 @@ Required: Required: - `contact_point` (String) The contact point to route notifications that match this rule to. -- `group_by` (List of String) A list of alert labels to group alerts into notifications by. Use the special label `...` to group alerts by all labels, effectively disabling grouping. Optional: - `continue` (Boolean) Whether to continue matching subsequent rules if an alert matches the current rule. Otherwise, the rule will be 'consumed' by the first policy to match it. +- `group_by` (List of String) A list of alert labels to group alerts into notifications by. Use the special label `...` to group alerts by all labels, effectively disabling grouping. Required for root policy only. If empty, the parent grouping is used. - `group_interval` (String) Minimum time interval between two notifications for the same group. Default is 5 minutes. - `group_wait` (String) Time to wait to buffer alerts of the same group before sending a notification. Default is 30 seconds. - `matcher` (Block List) Describes which labels this rule should match. When multiple matchers are supplied, an alert must match ALL matchers to be accepted by this policy. When no matchers are supplied, the rule will match all alert instances. (see [below for nested schema](#nestedblock--policy--policy--matcher)) @@ -168,11 +167,11 @@ Required: Required: - `contact_point` (String) The contact point to route notifications that match this rule to. -- `group_by` (List of String) A list of alert labels to group alerts into notifications by. Use the special label `...` to group alerts by all labels, effectively disabling grouping. Optional: - `continue` (Boolean) Whether to continue matching subsequent rules if an alert matches the current rule. Otherwise, the rule will be 'consumed' by the first policy to match it. +- `group_by` (List of String) A list of alert labels to group alerts into notifications by. Use the special label `...` to group alerts by all labels, effectively disabling grouping. Required for root policy only. If empty, the parent grouping is used. - `group_interval` (String) Minimum time interval between two notifications for the same group. Default is 5 minutes. - `group_wait` (String) Time to wait to buffer alerts of the same group before sending a notification. Default is 30 seconds. - `matcher` (Block List) Describes which labels this rule should match. When multiple matchers are supplied, an alert must match ALL matchers to be accepted by this policy. When no matchers are supplied, the rule will match all alert instances. (see [below for nested schema](#nestedblock--policy--policy--policy--matcher)) @@ -196,7 +195,7 @@ Required: Required: - `contact_point` (String) The contact point to route notifications that match this rule to. -- `group_by` (List of String) A list of alert labels to group alerts into notifications by. Use the special label `...` to group alerts by all labels, effectively disabling grouping. +- `group_by` (List of String) A list of alert labels to group alerts into notifications by. Use the special label `...` to group alerts by all labels, effectively disabling grouping. Required for root policy only. If empty, the parent grouping is used. Optional: From 24ddc8774ea2a2d3c4c1ca5c7e98caa3e10d8724 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Thu, 12 Oct 2023 09:29:19 -0400 Subject: [PATCH 3/3] change pack\unpack + fix tests --- .../resource_alerting_notification_policy.go | 13 +++++++++++-- .../resource_alerting_notification_policy_test.go | 7 ++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/internal/resources/grafana/resource_alerting_notification_policy.go b/internal/resources/grafana/resource_alerting_notification_policy.go index 67c4ccf8f..b2af99cc0 100644 --- a/internal/resources/grafana/resource_alerting_notification_policy.go +++ b/internal/resources/grafana/resource_alerting_notification_policy.go @@ -253,9 +253,12 @@ func packNotifPolicy(npt gapi.NotificationPolicyTree, data *schema.ResourceData) func packSpecificPolicy(p gapi.SpecificPolicy, depth uint) interface{} { result := map[string]interface{}{ "contact_point": p.Receiver, - "group_by": p.GroupBy, "continue": p.Continue, } + if len(p.GroupBy) > 0 { + result["group_by"] = p.GroupBy + } + if p.ObjectMatchers != nil && len(p.ObjectMatchers) > 0 { matchers := make([]interface{}, 0, len(p.ObjectMatchers)) for _, m := range p.ObjectMatchers { @@ -325,9 +328,15 @@ func unpackNotifPolicy(data *schema.ResourceData) (gapi.NotificationPolicyTree, func unpackSpecificPolicy(p interface{}) (gapi.SpecificPolicy, error) { json := p.(map[string]interface{}) + + var groupBy []string + if g, ok := json["group_by"]; ok { + groupBy = common.ListToStringSlice(g.([]interface{})) + } + policy := gapi.SpecificPolicy{ Receiver: json["contact_point"].(string), - GroupBy: common.ListToStringSlice(json["group_by"].([]interface{})), + GroupBy: groupBy, Continue: json["continue"].(bool), } diff --git a/internal/resources/grafana/resource_alerting_notification_policy_test.go b/internal/resources/grafana/resource_alerting_notification_policy_test.go index 65639c0d9..7d372e70f 100644 --- a/internal/resources/grafana/resource_alerting_notification_policy_test.go +++ b/internal/resources/grafana/resource_alerting_notification_policy_test.go @@ -5,10 +5,11 @@ import ( "testing" gapi "github.com/grafana/grafana-api-golang-client" - "github.com/grafana/terraform-provider-grafana/internal/common" - "github.com/grafana/terraform-provider-grafana/internal/testutils" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + + "github.com/grafana/terraform-provider-grafana/internal/common" + "github.com/grafana/terraform-provider-grafana/internal/testutils" ) func TestAccNotificationPolicy_basic(t *testing.T) { @@ -38,7 +39,7 @@ func TestAccNotificationPolicy_basic(t *testing.T) { resource.TestCheckResourceAttr("grafana_notification_policy.my_notification_policy", "policy.0.matcher.0.label", "mylabel"), resource.TestCheckResourceAttr("grafana_notification_policy.my_notification_policy", "policy.0.matcher.0.match", "="), resource.TestCheckResourceAttr("grafana_notification_policy.my_notification_policy", "policy.0.matcher.0.value", "myvalue"), - resource.TestCheckResourceAttr("grafana_notification_policy.my_notification_policy", "policy.0.group_by.0", "alertname"), + resource.TestCheckNoResourceAttr("grafana_notification_policy.my_notification_policy", "policy.0.group_by"), resource.TestCheckResourceAttr("grafana_notification_policy.my_notification_policy", "policy.0.continue", "true"), resource.TestCheckResourceAttr("grafana_notification_policy.my_notification_policy", "policy.0.mute_timings.0", "Some Mute Timing"), resource.TestCheckResourceAttr("grafana_notification_policy.my_notification_policy", "policy.0.group_wait", "45s"),