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

[ADDON] - Flux #26

Open
kcoleman731 opened this issue Apr 20, 2022 · 16 comments
Open

[ADDON] - Flux #26

kcoleman731 opened this issue Apr 20, 2022 · 16 comments
Labels
add-on enhancement New feature or request gitops

Comments

@kcoleman731
Copy link

kcoleman731 commented Apr 20, 2022

Add support for Flux as an add-on

https://fluxcd.io/docs/installation/#bootstrap-with-terraform

@atali
Copy link

atali commented Apr 28, 2022

is it possible to add the same behavour as argocd with the flag argocd_manage_add_ons

@bryantbiggs bryantbiggs added the enhancement New feature or request label Apr 28, 2022
@kcoleman731
Copy link
Author

@atali yep that is what we will look to do. I don't believe we will be able to pass values from from resource created via TF to Flux however like we do with the ArgoCD application values map. We refer to this functionality as the Gitops bridge

@frank-bee
Copy link

Any news here regarding flux support ? :-)

@joaocc
Copy link
Contributor

joaocc commented Oct 4, 2023

Hi. Is any work being done in this area?
Flux seems quite easy to add (I already installed as helm chart via the addon (singular) module, and the existing examples make already establish a pattern for software with more than 1 SA).
My questions would be more on which (if any) policies to add - I can think of at least ECR pull permissions on the same account and region for the different controllers).
The bootstrapping could be outside the scope, unless there is a simple way to include it here.
@bryantbiggs, not sure if this one is with you, but if there is any work being done, and you (or others) could point me there, I could try to contribute in the next few days, as I will be doing exactly this internally.
Thx

@askulkarni2
Copy link
Contributor

cc: @csantanapr who is working on this.

@joaocc we are working on finalizing the revamped design and blueprint for gitops-bridge which will include support for both argocd and flux. You can expect flux support to be added soon after. In the mean time, if you want to take a stab at adding a module flux I think we would welcome that.

@joaocc
Copy link
Contributor

joaocc commented Oct 5, 2023

