[stable/spinnaker] Use "tpl" function to render the content of additional scripts

[legal90]

What this PR does / why we need it:

This PR adds tpl fuction into templates for additionalScripts and additionalInitScript.
This allows to use variable interpolation in the body of these scripts. For example:

# custom_values.yaml

halyard:
  additionalScripts:
    create: true
    data:
      00_customize_accounts.sh: |-
        #!/bin/bash
        $HAL_COMMAND config provider kubernetes account edit {{ .Values.kubeConfig.deploymentContext }} \
          --namespaces {{ .Release.Namespace }} \
          --only-spinnaker-managed=true \
          --check-permissions-on-startup=false

Up until now we would have to hard-code the namespace and the account name in the custom script, which is not flexible.

Special notes for your reviewer:

These changes are backward compatible, because tpl works seamlessly with plain strings (which is the expected value there at this moment).
However, since that is a new feature for this chart, I bumped the minor version number.

tpl function is available in Helm >= v2.5.0.

Checklist

• DCO signed
• Chart Version bumped

[k8s-ci-robot]

Hi @legal90. Thanks for your PR.

I'm waiting for a helm member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[legal90]

/assign @viglesiasce

[maorfr]

/assign
/ok-to-test

[legal90]

@maorfr Thanks! Do you know what needs to be done with e2e tests to make this PR passed? I see that the test error is not related to this PR changeset.

[maorfr]

/test pull-charts-e2e

[maorfr]

maybe there is something wrong with the tpl syntax?
look at this for reference:
https://github.com/helm/charts/blob/master/stable/grafana/templates/configmap.yaml#L26

[legal90]

@maorfr Hm.. I don't think so. I verified this tpl syntax from my branch and it works as expected.
But e2e tests are failing with the same error in all other PRs too. For example:

• [stable/spinnaker] Allow service-settings to be extended with kubernetes configuration #11631
• [stable/spinnaker] Add a value for customizing service settings  #13049
• [stable/spinnaker] Allow service-settings to be extended with kubernetes configuration #11631

[legal90]

/test pull-charts-e2e

[maorfr]

/test pull-charts-e2e

[dwardu89]

/lgtm

[dwardu89]

/lgtm cancel

[dwardu89]

Removed LGTM while speaking to @legal90 related to some caveat using the tpl

after switching to `tpl`,  it will blow up if the user has some Go-like stuff in their definitions (`{{  }}`)

That is less-less likely, but some percentage of users might be affected. Let me show the actual example...```

[dwardu89]

@legal90 can you add a note to the README.md to advising users that are using the older chart versions that this change might require some work with a workaround please?

[legal90]

Yup, I will add a note about that. 👍

P.s. The actual example is:

  additionalConfigMaps:
    create: true
    data: 
      artifact-template-helm-chart.jinja: |-
        [
          {
            "name": "{{ properties.chart_name }}",
            "version": "{{ properties.chart_version }}",
            "reference": "{{ properties.helm_account }}",
            "type": "helm/chart"
          }
        ]

Since that is a Jinja template for Spinnaker, it should be dumped to a file as it is. After passing that through tpl it will stop working because a stuff betwee {{ }}will be treated as Go template.

[legal90]

@dwardu89 I have changed this PR and left tpl function calls only for rendering additionalScripts and additionalInitScript bodies.
So, I decided to do not include it to CconfigMaps and Secrets to minimize the risk of potential issues like the one described above.

So, I believe using tpl for scripts will be safe since the chance of having {{ }} there is much less.
Is that OK?

[dwardu89]

/lgtm

[legal90]

I've included the README.md changes from PR #13764, just to don't bump the version twice.

[dwardu89]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dwardu89, legal90

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• stable/spinnaker/OWNERS [dwardu89]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment