Reuse auth token when upgrading an Helm chart without token

[tigrato]

When upgrading an Helm chat and not providing the auth token because it was previously set, Helm deleted the secret and Statefulset pods become stuck because the secret does not exist.

This PR forces the creation the secret even if no token is provided.

Fixes #20761

[tigrato]

@hugoShaka I followed the following options but didn't like any.

• mark secret mount as optional: For join methods other than token, the secret is required because it defines the token name. We must validate if the mode is install or upgrade and enforce always the token value if the join mode is different than token.
• This approach where I pull the previous auth token from the Kube Secret
• Always create the secret and if users don't provide the auth token or token name we define it as empty.

Let me know your thoughts

[hugoShaka]

@tigrato If I understand correctly, the agent should be able to start because it still has a valid state (either in a PV or in its own secret), but the pods cannot be scheduled because they try to mount the secret containing the token, even if it is not needed and won't be used. We also want to fail fast during deployment if we know it won't work.

I see 3 cases:

• first deploy: we want a token as there's no state
• next joins with join methods issuing renewable certs (token, ec2, iam): we don't need token as there's potentially already a state
• next joins with join methods issuing non-renewable certs (kubernetes, github, circleci): we need tokens as we cannot reuse the existing state indefinitely

I fear lookups because they aren't testable and tend to break Helm's statelessness/idempotency properties.I would tend to prefer your other approach, where you either always create a secret or conditionally create a secret based on the provided values. This would allow to remove the join token from the cluster after installation, which is a nice thing for users relying on static or long-lived tokens.

In both methods, the user experience could be maintained with a couple of fail statements to provide feedback if we know this will not work. I am not convinced either by the optional method as we'll end up with a teleport.yaml config pointing to a non-existent file.

The downside with handling those cases separately is that isUpgrade is not equivalent to "we have a valid state already". I would say this is acceptable for a user to have a sightly longer feedback loop if they messed up the values multiple times. We could improve the user experience by adding --wait to our docs example, so even if the user manages to deploy without a token, Helm will return an error because the pods can't start.

[nklaassen]

you removed if or .Values.authToken .Values.joinParams.tokenName so couldn't these both be null/falsy? What gets put here in that case?

[tigrato]

By default, .Values.authToken is "" so coalesce will write "".

If you override the default value with nil, it will print whatever you define

[hugoShaka]

Could we fail on setups we know that won't work?

Something like

{{- $renewableJoinMethods := list "token" "ec2" "iam" }}
{{- $joinToken := coalesce .Values.joinParams.tokenName .Values.authToken }}
{{- if and (empty $joinToken) (not (has .Values.joinParams.method $renewableJoinMethods)) }}
{{- printf "Non renewable join method %s require a join token, set 'joinParams.tokenName' or 'authToken'" .Values.joinParams.method | fail }}
{{- end }}

[tigrato]

friendly ping @nklaassen