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 01b6c7c9c6)

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)))

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

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

lib/ansible/vars/manager.py

@@ -72,14 +72,18 @@ 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_'):
+            if k not in exceptions:
                 del clean[k]
         elif isinstance(dirty[k], dict):
             clean[k] = strip_internal_keys(dirty[k])

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)

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',