Add support for extensible IAM permissions

[yissacharcw]

This is a continuation of #550, now hopefully with CLA issues worked out, and with added tests.

---

This change is 

[k8s-ci-robot]

Hi @yissacharcw. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot 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.

If you have questions or suggestions related to this bot's behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

Once you've signed, please reply here (e.g. "I signed it!") and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.

If you have questions or suggestions related to this bot's behavior, please file an issue against the kubernetes/test-infra repository.

[yissacharcw]

I signed it!

[yissacharcw]

Fixes #379

[chrislovecnm]

How does this impact terraform users? Need to do more of a review as well

[chrislovecnm]

I am not a IAM expert by any means ... I pinged a couple of outside folks as well.

@kris-nova / @justinsb tag. I would like one of our security gurus to take a look as well.

[chrislovecnm]

We are trying to get #1183 in, and it most likely will impact this PR. Appreciate you patience. 1183 is a big change, with huge benefits.

[chrislovecnm]

Can we get a rebase?

[chrislovecnm]

Updating #1206

[yissachar]

@chrislovecnm Rebased and tested - everything is good now.

[chrislovecnm]

We need a rebase

[yissachar]

@chrislovecnm Rebased

[chrislovecnm]

I pinged @justinsb about this PR. Would like a review from him. Then we can merge.

[justinsb]

@yissachar my understanding is that you're planning on revisiting this when you have time, to create a second IAM policy without modification, and attach both. Going to mark as WIP so @chrislovecnm stops bugging me about it ;-)

[yissachar]

@justinsb That's correct. Just getting back to work now after the holidays, and catching up on things, but I should have time to work on it this week. I'll post back here once I have something to show.

[yissachar]

@justinsb @chrislovecnm I've updated the PR with the discussed changes (moving from modifying the existing IAM policy in-place to creating a separate IAM policy that is also mounted to the instances). I've also slightly refactored the cluster spec to have the field be a map instead of hardcoded fields for the masters and nodes. Seeing as we are starting to have more roles than just those two (e.g. bastion), this feels like a more elegant approach.

[yissachar]

I followed the existing code to delete IAM roles but I believe it's prone to becoming incorrect as more roles are added. Indeed, I see that we've already had an issue where we forgot to update the code to remove the bastion IAM role.

I think the code could be updated to iterate through all roles we have and add those to the list of things to be removed. This way, if we add another role in the future, deletion would automatically be handled and we wouldn't have to remember to specifically add it.

[justinsb]

LGTM

[justinsb]

This LGTM - thanks!