Feat/instance profile iam

[florian0410]

what

• Enhancement of Allow use of existing IAM role for EC2 instance profile #107 + Allow use of existing IAM role for EC2 instance profile #113, due to original developer seemingly abandoning the original PR.
  • Adds service_role_name as another 'override', like instance_role_name is in the original PR.
• Allow the user of the module to specify an existing IAM Role name for the instance profile.
• Allow the user of the module to specify an existing IAM Role name for the service profile.
• This IAM role name will be used to create the instance profile that is assigned to the EC2 instances managed by Elastic Beanstalk.
• Add lifecycle create_before_destroy since some of my testings showed that we could break the environment if we remove the IAM role before Beanstalk finished to update the environment.
• Add example using nlb

why

• Some environments/users do not have the ability to create their own IAM roles/policies, for security reasons. This change allows a user to provide their own IAM role if one already exists.
• Currently the module creates an IAM role and a series of permissions for the role.
• It is hard to specify what permission to use
• We cannot entirely define the permissions to use even with extended_ec2_policy_document

references

• closes Option to provide instance profile role or role policy #70
• closes Allow use of existing IAM role for EC2 instance profile #107
• closes Use customed 'iam_instance_profile' and 'iam_service_role' #127
• closes Allow use of existing IAM role for EC2 instance profile #113
• Give a solution for Default aws_iam_role_policy is too permissive (feedback is welcome) #181 (still some work to do after this)

Mentions

I reused propositions from #113 and #107 for this PR with some rebase, thank you to @bstascavage and @JBarna

[florian0410]

/test all

[nitrocode]

If these resources are created before destroyed, there would be a name conflict, no?

Why are these lifecycles added?

[florian0410]

The role should never change except if we rename it so it's safe to add.

I added it because I encountered a stuck issue with beanstalk while refreshing my environment. Imagine you are changing from default aws_iam_role to service_role_name but for some reason, beanstalk fail during process. Beanstalk will try to rollback but if terraform already removed the original role, it's going to hard stuck the environment and will require an advanced debug maybe with CLI or by recreating the whole environment.

By ensuring we destroy this resource at the end of beanstalk deployment, we make sure beanstalk is able to rollback to its previous configuration in case of failure.

[nitrocode]

I added it because I encountered a stuck issue with beanstalk while refreshing my environment. Imagine you are changing from default aws_iam_role to service_role_name but for some reason, beanstalk fail during process.

This sounds like a corner case. Is that right ?

The create_before_destroy lifecycle sounds like it could cause other problems. I'll defer to my teammates to see if they have any issues with it.

[florian0410]

Adding some insights for this discussion.

The actual default iam role and policy give a lot of rights for launching anything using beanstalk. The goal here is to let the user define a fully new instance profile from scratch.

Considering it's beanstalk, a single missing right can mess easily an environment. Someone who would switch from the module default iam role to a custom one may stuck its environment since we implement too many policies.

[nitrocode]

A missing right? What do you mean? Do you mean a missing resource?

Why should it matter how many policies we apply? That shouldn't be causing issues

[florian0410]

When I'm talking about missing rights, it's in the case where the user use its own instance_profile or service role for beanstalk.

In the module defaults, we already defined basic requirements that ensure beanstalk get health status of applications and have rights for deploying.

But when a user choose to manage its own rights, we should consider the eventuality where he does not define correctly these and miss some rights he thought were not needed. Which can lead to beanstalk failing the deployment and rolling back then.

I'm not sure if I'm clear enough, maybe we could reach through slack if you prefer ?

[nitrocode]

The create_before_destroy lifecycle sounds like it could cause other problems. I'll defer to my teammates to see if they have any issues with it.

cc: @aknysh @jamengual

[florian0410]

/test all

[florian0410]

/test all

[florian0410]

Hello @nitrocode just fixed some things according to your suggestions.

Is the security groups feature still blocking MR in this repository ?

[mergify]

This pull request is now in conflict. Could you fix it @florian0410? 🙏

[aknysh]

@florian0410 please resolve the conflicts

[aknysh]

@florian0410 please resolve the conflicts

[lbeltramino-uala]

@florian0410 please resolve the conflicts!

[lbeltramino]

@florian0410 please resolve the conflicts!

[damiromero-uala]

please resolve the conflicts! we need this

[Gowiem]

@lbeltramino-uala @lbeltramino @damiromero-uala -- At this point, I think @florian0410 is likely too busy and isn't likely to pick this one up. I would highly suggest that one of you take his work and create a new branch, work through the conflicts, and PR that. I would be happy to review, so please add me as a reviewer if you choose to do so. Thanks!