Merge pull request #22136 from hashicorp/b-codeartifact-equiv-policy-…

…diffs

codeartifact/permissions_policy: Fix equivalent policy diffs

.changelog/22136.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+resource/aws_codeartifact_domain_permissions_policy: Fix erroneous diffs in `policy` when no changes made or policies are equivalent
+```
+
+```release-note:bug
+resource/aws_codeartifact_repository_permissions_policy: Fix erroneous diffs in `policy` when no changes made or policies are equivalent
+```

internal/service/codeartifact/codeartifact_test.go

@@ -16,10 +16,11 @@ func TestAccCodeArtifact_serial(t *testing.T) {
 			"tags":                 testAccCodeArtifactDomain_tags,
 		},
 		"DomainPermissionsPolicy": {
-			"basic":             testAccCodeArtifactDomainPermissionsPolicy_basic,
-			"disappears":        testAccCodeArtifactDomainPermissionsPolicy_disappears,
-			"owner":             testAccCodeArtifactDomainPermissionsPolicy_owner,
-			"Disappears_domain": testAccCodeArtifactDomainPermissionsPolicy_Disappears_domain,
+			"basic":            testAccCodeArtifactDomainPermissionsPolicy_basic,
+			"disappears":       testAccCodeArtifactDomainPermissionsPolicy_disappears,
+			"owner":            testAccCodeArtifactDomainPermissionsPolicy_owner,
+			"disappearsDomain": testAccCodeArtifactDomainPermissionsPolicy_Disappears_domain,
+			"ignoreEquivalent": testAccCodeArtifactDomainPermissionsPolicy_ignoreEquivalent,
 		},
 		"Repository": {
 			"basic":              testAccCodeArtifactRepository_basic,
@@ -35,10 +36,11 @@ func TestAccCodeArtifact_serial(t *testing.T) {
 			"owner": testAccCodeArtifactRepositoryEndpointDataSource_owner,
 		},
 		"RepositoryPermissionsPolicy": {
-			"basic":             testAccCodeArtifactRepositoryPermissionsPolicy_basic,
-			"disappears":        testAccCodeArtifactRepositoryPermissionsPolicy_disappears,
-			"owner":             testAccCodeArtifactRepositoryPermissionsPolicy_owner,
-			"Disappears_domain": testAccCodeArtifactRepositoryPermissionsPolicy_Disappears_domain,
+			"basic":            testAccCodeArtifactRepositoryPermissionsPolicy_basic,
+			"disappears":       testAccCodeArtifactRepositoryPermissionsPolicy_disappears,
+			"owner":            testAccCodeArtifactRepositoryPermissionsPolicy_owner,
+			"disappearsDomain": testAccCodeArtifactRepositoryPermissionsPolicy_Disappears_domain,
+			"ignoreEquivalent": testAccCodeArtifactRepositoryPermissionsPolicy_ignoreEquivalent,
 		},
 	}
 

internal/service/codeartifact/domain_permissions_policy.go

@@ -8,6 +8,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/codeartifact"
 	"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/verify"
@@ -40,6 +41,10 @@ func ResourceDomainPermissionsPolicy() *schema.Resource {
 				Required:         true,
 				ValidateFunc:     validation.StringIsJSON,
 				DiffSuppressFunc: verify.SuppressEquivalentPolicyDiffs,
+				StateFunc: func(v interface{}) string {
+					json, _ := structure.NormalizeJsonString(v)
+					return json
+				},
 			},
 			"policy_revision": {
 				Type:     schema.TypeString,
@@ -58,9 +63,15 @@ func resourceDomainPermissionsPolicyPut(d *schema.ResourceData, meta interface{}
 	conn := meta.(*conns.AWSClient).CodeArtifactConn
 	log.Print("[DEBUG] Creating CodeArtifact Domain Permissions Policy")
 
+	policy, err := structure.NormalizeJsonString(d.Get("policy_document").(string))
+
+	if err != nil {
+		return fmt.Errorf("policy (%s) is invalid JSON: %w", policy, err)
+	}
+
 	params := &codeartifact.PutDomainPermissionsPolicyInput{
 		Domain:         aws.String(d.Get("domain").(string)),
-		PolicyDocument: aws.String(d.Get("policy_document").(string)),
+		PolicyDocument: aws.String(policy),
 	}
 
 	if v, ok := d.GetOk("domain_owner"); ok {
@@ -106,9 +117,22 @@ func resourceDomainPermissionsPolicyRead(d *schema.ResourceData, meta interface{
 	d.Set("domain", domainName)
 	d.Set("domain_owner", domainOwner)
 	d.Set("resource_arn", dm.Policy.ResourceArn)
-	d.Set("policy_document", dm.Policy.Document)
 	d.Set("policy_revision", dm.Policy.Revision)
 
+	policyToSet, err := verify.SecondJSONUnlessEquivalent(d.Get("policy_document").(string), aws.StringValue(dm.Policy.Document))
+
+	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_document", policyToSet)
+
 	return nil
 }
 

internal/service/codeartifact/domain_permissions_policy_test.go

@@ -56,6 +56,35 @@ func testAccCodeArtifactDomainPermissionsPolicy_basic(t *testing.T) {
 	})
 }
 
+func testAccCodeArtifactDomainPermissionsPolicy_ignoreEquivalent(t *testing.T) {
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_codeartifact_domain_permissions_policy.test"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { acctest.PreCheck(t); acctest.PreCheckPartitionHasService(codeartifact.EndpointsID, t) },
+		ErrorCheck:   acctest.ErrorCheck(t, codeartifact.EndpointsID),
+		Providers:    acctest.Providers,
+		CheckDestroy: testAccCheckDomainPermissionsDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccDomainPermissionsPolicyOrderConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckDomainPermissionsExists(resourceName),
+					resource.TestCheckResourceAttrPair(resourceName, "resource_arn", "aws_codeartifact_domain.test", "arn"),
+					resource.TestCheckResourceAttr(resourceName, "domain", rName),
+					resource.TestMatchResourceAttr(resourceName, "policy_document", regexp.MustCompile("codeartifact:CreateRepository")),
+					resource.TestMatchResourceAttr(resourceName, "policy_document", regexp.MustCompile("codeartifact:ListRepositoriesInDomain")),
+					resource.TestCheckResourceAttrPair(resourceName, "domain_owner", "aws_codeartifact_domain.test", "owner"),
+				),
+			},
+			{
+				Config:   testAccDomainPermissionsPolicyNewOrderConfig(rName),
+				PlanOnly: true,
+			},
+		},
+	})
+}
+
 func testAccCodeArtifactDomainPermissionsPolicy_owner(t *testing.T) {
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	resourceName := "aws_codeartifact_domain_permissions_policy.test"
@@ -286,3 +315,63 @@ EOF
 }
 `, rName)
 }
+
+func testAccDomainPermissionsPolicyOrderConfig(rName string) string {
+	return fmt.Sprintf(`
+resource "aws_kms_key" "test" {
+  description             = %[1]q
+  deletion_window_in_days = 7
+}
+
+resource "aws_codeartifact_domain" "test" {
+  domain         = %[1]q
+  encryption_key = aws_kms_key.test.arn
+}
+
+resource "aws_codeartifact_domain_permissions_policy" "test" {
+  domain = aws_codeartifact_domain.test.domain
+  policy_document = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Action = [
+        "codeartifact:CreateRepository",
+        "codeartifact:ListRepositoriesInDomain",
+      ]
+      Effect    = "Allow"
+      Principal = "*"
+      Resource  = aws_codeartifact_domain.test.arn
+    }]
+  })
+}
+`, rName)
+}
+
+func testAccDomainPermissionsPolicyNewOrderConfig(rName string) string {
+	return fmt.Sprintf(`
+resource "aws_kms_key" "test" {
+  description             = %[1]q
+  deletion_window_in_days = 7
+}
+
+resource "aws_codeartifact_domain" "test" {
+  domain         = %[1]q
+  encryption_key = aws_kms_key.test.arn
+}
+
+resource "aws_codeartifact_domain_permissions_policy" "test" {
+  domain = aws_codeartifact_domain.test.domain
+  policy_document = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Action = [
+        "codeartifact:ListRepositoriesInDomain",
+        "codeartifact:CreateRepository",
+      ]
+      Effect    = "Allow"
+      Principal = "*"
+      Resource  = aws_codeartifact_domain.test.arn
+    }]
+  })
+}
+`, rName)
+}

internal/service/codeartifact/repository_permissions_policy.go

@@ -8,6 +8,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/codeartifact"
 	"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/verify"
