From 99e9212579ee8fca9d40ff2d4344eb57ef8ed82f Mon Sep 17 00:00:00 2001 From: Oriol Date: Wed, 7 Aug 2024 09:30:13 +0200 Subject: [PATCH] fix: Handles update of `mongodbatlas_backup_compliance_policy` as a create operation (#2480) * handle update as a create * add test to make sure no plan changes appear when reapplying config with non default values * add changelog * fix projectId * fix name of resource in test * Update .changelog/2480.txt Co-authored-by: kyuan-mongodb <78768401+kyuan-mongodb@users.noreply.github.com> --------- Co-authored-by: kyuan-mongodb <78768401+kyuan-mongodb@users.noreply.github.com> --- .changelog/2480.txt | 3 + .../resource_backup_compliance_policy.go | 253 ++++++------------ .../resource_backup_compliance_policy_test.go | 81 ++++++ 3 files changed, 169 insertions(+), 168 deletions(-) create mode 100644 .changelog/2480.txt diff --git a/.changelog/2480.txt b/.changelog/2480.txt new file mode 100644 index 0000000000..9474013e4e --- /dev/null +++ b/.changelog/2480.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/mongodbatlas_backup_compliance_policy: Fixes an issue where the update operation modified attributes that were not supposed to be modified" +``` diff --git a/internal/service/backupcompliancepolicy/resource_backup_compliance_policy.go b/internal/service/backupcompliancepolicy/resource_backup_compliance_policy.go index b542c87056..8e5a4d2986 100644 --- a/internal/service/backupcompliancepolicy/resource_backup_compliance_policy.go +++ b/internal/service/backupcompliancepolicy/resource_backup_compliance_policy.go @@ -263,85 +263,8 @@ func resourceCreate(ctx context.Context, d *schema.ResourceData, meta any) diag. connV2 := meta.(*config.MongoDBClient).AtlasV2 projectID := d.Get("project_id").(string) - dataProtectionSettings := &admin.DataProtectionSettings20231001{ - ProjectId: conversion.StringPtr(projectID), - AuthorizedEmail: d.Get("authorized_email").(string), - AuthorizedUserFirstName: d.Get("authorized_user_first_name").(string), - AuthorizedUserLastName: d.Get("authorized_user_last_name").(string), - CopyProtectionEnabled: conversion.Pointer(d.Get("copy_protection_enabled").(bool)), - EncryptionAtRestEnabled: conversion.Pointer(d.Get("encryption_at_rest_enabled").(bool)), - PitEnabled: conversion.Pointer(d.Get("pit_enabled").(bool)), - RestoreWindowDays: conversion.Pointer(cast.ToInt(d.Get("restore_window_days"))), - OnDemandPolicyItem: expandDemandBackupPolicyItem(d), - } + err := updateOrCreateDataProtectionSetting(ctx, d, connV2, projectID) - var backupPoliciesItem []admin.BackupComplianceScheduledPolicyItem - if v, ok := d.GetOk("policy_item_hourly"); ok { - item := v.([]any) - itemObj := item[0].(map[string]any) - backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ - FrequencyType: cloudbackupschedule.Hourly, - RetentionUnit: itemObj["retention_unit"].(string), - FrequencyInterval: itemObj["frequency_interval"].(int), - RetentionValue: itemObj["retention_value"].(int), - }) - } - if v, ok := d.GetOk("policy_item_daily"); ok { - item := v.([]any) - itemObj := item[0].(map[string]any) - backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ - FrequencyType: cloudbackupschedule.Daily, - RetentionUnit: itemObj["retention_unit"].(string), - FrequencyInterval: itemObj["frequency_interval"].(int), - RetentionValue: itemObj["retention_value"].(int), - }) - } - if v, ok := d.GetOk("policy_item_weekly"); ok { - items := v.([]any) - for _, s := range items { - itemObj := s.(map[string]any) - backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ - FrequencyType: cloudbackupschedule.Weekly, - RetentionUnit: itemObj["retention_unit"].(string), - FrequencyInterval: itemObj["frequency_interval"].(int), - RetentionValue: itemObj["retention_value"].(int), - }) - } - } - if v, ok := d.GetOk("policy_item_monthly"); ok { - items := v.([]any) - for _, s := range items { - itemObj := s.(map[string]any) - backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ - FrequencyType: cloudbackupschedule.Monthly, - RetentionUnit: itemObj["retention_unit"].(string), - FrequencyInterval: itemObj["frequency_interval"].(int), - RetentionValue: itemObj["retention_value"].(int), - }) - } - } - if v, ok := d.GetOk("policy_item_yearly"); ok { - items := v.([]any) - for _, s := range items { - itemObj := s.(map[string]any) - backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ - FrequencyType: cloudbackupschedule.Yearly, - RetentionUnit: itemObj["retention_unit"].(string), - FrequencyInterval: itemObj["frequency_interval"].(int), - RetentionValue: itemObj["retention_value"].(int), - }) - } - } - if len(backupPoliciesItem) > 0 { - dataProtectionSettings.ScheduledPolicyItems = &backupPoliciesItem - } - - params := admin.UpdateDataProtectionSettingsApiParams{ - GroupId: projectID, - DataProtectionSettings20231001: dataProtectionSettings, - OverwriteBackupPolicies: conversion.Pointer(false), - } - _, _, err := connV2.CloudBackupsApi.UpdateDataProtectionSettingsWithParams(ctx, ¶ms).Execute() if err != nil { return diag.FromErr(fmt.Errorf(errorBackupPolicyUpdate, projectID, err)) } @@ -444,97 +367,8 @@ func resourceUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag. ids := conversion.DecodeStateID(d.Id()) projectID := ids["project_id"] - dataProtectionSettings := &admin.DataProtectionSettings20231001{ - ProjectId: conversion.StringPtr(projectID), - AuthorizedEmail: d.Get("authorized_email").(string), - AuthorizedUserFirstName: d.Get("authorized_user_first_name").(string), - AuthorizedUserLastName: d.Get("authorized_user_last_name").(string), - OnDemandPolicyItem: expandDemandBackupPolicyItem(d), - } - - if d.HasChange("copy_protection_enabled") { - dataProtectionSettings.CopyProtectionEnabled = conversion.Pointer(d.Get("copy_protection_enabled").(bool)) - } - - if d.HasChange("encryption_at_rest_enabled") { - dataProtectionSettings.EncryptionAtRestEnabled = conversion.Pointer(d.Get("encryption_at_rest_enabled").(bool)) - } - - if d.HasChange("pit_enabled") { - dataProtectionSettings.PitEnabled = conversion.Pointer(d.Get("pit_enabled").(bool)) - } - - if d.HasChange("restore_window_days") { - dataProtectionSettings.RestoreWindowDays = conversion.Pointer(cast.ToInt(d.Get("restore_window_days"))) - } + err := updateOrCreateDataProtectionSetting(ctx, d, connV2, projectID) - var backupPoliciesItem []admin.BackupComplianceScheduledPolicyItem - if v, ok := d.GetOk("policy_item_hourly"); ok { - item := v.([]any) - itemObj := item[0].(map[string]any) - backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ - FrequencyType: cloudbackupschedule.Hourly, - RetentionUnit: itemObj["retention_unit"].(string), - FrequencyInterval: itemObj["frequency_interval"].(int), - RetentionValue: itemObj["retention_value"].(int), - }) - } - if v, ok := d.GetOk("policy_item_daily"); ok { - item := v.([]any) - itemObj := item[0].(map[string]any) - backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ - FrequencyType: cloudbackupschedule.Daily, - RetentionUnit: itemObj["retention_unit"].(string), - FrequencyInterval: itemObj["frequency_interval"].(int), - RetentionValue: itemObj["retention_value"].(int), - }) - } - if v, ok := d.GetOk("policy_item_weekly"); ok { - items := v.([]any) - for _, s := range items { - itemObj := s.(map[string]any) - backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ - FrequencyType: cloudbackupschedule.Weekly, - RetentionUnit: itemObj["retention_unit"].(string), - FrequencyInterval: itemObj["frequency_interval"].(int), - RetentionValue: itemObj["retention_value"].(int), - }) - } - } - if v, ok := d.GetOk("policy_item_monthly"); ok { - items := v.([]any) - for _, s := range items { - itemObj := s.(map[string]any) - backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ - FrequencyType: cloudbackupschedule.Monthly, - RetentionUnit: itemObj["retention_unit"].(string), - FrequencyInterval: itemObj["frequency_interval"].(int), - RetentionValue: itemObj["retention_value"].(int), - }) - } - } - if v, ok := d.GetOk("policy_item_yearly"); ok { - items := v.([]any) - for _, s := range items { - itemObj := s.(map[string]any) - backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ - FrequencyType: cloudbackupschedule.Yearly, - RetentionUnit: itemObj["retention_unit"].(string), - FrequencyInterval: itemObj["frequency_interval"].(int), - RetentionValue: itemObj["retention_value"].(int), - }) - } - } - if len(backupPoliciesItem) > 0 { - dataProtectionSettings.ScheduledPolicyItems = &backupPoliciesItem - } - - params := admin.UpdateDataProtectionSettingsApiParams{ - GroupId: projectID, - DataProtectionSettings20231001: dataProtectionSettings, - OverwriteBackupPolicies: conversion.Pointer(false), - } - _, _, err := connV2.CloudBackupsApi.UpdateDataProtectionSettingsWithParams(ctx, ¶ms).Execute() if err != nil { return diag.FromErr(fmt.Errorf(errorBackupPolicyUpdate, projectID, err)) } @@ -622,3 +456,86 @@ func flattenBackupPolicyItems(items []admin.BackupComplianceScheduledPolicyItem, } return policyItems } + +func updateOrCreateDataProtectionSetting(ctx context.Context, d *schema.ResourceData, connV2 *admin.APIClient, projectID string) error { + dataProtectionSettings := &admin.DataProtectionSettings20231001{ + ProjectId: conversion.StringPtr(projectID), + AuthorizedEmail: d.Get("authorized_email").(string), + AuthorizedUserFirstName: d.Get("authorized_user_first_name").(string), + AuthorizedUserLastName: d.Get("authorized_user_last_name").(string), + CopyProtectionEnabled: conversion.Pointer(d.Get("copy_protection_enabled").(bool)), + EncryptionAtRestEnabled: conversion.Pointer(d.Get("encryption_at_rest_enabled").(bool)), + PitEnabled: conversion.Pointer(d.Get("pit_enabled").(bool)), + RestoreWindowDays: conversion.Pointer(cast.ToInt(d.Get("restore_window_days"))), + OnDemandPolicyItem: expandDemandBackupPolicyItem(d), + } + + var backupPoliciesItem []admin.BackupComplianceScheduledPolicyItem + if v, ok := d.GetOk("policy_item_hourly"); ok { + item := v.([]any) + itemObj := item[0].(map[string]any) + backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ + FrequencyType: cloudbackupschedule.Hourly, + RetentionUnit: itemObj["retention_unit"].(string), + FrequencyInterval: itemObj["frequency_interval"].(int), + RetentionValue: itemObj["retention_value"].(int), + }) + } + if v, ok := d.GetOk("policy_item_daily"); ok { + item := v.([]any) + itemObj := item[0].(map[string]any) + backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ + FrequencyType: cloudbackupschedule.Daily, + RetentionUnit: itemObj["retention_unit"].(string), + FrequencyInterval: itemObj["frequency_interval"].(int), + RetentionValue: itemObj["retention_value"].(int), + }) + } + if v, ok := d.GetOk("policy_item_weekly"); ok { + items := v.([]any) + for _, s := range items { + itemObj := s.(map[string]any) + backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ + FrequencyType: cloudbackupschedule.Weekly, + RetentionUnit: itemObj["retention_unit"].(string), + FrequencyInterval: itemObj["frequency_interval"].(int), + RetentionValue: itemObj["retention_value"].(int), + }) + } + } + if v, ok := d.GetOk("policy_item_monthly"); ok { + items := v.([]any) + for _, s := range items { + itemObj := s.(map[string]any) + backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ + FrequencyType: cloudbackupschedule.Monthly, + RetentionUnit: itemObj["retention_unit"].(string), + FrequencyInterval: itemObj["frequency_interval"].(int), + RetentionValue: itemObj["retention_value"].(int), + }) + } + } + if v, ok := d.GetOk("policy_item_yearly"); ok { + items := v.([]any) + for _, s := range items { + itemObj := s.(map[string]any) + backupPoliciesItem = append(backupPoliciesItem, admin.BackupComplianceScheduledPolicyItem{ + FrequencyType: cloudbackupschedule.Yearly, + RetentionUnit: itemObj["retention_unit"].(string), + FrequencyInterval: itemObj["frequency_interval"].(int), + RetentionValue: itemObj["retention_value"].(int), + }) + } + } + if len(backupPoliciesItem) > 0 { + dataProtectionSettings.ScheduledPolicyItems = &backupPoliciesItem + } + + params := admin.UpdateDataProtectionSettingsApiParams{ + GroupId: projectID, + DataProtectionSettings20231001: dataProtectionSettings, + OverwriteBackupPolicies: conversion.Pointer(false), + } + _, _, err := connV2.CloudBackupsApi.UpdateDataProtectionSettingsWithParams(ctx, ¶ms).Execute() + return err +} diff --git a/internal/service/backupcompliancepolicy/resource_backup_compliance_policy_test.go b/internal/service/backupcompliancepolicy/resource_backup_compliance_policy_test.go index 310793e882..bc99b374e0 100644 --- a/internal/service/backupcompliancepolicy/resource_backup_compliance_policy_test.go +++ b/internal/service/backupcompliancepolicy/resource_backup_compliance_policy_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion" "github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc" @@ -116,6 +117,41 @@ func TestAccBackupCompliancePolicy_withoutRestoreWindowDays(t *testing.T) { }) } +func TestAccBackupCompliancePolicy_UpdateSetsAllAttributes(t *testing.T) { + var ( + orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") + projectName = acc.RandomProjectName() // No ProjectIDExecution to avoid conflicts with backup compliance policy + projectOwnerID = os.Getenv("MONGODB_ATLAS_PROJECT_OWNER_ID") + ) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acc.PreCheckBasic(t) }, + ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, + Steps: []resource.TestStep{ + { + Config: configBasicWithOptionalAttributesWithNonDefaultValues(projectName, orgID, projectOwnerID), + Check: resource.ComposeAggregateTestCheckFunc( + checkExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "authorized_user_first_name", "First"), + resource.TestCheckResourceAttr(resourceName, "authorized_user_last_name", "Last"), + resource.TestCheckResourceAttr(resourceName, "pit_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "encryption_at_rest_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "copy_protection_enabled", "true"), + ), + }, + { + Config: configBasicWithOptionalAttributesWithNonDefaultValues(projectName, orgID, projectOwnerID), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + acc.DebugPlan(), + plancheck.ExpectEmptyPlan(), + }, + }, + }, + }, + }) +} + func basicTestCase(tb testing.TB, useYearly bool) *resource.TestCase { tb.Helper() @@ -419,3 +455,48 @@ func basicChecks() []resource.TestCheckFunc { checks = append(checks, checkExists(resourceName), checkExists(dataSourceName)) return checks } + +func configBasicWithOptionalAttributesWithNonDefaultValues(projectName, orgID, projectOwnerID string) string { + return acc.ConfigProjectWithSettings(projectName, orgID, projectOwnerID, false) + + `resource "mongodbatlas_backup_compliance_policy" "backup_policy_res" { + project_id = mongodbatlas_project.test.id + authorized_email = "test@example.com" + authorized_user_first_name = "First" + authorized_user_last_name = "Last" + copy_protection_enabled = true + pit_enabled = false + encryption_at_rest_enabled = false + + restore_window_days = 7 + + on_demand_policy_item { + frequency_interval = 0 + retention_unit = "days" + retention_value = 3 + } + + policy_item_hourly { + frequency_interval = 6 + retention_unit = "days" + retention_value = 7 + } + + policy_item_daily { + frequency_interval = 0 + retention_unit = "days" + retention_value = 7 + } + + policy_item_weekly { + frequency_interval = 0 + retention_unit = "weeks" + retention_value = 4 + } + + policy_item_monthly { + frequency_interval = 0 + retention_unit = "months" + retention_value = 12 + } + }` +}