From 96f1570138811ee514a9ac6012eeda9e4132e0d7 Mon Sep 17 00:00:00 2001 From: Espen Albert Date: Thu, 13 Nov 2025 09:42:24 +0000 Subject: [PATCH 1/2] fix: remove interval_min validation blocking notification type updates Remove validation that prevented updating alert notifications from types supporting interval_min to PAGER_DUTY/OPS_GENIE/VICTOR_OPS. Since interval_min is optional and not preserved via UseStateForUnknown, it will only be sent to the API if explicitly set in config, allowing updates to succeed. Fixes #3869 --- internal/service/alertconfiguration/model.go | 10 --- .../alertconfiguration/resource_test.go | 72 +++++++++++++++++++ 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/internal/service/alertconfiguration/model.go b/internal/service/alertconfiguration/model.go index d4e708568b..f3f92f2752 100644 --- a/internal/service/alertconfiguration/model.go +++ b/internal/service/alertconfiguration/model.go @@ -2,7 +2,6 @@ package alertconfiguration import ( "fmt" - "strings" "go.mongodb.org/atlas-sdk/v20250312009/admin" @@ -14,15 +13,6 @@ import ( func NewNotificationList(list []TfNotificationModel) (*[]admin.AlertsNotificationRootForGroup, error) { notifications := make([]admin.AlertsNotificationRootForGroup, len(list)) - for i := range list { - if !list[i].IntervalMin.IsNull() && list[i].IntervalMin.ValueInt64() > 0 { - typeName := list[i].TypeName.ValueString() - if strings.EqualFold(typeName, pagerDuty) || strings.EqualFold(typeName, opsGenie) || strings.EqualFold(typeName, victorOps) { - return nil, fmt.Errorf(`'interval_min' must not be set if type_name is 'PAGER_DUTY', 'OPS_GENIE' or 'VICTOR_OPS'`) - } - } - } - for i := range list { n := &list[i] notifications[i] = admin.AlertsNotificationRootForGroup{ diff --git a/internal/service/alertconfiguration/resource_test.go b/internal/service/alertconfiguration/resource_test.go index bebeab3093..874eb02c7b 100644 --- a/internal/service/alertconfiguration/resource_test.go +++ b/internal/service/alertconfiguration/resource_test.go @@ -570,6 +570,45 @@ func TestAccConfigRSAlertConfiguration_withVictorOps(t *testing.T) { }) } +func TestAccConfigRSAlertConfiguration_updateNotificationTypeFromTeamsToPagerDuty(t *testing.T) { + // This test reproduces issue #3869: updating from MICROSOFT_TEAMS with interval_min + // to PAGER_DUTY should work without requiring deletion and recreation + var ( + projectID = acc.ProjectIDExecution(t) + teamsWebhookURL = "https://outlook.office.com/webhook/11111111-1111-1111-1111-111111111111@22222222-2222-2222-2222-222222222222/IncomingWebhook/33333333333333333333333333333333/44444444-4444-4444-4444-444444444444" + pagerDutyKey = dummy32CharKey + ) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acc.PreCheckBasic(t) }, + ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, + CheckDestroy: checkDestroy(), + Steps: []resource.TestStep{ + { + Config: configWithTeamsNotificationAndIntervalMin(projectID, teamsWebhookURL, true), + Check: resource.ComposeAggregateTestCheckFunc( + checkExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "project_id", projectID), + resource.TestCheckResourceAttr(resourceName, "notification.#", "1"), + resource.TestCheckResourceAttr(resourceName, "notification.0.type_name", "MICROSOFT_TEAMS"), + resource.TestCheckResourceAttr(resourceName, "notification.0.interval_min", "30"), + ), + }, + { + Config: configWithPagerDutyNotification(projectID, pagerDutyKey, true), + Check: resource.ComposeAggregateTestCheckFunc( + checkExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "project_id", projectID), + resource.TestCheckResourceAttr(resourceName, "notification.#", "1"), + resource.TestCheckResourceAttr(resourceName, "notification.0.type_name", "PAGER_DUTY"), + // interval_min should not be set for PAGER_DUTY + resource.TestCheckNoResourceAttr(resourceName, "notification.0.interval_min"), + ), + }, + }, + }) +} + func TestAccConfigRSAlertConfiguration_withSeverityOverride(t *testing.T) { var ( projectID = acc.ProjectIDExecution(t) @@ -992,6 +1031,39 @@ func configWithVictorOps(projectID, apiKey string, enabled bool) string { `, projectID, apiKey, enabled) } +func configWithTeamsNotificationAndIntervalMin(projectID, webhookURL string, enabled bool) string { + return fmt.Sprintf(` + resource "mongodbatlas_alert_configuration" "test" { + project_id = %[1]q + enabled = %[3]t + event_type = "NO_PRIMARY" + + notification { + type_name = "MICROSOFT_TEAMS" + microsoft_teams_webhook_url = %[2]q + interval_min = 30 + delay_min = 0 + } + } + `, projectID, webhookURL, enabled) +} + +func configWithPagerDutyNotification(projectID, serviceKey string, enabled bool) string { + return fmt.Sprintf(` + resource "mongodbatlas_alert_configuration" "test" { + project_id = %[1]q + enabled = %[3]t + event_type = "NO_PRIMARY" + + notification { + type_name = "PAGER_DUTY" + service_key = %[2]q + delay_min = 0 + } + } + `, projectID, serviceKey, enabled) +} + func configWithEmptyMetricThresholdConfig(projectID string, enabled bool) string { return fmt.Sprintf(` resource "mongodbatlas_alert_configuration" "test" { From 625c4a3bcdab14c6920d1d66921c1eb23b740857 Mon Sep 17 00:00:00 2001 From: Espen Albert Date: Thu, 13 Nov 2025 10:22:44 +0000 Subject: [PATCH 2/2] fix: Add validation for interval_min in alert configuration notifications Implement a new validator to ensure that the `interval_min` attribute is not set for notification types that do not support it (PAGER_DUTY, OPS_GENIE, VICTOR_OPS). This change prevents configuration errors during the plan phase and enhances user feedback. --- .changelog/3882.txt | 3 + .../service/alertconfiguration/resource.go | 3 + .../alertconfiguration/resource_test.go | 22 +++++ .../validator_interval_min.go | 86 +++++++++++++++++++ 4 files changed, 114 insertions(+) create mode 100644 .changelog/3882.txt create mode 100644 internal/service/alertconfiguration/validator_interval_min.go diff --git a/.changelog/3882.txt b/.changelog/3882.txt new file mode 100644 index 0000000000..0e8ed6e6ad --- /dev/null +++ b/.changelog/3882.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/mongodbatlas_alert_configuration: Fixes issue preventing updates from notification types supporting `interval_min` (e.g., MICROSOFT_TEAMS) to types that don't support it (PAGER_DUTY, OPS_GENIE, VICTOR_OPS) +``` diff --git a/internal/service/alertconfiguration/resource.go b/internal/service/alertconfiguration/resource.go index b04833fef8..5090f9f922 100644 --- a/internal/service/alertconfiguration/resource.go +++ b/internal/service/alertconfiguration/resource.go @@ -290,6 +290,9 @@ func (r *alertConfigurationRS) Schema(ctx context.Context, req resource.SchemaRe "interval_min": schema.Int64Attribute{ Optional: true, Computed: true, + Validators: []validator.Int64{ + ValidIntervalMin(), + }, }, "mobile_number": schema.StringAttribute{ Optional: true, diff --git a/internal/service/alertconfiguration/resource_test.go b/internal/service/alertconfiguration/resource_test.go index 874eb02c7b..1cfb3da297 100644 --- a/internal/service/alertconfiguration/resource_test.go +++ b/internal/service/alertconfiguration/resource_test.go @@ -584,6 +584,11 @@ func TestAccConfigRSAlertConfiguration_updateNotificationTypeFromTeamsToPagerDut ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, CheckDestroy: checkDestroy(), Steps: []resource.TestStep{ + { + // Verify that explicitly setting interval_min with PAGER_DUTY fails at schema validation level + Config: configWithPagerDutyAndIntervalMin(projectID, pagerDutyKey, true), + ExpectError: regexp.MustCompile(`(?s).*'interval_min'.*must not be set.*PAGER_DUTY`), + }, { Config: configWithTeamsNotificationAndIntervalMin(projectID, teamsWebhookURL, true), Check: resource.ComposeAggregateTestCheckFunc( @@ -1064,6 +1069,23 @@ func configWithPagerDutyNotification(projectID, serviceKey string, enabled bool) `, projectID, serviceKey, enabled) } +func configWithPagerDutyAndIntervalMin(projectID, serviceKey string, enabled bool) string { + return fmt.Sprintf(` + resource "mongodbatlas_alert_configuration" "test" { + project_id = %[1]q + enabled = %[3]t + event_type = "NO_PRIMARY" + + notification { + type_name = "PAGER_DUTY" + service_key = %[2]q + interval_min = 30 + delay_min = 0 + } + } + `, projectID, serviceKey, enabled) +} + func configWithEmptyMetricThresholdConfig(projectID string, enabled bool) string { return fmt.Sprintf(` resource "mongodbatlas_alert_configuration" "test" { diff --git a/internal/service/alertconfiguration/validator_interval_min.go b/internal/service/alertconfiguration/validator_interval_min.go new file mode 100644 index 0000000000..58ab3d0000 --- /dev/null +++ b/internal/service/alertconfiguration/validator_interval_min.go @@ -0,0 +1,86 @@ +package alertconfiguration + +import ( + "context" + "fmt" + "strconv" + "strings" + + "github.com/hashicorp/terraform-plugin-framework-validators/helpers/validatordiag" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" +) + +// IntervalMinValidator validates that interval_min is not set for notification types +// that don't support it (PAGER_DUTY, OPS_GENIE, VICTOR_OPS). +// +// API-level validation cannot catch this error because when the notification type +// doesn't support interval_min, the nested_type in the API payload omits the field, +// so it's never sent to the API. Client-side validation ensures users get immediate +// feedback during Terraform's plan phase. +type IntervalMinValidator struct{} + +func (v IntervalMinValidator) Description(_ context.Context) string { + return "'interval_min' must not be set if type_name is 'PAGER_DUTY', 'OPS_GENIE' or 'VICTOR_OPS'" +} + +func (v IntervalMinValidator) MarkdownDescription(ctx context.Context) string { + return v.Description(ctx) +} + +func (v IntervalMinValidator) ValidateInt64(ctx context.Context, req validator.Int64Request, resp *validator.Int64Response) { + // If the value is unknown/null/0, there is nothing to validate. + if req.ConfigValue.ValueInt64() <= 0 { + return + } + + // Parse the path to find which notification index we're validating + // Path format: notification[0].interval_min + pathStr := req.Path.String() + notificationIndex := -1 + + // Extract the index from the path (e.g., "notification[0]" -> 0) + if idxStart := strings.Index(pathStr, "["); idxStart != -1 { + if idxEnd := strings.Index(pathStr[idxStart:], "]"); idxEnd != -1 { + idxStr := pathStr[idxStart+1 : idxStart+idxEnd] + if idx, err := strconv.Atoi(idxStr); err == nil { + notificationIndex = idx + } + } + } + + // If we couldn't parse the index, skip validation + if notificationIndex < 0 { + return + } + + // Get the entire notification list from config + var notifications []TfNotificationModel + diags := req.Config.GetAttribute(ctx, path.Root("notification"), ¬ifications) + if diags.HasError() { + // If we can't read notifications, skip validation (might be unknown during plan) + return + } + + // Check if we have the notification at the parsed index + if notificationIndex >= len(notifications) { + return + } + + notification := notifications[notificationIndex] + typeNameValue := notification.TypeName.ValueString() + // Check if the type_name is one of the unsupported types + if strings.EqualFold(typeNameValue, pagerDuty) || + strings.EqualFold(typeNameValue, opsGenie) || + strings.EqualFold(typeNameValue, victorOps) { + resp.Diagnostics.Append(validatordiag.InvalidAttributeValueDiagnostic( + req.Path, + v.Description(ctx), + fmt.Sprintf("%d", req.ConfigValue.ValueInt64()), + )) + } +} + +func ValidIntervalMin() validator.Int64 { + return IntervalMinValidator{} +}