@@ -45,6 +46,10 @@ func ResourceRepositoryPermissionsPolicy() *schema.Resource {
 				Required:         true,
 				ValidateFunc:     validation.StringIsJSON,
 				DiffSuppressFunc: verify.SuppressEquivalentPolicyDiffs,
+				StateFunc: func(v interface{}) string {
+					json, _ := structure.NormalizeJsonString(v)
+					return json
+				},
 			},
 			"policy_revision": {
 				Type:     schema.TypeString,
@@ -63,10 +68,16 @@ func resourceRepositoryPermissionsPolicyPut(d *schema.ResourceData, meta interfa
 	conn := meta.(*conns.AWSClient).CodeArtifactConn
 	log.Print("[DEBUG] Creating CodeArtifact Repository Permissions Policy")
 
+	policy, err := structure.NormalizeJsonString(d.Get("policy_document").(string))
+
+	if err != nil {
+		return fmt.Errorf("policy (%s) is invalid JSON: %w", policy, err)
+	}
+
 	params := &codeartifact.PutRepositoryPermissionsPolicyInput{
 		Domain:         aws.String(d.Get("domain").(string)),
 		Repository:     aws.String(d.Get("repository").(string)),
-		PolicyDocument: aws.String(d.Get("policy_document").(string)),
+		PolicyDocument: aws.String(policy),
 	}
 
 	if v, ok := d.GetOk("domain_owner"); ok {
@@ -114,9 +125,22 @@ func resourceRepositoryPermissionsPolicyRead(d *schema.ResourceData, meta interf
 	d.Set("domain_owner", domainOwner)
 	d.Set("repository", repoName)
 	d.Set("resource_arn", dm.Policy.ResourceArn)
-	d.Set("policy_document", dm.Policy.Document)
 	d.Set("policy_revision", dm.Policy.Revision)
 
+	policyToSet, err := verify.SecondJSONUnlessEquivalent(d.Get("policy_document").(string), aws.StringValue(dm.Policy.Document))
+
+	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_document", policyToSet)
+
 	return nil
 }
 

internal/service/codeartifact/repository_permissions_policy_test.go

@@ -56,6 +56,34 @@ func testAccCodeArtifactRepositoryPermissionsPolicy_basic(t *testing.T) {
 	})
 }
 
+func testAccCodeArtifactRepositoryPermissionsPolicy_ignoreEquivalent(t *testing.T) {
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_codeartifact_repository_permissions_policy.test"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { acctest.PreCheck(t); acctest.PreCheckPartitionHasService(codeartifact.EndpointsID, t) },
+		ErrorCheck:   acctest.ErrorCheck(t, codeartifact.EndpointsID),
+		Providers:    acctest.Providers,
+		CheckDestroy: testAccCheckRepositoryPermissionsDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccRepositoryPermissionsPolicyOrderConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRepositoryPermissionsExists(resourceName),
+					resource.TestCheckResourceAttrPair(resourceName, "resource_arn", "aws_codeartifact_repository.test", "arn"),
+					resource.TestCheckResourceAttr(resourceName, "domain", rName),
+					resource.TestMatchResourceAttr(resourceName, "policy_document", regexp.MustCompile("codeartifact:CreateRepository")),
+					resource.TestCheckResourceAttrPair(resourceName, "domain_owner", "aws_codeartifact_domain.test", "owner"),
+				),
+			},
+			{
+				Config:   testAccRepositoryPermissionsPolicyNewOrderConfig(rName),
+				PlanOnly: true,
+			},
+		},
+	})
+}
+
 func testAccCodeArtifactRepositoryPermissionsPolicy_owner(t *testing.T) {
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	resourceName := "aws_codeartifact_repository_permissions_policy.test"
@@ -306,3 +334,75 @@ EOF
 }
 `, rName)
 }
+
+func testAccRepositoryPermissionsPolicyOrderConfig(rName string) string {
+	return fmt.Sprintf(`
+resource "aws_kms_key" "test" {
+  description             = %[1]q
+  deletion_window_in_days = 7
+}
+
+resource "aws_codeartifact_domain" "test" {
+  domain         = %[1]q
+  encryption_key = aws_kms_key.test.arn
+}
+
+resource "aws_codeartifact_repository" "test" {
+  repository = %[1]q
+  domain     = aws_codeartifact_domain.test.domain
+}
+
+resource "aws_codeartifact_repository_permissions_policy" "test" {
+  domain     = aws_codeartifact_domain.test.domain
+  repository = aws_codeartifact_repository.test.repository
+  policy_document = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Action = [
+        "codeartifact:CreateRepository",
+        "codeartifact:ListRepositoriesInDomain",
+      ]
+      Effect    = "Allow"
+      Principal = "*"
+      Resource  = aws_codeartifact_domain.test.arn
+    }]
+  })
+}
+`, rName)
+}
+
+func testAccRepositoryPermissionsPolicyNewOrderConfig(rName string) string {
+	return fmt.Sprintf(`
+resource "aws_kms_key" "test" {
+  description             = %[1]q
+  deletion_window_in_days = 7
+}
+
+resource "aws_codeartifact_domain" "test" {
+  domain         = %[1]q
+  encryption_key = aws_kms_key.test.arn
+}
+
+resource "aws_codeartifact_repository" "test" {
+  repository = %[1]q
+  domain     = aws_codeartifact_domain.test.domain
+}
+
+resource "aws_codeartifact_repository_permissions_policy" "test" {
+  domain     = aws_codeartifact_domain.test.domain
+  repository = aws_codeartifact_repository.test.repository
+  policy_document = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Action = [
+        "codeartifact:ListRepositoriesInDomain",
+        "codeartifact:CreateRepository",
+      ]
+      Effect    = "Allow"
+      Principal = "*"
+      Resource  = aws_codeartifact_domain.test.arn
+    }]
+  })
+}
+`, rName)
+}