saz / puppet-sudo

Manage sudo with Puppet on Debian-, RedHat- and SUSE-based linux distributions and some BSDs
Other
107 stars 215 forks source link

'versioncmp' parameter 'a' expects a String value, got Undef #279

Open cdenneen opened 2 years ago

cdenneen commented 2 years ago

Looks like these are still valid issues:

https://github.com/saz/puppet-sudo/issues/264 https://github.com/saz/puppet-sudo/issues/260

cdenneen commented 2 years ago

Module Version 7.0.2 Puppet Version 6.25.1 CentOS 7

nevesigor commented 2 years ago

Hello,

This is important, it's broken on all RedHat family at least, since puppet 4 versioncmp function only accepts strings.

Centos: 7.9 Puppet Agent: 7.x

Change line 87 on puppet master from: if (versioncmp($::sudoversion, '1.7.2p1') < 0)

To: if (versioncmp("$::sudoversion", '1.7.2p1') < 0)

dwest-galois commented 2 years ago

I just ran into this issue this past week and found a fix/workaround that might help others before it gets fixed in the sudo module. This was all done with CentOS7, so I am not sure if it would be prevalent in other versions. In the following rb file, it looks like the sudoversion fact is created by running sudo -V 2>&1

https://github.com/saz/puppet-sudo/blob/master/lib/facter/sudoversion.rb

Whenever I tried to run that command manually, I got the following: # sudo --version /var/tmp/scl7Hipfg: line 8: --version: command not found So the module is not getting a valid string when looking up the sudo version. In this instance, the issue is instigated by the version of sudo I have installed. Well, more specifically, a wrapper script that came with devtoolset-7 and was first in my PATH: /opt/rh/devtoolset-7/root/usr/bin/sudo This is known to cause issues with parsing sudo options and has been documented in a few different bug reports, such as:

https://bugzilla.redhat.com/show_bug.cgi?id=1319936

So if you run into errors with the sudo module, check the PATH for sudo to see which one it is using. The fix for this is to either remove /opt/rh/devtoolset-7/root/usr/bin from your PATH or at least prepend /usr/bin/sudo to it. This only needs to happen for the root user since Puppet needs to run as root.

I hope this helps others save some time.

saz commented 1 year ago

@kilianw can you check, if you're seeing the same as reported by @dwest-galois ? Tests are working fine on CentOS 7 and I'm confused, how's that possible.

kilianw commented 1 year ago

@saz - My testing has been successful on CentOS 7, Alma 8 and Ubuntu 20.

The redhat devtoolset isn't installed.

saz commented 1 year ago

As this issue is related to some additional package on RedHat, it's not helping to fix the issue by just converting the fact to a string.

Instead, I'd rather change the fact:

  1. If we find sudo as command, check if the full path contains /devtoolset-
  2. If it's the case, prepend /usr/bin/sudo, instead of just using sudo
  3. Maybe the check should be done on RedHat only?

This should keep the fact compatible on all systems and fix it, if the devtools-7 package is installed.

saz commented 1 month ago

Is this change still needed? :thinking: