AWS Load Balancer Controller Helm chart option to reuse existing TLS secrets

[ghost]

Reposting to this repo as suggested by @kishorj
Original post from #aws/eks-charts#555

Describe the bug
We use the AWS Load Balancer Controller Helm chart from https://github.com/aws/eks-charts but hit an issue when we use it as a subchart with Helm generated TLS certs for the WebHook https://github.com/aws/eks-charts/blob/master/stable/aws-load-balancer-controller/templates/_helpers.tpl#L78 The TLS certs are regenerated unconditionally on every install/upgrade. Our parent chart creates TargetGroupBinding custom resources, so due to the Helm upgrade order, this is what happens during upgrade:

Existing WebHook is running with the current TLS serving cert
Helm upgrades the WebhookConfiguration first and puts in a new CA
TargetGroupBinding CR is applied
WebHook is triggered due to above, but the CA has been updated, and WebHook validation fails with x509 error (CA doesn’t match the TLS serving certs from the previous install)
Upgrade fails consistently
Steps to reproduce

Expected outcome
Fortunately this is easy to fix, and I’ve attached a patch to it that I put together last night. If the Kubernetes Secret for TLS materials exists and keepTLSSecret variable is set, the Secret is not updated but the existing one is used instead. WebHook CA stays the same, validation succeeds and so does the upgrade. This works with Helm 3.1 onwards where the lookup function was added. The default value of keepTLSSecret, if not set, is false, so this does not change the current behavior of the chart. Can you help us ask upstream if this could be included in a future version please?

Environment

Chart name: AWS Load Balancer Controller Helm chart
Chart version: Helm 3.1+
Kubernetes version:
Using EKS (yes/no), if so version? yes
Additional Context:
diff --git a/stable/aws-load-balancer-controller/templates/_helpers.tpl b/stable/aws-load-balancer-controller/templates/_helpers.tpl
index 4304085..d697680 100644
--- a/stable/aws-load-balancer-controller/templates/_helpers.tpl
+++ b/stable/aws-load-balancer-controller/templates/_helpers.tpl
@@ -77,6 +77,12 @@ Generate certificates for webhook
*/}}
{{- define "aws-load-balancer-controller.gen-certs" -}}
{{- $namePrefix := ( include "aws-load-balancer-controller.namePrefix" . ) -}}
+{{- $secret := lookup "v1" "Secret" .Release.Namespace (printf "%s-tls" $namePrefix) -}}
+{{- 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.%s" $namePrefix "webhook-service" .Release.Namespace ) ( printf "%s-%s.%s.svc" $namePrefix "webhook-service" .Release.Namespace ) -}}
{{- $ca := genCA "aws-load-balancer-controller-ca" 3650 -}}
{{- $cert := genSignedCert ( include "aws-load-balancer-controller.fullname" . ) nil $altNames 3650 $ca -}}
@@ -84,6 +90,7 @@ caCert: {{ $ca.Cert | b64enc }}
clientCert: {{ $cert.Cert | b64enc }}
clientKey: {{ $cert.Key | b64enc }}
{{- end -}}
+{{- end -}}

{{/*
Convert map to comma separated key=value string

[Rahul-D78]

/assign

[Rahul-D78]

I am new to Kubernetes can you please guide me how to fix this bug

[oliviassss]

@Rahul-D78 Hi are you working on this issue currently? If not I can take it, thanks

[oliviassss]

/assign

[jdomag]

@oliviassss
Yesterday I've fetched the latest master of the helm chart and set below value in values.yaml and upgrade my release. Existing pods were not recreated, but I believe they don't need to be. However I noticed today that my argoCD detected the change in TLS secret, so I'm not sure if it works as expected or maybe I should take some extra steps to make a use of your change?

# keepTLSSecret specifies whether to reuse existing TLS secret for chart upgrade
keepTLSSecret: true

[oliviassss]

@jdomag can you check which helm version are you using? if the keepTLSSecret is set to true it should reuse the existing TLS secret.

[kishorj]

@jdomag, could you also run helm template and verify helm generates the correct manifest?

[jdomag]

I use argoCD, not helm directly, however argoCD uses helm v3 as a default as described here https://argo-cd.readthedocs.io/en/stable/user-guide/helm/#helm-version

I see in ArgoCD that manifest was rendered properly setting TLSSecret to true - see screenshot:

[jdomag]

@kishorj @oliviassss
Is it possible that the fix suggested here prevents updating a secret during the helm upgrade, but aws load balancer controller rotate the certificates anyway every X hours? That would explain why argoCD goes out of sync after some time

[kishorj]

@jdomag, the controller does not modify the secrets/rotate certificates. The keepTLSSecret applies during chart upgrades only. We haven't released a new chart with the keepTLSSecret option yet, we plan to do that with the v2.3.0 release. Did you build the helm chart into your own repository?

[jdomag]

@kishorj
Yes I cloned the latest master of the repository and installed the controller using that helm chart.

[kishorj]

keepTLSSecret option is supported in the v2.3.0 release. Closing this issue.

@jdomag, your issue seems to be related to #2312, you can create a new issue if it is something else.