Making sudoers for puppetrun_cmd conditional

[lazyfrosch]

Not yet done, but to be the base of discussion.

It would be a good idea to only setup the sudo command if puppetrun_provider is actually puppet. Which is not set by default.

I'd like to get the opinion of the team.

intend to fix #269

[lazyfrosch]

Opinions? anybody?

[ekohl]

I think that it's a good idea.

Not sure how the exact implementation is going to look, but I was thinking about a base class (foreman_proxy::sudo) and something that concats the rules together.

[domcleal]

Indeed, I definitely agree that the puppetrun command should only be in the sudo rules when that provider's active.

Refactoring can be done separately, if needed at all.

[lazyfrosch]

My updated commit does the following in Augeas:

set /files/etc/sudoers/spec[user = 'foreman-proxy'][1]/user foreman-proxy
set /files/etc/sudoers/spec[user = 'foreman-proxy'][1]/host_group/host ALL
set /files/etc/sudoers/spec[user = 'foreman-proxy'][1]/host_group/command '/usr/bin/puppet cert *'
set /files/etc/sudoers/spec[user = 'foreman-proxy'][1]/host_group/command/runas_user root
set /files/etc/sudoers/spec[user = 'foreman-proxy'][1]/host_group/command/tag NOPASSWD
rm /files/etc/sudoers/spec[user = 'foreman-proxy'][1]/host_group/command[position() > 1]

set /files/etc/sudoers/spec[user = 'foreman-proxy'][2]/user foreman-proxy
set /files/etc/sudoers/spec[user = 'foreman-proxy'][2]/host_group/host ALL
set /files/etc/sudoers/spec[user = 'foreman-proxy'][2]/host_group/command '/usr/bin/puppet kick *'
set /files/etc/sudoers/spec[user = 'foreman-proxy'][2]/host_group/command/runas_user puppet
set /files/etc/sudoers/spec[user = 'foreman-proxy'][2]/host_group/command/tag NOPASSWD
rm /files/etc/sudoers/spec[user = 'foreman-proxy'][2]/host_group/command[position() > 1]

rm /files/etc/sudoers/spec[user = 'foreman-proxy'][position() > 2] # deletes all other rules for foreman proxy

set /files/etc/sudoers/Defaults[type = ':foreman-proxy']/type :foreman-proxy
set /files/etc/sudoers/Defaults[type = ':foreman-proxy']/requiretty/negate ''

Not sure if we should do the last rm, any opinions?

[ekohl]

Not an expert on augeas but can't you start with an rm for all and then add everything back? That would make it easier to extend.

[lazyfrosch]

@ekohl fixing specs, than you can see the template

[lazyfrosch]

Updated the change, please review.

The Augeas part has been refactored so it is easier to understand (I hope)

[lazyfrosch]

Please review, waiting on you guys 😉 🎐

[mmoll]

I can't say too much about the augeas part, but at least in my install, the result seems correct.

[domcleal]

Changes all look fine to me too, untested at the moment. Please also remove the "WIP" from the commit message & title if it's ready to be reviewed/merged.

[domcleal]

I don't think the templates should do this, it should probably be limited in what it manages as sudoers might be maintained by further Augeas resources - the point of this method I think is to allow the user to customise the sudoers file in addition to using this module.

[lazyfrosch]

Removed TODO there, ready to merge

@domcleal It was marked WIP due to the TODO you commented on 😉

[domcleal]

Works well, thanks. Tests will currently error due to a regression in stdlib.

[mmoll]

merged, danke @lazyfrosch!