Merge pull request #22130 from hashicorp/b-backup-vault-policy-diffs

backup/vault_policy: Fix diffs with equivalent policies
hashicorp · Dec 9, 2021 · fd08c19 · fd08c19
2 parents 8dacaf6 + bd0169f
commit fd08c19
Show file tree

Hide file tree

Showing 3 changed files with 136 additions and 57 deletions.
diff --git a/.changelog/22130.txt 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
+```
diff --git 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
 }

diff --git 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")),
 				),
 			},
@@ -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 = <<POLICY
-{
-  "Version": "2012-10-17",
-  "Id": "default",
-  "Statement": [
-    {
-      "Sid": "default",
-      "Effect": "Allow",
-      "Principal": {
-        "AWS": "*"
-      },
-      "Action": [
-		"backup:DescribeBackupVault",
-		"backup:DeleteBackupVault",
-		"backup:PutBackupVaultAccessPolicy",
-		"backup:DeleteBackupVaultAccessPolicy",
-		"backup:GetBackupVaultAccessPolicy",
-		"backup:StartBackupJob",
-		"backup:GetBackupVaultNotifications",
-		"backup:PutBackupVaultNotifications"
-      ],
-      "Resource": "${aws_backup_vault.test.arn}"
-    }
-  ]
-}
-POLICY
+  policy = jsonencode({
+    Version = "2012-10-17"
+    Id      = "default"
+    Statement = [{
+      Sid    = "default"
+      Effect = "Allow"
+      Principal = {
+        AWS = "*"
+      }
+      Action = [
+        "backup:DescribeBackupVault",
+        "backup:DeleteBackupVault",
+        "backup:PutBackupVaultAccessPolicy",
+        "backup:DeleteBackupVaultAccessPolicy",
+        "backup:GetBackupVaultAccessPolicy",
+        "backup:StartBackupJob",
+        "backup:GetBackupVaultNotifications",
+        "backup:PutBackupVaultNotifications",
+      ]
+      Resource = aws_backup_vault.test.arn
+    }]
+  })
 }
 `, rName)
 }
@@ -193,33 +214,64 @@ resource "aws_backup_vault" "test" {
 resource "aws_backup_vault_policy" "test" {
   backup_vault_name = aws_backup_vault.test.name
 
-  policy = <<POLICY
-{
-  "Version": "2012-10-17",
-  "Id": "default",
-  "Statement": [
-    {
-      "Sid": "default",
-      "Effect": "Allow",
-      "Principal": {
-        "AWS": "*"
-      },
-      "Action": [
-		"backup:DescribeBackupVault",
-		"backup:DeleteBackupVault",
-		"backup:PutBackupVaultAccessPolicy",
-		"backup:DeleteBackupVaultAccessPolicy",
-		"backup:GetBackupVaultAccessPolicy",
-		"backup:StartBackupJob",
-		"backup:GetBackupVaultNotifications",
-		"backup:PutBackupVaultNotifications",
-		"backup:ListRecoveryPointsByBackupVault"
-      ],
-      "Resource": "${aws_backup_vault.test.arn}"
-    }
-  ]
+  policy = jsonencode({
+    Version = "2012-10-17"
+    Id      = "default"
+    Statement = [{
+      Sid    = "default"
+      Effect = "Allow"
+      Principal = {
+        AWS = "*"
+      }
+      Action = [
+        "backup:DescribeBackupVault",
+        "backup:DeleteBackupVault",
+        "backup:PutBackupVaultAccessPolicy",
+        "backup:DeleteBackupVaultAccessPolicy",
+        "backup:GetBackupVaultAccessPolicy",
+        "backup:StartBackupJob",
+        "backup:GetBackupVaultNotifications",
+        "backup:PutBackupVaultNotifications",
+        "backup:ListRecoveryPointsByBackupVault",
+      ]
+      Resource = aws_backup_vault.test.arn
+    }]
+  })
+}
+`, rName)
+}
+
+func testAccBackupVaultPolicyNewOrderConfig(rName string) string {
+	return fmt.Sprintf(`
+resource "aws_backup_vault" "test" {
+  name = %[1]q
 }
-POLICY
+
+resource "aws_backup_vault_policy" "test" {
+  backup_vault_name = aws_backup_vault.test.name
+
+  policy = jsonencode({
+    Version = "2012-10-17"
+    Id      = "default"
+    Statement = [{
+      Sid    = "default"
+      Effect = "Allow"
+      Principal = {
+        AWS = "*"
+      }
+      Action = [
+        "backup:DeleteBackupVault",
+        "backup:PutBackupVaultNotifications",
+        "backup:GetBackupVaultNotifications",
+        "backup:GetBackupVaultAccessPolicy",
+        "backup:DeleteBackupVaultAccessPolicy",
+        "backup:DescribeBackupVault",
+        "backup:StartBackupJob",
+        "backup:PutBackupVaultAccessPolicy",
+      ]
+      Resource = aws_backup_vault.test.arn
+    }]
+  })
 }
 `, rName)
 }