From a68d90a71af4a799cd6f3bd3c3987432278a567a Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Wed, 20 Jan 2016 09:04:44 -0800 Subject: [PATCH] rework run_command's env setting to not change os.environ for the rest of the module. New param to run_command to modify the environment for just this invocation. Documentation and comment adjustments. --- lib/ansible/module_utils/basic.py | 67 +++++++++++-------- .../module_utils/basic/test_run_command.py | 5 +- 2 files changed, 41 insertions(+), 31 deletions(-) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index b420f18e6e..42ea8e7906 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -546,11 +546,10 @@ class AnsibleModule(object): if no_log_object: self.no_log_values.update(return_values(no_log_object)) - # check the locale as set by the current environment, and - # reset to LANG=C if it's an invalid/unavailable locale + # check the locale as set by the current environment, and reset to + # a known valid (LANG=C) if it's an invalid/unavailable locale self._check_locale() - self._check_arguments(check_invalid_arguments) # check exclusive early @@ -1094,7 +1093,6 @@ class AnsibleModule(object): # as it would be returned by locale.getdefaultlocale() locale.setlocale(locale.LC_ALL, '') except locale.Error: - e = get_exception() # fallback to the 'C' locale, which may cause unicode # issues but is preferable to simply failing because # of an unknown locale @@ -1757,25 +1755,29 @@ class AnsibleModule(object): # rename might not preserve context self.set_context_if_different(dest, context, False) - def run_command(self, args, check_rc=False, close_fds=True, executable=None, data=None, binary_data=False, path_prefix=None, cwd=None, use_unsafe_shell=False, prompt_regex=None): + def run_command(self, args, check_rc=False, close_fds=True, executable=None, data=None, binary_data=False, path_prefix=None, cwd=None, use_unsafe_shell=False, prompt_regex=None, environ_update=None): ''' Execute a command, returns rc, stdout, and stderr. - args is the command to run - If args is a list, the command will be run with shell=False. - If args is a string and use_unsafe_shell=False it will split args to a list and run with shell=False - If args is a string and use_unsafe_shell=True it run with shell=True. - Other arguments: - - check_rc (boolean) Whether to call fail_json in case of - non zero RC. Default is False. - - close_fds (boolean) See documentation for subprocess.Popen(). - Default is True. - - executable (string) See documentation for subprocess.Popen(). - Default is None. - - prompt_regex (string) A regex string (not a compiled regex) which - can be used to detect prompts in the stdout - which would otherwise cause the execution - to hang (especially if no input data is - specified) + + :arg args: is the command to run + * If args is a list, the command will be run with shell=False. + * If args is a string and use_unsafe_shell=False it will split args to a list and run with shell=False + * If args is a string and use_unsafe_shell=True it runs with shell=True. + :kw check_rc: Whether to call fail_json in case of non zero RC. + Default False + :kw close_fds: See documentation for subprocess.Popen(). Default True + :kw executable: See documentation for subprocess.Popen(). Default None + :kw data: If given, information to write to the stdin of the command + :kw binary_data: If False, append a newline to the data. Default False + :kw path_prefix: If given, additional path to find the command in. + This adds to the PATH environment vairable so helper commands in + the same directory can also be found + :kw cwd: iIf given, working directory to run the command inside + :kw use_unsafe_shell: See `args` parameter. Default False + :kw prompt_regex: Regex string (not a compiled regex) which can be + used to detect prompts in the stdout which would otherwise cause + the execution to hang (especially if no input data is specified) + :kwarg environ_update: dictionary to *update* os.environ with ''' shell = False @@ -1806,10 +1808,15 @@ class AnsibleModule(object): msg = None st_in = None - # Set a temporary env path if a prefix is passed - env=os.environ + # Manipulate the environ we'll send to the new process + old_env_vals = {} + if environ_update: + for key, val in environ_update.items(): + old_env_vals[key] = os.environ.get(key, None) + os.environ[key] = val if path_prefix: - env['PATH']="%s:%s" % (path_prefix, env['PATH']) + old_env_vals['PATH'] = os.environ['PATH'] + os.environ['PATH'] = "%s:%s" % (path_prefix, os.environ['PATH']) # create a printable version of the command for use # in reporting later, which strips out things like @@ -1851,11 +1858,10 @@ class AnsibleModule(object): close_fds=close_fds, stdin=st_in, stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, + env=os.environ, ) - if path_prefix: - kwargs['env'] = env if cwd and os.path.isdir(cwd): kwargs['cwd'] = cwd @@ -1934,6 +1940,13 @@ class AnsibleModule(object): except: self.fail_json(rc=257, msg=traceback.format_exc(), cmd=clean_args) + # Restore env settings + for key, val in old_env_vals.items(): + if val is None: + del os.environ[key] + else: + os.environ[key] = val + if rc != 0 and check_rc: msg = heuristic_log_sanitize(stderr.rstrip(), self.no_log_values) self.fail_json(cmd=clean_args, rc=rc, stdout=stdout, stderr=stderr, msg=msg) diff --git a/test/units/module_utils/basic/test_run_command.py b/test/units/module_utils/basic/test_run_command.py index 09ab14b6d2..0db6fbe7b9 100644 --- a/test/units/module_utils/basic/test_run_command.py +++ b/test/units/module_utils/basic/test_run_command.py @@ -39,6 +39,7 @@ class OpenStringIO(StringIO): def close(self): pass + @unittest.skipIf(sys.version_info[0] >= 3, "Python 3 is not supported on targets (yet)") class TestAnsibleModuleRunCommand(unittest.TestCase): @@ -111,10 +112,6 @@ class TestAnsibleModuleRunCommand(unittest.TestCase): self.assertEqual(args, ('ls a " b" "c "', )) self.assertEqual(kwargs['shell'], True) - def test_path_prefix(self): - self.module.run_command('foo', path_prefix='/opt/bin') - self.assertEqual('/opt/bin', self.os.environ['PATH'].split(':')[0]) - def test_cwd(self): self.os.getcwd.return_value = '/old' self.module.run_command('/bin/ls', cwd='/new')