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

Uid negation fix #474

Merged
merged 2 commits into from Jan 21, 2015
Merged

Conversation

jonnytdevops
Copy link
Contributor

Fix for MODULES-1688.

Re-applying a manifest with an unchanged UID will now not re-apply the rule unnecessarily. Works properly for negated and non-negated cases.

bodepd and others added 2 commits January 20, 2015 23:40
When using the uid feature of the firewall module,
it did not work with string based usernames as
documented.

The uid propery always synchronized with a message of
<number> does not match <username>.

This code overrides the uid getter method to perform
a check of both the data from the property hash as well
as using that data (assuming it is a uid) to resolve the
username.

While this patch is pretty simple, I have only tested it
on Ubuntu 14.04. I am not sure if it could be problematic
with other versions.

I have not yet written tests b/c I wanted to submit
my proposed fix for discussion while I get those
written.
Re-applying a manifest with an unchanged UID will now not re-apply
the rule unnecessarily.
@cyberious
Copy link
Contributor

@bodepd thoughts? You cool with us closing yours and merging this with yours as well?

@jonnytdevops
Copy link
Contributor Author

Fixes #466

@bodepd
Copy link
Contributor

bodepd commented Jan 21, 2015

no problem

@bodepd bodepd mentioned this pull request Jan 21, 2015
cyberious added a commit that referenced this pull request Jan 21, 2015
@cyberious cyberious merged commit 85938e5 into puppetlabs:master Jan 21, 2015
@jonnytdevops jonnytdevops deleted the uid_negation_fix branch January 21, 2015 19:55
@jonnytdevops jonnytdevops restored the uid_negation_fix branch January 21, 2015 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants