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

What happened to includedirsudoers? #197

Closed esalberg closed 2 months ago

esalberg commented 7 years ago

I ran into an issue where the Augeas lens for sudoers was failing on having entries with numbers, e.g.:

AB123 AB123SRVRS = (AB123USER) AB123STOP, AB123STRT

Those entries appear to be valid, at least per visudo - I was looking for the sudoers lens file to investigate further, when I noticed that the includedirsudoers code no longer appears to be part of the latest version of the module (we're running a bit behind).

On our legacy nodes, we use config_file_replace = false - however, we do need the sudoers.d directory included / added to the config so that we can start adding new entries in sudoers.d.

It looks like includedirsudoers was removed in this PR: https://github.com/saz/puppet-sudo/pull/191

However, the doc wasn't updated to explain how to use the new includedir parameters (if it's possible) to include dirs in a sudoers config file that isn't otherwise managed. The main doc still refers to includedirsudoers as a boolean option, even though it's no longer part of the code. I also don't see anything similar to the previous if $config_file_replace == false that called augeas before.

Is there a way to get the new code to add "#includedir /etc/sudoers.d" to an existing sudoers file with the new code? If not, would it be accepted if I submitted a PR to re-add that functionality?

mdella commented 6 years ago

Just following up with this topic on January 31st (after the release of 5.0.0). This parameter is still listed in the documentation however all my puppet updates are failing now with the error:

Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Resource Statement, Class[Sudo]: has no parameter named 'includedirsudoers' at /etc/puppetlabs/code/environments/production/manifests/sudoers.pp:2:3

In a CentOS7 environment, is this now considered deprecated?

saz commented 6 years ago

This was introduced without further notice with https://github.com/saz/puppet-sudo/commit/ff55b7cc410f6889c238a4c9a29a562b70800a75#diff-60ae41fd0a31977447947f59940ee9a4

I'm sorry for the inconvenience. I'll check the documentation and will update it. Easy way: drop includedirsudoers from your setup and it should be working as expected.

Don't know how I've missed that big change. :disappointed:

jameskirsop commented 6 years ago

Not to hijack this thread too much, but I'm a bit confused about how this should or shouldn't be working.

Given the below:

        sudo::conf { 'sudoers':
            priority => 10,
            content  => "%sudoers ALL=(ALL) NOPASSWD: ALL",
        }

A file is created (on CentOS7) in /etc/sudoers.d/ but the template and options still seem to indicate that this file is ignored by default. Would it not make sense to make sure that this folder is read for drop in files like the one created for the above?

Relevant except from the RHEL7 template file below but the same commented line for the config_dir is found through all others that I checked:

#includedir <%= @config_dir %>
<% @extra_include_dirs.each do |include_dir| -%>
#includedir <%= include_dir %>
<% end if @extra_include_dirs -%>
ubellavance commented 6 years ago

@jameskirsop this # is not a comment in this case. I know it's confusing. By the way, the new default is to have members of the wheel group to be full sudoers so your setting may be redundant.

jameskirsop commented 6 years ago

@ubellavance, that's very confusing!!

I'll have to work out why my test users in the group aren't able to elevate with sudo then!! I thought I'd found something, but obviously not!!

ubellavance commented 6 years ago

No worries, everyone gets bitten once. But this restriction comes from sudo itself, not from this module.

From sudoers manpage:

The #includedir directive can be used to create a sudoers.d directory that the system package manager can drop sudoers file rules into as part of package installation.

gilljb commented 4 years ago

I'm still seeing the error in 6.0.0

class { 'sudo': config_file_replace => false, includedirsudoers => true, }

Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Resource Statement, Class[Sudo]: has no parameter named 'includedirsudoers' (file: /etc/puppetlabs/code/environments/sudo_includedirsudoers/site/csd_base/manifests/access/sudo/add_sudoers.pp, line: 10, column: 3) on node

nicoloba commented 3 years ago

Hi! So, how is the right way to declare #includedir inside the class?

Thanks in advance!