MonkeyCanCode commented on PR #2808:
URL: https://github.com/apache/polaris/pull/2808#issuecomment-3424638257
> 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:
>
> ```yaml
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
