Facter path to external executables

[Hailee Kenney]

There seems to be a lot of Facter tickets that stem from a similar issue: finding the correct executable. Here are just a few examples: 

https://projects.puppetlabs.com/issues/5013

https://projects.puppetlabs.com/issues/13396

https://projects.puppetlabs.com/issues/13678

https://projects.puppetlabs.com/issues/14541

To me, these all seem related to how Facter is determining the path for external executables. Since there have been a lot of problems around this issue, it seems that something should be done about it, but it's not clear what. There have been several proposed fixes on the various tickets. Rather than dealing with these individual bugs, it would probably be good to come up with a unified solution for the problem. I was just hoping to get some opinions on if/how we should do something about this. 

Krzysztof Wilczynski has suggested implementing 'which' inside Facter. We have some facts that hardcode the path to the executable. 

Thoughts? 

Hailee Kenney 

Development Intern 

Puppet Labs, Inc. 

hai...@puppetlabs.com 

[Chris Price]

We have an implementation of "which" in puppet, and, IMO, it would make sense for both projects to share the same code and logic for this.  However, our project structure doesn't currently lend itself to that.

If you do decide to implement a 'which', you might just copy the one from puppet.  I'm not a big fan of the fact that the logic in those two copies of the same method may unintentionally diverge over time, but for now it'd probably be nice for the logic to be consistent between the two tools.

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppe...@googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

[Jeff Weiss]

I think the argument against 'which' is that we are implicitly trusting the users' $PATH, which may be a security vulnerability.  We could certainly sanitize $PATH by removing relative directories (the same way we have for the fact search path), and that gets us part of the way there, but I'm not certain if it gets us all the way to a great solution.

-Jeff

Jeff Weiss

Software Developer

Puppet Labs, Inc.

jeff....@puppetlabs.com

[Ken Barber]

Did you see the re-implementation of 'which' that Stephen Schulte is
working on here?

https://github.com/puppetlabs/facter/pull/189

Its taken from Puppet to a certain extent I believe, with some
backwards compatible handling that we thought we might need as well to
handle existing custom fact assumptions.

I think implementing that as a start takes us a long way, once you
have control of the executable lookup, you can bend it your own way I
believe. So this is most definitely a step #1 IMHO.

ken.

[Krzysztof Wilczynski]

Hi,

On Friday, May 18, 2012 10:30:18 AM UTC+1, Ken Barber wrote:

Did you see the re-implementation of 'which' that Stephen Schulte is
working on here?

https://github.com/puppetlabs/facter/pull/189

Nice one Ken! Stephen's implementation looks really nice! Definitely, a +1 :)

KW

[Ken Barber]

Yeah - I think it was based on your advice KW. Stephen is awesome when
it comes to this kind of stuff.

> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Developers" group.

> To view this discussion on the web visit
> https://groups.google.com/d/msg/puppet-dev/-/Mf-THp84e3YJ.

[Chris Price]

Just as a heads up--there have been some minor changes to that code in puppet over the last few months.  (Particularly, some special handling of the case where the "~" character appears in the user's PATH.)  I am not sure if these changes are relevant to facter (because I don't think Facter is doing as much manipulation of environment variables prior to spawning processes as puppet is), but it would be worth a quick once-over before this gets merged in, if it is decided that this is the way to go.

The pull req looks good to me, though, overall.