Merge pull request #22004 from hashicorp/b-ecr-preserve-json-policy-o…

…rder ecr: Preserve/ignore order in JSON/policy
hashicorp · Dec 2, 2021 · 3dbc25e · 3dbc25e
2 parents 9eddee4 + c4dd64b
commit 3dbc25e
Show file tree

Hide file tree

Showing 8 changed files with 351 additions and 75 deletions.
diff --git a/.changelog/22004.txt b/.changelog/22004.txt
@@ -0,0 +1,11 @@
+```release-note:bug
+resource/aws_ecr_repository_policy: Fix order-related diffs in `policy`
+```
+
+```release-note:bug
+resource/aws_ecr_registry_policy: Fix order-related diffs in `policy`
+```
+
+```release-note:bug
+resource/aws_iam_role: Prevent `arn` attribute from ever containing a unique ID immediately after role creation
+```
diff --git a/internal/service/ecr/errorcheck_test.go b/internal/service/ecr/errorcheck_test.go
@@ -0,0 +1,20 @@
+package ecr_test
+
+import (
+	"testing"
+
+	"github.com/aws/aws-sdk-go/service/ecr"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/hashicorp/terraform-provider-aws/internal/acctest"
+)
+
+func init() {
+	acctest.RegisterServiceErrorCheckFunc(ecr.EndpointsID, testAccErrorCheckSkip)
+}
+
+// testAccErrorCheckSkip skips tests that have error messages indicating unsupported features
+func testAccErrorCheckSkip(t *testing.T) resource.ErrorCheckFunc {
+	return acctest.ErrorCheckSkipMessagesContaining(t,
+		"This feature is disabled",
+	)
+}
diff --git a/internal/service/ecr/registry_policy.go b/internal/service/ecr/registry_policy.go
@@ -8,6 +8,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/ecr"
 	"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"
@@ -41,8 +42,14 @@ func ResourceRegistryPolicy() *schema.Resource {
 func resourceRegistryPolicyPut(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*conns.AWSClient).ECRConn
 
+	policy, err := structure.NormalizeJsonString(d.Get("policy").(string))
+
+	if err != nil {
+		return fmt.Errorf("policy (%s) is invalid JSON: %w", policy, err)
+	}
+
 	input := ecr.PutRegistryPolicyInput{
-		PolicyText: aws.String(d.Get("policy").(string)),
+		PolicyText: aws.String(policy),
 	}
 
 	out, err := conn.PutRegistryPolicy(&input)
@@ -72,7 +79,20 @@ func resourceRegistryPolicyRead(d *schema.ResourceData, meta interface{}) error
 	}
 
 	d.Set("registry_id", out.RegistryId)
-	d.Set("policy", out.PolicyText)
+
+	policyToSet, err := verify.SecondJSONUnlessEquivalent(d.Get("policy").(string), aws.StringValue(out.PolicyText))
+
+	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 an invalid JSON: %w", policyToSet, err)
+	}
+
+	d.Set("policy", policyToSet)
 
 	return nil
 }

diff --git a/internal/service/ecr/registry_policy_test.go b/internal/service/ecr/registry_policy_test.go
@@ -152,12 +152,8 @@ resource "aws_ecr_registry_policy" "test" {
         "Principal" : {
           "AWS" : "arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:root"
         },
-        "Action" : [
-          "ecr:ReplicateImage"
-        ],
-        "Resource" : [
-          "arn:${data.aws_partition.current.partition}:ecr:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:repository/*"
-        ]
+        "Action" : "ecr:ReplicateImage",
+        "Resource" : "arn:${data.aws_partition.current.partition}:ecr:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:repository/*",
       }
     ]
   })

diff --git a/internal/service/ecr/repository_policy.go b/internal/service/ecr/repository_policy.go
@@ -9,6 +9,7 @@ import (
 	"github.com/hashicorp/aws-sdk-go-base/tfawserr"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"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"
 	tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam"
@@ -49,15 +50,20 @@ func ResourceRepositoryPolicy() *schema.Resource {
 func resourceRepositoryPolicyPut(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*conns.AWSClient).ECRConn
 
+	policy, err := structure.NormalizeJsonString(d.Get("policy").(string))
+
+	if err != nil {
+		return fmt.Errorf("policy (%s) is invalid JSON: %w", policy, err)
+	}
+
 	input := ecr.SetRepositoryPolicyInput{
 		RepositoryName: aws.String(d.Get("repository").(string)),
-		PolicyText:     aws.String(d.Get("policy").(string)),
+		PolicyText:     aws.String(policy),
 	}
 
 	log.Printf("[DEBUG] Creating ECR repository policy: %#v", input)
 
 	// Retry due to IAM eventual consistency
-	var err error
 	var out *ecr.SetRepositoryPolicyOutput
 	err = resource.Retry(tfiam.PropagationTimeout, func() *resource.RetryError {
 		out, err = conn.SetRepositoryPolicy(&input)
@@ -141,7 +147,20 @@ func resourceRepositoryPolicyRead(d *schema.ResourceData, meta interface{}) erro
 
 	d.Set("repository", out.RepositoryName)
 	d.Set("registry_id", out.RegistryId)
-	d.Set("policy", out.PolicyText)
+
+	policyToSet, err := verify.SecondJSONUnlessEquivalent(d.Get("policy").(string), aws.StringValue(out.PolicyText))
+
+	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
 }