aws-ia / terraform-aws-eks-blueprints-addons

Terraform module which provisions addons on Amazon EKS clusters
https://aws-ia.github.io/terraform-aws-eks-blueprints-addons/main/
Apache License 2.0
272 stars 127 forks source link

Various GitOps flags don't appear to be respected. #104

Closed mkantzer closed 1 year ago

mkantzer commented 1 year ago

I may be misunderstanding the new direction of the blueprint/addon system, in which case sorry for jumping in before things are ready. But, wanted to flag a concern. It appears that there's still a general desire to give the options of "deploy everything via terraform" vs "set stuff up for GitOps to deploy charts to the cluster" (judging by new flags like the lb controller's _gitops bool.

However, it doesn't seem like that flag actually changes behavior in a meaningful way. It adds (for example) the lb controller info to the argocd local config, but it doesn't actually stop the helm releases from rendering into the terraform.

I'd assume we'd want to be using the _gitops flags to set var.create_release (or, i guess we'd want ![...]_gitops to reverse truthiness) in the generic module, so we can have it just set up (and pass to argo) the IRSA stuff.

askulkarni2 commented 1 year ago

Hi @mkantzer, thank you for your question and you are absolutely correct. We are working through some design ideas on how we will enable addons for pure a gitops based workflow. Ideally, we want to be agnostic of gitops tool being used and have clean separation of responsibilities when it comes AWS vs k8s based resources. As you pointed out IRSA and we few other AWS resources may be required by some add-ons and we are iterating on what would be the best way to do a hand-off of the resource ids (or should they?).

If you have any thoughts on this please feel free to share here.

mkantzer commented 1 year ago

I've been playing around with the current state of the project, and I'm kinda thinking that the argocd module shouldn't inherently manage add-ons, at least not in any way that's different from the management of any other argocd app or project (so: minimize automatically populating the addon application's values)

What I'm currently doing is:

I think this ends up being pretty flexible: the argocd module isn't trying to do anything particularly fancy, and is clearly scoped to "I set up argocd and applications, with a clear way to pass in values". Then you have the generic module with a similar goal to how it is now: Flags to set up IRSA and/or helm release, with clear outputs designed with an eye for passing into gitops-managed k8s systems. Finally, you have any "hey, we need something special" modules (such as for external-dns or what have you) that are basically small extensions of the generic system, with the same option to not create the helm releases.

This also shows a pretty clear example for someone who wants to use some other git-ops platform: you can fully re-use the non-argocd modules, and a module for should only deal with bootstrapping the platform and passing inputs to their rendering systems.

It also removes the need for any per-addon bespoke management within the argocd module.

You can see my setup here for reference:

bryantbiggs commented 1 year ago

thank you for the issue! At this time we are re-evaluating the integration between Terraform and GitOps operators like ArgoCD. We have removed the integration from the current project and will be tracking all feedback input in #114 while developing the next iteration of this integration

CCOLLOT commented 1 year ago

This is annoying. I don't see the point of not exposing var.create_release in this module. In my case I will probably have to manually create all resources (IAM policies etc...) just because I don't want Terraform to deploy the helm release. I know you guys are currently working on a better approach but in the meantime we have no ETA and, in my case, not being able to configure var.create_release to false makes me unable to use this module efficiently.

bryantbiggs commented 1 year ago

If you are looking for just the IRSA roles w/ permissions, you can use https://github.com/terraform-aws-modules/terraform-aws-iam/tree/master/modules/iam-role-for-service-accounts-eks

CCOLLOT commented 1 year ago

It's a nice suggestion, thanks 👍 . However, for some addons (ex: Karpenter), it's not enough as other resources must be created (ex: SQS queue). I think I'll temporarily keep the release in Terraform but we really need a solution to hand over the release to the desired Gitops engine.

Also, the iam-role-for-service-accounts-eks is much more verbose than the lightweight enable_xxx syntax. This is the level of abstraction we need! 🙏

bryantbiggs commented 1 year ago

That is what https://github.com/aws-ia/terraform-aws-eks-blueprints-addons/issues/114 is meant to handle - I would add any feedback to that issue