kms: Preserve order in JSON/policies

.changelog/21990.txt

@@ -0,0 +1,11 @@
+```release-note:bug
+resource/aws_kms_external_key: Fix order-related diffs in `policy`
+```
+
+```release-note:bug
+resource/aws_kms_replica_external_key: Fix order-related diffs in `policy`
+```
+
+```release-note:bug
+resource/aws_kms_replica_key: Fix order-related diffs in `policy`
+```

internal/service/kms/external_key.go

@@ -226,7 +226,15 @@ func resourceExternalKeyRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("key_state", key.metadata.KeyState)
 	d.Set("key_usage", key.metadata.KeyUsage)
 	d.Set("multi_region", key.metadata.MultiRegion)
-	d.Set("policy", key.policy)
+
+	policyToSet, err := verify.SecondJSONUnlessEquivalent(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)
+
 	if key.metadata.ValidTo != nil {
 		d.Set("valid_to", aws.TimeValue(key.metadata.ValidTo).Format(time.RFC3339))
 	} else {

internal/service/kms/external_key_test.go

@@ -287,8 +287,8 @@ func TestAccKMSExternalKey_keyMaterialBase64(t *testing.T) {
 func TestAccKMSExternalKey_policy(t *testing.T) {
 	var key1, key2 kms.KeyMetadata
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
-	policy1 := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions 1","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}`
-	policy2 := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions 2","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}`
+	policy1 := `{"Id":"kms-tf-1","Statement":[{"Action":"kms:*","Effect":"Allow","Principal":{"AWS":"*"},"Resource":"*","Sid":"Enable IAM User Permissions 1"}],"Version":"2012-10-17"}`
+	policy2 := `{"Id":"kms-tf-1","Statement":[{"Action":"kms:*","Effect":"Allow","Principal":{"AWS":"*"},"Resource":"*","Sid":"Enable IAM User Permissions 2"}],"Version":"2012-10-17"}`
 	resourceName := "aws_kms_external_key.test"
 
 	resource.ParallelTest(t, resource.TestCase{
@@ -329,7 +329,7 @@ func TestAccKMSExternalKey_policy(t *testing.T) {
 func TestAccKMSExternalKey_policyBypass(t *testing.T) {
 	var key kms.KeyMetadata
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
-	policy := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions 1","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}`
+	policy := `{"Id":"kms-tf-1","Statement":[{"Action":"kms:*","Effect":"Allow","Principal":{"AWS":"*"},"Resource":"*","Sid":"Enable IAM User Permissions 1"}],"Version":"2012-10-17"}`
 	resourceName := "aws_kms_external_key.test"
 
 	resource.ParallelTest(t, resource.TestCase{
@@ -642,9 +642,7 @@ resource "aws_kms_external_key" "test" {
   description             = %[1]q
   deletion_window_in_days = 7
 
-  policy = <<POLICY
-%[2]s
-POLICY
+  policy = %[2]q
 }
 `, rName, policy)
 }
@@ -657,9 +655,7 @@ resource "aws_kms_external_key" "test" {
 
   bypass_policy_lockout_safety_check = true
 
-  policy = <<POLICY
-%[2]s
-POLICY
+  policy = %[2]q
 }
 `, rName, policy)
 }

internal/service/kms/replica_external_key.go

@@ -237,7 +237,15 @@ func resourceReplicaExternalKeyRead(d *schema.ResourceData, meta interface{}) er
 	d.Set("key_id", key.metadata.KeyId)
 	d.Set("key_state", key.metadata.KeyState)
 	d.Set("key_usage", key.metadata.KeyUsage)
-	d.Set("policy", key.policy)
+
+	policyToSet, err := verify.SecondJSONUnlessEquivalent(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)
+
 	d.Set("primary_key_arn", key.metadata.MultiRegionConfiguration.PrimaryKey.Arn)
 	if key.metadata.ValidTo != nil {
 		d.Set("valid_to", aws.TimeValue(key.metadata.ValidTo).Format(time.RFC3339))

internal/service/kms/replica_external_key_test.go

@@ -61,7 +61,7 @@ func TestAccKMSReplicaExternalKey_basic(t *testing.T) {
 	})
 }
 
-func TestAccKMSReplicaExternalKey_DescriptionAndEnabled(t *testing.T) {
+func TestAccKMSReplicaExternalKey_descriptionAndEnabled(t *testing.T) {
 	var providers []*schema.Provider
 	var key kms.KeyMetadata
 	rName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
@@ -117,13 +117,13 @@ func TestAccKMSReplicaExternalKey_DescriptionAndEnabled(t *testing.T) {
 	})
 }
 
-func TestAccKMSReplicaExternalKey_Policy(t *testing.T) {
+func TestAccKMSReplicaExternalKey_policy(t *testing.T) {
 	var providers []*schema.Provider
 	var key kms.KeyMetadata
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	resourceName := "aws_kms_replica_external_key.test"
-	policy1 := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions 1","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}`
-	policy2 := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions 2","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}`
+	policy1 := `{"Id":"kms-tf-1","Statement":[{"Action":"kms:*","Effect":"Allow","Principal":{"AWS":"*"},"Resource":"*","Sid":"Enable IAM User Permissions 1"}],"Version":"2012-10-17"}`
+	policy2 := `{"Id":"kms-tf-1","Statement":[{"Action":"kms:*","Effect":"Allow","Principal":{"AWS":"*"},"Resource":"*","Sid":"Enable IAM User Permissions 2"}],"Version":"2012-10-17"}`
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck: func() {
@@ -164,7 +164,7 @@ func TestAccKMSReplicaExternalKey_Policy(t *testing.T) {
 	})
 }
 
-func TestAccKMSReplicaExternalKey_Tags(t *testing.T) {
+func TestAccKMSReplicaExternalKey_tags(t *testing.T) {
 	var providers []*schema.Provider
 	var key kms.KeyMetadata
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
@@ -290,9 +290,7 @@ resource "aws_kms_replica_external_key" "test" {
 
   bypass_policy_lockout_safety_check = %[3]t
 
-  policy = <<POLICY
-%[2]s
-POLICY
+  policy = %[2]q
 }
 `, rName, policy, bypassLockoutCheck))
 }

internal/service/kms/replica_key.go

@@ -209,7 +209,15 @@ func resourceReplicaKeyRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("key_rotation_enabled", key.rotation)
 	d.Set("key_spec", key.metadata.KeySpec)
 	d.Set("key_usage", key.metadata.KeyUsage)
-	d.Set("policy", key.policy)
+
+	policyToSet, err := verify.SecondJSONUnlessEquivalent(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)
+
 	d.Set("primary_key_arn", key.metadata.MultiRegionConfiguration.PrimaryKey.Arn)
 
 	tags := key.tags.IgnoreAWS().IgnoreConfig(ignoreTagsConfig)

internal/service/kms/replica_key_test.go

@@ -82,7 +82,7 @@ func TestAccKMSReplicaKey_disappears(t *testing.T) {
 	})
 }
 
-func TestAccKMSReplicaKey_DescriptionAndEnabled(t *testing.T) {
+func TestAccKMSReplicaKey_descriptionAndEnabled(t *testing.T) {
 	var providers []*schema.Provider
 	var key kms.KeyMetadata
 	rName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
@@ -134,13 +134,13 @@ func TestAccKMSReplicaKey_DescriptionAndEnabled(t *testing.T) {
 	})
 }
 
-func TestAccKMSReplicaKey_Policy(t *testing.T) {
+func TestAccKMSReplicaKey_policy(t *testing.T) {
 	var providers []*schema.Provider
 	var key kms.KeyMetadata
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	resourceName := "aws_kms_replica_key.test"
-	policy1 := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions 1","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}`
-	policy2 := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions 2","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}`
+	policy1 := `{"Id":"kms-tf-1","Statement":[{"Action":"kms:*","Effect":"Allow","Principal":{"AWS":"*"},"Resource":"*","Sid":"Enable IAM User Permissions 1"}],"Version":"2012-10-17"}`
+	policy2 := `{"Id":"kms-tf-1","Statement":[{"Action":"kms:*","Effect":"Allow","Principal":{"AWS":"*"},"Resource":"*","Sid":"Enable IAM User Permissions 2"}],"Version":"2012-10-17"}`
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck: func() {
@@ -177,7 +177,7 @@ func TestAccKMSReplicaKey_Policy(t *testing.T) {
 	})
 }
 
