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

Fix order of access.conf entries when allowed_users is a Hash #71

Closed
wants to merge 1 commit into from

Conversation

treydock
Copy link
Contributor

In Ruby 1.8.7 the order of a Hash is "random" [1]. One way to ensure consistent order of elements when iterating over a Hash is to sort by the keys then reference the value using Hash[key]. Example of RSpec test output before changing templates/access.conf.erb.

Failures:

  1) pam::accesslogin access.conf with hash entry containing string values should contain allowed_users
     Failure/Error: verify_contents(subject, 'access_conf', [
       expected: ["+ : username1 : cron", "+ : username2 : tty0", "- : ALL : ALL"]
            got: ["+ : username2 : tty0", "+ : username1 : cron", "- : ALL : ALL"] (using ==)
     # ./spec/classes/accesslogin_spec.rb:92

  2) pam::accesslogin access.conf with hash entries containing string, array and empty hash should contain allowed_users
     Failure/Error: verify_contents(subject, 'access_conf', [
       expected: ["+ : username : tty5", "+ : username1 : cron tty0", "+ : username2 : cron", "+ : username3 : tty0", "+ : username4 : ALL", "- : ALL : ALL"]
            got: ["+ : username : tty5", "+ : username3 : tty0", "+ : username1 : cron tty0", "+ : username2 : cron", "+ : username4 : ALL", "- : ALL : ALL"] (using ==)
     # ./spec/classes/accesslogin_spec.rb:150

I observed this issue in production when using a Hash defined in Hiera. Puppet kept changing the order of the "allow" entries in access.conf.

[1] : http://www.ruby-doc.org/core-1.8.7/Hash.html - "The order in which you traverse a hash by either key or value may seem arbitrary, and will generally not be in the insertion order."

@ghoneycutt
Copy link
Owner

Apologies again on the late response and merge. This should be a straight forward PR though the change in how the testing was done made it hard to merge as I'm not familiar with verify_contents() which is not well documented.

@ghoneycutt
Copy link
Owner

Merging functionality in PR #91

@ghoneycutt ghoneycutt closed this Jan 28, 2015
@treydock
Copy link
Contributor Author

@ghoneycutt Most of the functions from puppetlabs_spec_helper are undocumented. It basically takes multiline strings, splits by newline and ensures that the two arrays are equal. So the order and contents can be tested.

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.

2 participants