search

saltstack-formulas / samba-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
16 stars 72 forks source link

Make winbind services changes optional #50

Closed noelmcloughlin closed 5 years ago

noelmcloughlin commented 5 years ago

This PR is to address #49.

Four configurable boolean values are introduced to allow for controlled execution of the winbind-ad state. Default is True (winbind will restart) but behaviour is now tunable (False).

noelmcloughlin commented 5 years ago

@beuss to review.

beuss commented 5 years ago

Hi, In anycase, if authconfig is set to true, it'll be run on each highstate. Can't you apply it only if mkhomedir & winbind are missing from the pam file?

Regards

beuss commented 5 years ago

Moreover, not all combinations of booleans can be used, eg. authconfig false & nsswitch to true ends up in a failure because the require_in is not guarded with {% if samba.winbind.optional.authconfig %}

noelmcloughlin commented 5 years ago

Grep can be used for first comment - let me know what command works for you. egrep -i 'authconfig=true|winbind' . /etc/pam.d/<somedir>/<somefile>

Can you supply the pseudo code you need here for comment two?
I have no environment ready to check?

beuss commented 5 years ago

I'm wondering… using the grep means you already know what the auth config update will result in. So maybe using two simple file.line would achieve the same result? I don't know pam auth config deep enough to answer. 'cause using grep you wouldn't trigger the command if any of the entries are present, whereas you want both to be there.

noelmcloughlin commented 5 years ago

I cannot remember the implementation very well. I have CentOS/Ubuntu/Fedora runing Samba so I'll logon and check the configuration when I get time. thx

beuss commented 5 years ago

looking at what's done on some other formulas manipulating pam, you can have a common state for authconfig update that'll get triggered by the lack of mkhomedir (another state) or the lack of winbind (yet another state) lines in /var/lib/pam/seen

aboe76 commented 5 years ago

@noelmcloughlin and @beuss added an issue on saltstack source for this: https://github.com/saltstack/salt/issues/51612

noelmcloughlin commented 5 years ago

Thanks. I'll try to make time to fix this PR soon.

rbthomp commented 5 years ago

I use this in one of my formulas. Not sure if the help at all. - onlyif: diff <(authconfig --disablewinbindoffline --disablewinbindkrb5 --disablewinbind --disablewinbindauth --disablecache --updateall --test) <(authconfig --test)

noelmcloughlin commented 5 years ago

Thanks @rbthomp I added your comment to #49