Adding $use_sudoers bool #321
Conversation
…not the module would manage /etc/sudoers.
…sd and the addition of use_sudoers.
| $use_sudoersd = true | ||
|
|
||
| # Add contents to /etc/sudoers (true). Defaults to false. | ||
| $use_sudoers = false |
There was a problem hiding this comment.
This should default to true, so that the current behaviour of setting $use_sudoersd to false still works as intended. A default of false will change that so it suddenly no longer adds any entry. Both should be disabled manually to get your preferred behaviour of no sudoers management.
Nitpick: delete the second blank line below here too.
There was a problem hiding this comment.
Makes sense to me. Pushed a new commit with those changes.
… current default behavior.
| end | ||
| end | ||
|
|
||
| context 'when puppetrun_provider == puppetrun' do | ||
| context 'when use_sudoers => true' do |
There was a problem hiding this comment.
This is the same as the parent context I think? (use_sudoersd => false, but use_sudoers => true - now the default)
It's a little confusing having it twice and moving tests inside this context.
There was a problem hiding this comment.
Ah, yeah, I think I see what you're saying. It doesn't help that git diff makes this look really odd.
The general flow is:
use_sudoersd => false
use_sudoers => false
use_sudoers => true (because it use to default to false)
puppetca => false
puppetrun_cmd == "puppetrun"
I think you're saying that I shouldn't need the use_sudoers => true context anymore, and all the tests under that context should stay put, just not be nested in the context... is that correct?
There was a problem hiding this comment.
Cool, pushing those changes.
| end | ||
|
|
||
| it "should not modify #{etc_dir}/sudoers" do | ||
| should_not contain_file("#{etc_dir}/sudoers") |
There was a problem hiding this comment.
I think this is an Augeas resource, not a file. The example still passes with the proposed change reverted.
It should probably be should_not contain_augeas('sudo-foreman-proxy').
There was a problem hiding this comment.
I'll update that, thanks.
…hich is the correct check, instead of 'contain_file'.
| # $use_sudoersd:: Add a file to /etc/sudoers.d (true). | ||
| # type:Boolean | ||
| # | ||
| # $use_sudoers:: Add contents to /etc/sudoers (true). |
There was a problem hiding this comment.
Maybe mention that this is ignored if use_sudoersd is set to true?
There was a problem hiding this comment.
Will do - new commit points that out. It was already there in params.pp, so I only updated init.pp.
|
Thanks! |
Added new use_sudoers boolean as an additional control to whether or not the module would manage /etc/sudoers.
This change is because the "puppet" bool is used to control two different actions (configure puppet module in the smart proxy and add content to sudoers or sudoers.d) and should be separated in certain circumstances:
I created $use_sudoers (which is very similar to $use_sudoersd) to tell the module whether or not it can touch /etc/sudoers. If you set both $use_sudoersd and $use_sudoers to true, then the module should touch sudoers.d (due to the if/else statement's order). If you set them both to false, the module will leave sudoers and sudoers.d alone.
Opened a feature request about this: http://projects.theforeman.org/issues/18142