-
-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes for PAM support needed on RHEL7/Centos7 #227
Conversation
Thank you for sorting this out. EDIT: Could you also address the failing tests? |
Can do. Give me a day or so to work through it all with the tests. Should be simple, just busy at work today. |
# Conflicts: # manifests/params.pp
I can't get the tests running as currently you explicitly require puppet-lint 1.0.1 in the Gemfile. Current lint is 2.1.0. Could I upgrade the Gem to that and work on updating the tests? |
Sure, feel free to update it. |
Lint updated. Lots of changes to be 4.x compatible. Now the tests are completely broken :( I'll have to dig through and find out just what it is up as they seem to be broken in some really fundamental ways - like not even being able to instance the core class due to missing global variables like ::osfamily. Might be a few more days yet. |
@WetHippie Do you want to rebase (and squash) this for review? |
$key_ou = '', | ||
$key_cn = undef, | ||
$key_name = undef, | ||
$key_ou = undef, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this change
$tls_auth = false, | ||
) { | ||
|
||
include openvpn | ||
include ::openvpn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With puppet4+, the absolute path check is no longer needed, and in fact, include foo
is preferred over include ::foo
} | ||
|
||
concat::fragment { "${etc_directory}/openvpn/${server}/download-configs/${name}.ovpn/key": | ||
target => "${etc_directory}/openvpn/${server}/download-configs/${name}.ovpn", | ||
source => "${etc_directory}/openvpn/${server}/download-configs/${name}/keys/${name}/${name}.key", | ||
order => '06' | ||
order => '06', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should now be updated w/ the switch to Vox
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think converting to undefs is worthwhile (as is, perhaps, adding datatypes), but perhaps should be done in a separate PR from changes related to PAM?
Hi @WetHippie, are you still interested in getting this merged? If so, could you please rebase? |
Hi. I'm going to close this PR due to inactivity. Please rebase it and address the comments above if you're still interested in it. |
The following change adds in support for the changed path and filename for the PAM module. This can be used if you have PAM already configured to support LDAP rather than hitting up LDAP directly (ie UsePAM => true, rather than UseLDAP => true). ie this is another way of tackling the problems raised in #172