resource/aws_iam_role_policy_attachment: Prevent NoSuchEntity errors from race conditions

aws/resource_aws_iam_role_policy_attachment.go

@@ -6,7 +6,6 @@ import (
 	"strings"
 
 	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/iam"
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
@@ -54,44 +53,29 @@ func resourceAwsIamRolePolicyAttachmentCreate(d *schema.ResourceData, meta inter
 func resourceAwsIamRolePolicyAttachmentRead(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).iamconn
 	role := d.Get("role").(string)
-	arn := d.Get("policy_arn").(string)
-
-	_, err := conn.GetRole(&iam.GetRoleInput{
-		RoleName: aws.String(role),
-	})
+	policyARN := d.Get("policy_arn").(string)
 
-	if err != nil {
-		if awsErr, ok := err.(awserr.Error); ok {
-			if awsErr.Code() == "NoSuchEntity" {
-				log.Printf("[WARN] No such entity found for Policy Attachment (%s)", role)
-				d.SetId("")
-				return nil
-			}
-		}
-		return err
-	}
+	hasPolicyAttachment, err := iamRoleHasPolicyARNAttachment(conn, role, policyARN)
 
-	args := iam.ListAttachedRolePoliciesInput{
-		RoleName: aws.String(role),
+	if isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") {
+		log.Printf("[WARN] IAM Role (%s) not found, removing from state", role)
+		d.SetId("")
+		return nil
 	}
-	var policy string
-	err = conn.ListAttachedRolePoliciesPages(&args, func(page *iam.ListAttachedRolePoliciesOutput, lastPage bool) bool {
-		for _, p := range page.AttachedPolicies {
-			if *p.PolicyArn == arn {
-				policy = *p.PolicyArn
-			}
-		}
 
-		return policy == ""
-	})
 	if err != nil {
-		return err
+		return fmt.Errorf("error finding IAM Role (%s) Policy Attachment (%s): %s", role, policyARN, err)
 	}
-	if policy == "" {
-		log.Printf("[WARN] No such policy found for Role Policy Attachment (%s)", role)
+
+	if !hasPolicyAttachment {
+		log.Printf("[WARN] IAM Role (%s) Policy Attachment (%s) not found, removing from state", role, policyARN)
 		d.SetId("")
+		return nil
 	}
 
+	d.Set("policy_arn", policyARN)
+	d.Set("role", role)
+
 	return nil
 }
 
@@ -101,6 +85,11 @@ func resourceAwsIamRolePolicyAttachmentDelete(d *schema.ResourceData, meta inter
 	arn := d.Get("policy_arn").(string)
 
 	err := detachPolicyFromRole(conn, role, arn)
+
+	if isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") {
+		return nil
+	}
+
 	if err != nil {
 		return fmt.Errorf("Error removing policy %s from IAM Role %s: %v", arn, role, err)
 	}
@@ -138,3 +127,23 @@ func detachPolicyFromRole(conn *iam.IAM, role string, arn string) error {
 	})
 	return err
 }
+
+func iamRoleHasPolicyARNAttachment(conn *iam.IAM, role string, policyARN string) (bool, error) {
+	hasPolicyAttachment := false
+	input := &iam.ListAttachedRolePoliciesInput{
+		RoleName: aws.String(role),
+	}
+
+	err := conn.ListAttachedRolePoliciesPages(input, func(page *iam.ListAttachedRolePoliciesOutput, lastPage bool) bool {
+		for _, p := range page.AttachedPolicies {
+			if aws.StringValue(p.PolicyArn) == policyARN {
+				hasPolicyAttachment = true
+				return false
+			}
+		}
+
+		return !lastPage
+	})
+
+	return hasPolicyAttachment, err
+}

aws/resource_aws_iam_role_policy_attachment_test.go

@@ -62,7 +62,84 @@ func TestAccAWSRolePolicyAttachment_basic(t *testing.T) {
 		},
 	})
 }
