From 0040ea6911af00bea2cca6dbb14e86829826ad8e Mon Sep 17 00:00:00 2001
From: Dirk Avery <dirk.avery@gmail.com>
Date: Wed, 11 Jan 2023 15:43:02 -0500
Subject: [PATCH 1/4] iam: Improve policy diff handling

---
 internal/service/iam/group_policy.go      |  14 +-
 internal/service/iam/group_policy_test.go |   4 +-
 internal/service/iam/policy.go            |  15 +-
 internal/service/iam/policy_test.go       | 158 ++++++++++++
 internal/service/iam/role.go              |  26 +-
 internal/service/iam/role_policy.go       |  14 +-
 internal/service/iam/role_test.go         | 288 +++++++++++++++++++++-
 internal/service/iam/user_policy.go       |  15 +-
 internal/service/iam/user_policy_test.go  |  16 +-
 internal/verify/json.go                   |  41 ++-
 internal/verify/json_test.go              | 175 +++++++++++++
 11 files changed, 716 insertions(+), 50 deletions(-)

diff --git a/internal/service/iam/group_policy.go b/internal/service/iam/group_policy.go
index 261b70a6ebd..bd1490a4ff0 100644
--- a/internal/service/iam/group_policy.go
+++ b/internal/service/iam/group_policy.go
@@ -36,6 +36,10 @@ func ResourceGroupPolicy() *schema.Resource {
 				ValidateFunc:          verify.ValidIAMPolicyJSON,
 				DiffSuppressFunc:      verify.SuppressEquivalentPolicyDiffs,
 				DiffSuppressOnRefresh: true,
+				StateFunc: func(v interface{}) string {
+					json, _ := verify.LegacyPolicyNormalize(v)
+					return json
+				},
 			},
 			"name": {
 				Type:          schema.TypeString,
@@ -62,9 +66,14 @@ func ResourceGroupPolicy() *schema.Resource {
 func resourceGroupPolicyPut(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*conns.AWSClient).IAMConn()
 
+	policyDoc, err := verify.LegacyPolicyNormalize(d.Get("policy").(string))
+	if err != nil {
+		return fmt.Errorf("policy (%s) is invalid JSON: %w", policyDoc, err)
+	}
+
 	request := &iam.PutGroupPolicyInput{
 		GroupName:      aws.String(d.Get("group").(string)),
-		PolicyDocument: aws.String(d.Get("policy").(string)),
+		PolicyDocument: aws.String(policyDoc),
 	}
 
 	var policyName string
@@ -139,8 +148,7 @@ func resourceGroupPolicyRead(d *schema.ResourceData, meta interface{}) error {
 		return err
 	}
 
-	policyToSet, err := verify.SecondJSONUnlessEquivalent(d.Get("policy").(string), policy)
-
+	policyToSet, err := verify.LegacyPolicyToSet(d.Get("policy").(string), policy)
 	if err != nil {
 		return fmt.Errorf("while setting policy (%s), encountered: %w", policyToSet, err)
 	}
diff --git a/internal/service/iam/group_policy_test.go b/internal/service/iam/group_policy_test.go
index 817f447548c..41a226c8262 100644
--- a/internal/service/iam/group_policy_test.go
+++ b/internal/service/iam/group_policy_test.go
@@ -339,12 +339,12 @@ resource "aws_iam_group_policy" "test" {
 
   policy = <<EOF
 {
-  "Version": "2012-10-17",
   "Statement": {
     "Effect": "Allow",
     "Action": %[2]q,
     "Resource": "*"
-  }
+  },
+  "Version": "2012-10-17"
 }
 EOF
 }
diff --git a/internal/service/iam/policy.go b/internal/service/iam/policy.go
index fc3773838f9..4f760783eb2 100644
--- a/internal/service/iam/policy.go
+++ b/internal/service/iam/policy.go
@@ -50,6 +50,10 @@ func ResourcePolicy() *schema.Resource {
 				ValidateFunc:          verify.ValidIAMPolicyJSON,
 				DiffSuppressFunc:      verify.SuppressEquivalentPolicyDiffs,
 				DiffSuppressOnRefresh: true,
+				StateFunc: func(v interface{}) string {
+					json, _ := structure.NormalizeJsonString(v)
+					return json
+				},
 			},
 			"name": {
 				Type:          schema.TypeString,
@@ -97,7 +101,6 @@ func resourcePolicyCreate(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	policy, err := structure.NormalizeJsonString(d.Get("policy").(string))
-
 	if err != nil {
 		return fmt.Errorf("policy (%s) is invalid JSON: %w", policy, err)
 	}
@@ -259,18 +262,11 @@ func resourcePolicyRead(d *schema.ResourceData, meta interface{}) error {
 		}
 	}
 
-	policyToSet, err := verify.SecondJSONUnlessEquivalent(d.Get("policy").(string), policyDocument)
-
+	policyToSet, err := verify.PolicyToSet(d.Get("policy").(string), policyDocument)
 	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
@@ -285,7 +281,6 @@ func resourcePolicyUpdate(d *schema.ResourceData, meta interface{}) error {
 		}
 
 		policy, err := structure.NormalizeJsonString(d.Get("policy").(string))
-
 		if err != nil {
 			return fmt.Errorf("policy (%s) is invalid JSON: %w", policy, err)
 		}
diff --git a/internal/service/iam/policy_test.go b/internal/service/iam/policy_test.go
index 4c4b37945b2..f87ad997cc0 100644
--- a/internal/service/iam/policy_test.go
+++ b/internal/service/iam/policy_test.go
@@ -239,6 +239,94 @@ func TestAccIAMPolicy_policy(t *testing.T) {
 	})
 }
 
