From 00c149a34fa23995ad53b6d20da7b667b100468d Mon Sep 17 00:00:00 2001 From: boneff Date: Mon, 27 May 2024 12:04:15 +0300 Subject: [PATCH 1/2] feat: improve PD service time window validation to accept 86400 as a valid value --- pagerduty/resource_pagerduty_service.go | 7 +++-- pagerduty/resource_pagerduty_service_test.go | 27 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/pagerduty/resource_pagerduty_service.go b/pagerduty/resource_pagerduty_service.go index 03228046e..01e6510a3 100644 --- a/pagerduty/resource_pagerduty_service.go +++ b/pagerduty/resource_pagerduty_service.go @@ -360,6 +360,9 @@ func customizePagerDutyServiceDiff(context context.Context, diff *schema.Resourc if (timeWindowVal > 300) && (agpType != "" && hasChangeAgpType && (agpType != "intelligent" && agpType != "content_based")) { return fmt.Errorf("Alert grouping parameters configuration attribute \"time_window\" is only supported by \"intelligent\" and \"content-based\" type Alert Grouping") } + if timeWindowVal == 86400 && agpType != "content_based" { + return fmt.Errorf("Alert grouping parameters configuration attribute \"time_window\" with a value of 86400 is only supported by \"content-based\" type Alert Grouping") + } } return nil @@ -369,10 +372,10 @@ func validateTimeWindow(v interface{}, p cty.Path) diag.Diagnostics { var diags diag.Diagnostics tw := v.(int) - if tw < 300 || tw > 3600 { + if (tw < 300 || tw > 3600) && tw != 86400 { diags = append(diags, diag.Diagnostic{ Severity: diag.Error, - Summary: fmt.Sprintf("Alert grouping time window value must be between 300 and 3600, current setting is %d", tw), + Summary: fmt.Sprintf("Alert grouping time window value must be between 300 and 3600 or exactly 86400(86400 is supported only for content-based alert grouping), current setting is %d", tw), AttributePath: p, }) } diff --git a/pagerduty/resource_pagerduty_service_test.go b/pagerduty/resource_pagerduty_service_test.go index 10e6cfcc0..f14796830 100644 --- a/pagerduty/resource_pagerduty_service_test.go +++ b/pagerduty/resource_pagerduty_service_test.go @@ -309,6 +309,33 @@ func TestAccPagerDutyService_FormatValidation(t *testing.T) { { Config: testAccCheckPagerDutyServiceCustomInputValidationConfig(username, email, escalationPolicy, service, ` + alert_grouping_parameters { + type = "content_based" + config { + time_window = 86400 + } + } + `, + ), + PlanOnly: true, + }, + { + Config: testAccCheckPagerDutyServiceCustomInputValidationConfig(username, email, escalationPolicy, service, + ` + alert_grouping_parameters { + type = "intelligent" + config { + time_window = 86400 + } + } + `, + ), + PlanOnly: true, + ExpectError: regexp.MustCompile("Alert grouping parameters configuration attribute \"time_window\" with a value of 86400 is only supported by \"content-based\" type Alert Grouping"), + }, + { + Config: testAccCheckPagerDutyServiceCustomInputValidationConfig(username, email, escalationPolicy, service, + ` alert_grouping_parameters { type = "intelligent" } From 8fb9df889c17a44109059e6b1eb08ea709a85f08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Antonio=20Reyes?= Date: Fri, 14 Jun 2024 12:57:17 -0400 Subject: [PATCH 2/2] add docs update and minimal tweaks --- pagerduty/resource_pagerduty_service.go | 6 +-- pagerduty/resource_pagerduty_service_test.go | 56 ++++++++++---------- website/docs/r/service.html.markdown | 2 +- 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/pagerduty/resource_pagerduty_service.go b/pagerduty/resource_pagerduty_service.go index 01e6510a3..3ae23feaf 100644 --- a/pagerduty/resource_pagerduty_service.go +++ b/pagerduty/resource_pagerduty_service.go @@ -351,6 +351,9 @@ func customizePagerDutyServiceDiff(context context.Context, diff *schema.Resourc if agpType == "content_based" && (aggregateVal == "" || len(fieldsVal) == 0) { return fmt.Errorf("When using Alert grouping parameters configuration of type \"content_based\" is in use, attributes \"aggregate\" and \"fields\" are required") } + if timeWindowVal == 86400 && agpType != "content_based" { + return fmt.Errorf("Alert grouping parameters configuration attribute \"time_window\" with a value of 86400 is only supported by \"content-based\" type Alert Grouping") + } if (aggregateVal != "" || len(fieldsVal) > 0) && (agpType != "" && hasChangeAgpType && agpType != "content_based") { return fmt.Errorf("Alert grouping parameters configuration attributes \"aggregate\" and \"fields\" are only supported by \"content_based\" type Alert Grouping") } @@ -360,9 +363,6 @@ func customizePagerDutyServiceDiff(context context.Context, diff *schema.Resourc if (timeWindowVal > 300) && (agpType != "" && hasChangeAgpType && (agpType != "intelligent" && agpType != "content_based")) { return fmt.Errorf("Alert grouping parameters configuration attribute \"time_window\" is only supported by \"intelligent\" and \"content-based\" type Alert Grouping") } - if timeWindowVal == 86400 && agpType != "content_based" { - return fmt.Errorf("Alert grouping parameters configuration attribute \"time_window\" with a value of 86400 is only supported by \"content-based\" type Alert Grouping") - } } return nil diff --git a/pagerduty/resource_pagerduty_service_test.go b/pagerduty/resource_pagerduty_service_test.go index f14796830..024fce967 100644 --- a/pagerduty/resource_pagerduty_service_test.go +++ b/pagerduty/resource_pagerduty_service_test.go @@ -176,6 +176,34 @@ func TestAccPagerDutyService_FormatValidation(t *testing.T) { { Config: testAccCheckPagerDutyServiceCustomInputValidationConfig(username, email, escalationPolicy, service, ` + alert_grouping_parameters { + type = "intelligent" + config { + time_window = 86400 + } + } + `, + ), + PlanOnly: true, + ExpectError: regexp.MustCompile("Alert grouping parameters configuration attribute \"time_window\" with a value of 86400 is only supported by \"content-based\" type Alert Grouping"), + }, + { + Config: testAccCheckPagerDutyServiceCustomInputValidationConfig(username, email, escalationPolicy, service, + ` + alert_grouping_parameters { + type = "content_based" + config { + time_window = 86400 + aggregate = "all" + fields = ["custom_details.source_id"] + } + } + `, + ), + }, + { + Config: testAccCheckPagerDutyServiceCustomInputValidationConfig(username, email, escalationPolicy, service, + ` alert_grouping_parameters { type = "content_based" config {} @@ -191,6 +219,7 @@ func TestAccPagerDutyService_FormatValidation(t *testing.T) { alert_grouping_parameters { type = "content_based" config { + time_window = 300 aggregate = "all" fields = ["custom_details.source_id"] } @@ -309,33 +338,6 @@ func TestAccPagerDutyService_FormatValidation(t *testing.T) { { Config: testAccCheckPagerDutyServiceCustomInputValidationConfig(username, email, escalationPolicy, service, ` - alert_grouping_parameters { - type = "content_based" - config { - time_window = 86400 - } - } - `, - ), - PlanOnly: true, - }, - { - Config: testAccCheckPagerDutyServiceCustomInputValidationConfig(username, email, escalationPolicy, service, - ` - alert_grouping_parameters { - type = "intelligent" - config { - time_window = 86400 - } - } - `, - ), - PlanOnly: true, - ExpectError: regexp.MustCompile("Alert grouping parameters configuration attribute \"time_window\" with a value of 86400 is only supported by \"content-based\" type Alert Grouping"), - }, - { - Config: testAccCheckPagerDutyServiceCustomInputValidationConfig(username, email, escalationPolicy, service, - ` alert_grouping_parameters { type = "intelligent" } diff --git a/website/docs/r/service.html.markdown b/website/docs/r/service.html.markdown index 3f6faac91..dd96ea600 100644 --- a/website/docs/r/service.html.markdown +++ b/website/docs/r/service.html.markdown @@ -71,7 +71,7 @@ The `alert_grouping_parameters` block contains the following arguments: * `timeout` - (Optional) The duration in minutes within which to automatically group incoming alerts. This setting applies only when `type` is set to `time`. To continue grouping alerts until the incident is resolved, set this value to `0`. * `aggregate` - (Optional) One of `any` or `all`. This setting applies only when `type` is set to `content_based`. Group alerts based on one or all of `fields` value(s). * `fields` - (Optional) Alerts will be grouped together if the content of these fields match. This setting applies only when `type` is set to `content_based`. - * `time_window` - (Optional) The maximum amount of time allowed between Alerts. This setting applies only when `type` is set to `intelligent` or `content_based`. Value must be between `300` and `3600`. Any Alerts arriving greater than `time_window` seconds apart will not be grouped together. This is a rolling time window and is counted from the most recently grouped alert. The window is extended every time a new alert is added to the group, up to 24 hours. + * `time_window` - (Optional) The maximum amount of time allowed between Alerts. This setting applies only when `type` is set to `intelligent` or `content_based`. Value must be between `300` and `3600` or exactly `86400` (86400 is supported only for `content_based` alert grouping). Any Alerts arriving greater than `time_window` seconds apart will not be grouped together. This is a rolling time window and is counted from the most recently grouped alert. The window is extended every time a new alert is added to the group, up to 24 hours. The `auto_pause_notifications_parameters` block contains the following arguments: