envsubst: remove explicit lists of vars #818
Conversation
For nginx config, a new approach is implemented with perl. This is because unrestricted use of `envsubst` on nginx config files will replace nginx variables like $host, $request_uri with an empty string. Closes getodk#473
| nginx_envsubst() { | ||
| # Re-implementation of envsubst which is safe to call on nginx config files. | ||
| # This fn only substitutes variables in the form ${CAPS_AND_UNDERSCORES}, | ||
| # allowing nginx variables like $host, $request_uri etc. through unmodified. | ||
| perl -pe 's/\$\{([A-Z_]*)\}/$ENV{$1}/g' | ||
| } |
There was a problem hiding this comment.
So, this silently replaces with a 0-length string any variable in the template which does not exist in the environment. Envsubst does that too, so that's the original sin: it makes for hard(er) to debug problems. Depending on the config language and construct it might well make for a syntactically valid config file with an undesirable effect at runtime, not even at initalization time, of the program using that configfile.
See
$ echo -e 'hello\n${I_WILL_BE_SILENTLY_DISCARDED}\n${SHELL}' | perl -pe 's/\$\{([A-Z_]*)\}/$ENV{$1}/g'
hello
/bin/zsh
I think it would be better if it would simply bomb out if the key is not in the env (in Python os.environ['blabla'] would raise a KeyError). At least then it'd be very obvious that something is missing, and what that thing is, right at the moment when it is generated, rather than that something mysteriously goes wrong at runtime.
There was a problem hiding this comment.
I think handling missing vars would be a much better aim than trying to remove the explicit list of vars 👍
For nginx config, a new approach is implemented with perl. This is because unrestricted use of
envsubston nginx config files will replace nginx variables like $host, $request_uri with an empty string.Closes #473
What has been done to verify that this works as intended?
CI.
Why is this the best possible solution? Were any other approaches considered?
Discussed on #473.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
No expected effect.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No.
Before submitting this PR, please make sure you have:
nextbranch OR only changed documentation/infrastructure (masteris stable and used in production)