+// https://github.com/hashicorp/terraform-provider-aws/issues/23288#issuecomment-1175361016
+func TestAccIAMPolicy_diffs(t *testing.T) {
+	var out iam.GetPolicyOutput
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_iam_policy.test"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(t) },
+		ErrorCheck:               acctest.ErrorCheck(t, iam.EndpointsID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckPolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccPolicyConfig_diffs(rName, ""),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPolicyExists(resourceName, &out),
+					resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			{
+				Config: testAccPolicyConfig_diffs(rName, ""),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPolicyExists(resourceName, &out),
+					resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
+				),
+			},
+			{
+				Config:   testAccPolicyConfig_diffs(rName, ""),
+				PlanOnly: true,
+			},
+			{
+				Config:   testAccPolicyConfig_diffs(rName, ""),
+				PlanOnly: true,
+			},
+			{
+				Config:   testAccPolicyConfig_diffs(rName, ""),
+				PlanOnly: true,
+			},
+			{
+				Config:   testAccPolicyConfig_diffs(rName, ""),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccPolicyConfig_diffs(rName, "tags = {}"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPolicyExists(resourceName, &out),
+					resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
+				),
+			},
+			{
+				Config: testAccPolicyConfig_diffs(rName, "tags = {}"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPolicyExists(resourceName, &out),
+					resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
+				),
+			},
+			{
+				Config:   testAccPolicyConfig_diffs(rName, "tags = {}"),
+				PlanOnly: true,
+			},
+			{
+				Config:   testAccPolicyConfig_diffs(rName, "tags = {}"),
+				PlanOnly: true,
+			},
+			{
+				Config:   testAccPolicyConfig_diffs(rName, "tags = {}"),
+				PlanOnly: true,
+			},
+			{
+				Config:   testAccPolicyConfig_diffs(rName, "tags = {}"),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccPolicyConfig_diffs(rName, "tags = {}"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPolicyExists(resourceName, &out),
+					resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckPolicyExists(resource string, res *iam.GetPolicyOutput) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[resource]
@@ -448,3 +536,73 @@ EOF
 }
 `, rName, tagKey1, tagValue1, tagKey2, tagValue2)
 }
+
+func testAccPolicyConfig_diffs(rName string, tags string) string {
+	return fmt.Sprintf(`
+resource "aws_iam_policy" "test" {
+  name = %[1]q
+
+  policy = jsonencode({
+    Id = %[1]q
+    Version = "2012-10-17"
+    Statement = [{
+      Sid = "60c9d11f"
+      Effect = "Allow"
+      Action = [
+        "kms:Describe*",
+        "kms:Get*",
+        "kms:List*",
+        "kms:CreateKey",
+        "kms:DescribeKey",
+        "kms:ScheduleKeyDeletion",
+        "kms:TagResource",
+        "kms:UntagResource",
+        "s3:ListMultipartUploadParts",
+        "s3:ListBucketVersions",
+        "s3:ListBucketMultipartUploads",
+        "s3:ListBucket",
+        "s3:GetReplicationConfiguration",
+        "s3:GetObjectVersionTorrent",
+        "s3:GetObjectVersionTagging",
+        "s3:GetObjectVersionForReplication",
+        "s3:GetObjectVersionAcl",
+        "s3:GetObjectVersion",
+        "s3:GetObjectTorrent",
+        "s3:GetObjectTagging",
+        "s3:GetObjectRetention",
+        "s3:GetObjectLegalHold",
+        "s3:GetObjectAcl",
+        "s3:GetObject",
+        "s3:GetMetricsConfiguration",
+        "s3:GetLifecycleConfiguration",
+        "s3:GetJobTagging",
+        "s3:GetInventoryConfiguration",
+        "s3:GetEncryptionConfiguration",
+        "s3:GetBucketWebsite",
+        "s3:GetBucketVersioning",
+        "s3:GetBucketTagging",
+        "s3:GetBucketRequestPayment",
+        "s3:GetBucketPublicAccessBlock",
+        "s3:GetBucketPolicyStatus",
+        "s3:GetBucketPolicy",
+        "s3:GetBucketOwnershipControls",
+        "s3:GetBucketObjectLockConfiguration",
+        "s3:GetBucketNotification",
+        "s3:GetBucketLogging",
+        "s3:GetBucketLocation",
+        "s3:GetBucketCORS",
+        "s3:GetBucketAcl",
+        "s3:GetAnalyticsConfiguration",
+        "s3:GetAccessPointPolicyStatus",
+        "s3:GetAccessPointPolicy",
+        "s3:GetAccelerateConfiguration",
+        "s3:DescribeJob",
+      ]
+      Resource = "*"
+    }]
+  })
+
+  %[2]s
+}
+`, rName, tags)
+}
diff --git a/internal/service/iam/role.go b/internal/service/iam/role.go
index 57832856b77..d2b603591d8 100644
--- a/internal/service/iam/role.go
+++ b/internal/service/iam/role.go
@@ -36,9 +36,11 @@ func ResourceRole() *schema.Resource {
 		Read:   resourceRoleRead,
 		Update: resourceRoleUpdate,
 		Delete: resourceRoleDelete,
+
 		Importer: &schema.ResourceImporter{
 			State: resourceRoleImport,
 		},
+
 		Schema: map[string]*schema.Schema{
 			"arn": {
 				Type:     schema.TypeString,
@@ -93,6 +95,10 @@ func ResourceRole() *schema.Resource {
 							ValidateFunc:          verify.ValidIAMPolicyJSON,
 							DiffSuppressFunc:      verify.SuppressEquivalentPolicyDiffs,
 							DiffSuppressOnRefresh: true,
+							StateFunc: func(v interface{}) string {
+								json, _ := verify.LegacyPolicyNormalize(v)
+								return json
+							},
 						},
 					},
 				},
@@ -170,7 +176,6 @@ func resourceRoleCreate(d *schema.ResourceData, meta interface{}) error {
 	tags := defaultTagsConfig.MergeTags(tftags.New(d.Get("tags").(map[string]interface{})))
 
 	assumeRolePolicy, err := structure.NormalizeJsonString(d.Get("assume_role_policy").(string))
-
 	if err != nil {
 		return fmt.Errorf("assume_role_policy (%s) is invalid JSON: %w", assumeRolePolicy, err)
 	}
@@ -287,13 +292,11 @@ func resourceRoleRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("unique_id", role.RoleId)
 
 	assumeRolePolicy, err := url.QueryUnescape(aws.StringValue(role.AssumeRolePolicyDocument))
-
 	if err != nil {
 		return err
 	}
 
 	policyToSet, err := verify.PolicyToSet(d.Get("assume_role_policy").(string), assumeRolePolicy)
-
 	if err != nil {
 		return err
 	}
@@ -341,7 +344,6 @@ func resourceRoleUpdate(d *schema.ResourceData, meta interface{}) error {
 
 	if d.HasChange("assume_role_policy") {
 		assumeRolePolicy, err := structure.NormalizeJsonString(d.Get("assume_role_policy").(string))
-
 		if err != nil {
 			return fmt.Errorf("assume_role_policy (%s) is invalid JSON: %w", assumeRolePolicy, err)
 		}
@@ -857,24 +859,20 @@ func readRoleInlinePolicies(roleName string, meta interface{}) ([]*iam.PutRolePo
 			return nil, err
 		}
 
+		p, err := structure.NormalizeJsonString(policy)
+		if err != nil {
+			return nil, fmt.Errorf("policy (%s) is invalid JSON: %w", p, err)
+		}
+
 		apiObject := &iam.PutRolePolicyInput{
 			RoleName:       aws.String(roleName),
-			PolicyDocument: aws.String(policy),
+			PolicyDocument: aws.String(p),
 			PolicyName:     policyName,
 		}
 
 		apiObjects = append(apiObjects, apiObject)
 	}
 
-	/*
-		if len(apiObjects) == 0 {
-			apiObjects = append(apiObjects, &iam.PutRolePolicyInput{
-				PolicyDocument: aws.String(""),
-				PolicyName:     aws.String(""),
-			})
-		}
-	*/
-
 	return apiObjects, nil
 }
 
diff --git a/internal/service/iam/role_policy.go b/internal/service/iam/role_policy.go
index e1fd1431a49..33ca348f606 100644
--- a/internal/service/iam/role_policy.go
+++ b/internal/service/iam/role_policy.go
@@ -40,6 +40,10 @@ func ResourceRolePolicy() *schema.Resource {
 				ValidateFunc:          verify.ValidIAMPolicyJSON,
 				DiffSuppressFunc:      verify.SuppressEquivalentPolicyDiffs,
 				DiffSuppressOnRefresh: true,
+				StateFunc: func(v interface{}) string {
+					json, _ := verify.LegacyPolicyNormalize(v)
+					return json
+				},
 			},
 			"name": {
 				Type:          schema.TypeString,
@@ -69,9 +73,14 @@ func ResourceRolePolicy() *schema.Resource {
 func resourceRolePolicyPut(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*conns.AWSClient).IAMConn()
 
+	policy, err := verify.LegacyPolicyNormalize(d.Get("policy").(string))
+	if err != nil {
+		return fmt.Errorf("policy (%s) is invalid JSON: %w", policy, err)
+	}
+
 	request := &iam.PutRolePolicyInput{
 		RoleName:       aws.String(d.Get("role").(string)),
-		PolicyDocument: aws.String(d.Get("policy").(string)),
+		PolicyDocument: aws.String(policy),
 	}
 
 	var policyName string
@@ -146,8 +155,7 @@ func resourceRolePolicyRead(d *schema.ResourceData, meta interface{}) error {
 		return err
 	}
 
-	policyToSet, err := verify.SecondJSONUnlessEquivalent(d.Get("policy").(string), policy)
-
+	policyToSet, err := verify.LegacyPolicyToSet(d.Get("policy").(string), policy)
 	if err != nil {
 		return fmt.Errorf("while setting policy (%s), encountered: %w", policyToSet, err)
 	}
diff --git a/internal/service/iam/role_test.go b/internal/service/iam/role_test.go
index 76bf4ce6b11..804bf243288 100644
--- a/internal/service/iam/role_test.go
+++ b/internal/service/iam/role_test.go
@@ -175,6 +175,189 @@ func TestAccIAMRole_testNameChange(t *testing.T) {
 	})
 }
 
+// https://github.com/hashicorp/terraform-provider-aws/issues/23288
+// https://github.com/hashicorp/terraform-provider-aws/issues/28833
+func TestAccIAMRole_diffs(t *testing.T) {
+	var conf iam.Role
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_iam_role.test"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(t) },
+		ErrorCheck:               acctest.ErrorCheck(t, iam.EndpointsID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckRoleDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccRoleConfig_diffs(rName, ""),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffs(rName, ""),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffs(rName, ""),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffs(rName, ""),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffs(rName, ""),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffs(rName, ""),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffs(rName, ""),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffs(rName, ""),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffs(rName, ""),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffs(rName, ""),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffs(rName, ""),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffs(rName, ""),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffs(rName, "tags = {}"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffs(rName, "tags = {}"),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffs(rName, "tags = {}"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffs(rName, "tags = {}"),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffs(rName, "tags = {}"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffs(rName, "tags = {}"),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffs(rName, "tags = {}"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffs(rName, "tags = {}"),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffs(rName, "tags = {}"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffs(rName, "tags = {}"),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffs(rName, "tags = {}"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffs(rName, "tags = {}"),
+				PlanOnly: true,
+			},
+		},
+	})
+}
+
+// https://github.com/hashicorp/terraform-provider-aws/issues/28835
+func TestAccIAMRole_diffsCondition(t *testing.T) {
+	var conf iam.Role
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_iam_role.test"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(t) },
+		ErrorCheck:               acctest.ErrorCheck(t, iam.EndpointsID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckRoleDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccRoleConfig_diffsCondition(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffsCondition(rName),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffsCondition(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffsCondition(rName),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccRoleConfig_diffsCondition(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoleExists(resourceName, &conf),
+				),
+			},
+			{
+				Config:   testAccRoleConfig_diffsCondition(rName),
+				PlanOnly: true,
+			},
+		},
+	})
+}
+
 func TestAccIAMRole_badJSON(t *testing.T) {
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 
@@ -441,16 +624,16 @@ func TestAccIAMRole_InlinePolicy_basic(t *testing.T) {
 				),
 			},
 			{
-				ResourceName:      resourceName,
-				ImportState:       true,
-				ImportStateVerify: true,
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"inline_policy.0.policy"},
 			},
 		},
 	})
 }
 
 // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/19444
-// This test currently fails but should not. A new PR will fix it.
 func TestAccIAMRole_InlinePolicy_ignoreOrder(t *testing.T) {
 	var role iam.Role
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
@@ -1090,6 +1273,103 @@ resource "aws_iam_role" "test" {
 `, rName)
 }
 
+func testAccRoleConfig_diffs(rName, tags string) string {
+	return fmt.Sprintf(`
+data "aws_partition" "current" {}
+
+resource "aws_iam_user" "user1" {
+  name = "%[1]s-baa204a2"
+  path = "/"
+}
+
+resource "aws_iam_user" "user2" {
+  name = "%[1]s-fee06121"
+  path = "/"
+}
+
+resource "aws_iam_user" "user3" {
+  name = "%[1]s-2f0d132b"
+  path = "/"
+}
+
+resource "aws_iam_user" "user4" {
+  name = "%[1]s-d1eaee06"
+  path = "/"
+}
+
+resource "aws_iam_user" "user5" {
+  name = "%[1]s-d4a38c26"
+  path = "/"
+}
+
+resource "aws_iam_role" "test" {
+  name = %[1]q
+  path = "/"
+
+  assume_role_policy = jsonencode({
+    Id = %[1]q
+    Version = "2012-10-17"
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Principal = {
+        AWS = [
+          aws_iam_user.user1.arn,
+          aws_iam_user.user2.arn,
+          aws_iam_user.user3.arn,
+          aws_iam_user.user4.arn,
+          aws_iam_user.user5.arn,
+        ]
+      }
+      Effect = "Allow"
+      Sid    = ""
+    }]
+  })
+
+  %[2]s
+}
+`, rName, tags)
+}
+
+func testAccRoleConfig_diffsCondition(rName string) string {
+	return fmt.Sprintf(`
+data "aws_partition" "current" {}
+
+resource "aws_iam_user" "user1" {
+  name = "%[1]s-cde2c453"
+  path = "/"
+}
+
+resource "aws_iam_role" "test" {
+  name = %[1]q
+  path = "/"
+
+  assume_role_policy = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Action = "sts:AssumeRoleWithSAML"
+      Condition = {
+        IpAddress = {
+          "aws:SourceIp" = [
+            "0.0.0.0/0",
+          ]
+        }
+        StringEquals = {
+          "SAML:aud" = "https://signin.aws.amazon.com/saml"
+        }
+      }
+      Principal = {
+        AWS = [
+          aws_iam_user.user1.arn,
+        ]
+      }
+      Effect = "Allow"
+      Sid    = ""
+    }]
+  })
+}
+`, rName)
+}
+
 func testAccRoleConfig_description(rName string) string {
 	return fmt.Sprintf(`
 data "aws_partition" "current" {}
diff --git a/internal/service/iam/user_policy.go b/internal/service/iam/user_policy.go
index 295a6503669..b1114f201e8 100644
--- a/internal/service/iam/user_policy.go
+++ b/internal/service/iam/user_policy.go
@@ -35,6 +35,10 @@ func ResourceUserPolicy() *schema.Resource {
 				ValidateFunc:          verify.ValidIAMPolicyJSON,
 				DiffSuppressFunc:      verify.SuppressEquivalentPolicyDiffs,
 				DiffSuppressOnRefresh: true,
+				StateFunc: func(v interface{}) string {
+					json, _ := verify.LegacyPolicyNormalize(v)
+					return json
+				},
 			},
 			"name": {
 				Type:          schema.TypeString,
@@ -61,13 +65,17 @@ func ResourceUserPolicy() *schema.Resource {
 func resourceUserPolicyPut(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*conns.AWSClient).IAMConn()
 
+	p, err := verify.LegacyPolicyNormalize(d.Get("policy").(string))
+	if err != nil {
+		return fmt.Errorf("policy (%s) is invalid JSON: %w", p, err)
+	}
+
 	request := &iam.PutUserPolicyInput{
 		UserName:       aws.String(d.Get("user").(string)),
-		PolicyDocument: aws.String(d.Get("policy").(string)),
+		PolicyDocument: aws.String(p),
 	}
 
 	var policyName string
-	var err error
 	if !d.IsNewResource() {
 		_, policyName, err = UserPolicyParseID(d.Id())
 		if err != nil {
@@ -144,8 +152,7 @@ func resourceUserPolicyRead(d *schema.ResourceData, meta interface{}) error {
 		return err
 	}
 
-	policyToSet, err := verify.SecondJSONUnlessEquivalent(d.Get("policy").(string), policy)
-
+	policyToSet, err := verify.LegacyPolicyToSet(d.Get("policy").(string), policy)
 	if err != nil {
 		return fmt.Errorf("while setting policy (%s), encountered: %w", policyToSet, err)
 	}
diff --git a/internal/service/iam/user_policy_test.go b/internal/service/iam/user_policy_test.go
index 436294e3243..a94e69a56bb 100644
--- a/internal/service/iam/user_policy_test.go
+++ b/internal/service/iam/user_policy_test.go
@@ -19,8 +19,8 @@ import (
 
 func TestAccIAMUserPolicy_basic(t *testing.T) {
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
-	policy1 := `{"Version":"2012-10-17","Statement":{"Effect":"Allow","Action":"*","Resource":"*"}}`
-	policy2 := `{"Version":"2012-10-17","Statement":{"Effect":"Allow","Action":"iam:*","Resource":"*"}}`
+	policy1 := `{"Version":"2012-10-17","Statement":{"Action":"*","Effect":"Allow","Resource":"*"}}`
+	policy2 := `{"Version":"2012-10-17","Statement":{"Action":"iam:*","Effect":"Allow","Resource":"*"}}`
 	policyResourceName := "aws_iam_user_policy.test"
 	userResourceName := "aws_iam_user.test"
 
@@ -87,8 +87,8 @@ func TestAccIAMUserPolicy_disappears(t *testing.T) {
 
 func TestAccIAMUserPolicy_namePrefix(t *testing.T) {
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
-	policy1 := `{"Version":"2012-10-17","Statement":{"Effect":"Allow","Action":"*","Resource":"*"}}`
-	policy2 := `{"Version":"2012-10-17","Statement":{"Effect":"Allow","Action":"iam:*","Resource":"*"}}`
+	policy1 := `{"Version":"2012-10-17","Statement":{"Action":"*","Effect":"Allow","Resource":"*"}}`
+	policy2 := `{"Version":"2012-10-17","Statement":{"Action":"iam:*","Effect":"Allow","Resource":"*"}}`
 	policyResourceName := "aws_iam_user_policy.test"
 	userResourceName := "aws_iam_user.test"
 
@@ -128,8 +128,8 @@ func TestAccIAMUserPolicy_namePrefix(t *testing.T) {
 
 func TestAccIAMUserPolicy_generatedName(t *testing.T) {
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
-	policy1 := `{"Version":"2012-10-17","Statement":{"Effect":"Allow","Action":"*","Resource":"*"}}`
-	policy2 := `{"Version":"2012-10-17","Statement":{"Effect":"Allow","Action":"iam:*","Resource":"*"}}`
+	policy1 := `{"Version":"2012-10-17","Statement":{"Action":"*","Effect":"Allow","Resource":"*"}}`
+	policy2 := `{"Version":"2012-10-17","Statement":{"Action":"iam:*","Effect":"Allow","Resource":"*"}}`
 	policyResourceName := "aws_iam_user_policy.test"
 	userResourceName := "aws_iam_user.test"
 
@@ -167,8 +167,8 @@ func TestAccIAMUserPolicy_generatedName(t *testing.T) {
 
 func TestAccIAMUserPolicy_multiplePolicies(t *testing.T) {
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
-	policy1 := `{"Version":"2012-10-17","Statement":{"Effect":"Allow","Action":"*","Resource":"*"}}`
-	policy2 := `{"Version":"2012-10-17","Statement":{"Effect":"Allow","Action":"iam:*","Resource":"*"}}`
+	policy1 := `{"Version":"2012-10-17","Statement":{"Action":"*","Effect":"Allow","Resource":"*"}}`
+	policy2 := `{"Version":"2012-10-17","Statement":{"Action":"iam:*","Effect":"Allow","Resource":"*"}}`
 	policyResourceName1 := "aws_iam_user_policy.test"
 	policyResourceName2 := "aws_iam_user_policy.test2"
 	userResourceName := "aws_iam_user.test"
diff --git a/internal/verify/json.go b/internal/verify/json.go
index 94961aa67d4..1b6cdf4d06b 100644
--- a/internal/verify/json.go
+++ b/internal/verify/json.go
@@ -133,16 +133,53 @@ func SecondJSONUnlessEquivalent(old, new string) (string, error) {
 // Otherwise, it returns the new policy. Either policy is normalized.
 func PolicyToSet(exist, new string) (string, error) {
 	policyToSet, err := SecondJSONUnlessEquivalent(exist, new)
-
 	if err != nil {
 		return "", fmt.Errorf("while checking equivalency of existing policy (%s) and new policy (%s), encountered: %w", exist, new, err)
 	}
 
 	policyToSet, err = structure.NormalizeJsonString(policyToSet)
-
 	if err != nil {
 		return "", fmt.Errorf("policy (%s) is invalid JSON: %w", policyToSet, err)
 	}
 
 	return policyToSet, nil
 }
+
+// LegacyPolicyNormalize returns a "normalized" JSON policy document except
+// the Version element is first in the JSON as required by AWS in many places.
+// Version not being first is one reason for this error:
+// MalformedPolicyDocument: The policy failed legacy parsing
+func LegacyPolicyNormalize(policy interface{}) (string, error) {
+	np, err := structure.NormalizeJsonString(policy)
+	if err != nil {
+		return "", fmt.Errorf("legacy policy (%s) is invalid JSON: %w", policy, err)
+	}
+
+	//fmt.Printf("first norm: %s\n", np)
+
+	m := regexp.MustCompile(`(?s)^(\{\n?)(.*?)(,\s*)?(  )?("Version":\s*"2012-10-17")(,)?(\n)?(.*?)(\})`)
+
+	n := m.ReplaceAllString(np, `$1$4$5$3$2$6$7$8$9`)
+	_, err = structure.NormalizeJsonString(n)
+	if err != nil {
+		return "", fmt.Errorf("LegacyPolicyNormalize created a policy (%s) that is invalid JSON: %w", n, err)
+	}
+
+	return n, nil
+}
+
+// PolicyToSet returns the existing policy if the new policy is equivalent.
+// Otherwise, it returns the new policy. Either policy is normalized.
+func LegacyPolicyToSet(exist, new string) (string, error) {
+	policyToSet, err := SecondJSONUnlessEquivalent(exist, new)
+	if err != nil {
+		return "", fmt.Errorf("while checking equivalency of existing policy (%s) and new policy (%s), encountered: %w", exist, new, err)
+	}
+
+	policyToSet, err = LegacyPolicyNormalize(policyToSet)
+	if err != nil {
+		return "", fmt.Errorf("legacy policy (%s) is invalid JSON: %w", policyToSet, err)
+	}
+
+	return policyToSet, nil
+}
diff --git a/internal/verify/json_test.go b/internal/verify/json_test.go
index 61e42bddfa3..eb758125c2d 100644
--- a/internal/verify/json_test.go
+++ b/internal/verify/json_test.go
@@ -613,3 +613,178 @@ Outputs:
 		}
 	}
 }
+
+func TestLegacyPolicyNormalize(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		Name     string
+		Input    string
+		Expected string
+		Error    bool
+	}{
+		{
+			Name:     "basic",
+			Input:    `{"Statement":{"Action":"*","Effect":"Allow","Resource":"*"},"Version":"2012-10-17"}`,
+			Expected: `{"Version":"2012-10-17","Statement":{"Action":"*","Effect":"Allow","Resource":"*"}}`,
+			Error:    false,
+		},
+		{
+			Name: "normalWhitespace",
+			Input: `{
+  "Statement": {
+    "Effect": "Allow",
+    "Action": "*",
+    "Resource": "*"
+  },
+  "Version": "2012-10-17"
+}
+`,
+			Expected: `{"Version":"2012-10-17","Statement":{"Action":"*","Effect":"Allow","Resource":"*"}}`,
+			Error:    false,
+		},
+		{
+			Name: "badJSON",
+			Input: `{
+  "Statement": {
+    "Effect": "Allow",
+    "Action": "*",
+    "Resource": "*"
+  }
+  "Version": "2012-10-17"
+}
+`,
+			Expected: ``,
+			Error:    true,
+		},
+		{
+			Name: "principal",
+			Input: `{
+  "Statement": [
+    {
+      "Action": "sts:AssumeRole",
+      "Principal": {
+        "Service": "s3.amazonaws.com"
+      },
+      "Effect": "Allow",
+      "Sid": ""    }
+  ],
+  "Version": "2012-10-17"
+}
+`,
+			Expected: `{"Version":"2012-10-17","Statement":[{"Action":"sts:AssumeRole","Effect":"Allow","Principal":{"Service":"s3.amazonaws.com"},"Sid":""}]}`,
+			Error:    false,
+		},
+		{
+			Name: "id",
+			Input: `{
+  "Id": "Kygo",
+  "Statement": [
+    {
+      "Action": "sts:AssumeRole",
+      "Principal": {
+        "Service": "s3.amazonaws.com"
+      },
+      "Effect": "Allow",
+      "Sid": ""    }
+  ],
+  "Version": "2012-10-17"
+}
+`,
+			Expected: `{"Version":"2012-10-17","Id":"Kygo","Statement":[{"Action":"sts:AssumeRole","Effect":"Allow","Principal":{"Service":"s3.amazonaws.com"},"Sid":""}]}`,
+			Error:    false,
+		},
+		{
+			Name: "newOrder",
+			Input: `{
+  "Id": "Kygo",
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Action": "sts:AssumeRole",
+      "Principal": {
+        "Service": "s3.amazonaws.com"
+      },
+      "Effect": "Allow",
+      "Sid": ""    }
+  ]
+}
+`,
+			Expected: `{"Version":"2012-10-17","Id":"Kygo","Statement":[{"Action":"sts:AssumeRole","Effect":"Allow","Principal":{"Service":"s3.amazonaws.com"},"Sid":""}]}`,
+			Error:    false,
+		},
+		{
+			Name: "complex1",
+			Input: `{
+"Id": "kms-tf-1",
+"Version": "2012-10-17",
+"Statement": [
+  {
+    "Sid": "Enable IAM User Permissions",
+    "Effect": "Allow",
+    "Principal": {
+      "AWS": [
+        "arn:aws:iam::012345678901:role/felixjaehn",
+        "arn:aws:iam::012345678901:role/garethemery",
+        "arn:aws:iam::012345678901:role/kidnap",
+        "arn:aws:iam::012345678901:role/paulvandyk",
+        "arn:aws:iam::012345678901:role/tinlicker"
+      ]
+    },
+    "Action": [
+      "kms:Describe*",
+      "kms:Get*",
+      "kms:List*",
+      "kms:CreateKey",
+      "kms:DescribeKey",
+      "kms:ScheduleKeyDeletion",
+      "kms:TagResource",
+      "kms:UntagResource"
+    ],
+    "Resource": "*"
+  }
+]
+}`,
+			Expected: `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Action":["kms:Describe*","kms:Get*","kms:List*","kms:CreateKey","kms:DescribeKey","kms:ScheduleKeyDeletion","kms:TagResource","kms:UntagResource"],"Effect":"Allow","Principal":{"AWS":["arn:aws:iam::012345678901:role/felixjaehn","arn:aws:iam::012345678901:role/garethemery","arn:aws:iam::012345678901:role/kidnap","arn:aws:iam::012345678901:role/paulvandyk","arn:aws:iam::012345678901:role/tinlicker"]},"Resource":"*","Sid":"Enable IAM User Permissions"}]}`,
+			Error:    false,
+		},
+		{
+			Name: "complex2",
+			Input: `{
+"Id": "kms-tf-1",
+"ZedsDead": "StillWorse",
+"Version": "2012-10-17",
+"Statement": [
+  {
+    "Sid": "Enable IAM User Permissions"
+  }
+]
+}`,
+			Expected: `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions"}],"ZedsDead":"StillWorse"}`,
+			Error:    false,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.Name, func(t *testing.T) {
+			t.Parallel()
+
+			p, err := LegacyPolicyNormalize(tc.Input)
+
+			if tc.Error {
+				if err == nil {
+					t.Errorf("expected an error")
+				}
+			} else {
+				if err != nil {
+					t.Errorf("expected no error, got: %s", err)
+				}
+			}
+
+			if p != tc.Expected {
+				t.Errorf("expected %s, got: %s", tc.Expected, p)
+			}
+		})
+	}
+}

