diff --git a/ansible_mitogen/plugins/action/mitogen_fetch.py b/ansible_mitogen/plugins/action/mitogen_fetch.py index b9eece76..c1ef1902 100644 --- a/ansible_mitogen/plugins/action/mitogen_fetch.py +++ b/ansible_mitogen/plugins/action/mitogen_fetch.py @@ -18,23 +18,17 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import os - -from ansible.module_utils._text import to_bytes +import base64 +from ansible.errors import AnsibleError, AnsibleActionFail, AnsibleActionSkip +from ansible.module_utils.common.text.converters import to_bytes, to_text from ansible.module_utils.six import string_types from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.action import ActionBase -from ansible.utils.hashing import checksum, md5, secure_hash -from ansible.utils.path import makedirs_safe - +from ansible.utils.display import Display +from ansible.utils.hashing import checksum, checksum_s, md5, secure_hash +from ansible.utils.path import makedirs_safe, is_subpath -REMOTE_CHECKSUM_ERRORS = { - '0': "unable to calculate the checksum of the remote file", - '1': "the remote file does not exist", - '2': "no read permission on remote file", - '3': "remote file is a directory, fetch cannot work on directories", - '4': "python isn't present on the system. Unable to compute checksum", - '5': "stdlib json was not found on the remote machine. Only the raw module can work without those installed", -} +display = Display() class ActionModule(ActionBase): @@ -45,36 +39,94 @@ class ActionModule(ActionBase): task_vars = dict() result = super(ActionModule, self).run(tmp, task_vars) + del tmp # tmp no longer has any effect + try: if self._play_context.check_mode: - result['skipped'] = True - result['msg'] = 'check mode not (yet) supported for this module' - return result + raise AnsibleActionSkip('check mode not (yet) supported for this module') + source = self._task.args.get('src', None) + original_dest = dest = self._task.args.get('dest', None) flat = boolean(self._task.args.get('flat'), strict=False) fail_on_missing = boolean(self._task.args.get('fail_on_missing', True), strict=False) validate_checksum = boolean(self._task.args.get('validate_checksum', True), strict=False) + msg = '' # validate source and dest are strings FIXME: use basic.py and module specs - source = self._task.args.get('src') if not isinstance(source, string_types): - result['msg'] = "Invalid type supplied for source option, it must be a string" + msg = "Invalid type supplied for source option, it must be a string" - dest = self._task.args.get('dest') if not isinstance(dest, string_types): - result['msg'] = "Invalid type supplied for dest option, it must be a string" + msg = "Invalid type supplied for dest option, it must be a string" + + if source is None or dest is None: + msg = "src and dest are required" - if result.get('msg'): - result['failed'] = True - return result + if msg: + raise AnsibleActionFail(msg) source = self._connection._shell.join_path(source) source = self._remote_expand_user(source) - # calculate checksum for the remote file, don't bother if using - # become as slurp will be used Force remote_checksum to follow - # symlinks because fetch always follows symlinks - remote_checksum = self._remote_checksum(source, all_vars=task_vars, follow=True) + remote_stat = {} + remote_checksum = None + if True: + # Get checksum for the remote file even using become. Mitogen doesn't need slurp. + # Follow symlinks because fetch always follows symlinks + try: + remote_stat = self._execute_remote_stat(source, all_vars=task_vars, follow=True) + except AnsibleError as ae: + result['changed'] = False + result['file'] = source + if fail_on_missing: + result['failed'] = True + result['msg'] = to_text(ae) + else: + result['msg'] = "%s, ignored" % to_text(ae, errors='surrogate_or_replace') + + return result + + remote_checksum = remote_stat.get('checksum') + if remote_stat.get('exists'): + if remote_stat.get('isdir'): + result['failed'] = True + result['changed'] = False + result['msg'] = "remote file is a directory, fetch cannot work on directories" + + # Historically, these don't fail because you may want to transfer + # a log file that possibly MAY exist but keep going to fetch other + # log files. Today, this is better achieved by adding + # ignore_errors or failed_when to the task. Control the behaviour + # via fail_when_missing + if not fail_on_missing: + result['msg'] += ", not transferring, ignored" + del result['changed'] + del result['failed'] + + return result + + # use slurp if permissions are lacking or privilege escalation is needed + remote_data = None + if remote_checksum in (None, '1', ''): + slurpres = self._execute_module(module_name='ansible.legacy.slurp', module_args=dict(src=source), task_vars=task_vars) + if slurpres.get('failed'): + if not fail_on_missing: + result['file'] = source + result['changed'] = False + else: + result.update(slurpres) + + if 'not found' in slurpres.get('msg', ''): + result['msg'] = "the remote file does not exist, not transferring, ignored" + elif slurpres.get('msg', '').startswith('source is a directory'): + result['msg'] = "remote file is a directory, fetch cannot work on directories" + + return result + else: + if slurpres['encoding'] == 'base64': + remote_data = base64.b64decode(slurpres['content']) + if remote_data is not None: + remote_checksum = checksum_s(remote_data) # calculate the destination name if os.path.sep not in self._connection._shell.join_path('a', ''): @@ -83,13 +135,14 @@ class ActionModule(ActionBase): else: source_local = source - dest = os.path.expanduser(dest) + # ensure we only use file name, avoid relative paths + if not is_subpath(dest, original_dest): + # TODO: ? dest = os.path.expanduser(dest.replace(('../',''))) + raise AnsibleActionFail("Detected directory traversal, expected to be contained in '%s' but got '%s'" % (original_dest, dest)) + if flat: if os.path.isdir(to_bytes(dest, errors='surrogate_or_strict')) and not dest.endswith(os.sep): - result['msg'] = "dest is an existing directory, use a trailing slash if you want to fetch src into that directory" - result['file'] = dest - result['failed'] = True - return result + raise AnsibleActionFail("dest is an existing directory, use a trailing slash if you want to fetch src into that directory") if dest.endswith(os.sep): # if the path ends with "/", we'll use the source filename as the # destination filename @@ -106,23 +159,7 @@ class ActionModule(ActionBase): target_name = self._play_context.remote_addr dest = "%s/%s/%s" % (self._loader.path_dwim(dest), target_name, source_local) - dest = dest.replace("//", "/") - - if remote_checksum in REMOTE_CHECKSUM_ERRORS: - result['changed'] = False - result['file'] = source - result['msg'] = REMOTE_CHECKSUM_ERRORS[remote_checksum] - # Historically, these don't fail because you may want to transfer - # a log file that possibly MAY exist but keep going to fetch other - # log files. Today, this is better achieved by adding - # ignore_errors or failed_when to the task. Control the behaviour - # via fail_when_missing - if fail_on_missing: - result['failed'] = True - del result['changed'] - else: - result['msg'] += ", not transferring, ignored" - return result + dest = os.path.normpath(dest) # calculate checksum for the local file local_checksum = checksum(dest) @@ -132,7 +169,15 @@ class ActionModule(ActionBase): makedirs_safe(os.path.dirname(dest)) # fetch the file and check for changes - self._connection.fetch_file(source, dest) + if remote_data is None: + self._connection.fetch_file(source, dest) + else: + try: + f = open(to_bytes(dest, errors='surrogate_or_strict'), 'wb') + f.write(remote_data) + f.close() + except (IOError, OSError) as e: + raise AnsibleActionFail("Failed to fetch the file: %s" % e) new_checksum = secure_hash(dest) # For backwards compatibility. We'll return None on FIPS enabled systems try: @@ -157,10 +202,6 @@ class ActionModule(ActionBase): result.update(dict(changed=False, md5sum=local_md5, file=source, dest=dest, checksum=local_checksum)) finally: - try: - self._remove_tmp_path(self._connection._shell.tmpdir) - except AttributeError: - # .tmpdir was added to ShellModule in v2.6.0, so old versions don't have it - pass + self._remove_tmp_path(self._connection._shell.tmpdir) return result diff --git a/docs/changelog.rst b/docs/changelog.rst index 513ce8e6..9741ca5c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -25,6 +25,7 @@ v0.3.3.dev0 * :gh:issue:`918` Support Python 3.10 * :gh:issue:`920` Support Ansible :ans:conn:`~podman` connection plugin * :gh:issue:`836` :func:`mitogen.utils.with_router` decorator preserves the docstring in addition to the name. +* :gh:issue:`936` :ans:mod:`fetch` no longer emits `[DEPRECATION WARNING]: The '_remote_checksum()' method is deprecated.` v0.3.2 (2022-01-12) diff --git a/tests/ansible/regression/issue_615__streaming_transfer.yml b/tests/ansible/regression/issue_615__streaming_transfer.yml index b77aa254..3e174bed 100644 --- a/tests/ansible/regression/issue_615__streaming_transfer.yml +++ b/tests/ansible/regression/issue_615__streaming_transfer.yml @@ -1,23 +1,32 @@ # issue #615: 'fetch' with become: was internally using slurp. -- hosts: target +- hosts: test-targets any_errors_fatal: True gather_facts: no + # Without Mitogen this causes Ansible to use the slurp module, which is *slow* become: true vars: mitogen_ssh_compression: false tasks: - - shell: | - dd if=/dev/zero of=/tmp/512mb.zero bs=1048576 count=512; - chmod go= /tmp/512mb.zero + - block: + - shell: | + dd if=/dev/zero of=/tmp/512mb.zero bs=1048576 count=512; + chmod go= /tmp/512mb.zero - - fetch: - src: /tmp/512mb.zero - dest: /tmp/fetch-out + - fetch: + src: /tmp/512mb.zero + dest: /tmp/fetch-out - - file: - path: /tmp/fetch-out - state: absent - delegate_to: localhost + - file: + path: /tmp/512mb.zero + state: absent + + - file: + path: /tmp/fetch-out + state: absent + delegate_to: localhost + run_once: true + when: + - is_mitogen tags: - issue_615