Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need update iamserviceaccount to be able to attach/detach Policies, without breaking running applications and cross-account Trust Relationships #1497

Closed
whereisaaron opened this issue Oct 28, 2019 · 13 comments
Assignees
Labels
kind/feature New feature or request priority/important-soon Ideally to be resolved in time for the next release

Comments

@whereisaaron
Copy link

Why do you want this feature?
I want to be able to attach/detach an IAM Policy to an IAM Service Account Role, without breaking running applications and cross-account Trust Relationships.

Currently the only way to change the attached Policies is to entirely delete the Role and then recreate the Role. This breaks all applications using that Service Account in the cluster until the applications are deleted and recreated in the presence the new Service Account.

It also permanently breaks cross-account trust relationships. Which can be a major hassle. First the new Role will have a new CF-generated name. And even if we could set a fixed Role name, it would still break all cross-account Trust Relationships:

https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_principal.html

If your Principal element in a role trust policy contains an ARN that points to a specific IAM role, then that ARN is transformed to the role's unique principal ID when the policy is saved. This helps mitigate the risk of someone escalating their privileges by removing and recreating the role. You don't normally see this ID in the console, because there is also a reverse transformation back to the role's ARN when the trust policy is displayed. However, if you delete the role, then the relationship is broken. The policy no longer applies, even if you recreate the role because the new role has a new principal ID that does not match the ID stored in the trust policy.

The disruption is unnecessary, as Policy attachments to Roles are mutable in IAM and CloudFormation, but eksctl is missing the command to do it.

The only workaround I see is to manually patch the CF stack outside of eksctl and update the eksctl config file as if the role had been created that way.

What feature/behavior/change do you want?
I propose a new command, e.g.

eksctl update iamserviceaccount --config-file=foo.yaml --include ns/myserviceaccount

that updates the CF stack for the IAM Service Account, replacing the Policy attachments in the config file (or command).

@whereisaaron whereisaaron added the kind/feature New feature or request label Oct 28, 2019
@cpaika
Copy link
Contributor

cpaika commented Nov 26, 2019

I'm facing the exact same issue. My workaround is (probably) going to be generating the iam role via cloudformation out of band, and associating that with the service account so I can change the role over time. But agreed - would love if eksctl could support updates

@rdubya16
Copy link

rdubya16 commented Apr 2, 2020

We need this feature as well. There is no way to amend a role without deleting the CF stack and the service account in k8s, then running eksctl create iamserviceaccount --approve --override-existing-serviceaccounts again

@mike-stewart
Copy link
Contributor

mike-stewart commented Apr 22, 2020

@martina-if @dholbach Is this a relatively straightforward change? If so, would you be open to accepting a PR? We don't have a lot of Go expertise but not being able to update roles could be a blocker for us so I could take a stab at it if you're open to it.

Or is there some other workaround to this that we're missing, short of manually updating roles after they were first created or managing them with another tool?

@cPu1
Copy link
Collaborator

cPu1 commented May 7, 2020

Apologies this went unnoticed for too long. This looks like an important and useful feature.

eksctl update iamserviceaccount --config-file=foo.yaml --include ns/myserviceaccount

that updates the CF stack for the IAM Service Account, replacing the Policy attachments in the config file (or command).

The only addition I'd make to the proposed command is to allow updating other fields like AttachPolicyARNs and AttachPolicy, since they can be updated without interruption.

@cdenneen
Copy link

cdenneen commented Oct 27, 2020

@michaelbeaumont any update on how to get these service accounts to update?
I have 4 that need updates but cannot figure out how to do this. (the update doesn't show they need update even though the current stack/iam only have 3 policies as opposed to the new 4 policies for each)
Also have new service account that got created but didn't update the annotation for the new ARN.

Even if not officially fixed is there a temp work around for this to keep us moving forward?

@rdubya16
Copy link

rdubya16 commented Oct 27, 2020

@cdenneen We have used a few workarounds, most of them not ideal:

  1. Update the policy that eksctl creates manually making sure to update the eksctl yaml as well
  2. Delete the stack that eksctl created and run the command below and restart all your pods (if you can tolerate a brief outage)
    eksctl create iamserviceaccount --approve --override-existing-serviceaccounts -f cluster.yaml
    Note: not even sure the override option works properly. Ive had to delete the service account before to get this option to work and set a new role arn
  3. Create a new iam policy/service account with a version number in it. Run the above command again and update your pods with the new versioned service account and delete the old stack for the old iam policies

None of these are great, it seems this is some really basic functionality thats lacking...

@michaelbeaumont
Copy link
Contributor

Yes, @rdubya16, indeed.

Also have new service account that got created but didn't update the annotation for the new ARN.

I want to understand exactly what happened, you added a new SA to iam.serviceAccounts and ran create iamserviceaccount. What is the result?

@cdenneen
Copy link

@michaelbeaumont
Had 4 existing service accounts that we added 4th attach policy ARN but those aren't showing for an update.
Had 1 existing service account that did update, created a new service account role but the k8s service account is still pointing to the OLD arn for the role which no longer exists since the new role was created... so in this case I believe I need to edit the service account annotation manually (not ideal)

For the other 4 I think @rdubya16 suggestions might be the only option (#2 not so much) and (#3 getting very messy and unmanageable long term I would imagine... ) 1 is probably best bet since its sort of aligns with the other annotation update and actually with what should have happened if eksctl updated the stack properly. So yes my eksctl config already shows 4 attached policy arns so updating the 4 stacks seems logical (but without any sort of proper validation this can always be dangerous, i.e. if validation was working then the stack updates would be happening so now what will happen after I manually update them)

@michaelbeaumont
Copy link
Contributor

Ultimately, this is due to the fact that create iamserviceaccount was not written to reconcile the config with the cluster, i.e. --override-existing-serviceaccounts is intended for cases where the kubernetes SA exists but not the IAM stack.
We're tracking the ability to update the policy for existing accounts from the config file in this issue

@cdenneen
Copy link

@michaelbeaumont you’d think however the SAs it created from scratch it should be able to properly update the policy’s on the role and then subsequently update the annotations to the updated role arn created.

@rdubya16
Copy link

@michaelbeaumont Is there another issue to follow for this? We will soon be managing 50ish clusters with eksctl and its a huge pain to rollout iam permissions changes to all of them currently in a consistent manner. We can use CI to validate our eksctl values files but its currently impossible to verify if they are insync with cloudformation without checking manually. This feature is badly needed for us.

@aclevername
Copy link
Contributor

this is currently being worked on #3064 feedback/thoughts are welcome 😄

@aclevername
Copy link
Contributor

eksctl update iamserviceaccount will be out in the next release 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request priority/important-soon Ideally to be resolved in time for the next release
Projects
None yet
Development

No branches or pull requests

9 participants