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

Add root user plugin #698

Closed
wants to merge 4 commits into from
Closed

Add root user plugin #698

wants to merge 4 commits into from

Conversation

sh9189
Copy link
Contributor

@sh9189 sh9189 commented Dec 28, 2015

Continuing from #624, add root_user attribute to return correct root user on different platforms

John Bellone and others added 4 commits December 28, 2015 13:20
This abstracts out the root user information just as the root group
information is. It makes the appropriate name available via the
root_user attribute.
@mcquin
Copy link
Contributor

mcquin commented Jan 4, 2016

👍

\cc @chef/client-core

@lamont-granquist
Copy link
Contributor

i dunno, it slightly triggers some neckbeard pedantry in me, since technically on unix, the root user is any user with a uid of zero, so there could be multiple root users (although most often multiple root users will probably mean the box is hacked) and the root user doesn't need to be named root (and this feature of unix might be used by some very special purpose configurations).

@sh9189
Copy link
Contributor Author

sh9189 commented Jan 5, 2016

@lamont-granquist You are right, But it looks like we rely on root user being named 'root' in root_group plugin https://github.com/chef/ohai/blob/master/lib/ohai/plugins/root_group.rb#L29. I can make the change in both areas to look for uid=0 instead of name=root if that's what we want.

@lamont-granquist
Copy link
Contributor

yeah, i'm not sure if i'm being overly pedantic or not...

@lamont-granquist
Copy link
Contributor

One thing to consider here is that if we use the Etc.getpwent functions that we'll probably wind up iterating through LDAP for this on some boxes, which would be terribly bad...

@lamont-granquist
Copy link
Contributor

So, another thing here is that the code here is very trivial and cookbook code can just implement this logic as it sees fit to do. To make it useful, it should probably go through /etc/passwd (without hitting LDAP) and extract out all the uid = 0 users. Which I'm not certain anyone really needs.

@lamont-granquist
Copy link
Contributor

Yeah if this is just being done for the sake of symmetry with root_user then I think I'm 👎

Would need a more compelling use case on how this should be used in cookbooks, otherwise cookbooks should just use "root" (which is all this does and would save typing) and then use "Administrator" or "System" directly for windows as they see fit. If there's existing cookbook code that this abstraction would simply then it'd be useful to get a pointer to that.

@thommay
Copy link
Contributor

thommay commented Jan 19, 2016

I agree with the 👎

@sh9189
Copy link
Contributor Author

sh9189 commented Jan 19, 2016

@lamont-granquist A simple use case is to create a root user owned resource (files/directories) in a similar manner on both windows/unix based platforms.
For example, chef client directories need to be setup to be root owned and the chef-client cookbook has very similar code at https://github.com/chef-cookbooks/chef-client/blob/master/libraries/helpers.rb#L37-L43 to find out root user based on platform.
Also another example is when chef handlers are deployed, chef handler file is typically set up to be root owned. For example https://github.com/chef-cookbooks/chef_handler/blob/master/recipes/default.rb#L26
Adding an attribute at ohai level which determines root user will help so that every cookbook does not have to rewrite the same logic.

@lamont-granquist
Copy link
Contributor

Your first example isn't really helping your case, though. The root user on unix is clearly "root" but on windows we've got "System", "Administrator" and that algorithm there. That's suggesting to me that the right approach is similar to the third example where each cookbook should expose an attribute for their file ownership perms, then the cookbook sets its own default which can be overridden. And baking this into ohai just makes it brittle and hard to change (see node['fqdn'] everywhere and the fun that caused).

I think what I'd prefer to see would be some chef-sugar added so that users could do:

default['cookbook']['root_user'] = windows? ? "Administrator" : "root"

And where it looks like the code for windows in the chef-client cookbook could be baked into a reusable method.

@sh9189
Copy link
Contributor Author

sh9189 commented Jan 20, 2016

I was using the examples just to illustrate the use case for a root_user ohai attribute. If we need to change logic on how to find root user on unix or to find equivalent one on windows, I would be happy to do that. But if there is no need for a root_user ohai attribute (if the use case I described would be best solved by a reusable helper in a cookbook), then please close this one. I am not a windows expert but it looks like using 'System' will be the simplest solution to root user equivalent on windows. 'System' account has equivalent privileges as 'Adminstrator' user account, the only difference seems to be that 'Administrator' is an actual user account with a password as per https://support.microsoft.com/en-us/kb/120929.
There is also an issue where an user has said that using 'Administrator' account is problematic (chef-boneyard/chef-client#333)

@lamont-granquist
Copy link
Contributor

I am not a windows-guru, but @smurawski made the comment today about Administrator vs. System and the code in chef-client looks like it dynamically looks up the correct account to use somehow. I'd like to see someone who knows the windows ecosystem better figure out the right approach there.

And I do lean towards not wanting to use an ohai attribute for this, unless there is absolutely a definitive algorithm to use because the node[:fqdn] issues shows how brittle ohai attributes are to use.

@smurawski
Copy link
Contributor

I'm 👎 on the idea, as you can make equally strong cases about what a "root" user is on Windows - whether that's administrator or system.

This is a problem better solved outside of ohai itself, whether a plugin or a common environment attribute or something to that effect.

@lamont-granquist
Copy link
Contributor

Yeah, I'm still 👎 on this. Its now both incredibly simple windows? ? "SYSTEM" : "root" and yet doesn't consider the "Administrator" vs "System" detail at all (and I don't think it really can). I think this information needs to live in the domain (the site or the cookbook) that makes the choice about if it should be "SYSTEM" or "Administrator" or whatever.

This is not a parallel to the root_group since "root" vs. "wheel" while being trivial also has an algorithm which is objectively correct for the distribution chef/ohai is running on and a definitive answer exists to that question.

@tas50
Copy link
Contributor

tas50 commented Apr 4, 2016

I'm going to close this one at this point since without a robust configuration system I think this would cause more harm on the Windows system then good.

@tas50 tas50 closed this Apr 4, 2016
lamont-granquist referenced this pull request in poise/poise Apr 10, 2016
This feels like it should be okay for all existing code I think.
@thommay thommay added Expeditor: Skip Version Bump Used to skip built_in:bump_version and removed Meta: Exclude From Changelog labels Sep 22, 2017
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Expeditor: Skip Version Bump Used to skip built_in:bump_version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants