Support templates in ingress host

[yahel2410]

Motivation

Added support for templates in ingress host field.

Changes

Run the hosts through tpl function. I had to save the base template in a var, to use it inside the range loop

Testing

Added test values

[alexrashed]

HI @yahel2410
Thanks a lot for the contribution! But I am honestly a bit confused by motivation for the change.
Why would it be necessary to run one var through another? Why not just set the proper value in the first place?

[yahel2410]

Hi @alexrashed, let me elaborate a bit more about our use-case
We use a shared "gitops" repository for all developers. Each developer has it's own values.yaml with his details.
When we deploy localstack we need each ingress host to contain the developer name, so we use the -f flag to override the default values

e.g

my-values.yaml

name: yahel
env: dev

Helm cmd

helm template --debug localstack ./charts/localstack -f my-values.yaml

[alexrashed]

That is understandable, but why don't you just use the value directly in the values file?

ingress:
  enabled: true
  hosts:
    - host: yahel-dev.example.com

[yahel2410]

Along with localstack we deploy other charts that don't always use the same ingress template structure.
For example, some charts only allow one ingress, so the value cannot be set as list like in your chart.

Being able to resuse the name value will keep our values files much skinnier.

[alexrashed]

Can you maybe give us more concrete examples for this? I still don't see the clear value, and this argument can be made for each and every single value in the chart...

[yahel2410]

Sure.
For example, we also use this helm chart
As you can see, they accept the ingress as a single map instead of list.

So would need to hold this 2 blocks in our per dev values file

ingress:
  enabled: true
  host: yahel-dev.example.com                               # for kafka-ui chart
  hosts:
    - host: yahel-dev.example.com                            # for localstack chart

[yahel2410]

Hi @alexrashed, do you think we can merge that? it can help us a lot

[alexrashed]

I took a closer look and we are already using the templating functions for some of the inputs already. I don't want to overboard with that (because as I mentioned you could do this for each and every single value), but your use case seems fine.
I added a few comments to fix your test values and simplify the code. Once the suggestions have been implemented, I'm happy to merge and release your changes.

But since the invalid test values indicate that you did not test your changes upfront: Please make sure to do so. This chart is used by a lot of users, and we don't want to break anything. 😉

[yahel2410]

Thank you for the review and comments @alexrashed.
I fixed them and push the changes to a new commit.

[alexrashed]

Thanks for applying the comments, one more iteration and we should be good to go.

[alexrashed]

Thanks for addressing the comments, now we are good to go!
Thanks again for your contribution, I'll directly move forward and merge + release this change.

[alexrashed]

FYI: These changes are now contained in version 0.6.14 of the chart ;)

[yahel2410]

Amazing. Thanks a lot! @alexrashed