Merge pull request #22170 from hashicorp/b-mediastore-container-polic…

…y-diffs mediastore/container_policy: Fix equivalent policy diffs
hashicorp · Dec 13, 2021 · f61f7f9 · f61f7f9
2 parents 54598b3 + 2d622e0
commit f61f7f9
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 30 deletions.
diff --git a/.changelog/22170.txt b/.changelog/22170.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_media_store_container_policy: Fix erroneous diffs in `policy` when no changes made or policies are equivalent
+```
diff --git a/internal/service/mediastore/container_policy.go b/internal/service/mediastore/container_policy.go
@@ -1,12 +1,14 @@
 package mediastore
 
 import (
+	"fmt"
 	"log"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/mediastore"
 	"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-provider-aws/internal/conns"
 	"github.com/hashicorp/terraform-provider-aws/internal/verify"
 )
@@ -32,6 +34,10 @@ func ResourceContainerPolicy() *schema.Resource {
 				Required:         true,
 				ValidateFunc:     verify.ValidIAMPolicyJSON,
 				DiffSuppressFunc: verify.SuppressEquivalentPolicyDiffs,
+				StateFunc: func(v interface{}) string {
+					json, _ := structure.NormalizeJsonString(v)
+					return json
+				},
 			},
 		},
 	}
@@ -40,12 +46,18 @@ func ResourceContainerPolicy() *schema.Resource {
 func resourceContainerPolicyPut(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*conns.AWSClient).MediaStoreConn
 
+	policy, err := structure.NormalizeJsonString(d.Get("policy").(string))
+
+	if err != nil {
+		return fmt.Errorf("policy (%s) is invalid JSON: %w", policy, err)
+	}
+
 	input := &mediastore.PutContainerPolicyInput{
 		ContainerName: aws.String(d.Get("container_name").(string)),
-		Policy:        aws.String(d.Get("policy").(string)),
+		Policy:        aws.String(policy),
 	}
 
-	_, err := conn.PutContainerPolicy(input)
+	_, err = conn.PutContainerPolicy(input)
 	if err != nil {
 		return err
 	}
@@ -77,7 +89,15 @@ func resourceContainerPolicyRead(d *schema.ResourceData, meta interface{}) error
 	}
 
 	d.Set("container_name", d.Id())
-	d.Set("policy", resp.Policy)
+
+	policyToSet, err := verify.PolicyToSet(d.Get("policy").(string), aws.StringValue(resp.Policy))
+
+	if err != nil {
+		return err
+	}
+
+	d.Set("policy", policyToSet)
+
 	return nil
 }
 

diff --git a/internal/service/mediastore/container_policy_test.go b/internal/service/mediastore/container_policy_test.go
@@ -2,6 +2,7 @@ package mediastore_test
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -15,17 +16,19 @@ import (
 )
 
 func TestAccMediaStoreContainerPolicy_basic(t *testing.T) {
-	rname := sdkacctest.RandString(5)
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	resourceName := "aws_media_store_container_policy.test"
 
+	rName = strings.ReplaceAll(rName, "-", "_")
+
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { acctest.PreCheck(t); testAccPreCheck(t) },
 		ErrorCheck:   acctest.ErrorCheck(t, mediastore.EndpointsID),
 		Providers:    acctest.Providers,
 		CheckDestroy: testAccCheckContainerPolicyDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccMediaStoreContainerPolicyConfig(rname, sdkacctest.RandString(5)),
+				Config: testAccMediaStoreContainerPolicyConfig(rName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckContainerPolicyExists(resourceName),
 					resource.TestCheckResourceAttrSet(resourceName, "container_name"),
@@ -38,7 +41,7 @@ func TestAccMediaStoreContainerPolicy_basic(t *testing.T) {
 				ImportStateVerify: true,
 			},
 			{
-				Config: testAccMediaStoreContainerPolicyConfig(rname, sdkacctest.RandString(5)),
+				Config: testAccMediaStoreContainerPolicyConfig(rName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckContainerPolicyExists(resourceName),
 					resource.TestCheckResourceAttrSet(resourceName, "container_name"),
@@ -99,7 +102,7 @@ func testAccCheckContainerPolicyExists(name string) resource.TestCheckFunc {
 	}
 }
 
-func testAccMediaStoreContainerPolicyConfig(rName, sid string) string {
+func testAccMediaStoreContainerPolicyConfig(rName string) string {
 	return fmt.Sprintf(`
 data "aws_region" "current" {}
 
@@ -108,35 +111,29 @@ data "aws_caller_identity" "current" {}
 data "aws_partition" "current" {}
 
 resource "aws_media_store_container" "test" {
-  name = "tf_mediastore_%s"
+  name = %[1]q
 }
 
 resource "aws_media_store_container_policy" "test" {
   container_name = aws_media_store_container.test.name
 
-  policy = <<EOF
-{
-  "Version": "2012-10-17",
-  "Statement": [
-    {
-      "Sid": "%s",
-      "Action": [
-        "mediastore:*"
-      ],
-      "Principal": {
-        "AWS": "arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:root"
-      },
-      "Effect": "Allow",
-      "Resource": "arn:${data.aws_partition.current.partition}:mediastore:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:container/${aws_media_store_container.test.name}/*",
-      "Condition": {
-        "Bool": {
-          "aws:SecureTransport": "true"
+  policy = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Sid    = "lucky"
+      Action = "mediastore:*"
+      Principal = {
+        AWS = "arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:root"
+      }
+      Effect   = "Allow"
+      Resource = "arn:${data.aws_partition.current.partition}:mediastore:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:container/${aws_media_store_container.test.name}/*"
+      Condition = {
+        Bool = {
+          "aws:SecureTransport" = "true"
         }
       }
-    }
-  ]
-}
-EOF
+    }]
+  })
 }
-`, rName, sid)
+`, rName)
 }