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

Fixes ACL's with spaces #30

Merged
merged 2 commits into from
Feb 8, 2017
Merged

Fixes ACL's with spaces #30

merged 2 commits into from
Feb 8, 2017

Conversation

i1tech
Copy link

@i1tech i1tech commented Dec 29, 2016

Fix for ACL's with spaces, which otherwise cause ACL's to be applied with every puppet agent run.

@dobbymoodge
Copy link

@i1tech Sorry for the slow response, can you update the tests to cover this functionality?

@dobbymoodge
Copy link

Also, when can ACLs have spaces in them?

@i1tech
Copy link
Author

i1tech commented Feb 6, 2017

Spaces in ACL's are common when Auth'ing against an Active Directory Domain controller, especially in the case of Groups. (ie "Domain Adminsitrators", "Service Accounts", etc) Depending on how the Linux side is setup, using SSS, LDAP, NSLCD+LDAP, Winbind, LikewiseOpen, Centrify, etc. you will also find spaces embedded in the username. I'll try to find some time to work on the tests.

@dobbymoodge
Copy link

@i1tech Also, please rebase so Travis stops whining about Ruby 1.8.7. :D

@roidelapluie
Copy link
Member

@dobbymoodge you have push access to the repo .. that is a new github feature

@dobbymoodge
Copy link

@roidelapluie ah, so I do. Now I understand what you were going on about. ☺️

@dobbymoodge
Copy link

@roidelapluie @i1tech I added a test, but I'm not sure I did it the Right Way. In particular, I wonder if I should be able to stub out the 2 methods without pulling in RSpec::Mocks. Thoughts?

@i1tech
Copy link
Author

i1tech commented Feb 7, 2017

Thanks for adding that test, it would probably have been a while before I was able to get to it. Seem very busy these days.

@dobbymoodge
Copy link

@i1tech You've actually tested this in an environment with spaces in the group/usernames?

@dobbymoodge
Copy link

Waiting for feedback from @roidelapluie on the quality of this test before I merge. :)

@i1tech
Copy link
Author

i1tech commented Feb 8, 2017

Absolutely, we are running the patched version of the module on our Puppet4 server.

@dobbymoodge dobbymoodge merged commit 704bea3 into voxpupuli:master Feb 8, 2017
@dobbymoodge
Copy link

I'll go ahead and merge then, and we can fix the test later if needed.

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