diff options
author | Sam Doran <sdoran@redhat.com> | 2019-05-23 10:17:17 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-05-23 10:17:17 -0400 |
commit | 8f4f3750fe24ec2d1bd295b797b1b71e0d0ddd29 (patch) | |
tree | ba9261e78f306d686f6cb713608dbcb65026a84a | |
parent | 22b9525aa95df4cef8f75573af8bcc3783847256 (diff) | |
download | ansible-8f4f3750fe24ec2d1bd295b797b1b71e0d0ddd29.tar.gz |
Ensure uri module always returns status even on failure (#56240)
- Also return url and update docs for other values to indicate they are only returned on success.
- Add integration tests
- Use info variable for common return values
- Use -1 as default status rather than None. This is lines up with with existing code in urls.py
- Add unit tests to ensure status and url are returned on failure
-rw-r--r-- | changelogs/fragments/55897-uri-correct-return-values.yaml | 2 | ||||
-rw-r--r-- | lib/ansible/module_utils/urls.py | 8 | ||||
-rw-r--r-- | lib/ansible/modules/net_tools/basics/uri.py | 4 | ||||
-rw-r--r-- | test/integration/targets/uri/tasks/main.yml | 7 | ||||
-rw-r--r-- | test/units/module_utils/urls/test_fetch_url.py | 6 |
5 files changed, 19 insertions, 8 deletions
diff --git a/changelogs/fragments/55897-uri-correct-return-values.yaml b/changelogs/fragments/55897-uri-correct-return-values.yaml new file mode 100644 index 0000000000..4b6d3bd8e5 --- /dev/null +++ b/changelogs/fragments/55897-uri-correct-return-values.yaml @@ -0,0 +1,2 @@ +bugfixes: + - uri - always return a value for status even during failure (https://github.com/ansible/ansible/issues/55897) diff --git a/lib/ansible/module_utils/urls.py b/lib/ansible/module_utils/urls.py index 895ae71487..fdde2ee261 100644 --- a/lib/ansible/module_utils/urls.py +++ b/lib/ansible/module_utils/urls.py @@ -1429,7 +1429,7 @@ def fetch_url(module, url, data=None, headers=None, method=None, cookies = cookiejar.LWPCookieJar() r = None - info = dict(url=url) + info = dict(url=url, status=-1) try: r = open_url(url, data=data, headers=headers, method=method, use_proxy=use_proxy, force=force, last_mod_time=last_mod_time, timeout=timeout, @@ -1471,11 +1471,11 @@ def fetch_url(module, url, data=None, headers=None, method=None, except NoSSLError as e: distribution = get_distribution() if distribution is not None and distribution.lower() == 'redhat': - module.fail_json(msg='%s. You can also install python-ssl from EPEL' % to_native(e)) + module.fail_json(msg='%s. You can also install python-ssl from EPEL' % to_native(e), **info) else: - module.fail_json(msg='%s' % to_native(e)) + module.fail_json(msg='%s' % to_native(e), **info) except (ConnectionError, ValueError) as e: - module.fail_json(msg=to_native(e)) + module.fail_json(msg=to_native(e), **info) except urllib_error.HTTPError as e: try: body = e.read() diff --git a/lib/ansible/modules/net_tools/basics/uri.py b/lib/ansible/modules/net_tools/basics/uri.py index f1c0c747b6..eea77d8a85 100644 --- a/lib/ansible/modules/net_tools/basics/uri.py +++ b/lib/ansible/modules/net_tools/basics/uri.py @@ -314,7 +314,7 @@ RETURN = r''' # The return information includes all the HTTP headers in lower-case. elapsed: description: The number of seconds that elapsed while performing the download - returned: always + returned: on success type: int sample: 23 msg: @@ -324,7 +324,7 @@ msg: sample: OK (unknown bytes) redirected: description: Whether the request was redirected - returned: always + returned: on success type: bool sample: false status: diff --git a/test/integration/targets/uri/tasks/main.yml b/test/integration/targets/uri/tasks/main.yml index 6210cc23bd..2473bcf705 100644 --- a/test/integration/targets/uri/tasks/main.yml +++ b/test/integration/targets/uri/tasks/main.yml @@ -102,9 +102,12 @@ - name: Assert that the file was not downloaded assert: that: - - "result.failed == true" + - result.failed == true - "'Failed to validate the SSL certificate' in result.msg or ( result.msg is match('hostname .* doesn.t match .*'))" - - "stat_result.stat.exists == false" + - stat_result.stat.exists == false + - result.status is defined + - result.status == -1 + - result.url == 'https://' ~ badssl_host ~ '/' - name: Clean up any cruft from the results directory file: diff --git a/test/units/module_utils/urls/test_fetch_url.py b/test/units/module_utils/urls/test_fetch_url.py index 0af03e74fb..93a9399c91 100644 --- a/test/units/module_utils/urls/test_fetch_url.py +++ b/test/units/module_utils/urls/test_fetch_url.py @@ -157,6 +157,8 @@ def test_fetch_url_nossl(open_url_mock, fake_ansible_module, mocker): fetch_url(fake_ansible_module, 'http://ansible.com/') assert 'python-ssl' in excinfo.value.kwargs['msg'] + assert'http://ansible.com/' == excinfo.value.kwargs['url'] + assert excinfo.value.kwargs['status'] == -1 def test_fetch_url_connectionerror(open_url_mock, fake_ansible_module): @@ -165,12 +167,16 @@ def test_fetch_url_connectionerror(open_url_mock, fake_ansible_module): fetch_url(fake_ansible_module, 'http://ansible.com/') assert excinfo.value.kwargs['msg'] == 'TESTS' + assert'http://ansible.com/' == excinfo.value.kwargs['url'] + assert excinfo.value.kwargs['status'] == -1 open_url_mock.side_effect = ValueError('TESTS') with pytest.raises(FailJson) as excinfo: fetch_url(fake_ansible_module, 'http://ansible.com/') assert excinfo.value.kwargs['msg'] == 'TESTS' + assert'http://ansible.com/' == excinfo.value.kwargs['url'] + assert excinfo.value.kwargs['status'] == -1 def test_fetch_url_httperror(open_url_mock, fake_ansible_module): |