Support "environment-vars" setting in puppetserver.conf#806
Support "environment-vars" setting in puppetserver.conf#806ekohl merged 2 commits intotheforeman:masterfrom
Conversation
Environment variables can now be made visible to the puppetserver process. Fixes theforemanGH-792
|
@ekohl Is it possible to get this reviewed? |
ekohl
left a comment
ekohl
left a comment
There was a problem hiding this comment.
Overall this looks sane. Some small implementation details inline.
| Optional[Variant[String,Array[String]]] $server_jvm_extra_args = $puppet::params::server_jvm_extra_args, | ||
| Optional[String] $server_jvm_cli_args = $puppet::params::server_jvm_cli_args, | ||
| Optional[Stdlib::Absolutepath] $server_jruby_gem_home = $puppet::params::server_jruby_gem_home, | ||
| Hash[String, String] $server_environment_vars = $puppet::params::server_environment_vars, |
There was a problem hiding this comment.
Thinking out loud: Do we also want to accept Integer here? Users may write in hiera:
puppet::server_environment_vars:
myvar: 1I think in Puppet this ends up as an Integer unless they write "1".
There was a problem hiding this comment.
I was thinking from a technical perspective that environment variables are always strings by nature. But we can allow more types if you like and rely on implicit conversion during templating.
| # or jruby-puppet.gem-home. | ||
| environment-vars: { | ||
| <%- @server_environment_vars.each do |env_key, env_val| -%> | ||
| "<%= env_key %>" : <%= env_val %> |
There was a problem hiding this comment.
In case of the variable name containing dots it is necessary: https://github.com/lightbend/config/blob/main/HOCON.md#paths-as-keys
There was a problem hiding this comment.
I wonder if we should then always quote it, just to be safe.
There was a problem hiding this comment.
I am not sure I can follow. In this code the keys are already always quoted. Or do you mean every other key in HOCON files managed by this module? We only need to do this if keys are defined by user input.
There was a problem hiding this comment.
I'm talking about env_val. Where env_key is quoted, env_val isn't. That sounds like a potential problem.
There was a problem hiding this comment.
The more I read about HOCON the more complicated it gets. My use case is to set the following
...
environment-vars: {
"FOO" : ${FOO}
}
...${FOO} will be replaced by the corresponding environment variable making it possible to pass through variables set in /etc/default/puppetserver. Quoting it in any way prevents substitutions from being evaluated.
So I guess we somehow must let the user decide whether to quote the value or not. So how about this:
We render the value verbatim into the config and adapt the docs to show that the following is possible:
puppet::server_environment_vars:
myvar: 'some string'
myvar_from_env: '${myvar_from_env}'
myvar_quoted: '"somestring with forbidden chars $[]{}"'
myvar_multiline: |-
"""this
is
a
multi-line
string
"""Would that be ok?
There was a problem hiding this comment.
I finally found the time to improve the variable doc comment. Can you take a look?
|
Hi guys, I need this feature too. It would be really great if it continues here. Thanks and greetings |
4 participants
Environment variables can now be made visible to the puppetserver process.
Fixes GH-792