✨ Add support for AWS_CONTROLLER_IAM_ROLE env var

[MarcusNoble]

What this PR does / why we need it:
Provide the ability to set the AWS_CONTROLLER_IAM_ROLE environment var which will then be used to add an EKS annotation to the service account the management components run as. This will allow those components to run as that IAM role when using IAM for Service Accounts.

Added docs. Preview: https://deploy-preview-2052--kubernetes-sigs-cluster-api-provider-aws.netlify.app/topics/specify-management-iam-role.html

Includes Kiam support taken from: #1975

Note: Due to aws/amazon-eks-pod-identity-webhook#8 until EKS 1.19 is available with the appropriate fix in place, this requires us to set the fsGroup to ensure the token is readable as we're using non-root containers.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes: #2053
Related to: #1919 and #1552

[k8s-ci-robot]

Hi @MarcusNoble. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[richardcase]

Thanks @MarcusNoble. I think we probably need to discuss how IRSA is implemented in the provider and how to make it optional.

@michaelbeaumont - what are your thoughts on this?

A few things come to mind, if we automatically create the IAM role with the trust relationship to the OIDC provider for the cluster do we need a new feature flag for this? We could use EKSEnableIAM but with this we also get the IAM role created per cluster for the nodes etc. The docs will need updating as well.

[richardcase]

Needs further discussion, so:
/hold

[randomvariable]

IRSA doesn't need to be specifically enabled, it's part of the default credential provider chain lookup.
Need to diff this with #1975 for the kiam support as well.

Will need to ensure in any case that the 3 workflows work depending on which variables are set:

• Static credentials via env vars or credentials file
• IAM role set for kiam
• IRSA set

[MarcusNoble]

@richardcase It's optional by default. If the env var isn't set then it will function exactly as it does today.

I don't see a harm in having the role created outside of cluster-api providing the permissions needed are documented somewhere (I would think they'd already need to be for whatever user is created to be used currently).

If auto-provisioning this role then some changes would need to be made to this.

For what it's worth, for my usecase creating the role myself would be preferred as we're looking to then use that role to assume roles in other AWS accounts to create workload clusters if this PR goes ahead - #1919

[richardcase]

It's optional by default. If the env var isn't set then it will function exactly as it does today.

Ah yes, i misread the envsubst, thats good.

I don't see a harm in having the role created outside of cluster-api providing the permissions needed are documented somewhere (I would think they'd already need to be for whatever user is created to be used currently).

Agree that pre-creating the role and using that must be supported and that can be covered by documentation. We already have the roles that are created as part of the bootstrap stack for the controllers, control-plane etc.

But even if we use the roles created by the bootstrap stack don't we have to modify the trust relationship to support IRSA? And add the identity provider for the cluster? (i'm probably misunderstanding 😁 )

[MarcusNoble]

I was saying I don't think it's a problem to ask the users to create the role themselves instead of the bootstrap stack. This would (currently) only apply to those that have created the management cluster themsleves rather than using a bootstrap cluster so it's not a huge ask to also have a role created.

[richardcase]

Ahhh, specifically for the management cluster and not IRSA for the new workload clusters.

[richardcase]

/unhold
/ok-to-test

[randomvariable]

This document needs to be separated out into seperate instructions for IRSA and Kiam. The latter is still heavily used.

[randomvariable]

Given Kiam is still being used by many people, can we make this more specific for AWS_CONTROLLER_IRSA_IAM_ROLE so we can also add support Kiam separately? Can you also add in the KIAM bits from #1975 please (which I'll close)?

@pingles, can I just check what the behaviour of Kiam is if someone supplies the annotation iam.amazonaws.com/role: ""? Will it just get ignored?

[MarcusNoble]

I'll merge in the Kiam changes.

It doesn't need a specific env var. The annotations for both IRSA and Kiam can be used at the same time and whatever solution is in use will pick up the correct value and ignore the other.

[randomvariable]

That works for me, thanks.

[MarcusNoble]

@randomvariable Updated to include the needed annotations and docs for Kiam and Kube2iam (both use the same approach)

[MarcusNoble]

/assign @timothysc

[randomvariable]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: randomvariable

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [randomvariable]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment