New resource: aws_iot_policy_attachment

[spirius]

Added aws_iot_policy_attachment resource.

Test results:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSIoTPolicyAttachment_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSIoTPolicyAttachment_basic -timeout 120m
=== RUN   TestAccAWSIoTPolicyAttachment_basic
--- PASS: TestAccAWSIoTPolicyAttachment_basic (23.61s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	23.650s

[bflad]

Hi @spirius 👋 Thanks for submitting this! Its off to a good start. Can you see the below initial feedback and let us know if you have any questions or do not have time to implement them?

[bflad]

This appears it should be using listIotPolicyAttachmentPages to properly paginate. 👍

[bflad]

The Exists acceptance test function should only concern itself with the singular target <-> policy attachment the resource manages.

[bflad]

This testAccCheckAWSIotPolicyAttchmentDestroy_basic function should be renamed testAccCheckAWSIotPolicyAttchmentDestroy and perform a check that each resource that defined attachments have been removed as expected:

func testAccCheckAWSIotPolicyAttchmentDestroy(s *terraform.State) error {
  conn := testAccProvider.Meta().(*AWSClient).iotconn

  for _, rs := range s.RootModule().Resources {
    if rs.Type != "aws_iot_policy_attachment" {
      continue
    }

    input := &iot.ListAttachedPoliciesInput{
      PageSize:  aws.Int64(250),
      Recursive: aws.Bool(false),
      Target:    aws.String(rs.Primary.Attributes["target"]),
    }

    var policy *iot.Policy
    err := listIotPolicyAttachmentPages(conn, input, /* ... */)

    if err != nil {
      return err
    }

    if policy == nil {
      continue
    }

    return fmt.Errorf("IOT Policy Attachment (%s) still exists", d.Id())
  }

  return nil
}

[bflad]

It seems it might make sense to create a wrapper function that encapsulates this logic since it will be needed multiple times, e.g.

func getIotPolicyAttachment(conn, target, policyName) (*iot.Policy, error) {
  var policy *iot.Policy

  input := &iot.ListAttachedPoliciesInput{
    PageSize:  aws.Int64(250),
    Recursive: aws.Bool(false),
    Target:    aws.String(target),
  }

  err := listIotPolicyAttachmentPages(conn, input, func(out *iot.ListAttachedPoliciesOutput, lastPage bool) bool {
    for _, att := range out.Policies {
      if policyName == aws.StringValue(att.PolicyName) {
        policy = att
        return false
      }
    }
    return true
  })

  return policy, err
}

[bflad]

Nitpick: instead of logging the error message, it would be more helpful to return it so operators see it without enabling logging:

if err != nil {
  return fmt.Errorf("error attaching policy %s to target %s: %s", policyName, target, err)
}

[bflad]

Nitpick: Same here regarding error messages:

if err != nil {
  return fmt.Errorf("error listing policy attachments for target %s: %s", target, err)
}

[bflad]

Nitpick: Same here regarding error messages:

if err != nil {
  return fmt.Errorf("error detaching policy %s from target %s: %s", policyName, target, err)
}

[spirius]

hi @bflad, thanks for the review, I will update the PR and let you know

[spirius]

hi @bflad, sorry for delay, I've made appropriate changes, can you please review. Similar changes are done for #5868 as well.

[bflad]

Let's get this in, thanks @spirius! 🚀

--- PASS: TestAccAWSIotPolicyAttachment_basic (14.58s)

[bflad]

This has been released in version 1.42.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!