Use safer aws_iam_role_policy_attachment

[nitrocode]

Drop aws_iam_policy_attachment due to the resource warning

WARNING: The aws_iam_policy_attachment resource creates exclusive attachments of IAM policies. Across the entire AWS account, all of the users/roles/groups to which a single policy is attached must be declared by a single aws_iam_policy_attachment resource. This means that even any users/roles/groups that have the attached policy via any other mechanism (including other Terraform resources) will have that attached policy revoked by this resource. Consider aws_iam_role_policy_attachment, aws_iam_user_policy_attachment, or aws_iam_group_policy_attachment instead. These resources do not enforce exclusive attachment of an IAM policy.

Use instead the aws_iam_role_policy_attachment

[raymondbutcher]

Using this resource is not a problem if you create a policy and use it strictly in one place. This module does that and it works fine.

However, if you then use this module's policies outside of the module (which was never intended, as it's not an output of the module) then you'll run into the issues that the warning describes. That seems to be the case here (after chatting on Slack).

The way it works now is fine and working as intended, but I agree that it would be better if it were using aws_iam_role_policy_attachment, because it is safer.

However, I'm not sure how this change would be applied by Terraform and AWS. These 2 potential scenarios concern me:

• Terraform deletes the aws_iam_policy_attachment resource which detaches the policy. It then creates the aws_iam_role_policy_attachment resource which attaches the policy. There was a brief period where the policy was not attached.
• Terraform creates the aws_iam_role_policy_attachment resource which attaches the policy a second time (and possibly breaks). It then deletes the aws_iam_policy_attachment resource which would potentially detach the policy that was added by the other resource. It may remain with the policy not attached and require another terraform apply to attach it again.

I wonder if we rename the roles, will it handle the transition more smoothly? In that case, it might be worth switching to the terraform-aws-lambda-role module.

This is a fairly substantial change and it only helps people who are doing something that they should stop doing anyway. So I'm on the fence.

[nitrocode]

@raymondbutcher I can understand that frustration.

However, if you then use this module's policies outside of the module (which was never intended, as it's not an output of the module) then you'll run into the issues that the warning describes. That seems to be the case here (after chatting on Slack).

data sources ¯_(ツ)_/¯ ... but I agree

However, I'm not sure how this change would be applied by Terraform and AWS. These 2 potential scenarios concern me

It requires a specific set of steps to do safely.

I wonder if we rename the roles, will it handle the transition more smoothly? In that case, it might be worth switching to the terraform-aws-lambda-role module.

I don't believe so but maybe?

There only seem to be a handful of options

Options

Version pin and cut over

It's possible no one would be affected since the module's policy may never have been reused.

The README.md could be updated to use a version pin like the one shown in the terraform registry.

module "lambda" {
  source  = "claranet/lambda/aws"
  version = "0.12.0"

  ...
}

A new major version 0.13.0 could be cut over with these changes including a warning to use the older version if there are concerns with this change.

Fork and archive

If this repo won't get this change, there can be a separate public claranet fork, README.md update explaining why, repo archival, and then all issues and pending PRs can be migrated to the fork. This will allow people to migrate manually from the older fork to the newer fork.

Nothing

If nothing is decided to do, then at the very least the usage of the bad resource should be documented.

[nitrocode]

@raymondbutcher could you please add your thoughts to the last message?

[nitrocode]

@raymondbutcher friendly bump