Add function to safely check variable existence in templates

[phillipj]

Howdy @tazjin!

These changes adds the definedOrEmpty template function which allows templates to get values from possibly non-existent variables in a safe manner.

If no variable exists with the name provided, an empty string will be returned. This makes it a good fit for combining with sprig's default function:

{{ definedOrEmpty "version" | default gitHEAD }}

Would either result in the version variable's value, or if it has not been declared, would use the result of gitHEAD function instead.

Any thoughts?

P.S. I'm not opinionated about the function name.

Refs #140 (comment)

[tazjin]

Is there a use-case other than defaulting? The issue with the current default function is that it is broken by the template engine setting causing things to fail on nil-values, maybe this function should instead be a better default?

[phillipj]

Nothing else than defaulting comes to mind, no.

You're thinking of overriding sprig's default with the function I'm suggesting in the current changeset?

Meaning something like:

--- a/templater/templater.go
+++ b/templater/templater.go
@@ -203,7 +203,7 @@ func templateFuncs(c *context.Context, rs *context.ResourceSet) template.FuncMap

                return string(data), nil
        }
-       m["definedOrEmpty"] = func(varName string) string {
+       m["default"] = func(varName string) string {
                if val, ok := rs.Values[varName]; ok {
                        return fmt.Sprint(val)
                }

[phillipj]

Forgot to mention that my understanding of the underlying problem here, is not failing on nil-values, but variables that is not even part of the variables map when templating.

The sprig docs explicitly gives the impression that default handles nil as "an empty value": http://masterminds.github.io/sprig/defaults.html#default

[tazjin]

You're thinking of overriding sprig's default with the function I'm suggesting in the current changeset?

Yep!

but variables that is not even part of the variables map when templating

You're right, that's what I meant - having a slow brain day in the aftermath of Underwater Pub's last opera night yesterday ;-)

[phillipj]

Alrighty, so overriding the default function with a safe one could be a nice feature..

The only challenge I'm seeing is for this new default function to know the difference between an ordinary string, and when we want to reference variable by its name with a string.

A couple of examples to illustrate what I'm getting at. Here sprig's default explodes because we reference a version variable that is non-existent:

{{ default gitHEAD .version }}

> kontemplate: error: Error templating resource sets: Error while templating my-awesome-app/config.yaml: template: config.yaml:14:96: executing "config.yaml" at <.version>: map has no entry for key "version"

Now if we were to override this default function with the functionality suggested in this PR:

{{ default gitHEAD "version" }}

How would we know that "version" in this use-case is meant to be a variable name lookup, rather than a good old string with "version" as value? 🤷‍♂️

[tazjin]

Using default on a string literal wouldn't make sense because it's always non-nil, right? So we can just say that default's second argument always refers to a variable.

[phillipj]

Very good point indeed! I'll give it a go later and see how it turns out.

…

On Fri, 29 Jun 2018 at 17:19, Vincent Ambo ***@***.***> wrote: Using default on a string literal wouldn't make sense because it's always non-nil, right? So we can just say that default's second argument *always* refers to a variable. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#148 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABLLEy7CFeeRFpHjTTeeWTu1y2UPiVpDks5uBkWBgaJpZM4U6b4_> .

[phillipj]

That worked like a charm, great suggestion 👍

{{ default gitHEAD "version" }}

For sure easier to grasp than my initial suggestion, and covers my use-case perfectly.

Took some inspiration from sprig's default implementation, mostly interface{} vs string as arguments and return value.

Pushed a second commit making it easier to see the actual changes made compared to the first approach -- probably squashable before merging tho.

[tazjin]

FYI, I'm on vacation atm - hence a little bit of delay :)

[tazjin]

Thanks!

[tazjin]

This change is included in version 1.7.0 :)

[phillipj]

Hooray, thanks for cutting a new version! 🎉

[jbell-shibumi]

I think I'm encountering the issue that this PR discusses / fixes, and I'm not sure how to use it correctly ...

With a map with subitems, (say, .config.value), this doesn't seem to work for me...

{{ $var := default "missing" .config }} 

If .config doesn't exist, $var is not set to "missing" but instead, I get an error...

map has no entry for key .config

[phillipj]

I would try passing in your last argument as a string and without the dot prefix: {{ default "missing" "config" }}

Does that work by any chance?

[tazjin]

@jbell-shibumi The default function attempts to access fields via string indexes, which have to be specified as strings. Would you mind filing a separate issue for this being unclear so I can track it as a documentation problem?