kms/key: Preserve order in JSON policy lists

.changelog/21969.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_kms_key: Fix order-related diffs in `policy`
+```

internal/service/kms/key.go

@@ -199,7 +199,14 @@ func resourceKeyRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("key_id", key.metadata.KeyId)
 	d.Set("key_usage", key.metadata.KeyUsage)
 	d.Set("multi_region", key.metadata.MultiRegion)
-	d.Set("policy", key.policy)
+
+	policyToSet, err := verify.NewPolicyUnlessEquivalent(d.Get("policy").(string), key.policy)
+
+	if err != nil {
+		return fmt.Errorf("while setting policy (%s), encountered: %w", key.policy, err)
+	}
+
+	d.Set("policy", policyToSet)
 
 	tags := key.tags.IgnoreAWS().IgnoreConfig(ignoreTagsConfig)
 

internal/service/kms/key_test.go

@@ -121,11 +121,11 @@ func TestAccKMSKey_asymmetricKey(t *testing.T) {
 	})
 }
 
-func TestAccKMSKey_policy(t *testing.T) {
+func TestAccKMSKey_Policy_basic(t *testing.T) {
 	var key kms.KeyMetadata
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	resourceName := "aws_kms_key.test"
-	expectedPolicyText := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}`
+	expectedPolicyText := fmt.Sprintf(`{"Version":"2012-10-17","Id":%[1]q,"Statement":[{"Sid":"Enable IAM User Permissions","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}`, rName)
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { acctest.PreCheck(t) },
@@ -156,7 +156,7 @@ func TestAccKMSKey_policy(t *testing.T) {
 	})
 }
 
-func TestAccKMSKey_policyBypass(t *testing.T) {
+func TestAccKMSKey_Policy_bypass(t *testing.T) {
 	var key kms.KeyMetadata
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	resourceName := "aws_kms_key.test"
@@ -188,7 +188,7 @@ func TestAccKMSKey_policyBypass(t *testing.T) {
 	})
 }
 
-func TestAccKMSKey_policyBypassUpdate(t *testing.T) {
+func TestAccKMSKey_Policy_bypassUpdate(t *testing.T) {
 	var before, after kms.KeyMetadata
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	resourceName := "aws_kms_key.test"
@@ -244,6 +244,49 @@ func TestAccKMSKey_Policy_iamRole(t *testing.T) {
 	})
 }
 
+// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/11801
+func TestAccKMSKey_Policy_iamRoleOrder(t *testing.T) {
+	var key kms.KeyMetadata
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_kms_key.test"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { acctest.PreCheck(t) },
+		ErrorCheck:   acctest.ErrorCheck(t, kms.EndpointsID),
+		Providers:    acctest.Providers,
+		CheckDestroy: testAccCheckKeyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccKeyPolicyIAMMultiRoleConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckKeyExists(resourceName, &key),
+				),
+			},
+			{
+				Config: testAccKeyPolicyIAMMultiRoleConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckKeyExists(resourceName, &key),
+				),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccKeyPolicyIAMMultiRoleConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckKeyExists(resourceName, &key),
+				),
+				PlanOnly: true,
+			},
+			{
+				Config: testAccKeyPolicyIAMMultiRoleConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckKeyExists(resourceName, &key),
+				),
+				PlanOnly: true,
+			},
+		},
+	})
+}
+
 // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/7646
 func TestAccKMSKey_Policy_iamServiceLinkedRole(t *testing.T) {
 	var key kms.KeyMetadata
@@ -495,23 +538,19 @@ resource "aws_kms_key" "test" {
   description             = %[1]q
   deletion_window_in_days = 7
 
-  policy = <<POLICY
-{
-  "Version": "2012-10-17",
-  "Id": "kms-tf-1",
-  "Statement": [
-    {
-      "Sid": "Enable IAM User Permissions",
-      "Effect": "Allow",
-      "Principal": {
-        "AWS": "*"
-      },
-      "Action": "kms:*",
-      "Resource": "*"
-    }
-  ]
-}
-POLICY
+  policy = jsonencode({
+    Id = %[1]q
+    Statement = [{
+      Sid    = "Enable IAM User Permissions"
+      Effect = "Allow"
+      Principal = {
+        AWS = "*"
+      }
+      Action   = "kms:*"
+      Resource = "*"
+    }]
+    Version = "2012-10-17"
+  })
 }
 `, rName)
 }
@@ -526,32 +565,30 @@ resource "aws_kms_key" "test" {
 
   bypass_policy_lockout_safety_check = %[2]t
 
-  policy = <<-POLICY
-    {
-      "Version": "2012-10-17",
-      "Id": "kms-tf-1",
-      "Statement": [
-        {
-          "Sid": "Enable IAM User Permissions",
-          "Effect": "Allow",
-          "Principal": {
-            "AWS": "${data.aws_caller_identity.current.arn}"
-          },
-          "Action": [
-            "kms:CreateKey",
-            "kms:DescribeKey",
-            "kms:ScheduleKeyDeletion",
-            "kms:Describe*",
-            "kms:Get*",
-            "kms:List*",
-            "kms:TagResource",
-            "kms:UntagResource"
-          ],
-          "Resource": "*"
+  policy = jsonencode({
+    Id = %[1]q
+    Statement = [
+      {
+        Action = [
+          "kms:CreateKey",
+          "kms:DescribeKey",
+          "kms:ScheduleKeyDeletion",
+          "kms:Describe*",
+          "kms:Get*",
+          "kms:List*",
+          "kms:TagResource",
+          "kms:UntagResource",
+        ]
+        Effect = "Allow"
+        Principal = {
+          AWS = data.aws_caller_identity.current.arn
         }
-      ]
-    }
-  POLICY
+        Resource = "*"
+        Sid      = "Enable IAM User Permissions"
+      },
+    ]
+    Version = "2012-10-17"
+  })
 }
 `, rName, bypassFlag)
 }
