terraform-aws-modules / terraform-aws-iam

Terraform module to create AWS IAM resources šŸ‡ŗšŸ‡¦
https://registry.terraform.io/modules/terraform-aws-modules/iam/aws
Apache License 2.0
787 stars 996 forks source link

feat: Enable override policy name iam-group-with-assumable-roles-policy #468

Closed schollii closed 6 months ago

schollii commented 7 months ago

Description

Add an extra variable so that the assume-roles policy created by iam-group-with-assumable-roles-policy module can be customized with minimal effort: just have an optional suffix that defaults to empty so this PR is fully backwards compatible. Eg

module "iam_group_with_assumable_roles_policy_production_admin" {
  source = "../../modules/iam-group-with-assumable-roles-policy"

  name = "production-admin"
  assumable_roles = [module.iam_assumable_roles_in_prod.admin_iam_role_arn]
  assumable_roles_policy_name_suffix = "-assumable-roles"
  ...

Motivation and Context

I have been using this module for many months and I really dislike the default name, which is just the group name, because I for one have IAM groups that have a policies named after them since they represenent the group's "main permissions". Eg if I create group Foo, I have policy named Foo that gives that group it's basic set of permissions (eg read-only permissions), and I want a policy FooAssumeRoles that provides the roles that can be assumed by members of that group.

Breaking Changes

None

How Has This Been Tested?

I applied using the example, in my own AWS account, on master branch. Then I switched to my branch and re-planned: no changes. Then, I set a non-empty value to assumable_roles_policy_name_suffix and the diff was correct: policy would be replaced due to new name and the attachment too.

bryantbiggs commented 6 months ago

@Smana is there a reason you are outputting these statements across a number of our repositories? You've just displayed several details that most would not want to be publicly available

Smana commented 6 months ago

I'm confused, I'm currently testing the tofu-controller branch planner and IĀ probably misconfigured something. I'm gonna delete these comments, sorry for the inconvenience.

Smana commented 6 months ago

That should be done by now. I'm gonna talk to the maintainers because honestly IĀ don't know what I did wrong here. Let me know if IĀ missed something.

schollii commented 6 months ago

@bryantbiggs I am not able to see why the "collect workflow inputs" pre-commit check fails, no logs for that action except "internal error" Screenshot_20240402-082119

antonbabenko commented 6 months ago

@schollii I rerun GH Actions, and now it shows that something is not right - https://github.com/terraform-aws-modules/terraform-aws-iam/actions/runs/8318806399/job/23343434805?pr=468

schollii commented 6 months ago

Yeah the collect workflow passes now. Not sure why all these additional files got changed, it's when I ran the pre-commit tool locally I think. Also I forgot to restore the example file so it will run on your aws, I needed to test in my own account. I'll clean all this up, sorry for the messy PR.

schollii commented 6 months ago

OK @antonbabenko the PR contains only the 4 files I intended to change and there is 1 wrapper file which I'm not sure should be changed but all pre-commit checks pass now

oliver-sentianse commented 6 months ago

Thanks for quick merge, I'll be making use of it today!

antonbabenko commented 6 months ago

This PR is included in version 5.39.0 :tada:

github-actions[bot] commented 5 months ago

I'm going to lock this pull request because it has been closed for 30 days ā³. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.