Closed canihavethisone closed 1 year ago
Hi @canihavethisone thank you for raising this one. We've merged a fix for this issue into main, and we will be cutting a minor release shortly so that the fix is available on the forge. I'll update this issue once the fix is LIVE on the forge. Thanks for your patience with this one while we figured it out.
@jordanbreen28 cheers! Moot point, but i see the solution used a rather cleaver contain
that restores the use of which
. My research suggested that command
is a preferred way to check for the presence of a binary across linux versions. Curious if this was considered or if, as i say, its a bit moot.
Thanks again.
Hi @canihavethisone v4.0.1 is now live on the forge and should have resolved this issue. The information you provided in the issue formed the basis of the solution that was implemented i.e. using which to determine the presence of a binary.
We found that confining the fact to only systems where the binary was present was better route to go, as was mentioned in this comment .
Thank you again for raising this and bringing it to our attention! (I'll keep this issue open a little while longer to give you a chance to test the new release)
i see the solution used a rather cleaver
contain
that restores the use ofwhich
. My research suggested thatcommand
is a preferred way to check for the presence of a binary across linux versions. Curious if this was considered or if, as i say, its a bit moot.
You are correct that in bash scripts, command
is prefered over which
(which is really a csh
feature). However, in Facter's Ruby library, the name of the function to look for a binary is simply called which
. The implementation of it is different on Windows and Linux, but in each case it actually iterates over the paths in Ruby code. Older versions of Puppet/Facter actually called the shell command which
, which was awful, since this forked first /bin/sh, then /bin/csh, but this is some time ago.
(end of history lesson :) )
Awesome! I didn't realise in this case the which
is leveraging native ruby, not shell. Thanks for that lesson!
I guess its a case of which which
is which? :)
I'm gonna go ahead and close this issue @canihavethisone now that the fix has been released. Please do reopen if you find this problem to still persist :) Thanks again for raising this!
Describe the Bug
The recent change in v4.0.0 the use updated exec formatting ie
version = Facter::Core::Execution.execute('ip6tables --version')
andversion = Facter::Core::Execution.execute('iptables --version')
... attempts to execute on systems where the binaries are not present - the old format of
Util::Resolution.exec
did not appear to have this issue.This is an error generating issue when this module is present on servers that are also servicing clients without iptables as the fact attempts to execute. For example, a server that has both the
puppetlabs-firewall
module for CentOS7 hosts and thepuppet-nftables
module for CentOS8/9 hosts, these new facts will raise errors on the CentOS8/9 hostsExpected Behavior
The facts should not error. Ideally, they should only execute if the iptables and ip6tables binaries are present
Steps to Reproduce
Steps to reproduce the behavior:
Environment
Additional Context
One (possibly crude) way to fix the exec while preserving the new format is to first test for the binary within the exec thus:
version = Facter::Core::Execution.execute('command -v iptables &> /dev/null && iptables --version')
There is probably a nicer way to handle this however. Alternatively, consider reverting to the
Util::Resolution.exec
used previously as per it's doco it handles the absence of a binary better as it runs an initialwhich
before attempting to execute (that could also be used above instead ofcommand
, however it seems we are now using an alternate ruby function without net gain).https://www.rubydoc.info/gems/facter/1.5.8/Facter%2FUtil%2FResolution.exec: