From 9c5d3060e5db94d851b97fbaa0fcd814a99435ba Mon Sep 17 00:00:00 2001 From: Mike Bonnet Date: Tue, 23 Jan 2024 05:54:18 -0800 Subject: [PATCH] uri: Handle the "force" parameter properly (#82187) * uri: Two tests that demonstrate missing handling of the "force" parameter Add unit and integration tests that demonstrate that the uri module is not handling the "force" parameter. The unit test demonstrates that when "force" is present in the module parameters, it is not being passed through to fetch_url(). The integration test demonstrates that "force" does not disable caching as documented, and calls with a "dest" parameter that points to an existing file can result in a "304 Not Modified" response. * uri: Handle the "force" parameter properly The uri module documents a "force" parameter that can be used to disable caching. The module accepted the parameter but didn't pass it through to the fetch_url() method which implements the logic to handle setting the appropriate headers for disabling caching. This change passes the "force" parameter through as expected, allowing caching to be disabled when requested by the module caller. --- .../fragments/82187-uri-handle-force.yml | 6 +++ lib/ansible/modules/uri.py | 2 +- test/integration/targets/uri/tasks/main.yml | 16 +++++++ test/units/modules/test_uri.py | 43 +++++++++++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/82187-uri-handle-force.yml create mode 100644 test/units/modules/test_uri.py diff --git a/changelogs/fragments/82187-uri-handle-force.yml b/changelogs/fragments/82187-uri-handle-force.yml new file mode 100644 index 00000000000..752c259e546 --- /dev/null +++ b/changelogs/fragments/82187-uri-handle-force.yml @@ -0,0 +1,6 @@ +--- +bugfixes: + - ansible.builtin.uri - the module was ignoring the ``force`` parameter and always + requesting a cached copy (via the ``If-Modified-Since`` header) when downloading + to an existing local file. Disable caching when ``force`` is ``true``, as + documented (https://github.com/ansible/ansible/issues/82166). diff --git a/lib/ansible/modules/uri.py b/lib/ansible/modules/uri.py index 343c814ae00..d721c02487f 100644 --- a/lib/ansible/modules/uri.py +++ b/lib/ansible/modules/uri.py @@ -584,7 +584,7 @@ def uri(module, url, dest, body, body_format, method, headers, socket_timeout, c method=method, timeout=socket_timeout, unix_socket=module.params['unix_socket'], ca_path=ca_path, unredirected_headers=unredirected_headers, use_proxy=module.params['use_proxy'], decompress=decompress, - ciphers=ciphers, use_netrc=use_netrc, **kwargs) + ciphers=ciphers, use_netrc=use_netrc, force=module.params['force'], **kwargs) if src: # Try to close the open file handle diff --git a/test/integration/targets/uri/tasks/main.yml b/test/integration/targets/uri/tasks/main.yml index 23aa6436f9f..c98c85bac59 100644 --- a/test/integration/targets/uri/tasks/main.yml +++ b/test/integration/targets/uri/tasks/main.yml @@ -633,6 +633,22 @@ that: - "'content' in file_out.stdout" +- name: Test downloading cached file + uri: + url: "https://{{ httpbin_host }}/cache" + +- name: Test downloading cached file to existing file results in "304 Not Modified" + uri: + url: "https://{{ httpbin_host }}/cache" + dest: "{{ remote_tmp_dir }}/output" + status_code: [304] + +- name: Test downloading cached file to existing file with "force" + uri: + url: "https://{{ httpbin_host }}/cache" + dest: "{{ remote_tmp_dir }}/output" + force: true + - name: Clean up file: dest: "{{ remote_tmp_dir }}/output" diff --git a/test/units/modules/test_uri.py b/test/units/modules/test_uri.py new file mode 100644 index 00000000000..2aeb4641d41 --- /dev/null +++ b/test/units/modules/test_uri.py @@ -0,0 +1,43 @@ +# -*- coding: utf-8 -*- +# Copyright: +# (c) 2023 Ansible Project +# License: GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import annotations + +from unittest.mock import MagicMock, patch +from units.modules.utils import AnsibleExitJson, AnsibleFailJson, ModuleTestCase, set_module_args +from ansible.modules import uri + + +class TestUri(ModuleTestCase): + + def test_main_no_args(self): + """Module must fail if called with no args.""" + with self.assertRaises(AnsibleFailJson): + set_module_args({}) + uri.main() + + def test_main_no_force(self): + """The "force" parameter to fetch_url() must be absent or false when the module is called without "force".""" + set_module_args({"url": "http://example.com/"}) + resp = MagicMock() + resp.headers.get_content_type.return_value = "text/html" + info = {"url": "http://example.com/", "status": 200} + with patch.object(uri, "fetch_url", return_value=(resp, info)) as fetch_url: + with self.assertRaises(AnsibleExitJson): + uri.main() + fetch_url.assert_called_once() + assert not fetch_url.call_args[1].get("force") + + def test_main_force(self): + """The "force" parameter to fetch_url() must be true when the module is called with "force".""" + set_module_args({"url": "http://example.com/", "force": True}) + resp = MagicMock() + resp.headers.get_content_type.return_value = "text/html" + info = {"url": "http://example.com/", "status": 200} + with patch.object(uri, "fetch_url", return_value=(resp, info)) as fetch_url: + with self.assertRaises(AnsibleExitJson): + uri.main() + fetch_url.assert_called_once() + assert fetch_url.call_args[1].get("force")