[FEATURE] Support for passing *_helm_config to ArgoCD

[kahirokunn]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

What is the outcome that you are trying to reach?

passing *_helm_config to ArgoCD.
eg. aws_load_balancer_controller_helm_config

Describe the solution you would like

Add code to pass *_helm_config to ArgoCD.

Describe alternatives you have considered

It is not intuitive to change the handling of *_helm_config depending on whether you are using ArgoCD or not.
So I find it difficult to solve this except by passing *_helm_config to ArgoCD.

Additional context

When I wrote aws_load_balancer_controller_helm_config, nothing showed up in the terraform plan.
This only happens when using ArgoCD, because there is no code written to pass *_helm_config to ArgoCD Application resource.
This felt like a bug.
However, there was a possibility that the feature implementation simply had not caught up, so I created a feature request.

aws_load_balancer_controller_helm_config = {
  values = [
    <<-EOT
    podDisruptionBudget:
      maxUnavailable: 1
    EOT
  ]
}

Also, I can pass helm_values by writing as follows, but the current implementation is not sufficient, because all values of awsLoadBalancerController will be lost in terraform's shallow merge.

argocd_applications = {
  addons = {
    path               = "chart"
    repo_url           = local.argocd_addons_repo_url
    add_on_application = true
    values = {
      awsLoadBalancerController = {
        podDisruptionBudget = {
          maxUnavailable = 1
        }
      }
    }
  }
}

Also, isn't it unnatural that our value is not passed when using ArgoCD?

https://github.com/aws-ia/terraform-aws-eks-blueprints/blob/854974d80304d736fce537699f86b8657c5fde98/modules/kubernetes-addons/aws-load-balancer-controller/values.yaml

[kahirokunn]

If you are interested in working on this issue or have submitted a pull request, please leave a comment

This feature has already been implemented: https://github.com/kahirokunn/terraform-aws-eks-blueprints/tree/pass-helm-values

[askulkarni2]

Hi @kahirokunn, when we hand off deployment to ArgoCD we expect that the helm values be passed via gitops otherwise what would be the point of managing addons via gitops. The only values we consider passing via the argocd application are the values for service accounts and other AWS resource ids which cannot other be provisioned via GitOps. See GitOps Bridge for more information.

[kahirokunn]

@askulkarni2

The only values we consider passing via the argocd application are the values for service accounts and other AWS resource ids which cannot other be provisioned via GitOps.

We often want to pass ARNs to the appropriate places in the respective Helm.
For example, we may create an ACM or WAF in Terraform and want to pass the ARNs to Ingress annotations or any other place in yaml.
So I think we should be able to pass helm values.
Also, when passing helm values, they should be deep merged, not shallow merged.

Hardcoding ARNs into the helm repository is not smart.

[kahirokunn]

Since terraform manages the aws resources, terraform should provide the aws resource arn.
If everything should be managed by git ops, then the irsa module should not follow that policy.

[kahirokunn]

It is necessary to interact with the maintainer, but they do not discuss much 😢

[allamand]

Hi @kahirokunn, I understand the need to pass some values from Terraform to the Helm Charts add-on values.

I enable this for External-dns addon using this PR : aws-ia/terraform-aws-eks-blueprints#1035, that only allow to configure flat values And then relying on the GitOps bridge in ArgoCD to set the value in the correct place like this one for external-dns : https://github.com/aws-samples/eks-blueprints-add-ons/blob/main/chart/templates/external-dns.yaml#L16-L25

[kahirokunn]

@allamand LGTM！

[github-actions]

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

[kahirokunn]

keep

[github-actions]

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

[bryantbiggs]

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

[taliesins]

The approach we took was to do a deep merge:
https://registry.terraform.io/providers/cloudposse/utils/latest

data "utils_deep_merge_yaml" "helm_config" {
  input = [
    yamlencode(local.default_helm_config),
    yamlencode(var.helm_config),
    yamlencode(local.addon_helm_values)
  ]
  append_list    = true
  deep_copy_list = true
}