Issue #12012 has been updated by Matthaus Litteken.

Status changed from Merged - Pending Release to Closed

Released in facter 2.0.0rc1
----------------------------------------
Bug #12012: Facter globally overrides LANG environment variable at load time
https://projects.puppetlabs.com/issues/12012#change-62956

Author: Chris Price
Status: Closed
Priority: Normal
Assignee: Chris Price
Category: library
Target version: 2.0.0
Keywords: 
Branch: https://github.com/puppetlabs/facter/pull/159
Affected Facter version: 


In facter.rb, ~ line 41-44, we have this:


    # Set LANG to force i18n to C
    #
    ENV['LANG'] = 'C'

This can cause problems when users try to perform an 'exec' with a program that 
responds to this environment variable  (see Puppet #11860).  However, setting 
this variable can be necessary / mandatory for cases where we need to parse the 
output of a command to obtain a status or value (which facter clearly does a 
LOT of), to ensure that the output of the program is formatted consistently.

Rather than globally modifying this variable we probably need to temporarily 
override it when we are about to execute a program whose output we intend to 
parse, and then restore the previous value as soon as we've finished executing 
the program.  In the case of Facter, it appears that most (all?) of this sort 
of behavior is funneled through the method Facter::Util::Resolution.exec.  
Assuming that is the case, we should be able to remove the global modification 
of the environment var from facter.rb and simply override / restore the value 
inside of this "exec" method.

There are a few utility methods lying around that appear to be intended for 
this sort of scenario (temporarily overriding / restoring environment 
variables), but neither of them is currently located in a spot that would make 
it amenable for use in resolving this ticket.  They are:

facter/spec/unit/util/loader_spec.rb, ~ line 24: method "with_env"; can't 
currently use this because it's defined in test code rather than production code
puppet/lib/puppet/util/execution.rb, ~ line 6: method "withenv"; can't use this 
from Facter because it's defined in puppet.

Proposals:

1) Copy "withenv" from puppet/util/execution.rb into Facter codebase so that it 
can be used by both code trees (should this be a separate ticket?)

2) Refactor loader_spec.rb to use "withenv" from new location

3) Remove redundant "with_env" definition from 
facter/spec/unit/util/loader_spec.rb

4) Remove line that sets ENV['LANG'] in facter.rb

5) Refactor Facter::Util::Resolution.exec to use new "withenv" to temporarily 
override LANG for the scope of that method

6) Refactor Puppet code to use "withenv" from the Facter code base rather than 
from puppet/lib/puppet/util/execution.rb (another separate ticket?)

7) Remove "withenv" from puppet/util/execution.rb so that we are not 
maintaining two copies of this functionality


Concerns:

a) How to determine if there are any Facter facts that are executing programs 
and parsing their output *without* calling into Facter::Util::Resolution.exec 
to accomplish it?  Any such facts could potentially be affected by the fact 
that they may no longer be running with ENV['LANG'] forced to 'C'.

b) Documentation; customers' custom facts may have been relying on the old 
behavior without even realizing it... how do we convey the change to them?

c) (puppet) If we remove "withenv" from puppet/util/execution.rb, are existing 
spec tests exhaustive enough to confirm that we've successfully refactored all 
of the places in the puppet code that were calling the old version?

d) (puppet) If we remove "withenv" from puppet/util/execution.rb, any concerns 
that we will break modules / customer code?


Any comments / input greatly appreciated...


-- 
You have received this notification because you have either subscribed to it, 
or are involved in it.
To change your notification preferences, please click here: 
http://projects.puppetlabs.com/my/account

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Bugs" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-bugs?hl=en.

Reply via email to