From c817bef3ae39e8c2f43481c69e387776bc4e2b8c Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Tue, 4 Dec 2018 14:42:24 -0800 Subject: [PATCH] Fix for callback plugins on Python3 when a module returns dictionary keys that aren't strings This fixes one of the problems reported in https://github.com/ansible/ansible/issues/49343 Upstream Python3 bug for the json traceback: https://bugs.python.org/issue25457 and PR that may fix it: https://github.com/python/cpython/pull/8011 --- .../fragments/fix-callbacks-mixed-keys.yaml | 4 +++ lib/ansible/plugins/callback/__init__.py | 22 +++++++++++- test/units/plugins/callback/test_callback.py | 36 +++++++++++++------ 3 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 changelogs/fragments/fix-callbacks-mixed-keys.yaml diff --git a/changelogs/fragments/fix-callbacks-mixed-keys.yaml b/changelogs/fragments/fix-callbacks-mixed-keys.yaml new file mode 100644 index 00000000000..a922cb41c71 --- /dev/null +++ b/changelogs/fragments/fix-callbacks-mixed-keys.yaml @@ -0,0 +1,4 @@ +--- +bugfixes: + - Fix for callback plugins on Python3 when a module returns non-string field + names in its results. (https://github.com/ansible/ansible/issues/49343) diff --git a/lib/ansible/plugins/callback/__init__.py b/lib/ansible/plugins/callback/__init__.py index 7b02bd1dc72..cbf985781b8 100644 --- a/lib/ansible/plugins/callback/__init__.py +++ b/lib/ansible/plugins/callback/__init__.py @@ -29,12 +29,21 @@ from copy import deepcopy from ansible import constants as C from ansible.module_utils.common._collections_compat import MutableMapping +from ansible.module_utils.six import PY3 +from ansible.module_utils._text import to_text from ansible.parsing.ajson import AnsibleJSONEncoder from ansible.plugins import AnsiblePlugin, get_plugin_class from ansible.utils.color import stringc from ansible.utils.display import Display from ansible.vars.clean import strip_internal_keys +if PY3: + # OrderedDict is needed for a backwards compat shim on Python3.x only + # https://github.com/ansible/ansible/pull/49512 + from collections import OrderedDict +else: + OrderedDict = None + global_display = Display() try: @@ -119,7 +128,18 @@ class CallbackBase(AnsiblePlugin): if 'exception' in abridged_result: del abridged_result['exception'] - return json.dumps(abridged_result, cls=AnsibleJSONEncoder, indent=indent, ensure_ascii=False, sort_keys=sort_keys) + try: + jsonified_results = json.dumps(abridged_result, cls=AnsibleJSONEncoder, indent=indent, ensure_ascii=False, sort_keys=sort_keys) + except TypeError: + # Python3 bug: throws an exception when keys are non-homogenous types: + # https://bugs.python.org/issue25457 + # sort into an OrderedDict and then json.dumps() that instead + if not OrderedDict: + raise + jsonified_results = json.dumps(OrderedDict(sorted(abridged_result.items(), key=to_text)), + cls=AnsibleJSONEncoder, indent=indent, + ensure_ascii=False, sort_keys=False) + return jsonified_results def _handle_warnings(self, res): ''' display warnings, if enabled and any exist in the result ''' diff --git a/test/units/plugins/callback/test_callback.py b/test/units/plugins/callback/test_callback.py index 04c395d3c61..01b89e0811e 100644 --- a/test/units/plugins/callback/test_callback.py +++ b/test/units/plugins/callback/test_callback.py @@ -19,6 +19,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import json import re import textwrap import types @@ -26,6 +27,7 @@ import types from units.compat import unittest from units.compat.mock import patch, mock_open, MagicMock +import pytest from ansible.plugins.callback import CallbackBase @@ -133,7 +135,7 @@ class TestCallbackResults(unittest.TestCase): self.assertEqual(result, expected_result) -class TestCallbackDumpResults(unittest.TestCase): +class TestCallbackDumpResults(object): def test_internal_keys(self): cb = CallbackBase() result = {'item': 'some_item', @@ -144,26 +146,26 @@ class TestCallbackDumpResults(unittest.TestCase): 'bad_dict_key': {'_ansible_internal_blah': 'SENTINEL'}, 'changed': True} json_out = cb._dump_results(result) - self.assertFalse('"_ansible_' in json_out) - self.assertFalse('SENTINEL' in json_out) - self.assertTrue('LEFTIN' in json_out) + assert '"_ansible_' not in json_out + assert 'SENTINEL' not in json_out + assert 'LEFTIN' in json_out def test_exception(self): cb = CallbackBase() result = {'item': 'some_item LEFTIN', 'exception': ['frame1', 'SENTINEL']} json_out = cb._dump_results(result) - self.assertFalse('SENTINEL' in json_out) - self.assertFalse('exception' in json_out) - self.assertTrue('LEFTIN' in json_out) + assert 'SENTINEL' not in json_out + assert 'exception' not in json_out + assert 'LEFTIN' in json_out def test_verbose(self): cb = CallbackBase() result = {'item': 'some_item LEFTIN', '_ansible_verbose_always': 'chicane'} json_out = cb._dump_results(result) - self.assertFalse('SENTINEL' in json_out) - self.assertTrue('LEFTIN' in json_out) + assert 'SENTINEL' not in json_out + assert 'LEFTIN' in json_out def test_diff(self): cb = CallbackBase() @@ -171,8 +173,20 @@ class TestCallbackDumpResults(unittest.TestCase): 'diff': ['remove stuff', 'added LEFTIN'], '_ansible_verbose_always': 'chicane'} json_out = cb._dump_results(result) - self.assertFalse('SENTINEL' in json_out) - self.assertTrue('LEFTIN' in json_out) + assert 'SENTINEL' not in json_out + assert 'LEFTIN' in json_out + + def test_mixed_keys(self): + cb = CallbackBase() + result = {3: 'pi', + 'tau': 6} + json_out = cb._dump_results(result) + round_trip_result = json.loads(json_out) + assert len(round_trip_result) == 2 + assert '3' in round_trip_result + assert 'tau' in round_trip_result + assert round_trip_result['3'] == 'pi' + assert round_trip_result['tau'] == 6 class TestCallbackDiff(unittest.TestCase):