Fill Role names in kops-controller-config instead of instance profile names when it is specified

[h3poteto]

refs: #10719

The role names are checked in node bootstrap. If profile names are provided, bootstrap will fail. Because profile name and role name do not always mactch in AWS IAM. So I fixed nodesRoles in kops-controller-config to store IAM Role names associated with specified IAM Instance Profile.

I think that this should be cherry-picked to 1.19.

[k8s-ci-robot]

Hi @h3poteto. 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 /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.

[olemarkus]

/ok-to-test

[rifelpet]

/cc @johngmyers if you have time to review this.

I'm debating the pros and cons of having kops-controller fetch the list of roles for each instance profile at startup time vs having the kops CLI get that list and populate the configmap with the list of roles.

I'll admit I don't fully understand how having multiple roles associated with an instance profile and having that instance profile attached to an instance affects credentials provided via instance metadata, specifically when nodeup creates an sts.GetCallerIdentity request. How is the role chosen between all of the roles associated with the instance's instance profile?

[h3poteto]

I'm debating the pros and cons of having kops-controller fetch the list of roles for each instance profile at startup time vs having the kops CLI get that list and populate the configmap with the list of roles.

First I considered it, that is the kops CLI gets IAM Roles and fill it in kops-controller-config. But it is difficult because kops-controller-config is generated in template functions:

kops/upup/pkg/fi/cloudup/template_functions.go

Line 454 in 0271f0e

NodesRoles: nodesRoles.List(),

So it is difficult to call AWS API in this function.

How is the role chosen between all of the roles associated with the instance's instance profile?

OK, I'll check it.

[johngmyers]

Per AWS documentation, an instance profile can contain at most one IAM role. (I admit to being baffled as to why instance profiles exist, as opposed to assigning roles directly.)

The role in an instance profile can change, but I don't think kops needs to support noticing this change without an apply_cluster. I would prefer early binding of the permitted role, during apply_cluster. I would accept binding upon kops-controller startup, but think doing an API query on every token verify like the code appears to do is questionable.

I think we need to add a field of type fi.Cloud to TemplateFunctions. Then it would be possible to call the AWS IAM API from within KopsControllerConfig.

[h3poteto]

Indeed, if TemplateFunctions has fi.Cloud, it is possible.

Which do you think is better, to get IAM Roles in kops-controller or to get IAM Roles in template functions and write it in configmap?

[h3poteto]

Per AWS documentation, an instance profile can contain at most one IAM role.

Thank you.
I also tried, but can't assign multiple role to a instance profile.

[rifelpet]

My mistake, I was remembering incorrectly and was thrown off by the Roles field here being a list. I agree in preferring the lookup happen in TemplateFunctions and providing the role list in the config file. We will need to provide a fi.Cloud to template functions. TemplateFunctions is setup in apply_cluster.go which already has a fi.Cloud so it should be straight forward to pass in.

[h3poteto]

I see. I will rewrite this changes.

[h3poteto]

/retest

[h3poteto]

@rifelpet I rewrote this pull request, please review this again.

[h3poteto]

/retest

[rifelpet]

This looks good, thanks!

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: h3poteto, rifelpet

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 [rifelpet]

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

[h3poteto]

/retest