From 7c66c90afcb4dbe15f5815c44e9c65f887a446bc Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Mon, 8 Oct 2018 14:41:57 +0200 Subject: [PATCH] introduce `module_utils.urls.fetch_file` as a wrapper to download and save files (#19172) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * module_utils.urls: add fetch_file function * apt: use fetch_file instead of own download() * unarchive: use fetch_file instead of own codecopy * apt: add test for deb=http://… * unarchive: add test for a remote file download and unarchive * yum: replace fetch_rpm_from_url by fetch_file * use NamedTemporaryFile * don't add a dot to fileext, it's already there --- lib/ansible/module_utils/urls.py | 37 +++++++++++++++++++ lib/ansible/modules/files/unarchive.py | 28 +------------- lib/ansible/modules/packaging/os/apt.py | 34 +---------------- lib/ansible/modules/packaging/os/yum.py | 30 ++------------- test/integration/targets/apt/tasks/apt.yml | 22 +++++++++++ .../targets/unarchive/tasks/main.yml | 20 ++++++++++ 6 files changed, 86 insertions(+), 85 deletions(-) diff --git a/lib/ansible/module_utils/urls.py b/lib/ansible/module_utils/urls.py index c5e1632181c..775220d2ea6 100644 --- a/lib/ansible/module_utils/urls.py +++ b/lib/ansible/module_utils/urls.py @@ -1324,3 +1324,40 @@ def fetch_url(module, url, data=None, headers=None, method=None, tempfile.tempdir = old_tempdir return r, info + + +def fetch_file(module, url, data=None, headers=None, method=None, + use_proxy=True, force=False, last_mod_time=None, timeout=10): + '''Download and save a file via HTTP(S) or FTP (needs the module as parameter). + This is basically a wrapper around fetch_url(). + + :arg module: The AnsibleModule (used to get username, password etc. (s.b.). + :arg url: The url to use. + + :kwarg data: The data to be sent (in case of POST/PUT). + :kwarg headers: A dict with the request headers. + :kwarg method: "POST", "PUT", etc. + :kwarg boolean use_proxy: Default: True + :kwarg boolean force: If True: Do not get a cached copy (Default: False) + :kwarg last_mod_time: Default: None + :kwarg int timeout: Default: 10 + + :returns: A string, the path to the downloaded file. + ''' + # download file + bufsize = 65536 + file_name, file_ext = os.path.splitext(str(url.rsplit('/', 1)[1])) + fetch_temp_file = tempfile.NamedTemporaryFile(dir=module.tmpdir, prefix=file_name, suffix=file_ext, delete=False) + module.add_cleanup_file(fetch_temp_file.name) + try: + rsp, info = fetch_url(module, url, data, headers, method, use_proxy, force, last_mod_time, timeout) + if not rsp: + module.fail_json(msg="Failure downloading %s, %s" % (url, info['msg'])) + data = rsp.read(bufsize) + while data: + fetch_temp_file.write(data) + data = rsp.read(bufsize) + fetch_temp_file.close() + except Exception as e: + module.fail_json(msg="Failure downloading %s, %s" % (url, to_native(e))) + return fetch_temp_file.name diff --git a/lib/ansible/modules/files/unarchive.py b/lib/ansible/modules/files/unarchive.py index 1f639b84db2..8e37397c66c 100644 --- a/lib/ansible/modules/files/unarchive.py +++ b/lib/ansible/modules/files/unarchive.py @@ -144,7 +144,7 @@ import traceback from zipfile import ZipFile, BadZipfile from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.urls import fetch_url +from ansible.module_utils.urls import fetch_file from ansible.module_utils._text import to_bytes, to_native, to_text try: # python 3.3+ @@ -162,9 +162,6 @@ MOD_TIME_DIFF_RE = re.compile(r': Mod time differs$') EMPTY_FILE_RE = re.compile(r': : Warning: Cannot stat: No such file or directory$') MISSING_FILE_RE = re.compile(r': Warning: Cannot stat: No such file or directory$') ZIP_FILE_MODE_RE = re.compile(r'([r-][w-][SsTtx-]){3}') -# When downloading an archive, how much of the archive to download before -# saving to a tempfile (64k) -BUFSIZE = 65536 def crc32(path): @@ -821,28 +818,7 @@ def main(): module.fail_json(msg="Source '%s' failed to transfer" % src) # If remote_src=true, and src= contains ://, try and download the file to a temp directory. elif '://' in src: - new_src = os.path.join(module.tmpdir, to_native(src.rsplit('/', 1)[1], errors='surrogate_or_strict')) - try: - rsp, info = fetch_url(module, src) - # If download fails, raise a proper exception - if rsp is None: - raise Exception(info['msg']) - - # open in binary mode for python3 - f = open(new_src, 'wb') - # Read 1kb at a time to save on ram - while True: - data = rsp.read(BUFSIZE) - data = to_bytes(data, errors='surrogate_or_strict') - - if len(data) < 1: - break # End of file, break while loop - - f.write(data) - f.close() - src = new_src - except Exception as e: - module.fail_json(msg="Failure downloading %s, %s" % (src, to_native(e))) + src = fetch_file(module, src) else: module.fail_json(msg="Source '%s' does not exist" % src) if not os.access(src, os.R_OK): diff --git a/lib/ansible/modules/packaging/os/apt.py b/lib/ansible/modules/packaging/os/apt.py index c5355af1393..e177d81d180 100644 --- a/lib/ansible/modules/packaging/os/apt.py +++ b/lib/ansible/modules/packaging/os/apt.py @@ -266,7 +266,7 @@ import time from ansible.module_utils.basic import AnsibleModule from ansible.module_utils._text import to_bytes, to_native -from ansible.module_utils.urls import fetch_url +from ansible.module_utils.urls import fetch_file # APT related constants APT_ENV_VARS = dict( @@ -846,36 +846,6 @@ def upgrade(m, mode="yes", force=False, default_release=None, m.exit_json(changed=True, msg=out, stdout=out, stderr=err, diff=diff) -def download(module, deb): - package = os.path.join(module.tmpdir, to_native(deb.rsplit('/', 1)[1], errors='surrogate_or_strict')) - # When downloading a deb, how much of the deb to download before - # saving to a tempfile (64k) - BUFSIZE = 65536 - - try: - rsp, info = fetch_url(module, deb, method='GET') - if info['status'] != 200: - module.fail_json(msg="Failed to download %s, %s" % (deb, - info['msg'])) - # Ensure file is open in binary mode for Python 3 - f = open(package, 'wb') - # Read 1kb at a time to save on ram - while True: - data = rsp.read(BUFSIZE) - data = to_bytes(data, errors='surrogate_or_strict') - - if len(data) < 1: - break # End of file, break while loop - - f.write(data) - f.close() - deb = package - except Exception as e: - module.fail_json(msg="Failure downloading %s, %s" % (deb, to_native(e))) - - return deb - - def get_cache_mtime(): """Return mtime of a valid apt cache file. Stat the apt cache file and if no cache file is found return 0 @@ -1053,7 +1023,7 @@ def main(): if p['state'] != 'present': module.fail_json(msg="deb only supports state=present") if '://' in p['deb']: - p['deb'] = download(module, p['deb']) + p['deb'] = fetch_file(module, p['deb']) install_deb(module, p['deb'], cache, install_recommends=install_recommends, allow_unauthenticated=allow_unauthenticated, diff --git a/lib/ansible/modules/packaging/os/yum.py b/lib/ansible/modules/packaging/os/yum.py index ac1080ea4d8..74fd7ee35f3 100644 --- a/lib/ansible/modules/packaging/os/yum.py +++ b/lib/ansible/modules/packaging/os/yum.py @@ -352,13 +352,11 @@ except ImportError: transaction_helpers = False from contextlib import contextmanager +from ansible.module_utils.urls import fetch_file def_qf = "%{epoch}:%{name}-%{version}-%{release}.%{arch}" rpmbin = None -# 64k. Number of bytes to read at a time when manually downloading pkgs via a url -BUFSIZE = 65536 - class YumModule(YumDnf): """ @@ -384,28 +382,6 @@ class YumModule(YumDnf): self.pkg_mgr_name = "yum" self.lockfile = '/var/run/yum.pid' - def fetch_rpm_from_url(self, spec): - # FIXME: Remove this once this PR is merged: - # https://github.com/ansible/ansible/pull/19172 - - # download package so that we can query it - package_name, dummy = os.path.splitext(str(spec.rsplit('/', 1)[1])) - package_file = tempfile.NamedTemporaryFile(dir=self.module.tmpdir, prefix=package_name, suffix='.rpm', delete=False) - self.module.add_cleanup_file(package_file.name) - try: - rsp, info = fetch_url(self.module, spec) - if not rsp: - self.module.fail_json(msg="Failure downloading %s, %s" % (spec, info['msg'])) - data = rsp.read(BUFSIZE) - while data: - package_file.write(data) - data = rsp.read(BUFSIZE) - package_file.close() - except Exception as e: - self.module.fail_json(msg="Failure downloading %s, %s" % (spec, to_native(e))) - - return package_file.name - def yum_base(self): my = yum.YumBase() my.preconf.debuglevel = 0 @@ -884,7 +860,7 @@ class YumModule(YumDnf): if '://' in spec: with self.set_env_proxy(): - package = self.fetch_rpm_from_url(spec) + package = fetch_file(self.module, spec) else: package = spec @@ -1205,7 +1181,7 @@ class YumModule(YumDnf): elif '://' in spec: # download package so that we can check if it's already installed with self.set_env_proxy(): - package = self.fetch_rpm_from_url(spec) + package = fetch_file(self.module, spec) envra = self.local_envra(package) if envra is None: diff --git a/test/integration/targets/apt/tasks/apt.yml b/test/integration/targets/apt/tasks/apt.yml index 0dd3ea36d65..bd8f225a546 100644 --- a/test/integration/targets/apt/tasks/apt.yml +++ b/test/integration/targets/apt/tasks/apt.yml @@ -8,6 +8,16 @@ python_apt: python3-apt when: ansible_python_version is version('3', '>=') +- name: use Debian mirror + set_fact: + distro_mirror: http://ftp.debian.org/debian + when: ansible_distribution == 'Debian' + +- name: use Ubuntu mirror + set_fact: + distro_mirror: http://archive.ubuntu.com/ubuntu + when: ansible_distribution == 'Ubuntu' + # UNINSTALL 'python-apt' # The `apt` module has the smarts to auto-install `python-apt`. To test, we # will first uninstall `python-apt`. @@ -132,6 +142,18 @@ - name: uninstall hello with apt apt: pkg=hello state=absent purge=yes +- name: install deb file from URL + apt: deb="{{ distro_mirror }}/pool/main/h/hello/hello_{{ hello_version.stdout }}_{{ hello_architecture.stdout }}.deb" + register: apt_url + +- name: verify installation of hello + assert: + that: + - "apt_url.changed" + +- name: uninstall hello with apt + apt: pkg=hello state=absent purge=yes + - name: force install of deb apt: deb="/var/cache/apt/archives/hello_{{ hello_version.stdout }}_{{ hello_architecture.stdout }}.deb" force=true register: dpkg_force diff --git a/test/integration/targets/unarchive/tasks/main.yml b/test/integration/targets/unarchive/tasks/main.yml index 12cfcf10af9..ffe25fcf4ad 100644 --- a/test/integration/targets/unarchive/tasks/main.yml +++ b/test/integration/targets/unarchive/tasks/main.yml @@ -590,3 +590,23 @@ - name: remove our tar.gz unarchive destination file: path={{ output_dir }}/test-unarchive-tar-gz state=absent + +# Test downloading a file before unarchiving it +- name: create our unarchive destination + file: path={{output_dir}}/test-unarchive-tar-gz state=directory + +- name: unarchive a tar from an URL + unarchive: + src: "https://releases.ansible.com/ansible/ansible-latest.tar.gz" + dest: "{{ output_dir }}/test-unarchive-tar-gz" + mode: "0700" + remote_src: yes + register: unarchive13 + +- name: Test that unarchive succeeded + assert: + that: + - "unarchive13.changed == true" + +- name: remove our tar.gz unarchive destination + file: path={{ output_dir }}/test-unarchive-tar-gz state=absent