(#12357) Disable root_home fact on Windows

[jeffmccune]

Without this patch the root_home fact fails on windows. This patch
fixes the problem by implementing a code block confined to windows for
the fact's value resolution.

Because there is no root account on Windows the code block simply
returns nil which makes the Facter fact undefined on Windows
platforms.

[joshcooper]

+1 I agree that it's better to not implement the fact on Windows, since the accounts module is not supported on Windows. If/when that happens, we can revisit this fact.

[jeffmccune]

These shouldn't be targeting master... Need to close these.

[kbarber]

For example, while the tests succeed - you get a debugging message.

So - unfortunately setting nil for a fact will just make it skip over that resolution mechanism :-). Because you have a fallback, that is not confined, ultimately windows will just try and run 'getent' anyway. So you get:

[ken@kb puppetlabs-stdlib]# rspec spec/unit/facter/root_home_spec.rb
"/Users/ken/Development/puppetlabs-stdlib/spec"
...Could not retrieve root_home: private method `split' called for nil:NilClass
.

Anyway ... perhaps an alternative approach is to work with the fact that Resolution.exec is silent when the command doesn't exist?

Something like:

https://github.com/kbarber/puppetlabs-stdlib/blob/ticket/master/12357_disable_root_home_fact_on_windows_kb/lib/facter/root_home.rb#L10

That way the thing just returns nil if getent doesn't exist/returns nothing. Which is probably better for a lot of platforms anyway. I just tested this on 2008 and it seems to do the right thing (ie. its silent when I request root_home). What do you think?

[jeffmccune]

This is actually targeted at master.

[jeffmccune]

@kbarber Hrm, crap. I should have tested this more before changing it to nil.

I definitely need a alternate approach, but I don't want to take your approach because I want to avoid trying to execute a process I know will fail on windows.

[slippycheeze]

That actually seems like the most correct directory to me.

[kbarber]

So obviously setting to nil just 'skips over' this resolution - which isn't what you want. Your options are:

• Relying on Resolution.exec to return nothing isn't a bad choice, and is quite common btw. And will simply return nil on platforms without the command.
• Another option is that you could make your other resolution be more confined, and exclude windows, however you would have to include all OS that support getent (so be careful to include all the bsd's and unix variations if they support it). Or you can just use osfamily if your willing to drop the versions of facter support that didn't have osfamily. This is more precise then option 1, but requires proper research and if a new distro comes along - things can slip through the cracks. Using 'kernel' is also another option.
• Returning an empty string or 'unknown' or some such - but I'm not such a fan of this, more because the semantic convention for a non-defined fact imply 'unknown' versus just simply stating 'unknown'.

Otherwise a negative confine or a confine block would be ideal, but this isn't supported in facter today - but at least a facter block will probably be supported in 2.0.

[kbarber]

And yeah - to mirror @daniel-pittman's comment - if its not too hard, I'd return the Administrator home dir - or more precisely the home directory of the user who has an SID of 500 :-).

[joshcooper]

This is a bit problematic to implement. The Adminstrator SID contains a domain component which is unique, e.g. S-1-5-21-887289371-3252031711-946894838-500. The last component (RID) is always 500, but there could be multiple such accounts, e.g. the local Administrator and the domain Administrator. The other issue is MS "best practice" is to disable the Administrator account. In which case, the "C:\Users\Administrator" directory may not even exist, which could lead to failures if trying to managing ssh keys for the 'Administrator' account.

[jeffmccune]

@kbarber OK, this is ready to go again. I fixed this by addressing the root cause of the problem, which is calling methods on an object that might be nil in some cases.

This should be more robust now, since getent passwd may not be available on all Unix platforms and we assumed it would exist and return something interesting.

root_ent && root_ent.split(":")[5]

[joshcooper]

@jeffmccune will merge as soon as we figure out which stdlib branch we are including in PE 2.5

[jeffmccune]

#42 replaces this pull request. It's targeted at 2.3.x instead of master.