Skip to content
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

Added Solaris 9 support #54

Closed
wants to merge 2 commits into from

Conversation

nalyanyam
Copy link
Contributor

Added base support for Solaris 9

@ghoneycutt
Copy link
Owner

@nalyanyam Could you please push the VAS portion to this branch as a separate commit and then find someone to test and report back here?

@nalyanyam
Copy link
Contributor Author

@ghoneycutt
Could you please check and see if you could merge this Solaris 9 basic support. Extra configs for ensure_vas, etc could be specified in hiera. It is a bit difficult to get this module tested on Solaris 9 by others. Most users are running Solaris 10, so getting the module tested on Solaris 9 may take time.
Thanks

@@ -360,6 +360,31 @@
}
'Solaris': {
case $::kernelrelease {
'5.9': {
$default_pam_auth_lines = [ '# login service (explicit because of pam_dial_auth)',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should not be putting in commented out lines that are not real comments in the file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback.
These comments are only useful hints in pam.conf. Meant to make it easier for the person checking the file to understand why it is configured that way.
However, if that is the design strategy for the modules, then i will remove them and recommit. I will get these changes done on Monday.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nalyanyam: you can still add your comments via hiera ;)

@ghoneycutt
Copy link
Owner

Looking good. If you could remove those commented out lines noted above, I am more than happy to merge. Thanks for taking the time to add Solaris 9 support!

@ghoneycutt
Copy link
Owner

@Phil-Friderici do these default values look good to you? I want to ensure they are Solaris defaults and not E defaults.

@Phil-Friderici
Copy link
Contributor

@ghoneycutt: sorry, I don't know Solaris this well :(

@nalyanyam
Copy link
Contributor Author

@ghoneycutt
Ok, I have now removed the commented out lines.

Actually the Solaris 9 pam defaults are not very different from Solaris 10 defaults apart from a couple of lines.

@ghoneycutt
Copy link
Owner

@nalyanyam Thank you! I squashed and reworded your commit in PR #57

@ghoneycutt ghoneycutt closed this May 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants