Correct template rendering for authentication options

[MonkeyCanCode]

The Helm chart installation was failing with a type error when processing authentication values. This was caused by the tpl function being incorrectly used on string literals instead of templates.

This commit resolves the issue by removing the unnecessary tpl calls from the polaris.configVolumeAuthenticationOptions helper template.

Sample error reference:

➜  polaris git:(main) ✗ helm upgrade --install --namespace polaris \
  --values helm/polaris/ci/persistence-values.yaml \
  polaris helm/polaris
Release "polaris" does not exist. Installing it now.
Error: template: polaris/templates/deployment.yaml:130:12: executing "polaris/templates/deployment.yaml" at <include "polaris.configVolume" .>: error calling include: template: polaris/templates/_helpers.tpl:179:10: executing "polaris.configVolume" at <include "polaris.configVolumeAuthenticationOptions" (list "" .Values.authentication)>: error calling include: template: polaris/templates/_helpers.tpl:351:29: executing "polaris.configVolumeAuthenticationOptions" at <.>: wrong type for value; expected chartutil.Values; got []interface {}

[dimas-b]

I'll defer to @adutra for reviewing this change... maybe there was a use case for tpl, of which I'm not aware 🤔

[adutra]

Hi @MonkeyCanCode thanks for spotting this issue!

I don't think the root cause is the usage of tpl. The issue seems to be with the . parameter. In this context, it does not point to the global .Values object, that's why the template rendering is failing.

I think that a more correct approach would be to pass the global .Values as a parameter to this function, just like we do in polaris.authenticationOptions. Something like:

{{- define "polaris.configVolumeAuthenticationOptions" -}}
{{- $realm := index . 0 -}}
{{- $auth := index . 1 -}}
{{- $global := index . 2 -}}
...
- secret:
    name: {{ tpl $secretName $global }}

The choice of templetizing every value exposed in the chart is a bit opinionated, but at the same time it allows for a lot of flexibility when defining values. For example, the secret name could be defined as:

authentication.tokenBroker.secret.name: {{ .Release.Namespace }}-auth-secret

I don't mind having a broader discussion about whether it makes sense to templetize everything, but here, my feeling is that we are trying to fix a wrong template function, so imho we shouldn't remove the tpl function call, but instead fix its invocation.

WDYT?

[MonkeyCanCode]

Hi @MonkeyCanCode thanks for spotting this issue!

I don't think the root cause is the usage of tpl. The issue seems to be with the . parameter. In this context, it does not point to the global .Values object, that's why the template rendering is failing.

I think that a more correct approach would be to pass the global .Values as a parameter to this function, just like we do in polaris.authenticationOptions. Something like:

{{- define "polaris.configVolumeAuthenticationOptions" -}}
{{- $realm := index . 0 -}}
{{- $auth := index . 1 -}}
{{- $global := index . 2 -}}
...
- secret:
    name: {{ tpl $secretName $global }}

The choice of templetizing every value exposed in the chart is a bit opinionated, but at the same time it allows for a lot of flexibility when defining values. For example, the secret name could be defined as:

authentication.tokenBroker.secret.name: {{ .Release.Namespace }}-auth-secret

I don't mind having a broader discussion about whether it makes sense to templetize everything, but here, my feeling is that we are trying to fix a wrong template function, so imho we shouldn't remove the tpl function call, but instead fix its invocation.

WDYT?

@adutra thanks for the review. I am fine with making everything templatize. Updated the PR with the fix. Please take another look when you have time.