From c09c01a1f523da3b6f1f5597f51f5fc446f5c1df Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 19 Jan 2016 18:37:59 -0500 Subject: [PATCH 01/14] go back to defaulting wrapping commands in shell this was taken out in an effort to default to the user's shell but creates issues as this is not known ahead of time and its painful to set executable and shell_type for all servers, it should only be needed for those that restrict the user to specific shells and when /bin/sh is not available. raw and command may still bypass this by explicitly passing None. fixes #13882 still conditional --- lib/ansible/plugins/action/__init__.py | 5 ++--- test/units/plugins/action/test_action.py | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 0b33d576c0..ad30512150 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -481,8 +481,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): display.debug("done with _execute_module (%s, %s)" % (module_name, module_args)) return data - def _low_level_execute_command(self, cmd, sudoable=True, in_data=None, - executable=None, encoding_errors='replace'): + def _low_level_execute_command(self, cmd, sudoable=True, in_data=None, executable=C.DEFAULT_EXECUTABLE, encoding_errors='replace'): ''' This is the function which executes the low level shell command, which may be commands to create/remove directories for temporary files, or to @@ -498,7 +497,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): ''' if executable is not None: - cmd = executable + ' -c ' + cmd + cmd = executable + ' -c ' + pipes.quote(cmd) display.debug("_low_level_execute_command(): starting") if not cmd: diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 0e47b6a538..afb5d767e1 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -49,7 +49,7 @@ class TestActionBase(unittest.TestCase): play_context.remote_user = 'apo' action_base._low_level_execute_command('ECHO', sudoable=True) - play_context.make_become_cmd.assert_called_once_with('ECHO', executable=None) + play_context.make_become_cmd.assert_called_once_with("/bin/sh -c ECHO", executable='/bin/sh') play_context.make_become_cmd.reset_mock() @@ -58,6 +58,6 @@ class TestActionBase(unittest.TestCase): try: play_context.remote_user = 'root' action_base._low_level_execute_command('ECHO SAME', sudoable=True) - play_context.make_become_cmd.assert_called_once_with('ECHO SAME', executable=None) + play_context.make_become_cmd.assert_called_once_with("/bin/sh -c 'ECHO SAME'", executable='/bin/sh') finally: C.BECOME_ALLOW_SAME_USER = become_allow_same_user From ac89b0de7a0cfa0109c598ed253761f553698cb4 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Wed, 20 Jan 2016 12:16:27 -0500 Subject: [PATCH 02/14] Fix incorrect handling of any_errors_fatal in the linear strategy Instead of bombing out of the strategy, we now properly mark hosts failed so that the play iterator can handle block rescue/always properly. Fixes #14024 --- lib/ansible/plugins/strategy/linear.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py index 40c435ca53..61b0c01503 100644 --- a/lib/ansible/plugins/strategy/linear.py +++ b/lib/ansible/plugins/strategy/linear.py @@ -342,13 +342,20 @@ class StrategyModule(StrategyBase): display.debug("results queue empty") display.debug("checking for any_errors_fatal") - had_failure = include_failure + failed_hosts = [] for res in results: if res.is_failed() or res.is_unreachable(): - had_failure = True - break - if task and task.any_errors_fatal and had_failure: - return False + failed_hosts.append(res._host.name) + + # if any_errors_fatal and we had an error, mark all hosts as failed + if task and task.any_errors_fatal and len(failed_hosts) > 0: + for host in hosts_left: + # don't double-mark hosts, or the iterator will potentially + # fail them out of the rescue/always states + if host.name not in failed_hosts: + self._tqm._failed_hosts[host.name] = True + iterator.mark_host_failed(host) + display.debug("done checking for any_errors_fatal") except (IOError, EOFError) as e: display.debug("got IOError/EOFError in task loop: %s" % e) From d49b11e9962df4bde4b8f3d61029305af4115748 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 20 Jan 2016 13:08:16 -0600 Subject: [PATCH 03/14] Only use os.path.basename if get_file_content returned a value, and ensure that service_mgr has line endings stripped. Fixes #14026 --- lib/ansible/module_utils/facts.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/ansible/module_utils/facts.py b/lib/ansible/module_utils/facts.py index 3cb66a83e8..18fa26332b 100644 --- a/lib/ansible/module_utils/facts.py +++ b/lib/ansible/module_utils/facts.py @@ -559,9 +559,11 @@ class Facts(object): # also other OSs other than linux might need to check across several possible candidates # try various forms of querying pid 1 - proc_1 = os.path.basename(get_file_content('/proc/1/comm')) + proc_1 = get_file_content('/proc/1/comm') if proc_1 is None: rc, proc_1, err = module.run_command("ps -p 1 -o comm|tail -n 1", use_unsafe_shell=True) + else: + proc_1 = os.path.basename(proc_1) if proc_1 == 'init' or proc_1.endswith('sh'): # many systems return init, so this cannot be trusted, if it ends in 'sh' it probalby is a shell in a container @@ -569,7 +571,7 @@ class Facts(object): # if not init/None it should be an identifiable or custom init, so we are done! if proc_1 is not None: - self.facts['service_mgr'] = proc_1 + self.facts['service_mgr'] = proc_1.strip() # start with the easy ones elif self.facts['distribution'] == 'MacOSX': From a68d90a71af4a799cd6f3bd3c3987432278a567a Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Wed, 20 Jan 2016 09:04:44 -0800 Subject: [PATCH 04/14] 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') From a1063827867cada5414577a85e2f13121cb7b948 Mon Sep 17 00:00:00 2001 From: Matthew Jones Date: Wed, 20 Jan 2016 15:03:56 -0500 Subject: [PATCH 05/14] Add a config option for rackspace inventory cache Adding a config and environment variable option for tuning the cache age check in the rackspace inventory module --- contrib/inventory/rax.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contrib/inventory/rax.py b/contrib/inventory/rax.py index 0028f54d20..4ac6b0f47e 100755 --- a/contrib/inventory/rax.py +++ b/contrib/inventory/rax.py @@ -355,9 +355,12 @@ def get_cache_file_path(regions): def _list(regions, refresh_cache=True): + cache_max_age = int(get_config(p, 'rax', 'cache_max_age', + 'RAX_CACHE_MAX_AGE', 600)) + if (not os.path.exists(get_cache_file_path(regions)) or refresh_cache or - (time() - os.stat(get_cache_file_path(regions))[-1]) > 600): + (time() - os.stat(get_cache_file_path(regions))[-1]) > cache_max_age): # Cache file doesn't exist or older than 10m or refresh cache requested _list_into_cache(regions) From 3201f5d90e311735b54af21fc621a9197e1eb788 Mon Sep 17 00:00:00 2001 From: Selivanov Pavel Date: Wed, 20 Jan 2016 23:11:44 +0300 Subject: [PATCH 06/14] plugins/strategy: added significant details to parser error message. See discussion at https://github.com/ansible/ansible/issues/13753 --- lib/ansible/plugins/strategy/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index 0b7f48b563..3013eac3d7 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -492,7 +492,7 @@ class StrategyBase: tags = [ tags ] if len(tags) > 0: if len(b._task_include.tags) > 0: - raise AnsibleParserError("Include tasks should not specify tags in more than one way (both via args and directly on the task)", + raise AnsibleParserError("Include tasks should not specify tags in more than one way (both via args and directly on the task). Mixing tag specify styles is prohibited for whole import hierarchy, not only for single import statement", obj=included_file._task._ds) display.deprecated("You should not specify tags in the include parameters. All tags should be specified using the task-level option") b._task_include.tags = tags From 61009604e33c0e20125d01fc5ed176fec43cdfd5 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Wed, 20 Jan 2016 12:18:52 -0800 Subject: [PATCH 07/14] Update submodules to bring in yum fix --- lib/ansible/modules/core | 2 +- lib/ansible/modules/extras | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/modules/core b/lib/ansible/modules/core index ffea58ee86..d7fac82f97 160000 --- a/lib/ansible/modules/core +++ b/lib/ansible/modules/core @@ -1 +1 @@ -Subproject commit ffea58ee86dbee20dc272c74cd5f8e02f6f317e6 +Subproject commit d7fac82f97c153af08dbea2b2ae9718b19abeb8a diff --git a/lib/ansible/modules/extras b/lib/ansible/modules/extras index e9450df878..f798240f43 160000 --- a/lib/ansible/modules/extras +++ b/lib/ansible/modules/extras @@ -1 +1 @@ -Subproject commit e9450df878632531fae574b5eaf28bf0f7916948 +Subproject commit f798240f436a16a828f48759bbd176b6bccdfe75 From a1318e16641a89cfbd41d072670d374bbd0b3cf7 Mon Sep 17 00:00:00 2001 From: Matthew Jones Date: Wed, 20 Jan 2016 15:27:06 -0500 Subject: [PATCH 08/14] Add rax cache age ini documentation --- contrib/inventory/rax.ini | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/contrib/inventory/rax.ini b/contrib/inventory/rax.ini index 5a269e16a3..15948e7b2e 100644 --- a/contrib/inventory/rax.ini +++ b/contrib/inventory/rax.ini @@ -55,3 +55,12 @@ # will be ignored, and 4 will be used. Accepts a comma separated list, # the first found wins. # access_ip_version = 4 + +# Environment Variable: RAX_CACHE_MAX_AGE +# Default: 600 +# +# A configuration the changes the behavior or the inventory cache. +# Inventory listing performed before this value will be returned from +# the cache instead of making a full request for all inventory. Setting +# this value to 0 will force a full request. +# cache_max_age = 600 \ No newline at end of file From 54cde0d08244fe6eb0fb53fc9f3174eee6f660aa Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Wed, 20 Jan 2016 15:26:45 -0500 Subject: [PATCH 09/14] Standardize removal of BECOME-SUCCESS method and use it for async too Fixes #13965 Fixes #13971 --- lib/ansible/plugins/action/__init__.py | 9 +++++++++ lib/ansible/plugins/action/async.py | 4 ++++ lib/ansible/plugins/action/raw.py | 5 +---- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index ad30512150..62a2e7806f 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -24,6 +24,7 @@ import json import os import pipes import random +import re import stat import tempfile import time @@ -356,6 +357,14 @@ class ActionBase(with_metaclass(ABCMeta, object)): return data[idx:] + def _strip_success_message(self, data): + ''' + Removes the BECOME-SUCCESS message from the data. + ''' + if data.strip().startswith('BECOME-SUCCESS-'): + data = re.sub(r'^((\r)?\n)?BECOME-SUCCESS.*(\r)?\n', '', data) + return data + def _execute_module(self, module_name=None, module_args=None, tmp=None, task_vars=None, persist_files=False, delete_remote_tmp=True): ''' Transfer and run a module along with its arguments. diff --git a/lib/ansible/plugins/action/async.py b/lib/ansible/plugins/action/async.py index 8a7175aeb8..5e04f37ff1 100644 --- a/lib/ansible/plugins/action/async.py +++ b/lib/ansible/plugins/action/async.py @@ -75,4 +75,8 @@ class ActionModule(ActionBase): result['changed'] = True + # be sure to strip out the BECOME-SUCCESS message, which may + # be there depending on the output of the module + result['stdout'] = self._strip_success_message(result.get('stdout', '')) + return result diff --git a/lib/ansible/plugins/action/raw.py b/lib/ansible/plugins/action/raw.py index d6fa2f3559..c9718db413 100644 --- a/lib/ansible/plugins/action/raw.py +++ b/lib/ansible/plugins/action/raw.py @@ -19,8 +19,6 @@ __metaclass__ = type from ansible.plugins.action import ActionBase -import re - class ActionModule(ActionBase): TRANSFERS_FILES = False @@ -42,7 +40,6 @@ class ActionModule(ActionBase): # for some modules (script, raw), the sudo success key # may leak into the stdout due to the way the sudo/su # command is constructed, so we filter that out here - if result.get('stdout','').strip().startswith('BECOME-SUCCESS-'): - result['stdout'] = re.sub(r'^((\r)?\n)?BECOME-SUCCESS.*(\r)?\n', '', result['stdout']) + result['stdout'] = self._strip_success_message(result.get('stdout', '')) return result From c2ac1507ea7b34ed7ce7ea957e3c8c8e6377625a Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Wed, 20 Jan 2016 18:31:40 -0500 Subject: [PATCH 10/14] corrected host/group match in inventory_hostnames now the lookup works when using ! and & operators fixes #13997 --- lib/ansible/plugins/lookup/inventory_hostnames.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/ansible/plugins/lookup/inventory_hostnames.py b/lib/ansible/plugins/lookup/inventory_hostnames.py index a86d2270bb..651055b6f7 100644 --- a/lib/ansible/plugins/lookup/inventory_hostnames.py +++ b/lib/ansible/plugins/lookup/inventory_hostnames.py @@ -26,10 +26,15 @@ class LookupModule(LookupBase): def get_hosts(self, variables, pattern): hosts = [] - if pattern in variables['groups']: - hosts = variables['groups'][pattern] - elif pattern in variables['groups']['all']: - hosts = [pattern] + if pattern[0] in ('!','&'): + obj = pattern[1:] + else: + obj = pattern + + if obj in variables['groups']: + hosts = variables['groups'][obj] + elif obj in variables['groups']['all']: + hosts = [obj] return hosts def run(self, terms, variables=None, **kwargs): From 365c5b23ce8dad5fcbaadcb9b86b5f7da4874437 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Wed, 20 Jan 2016 18:23:30 -0500 Subject: [PATCH 11/14] Re-add cache clearing call to Inventory init This prevents a bug where the existing cache outside of the class is not cleared when creating a new Inventory object. This only really affects people using the API directly right now, but wanted to fix it to prevent weird errors from popping up. --- lib/ansible/inventory/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index eb8d190550..3d9ad3516d 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -78,6 +78,10 @@ class Inventory(object): self._restriction = None self._subset = None + # clear the cache here, which is only useful if more than + # one Inventory objects are created when using the API directly + self.clear_pattern_cache() + self.parse_inventory(host_list) def serialize(self): From 627dec716b1aee04676e2b231609ff0890cd966e Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Wed, 20 Jan 2016 20:53:31 -0500 Subject: [PATCH 12/14] Template the run_once value in the linear strategy as we use it there This is pre-post_validation, so we have to template it on the fly as we use it to determine if we bypass the host loop. Fixes #11876 --- lib/ansible/plugins/strategy/linear.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py index 61b0c01503..7750f50ff0 100644 --- a/lib/ansible/plugins/strategy/linear.py +++ b/lib/ansible/plugins/strategy/linear.py @@ -194,8 +194,6 @@ class StrategyModule(StrategyBase): try: action = action_loader.get(task.action, class_only=True) - if task.run_once or getattr(action, 'BYPASS_HOST_LOOP', False): - run_once = True except KeyError: # we don't care here, because the action may simply not have a # corresponding action plugin @@ -227,6 +225,8 @@ class StrategyModule(StrategyBase): templar = Templar(loader=self._loader, variables=task_vars) display.debug("done getting variables") + run_once = templar.template(task.run_once) + if not callback_sent: display.debug("sending task start callback, copying the task so we can template it temporarily") saved_name = task.name @@ -249,7 +249,7 @@ class StrategyModule(StrategyBase): self._queue_task(host, task, task_vars, play_context) # if we're bypassing the host loop, break out now - if run_once: + if run_once or getattr(action, 'BYPASS_HOST_LOOP', False): break results += self._process_pending_results(iterator, one_pass=True) From f26adcc7da7f8e6605167203249648f7b0e74fb7 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 21 Jan 2016 10:53:02 -0500 Subject: [PATCH 13/14] avoid shredding empty files, also x/0 also cleaned up unused import and exception var --- lib/ansible/parsing/vault/__init__.py | 32 ++++++++++++++------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/ansible/parsing/vault/__init__.py b/lib/ansible/parsing/vault/__init__.py index f38525e028..dc30dd0ffb 100644 --- a/lib/ansible/parsing/vault/__init__.py +++ b/lib/ansible/parsing/vault/__init__.py @@ -71,7 +71,7 @@ try: except ImportError: pass -from ansible.compat.six import PY3, byte2int +from ansible.compat.six import PY3 from ansible.utils.unicode import to_unicode, to_bytes HAS_ANY_PBKDF2HMAC = HAS_PBKDF2 or HAS_PBKDF2HMAC @@ -236,22 +236,24 @@ class VaultEditor: """ file_len = os.path.getsize(tmp_path) - max_chunk_len = min(1024*1024*2, file_len) - passes = 3 - with open(tmp_path, "wb") as fh: - for _ in range(passes): - fh.seek(0, 0) - # get a random chunk of data, each pass with other length - chunk_len = random.randint(max_chunk_len//2, max_chunk_len) - data = os.urandom(chunk_len) + if file_len > 0: # avoid work when file was empty + max_chunk_len = min(1024*1024*2, file_len) - for _ in range(0, file_len // chunk_len): - fh.write(data) - fh.write(data[:file_len % chunk_len]) + passes = 3 + with open(tmp_path, "wb") as fh: + for _ in range(passes): + fh.seek(0, 0) + # get a random chunk of data, each pass with other length + chunk_len = random.randint(max_chunk_len//2, max_chunk_len) + data = os.urandom(chunk_len) - assert(fh.tell() == file_len) # FIXME remove this assert once we have unittests to check its accuracy - os.fsync(fh) + for _ in range(0, file_len // chunk_len): + fh.write(data) + fh.write(data[:file_len % chunk_len]) + + assert(fh.tell() == file_len) # FIXME remove this assert once we have unittests to check its accuracy + os.fsync(fh) def _shred_file(self, tmp_path): @@ -273,7 +275,7 @@ class VaultEditor: try: r = call(['shred', tmp_path]) - except OSError as e: + except OSError: # shred is not available on this system, or some other error occured. r = 1 From d6ae9e2c291cdc8f60f066c7339ef28731e96d4e Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Tue, 19 Jan 2016 17:56:07 +0100 Subject: [PATCH 14/14] Avoid recursively checking JSON inventory for Unicode by moving to en-bloc unicode conversion to act on scripts stdout Both python-json and simplejson always return unicode strings when using their loads() method on unicode strings. This is true at least since 2009. This makes checking each substring unnecessary, because we do not need to recursively check the strings contained in the inventory dict later one-by-one This commit makes parsing of large dynamic inventory at least 2 seconds faster. cf: https://github.com/towolf/ansible-large-inventory-testcase --- lib/ansible/inventory/script.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/ansible/inventory/script.py b/lib/ansible/inventory/script.py index 042fa8c24a..999e472f53 100644 --- a/lib/ansible/inventory/script.py +++ b/lib/ansible/inventory/script.py @@ -31,7 +31,7 @@ from ansible.errors import AnsibleError from ansible.inventory.host import Host from ansible.inventory.group import Group from ansible.module_utils.basic import json_dict_bytes_to_unicode -from ansible.utils.unicode import to_str +from ansible.utils.unicode import to_str, to_unicode class InventoryScript: @@ -58,7 +58,13 @@ class InventoryScript: if sp.returncode != 0: raise AnsibleError("Inventory script (%s) had an execution error: %s " % (filename,stderr)) - self.data = stdout + # make sure script output is unicode so that json loader will output + # unicode strings itself + try: + self.data = to_unicode(stdout, errors="strict") + except Exception as e: + raise AnsibleError("inventory data from {0} contained characters that cannot be interpreted as UTF-8: {1}".format(to_str(self.filename), to_str(e))) + # see comment about _meta below self.host_vars_from_top = None self._parse(stderr) @@ -78,8 +84,6 @@ class InventoryScript: sys.stderr.write(err + "\n") raise AnsibleError("failed to parse executable inventory script results from {0}: data needs to be formatted as a json dict".format(to_str(self.filename))) - self.raw = json_dict_bytes_to_unicode(self.raw) - group = None for (group_name, data) in self.raw.items():