Skip to content

(#22704) make directoryservice create normal uids#1944

Closed
leinaddm wants to merge 2 commits intopuppetlabs:masterfrom
leinaddm:fix_22704_low_ids_directoryservice
Closed

(#22704) make directoryservice create normal uids#1944
leinaddm wants to merge 2 commits intopuppetlabs:masterfrom
leinaddm:fix_22704_low_ids_directoryservice

Conversation

@leinaddm
Copy link

uids for normal users should start at 501.
Before this patch, when uid was not specified for the user resource, the
directoryservice provider would choose a low uid for the new user.

uids for normal users should start at 501.
Before this patch, when uid was not specified for the user resource, the
directoryservice provider would choose a low uid for the new user.
@puppetcla
Copy link

Waiting for CLA signature by @leinaddm

@leinaddm - We require a Contributor License Agreement (CLA) for people who contribute to Puppet, but we have an easy click-through license with instructions, which is available at https://cla.puppetlabs.com/

Note: if your contribution is trivial and you think it may be exempt from the CLA, please post a short reply to this comment with details. http://docs.puppetlabs.com/community/trivial_patch_exemption.html

@puppetcla
Copy link

CLA signed by all contributors.

@joshcooper
Copy link
Contributor

Hi @leinaddm, thanks for submitting your contribution! I agree Puppet is doing the wrong thing when creating users, but if we merge this as-is, then users that puppet creates will no longer be hidden (due to the uid > 500). Users could be relying on this behavior, and this patch would break that.

There is some related discussion in http://projects.puppetlabs.com/issues/19783, where Jesse suggests enabling the Hide500Users option. Perhaps we combine the two? I'm not a mac expert, so I'd recommend bringing this up on the puppet-dev mailing list.

Also, it would be great if you could add a spec test to spec/unit/provider/user/directoryservice_spec.rb for what happens when next_system_id is called without any arguments. If you need help writing those, we can help on #puppet-dev on irc.

@leinaddm
Copy link
Author

leinaddm commented Oct 4, 2013

Hi @joshcooper,

I doubt anyone is using it without specifying the uid. On my test macs
the accounts created in that way are broken: createhomedir does not work
on them. I think the Hide500Users discussion, to hide the puppet account
if created with a higher id, is unrelated but I don't think it would be
a good idea for puppet to turn it on by default.

I'll add some tests later today.

Thanks, Daniel.

@zaphod42
Copy link
Contributor

@leinaddm Thanks for the contribution, but after some debate we have a few concerns about this approach. The uid selection should be based on the system parameter, which the provider doesn't appear to honor. Also, changing the uid selection could create different behavior for users in cases that they don't expect. @kylog, @joshcooper, and I think that the correct approach here is to honor the system parameter and make the uid selection based on that.

Thanks for the contribution, again. Would you be interested in trying to get the system parameter (http://docs.puppetlabs.com/references/latest/type.html#user-attribute-system) working?

@zaphod42 zaphod42 closed this Nov 13, 2013
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