trinodb / charts

Apache License 2.0
151 stars 173 forks source link

Support templated values in coordinator and worker configmaps #217

Closed yardenc2003 closed 2 months ago

yardenc2003 commented 2 months ago

The chart currently does not support using templated .Values, i.e, values which include {{ template "..." . }}, {{ .Files.Get ... }} etc. won't render.

My use case was that I had additionalConfigFiles which were quite lengthy, so I didn't want to include their content in the values file, hence decided to read them from some configs directory instead.

For example, in the values I had:

additionalConfigFiles:
  some-file.txt: |
    {{ .Files.Get "test-file.txt" }}

After I deployed the chart, I found that the file content was simply {{ .Files.Get "test-file.txt" }} (not rendered).

The following changes solve that for both configmap-coordinator.yaml and configmap-worker.yaml, so that you can use some templated values there (added that support only for the values that seem relevant to me). This change can be applied across all the project manifests, but for now I've only applied it on 2.

cla-bot[bot] commented 2 months ago

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

nineinchnick commented 2 months ago

This is hard to reason about without specific examples to show the use cases.

yardenc2003 commented 2 months ago

@nineinchnick To be honest, I used it as a way to load long properties files (could be a 100-line rules.json file) from some configs dir without including their content in the values file itself, via the .Files obj, but then I found a note in Helm docs which says that this won't work once I use it as a subchart, so I guess it's not that useful after all.

Thank you for your time, closing the PR.