terraform-aws-eks icon indicating copy to clipboard operation
terraform-aws-eks copied to clipboard

Karpenter IRSA IAM policy to be like blueprints one

Open FernandoMiguel opened this issue 1 year ago β€’ 13 comments

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

FernandoMiguel avatar Sep 05 '23 11:09 FernandoMiguel

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

bryantbiggs avatar Sep 05 '23 12:09 bryantbiggs

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?

FernandoMiguel avatar Sep 05 '23 12:09 FernandoMiguel

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 avatar Sep 05 '23 12:09 bryantbiggs

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?

mbarrien avatar Oct 31 '23 07:10 mbarrien

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

bryantbiggs avatar Oct 31 '23 11:10 bryantbiggs

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

sidick avatar Oct 31 '23 15:10 sidick

I agree - which is why its already provided https://github.com/terraform-aws-modules/terraform-aws-eks/blob/bb8aae31ec555f586a3ff482e35f10deb9b1a30b/modules/karpenter/outputs.tf#L57

bryantbiggs avatar Oct 31 '23 16:10 bryantbiggs

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 https://github.com/aws/karpenter/commit/325a4b92e91a16809040990675160d9e61bc564b#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 https://github.com/aws/karpenter/pull/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 https://github.com/aws/karpenter/pull/3948 as well as these changes.

mbarrien avatar Oct 31 '23 16:10 mbarrien

again -> https://github.com/terraform-aws-modules/terraform-aws-eks/issues/2733#issuecomment-1787067535

bryantbiggs avatar Oct 31 '23 16:10 bryantbiggs

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.

mbarrien avatar Oct 31 '23 16:10 mbarrien

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.

sidick avatar Oct 31 '23 17:10 sidick

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: "}

ahilmathew avatar Nov 08 '23 09:11 ahilmathew

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

mbarrien avatar Nov 08 '23 09:11 mbarrien

This issue has been resolved in version 20.0.0 :tada:

antonbabenko avatar Feb 02 '24 14:02 antonbabenko

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[bot] avatar Mar 04 '24 02:03 github-actions[bot]