Merge pull request #14193 from DrFaust92/r/ecr_repo_policy

r/ecr_repository_policy - Read after update + validate `policy` + disappears test
hashicorp · Mar 25, 2021 · 70900dd · 70900dd
2 parents f3e15b5 + 5ad49fc
commit 70900dd
Show file tree

Hide file tree

Showing 3 changed files with 139 additions and 105 deletions.
diff --git a/.changelog/14193.txt b/.changelog/14193.txt
@@ -0,0 +1,3 @@
+```release-note:enhancement
+resource/aws_ecr_repository_policy: Add plan time validation for `policy`
+```
diff --git a/aws/resource_aws_ecr_repository_policy.go b/aws/resource_aws_ecr_repository_policy.go
@@ -6,17 +6,17 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/ecr"
 	"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/validation"
 )
 
 func resourceAwsEcrRepositoryPolicy() *schema.Resource {
 	return &schema.Resource{
-		Create: resourceAwsEcrRepositoryPolicyCreate,
+		Create: resourceAwsEcrRepositoryPolicyPut,
 		Read:   resourceAwsEcrRepositoryPolicyRead,
-		Update: resourceAwsEcrRepositoryPolicyUpdate,
+		Update: resourceAwsEcrRepositoryPolicyPut,
 		Delete: resourceAwsEcrRepositoryPolicyDelete,
 		Importer: &schema.ResourceImporter{
 			State: schema.ImportStatePassthrough,
@@ -31,6 +31,7 @@ func resourceAwsEcrRepositoryPolicy() *schema.Resource {
 			"policy": {
 				Type:             schema.TypeString,
 				Required:         true,
+				ValidateFunc:     validation.StringIsJSON,
 				DiffSuppressFunc: suppressEquivalentAwsPolicyDiffs,
 			},
 			"registry_id": {
@@ -41,23 +42,23 @@ func resourceAwsEcrRepositoryPolicy() *schema.Resource {
 	}
 }
 
-func resourceAwsEcrRepositoryPolicyCreate(d *schema.ResourceData, meta interface{}) error {
+func resourceAwsEcrRepositoryPolicyPut(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).ecrconn
 
 	input := ecr.SetRepositoryPolicyInput{
 		RepositoryName: aws.String(d.Get("repository").(string)),
 		PolicyText:     aws.String(d.Get("policy").(string)),
 	}
 
-	log.Printf("[DEBUG] Creating ECR resository policy: %s", input)
+	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(2*time.Minute, func() *resource.RetryError {
 		out, err = conn.SetRepositoryPolicy(&input)
 
-		if isAWSErr(err, "InvalidParameterException", "Invalid repository policy provided") {
+		if isAWSErr(err, ecr.ErrCodeInvalidParameterException, "Invalid repository policy provided") {
 			return resource.RetryableError(err)
 		}
 		if err != nil {
@@ -69,15 +70,12 @@ func resourceAwsEcrRepositoryPolicyCreate(d *schema.ResourceData, meta interface
 		out, err = conn.SetRepositoryPolicy(&input)
 	}
 	if err != nil {
-		return fmt.Errorf("Error creating ECR Repository Policy: %s", err)
+		return fmt.Errorf("error creating ECR Repository Policy: %w", err)
 	}
 
-	repositoryPolicy := *out
+	log.Printf("[DEBUG] ECR repository policy created: %s", aws.StringValue(out.RepositoryName))
 
-	log.Printf("[DEBUG] ECR repository policy created: %s", *repositoryPolicy.RepositoryName)
-
-	d.SetId(aws.StringValue(repositoryPolicy.RepositoryName))
-	d.Set("registry_id", repositoryPolicy.RegistryId)
+	d.SetId(aws.StringValue(out.RepositoryName))
 
 	return resourceAwsEcrRepositoryPolicyRead(d, meta)
 }
@@ -90,70 +88,20 @@ func resourceAwsEcrRepositoryPolicyRead(d *schema.ResourceData, meta interface{}
 		RepositoryName: aws.String(d.Id()),
 	})
 	if err != nil {
-		if ecrerr, ok := err.(awserr.Error); ok {
-			switch ecrerr.Code() {
-			case "RepositoryNotFoundException", "RepositoryPolicyNotFoundException":
-				d.SetId("")
-				return nil
-			default:
-				return err
-			}
+		if isAWSErr(err, ecr.ErrCodeRepositoryNotFoundException, "") ||
+			isAWSErr(err, ecr.ErrCodeRepositoryPolicyNotFoundException, "") {
+			log.Printf("[WARN] ECR Repository Policy %s not found, removing", d.Id())
+			d.SetId("")
+			return nil
 		}
 		return err
 	}
 
-	log.Printf("[DEBUG] Received repository policy %s", out)
-
-	repositoryPolicy := out
-
-	d.SetId(aws.StringValue(repositoryPolicy.RepositoryName))
-	d.Set("repository", repositoryPolicy.RepositoryName)
-	d.Set("registry_id", repositoryPolicy.RegistryId)
-	d.Set("policy", repositoryPolicy.PolicyText)
-
-	return nil
-}
-
-func resourceAwsEcrRepositoryPolicyUpdate(d *schema.ResourceData, meta interface{}) error {
-	conn := meta.(*AWSClient).ecrconn
-
-	if !d.HasChange("policy") {
-		return nil
-	}
-
-	input := ecr.SetRepositoryPolicyInput{
-		RepositoryName: aws.String(d.Get("repository").(string)),
-		RegistryId:     aws.String(d.Get("registry_id").(string)),
-		PolicyText:     aws.String(d.Get("policy").(string)),
-	}
-
-	log.Printf("[DEBUG] Updating ECR resository policy: %s", input)
-
-	// Retry due to IAM eventual consistency
-	var err error
-	var out *ecr.SetRepositoryPolicyOutput
-	err = resource.Retry(2*time.Minute, func() *resource.RetryError {
-		out, err = conn.SetRepositoryPolicy(&input)
-
-		if isAWSErr(err, "InvalidParameterException", "Invalid repository policy provided") {
-			return resource.RetryableError(err)
-		}
-		if err != nil {
-			return resource.NonRetryableError(err)
-		}
-		return nil
-	})
-	if isResourceTimeoutError(err) {
-		out, err = conn.SetRepositoryPolicy(&input)
-	}
-	if err != nil {
-		return fmt.Errorf("Error updating ECR Repository Policy: %s", err)
-	}
-
-	repositoryPolicy := *out
+	log.Printf("[DEBUG] Received repository policy %#v", out)
 
-	d.SetId(aws.StringValue(repositoryPolicy.RepositoryName))
-	d.Set("registry_id", repositoryPolicy.RegistryId)
+	d.Set("repository", out.RepositoryName)
+	d.Set("registry_id", out.RegistryId)
+	d.Set("policy", out.PolicyText)
 
 	return nil
 }
@@ -166,13 +114,9 @@ func resourceAwsEcrRepositoryPolicyDelete(d *schema.ResourceData, meta interface
 		RegistryId:     aws.String(d.Get("registry_id").(string)),
 	})
 	if err != nil {
-		if ecrerr, ok := err.(awserr.Error); ok {
-			switch ecrerr.Code() {
-			case "RepositoryNotFoundException", "RepositoryPolicyNotFoundException":
-				return nil
-			default:
-				return err
-			}
+		if isAWSErr(err, ecr.ErrCodeRepositoryNotFoundException, "") ||
+			isAWSErr(err, ecr.ErrCodeRepositoryPolicyNotFoundException, "") {
+			return nil
 		}
 		return err
 	}