bzed / bzed-dehydrated

Puppet module for centralized CSR signing using Let’s Encrypt™ and lukas2511/dehydrated - keeping your keys safe on the host they belong to.
9 stars 11 forks source link

allow environment adjustments (e.g. PATH) for dehydrated_host_script #3

Closed benaryorg closed 5 years ago

benaryorg commented 6 years ago

The cronjob does not currently take any parameters other than the user. In my specific case ruby, which is required for the cronjob to run, is not installed on a system level and needs PATH adjustments (possible due to #1).

My idea was to pass the environment set here:

https://github.com/bzed/bzed-dehydrated/blob/c1673e6a8e4722fa4eb786ca7e67dd6a23502c9f/manifests/init.pp#L149

As a parameter to the cronjob too:

https://github.com/bzed/bzed-dehydrated/blob/c1673e6a8e4722fa4eb786ca7e67dd6a23502c9f/manifests/setup/dehydrated_host.pp#L101-L105

I don't know about the impact of that specific change, hence the issue (as opposed to a PR). What approach would you recommend? Maybe a second variable? Exposing all of the cronjob parameters as dehydrated::cron, to be as flexible as possible?

bzed commented 6 years ago

My first idea would be to setup the cronjob with

command => "PATH='${path}' ${cron_command}"

as the puppet agents ships with its own ruby interpreter, which should be fine to run the script. Everything else that is needed in the environment for dehydrated to be run can be configured already.

The other option is to make the path to rubu configurable. We could collect that with a fact - RbConfig.ruby should return the ruby interpreter facter uses.

I'd like to avoid having an extra way to pass environment variables as things get confusing then, it is possible to pass default environment variables or variables for every certificate already. If there is something to improve here, let me know :)

benaryorg commented 6 years ago

Getting the path used by factor is actually a pretty good idea, provide that the ruby version used there is compatible with the script provided by you. However I'd prefer a strict call to /usr/bin/env which allows for setting environment variables for a subprocess without relying on shell syntax, which turns out to be a problem with cron:

SYNOPSIS
       env [OPTION]... [-] [NAME=VALUE]... [COMMAND [ARG]...]

Allowing for a convenient:

command => "/usr/bin/env PATH=${path} ${cron_command}"

Where ${path} could be passed through shell_escape.

bzed commented 6 years ago

If your puppet/facter version is compatible with this module, then the ruby version is recent enough. CentOS7 ships with Ruby 2.0 and that works just fine, puppet 4.10 comes with ruby 2.1.9. And there is no special module used in the script. I somehow like the idea of using the ruby version that is used by puppet.

But your suggested way of using env with PATH is also nice... As I won't have the time to work on this for the next three weeks - please implement what you like and send a PR :)