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 failurepull/4420/head
parent
22b9525aa9
commit
8f4f3750fe
|
@ -0,0 +1,2 @@
|
||||||
|
bugfixes:
|
||||||
|
- uri - always return a value for status even during failure (https://github.com/ansible/ansible/issues/55897)
|
|
@ -1429,7 +1429,7 @@ def fetch_url(module, url, data=None, headers=None, method=None,
|
||||||
cookies = cookiejar.LWPCookieJar()
|
cookies = cookiejar.LWPCookieJar()
|
||||||
|
|
||||||
r = None
|
r = None
|
||||||
info = dict(url=url)
|
info = dict(url=url, status=-1)
|
||||||
try:
|
try:
|
||||||
r = open_url(url, data=data, headers=headers, method=method,
|
r = open_url(url, data=data, headers=headers, method=method,
|
||||||
use_proxy=use_proxy, force=force, last_mod_time=last_mod_time, timeout=timeout,
|
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:
|
except NoSSLError as e:
|
||||||
distribution = get_distribution()
|
distribution = get_distribution()
|
||||||
if distribution is not None and distribution.lower() == 'redhat':
|
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:
|
else:
|
||||||
module.fail_json(msg='%s' % to_native(e))
|
module.fail_json(msg='%s' % to_native(e), **info)
|
||||||
except (ConnectionError, ValueError) as e:
|
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:
|
except urllib_error.HTTPError as e:
|
||||||
try:
|
try:
|
||||||
body = e.read()
|
body = e.read()
|
||||||
|
|
|
@ -314,7 +314,7 @@ RETURN = r'''
|
||||||
# The return information includes all the HTTP headers in lower-case.
|
# The return information includes all the HTTP headers in lower-case.
|
||||||
elapsed:
|
elapsed:
|
||||||
description: The number of seconds that elapsed while performing the download
|
description: The number of seconds that elapsed while performing the download
|
||||||
returned: always
|
returned: on success
|
||||||
type: int
|
type: int
|
||||||
sample: 23
|
sample: 23
|
||||||
msg:
|
msg:
|
||||||
|
@ -324,7 +324,7 @@ msg:
|
||||||
sample: OK (unknown bytes)
|
sample: OK (unknown bytes)
|
||||||
redirected:
|
redirected:
|
||||||
description: Whether the request was redirected
|
description: Whether the request was redirected
|
||||||
returned: always
|
returned: on success
|
||||||
type: bool
|
type: bool
|
||||||
sample: false
|
sample: false
|
||||||
status:
|
status:
|
||||||
|
|
|
@ -102,9 +102,12 @@
|
||||||
- name: Assert that the file was not downloaded
|
- name: Assert that the file was not downloaded
|
||||||
assert:
|
assert:
|
||||||
that:
|
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 .*'))"
|
- "'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
|
- name: Clean up any cruft from the results directory
|
||||||
file:
|
file:
|
||||||
|
|
|
@ -157,6 +157,8 @@ def test_fetch_url_nossl(open_url_mock, fake_ansible_module, mocker):
|
||||||
fetch_url(fake_ansible_module, 'http://ansible.com/')
|
fetch_url(fake_ansible_module, 'http://ansible.com/')
|
||||||
|
|
||||||
assert 'python-ssl' in excinfo.value.kwargs['msg']
|
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):
|
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/')
|
fetch_url(fake_ansible_module, 'http://ansible.com/')
|
||||||
|
|
||||||
assert excinfo.value.kwargs['msg'] == 'TESTS'
|
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')
|
open_url_mock.side_effect = ValueError('TESTS')
|
||||||
with pytest.raises(FailJson) as excinfo:
|
with pytest.raises(FailJson) as excinfo:
|
||||||
fetch_url(fake_ansible_module, 'http://ansible.com/')
|
fetch_url(fake_ansible_module, 'http://ansible.com/')
|
||||||
|
|
||||||
assert excinfo.value.kwargs['msg'] == 'TESTS'
|
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):
|
def test_fetch_url_httperror(open_url_mock, fake_ansible_module):
|
||||||
|
|
Loading…
Reference in New Issue