Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new functions to the template engine #11974

Closed
karimiehsan90 opened this issue Apr 8, 2023 · 13 comments
Closed

Add new functions to the template engine #11974

karimiehsan90 opened this issue Apr 8, 2023 · 13 comments

Comments

@karimiehsan90
Copy link

I wanna add some functions to the template engine and I want ask your feedback first:
I wanna add the below functions:

  1. join: It gives a list and a string and joins the list with the string. Example: {{ ['a', 'b', 'c'] | join ',' }} to a,b,c.
  2. toMillis, toSeconds and etc: It gives a human readable duration and converts it to milliseconds, seconds and etc. Example: {{ '1h' | toSeconds }} to 3600
  3. toBytes: It gives a human readable size and converts it to bytes. Example: {{ '1 KB' | toBytes }} to 1024

First I wanted to add them into the sprig library, but unfortunately it seems not to be maintained anymore: Masterminds/sprig#365

@gjenkins8
Copy link
Contributor

To note, join already exists: http://masterminds.github.io/sprig/string_slice.html

@karimiehsan90
Copy link
Author

Thank you for your response. I saw that but its usage is not clean yet. I have a Kafka cluster and I wanna set its bootstrap-servers. here is my code:

{{- define "kafka.bootstrap.server" -}}
{{- .Values.kafka.hosts | join (printf ":%d," (.Values.kafka.port | int)) }}:{{ .Values.kafka.port }}
{{- end -}}

What is your idea about adding a new function named append that appends a string to each item of the slice?

With append function it will be:

{{- define "kafka.bootstrap.server" -}}
{{- .Values.kafka.hosts | append (printf ":%d" (.Values.kafka.port | int)) | join "," }}
{{- end -}}

karimiehsan90 added a commit to karimiehsan90/helm that referenced this issue Apr 12, 2023
Add toSeconds function to convert duration to seconds
Add toMilliSeconds function to convert duration to milliseconds

It's related to: helm#11974
karimiehsan90 added a commit to karimiehsan90/helm that referenced this issue Apr 12, 2023
Add toSeconds function to convert duration to seconds
Add toMilliSeconds function to convert duration to milliseconds

It's related to: helm#11974

Signed-off-by: Ehsan Karimi <karimiehsan901@gmail.com>
@joejulian
Copy link
Contributor

Thank you for your response. I saw that but its usage is not clean yet. I have a Kafka cluster and I wanna set its bootstrap-servers. here is my code:

{{- define "kafka.bootstrap.server" -}}
{{- .Values.kafka.hosts | join (printf ":%d," (.Values.kafka.port | int)) }}:{{ .Values.kafka.port }}
{{- end -}}

What is your idea about adding a new function named append that appends a string to each item of the slice?

With append function it will be:

{{- define "kafka.bootstrap.server" -}}
{{- .Values.kafka.hosts | append (printf ":%d" (.Values.kafka.port | int)) | join "," }}
{{- end -}}

You mean like http://masterminds.github.io/sprig/lists.html#append-mustappend ?

Familiarize yourself with the sprig functions. There's a lot you can do with them.

@karimiehsan90
Copy link
Author

@joejulian No I don't mean it.
Suppose this: {{ ["a", "b", "c"] | append "d" }}
The sprig append function result will be: ["a", "b", "c", "d"]
But my function result will be: ["ad", "bd", "cd"]
In fact it's map_append function.

karimiehsan90 added a commit to karimiehsan90/helm that referenced this issue Apr 13, 2023
Add toSeconds function to convert duration to seconds
Add toMilliSeconds function to convert duration to milliseconds

It's related to: helm#11974

Signed-off-by: Ehsan Karimi <karimiehsan901@gmail.com>
@joejulian
Copy link
Contributor

Ah, in the go template language, you'll have to write a loop for that:

{{ $source := list "a" "b" "c" }}
{{ $dest := list }}
{{ range $source }}
{{ $dest = append $dest (printf "%s%s" . "d") }}
{{ end }}

helm-playground example

@joejulian
Copy link
Contributor

Each of these can be solved using named templates and that's probably the best way to handle this. Sprig is maintained, but is very resistant to adding new functions.

Here's an example of SI-to-bytes

@karimiehsan90
Copy link
Author

karimiehsan90 commented Apr 18, 2023

@joejulian I know it is possible now. But this loop will be duplicated for all of the developers and it's a code smell.
Why I should write this loop and add some complexity for this simple problem?

This is my solution for the current helm:

{{- define "kafka.bootstrap.server" -}}
{{- range $i, $host := .Values.kafka.hosts }}
{{- if ne $i 0 }},{{ end }}
{{- $host }}:{{ $.Values.kafka.port }}
{{- end }}
{{- end -}}

And it will be after adding the mapAppend function:

{{- define "kafka.bootstrap.server" -}}
{{- .Values.kafka.hosts | mapAppend (printf ":%d" (.Values.kafka.port | int)) | join "," }}
{{- end -}}

Do you see the difference? The second one is much more readable than the first one.

@joejulian
Copy link
Contributor

Sure. The new function does make it simpler and more readable, but the chance that a new function is being added to helm is very slim. The maintainers have been very clear in the past that if it can be done with the existing tools, then no new functions will be added.

{{- define "kafka.bootstrap.server" -}}
{{- $hosts := list }}
{{- range $host := .Values.kafka.hosts }}
  {{- $hosts = append $hosts (printf "%s:%s" $host .Values.kafka.port) }}
{{- end }}
{{- join "," $hosts }}
{{- end -}}

@joejulian
Copy link
Contributor

Got me wondering what I did, but our structure's different from that:

redpanda-data/helm-charts/main/charts/redpanda/templates/configmap.yaml

      seed_servers:
{{- range (include "seed-server-list" . | mustFromJson) }}
        - host:
            address: {{ . }}
            port: {{ $values.listeners.rpc.port }}
{{- end }}

@karimiehsan90
Copy link
Author

@joejulian I disagree with this paradigm. In my opinion the template engine must be as simple as possible. After 3 years writing Ansible Roles and Helm Charts, I feel I am tied in the Go template engine. In Jinja we have all these functions and Ansible is extensible (Thanks for the python interpreter). But in Helm, this simple common problem must be solved with a loop (and in some cases a condition).
I disagree with one way for each problem. If we think so, we can do all the things with Assembly and Go, Java, Python and other high level languages are spurious. :D

@joejulian
Copy link
Contributor

Well feel free to open a PR. I'm just sharing what I know from past conversations with the maintainers.

@karimiehsan90
Copy link
Author

@joejulian
OK. Thank you for your attention. I have opened a pull request last week for the second function (duration functions): #11986
I think they are in after-release holidays and they didn't review it yet :))

@github-actions
Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants