Update SLES 10 & 12 pam/login and pam/sshd to behave similarly to SLES 11 for pam_access

[ghoneycutt]

@Phil-Friderici This passed even though the fixtures in the spec tests were not altered. Would you mind taking a look? Also, do you have any SLES loving colleagues that might be able to test the functionality of this PR?

[ghoneycutt]

@benkevan Thank you for the contribution! We need to get the spec tests figured out as well as have someone else verify that this works functionality and then I'll get it merged.

[benkevan]

@ghoneycutt I've confirmed it worked. I'll fix to move the entry to the top of the "account" declarations since that's my preference.

I can work out the spec tests but didn't see any that would fail.

[Phil-Friderici]

nobody loves Suse (scnr)

I'll have a look at the spec tests and why they are not failing here.

[Phil-Friderici]

The specs doesn't fail, because the change implements the expected behaviour on SLES12 [1],[2]. So the question is: why didn't it fail before ?

[1] https://github.com/ghoneycutt/puppet-module-pam/blob/master/spec/classes/init_spec.rb#L492-L504 [2] https://github.com/ghoneycutt/puppet-module-pam/blob/master/spec/classes/init_spec.rb#L519-L531

[Phil-Friderici]

tests for functionality of sshd_pam_access & login_pam_access content are limited to RedHat and Suse-11 only [1], [2]. Therefore they didn't failed.

[1] https://github.com/ghoneycutt/puppet-module-pam/blob/master/spec/classes/init_spec.rb#L506-L517 [2] https://github.com/ghoneycutt/puppet-module-pam/blob/master/spec/classes/init_spec.rb#L479-L490

[benkevan]

I've removed the restriction of checking only against 11. Which means we'll have to go back and fix the spec failures (of all the SLES systems, assuming more than just SLES 12 fails).

I'll try to get to this. I have SLES 12 available, but no SLES 9 or 10.

[ghoneycutt]

Please don't make the change to SLES 10 unless you are willing to functionally test it.

[ghoneycutt]

@Phil-Friderici Lulz. Somebody you work with likes it.

[benkevan]

@ghoneycutt I have a system I'm checking the functionality of the configurations. I'm unable to test through a deployment from puppet.

I can pull those if you prefer an end to end tests with puppet functionality, in this case, I just confirmed that pam_access.so functions as expected on the SUSE 10 system which is aligned with the changes being applied.

[ghoneycutt]

@benkevan That's cool. The spec tests tell us it will compile correctly, so if you tested that the line there works, even if done manually, is fine.

Appreciate all the work!

[benkevan]

@ghoneycutt I prefer to spend the time getting off of SLES 10, but hey, can't win them all right? Once the build completes successfully (post spec failures), then we should be good.

and @Phil-Friderici I have to admit, after many years within an enterprise space, SLES is actually pretty solid (that or I've had way too much coffee today and speaking gibberish).

[benkevan]

@ghoneycutt

There.. builds should all pass, and I've updated the title to represent the full change as it morphed from the initial addition of only SLES 12 to SLES 10 and 12 with required spec updates.

I've tested functional configuration successfully on SLES 10 and SLES 12.

Thank you.

[ghoneycutt]

Looks good. Could you please squash your commits and repush to this branch and I'll get it merged.

[Phil-Friderici]

Actually I should like Suse as it's origin is from Germany. Still I dislike yast and that it touch(ed) configurations files that you didn't wanted to change too often for me. Hope that is gone these days :P

[benkevan]

@Phil-Friderici agreed. I was thinking the same when going through the code, but didn't want that as part of the SLES addition release.

@ghoneycutt apologise for the newbness. Trying to figure out how to squash commits that I've already committed to my forked master.

[ghoneycutt]

@benkevan That's why you don't want to work out of your master branch. Instead, create a new topic branch.

Here's a sample repo I use to teach people how to contribute to upstream projects. You're welcome to give it a go :)

https://github.com/unixmonkey/learngit

[benkevan]

I've created a new PR which is from a new branch to maintain history. I kept 2 distinct commits. One that's specific for the SUSE updates which I originally wanted to achieve. A second commit for the additional updates to check for failures and the subsequent addition of Debian and Ubuntu systems that had pam_access.so defined within their templates.

[ghoneycutt]

Thanks @benkevan

[ghoneycutt]

That is PR #162