summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSam Doran <sdoran@redhat.com>2019-05-23 10:17:17 -0400
committerGitHub <noreply@github.com>2019-05-23 10:17:17 -0400
commit8f4f3750fe24ec2d1bd295b797b1b71e0d0ddd29 (patch)
treeba9261e78f306d686f6cb713608dbcb65026a84a
parent22b9525aa95df4cef8f75573af8bcc3783847256 (diff)
downloadansible-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.yaml2
-rw-r--r--lib/ansible/module_utils/urls.py8
-rw-r--r--lib/ansible/modules/net_tools/basics/uri.py4
-rw-r--r--test/integration/targets/uri/tasks/main.yml7
-rw-r--r--test/units/module_utils/urls/test_fetch_url.py6
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):