aws-load-balancer-controller: keepTLSSecret value not being honoured

not working with Pulumi helm module either

I’m siding with the workaround theory, I don’t want to have to enable cert-manager as it’s more things to think about also this used to just work. It was a recent upgrade at some point that made it break, I know this because we have an older cluster with a old version of this controller installed and it doesn’t hit these issues.

With the recent chart version v1.3.x, we’ve also added support for directly specifying the TLS certificates/keys during chart install. Here is the snippet from the values.yaml:

# webhookTLS specifies TLS cert/key for the webhook
webhookTLS:
  caCert:
  cert:
  key:

This method doesn’t depend on querying the cluster for existing secret, and provide consistent results with the --dry-run or helm template without --validate. Please see if this option is useful in this situation. Note: you need to specify all three attributes - caCert, cert and the key.

Hey @oliviassss @kishorj is there any update on this at all? Is the offical recommended way to do this generate our own secrets and store them inside the cluster or is there a bug that needs addressing? Happy to help out in some way if possible 😃

Same here - helmfile shows differences every time… 😦

The workaround to use cert-manager isn’t working 100% with ArgoCD as it’s showing the two webhooks permanently out of sync.

The problem I think is that the Helm chart is templating in the “empty” Cg== CA bundle, and because the webhooks have the cert-manager annotations added, cert-manager’s own mutating webhooks then replace the CA bundle fields with the correct values. ArgoCD then shows a diff as the original copy of the manifest has Cg== which doesn’t match the live version.

I think the fix here is to not template any CA bundle in at all if using cert-manager, i.e.:

{{ if not $.Values.enableCertManager }}
caBundle: {{ $tls.caCert }}
{{ end }}

That way ArgoCD shouldn’t care when the live version of the manifest has that field, based on another in-house webhook which also uses cert-manager but doesn’t show this permanent diff.

Can confirm this does not work when running helmfile apply.

We have the same issue. I think its this part of code in _helpers.tpl The chart generates the cert if not supplied using genCA+genSignedCert But keepTLSSecret (else if and part) is not used there so at every tf apply with upgrade or argocd refresh a new set is generated causing unavailable time :

{{- $secret := lookup "v1" "Secret" .Release.Namespace $secretName -}}
{{- if (and .Values.webhookTLS.caCert .Values.webhookTLS.cert .Values.webhookTLS.key) -}}
caCert: {{ .Values.webhookTLS.caCert | b64enc }}
clientCert: {{ .Values.webhookTLS.cert | b64enc }}
clientKey: {{ .Values.webhookTLS.key | b64enc }}
{{- else if and .Values.keepTLSSecret $secret -}}
caCert: {{ index $secret.data "ca.crt" }}
clientCert: {{ index $secret.data "tls.crt" }}
clientKey: {{ index $secret.data "tls.key" }}
{{- else -}}
{{- $altNames := list (printf "%s.%s" $serviceName .Release.Namespace) (printf "%s.%s.svc" $serviceName .Release.Namespace) (printf "%s.%s.svc.%s" $serviceName .Release.Namespace .Values.cluster.dnsDomain) -}}
{{- $ca := genCA "aws-load-balancer-controller-ca" 3650 -}}
{{- $cert := genSignedCert (include "aws-load-balancer-controller.fullname" .) nil $altNames 3650 $ca -}}
caCert: {{ $ca.Cert | b64enc }}
clientCert: {{ $cert.Cert | b64enc }}
clientKey: {{ $cert.Key | b64enc }}
{{- end -}}

@bodgit We have the same issue when using the helm chart through tanka and I came to the same conclusion. Have an open PR to add exactly what you are suggesting, https://github.com/kubernetes-sigs/aws-load-balancer-controller/pull/2649.

when you set enableCertManager: true how do you configure keepTLSSecret ? true or false ?

@ajmal-bash when using enableCertManager: true , the keepTLSSecret value is being ignored by the webhook template, see e.g. https://github.com/kubernetes-sigs/aws-load-balancer-controller/blob/c1b95793b0fc3902caf795ac5b5e77fd676aab81/helm/aws-load-balancer-controller/templates/webhook.yaml#L15

My current workaround is to read the values from the aws-load-balancer-tls and specify those for the webhookTLS.* values.

data "kubernetes_secret" "alb-tls" {
  metadata {
    name      = "aws-load-balancer-tls"
    namespace = "kube-system"
  }
  binary_data = {
    "ca.crt"  = ""
    "tls.crt" = ""
    "tls.key" = ""
  }
}

resource "helm_release" "aws-alb-ingress-controller" {
  name       = "aws-load-balancer-controller"
  namespace  = "kube-system"
  chart      = "aws-load-balancer-controller"
  repository = "https://aws.github.io/eks-charts"
  version    = "1.4.0"

  set {
    name  = "webhookTLS.caCert"
    value = base64decode(lookup(data.kubernetes_secret.alb-tls.binary_data, "ca.crt"))
  }

  set {
    name  = "webhookTLS.cert"
    value = base64decode(lookup(data.kubernetes_secret.alb-tls.binary_data, "tls.crt"))
  }

  set {
    name  = "webhookTLS.key"
    value = base64decode(lookup(data.kubernetes_secret.alb-tls.binary_data, "tls.key"))
  }
  ...

Of course, this only works once the chart has been installed, since the secret needs to exist already. A slightly better workaround would be to also generate the TLS secrets with terraform.

@jdomag I haven’t tried it yet (I use helmfile not ArgoCD for deploying this project, nor have tried the 2.3 chart myself yet) but there supposedly is an ArgoCD workaround. See this comment to let ArgoCD ignore changes. Please let us know if this still works or the config needs updating for the latest helm chart.

Feedback on 2.3 - using lookup in the helm templating will not work in many non-vanilla helm scenarios & is probably the cause for the continuing problems (this issue) - please reimplement the fix.

Thanks @yktakaha4 !!! I can confirm this workaround works for helmfile users

As a workaround for this problem, I added the diffArgs attribute to helmfile/helmfile. Can be used with v0.158.0 or higher. https://github.com/helmfile/helmfile/releases/tag/v0.158.0

You can ignore helmfile diff differences by resource, as shown in the example below. https://github.com/helmfile/helmfile/pull/1019#issue-1892758810

in helmfile.yaml

helmDefaults:
  diffArgs:
    - --suppress-secrets
    - --suppress
    - MutatingWebhookConfiguration
    - --suppress
    - ValidatingWebhookConfiguration

Preferably, I’d like to suppress MutatingWebhookConfiguration and ValidatingWebhookConfiguration on a per-attribute basis, but this requires helm-diff to be fixed. https://github.com/databus23/helm-diff/pull/468

Any update/progress on this one?

I’ll delete my fork of the repo and keep an eye on your PR then 😄

@tamirhad thank you, that did the trick! I have to add this config “enableCertManager”: true on the aws load balancer controller

@gitmaniak @tamirhad when you set enableCertManager: true how do you configure keepTLSSecret ? true or false ?