-func TestAccKMSReplicaKey_Tags(t *testing.T) {
+func TestAccKMSReplicaKey_tags(t *testing.T) {
 	var providers []*schema.Provider
 	var key kms.KeyMetadata
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
@@ -227,7 +227,7 @@ func TestAccKMSReplicaKey_Tags(t *testing.T) {
 	})
 }
 
-func TestAccKMSReplicaKey_TwoReplicas(t *testing.T) {
+func TestAccKMSReplicaKey_twoReplicas(t *testing.T) {
 	var providers []*schema.Provider
 	var key kms.KeyMetadata
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
@@ -307,9 +307,7 @@ resource "aws_kms_replica_key" "test" {
 
   bypass_policy_lockout_safety_check = %[3]t
 
-  policy = <<POLICY
-%[2]s
-POLICY
+  policy = %[2]q
 }
 `, rName, policy, bypassLockoutCheck))
 }

internal/verify/json.go

@@ -6,6 +6,7 @@ import (
 	"log"
 	"reflect"
 	"regexp"
+	"strings"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure"
@@ -82,7 +83,11 @@ func JSONBytesEqual(b1, b2 []byte) bool {
 func SecondJSONUnlessEquivalent(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 == "" {
+	if strings.TrimSpace(new) == "" {
+		return "", nil
+	}
+
+	if strings.TrimSpace(old) == "" {
 		return new, nil
 	}
 

internal/verify/json_test.go

@@ -6,19 +6,19 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
-func TestLooksLikeJsonString(t *testing.T) {
-	looksLikeJson := ` {"abc":"1"} `
-	doesNotLookLikeJson := `abc: 1`
+func TestLooksLikeJSONString(t *testing.T) {
+	looksLikeJSON := ` {"abc":"1"} `
+	doesNotLookLikeJSON := `abc: 1`
 
-	if !looksLikeJSONString(looksLikeJson) {
-		t.Errorf("Expected looksLikeJson to return true for %s", looksLikeJson)
+	if !looksLikeJSONString(looksLikeJSON) {
+		t.Errorf("Expected looksLikeJSON to return true for %s", looksLikeJSON)
 	}
-	if looksLikeJSONString(doesNotLookLikeJson) {
-		t.Errorf("Expected looksLikeJson to return false for %s", doesNotLookLikeJson)
+	if looksLikeJSONString(doesNotLookLikeJSON) {
+		t.Errorf("Expected looksLikeJSON to return false for %s", doesNotLookLikeJSON)
 	}
 }
 
-func TestJsonBytesEqualQuotedAndUnquoted(t *testing.T) {
+func TestJSONBytesEqualQuotedAndUnquoted(t *testing.T) {
 	unquoted := `{"test": "test"}`
 	quoted := "{\"test\": \"test\"}"
 
@@ -34,7 +34,7 @@ func TestJsonBytesEqualQuotedAndUnquoted(t *testing.T) {
 	}
 }
 
-func TestJsonBytesEqualWhitespaceAndNoWhitespace(t *testing.T) {
+func TestJSONBytesEqualWhitespaceAndNoWhitespace(t *testing.T) {
 	noWhitespace := `{"test":"test"}`
 	whitespace := `
 {
@@ -362,13 +362,13 @@ func TestNormalizeJSONOrYAMLString(t *testing.T) {
 	var err error
 	var actual string
 
-	validNormalizedJson := `{"abc":"1"}`
-	actual, err = NormalizeJSONOrYAMLString(validNormalizedJson)
+	validNormalizedJSON := `{"abc":"1"}`
+	actual, err = NormalizeJSONOrYAMLString(validNormalizedJSON)
 	if err != nil {
 		t.Fatalf("Expected not to throw an error while parsing template, but got: %s", err)
 	}
-	if actual != validNormalizedJson {
-		t.Fatalf("Got:\n\n%s\n\nExpected:\n\n%s\n", actual, validNormalizedJson)
+	if actual != validNormalizedJSON {
+		t.Fatalf("Got:\n\n%s\n\nExpected:\n\n%s\n", actual, validNormalizedJSON)
 	}
 
 	validNormalizedYaml := `abc: 1