Skip to content

Commit

Permalink
Ensure uri module always returns status even on failure (#56240)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
samdoran committed May 23, 2019
1 parent 22b9525 commit 8f4f375
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 8 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/55897-uri-correct-return-values.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- uri - always return a value for status even during failure (https://github.com/ansible/ansible/issues/55897)
8 changes: 4 additions & 4 deletions lib/ansible/module_utils/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions lib/ansible/modules/net_tools/basics/uri.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@
# 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:
Expand All @@ -324,7 +324,7 @@
sample: OK (unknown bytes)
redirected:
description: Whether the request was redirected
returned: always
returned: on success
type: bool
sample: false
status:
Expand Down
7 changes: 5 additions & 2 deletions test/integration/targets/uri/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions test/units/module_utils/urls/test_fetch_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand Down

0 comments on commit 8f4f375

Please sign in to comment.