[kube-prometheus-stack] Issue #333

[ryan-dyer-sp]

Allow additional templating within values.yaml file

Signed-off-by: Ryan Dyer ryan-dyer-sp@users.noreply.github.com

What this PR does / why we need it:

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

• fixes [kube-prometheus-stack] Missing templating for some fields within values.yaml #333

Special notes for your reviewer:

@bismarck @vsliouniaev @gianrubio @gkarthiks @scottrigby @Xtigyro For review please.

Checklist

• DCO signed
• Chart Version bumped
• Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

[bismarck]

After reading helm/helm#2492 (comment), this PR is a no for me.

[ryan-dyer-sp]

After reading helm/helm#2492 (comment), this PR is a no for me.

Can you provide additional info other than see Issue that has been going on for 3 years?

The use of the tpl function in order to template values.yaml files is documented here: https://helm.sh/docs/howto/charts_tips_and_tricks/ .

It is also already used in this chart in dozens of locations already:

> kube-prometheus-stack/templates > grep -r tpl .
./grafana/servicemonitor.yaml:{{ tpl (toYaml .Values.grafana.serviceMonitor.metricRelabelings | indent 6) . }}
./grafana/configmaps-datasources.yaml:{{ tpl (toYaml .Values.grafana.additionalDataSources | indent 4) . }}
./prometheus-operator/servicemonitor.yaml:{{ tpl (toYaml .Values.prometheusOperator.serviceMonitor.metricRelabelings | indent 6) . }}
./exporters/node-exporter/servicemonitor.yaml:{{ tpl (toYaml .Values.nodeExporter.serviceMonitor.metricRelabelings | indent 4) . }}
./exporters/kube-controller-manager/servicemonitor.yaml:{{ tpl (toYaml .Values.kubeControllerManager.serviceMonitor.metricRelabelings | indent 4) . }}
./exporters/core-dns/servicemonitor.yaml:{{ tpl (toYaml .Values.coreDns.serviceMonitor.metricRelabelings | indent 4) . }}
./exporters/kube-dns/servicemonitor.yaml:{{ tpl (toYaml .Values.kubeDns.serviceMonitor.dnsmasqMetricRelabelings | indent 4) . }}
./exporters/kube-dns/servicemonitor.yaml:{{ tpl (toYaml .Values.kubeDns.serviceMonitor.metricRelabelings | indent 4) . }}
./exporters/kube-state-metrics/serviceMonitor.yaml:{{ tpl (toYaml .Values.kubeStateMetrics.serviceMonitor.metricRelabelings | indent 4) . }}
./exporters/kube-etcd/servicemonitor.yaml:{{ tpl (toYaml .Values.kubeEtcd.serviceMonitor.metricRelabelings | indent 4) . }}
./exporters/kube-api-server/servicemonitor.yaml:{{ tpl (toYaml .Values.kubeApiServer.serviceMonitor.metricRelabelings | indent 6) . }}
./exporters/kube-scheduler/servicemonitor.yaml:{{ tpl (toYaml .Values.kubeScheduler.serviceMonitor.metricRelabelings | indent 4) . }}
./exporters/kubelet/servicemonitor.yaml:{{ tpl (toYaml .Values.kubelet.serviceMonitor.metricRelabelings | indent 4) . }}
./exporters/kubelet/servicemonitor.yaml:{{ tpl (toYaml .Values.kubelet.serviceMonitor.cAdvisorMetricRelabelings | indent 4) . }}
./exporters/kubelet/servicemonitor.yaml:{{ tpl (toYaml .Values.kubelet.serviceMonitor.probesMetricRelabelings | indent 4) . }}
./exporters/kubelet/servicemonitor.yaml:{{ tpl (toYaml .Values.kubelet.serviceMonitor.resourceMetricRelabelings | indent 4) . }}
./exporters/kubelet/servicemonitor.yaml:{{ tpl (toYaml .Values.kubelet.serviceMonitor.metricRelabelings | indent 4) . }}
./exporters/kubelet/servicemonitor.yaml:{{ tpl (toYaml .Values.kubelet.serviceMonitor.cAdvisorMetricRelabelings | indent 4) . }}
./exporters/kubelet/servicemonitor.yaml:{{ tpl (toYaml .Values.kubelet.serviceMonitor.resourceMetricRelabelings | indent 4) . }}
./alertmanager/servicemonitor.yaml:{{ tpl (toYaml .Values.alertmanager.serviceMonitor.metricRelabelings | indent 6) . }}
./alertmanager/ingressperreplica.yaml:              - path: {{ tpl $p $ }}
./alertmanager/ingress.yaml:    - host: {{ tpl $host $ }}
./alertmanager/ingress.yaml:          - path: {{ tpl $p $ }}
./alertmanager/ingress.yaml:          - path: {{ tpl $p $ }}
./alertmanager/alertmanager.yaml:  externalUrl: "http://{{ tpl (index .Values.alertmanager.ingress.hosts 0) $ }}{{ .Values.alertmanager.alertmanagerSpec.routePrefix }}"
./alertmanager/secret.yaml:{{- if .Values.alertmanager.tplConfig }}
./alertmanager/secret.yaml:  alertmanager.yaml: {{ tpl (toYaml .Values.alertmanager.config) . | b64enc | quote }}
./prometheus/additionalScrapeConfigs.yaml:  additional-scrape-configs.yaml: {{ tpl (toYaml .Values.prometheus.prometheusSpec.additionalScrapeConfigs) $ | b64enc | quote }}
./prometheus/rules-1.14/kubernetes-system-kubelet.yaml:        runbook_url: {{ .Values.defaultRules.runbookUrl }}alert-name-kubeletplegdurationhigh
./prometheus/ingressThanosSidecar.yaml:    - host: {{ tpl $host $ }}
./prometheus/ingressThanosSidecar.yaml:          - path: {{ tpl $p $ }}
./prometheus/ingressThanosSidecar.yaml:          - path: {{ tpl $p $ }}
./prometheus/prometheus.yaml:  externalUrl: "{{ tpl .Values.prometheus.prometheusSpec.externalUrl . }}"
./prometheus/prometheus.yaml:  externalUrl: "http://{{ tpl (index .Values.prometheus.ingress.hosts 0) . }}{{ .Values.prometheus.prometheusSpec.routePrefix }}"
./prometheus/prometheus.yaml:{{ tpl (toYaml .Values.prometheus.prometheusSpec.podMetadata) $ | indent 4 }}
./prometheus/servicemonitor.yaml:{{ tpl (toYaml .Values.prometheus.serviceMonitor.metricRelabelings | indent 6) . }}
./prometheus/ingressperreplica.yaml:              - path: {{ tpl $p $ }}
./prometheus/ingress.yaml:    - host: {{ tpl $host $ }}
./prometheus/ingress.yaml:          - path: {{ tpl $p $ }}
./prometheus/ingress.yaml:          - path: {{ tpl $p $ }}
./prometheus/ingress.yaml:{{ tpl (toYaml .Values.prometheus.ingress.tls) $ | indent 4 }}

Looking at the above, I see that the changes I suggest dont follow the same format as others and I'm happy to make them look the same. Or if there is an alternate solution that could be suggested, I'll work on it.

[bismarck]

@vsliouniaev Care to chime in?

I don't think it's wise to have templating in values.yaml. As stated in the issue linked:

Templating values files introduces non-YAML syntax into a YAML file.

[ryan-dyer-sp]

Any update? Though I can appreciate the opinion, it doesnt appear to be an opinion that is officially supported by helm given their support for the tpl function and documentation on how to use it. And as mentioned this isnt something that doesnt already exist within the repo in multiple places.

[smlx]

I don't think it's wise to have templating in values.yaml.

I think there's some confusion here. This is adding template interpolation of values within template files, not templating values.yaml.

[vsliouniaev]

@bismarck I'm ok with this, since it's just use of the tpl function.

lgtm

[bismarck]

@ryan-dyer-sp Could you bump the chart version one more time?

[ryan-dyer-sp]

@bismarck To 11.1.5? I'm showing .3 to be the most recent.

[bismarck]

@ryan-dyer-sp 11.1.4 is the most recent. Could you bump to .5?