+
+func TestAccAWSRolePolicyAttachment_disappears(t *testing.T) {
+	var attachedRolePolicies iam.ListAttachedRolePoliciesOutput
+
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+	resourceName := "aws_iam_role_policy_attachment.test"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSRolePolicyAttachmentDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSIAMRolePolicyAttachmentConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSRolePolicyAttachmentExists(resourceName, 1, &attachedRolePolicies),
+					testAccCheckAWSIAMRolePolicyAttachmentDisappears(resourceName),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+		},
+	})
+}
+
+func TestAccAWSRolePolicyAttachment_disappears_Role(t *testing.T) {
+	var attachedRolePolicies iam.ListAttachedRolePoliciesOutput
+	var role iam.GetRoleOutput
+
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+	iamRoleResourceName := "aws_iam_role.test"
+	resourceName := "aws_iam_role_policy_attachment.test"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSRolePolicyAttachmentDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSIAMRolePolicyAttachmentConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSRoleExists(iamRoleResourceName, &role),
+					testAccCheckAWSRolePolicyAttachmentExists(resourceName, 1, &attachedRolePolicies),
+					// DeleteConflict: Cannot delete entity, must detach all policies first.
+					testAccCheckAWSIAMRolePolicyAttachmentDisappears(resourceName),
+					testAccCheckAWSRoleDisappears(&role),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+		},
+	})
+}
+
 func testAccCheckAWSRolePolicyAttachmentDestroy(s *terraform.State) error {
+	conn := testAccProvider.Meta().(*AWSClient).iamconn
+
+	for _, rs := range s.RootModule().Resources {
+		if rs.Type != "aws_iam_role_policy_attachment" {
+			continue
+		}
+
+		policyARN := rs.Primary.Attributes["policy_arn"]
+		role := rs.Primary.Attributes["role"]
+
+		hasPolicyAttachment, err := iamRoleHasPolicyARNAttachment(conn, role, policyARN)
+
+		if isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") {
+			continue
+		}
+
+		if err != nil {
+			return err
+		}
+
+		if hasPolicyAttachment {
+			return fmt.Errorf("IAM Role (%s) Policy Attachment (%s) still exists", role, policyARN)
+		}
+	}
+
 	return nil
 }
 
@@ -114,6 +191,23 @@ func testAccCheckAWSRolePolicyAttachmentAttributes(policies []string, out *iam.L
 	}
 }
 
+func testAccCheckAWSIAMRolePolicyAttachmentDisappears(resourceName string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		conn := testAccProvider.Meta().(*AWSClient).iamconn
+
+		rs, ok := s.RootModule().Resources[resourceName]
+
+		if !ok {
+			return fmt.Errorf("Not found: %s", resourceName)
+		}
+
+		policyARN := rs.Primary.Attributes["policy_arn"]
+		role := rs.Primary.Attributes["role"]
+
+		return detachPolicyFromRole(conn, role, policyARN)
+	}
+}
+
 func testAccAWSIAMRolePolicyAttachmentImportStateIdFunc(resourceName string) resource.ImportStateIdFunc {
 	return func(s *terraform.State) (string, error) {
 		rs, ok := s.RootModule().Resources[resourceName]
@@ -259,3 +353,34 @@ EOF
 			policy_arn = "${aws_iam_policy.policy3.arn}"
 	}`, rInt, rInt, rInt, rInt)
 }
+
+func testAccAWSIAMRolePolicyAttachmentConfig(rName string) string {
+	return fmt.Sprintf(`
+data "aws_partition" "current" {}
+
+resource "aws_iam_role" "test" {
+  name = %[1]q
+
+  assume_role_policy = <<EOF
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Action": "sts:AssumeRole",
+      "Principal": {
+        "Service": "ec2.amazonaws.com"
+      },
+      "Effect": "Allow",
+      "Sid": ""
+    }
+  ]
+}
+EOF
+}
+
+resource "aws_iam_role_policy_attachment" "test" {
+  policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/AdministratorAccess"
+  role       = "${aws_iam_role.test.name}"
+}
+`, rName)
+}