@@ -580,7 +617,7 @@ resource "aws_kms_key" "test" {
   deletion_window_in_days = 7
 
   policy = jsonencode({
-    Id = "kms-tf-1"
+    Id = %[1]q
     Statement = [
       {
         Action = "kms:*"
@@ -602,7 +639,7 @@ resource "aws_kms_key" "test" {
         ]
         Effect = "Allow"
         Principal = {
-          AWS = [aws_iam_role.test.arn]
+          AWS = aws_iam_role.test.arn
         }
 
         Resource = "*"
@@ -615,6 +652,131 @@ resource "aws_kms_key" "test" {
 `, rName)
 }
 
+func testAccKeyPolicyIAMMultiRoleConfig(rName string) string {
+	return fmt.Sprintf(`
+data "aws_partition" "current" {}
+
+resource "aws_iam_role" "test1" {
+  name = "%[1]s-sultan"
+
+  assume_role_policy = jsonencode({
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Effect = "Allow"
+      Principal = {
+        Service = "ec2.${data.aws_partition.current.dns_suffix}"
+      }
+    }]
+    Version = "2012-10-17"
+  })
+}
+
+resource "aws_iam_role" "test2" {
+  name = "%[1]s-shepard"
+
+  assume_role_policy = jsonencode({
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Effect = "Allow"
+      Principal = {
+        Service = "ec2.${data.aws_partition.current.dns_suffix}"
+      }
+    }]
+    Version = "2012-10-17"
+  })
+}
+
+resource "aws_iam_role" "test3" {
+  name = "%[1]s-tritonal"
+
+  assume_role_policy = jsonencode({
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Effect = "Allow"
+      Principal = {
+        Service = "ec2.${data.aws_partition.current.dns_suffix}"
+      }
+    }]
+    Version = "2012-10-17"
+  })
+}
+
+resource "aws_iam_role" "test4" {
+  name = "%[1]s-artlec"
+
+  assume_role_policy = jsonencode({
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Effect = "Allow"
+      Principal = {
+        Service = "ec2.${data.aws_partition.current.dns_suffix}"
+      }
+    }]
+    Version = "2012-10-17"
+  })
+}
+
+resource "aws_iam_role" "test5" {
+  name = "%[1]s-cazzette"
+
+  assume_role_policy = jsonencode({
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Effect = "Allow"
+      Principal = {
+        Service = "ec2.${data.aws_partition.current.dns_suffix}"
+      }
+    }]
+    Version = "2012-10-17"
+  })
+}
+
+data "aws_iam_policy_document" "test" {
+  policy_id = %[1]q
+  statement {
+    actions = [
+      "kms:*",
+    ]
+    effect = "Allow"
+    principals {
+      identifiers = ["*"]
+      type        = "AWS"
+    }
+    resources = ["*"]
+  }
+
+  statement {
+    actions = [
+      "kms:Encrypt",
+      "kms:Decrypt",
+      "kms:ReEncrypt*",
+      "kms:GenerateDataKey*",
+      "kms:DescribeKey",
+    ]
+    effect = "Allow"
+    principals {
+      identifiers = [
+        aws_iam_role.test2.arn,
+        aws_iam_role.test1.arn,
+        aws_iam_role.test4.arn,
+        aws_iam_role.test3.arn,
+        aws_iam_role.test5.arn,
+      ]
+      type = "AWS"
+    }
+    resources = ["*"]
+  }
+}
+
+resource "aws_kms_key" "test" {
+  description             = %[1]q
+  deletion_window_in_days = 7
+
+  policy = data.aws_iam_policy_document.test.json
+}
+`, rName)
+}
+
 func testAccKeyPolicyIAMServiceLinkedRoleConfig(rName string) string {
 	return fmt.Sprintf(`
 data "aws_partition" "current" {}
@@ -629,7 +791,7 @@ resource "aws_kms_key" "test" {
   deletion_window_in_days = 7
 
   policy = jsonencode({
-    Id = "kms-tf-1"
+    Id = %[1]q
     Statement = [
       {
         Action = "kms:*"
@@ -651,7 +813,7 @@ resource "aws_kms_key" "test" {
         ]
         Effect = "Allow"
         Principal = {
-          AWS = [aws_iam_service_linked_role.test.arn]
+          AWS = aws_iam_service_linked_role.test.arn
         }
 
         Resource = "*"

internal/verify/policies.go

@@ -0,0 +1,25 @@
+package verify
+
+import (
+	awspolicy "github.com/jen20/awspolicyequivalence"
+)
+
+func NewPolicyUnlessEquivalent(old, new string) (string, error) {
+	// valid empty JSON is "{}" not "" so handle special case to avoid
+	// Error unmarshaling policy: unexpected end of JSON input
+	if old == "" || new == "" {
+		return new, nil
+	}
+
+	equivalent, err := awspolicy.PoliciesAreEquivalent(old, new)
+
+	if err != nil {
+		return "", err
+	}
+
+	if equivalent {
+		return old, nil
+	}
+
+	return new, nil
+}