diff options
author | Brian Coca <bcoca@users.noreply.github.com> | 2021-04-05 00:02:23 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-04-04 23:02:23 -0500 |
commit | 69d18e61edf959fa9b1383faf4bfbe7d30ed970d (patch) | |
tree | 200dfad585a9f9a92f1ac7eca62e4e3e31bc9d78 | |
parent | 51852557dffc2ce5f501c1ab4986e867f55fec07 (diff) | |
download | ansible-69d18e61edf959fa9b1383faf4bfbe7d30ed970d.tar.gz |
module output is only json objects (#73765) (#73777)
remove json lists as they are not valid from modules
fixes #73744
(cherry picked from commit 43300e22798e4c9bd8ec2e321d28c5e8d2018aeb)
-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 4e5e82adb7..84e4b9a9d3 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -1029,7 +1029,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 f0c2d1ed98..5275f076ed 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -236,6 +236,7 @@ test/integration/targets/ignore_unreachable/fake_connectors/bad_exec.py future-i test/integration/targets/ignore_unreachable/fake_connectors/bad_exec.py metaclass-boilerplate test/integration/targets/ignore_unreachable/fake_connectors/bad_put_file.py future-import-boilerplate test/integration/targets/ignore_unreachable/fake_connectors/bad_put_file.py metaclass-boilerplate +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 |