From ff6049687f971291a60646d6d233856bf528f273 Mon Sep 17 00:00:00 2001
From: Dirk Avery <dirk.avery@gmail.com>
Date: Wed, 11 Jan 2023 15:54:06 -0500
Subject: [PATCH 2/4] Add changelog

---
 .changelog/28836.txt | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 .changelog/28836.txt

diff --git a/.changelog/28836.txt b/.changelog/28836.txt
new file mode 100644
index 00000000000..0a76e23d839
--- /dev/null
+++ b/.changelog/28836.txt
@@ -0,0 +1,19 @@
+```release-note:bug
+resource/aws_iam_group_policy: Improve refresh to avoid unnecessary diffs in `policy`
+```
+
+```release-note:bug
+resource/aws_iam_policy: Improve refresh to avoid unnecessary diffs in `policy`, `tags`
+```
+
+```release-note:bug
+resource/aws_iam_role: Improve refresh to avoid unnecessary diffs in `inline_policy.*.policy`, `tags`
+```
+
+```release-note:bug
+resource/aws_iam_role_policy: Improve refresh to avoid unnecessary diffs in `policy`
+```
+
+```release-note:bug
+resource/aws_iam_user_policy: Improve refresh to avoid unnecessary diffs in `policy`
+```
\ No newline at end of file

From 38683d06bce08b8a392ece53aeb64a0c8520889e Mon Sep 17 00:00:00 2001
From: Dirk Avery <dirk.avery@gmail.com>
Date: Wed, 11 Jan 2023 15:54:21 -0500
Subject: [PATCH 3/4] Fixes and cleanup

---
 internal/service/iam/policy_test.go | 2 +-
 internal/service/iam/role.go        | 2 +-
 internal/verify/json.go             | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/internal/service/iam/policy_test.go b/internal/service/iam/policy_test.go
index f87ad997cc0..a3f97c560fd 100644
--- a/internal/service/iam/policy_test.go
+++ b/internal/service/iam/policy_test.go
@@ -239,7 +239,7 @@ func TestAccIAMPolicy_policy(t *testing.T) {
 	})
 }
 
-// https://github.com/hashicorp/terraform-provider-aws/issues/23288#issuecomment-1175361016
+// https://github.com/hashicorp/terraform-provider-aws/issues/28833
 func TestAccIAMPolicy_diffs(t *testing.T) {
 	var out iam.GetPolicyOutput
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
diff --git a/internal/service/iam/role.go b/internal/service/iam/role.go
index d2b603591d8..1a6794beb5c 100644
--- a/internal/service/iam/role.go
+++ b/internal/service/iam/role.go
@@ -859,7 +859,7 @@ func readRoleInlinePolicies(roleName string, meta interface{}) ([]*iam.PutRolePo
 			return nil, err
 		}
 
-		p, err := structure.NormalizeJsonString(policy)
+		p, err := verify.LegacyPolicyNormalize(policy)
 		if err != nil {
 			return nil, fmt.Errorf("policy (%s) is invalid JSON: %w", p, err)
 		}
diff --git a/internal/verify/json.go b/internal/verify/json.go
index 1b6cdf4d06b..38edca68e5d 100644
--- a/internal/verify/json.go
+++ b/internal/verify/json.go
@@ -168,8 +168,8 @@ func LegacyPolicyNormalize(policy interface{}) (string, error) {
 	return n, nil
 }
 
-// PolicyToSet returns the existing policy if the new policy is equivalent.
-// Otherwise, it returns the new policy. Either policy is normalized.
+// LegacyPolicyToSet returns the existing policy if the new policy is equivalent.
+// Otherwise, it returns the new policy. Either policy is legacy normalized.
 func LegacyPolicyToSet(exist, new string) (string, error) {
 	policyToSet, err := SecondJSONUnlessEquivalent(exist, new)
 	if err != nil {

From 69f5eede5dd53b133086868f7de78f46d162df51 Mon Sep 17 00:00:00 2001
From: Dirk Avery <dirk.avery@gmail.com>
Date: Wed, 11 Jan 2023 16:00:16 -0500
Subject: [PATCH 4/4] Lint

---
 internal/service/iam/policy_test.go | 4 ++--
 internal/service/iam/role_test.go   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/internal/service/iam/policy_test.go b/internal/service/iam/policy_test.go
index a3f97c560fd..7122ad9dd30 100644
--- a/internal/service/iam/policy_test.go
+++ b/internal/service/iam/policy_test.go
@@ -543,10 +543,10 @@ resource "aws_iam_policy" "test" {
   name = %[1]q
 
   policy = jsonencode({
-    Id = %[1]q
+    Id      = %[1]q
     Version = "2012-10-17"
     Statement = [{
-      Sid = "60c9d11f"
+      Sid    = "60c9d11f"
       Effect = "Allow"
       Action = [
         "kms:Describe*",
diff --git a/internal/service/iam/role_test.go b/internal/service/iam/role_test.go
index 804bf243288..aad05bad305 100644
--- a/internal/service/iam/role_test.go
+++ b/internal/service/iam/role_test.go
@@ -1307,7 +1307,7 @@ resource "aws_iam_role" "test" {
   path = "/"
 
   assume_role_policy = jsonencode({
-    Id = %[1]q
+    Id      = %[1]q
     Version = "2012-10-17"
     Statement = [{
       Action = "sts:AssumeRole"