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

Karpenter IRSA IAM policy to be like blueprints one #2733

Closed
FernandoMiguel opened this issue Sep 5, 2023 · 15 comments · Fixed by #2858
Closed

Karpenter IRSA IAM policy to be like blueprints one #2733

FernandoMiguel opened this issue Sep 5, 2023 · 15 comments · Fixed by #2858
Milestone

Comments

@FernandoMiguel
Copy link
Contributor

Is your request related to a problem? Please describe.

We are porting Karpenter module from the old blueprints v4 to this module submodule, but the IRSA IAM policy is remarkably different to make the same infra fail to run when moving between modules

Describe the solution you'd like.

For karpenter IRSA IAM policy in this module be more like blueprints one

Describe alternatives you've considered.

  • we temporarily added the missing permissions to a new extra policy we pass to the module under irsa_additional

Additional context

https://github.com/aws-ia/terraform-aws-eks-blueprints/blob/80bd3b9/modules/kubernetes-addons/karpenter/data.tf
vs
https://github.com/terraform-aws-modules/terraform-aws-eks/blob/771465be/modules/karpenter/main.tf#L70-L163

@bryantbiggs
Copy link
Member

the IAM policy will be updated, but its going to be updated to match what is recommended by Karpenter https://github.com/aws/karpenter/blob/cce5dbc8ca580f17772e7f532d258011f102d966/website/content/en/v0.30/getting-started/getting-started-with-karpenter/cloudformation.yaml#L40-L221

This is under review for v20.x

@FernandoMiguel
Copy link
Contributor Author

I have a test cluster where I can test v20 branch.
Do you know if new labels are required to match? Or any new requirements?

@bryantbiggs
Copy link
Member

I don't believe new labels are required - if anything, label restrictions should be relaxed on the new policy https://github.com/aws/karpenter/pull/4180/files

@bryantbiggs bryantbiggs added this to the v20.0.0 milestone Oct 9, 2023
@mbarrien
Copy link
Contributor

mbarrien commented Oct 31, 2023

Karpenter v0.32.0 has now been released, which is the next major release since it add v1beta1 NodePool and EC2NodeClass resources. Since my prior PR #2788 was rejected, will there be a new one written by someone else, or should I write a new PR based solely on the recommendations that breaks the backwards compatibility I attempted in the previous PR?

@bryantbiggs
Copy link
Member

are there permissions missing from the current Karpenter controller IRSA? If so, we can add those. However, the complete overhaul of permissions to match the Karpenter project will happen on v20

@sidick
Copy link

sidick commented Oct 31, 2023

are there permissions missing from the current Karpenter controller IRSA? If so, we can add those. However, the complete overhaul of permissions to match the Karpenter project will happen on v20

As of 0.32.0 Karpenter uses a role for the nodes rather than instance profile, it should be reasonably easy to create both and let people use whichever suits. See https://karpenter.sh/docs/upgrading/v1beta1-migration/#instanceprofile

@bryantbiggs
Copy link
Member

I agree - which is why its already provided

@mbarrien
Copy link
Contributor

It's not just a matter of passing in the role and having this module creating an instance profile for it... in Karpenter v0.32.0 the Karpenter controller is in charge of creating instance profiles for the passed in role; you can no longer pass in an instance profile directly. See aws/karpenter-provider-aws@325a4b9#diff-4c0112f5dbd0f8a995e2a8bba418f3d5f3ee5897beb747fb2e9f09208f14c9cd where they basically add the following permissions:

iam:CreateInstanceProfile
iam:TagInstanceProfile
iam:AddRoleToInstanceProfile
iam:RemoveRoleFromInstanceProfile
iam:DeleteInstanceProfile
iam:GetInstanceProfile

In addition, the module here has drifted so much from what is recommended because it is missing the changes in aws/karpenter-provider-aws#3948 where they scoped down permission based on tags, so copying these over as is is not straightforward.

Note that Karpenter v0.32.0 will not work with this module as it currently is, until the above permissions are added somehow.

If you insist on not doing the overhaul now, and are okay with Karpenter having overly broad access to change all profiles and not just ones that are properly tagged, you can add the above permissions to the big list in https://github.com/terraform-aws-modules/terraform-aws-eks/blob/v19.17.4/modules/karpenter/main.tf#L73-L88 and be done with it until v20. Or you could instead reconsider reopening #2788 which attempted to bring in the changes from aws/karpenter-provider-aws#3948 as well as these changes.

@bryantbiggs
Copy link
Member

again -> #2733 (comment)

@mbarrien
Copy link
Contributor

I am responding exactly to that comment. I have listed exactly which permissions need to be added if you want to do it minimally. Do you want a PR for that minimal change? This module will not work with Karpenter v0.32.0 as is, so it cannot wait for the v20 overhaul unless you want to wait until v20 to make it compatible.

@sidick
Copy link

sidick commented Oct 31, 2023

Just an FYI this module does work with Karpenter 0.32 as long as you stick to using the older alpha APIs and don't use the new ones yet.

@ahilmathew
Copy link

ahilmathew commented Nov 8, 2023

Looks like the IRSA needs the iam:GetInstanceProfile permission.

User: arn:aws:sts::1234567789:assumed-role/KarpenterIRSA-dev/169941234567890 is not authorized to perform: iam:GetInstanceProfile on resource: instance profile dev_16604098787662 because no identity-based policy allows the iam:GetInstanceProfile action\n\tstatus code: 403, request id: "}

@mbarrien
Copy link
Contributor

mbarrien commented Nov 8, 2023

Looks like the IRSA needs the iam:GetInstanceProfile permission.

User: arn:aws:sts::1234567789:assumed-role/KarpenterIRSA-dev/169941234567890 is not authorized to perform: iam:GetInstanceProfile on resource: instance profile dev_16604098787662 because no identity-based policy allows the iam:GetInstanceProfile action\n\tstatus code: 403, request id: "}

v19.18.0+ should be providing this as long as you pass enable_karpenter_instance_profile_creation=True

@antonbabenko
Copy link
Member

This issue has been resolved in version 20.0.0 🎉

Copy link

github-actions bot commented Mar 4, 2024

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
6 participants