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

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

Open
benaryorg opened this Issue Nov 15, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@benaryorg
Contributor

benaryorg commented Nov 15, 2018

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:

Hash $dehydrated_environment = $::dehydrated::params::dehydrated_environment,

As a parameter to the cronjob too:

cron { 'dehydrated_host_script':
command => $cron_command,
user => $::dehydrated::dehydrated_user,
minute => [3,18,33,48,]
}

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

This comment has been minimized.

Owner

bzed commented Nov 15, 2018

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

This comment has been minimized.

Contributor

benaryorg commented Nov 15, 2018

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

This comment has been minimized.

Owner

bzed commented Nov 15, 2018

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment