From 29ed0cf038a951b0e90e3c1f415bfe8774f96532 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 9 Dec 2021 08:36:03 -0500 Subject: [PATCH 1/4] backup/vault_policy: Fix diffs with policy --- internal/service/backup/vault_policy.go | 30 ++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/internal/service/backup/vault_policy.go b/internal/service/backup/vault_policy.go index a9cd201f226..316d6227634 100644 --- a/internal/service/backup/vault_policy.go +++ b/internal/service/backup/vault_policy.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/service/backup" "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" @@ -39,6 +40,10 @@ func ResourceVaultPolicy() *schema.Resource { Required: true, ValidateFunc: validation.StringIsJSON, DiffSuppressFunc: verify.SuppressEquivalentPolicyDiffs, + StateFunc: func(v interface{}) string { + json, _ := structure.NormalizeJsonString(v) + return json + }, }, }, } @@ -47,13 +52,19 @@ func ResourceVaultPolicy() *schema.Resource { func resourceVaultPolicyPut(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).BackupConn + policy, err := structure.NormalizeJsonString(d.Get("policy").(string)) + + if err != nil { + return fmt.Errorf("policy (%s) is invalid JSON: %w", policy, err) + } + name := d.Get("backup_vault_name").(string) input := &backup.PutBackupVaultAccessPolicyInput{ BackupVaultName: aws.String(name), - Policy: aws.String(d.Get("policy").(string)), + Policy: aws.String(policy), } - _, err := conn.PutBackupVaultAccessPolicy(input) + _, err = conn.PutBackupVaultAccessPolicy(input) if err != nil { return fmt.Errorf("error creating Backup Vault Policy (%s): %w", name, err) @@ -81,7 +92,20 @@ func resourceVaultPolicyRead(d *schema.ResourceData, meta interface{}) error { d.Set("backup_vault_arn", output.BackupVaultArn) d.Set("backup_vault_name", output.BackupVaultName) - d.Set("policy", output.Policy) + + policyToSet, err := verify.SecondJSONUnlessEquivalent(d.Get("policy").(string), aws.StringValue(output.Policy)) + + if err != nil { + return fmt.Errorf("while setting policy (%s), encountered: %w", policyToSet, err) + } + + policyToSet, err = structure.NormalizeJsonString(policyToSet) + + if err != nil { + return fmt.Errorf("policy (%s) is invalid JSON: %w", policyToSet, err) + } + + d.Set("policy", policyToSet) return nil } From a7d196dc83c1ea214f393760a5961d0a4928dfcb Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 9 Dec 2021 08:36:16 -0500 Subject: [PATCH 2/4] tests: Fix diffs with policy --- internal/service/backup/vault_policy_test.go | 156 ++++++++++++------- 1 file changed, 104 insertions(+), 52 deletions(-) diff --git a/internal/service/backup/vault_policy_test.go b/internal/service/backup/vault_policy_test.go index 7d845ac3e54..f7cd3440321 100644 --- a/internal/service/backup/vault_policy_test.go +++ b/internal/service/backup/vault_policy_test.go @@ -96,6 +96,31 @@ func TestAccBackupVaultPolicy_Disappears_vault(t *testing.T) { }) } +func TestAccBackupVaultPolicy_ignoreEquivalent(t *testing.T) { + var vault backup.GetBackupVaultAccessPolicyOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_backup_vault_policy.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, backup.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckVaultPolicyDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBackupVaultPolicyConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckVaultPolicyExists(resourceName, &vault), + resource.TestMatchResourceAttr(resourceName, "policy", regexp.MustCompile("\"Version\":\"2012-10-17\""))), + }, + { + Config: testAccBackupVaultPolicyNewOrderConfig(rName), + PlanOnly: true, + }, + }, + }) +} + func testAccCheckVaultPolicyDestroy(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).BackupConn @@ -154,32 +179,28 @@ resource "aws_backup_vault" "test" { resource "aws_backup_vault_policy" "test" { backup_vault_name = aws_backup_vault.test.name - policy = < Date: Thu, 9 Dec 2021 08:38:37 -0500 Subject: [PATCH 3/4] Add changelog --- .changelog/22130.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/22130.txt diff --git a/.changelog/22130.txt b/.changelog/22130.txt new file mode 100644 index 00000000000..e8bbd2f66ec --- /dev/null +++ b/.changelog/22130.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_backup_vault_policy: Fix erroneous diffs in `policy` when no changes made or policies are equivalent +``` \ No newline at end of file From bd0169fbbf2e94f9738e235ef2b6deab4d96b976 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 9 Dec 2021 08:48:56 -0500 Subject: [PATCH 4/4] Lint Shyrock --- internal/service/backup/vault_policy_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/backup/vault_policy_test.go b/internal/service/backup/vault_policy_test.go index f7cd3440321..d3098eddd6a 100644 --- a/internal/service/backup/vault_policy_test.go +++ b/internal/service/backup/vault_policy_test.go @@ -30,7 +30,7 @@ func TestAccBackupVaultPolicy_basic(t *testing.T) { Config: testAccBackupVaultPolicyConfig(rName), Check: resource.ComposeTestCheckFunc( testAccCheckVaultPolicyExists(resourceName, &vault), - resource.TestMatchResourceAttr(resourceName, "policy", regexp.MustCompile("^{\"Version\":\"2012-10-17\".+"))), + resource.TestMatchResourceAttr(resourceName, "policy", regexp.MustCompile("^{\"Id\":\"default\".+"))), }, { ResourceName: resourceName, @@ -41,7 +41,7 @@ func TestAccBackupVaultPolicy_basic(t *testing.T) { Config: testAccBackupVaultPolicyConfigUpdated(rName), Check: resource.ComposeTestCheckFunc( testAccCheckVaultPolicyExists(resourceName, &vault), - resource.TestMatchResourceAttr(resourceName, "policy", regexp.MustCompile("^{\"Version\":\"2012-10-17\".+")), + resource.TestMatchResourceAttr(resourceName, "policy", regexp.MustCompile("^{\"Id\":\"default\".+")), resource.TestMatchResourceAttr(resourceName, "policy", regexp.MustCompile("backup:ListRecoveryPointsByBackupVault")), ), }, @@ -218,7 +218,7 @@ resource "aws_backup_vault_policy" "test" { Version = "2012-10-17" Id = "default" Statement = [{ - Sid = "default" + Sid = "default" Effect = "Allow" Principal = { AWS = "*"