From b61bfc87b85776a633ba27198a1587048079d60e Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 16 Oct 2017 09:44:11 -0400 Subject: [PATCH] better cleanup on task results display (#27175) * better cleanup on task results display callbacks get 'clean' copy of result objects moved cleanup into result object itself removed now redundant callback cleanup moved no_log tests * moved import as per feedback (cherry picked from commit 01b6c7c9c6b7459a3cb53ffc2fe02a8dcc1a3acc) --- lib/ansible/executor/task_queue_manager.py | 14 +++++++- lib/ansible/executor/task_result.py | 34 ++++++++++++++++++++ lib/ansible/plugins/callback/__init__.py | 8 ++--- lib/ansible/vars/manager.py | 8 +++-- test/units/executor/test_task_result.py | 9 ++++++ test/units/plugins/callback/test_callback.py | 10 ------ 6 files changed, 64 insertions(+), 19 deletions(-) diff --git a/lib/ansible/executor/task_queue_manager.py b/lib/ansible/executor/task_queue_manager.py index a70a3923c9..d8f41ccf65 100644 --- a/lib/ansible/executor/task_queue_manager.py +++ b/lib/ansible/executor/task_queue_manager.py @@ -27,6 +27,7 @@ from ansible import constants as C from ansible.errors import AnsibleError from ansible.executor.play_iterator import PlayIterator from ansible.executor.stats import AggregateStats +from ansible.executor.task_result import TaskResult from ansible.module_utils.six import string_types from ansible.module_utils._text import to_text from ansible.playbook.block import Block @@ -371,9 +372,20 @@ class TaskQueueManager: if gotit is not None: methods.append(gotit) + # send clean copies + new_args = [] + for arg in args: + # FIXME: add play/task cleaners + if isinstance(arg, TaskResult): + new_args.append(arg.clean_copy()) + # elif isinstance(arg, Play): + # elif isinstance(arg, Task): + else: + new_args.append(arg) + for method in methods: try: - method(*args, **kwargs) + method(*new_args, **kwargs) except Exception as e: # TODO: add config toggle to make this fatal or not? display.warning(u"Failure using method (%s) in callback plugin (%s): %s" % (to_text(method_name), to_text(callback_plugin), to_text(e))) diff --git a/lib/ansible/executor/task_result.py b/lib/ansible/executor/task_result.py index f62d430bf1..8cea14380d 100644 --- a/lib/ansible/executor/task_result.py +++ b/lib/ansible/executor/task_result.py @@ -19,7 +19,12 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +from copy import deepcopy + from ansible.parsing.dataloader import DataLoader +from ansible.vars.manager import strip_internal_keys + +_IGNORE = ('changed', 'failed', 'skipped') class TaskResult: @@ -83,3 +88,32 @@ class TaskResult: if isinstance(res, dict): flag |= res.get(key, False) return flag + + def clean_copy(self): + + ''' returns 'clean' taskresult object ''' + + # FIXME: clean task_fields, _task and _host copies + result = TaskResult(self._host, self._task, {}, self._task_fields) + + # statuses are already reflected on the event type + if result._task and result._task.action in ['debug']: + # debug is verbose by default to display vars, no need to add invocation + ignore = _IGNORE + ('invocation',) + else: + ignore = _IGNORE + + if self._result.get('_ansible_no_log', False): + result._result = {"censored": "the output has been hidden due to the fact that 'no_log: true' was specified for this result"} + elif self._result: + result._result = deepcopy(self._result) + + # actualy remove + for remove_key in ignore: + if remove_key in result._result: + del result._result[remove_key] + + # remove almost ALL internal keys, keep ones relevant to callback + strip_internal_keys(result._result, exceptions=('_ansible_verbose_always', '_ansible_item_label', '_ansible_no_log')) + + return result diff --git a/lib/ansible/plugins/callback/__init__.py b/lib/ansible/plugins/callback/__init__.py index 739fb24e48..76bafd5932 100644 --- a/lib/ansible/plugins/callback/__init__.py +++ b/lib/ansible/plugins/callback/__init__.py @@ -85,8 +85,6 @@ class CallbackBase(AnsiblePlugin): self._plugin_options = options def _dump_results(self, result, indent=None, sort_keys=True, keep_invocation=False): - if result.get('_ansible_no_log', False): - return json.dumps(dict(censored="the output has been hidden due to the fact that 'no_log: true' was specified for this result")) if not indent and (result.get('_ansible_verbose_always') or self._display.verbosity > 2): indent = 4 @@ -219,10 +217,8 @@ class CallbackBase(AnsiblePlugin): del result._result['results'] def _clean_results(self, result, task_name): - if task_name in ['debug']: - for remove_key in ('changed', 'invocation', 'failed', 'skipped'): - if remove_key in result: - del result[remove_key] + ''' removes data from results for display ''' + pass def set_play_context(self, play_context): pass diff --git a/lib/ansible/vars/manager.py b/lib/ansible/vars/manager.py index 658eda0b38..e203dbe681 100644 --- a/lib/ansible/vars/manager.py +++ b/lib/ansible/vars/manager.py @@ -72,15 +72,19 @@ def preprocess_vars(a): return data -def strip_internal_keys(dirty): +def strip_internal_keys(dirty, exceptions=None): ''' All keys stating with _ansible_ are internal, so create a copy of the 'dirty' dict and remove them from the clean one before returning it ''' + + if exceptions is None: + exceptions = () clean = dirty.copy() for k in dirty.keys(): if isinstance(k, string_types) and k.startswith('_ansible_'): - del clean[k] + if k not in exceptions: + del clean[k] elif isinstance(dirty[k], dict): clean[k] = strip_internal_keys(dirty[k]) return clean diff --git a/test/units/executor/test_task_result.py b/test/units/executor/test_task_result.py index 74717b73d5..82e20dc168 100644 --- a/test/units/executor/test_task_result.py +++ b/test/units/executor/test_task_result.py @@ -138,3 +138,12 @@ class TestTaskResult(unittest.TestCase): # test with failed_when in result tr = TaskResult(mock_host, mock_task, dict(failed_when_result=True)) self.assertTrue(tr.is_failed()) + + def test_task_result_no_log(self): + mock_host = MagicMock() + mock_task = MagicMock() + + # no_log should remove secrets + tr = TaskResult(mock_host, mock_task, dict(_ansible_no_log=True, secret='DONTSHOWME')) + clean = tr.clean_copy() + self.assertTrue('secret' not in clean._result) diff --git a/test/units/plugins/callback/test_callback.py b/test/units/plugins/callback/test_callback.py index c793bc5995..0e230472fa 100644 --- a/test/units/plugins/callback/test_callback.py +++ b/test/units/plugins/callback/test_callback.py @@ -93,16 +93,6 @@ class TestCallbackDumpResults(unittest.TestCase): self.assertFalse('SENTINEL' in json_out) self.assertTrue('LEFTIN' in json_out) - def test_no_log(self): - cb = CallbackBase() - result = {'item': 'some_item', - '_ansible_no_log': True, - 'some_secrets': 'SENTINEL'} - json_out = cb._dump_results(result) - self.assertFalse('SENTINEL' in json_out) - self.assertTrue('no_log' in json_out) - self.assertTrue('output has been hidden' in json_out) - def test_exception(self): cb = CallbackBase() result = {'item': 'some_item LEFTIN',