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

Compare uid and gid instead of name and group #32674

Merged
merged 2 commits into from
May 3, 2016

Conversation

twangboy
Copy link
Contributor

What does this PR do?

Compares UID and GID instead of names to account for differences in capitalization

What issues does this PR fix or reference?

#11801

Previous Behavior

file.managed would return failure because it couldn't match 'Administrator' with 'administrator' due to differences in capitalization. Owner would change, but it would report failure.

New Behavior

Now it compares the uid and gid instead of user and group names.

Tests written?

No

@cachedout
Copy link
Contributor

I have a memory of us changing this for a good reason and I'm worried this might cause a regression.

@rallytime I seem to recall you working on something like this but a long, long time ago. Is this the same issue?

@rallytime
Copy link
Contributor

@cachedout I think the one you were thinking of is this change: https://github.com/saltstack/salt/pull/11732/files - which was an implementation of the feature request in #10640. I had a follow-up PR with a fix in #11753.

The feature I was implementing there was to allow the file state to accept numerical user and group ids, so my changes were right above the lines that are being changed here.

@damon-atkins
Copy link
Contributor

Keep in mind username can also be a plain number

@cachedout
Copy link
Contributor

@twangboy Let me know when you're able to follow up with the concern from @damon-atkins, please.

@damon-atkins
Copy link
Contributor

damon-atkins commented Apr 30, 2016

You can only use the numbers as a UID if the user does not exist, or use different properties to indicate its a number e.g. own=fred vs ownerid=432 and not allow both to be set. In some respect if a user 432 does not exist it would be nice to get an error.

But doing lookup target username UID, and lookup current username UID and compare is fine or in windows comparing SID

echo `getent passwd root 2>&1`  vs getent `passwd rOot 2>&1`
root:x:0:0:root:/root:/bin/bash vs getent passwd: Unknown user name 'rOot'.

If you lowercase the usernames and compare on Unix different result.

@twangboy
Copy link
Contributor Author

twangboy commented May 2, 2016

@damon-atkins I gated the uid comparison so it will only compare uid in windows.

@cachedout cachedout merged commit 4bb3ca5 into saltstack:2015.8 May 3, 2016
@twangboy twangboy deleted the fix_11801 branch August 30, 2016 18:20
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.

4 participants