puppetlabs / puppet

Server automation framework and application
https://puppet.com/open-source/#osp
Apache License 2.0
7.39k stars 2.19k forks source link

exec resource: path param should default to the path fact #9440

Open bastelfreak opened 1 month ago

bastelfreak commented 1 month ago

Use Case

For some operations, exec resources are still common. The majority of exec resources I see use hardcoded paths like:

exec { 'nftables_memory_state_check':
  command     => '/bin/echo "reloading nftables"',
  refreshonly => true,
}

This often leads to problems because some distributions use /bin or /usr/bin and others /usr/sbin/. That makes it hard to write the resources for multiple operating systems. because of that I now mostly see this:

exec { 'nftables_memory_state_check':
  command     => 'echo "reloading nftables"',
  refreshonly => true,
  path        => $facts['path'],
}

I think path => $facts['path'], is repeated a lot. Because of that I think the path attribute should default to $facts['path'] when the user does not overwrite it.

Describe the Solution You Would Like

Update path to default to $facts['path'].

Describe Alternatives You've Considered

Using absolute paths or always setting path => $facts['path'],.

Additional Context

joshcooper commented 1 month ago

Ah yeah, this is a common problem on Windows too, because you can't be sure where Windows installed.

AriaXLi commented 1 month ago

Hi @bastelfreak, thank you for reporting this issue. While we agree this is likely an improvement, we do not anticipate addressing this any time soon due to other issues demanding precedence. We may revisit this at a later time.

If you are interested in submitting a patch to implement this issue, please open a pull request and link this issue. I've added the help wanted label in case any contributors could submit a patch.

bastelfreak commented 1 month ago

I implemented it in #9445