From be0cdc0ea28cdfd0ba4fb448fe66b4dde2aedcb6 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Wed, 16 Jun 2021 13:51:07 -0400 Subject: [PATCH] deprecate `_remote_checksum()` and remove use in fetch (#74848) * Remove use of _remote_checksum from fetch module * Add deprecation message displayed during runtime * Increase test coverage for fetch * Add tests covering the use of stat from the fetch module This required creating an unpriveleged user account and connecting as that user remotely since it is not possible to create a file that the root user cannot stat. * Use fact caching to persist remote tmp dir across playbook runs * Add variables to setup_remote_tmp test role to allow caching of the remote temp dir fact and preventing removal of the remote_tmp_dir --- lib/ansible/plugins/action/__init__.py | 7 +- lib/ansible/plugins/action/fetch.py | 80 +++++----- test/integration/targets/fetch/cleanup.yml | 16 ++ test/integration/targets/fetch/hosts.yml | 8 + .../fetch/roles/fetch_tests/defaults/main.yml | 1 + .../fetch/roles/fetch_tests/handlers/main.yml | 8 + .../fetch_tests/tasks/fail_on_missing.yml | 53 +++++++ .../roles/fetch_tests/tasks/failures.yml | 41 +++++ .../fetch/roles/fetch_tests/tasks/main.yml | 146 +----------------- .../fetch/roles/fetch_tests/tasks/normal.yml | 38 +++++ .../fetch/roles/fetch_tests/tasks/setup.yml | 38 +++++ .../fetch/roles/fetch_tests/tasks/symlink.yml | 13 ++ .../fetch/roles/fetch_tests/vars/Darwin.yml | 3 + .../fetch/roles/fetch_tests/vars/default.yml | 1 + test/integration/targets/fetch/runme.sh | 26 +++- .../targets/fetch/setup_unreadable_test.yml | 39 +++++ .../fetch/test_unreadable_with_stat.yml | 27 ++++ .../setup_remote_tmp_dir/defaults/main.yml | 2 + .../setup_remote_tmp_dir/handlers/main.yml | 2 + .../setup_remote_tmp_dir/tasks/default.yml | 1 + 20 files changed, 369 insertions(+), 181 deletions(-) create mode 100644 test/integration/targets/fetch/cleanup.yml create mode 100644 test/integration/targets/fetch/hosts.yml create mode 100644 test/integration/targets/fetch/roles/fetch_tests/defaults/main.yml create mode 100644 test/integration/targets/fetch/roles/fetch_tests/handlers/main.yml create mode 100644 test/integration/targets/fetch/roles/fetch_tests/tasks/fail_on_missing.yml create mode 100644 test/integration/targets/fetch/roles/fetch_tests/tasks/failures.yml create mode 100644 test/integration/targets/fetch/roles/fetch_tests/tasks/normal.yml create mode 100644 test/integration/targets/fetch/roles/fetch_tests/tasks/setup.yml create mode 100644 test/integration/targets/fetch/roles/fetch_tests/tasks/symlink.yml create mode 100644 test/integration/targets/fetch/roles/fetch_tests/vars/Darwin.yml create mode 100644 test/integration/targets/fetch/roles/fetch_tests/vars/default.yml create mode 100644 test/integration/targets/fetch/setup_unreadable_test.yml create mode 100644 test/integration/targets/fetch/test_unreadable_with_stat.yml create mode 100644 test/integration/targets/setup_remote_tmp_dir/defaults/main.yml diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index efccc419141..68b949534ba 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -794,7 +794,8 @@ class ActionBase(with_metaclass(ABCMeta, object)): return mystat['stat'] def _remote_checksum(self, path, all_vars, follow=False): - ''' + """Deprecated. Use _execute_remote_stat() instead. + Produces a remote checksum given a path, Returns a number 0-4 for specific errors instead of checksum, also ensures it is different 0 = unknown error @@ -803,7 +804,9 @@ class ActionBase(with_metaclass(ABCMeta, object)): 3 = its a directory, not a file 4 = stat module failed, likely due to not finding python 5 = appropriate json module not found - ''' + """ + self._display.deprecated("The '_remote_checksum()' method is deprecated. " + "The plugin author should update the code to use '_execute_remote_stat()' instead", "2.16") x = "0" # unknown error has occurred try: remote_stat = self._execute_remote_stat(path, all_vars, follow=follow) diff --git a/lib/ansible/plugins/action/fetch.py b/lib/ansible/plugins/action/fetch.py index 4f05d2bfad2..992ba5a5123 100644 --- a/lib/ansible/plugins/action/fetch.py +++ b/lib/ansible/plugins/action/fetch.py @@ -19,9 +19,8 @@ __metaclass__ = type import os import base64 - -from ansible.errors import AnsibleActionFail, AnsibleActionSkip -from ansible.module_utils._text import to_bytes +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 @@ -69,23 +68,59 @@ class ActionModule(ActionBase): source = self._connection._shell.join_path(source) source = self._remote_expand_user(source) + remote_stat = {} remote_checksum = None if not self._connection.become: - # 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) + # Get checksum for the remote file. Don't bother if using become as slurp will be used. + # 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 ('1', '2', 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 and (slurpres.get('msg').startswith('file not found') or remote_checksum == '1'): - result['msg'] = "the remote file does not exist, not transferring, ignored" + 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': @@ -124,33 +159,6 @@ class ActionModule(ActionBase): target_name = self._play_context.remote_addr dest = "%s/%s/%s" % (self._loader.path_dwim(dest), target_name, source_local) - if remote_checksum in ('0', '1', '2', '3', '4', '5'): - result['changed'] = False - result['file'] = source - if remote_checksum == '0': - result['msg'] = "unable to calculate the checksum of the remote file" - elif remote_checksum == '1': - result['msg'] = "the remote file does not exist" - elif remote_checksum == '2': - result['msg'] = "no read permission on remote file" - elif remote_checksum == '3': - result['msg'] = "remote file is a directory, fetch cannot work on directories" - elif remote_checksum == '4': - result['msg'] = "python isn't present on the system. Unable to compute checksum" - elif remote_checksum == '5': - result['msg'] = "stdlib json was not found on the remote machine. Only the raw module can work without those installed" - # 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 diff --git a/test/integration/targets/fetch/cleanup.yml b/test/integration/targets/fetch/cleanup.yml new file mode 100644 index 00000000000..792b603c63b --- /dev/null +++ b/test/integration/targets/fetch/cleanup.yml @@ -0,0 +1,16 @@ +- name: Cleanup user account + hosts: testhost + + tasks: + - name: remove test user + user: + name: fetcher + state: absent + remove: yes + force: yes + + - name: delete temporary directory + file: + path: "{{ remote_tmp_dir }}" + state: absent + no_log: yes diff --git a/test/integration/targets/fetch/hosts.yml b/test/integration/targets/fetch/hosts.yml new file mode 100644 index 00000000000..8465ef166c2 --- /dev/null +++ b/test/integration/targets/fetch/hosts.yml @@ -0,0 +1,8 @@ +all: + hosts: + testhost: + ansible_host: localhost + ansible_connection: ssh + ansible_python_interpreter: "{{ ansible_playbook_python }}" + ansible_host_key_checking: no + ansible_ssh_common_args: -o UserKnownHostsFile={{ output_dir }}/known_hosts -o StrictHostKeyChecking=no diff --git a/test/integration/targets/fetch/roles/fetch_tests/defaults/main.yml b/test/integration/targets/fetch/roles/fetch_tests/defaults/main.yml new file mode 100644 index 00000000000..f0b9cfc4014 --- /dev/null +++ b/test/integration/targets/fetch/roles/fetch_tests/defaults/main.yml @@ -0,0 +1 @@ +skip_cleanup: no diff --git a/test/integration/targets/fetch/roles/fetch_tests/handlers/main.yml b/test/integration/targets/fetch/roles/fetch_tests/handlers/main.yml new file mode 100644 index 00000000000..c6c296af4d2 --- /dev/null +++ b/test/integration/targets/fetch/roles/fetch_tests/handlers/main.yml @@ -0,0 +1,8 @@ +- name: remove test user + user: + name: fetcher + state: absent + remove: yes + force: yes + become: yes + when: not skip_cleanup | bool diff --git a/test/integration/targets/fetch/roles/fetch_tests/tasks/fail_on_missing.yml b/test/integration/targets/fetch/roles/fetch_tests/tasks/fail_on_missing.yml new file mode 100644 index 00000000000..d918aaeb250 --- /dev/null +++ b/test/integration/targets/fetch/roles/fetch_tests/tasks/fail_on_missing.yml @@ -0,0 +1,53 @@ +- name: Attempt to fetch a non-existent file - do not fail on missing + fetch: + src: "{{ remote_tmp_dir }}/doesnotexist" + dest: "{{ output_dir }}/fetched" + fail_on_missing: no + register: fetch_missing_nofail + +- name: Attempt to fetch a non-existent file - fail on missing + fetch: + src: "{{ remote_tmp_dir }}/doesnotexist" + dest: "{{ output_dir }}/fetched" + fail_on_missing: yes + register: fetch_missing + ignore_errors: yes + +- name: Attempt to fetch a non-existent file - fail on missing implicit + fetch: + src: "{{ remote_tmp_dir }}/doesnotexist" + dest: "{{ output_dir }}/fetched" + register: fetch_missing_implicit + ignore_errors: yes + +- name: Attempt to fetch a directory - should not fail but return a message + fetch: + src: "{{ remote_tmp_dir }}" + dest: "{{ output_dir }}/somedir" + fail_on_missing: no + register: fetch_dir + +- name: Attempt to fetch a directory - should fail + fetch: + src: "{{ remote_tmp_dir }}" + dest: "{{ output_dir }}/somedir" + fail_on_missing: yes + register: failed_fetch_dir + ignore_errors: yes + +- name: Check fetch missing with failure with implicit fail + assert: + that: + - fetch_missing_nofail.msg is search('ignored') + - fetch_missing_nofail is not changed + - fetch_missing is failed + - fetch_missing is not changed + - fetch_missing.msg is search ('remote file does not exist') + - fetch_missing_implicit is failed + - fetch_missing_implicit is not changed + - fetch_missing_implicit.msg is search ('remote file does not exist') + - fetch_dir is not changed + - fetch_dir.msg is search('is a directory') + - failed_fetch_dir is failed + - failed_fetch_dir is not changed + - failed_fetch_dir.msg is search('is a directory') diff --git a/test/integration/targets/fetch/roles/fetch_tests/tasks/failures.yml b/test/integration/targets/fetch/roles/fetch_tests/tasks/failures.yml new file mode 100644 index 00000000000..8a6b5b7b367 --- /dev/null +++ b/test/integration/targets/fetch/roles/fetch_tests/tasks/failures.yml @@ -0,0 +1,41 @@ +- name: Fetch with no parameters + fetch: + register: fetch_no_params + ignore_errors: yes + +- name: Fetch with incorrect source type + fetch: + src: [1, 2] + dest: "{{ output_dir }}/fetched" + register: fetch_incorrect_src + ignore_errors: yes + +- name: Try to fetch a file inside an inaccessible directory + fetch: + src: "{{ remote_tmp_dir }}/noaccess/file1" + dest: "{{ output_dir }}" + register: failed_fetch_no_access + become: yes + become_user: fetcher + become_method: su + ignore_errors: yes + +- name: Dest is an existing directory name without trailing slash and flat=yes, should fail + fetch: + src: "{{ remote_tmp_dir }}/orig" + dest: "{{ output_dir }}" + flat: yes + register: failed_fetch_dest_dir + ignore_errors: true + +- name: Ensure fetch failed + assert: + that: + - fetch_no_params is failed + - fetch_no_params.msg is search('src and dest are required') + - fetch_incorrect_src is failed + - fetch_incorrect_src.msg is search('Invalid type supplied for source') + - failed_fetch_no_access is failed + - failed_fetch_no_access.msg is search('file is not readable') + - failed_fetch_dest_dir is failed + - failed_fetch_dest_dir.msg is search('dest is an existing directory') diff --git a/test/integration/targets/fetch/roles/fetch_tests/tasks/main.yml b/test/integration/targets/fetch/roles/fetch_tests/tasks/main.yml index 267ae0f0cd4..eefe95c890d 100644 --- a/test/integration/targets/fetch/roles/fetch_tests/tasks/main.yml +++ b/test/integration/targets/fetch/roles/fetch_tests/tasks/main.yml @@ -1,141 +1,5 @@ -# test code for the pip module -# (c) 2014, Michael DeHaan - -# This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . - -- name: create a file that we can use to fetch - copy: content="test" dest={{ remote_tmp_dir }}/orig - -- name: fetch the test file - fetch: src={{ remote_tmp_dir }}/orig dest={{ output_dir }}/fetched - register: fetched - -- debug: var=fetched - -- name: Assert that we fetched correctly - assert: - that: - - 'fetched["changed"] == True' - - 'fetched["checksum"] == "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3"' - - 'fetched["remote_checksum"] == "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3"' - - 'lookup("file", output_dir + "/fetched/" + inventory_hostname + remote_tmp_dir + "/orig") == "test"' - -# TODO: check the become and non-become forms of fetch because in one form we'll do -# the get method of the connection plugin and in the become case we'll use the -# fetch module. - -- name: fetch a second time to show idempotence - fetch: src={{ remote_tmp_dir }}/orig dest={{ output_dir }}/fetched - register: fetched - -- name: Assert that the file was not fetched the second time - assert: - that: - - 'fetched["changed"] == False' - - 'fetched["checksum"] == "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3"' - -- name: attempt to fetch a non-existent file - do not fail on missing - fetch: src={{ remote_tmp_dir }}/doesnotexist dest={{ output_dir }}/fetched fail_on_missing=False - register: fetch_missing_nofail - -- name: check fetch missing no fail result - assert: - that: - - "fetch_missing_nofail.msg" - - "fetch_missing_nofail is not changed" - -- name: attempt to fetch a non-existent file - fail on missing - fetch: src={{ remote_tmp_dir }}/doesnotexist dest={{ output_dir }}/fetched fail_on_missing=yes - register: fetch_missing - ignore_errors: true - -- name: check fetch missing with failure - assert: - that: - - "fetch_missing is failed" - - "fetch_missing.msg" - - "fetch_missing is not changed" - -- name: attempt to fetch a non-existent file - fail on missing implicit - fetch: src={{ remote_tmp_dir }}/doesnotexist dest={{ output_dir }}/fetched - register: fetch_missing_implicit - ignore_errors: true - -- name: check fetch missing with failure with implicit fail - assert: - that: - - "fetch_missing_implicit is failed" - - "fetch_missing_implicit.msg" - - "fetch_missing_implicit is not changed" - -- name: attempt to fetch a directory - should not fail but return a message - fetch: src={{ remote_tmp_dir }} dest={{ output_dir }}/somedir fail_on_missing=False - register: fetch_dir - -- name: check fetch directory result - assert: - that: - - "fetch_dir is not changed" - - "fetch_dir.msg" - -- name: attempt to fetch a directory - should fail - fetch: src={{ remote_tmp_dir }} dest={{ output_dir }}/somedir fail_on_missing=True - register: failed_fetch_dir - ignore_errors: true - -- name: check fetch directory result - assert: - that: - - "failed_fetch_dir is failed" - - "fetch_dir.msg" - -- name: create symlink to a file that we can fetch - file: - path: "{{ remote_tmp_dir }}/link" - src: "{{ remote_tmp_dir }}/orig" - state: "link" - -- name: fetch the file via a symlink - fetch: src={{ remote_tmp_dir }}/link dest={{ output_dir }}/fetched-link - register: fetched - -- debug: var=fetched - -- name: Assert that we fetched correctly - assert: - that: - - 'fetched["changed"] == True' - - 'fetched["checksum"] == "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3"' - - 'fetched["remote_checksum"] == "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3"' - - 'lookup("file", output_dir + "/fetched-link/" + inventory_hostname + remote_tmp_dir + "/link") == "test"' - -# TODO: check the become and non-become forms of fetch because in one form we'll do -# the get method of the connection plugin and in the become case we'll use the -# fetch module. - -- name: dest is an existing directory name without trailing slash and flat=yes, should fail - fetch: - src: "{{ remote_tmp_dir }}/orig" - dest: "{{ output_dir }}" - flat: yes - register: failed_fetch_dest_dir - ignore_errors: true - -- name: check that it indeed failed - assert: - that: - - "failed_fetch_dest_dir is failed" - - "failed_fetch_dest_dir.msg" +- import_tasks: setup.yml +- import_tasks: normal.yml +- import_tasks: symlink.yml +- import_tasks: fail_on_missing.yml +- import_tasks: failures.yml diff --git a/test/integration/targets/fetch/roles/fetch_tests/tasks/normal.yml b/test/integration/targets/fetch/roles/fetch_tests/tasks/normal.yml new file mode 100644 index 00000000000..6f3ab62080d --- /dev/null +++ b/test/integration/targets/fetch/roles/fetch_tests/tasks/normal.yml @@ -0,0 +1,38 @@ +- name: Fetch the test file + fetch: src={{ remote_tmp_dir }}/orig dest={{ output_dir }}/fetched + register: fetched + +- name: Fetch a second time to show no changes + fetch: src={{ remote_tmp_dir }}/orig dest={{ output_dir }}/fetched + register: fetched_again + +- name: Fetch the test file in check mode + fetch: + src: "{{ remote_tmp_dir }}/orig" + dest: "{{ output_dir }}/fetched" + check_mode: yes + register: fetch_check_mode + +- name: Fetch with dest ending in path sep + fetch: + src: "{{ remote_tmp_dir }}/orig" + dest: "{{ output_dir }}/" + flat: yes + +- name: Fetch with dest with relative path + fetch: + src: "{{ remote_tmp_dir }}/orig" + dest: "{{ output_dir[1:] }}" + flat: yes + +- name: Assert that we fetched correctly + assert: + that: + - fetched is changed + - fetched.checksum == "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3" + - fetched_again is not changed + - fetched_again.checksum == "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3" + - fetched.remote_checksum == "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3" + - lookup("file", output_dir + "/fetched/" + inventory_hostname + remote_tmp_dir + "/orig") == "test" + - fetch_check_mode is skipped + - fetch_check_mode.msg is search('not \(yet\) supported') diff --git a/test/integration/targets/fetch/roles/fetch_tests/tasks/setup.yml b/test/integration/targets/fetch/roles/fetch_tests/tasks/setup.yml new file mode 100644 index 00000000000..e38086d378a --- /dev/null +++ b/test/integration/targets/fetch/roles/fetch_tests/tasks/setup.yml @@ -0,0 +1,38 @@ +- name: Include system specific variables + include_vars: "{{ lookup('first_found', params) }}" + vars: + params: + files: + - "{{ ansible_facts.system }}.yml" + - default.yml + paths: + - "{{ role_path }}/vars" + +- name: Create test user + user: + name: fetcher + create_home: yes + groups: "{{ _fetch_additional_groups | default(omit) }}" + append: "{{ True if _fetch_additional_groups else False }}" + become: yes + notify: + - remove test user + +- name: Create a file that we can use to fetch + copy: + content: "test" + dest: "{{ remote_tmp_dir }}/orig" + +- name: Create symlink to a file that we can fetch + file: + path: "{{ remote_tmp_dir }}/link" + src: "{{ remote_tmp_dir }}/orig" + state: "link" + +- name: Create an inaccessible directory + file: + path: "{{ remote_tmp_dir }}/noaccess" + state: directory + mode: '0600' + owner: root + become: yes diff --git a/test/integration/targets/fetch/roles/fetch_tests/tasks/symlink.yml b/test/integration/targets/fetch/roles/fetch_tests/tasks/symlink.yml new file mode 100644 index 00000000000..41d7b35a1f0 --- /dev/null +++ b/test/integration/targets/fetch/roles/fetch_tests/tasks/symlink.yml @@ -0,0 +1,13 @@ +- name: Fetch the file via a symlink + fetch: + src: "{{ remote_tmp_dir }}/link" + dest: "{{ output_dir }}/fetched-link" + register: fetched_symlink + +- name: Assert that we fetched correctly + assert: + that: + - fetched_symlink is changed + - fetched_symlink.checksum == "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3" + - fetched_symlink.remote_checksum == "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3" + - 'lookup("file", output_dir + "/fetched-link/" + inventory_hostname + remote_tmp_dir + "/link") == "test"' diff --git a/test/integration/targets/fetch/roles/fetch_tests/vars/Darwin.yml b/test/integration/targets/fetch/roles/fetch_tests/vars/Darwin.yml new file mode 100644 index 00000000000..0654b7113a0 --- /dev/null +++ b/test/integration/targets/fetch/roles/fetch_tests/vars/Darwin.yml @@ -0,0 +1,3 @@ +# macOS requires users to be in an additional group for ssh access + +_fetch_additional_groups: com.apple.access_ssh diff --git a/test/integration/targets/fetch/roles/fetch_tests/vars/default.yml b/test/integration/targets/fetch/roles/fetch_tests/vars/default.yml new file mode 100644 index 00000000000..69d7958de75 --- /dev/null +++ b/test/integration/targets/fetch/roles/fetch_tests/vars/default.yml @@ -0,0 +1 @@ +_fetch_additional_groups: [] diff --git a/test/integration/targets/fetch/runme.sh b/test/integration/targets/fetch/runme.sh index 7e909dde095..6a9bfa9902f 100755 --- a/test/integration/targets/fetch/runme.sh +++ b/test/integration/targets/fetch/runme.sh @@ -2,11 +2,33 @@ set -eux +function cleanup { + ansible-playbook -i hosts.yml cleanup.yml -e "output_dir=${OUTPUT_DIR}" -b "$@" + unset ANSIBLE_CACHE_PLUGIN + unset ANSIBLE_CACHE_PLUGIN_CONNECTION +} + +trap 'cleanup "$@"' EXIT + # setup required roles ln -s ../../setup_remote_tmp_dir roles/setup_remote_tmp_dir # run old type role tests -ansible-playbook -i ../../inventory run_fetch_tests.yml -e "output_dir=${OUTPUT_DIR}" -v "$@" +ansible-playbook -i ../../inventory run_fetch_tests.yml -e "output_dir=${OUTPUT_DIR}" "$@" + +# run same test with become +ansible-playbook -i ../../inventory run_fetch_tests.yml -e "output_dir=${OUTPUT_DIR}" -b "$@" # run tests to avoid path injection from slurp when fetch uses become -ansible-playbook -i ../../inventory injection/avoid_slurp_return.yml -e "output_dir=${OUTPUT_DIR}" -v "$@" +ansible-playbook -i ../../inventory injection/avoid_slurp_return.yml -e "output_dir=${OUTPUT_DIR}" "$@" + +## Test unreadable file with stat. Requires running without become and as a user other than root. +# +# Change the known_hosts file to avoid changing the test environment +export ANSIBLE_CACHE_PLUGIN=jsonfile +export ANSIBLE_CACHE_PLUGIN_CONNECTION="${OUTPUT_DIR}/cache" +# Create a non-root user account and configure SSH acccess for that account +ansible-playbook -i hosts.yml setup_unreadable_test.yml -e "output_dir=${OUTPUT_DIR}" "$@" + +# Run the tests as the unprivileged user without become to test the use of the stat module from the fetch module +ansible-playbook --user fetcher -i hosts.yml test_unreadable_with_stat.yml -e "output_dir=${OUTPUT_DIR}" "$@" diff --git a/test/integration/targets/fetch/setup_unreadable_test.yml b/test/integration/targets/fetch/setup_unreadable_test.yml new file mode 100644 index 00000000000..4c08969c858 --- /dev/null +++ b/test/integration/targets/fetch/setup_unreadable_test.yml @@ -0,0 +1,39 @@ +- name: Create a user account and configure ssh access + hosts: testhost + gather_facts: no + + tasks: + - import_role: + name: fetch_tests + tasks_from: setup.yml + vars: + # Keep the remote temp dir and cache the remote_tmp_dir fact. The directory itself + # and the fact that contains the path are needed in a separate ansible-playbook run. + setup_remote_tmp_dir_skip_cleanup: yes + setup_remote_tmp_dir_cache_path: yes + + # This prevents ssh access. It is fixed in some container images but not all. + # https://github.com/ansible/distro-test-containers/pull/70 + - name: Remove /run/nologin + file: + path: /run/nologin + state: absent + + # Setup ssh access for the unprivileged user. + - name: Get home directory for temporary user + command: echo ~fetcher + register: fetcher_home + + - name: Create .ssh dir + file: + path: "{{ fetcher_home.stdout }}/.ssh" + state: directory + owner: fetcher + mode: '0700' + + - name: Configure authorized_keys + copy: + src: "~root/.ssh/authorized_keys" + dest: "{{ fetcher_home.stdout }}/.ssh/authorized_keys" + owner: fetcher + mode: '0600' diff --git a/test/integration/targets/fetch/test_unreadable_with_stat.yml b/test/integration/targets/fetch/test_unreadable_with_stat.yml new file mode 100644 index 00000000000..e00026bc6fe --- /dev/null +++ b/test/integration/targets/fetch/test_unreadable_with_stat.yml @@ -0,0 +1,27 @@ +# This playbook needs to be run as a non-root user without become. Under +# those circumstances, the fetch module uses stat and not slurp. + +- name: Test unreadable file using stat + hosts: testhost + gather_facts: no + + tasks: + - name: Try to fetch a file inside an inaccessible directory + fetch: + src: "{{ remote_tmp_dir }}/noaccess/file1" + dest: "{{ output_dir }}" + register: failed_fetch_no_access + ignore_errors: yes + + - name: Try to fetch a file inside an inaccessible directory without fail_on_missing + fetch: + src: "{{ remote_tmp_dir }}/noaccess/file1" + dest: "{{ output_dir }}" + fail_on_missing: no + register: failed_fetch_no_access_fail_on_missing + + - assert: + that: + - failed_fetch_no_access is failed + - failed_fetch_no_access.msg is search('Permission denied') + - failed_fetch_no_access_fail_on_missing.msg is search(', ignored') diff --git a/test/integration/targets/setup_remote_tmp_dir/defaults/main.yml b/test/integration/targets/setup_remote_tmp_dir/defaults/main.yml new file mode 100644 index 00000000000..3375fdf92c9 --- /dev/null +++ b/test/integration/targets/setup_remote_tmp_dir/defaults/main.yml @@ -0,0 +1,2 @@ +setup_remote_tmp_dir_skip_cleanup: no +setup_remote_tmp_dir_cache_path: no diff --git a/test/integration/targets/setup_remote_tmp_dir/handlers/main.yml b/test/integration/targets/setup_remote_tmp_dir/handlers/main.yml index 229037c8b22..3c5b14f208c 100644 --- a/test/integration/targets/setup_remote_tmp_dir/handlers/main.yml +++ b/test/integration/targets/setup_remote_tmp_dir/handlers/main.yml @@ -1,5 +1,7 @@ - name: delete temporary directory include_tasks: default-cleanup.yml + when: not setup_remote_tmp_dir_skip_cleanup | bool - name: delete temporary directory (windows) include_tasks: windows-cleanup.yml + when: not setup_remote_tmp_dir_skip_cleanup | bool diff --git a/test/integration/targets/setup_remote_tmp_dir/tasks/default.yml b/test/integration/targets/setup_remote_tmp_dir/tasks/default.yml index 1e0f51b8904..3be42effa31 100644 --- a/test/integration/targets/setup_remote_tmp_dir/tasks/default.yml +++ b/test/integration/targets/setup_remote_tmp_dir/tasks/default.yml @@ -9,3 +9,4 @@ - name: record temporary directory set_fact: remote_tmp_dir: "{{ remote_tmp_dir.path }}" + cacheable: "{{ setup_remote_tmp_dir_cache_path | bool }}"