From 957b376f9eb959f4f3627a622f7776a26442bf9c Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 22 Dec 2015 22:45:25 -0500 Subject: [PATCH] better module error handling * now module errors clearly state msg=MODULE FAILURE * module's stdout and stderr go into module_stdout and module_stderr keys which only appear during parsing failure * invocation module_args are deleted from results provided by action plugin as errors can keep us from overwriting and then disclosing info that was meant to be kept hidden due to no_log * fixed invocation module_args set by basic.py as it was creating different keys as the invocation in action plugin base. * results now merge --- lib/ansible/module_utils/basic.py | 4 ++-- lib/ansible/plugins/action/__init__.py | 5 +++-- lib/ansible/plugins/action/normal.py | 5 ++++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 91ea874d859..0391035e883 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -1530,7 +1530,7 @@ class AnsibleModule(object): if not 'changed' in kwargs: kwargs['changed'] = False if 'invocation' not in kwargs: - kwargs['invocation'] = self.params + kwargs['invocation'] = {'module_args': self.params} kwargs = remove_values(kwargs, self.no_log_values) self.do_cleanup_files() print(self.jsonify(kwargs)) @@ -1542,7 +1542,7 @@ class AnsibleModule(object): assert 'msg' in kwargs, "implementation error -- msg to explain the error is required" kwargs['failed'] = True if 'invocation' not in kwargs: - kwargs['invocation'] = self.params + kwargs['invocation'] = {'module_args': self.params} kwargs = remove_values(kwargs, self.no_log_values) self.do_cleanup_files() print(self.jsonify(kwargs)) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 765ba663164..5383f8afd43 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -460,9 +460,10 @@ class ActionBase(with_metaclass(ABCMeta, object)): if 'stderr' in res and res['stderr'].startswith(u'Traceback'): data['exception'] = res['stderr'] else: - data['msg'] = res.get('stdout', u'') + data['msg'] = "MODULE FAILURE" + data['module_stdout'] = res.get('stdout', u'') if 'stderr' in res: - data['msg'] += res['stderr'] + data['module_stderr'] = res['stderr'] # pre-split stdout into lines, if stdout is in the data and there # isn't already a stdout_lines value there diff --git a/lib/ansible/plugins/action/normal.py b/lib/ansible/plugins/action/normal.py index f9b55e1ff57..932ad8309c3 100644 --- a/lib/ansible/plugins/action/normal.py +++ b/lib/ansible/plugins/action/normal.py @@ -18,6 +18,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type from ansible.plugins.action import ActionBase +from ansible.utils.vars import merge_hash class ActionModule(ActionBase): @@ -27,7 +28,9 @@ class ActionModule(ActionBase): task_vars = dict() results = super(ActionModule, self).run(tmp, task_vars) - results.update(self._execute_module(tmp=tmp, task_vars=task_vars)) + # remove as modules might hide due to nolog + del results['invocation']['module_args'] + results = merge_hash(results, self._execute_module(tmp=tmp, task_vars=task_vars)) # Remove special fields from the result, which can only be set # internally by the executor engine. We do this only here in # the 'normal' action, as other action plugins may set this.