diff options
| -rw-r--r-- | changelogs/fragments/fix_json_module_parsing.yml | 2 | ||||
| -rw-r--r-- | lib/ansible/module_utils/json_utils.py | 4 | ||||
| -rw-r--r-- | lib/ansible/modules/async_wrapper.py | 8 | ||||
| -rw-r--r-- | lib/ansible/plugins/action/__init__.py | 2 | ||||
| -rw-r--r-- | test/integration/targets/json_cleanup/aliases | 1 | ||||
| -rw-r--r-- | test/integration/targets/json_cleanup/library/bad_json | 11 | ||||
| -rw-r--r-- | test/integration/targets/json_cleanup/module_output_cleaning.yml | 26 | ||||
| -rwxr-xr-x | test/integration/targets/json_cleanup/runme.sh | 5 | ||||
| -rw-r--r-- | test/sanity/ignore.txt | 1 |
9 files changed, 51 insertions, 9 deletions
diff --git a/changelogs/fragments/fix_json_module_parsing.yml b/changelogs/fragments/fix_json_module_parsing.yml new file mode 100644 index 0000000000..051aab5912 --- /dev/null +++ b/changelogs/fragments/fix_json_module_parsing.yml @@ -0,0 +1,2 @@ +bugfixes: + - restrict module valid JSON parsed output to objects as lists are not valid responses. diff --git a/lib/ansible/module_utils/json_utils.py b/lib/ansible/module_utils/json_utils.py index d5639fa3f8..0e95aa677c 100644 --- a/lib/ansible/module_utils/json_utils.py +++ b/lib/ansible/module_utils/json_utils.py @@ -32,7 +32,7 @@ import json # NB: a copy of this function exists in ../../modules/core/async_wrapper.py. Ensure any # changes are propagated there. -def _filter_non_json_lines(data): +def _filter_non_json_lines(data, objects_only=False): ''' Used to filter unrelated output around module JSON output, like messages from tcagetattr, or where dropbear spews MOTD on every single command (which is nuts). @@ -50,7 +50,7 @@ def _filter_non_json_lines(data): if line.startswith(u'{'): endchar = u'}' break - elif line.startswith(u'['): + elif not objects_only and line.startswith(u'['): endchar = u']' break else: diff --git a/lib/ansible/modules/async_wrapper.py b/lib/ansible/modules/async_wrapper.py index 5379726ae6..7ba8271ef6 100644 --- a/lib/ansible/modules/async_wrapper.py +++ b/lib/ansible/modules/async_wrapper.py @@ -74,7 +74,7 @@ def _filter_non_json_lines(data): Used to filter unrelated output around module JSON output, like messages from tcagetattr, or where dropbear spews MOTD on every single command (which is nuts). - Filters leading lines before first line-starting occurrence of '{' or '[', and filter all + Filters leading lines before first line-starting occurrence of '{', and filter all trailing lines after matching close character (working from the bottom of output). ''' warnings = [] @@ -85,10 +85,6 @@ def _filter_non_json_lines(data): for start, line in enumerate(lines): line = line.strip() if line.startswith(u'{'): - endchar = u'}' - break - elif line.startswith(u'['): - endchar = u']' break else: raise ValueError('No start of json char found') @@ -97,7 +93,7 @@ def _filter_non_json_lines(data): lines = lines[start:] for reverse_end_offset, line in enumerate(reversed(lines)): - if line.strip().endswith(endchar): + if line.strip().endswith(u'}'): break else: raise ValueError('No end of json char found') diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 15ce29eb0d..f492c08f05 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -1148,7 +1148,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): def _parse_returned_data(self, res): try: - filtered_output, warnings = _filter_non_json_lines(res.get('stdout', u'')) + filtered_output, warnings = _filter_non_json_lines(res.get('stdout', u''), objects_only=True) for w in warnings: display.warning(w) diff --git a/test/integration/targets/json_cleanup/aliases b/test/integration/targets/json_cleanup/aliases new file mode 100644 index 0000000000..765b70da79 --- /dev/null +++ b/test/integration/targets/json_cleanup/aliases @@ -0,0 +1 @@ +shippable/posix/group2 diff --git a/test/integration/targets/json_cleanup/library/bad_json b/test/integration/targets/json_cleanup/library/bad_json new file mode 100644 index 0000000000..1df8c725f0 --- /dev/null +++ b/test/integration/targets/json_cleanup/library/bad_json @@ -0,0 +1,11 @@ +#!/usr/bin/env bash + +set -eu + +echo 'this stuff should be ignored' + +echo '[ looks like a json list]' + +echo '{"changed": false, "failed": false, "msg": "good json response"}' + +echo 'moar garbage' diff --git a/test/integration/targets/json_cleanup/module_output_cleaning.yml b/test/integration/targets/json_cleanup/module_output_cleaning.yml new file mode 100644 index 0000000000..165352aab8 --- /dev/null +++ b/test/integration/targets/json_cleanup/module_output_cleaning.yml @@ -0,0 +1,26 @@ +- name: ensure we clean module output well + hosts: localhost + gather_facts: false + tasks: + - name: call module that spews extra stuff + bad_json: + register: clean_json + ignore_errors: true + + - name: all expected is there + assert: + that: + - clean_json is success + - clean_json is not changed + - "clean_json['msg'] == 'good json response'" + + - name: all non wanted is not there + assert: + that: + - item not in clean_json.values() + loop: + - this stuff should be ignored + - [ looks like a json list] + - '[ looks like a json list]' + - ' looks like a json list' + - moar garbage diff --git a/test/integration/targets/json_cleanup/runme.sh b/test/integration/targets/json_cleanup/runme.sh new file mode 100755 index 0000000000..2de3bd0ecb --- /dev/null +++ b/test/integration/targets/json_cleanup/runme.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +set -eux + +ansible-playbook module_output_cleaning.yml "$@" diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index 8a30f046c5..2fed1829a5 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -167,6 +167,7 @@ test/integration/targets/collections_relative_imports/collection_root/ansible_co test/integration/targets/gathering_facts/library/bogus_facts shebang test/integration/targets/gathering_facts/library/facts_one shebang test/integration/targets/gathering_facts/library/facts_two shebang +test/integration/targets/json_cleanup/library/bad_json shebang test/integration/targets/incidental_win_dsc/files/xTestDsc/1.0.0/DSCResources/ANSIBLE_xSetReboot/ANSIBLE_xSetReboot.psm1 pslint!skip test/integration/targets/incidental_win_dsc/files/xTestDsc/1.0.0/DSCResources/ANSIBLE_xTestResource/ANSIBLE_xTestResource.psm1 pslint!skip test/integration/targets/incidental_win_dsc/files/xTestDsc/1.0.0/xTestDsc.psd1 pslint!skip |