I was trying to understand if there were any changes to the current v1.9.x design of this blueprints addons model (ie, single monolith-file), and/or if there was any branch already being tested. If not, I can try to create one.
I was also thinking of proposing one as submodule to be called from the current main.tf (similarly to how https://github.com/aws-ia/terraform-aws-eks-blueprints/tree/v4.32.1/modules/kubernetes-addons were structured, but only creating one level) to get feedback. If current maintainers prefer to stick with a single file, it would be trivial to merge that content back to main.tf, as I would keep variables and outputs identical.

@joaocc
Copy link
Contributor

joaocc commented Oct 5, 2023

I am almost ready to push an initial draft version to a branch fork.

I have a current question regarding IRSA.
As per the documentation (https://fluxcd.io/flux/installation/configuration/workload-identity/#aws-iam-roles-for-service-accounts), the recommended approach is to create 2 separate IAM roles (mapped to 3 of the 6 controllers).
While I would prob 3 being better for environments with more strict separation needs, I would like to keep with the recommended approach.

However, in my understanding, the terraform-aws-eks-blueprints-addon module only supports one IAM role for each addon.

If it is confirmed that we can only map one role in terraform-aws-eks-blueprints-addon, and wanting to keep with the same approach as others (like aws_efs_csi_driver) it seems that we will be forced to have to map all 3 controllers to the same same role Also tried to find how to do this mapping and wasn't able to find it, since terraform-aws-eks-blueprints-addon only receives a list of annotations on which it will write the value of aws_iam_role.this[0].arn.

Is my understanding correct (@csantanapr, @askulkarni2, @bryantbiggs)? Or is there any way to configure terraform-aws-eks-blueprints-addon to create different roles?

I already created an issue (aws-ia/terraform-aws-eks-blueprints-addon#23) to discuss the ability to specify extra roles, but it doesn't seem to be easy to do in a way that doesn't break compact with existing modules. Maybe @bryantbiggs can support a v2.0 where this is possible (will add ticket to discus this)

In the meanwhile I will try to propose an initial implementation with a single role, as it is better than not having any :)

@bryantbiggs
Copy link
Contributor

I think this is getting to be a bit of an xy-problem. What method have you taken to enable FluxCD support?

@joaocc
Copy link
Contributor

joaocc commented Oct 5, 2023

I originally installed it directly via the addon module as it wasn't supported in the addons module.

@bryantbiggs
Copy link
Contributor

Per FluxCD:

The recommend way of installing Flux on Kubernetes clusters is by using the bootstrap procedure.

Of which they list two methods - CLI or w/ their Terraform provider. Why not use the bootstrap Terraform resource https://registry.terraform.io/providers/fluxcd/flux/latest/docs/resources/bootstrap_git ?

@brenwhyte
Copy link

brenwhyte commented Oct 5, 2023

Per FluxCD:

The recommend way of installing Flux on Kubernetes clusters is by using the bootstrap procedure.

Of which they list two methods - CLI or w/ their Terraform provider. Why not use the bootstrap Terraform resource https://registry.terraform.io/providers/fluxcd/flux/latest/docs/resources/bootstrap_git ?

Yeap this is what we use. There is no need to have Flux bootstraping as part of this project.

@bryantbiggs
Copy link
Contributor

There is no need to have Flux bootstraping as part of this project.

I'm starting to agree with that as well - I hadn't spent time looking into FluxCD integration until just now and after seeing this, I wonder what value it provides here

What permissions do the Flux IRSA roles require? Are those going to be custom to each specific implementation?

@joaocc
Copy link
Contributor

joaocc commented Oct 5, 2023

@brenwhyte, if you look at the same doc (https://registry.terraform.io/providers/fluxcd/flux/latest/docs/guides/install-helm-release) the also mention some valid reasons for installing it via helm:

Using the Flux Helm chart is a better option when Flux needs to be installed without any bootstrap configuration. The Helm Terraform provider can be used to install the chart in a Kubernetes cluster. Custom resource definitions will be applied along with all of the Flux components.

For context on why we are not using the CLI for bootstrapping, and this may be just our case, and maybe for historical reasons (or lack of time to find a better way), bootstrapping from CLI to a git repo which already had content lead to flux creating a parallel git tree, which forced us to manually fix it. Also, in terms of credentials, internally we prefer to not require PATs to deploy servers, if an ssh-key is all we require. At some point we decided it was better to install via helm early in the EKS setup process, and create the secrets and initial seed resources via kubernetes_manifests.

Since we were already using other addons we are already deploying (cert-manager, external-dns, external-secrets, efs-csi, cluster-autoscaler, ...), and since the IRSA bits are very nice in avoiding work and ensuring consistency, we started looking into adding it to the addons, and sharing this back to the community.

If @brenwhyte, @bryantbiggs and others in the project think approach of installing FluxCD doesn't have a place in the addons project, we can simply retire the PR, and keep using it in our fork for internal uses.

Another alternative would of course be to implement FluxCD in addons using the bootstrap provider, but that would add another provider to requirements, and would be less consistent with the rest of the addons.

Answering to @bryantbiggs question, the documentation of FluxCD defines 2 roles on 3 sa-accounts, for access to ECR and for access to KMS (https://fluxcd.io/flux/installation/configuration/workload-identity/#aws-iam-roles-for-service-accounts).

@bryantbiggs
Copy link
Contributor

From their docs

to grant Flux controllers access to cloud resources such as container registries, KMS, S3, etc.

I don't think those are the permissions they are stating as required, but just as examples

@joaocc
Copy link
Contributor

joaocc commented Oct 5, 2023

Yes, indeed not required, but certainly arguable that accessing ECR (and OCI on S3) via IRSA would be one of the desired goals when deploying to EKS.
Providing useful defaults for these common cases (as for instance is done on the AWS EFS CSI driver where access is given to all EFS on the AWS account - maybe a bit excessive but prob a very useful default for most users but overridable via some vars)

Based on the feedback above, I did a bit more research, and there seems to be a set of work in providing exactly this type of capabilities for FluxCD (listed in fluxcd/flux2#3003 and in the docs https://fluxcd.io/flux/security/contextual-authorization/).

in the case of AWS, they identify and support 7 use cases, 5 of them already supported.
The original roles/SAs i mentioned above (source, kustomize, image-reflector) are indeed mentioned. some use cases seem generalizable (source-controller obtaining charts via OCI and Helm access to ECR), kustomize accessing SOPs secrets with KMS, and image-reflector also with regards to ECR...

Supported      Source Controller             Bucket Repository Authentication      Guide
Supported      Source Controller             OCI Repository Authentication         Guide
Supported      Image Reflector Controller    Container Registry Authentication     Guide
Supported      Kustomize Controller          SOPS Integration with Cloud KMS       Guide
Supported      Source Controller             Helm OCI Repository Authentication    Guide
Not Supported  Source Controller             Git Repository Authentication (RO)    fluxcd/source-controller#835
Not Supported  Image Automation Controller   Git Repository Authentication (RW)    

But maybe I am not understanding the intent or strategy for the addons, and I am trying to add things that are too specific.
What would be the desired approach for adding FluxCD to the addons?

@csantanapr
Copy link
Contributor

The future plan is to not include argocd or fluxcd in this terraform module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add-on enhancement New feature or request gitops
Projects
Status: No status
9 participants