8000 Restore env and expandenv. by mck-iain · Pull Request #46 · noqcks/gucci · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Restore env and expandenv. #46

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

Merged
merged 2 commits into from
Oct 24, 2022
Merged

Restore env and expandenv. #46

merged 2 commits into from
Oct 24, 2022

Conversation

mck-iain
Copy link
Contributor

Although the ability to reference environment variables with the {{ .VARNAME }} syntax is useful, it throws when VARNAME is not defined.

It is sometimes useful to have the shell behaviour of an undefined environment variable expanding to the empty string.

For instance, when FOO is undefined, template It is set to "{{ .FOO }}" throws map has no entry for key "FOO" when it might be desirable for it to expand to It is set to "" instead.

With env, the above is possible. It is set to "{{ env "FOO" }}" works as expected.

#14 added Sprig functions, which is nice, and removed env and expandenv with the justification that "Helm doesn't have them" but I think they are useful enough to keep.

@noqcks
Copy link
Owner
noqcks commented Oct 24, 2022

I agree here and think we should add them back. In the case of Helm, these methods have been removed because of the security implications of running a 3rd party chart in which the chart author could have access to Tiller's environment. And since we already parse environment variables, and have since release 1.0.0 of gucci, I think it's fine to add these sprig methods.

@noqcks noqcks merged commit b882345 into noqcks:master Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0