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

Add optional IRSA provisioning for most common EKS components ? #1825

Closed
philicious opened this issue Feb 1, 2022 · 18 comments · Fixed by terraform-aws-modules/terraform-aws-iam#184

Comments

@philicious
Copy link
Contributor

philicious commented Feb 1, 2022

This module helps and eases EKS provisioning tremendously, especially for people less profound with EKS and/or Kubernetes.
However there are still more steps many users likely need to take to have a truely basic EKS setup:

I have created and managed over 10 EKS clusters productively with this module and easily 20 more for testing and engineering. Which also lead to adding LT support a yr ago ( #997 ), and the example LT by now made it into v18.x. Which is a huge leap forward in general.

Now, besides LTs, I also always need IRSA for cluster-autoscaler, external-dns, aws-loadbalancer-control and often also aws-ebs-csi-driver. So I basically have an extra .tf file for each in my wrapper module, utilizing https://registry.terraform.io/modules/terraform-aws-modules/iam/aws/latest/submodules/iam-assumable-role-with-oidc and a fitting policy.

Similarly, but more rare case, I also have a policy for a KMS key, so the ASGs are able to mount the encrypted disks

(Some of) these might be a good addition to this upstream module, disabled by default. It would ease provisioning of EKS clusters even more.

What do you guys think? I would volunteer preparing PRs and such

@philicious
Copy link
Contributor Author

cc @bryantbiggs @ArchiFleKs @barryib @stevehipwell (just highlighting some of you I know have contributed in discussions and PRs before )

@ArchiFleKs
Copy link
Contributor

I'd be happy to see this implemented, first for VPC-CNI addon which by default runs with node IAM roles I think.

For the other "standard" EKS addons like ebs-csi-drivers (which will be in official addons soon I think), AWS load balancer controller etc why not. But I'm not sure it is the goal of this module to deploy this addons directly ? It would just be the IRSA roles then ?

I personally maintains this https://github.com/particuleio/terraform-kubernetes-addons/tree/main/modules/aws which I installed on top of all my EKS clusters that takes care of all the "addons" that make use of IRSA.

@philicious
Copy link
Contributor Author

@ArchiFleKs I am solely speaking of the IRSA part, as those require the EKS OIDC issuer URL and are thereby quite close to this module, logically.

the way of provisioning the components themselves, I would consider a users choice as there are multiple ways that can make sense depending on scenario

@philicious
Copy link
Contributor Author

@ArchiFleKs thats an awesome toolbelt, kind of reminds me of mine. I guess we could easily throw existing code and ideas together, after selecting a few really common ones

@bryantbiggs
Copy link
Member

have you seen https://github.com/aws-samples/aws-eks-accelerator-for-terraform which actually utilizes this module (for the control plane)? Specifically https://github.com/aws-samples/aws-eks-accelerator-for-terraform/tree/main/modules/kubernetes-addons

I think there are interesting discussions around who, what, where. Usability is always quite important, but so are (IMO):

  • Maintainability
  • Avoiding duplicated efforts
  • Well defined modules that are loosely coupled

@philicious
Copy link
Contributor Author

philicious commented Feb 1, 2022

have you seen https://github.com/aws-samples/aws-eks-accelerator-for-terraform which actually utilizes this module (for the control plane)? Specifically https://github.com/aws-samples/aws-eks-accelerator-for-terraform/tree/main/modules/kubernetes-addons

yes, this is kind of what I had in mind. the eks-accelerator in total might be to big for some users (like me).
so adding a few basic components in here might still be a thing.

furthermore, there are reasons one wants to provision certain addons yourself and not use the AWS-managed ones.

@bryantbiggs
Copy link
Member

I'm not following - you should be able to individually select and use the addons that the EKS accelerator team has provided as independent, external module instantiations. This would let you pick and choose what you want as well as mix and match. I haven't tried this myself but looking at the directory structure, you should be able to do this (might need to add the Terraform required versions to those sub-module to be better as stand alone modules - @kcoleman731 / @vara-bonthu):

module "helm_addon" {
    source = "github.com/aws-samples/aws-eks-accelerator-for-terraform//modules/kubernetes-addons/helm_addon"
 	
	# <enter params here>   
}

I'm not seeing the benefit in duplicating those addon sub-modules under this module - unless I'm misunderstanding what your request is

furthermore, there are reasons one wants to provision certain addons yourself and not use the AWS-managed ones.

Agreed - but what does that have to do with this module? Apologies if I'm blatantly missing something

@philicious
Copy link
Contributor Author

philicious commented Feb 1, 2022

you should be able to individually select and use the addons that the EKS accelerator team has provided as independent, external module instantiations. This would let you pick and choose what you want as well as mix and match.

for instance https://github.com/aws-samples/aws-eks-accelerator-for-terraform/blob/main/modules/kubernetes-addons/aws-ebs-csi-driver/main.tf provisions the ebs-csi as a true EKS addon but you might want to provision it self-managed and need to take care of IRSA yourself.

e.g. with https://github.com/aws-samples/aws-eks-accelerator-for-terraform/blob/main/modules/kubernetes-addons/aws-load-balancer-controller/main.tf your described way is more feasible but already brings in the component deployment via helm then, which might not fit in existing tooling, etc

--

Agreed - but what does that have to do with this module? Apologies if I'm blatantly missing something

well, I consider this module here as a library for building a minimum EKS with ease. kind of like GKE feels out of the box.
in contrary, the eks-accelerator goes far beyond a libarary and is more of a framework.

so for this kind of "basic EKS library" I thought it might be worthy adding the IRSA provisioning for basic cluster components that fit most peoples needs.

so I wanted to start a discussion around if and what might be useful additions similar to what I described. as I feel this module has become really great latest with 18.x but has more potential to grow

@philicious philicious changed the title Add optional IRSA provisioning for most common EKS addons ? Add optional IRSA provisioning for most common EKS components ? Feb 1, 2022
@kcoleman731
Copy link

kcoleman731 commented Feb 1, 2022

@bryantbiggs - thanks for the heads up on this thread!

@philicious - appreciate your feedback on the eks-accelerator feeling more like a framework than a library. We definitely want to encourage the use of specific submodules in cases where the entire project isn’t needed. We will add an example which demonstrates how to consume the kubernetes-addons module directly as @bryantbiggs called out above.

One submodule I do want to point out is irsa. Multiple add-ons in our kubernetes-addons module leverage it to provision appropriate irsa resources. You can see sample usage here. It can be consumed independently as well. Perhaps this can be helpful with your use case?

@bryantbiggs
Copy link
Member

I opened this #1827 just to see if I was tracking with what you were saying @philicious

I don't love it - but I don't hate it. The more I think about it, we already have two IAM modules for OIDC/EKS which enable IRSA, this just seems silly to add a third just for namespaces and service accounts (unless I'm missing something else).

@max-rocket-internet since you did the latest EKS IAM role - care to share any thoughts?

@stevehipwell
Copy link
Contributor

I might have missed something here, but I'm not sure why this would be a sub-module of EKS and not a standalone module? I often don't want to create my roles when I create my cluster as they're in a higher layer, and more often than not I just want the ARN to pass into a Helm chart or manifest.

I have an internal module to do this with that I've been using since IRSA was released which I'm happy to share. It's very simple.

@bryantbiggs
Copy link
Member

if anything was added, it would not be added to the root module - it can be defined as a sub-module under the modules/ directory and then users can access it with:

module "irsa" {
  source  = "terraform-aws-modules/eks/aws//modules/irsa"
  version = "~> 18.3"
  
  ...
}

So its codified/managed in one repository, but its usage is external to the EKS module (from a users perspective))

@philicious
Copy link
Contributor Author

philicious commented Feb 1, 2022

Perhaps this can be helpful with your use case?

@kcoleman731
I am personally fine with what I have and also re-use to accelerate EKS accessibility and adoption at my customers (similar to a smaller version of aws-eks-accelerator-for-terraform and built upon this module here + others like this https://github.com/terraform-aws-modules/terraform-aws-iam/tree/master/modules/iam-assumable-role-with-oidc )

from my consulting perspective in different projects, I feel that especially setting up the required IRSA components with EKS is not as accessible to the broader audience and dev/ops experience as it could be

so my initial thoughts were only about the IAM Roles + Policies which are harder to find in docs and then to wire together in TF.
So its not about the component deployments themselves, as that differs per use case scenario. The kubernetes-addons of the eks-accelerator can fill the gap for some but are doing "to much" for others.

In the end an improved documentation and/or examples might aswell be a good solution. I am happy to help in any direction.

One submodule I do want to point out is irsa.

ye, the issue here would again be the missing policies that a user would have to find and fill in themselves.

--

I will condense and precise my initial post (tomorrow, its late over here), taking other comments into account and transfer it to #1827 (thanks @bryantbiggs !) , so we and others can continue this thread there

@bryantbiggs
Copy link
Member

oh, so it would be like what we have with the S3 bucket module - various flags to enable different policies.

so if we used the EKS IAM module (its specific to EKS so seems logical), and just added the various IAM policies to where users can do something like:

module "eks_irsa" {
	source = "terraform-aws-modules/iam/aws//modules/iam-eks-role"
	
	enable_node_autoscaler_policy = true
	
	...
}

Is that closer? I think that could be quite useful - I could see some race/dependency issues possibly, but could be useful

@philicious
Copy link
Contributor Author

@bryantbiggs now you got me. Or I got you 💯

(limited to the most common and basic additional components (tbd) like autoscaler, ebs-csi, external-dns etc you name it)

@bryantbiggs
Copy link
Member

ok cool - I'm going to close the draft PR then. I'll draft up a PR tonight/tomorrow to see what this roughly looks like and then we can start sourcing PRs for various policies

@bryantbiggs
Copy link
Member

initial draft - terraform-aws-modules/terraform-aws-iam#184

@github-actions
Copy link

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 Nov 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants