search

ghoneycutt / puppet-module-pam

Puppet module to manage PAM
Other
18 stars 79 forks source link

Update PAM access to include SLES 10, 12, Ubuntu, Debian including checked to ensure they don't exist when not defined. #162

Closed benkevan closed 5 years ago

ghoneycutt commented 7 years ago

Awesome work!

benkevan commented 7 years ago

Thanks. You can close the other PR since I've moved to a non-master remote branch.

benkevan commented 7 years ago

I may break the pull and run Travis against my branch and do a new PR. each step appears to find little things here and there. I've done apt packages since Debian had been the only thing complaining. But still getting some build failures. On Mon, Apr 24, 2017 at 1:23 PM Garrett Honeycutt notifications@github.com wrote:

@ghoneycutt commented on this pull request.

In .travis.yml https://github.com/ghoneycutt/puppet-module-pam/pull/162#discussion_r113046456 :

@@ -1,6 +1,11 @@

language: ruby

+addons:

  • apt:
  • packages:
    • lsb-release

Thanks for spotting this. I think the LSB facts are a vestige of having to support Suse 9, which did not support the non-LSB facts. Not sure if that's still the case. Hopefully something we can refactor when this moves to Puppet v4 only.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ghoneycutt/puppet-module-pam/pull/162#pullrequestreview-34406880, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUr8d_vE9tWkONJjUG9FZIYDayLA9fCks5rzQSzgaJpZM4NGhdn .

ghoneycutt commented 7 years ago

Looks like lsbdistid needs to be specified in the spec tests. You can do this per check or for everything in spec/spec_helper.rb

ghoneycutt commented 7 years ago

No need for a new PR, just keep push to your branch and it will show up here.

Phil-Friderici commented 7 years ago

@benkevan I know the pain when you just thought, let's do this quick fix. And suddenly your realize you have opend the box of Pandorras special side effects ;)

Good to see the specs fixed too. Great work !

benkevan commented 7 years ago

@ghoneycutt should the supported pam_access tests be added to accesslogin_spec.rb rather than spec_helper.rb? Currently, the Ubuntu lsbdistid is defined within init_spec[1].

Overall it doesn't appear that accesslogin_spec.rb has been updated as compatibility grew.

[1] https://github.com/ghoneycutt/puppet-module-pam/blob/master/spec/classes/init_spec.rb#L165-L185

ghoneycutt commented 5 years ago

We have done a major version upgrade to support Puppet 5 and 6. This involves the transition to hiera data in modules, acceptance testing, type validation in parameters and more.

Your contribution is greatly appreciated! Please check it out the new version 3 and re-open if needed.