From f79c606aae07c8ee9413bf44c22f347ef9ffaf26 Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Thu, 18 Jun 2020 00:34:50 +0530 Subject: [PATCH] copy: ensure _original_basename is set (#70088) * added changelog fragment * added quick and basic test * Revert "added quick and basic test" * This reverts commit 75f4141656635841d4ce18994667e3b44b7b1289. * added better tests * now also creating files to copy on the remote * removed tests for recursive copying which is not supported by remote_src Fixes: #47050 (cherry picked from commit 79dfae9624480f757795224732bfaa9ca1dd4543) Co-authored-by: Moritz Grimm --- ...copy_ensure-_original_basename-is-set.yaml | 3 ++ lib/ansible/modules/files/copy.py | 5 ++- ...in_non_existent_directories_remote_src.yml | 43 +++++++++++++++++++ ...st_file_in_non_existent_dir_remote_src.yml | 32 ++++++++++++++ test/integration/targets/copy/tasks/tests.yml | 17 +++++++- 5 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/47050-copy_ensure-_original_basename-is-set.yaml create mode 100644 test/integration/targets/copy/tasks/dest_in_non_existent_directories_remote_src.yml create mode 100644 test/integration/targets/copy/tasks/src_file_dest_file_in_non_existent_dir_remote_src.yml diff --git a/changelogs/fragments/47050-copy_ensure-_original_basename-is-set.yaml b/changelogs/fragments/47050-copy_ensure-_original_basename-is-set.yaml new file mode 100644 index 00000000000..5280d20c9ff --- /dev/null +++ b/changelogs/fragments/47050-copy_ensure-_original_basename-is-set.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: + - copy - Fixed copy module not working in case that remote_src is enabled and dest ends in a / (https://github.com/ansible/ansible/pull/47238) diff --git a/lib/ansible/modules/files/copy.py b/lib/ansible/modules/files/copy.py index 21aa74c9379..f0ea6376615 100644 --- a/lib/ansible/modules/files/copy.py +++ b/lib/ansible/modules/files/copy.py @@ -573,8 +573,9 @@ def main(): ) # Special handling for recursive copy - create intermediate dirs - if _original_basename and dest.endswith(os.sep): - dest = os.path.join(dest, _original_basename) + if dest.endswith(os.sep): + if _original_basename: + dest = os.path.join(dest, _original_basename) b_dest = to_bytes(dest, errors='surrogate_or_strict') dirname = os.path.dirname(dest) b_dirname = to_bytes(dirname, errors='surrogate_or_strict') diff --git a/test/integration/targets/copy/tasks/dest_in_non_existent_directories_remote_src.yml b/test/integration/targets/copy/tasks/dest_in_non_existent_directories_remote_src.yml new file mode 100644 index 00000000000..fad53e71db6 --- /dev/null +++ b/test/integration/targets/copy/tasks/dest_in_non_existent_directories_remote_src.yml @@ -0,0 +1,43 @@ +# src is a file, dest is a non-existent directory (2 levels of directories): +# checks that dest is created +- name: Ensure that dest top directory doesn't exist + file: + path: '{{ remote_dir }}/{{ item.dest.split("/")[0] }}' + state: absent + +- name: create subdir + file: + path: subdir + state: directory + +- name: create src file + file: + path: "{{ item }}" + state: touch + loop: + - foo.txt + - subdir/bar.txt + +- name: Copy file, dest is a nonexistent target directory + copy: + src: '{{ item.src }}' + dest: '{{ remote_dir }}/{{ item.dest }}' + remote_src: true + register: copy_result + +- name: assert copy worked + assert: + that: + - 'copy_result is successful' + - 'copy_result is changed' + +- name: stat copied file + stat: + path: '{{ remote_dir }}/{{ item.check }}' + register: stat_file_in_dir_result + +- name: assert that file exists + assert: + that: + - stat_file_in_dir_result.stat.exists + - stat_file_in_dir_result.stat.isreg diff --git a/test/integration/targets/copy/tasks/src_file_dest_file_in_non_existent_dir_remote_src.yml b/test/integration/targets/copy/tasks/src_file_dest_file_in_non_existent_dir_remote_src.yml new file mode 100644 index 00000000000..61d8796968b --- /dev/null +++ b/test/integration/targets/copy/tasks/src_file_dest_file_in_non_existent_dir_remote_src.yml @@ -0,0 +1,32 @@ +- name: Ensure that dest top directory doesn't exist + file: + path: '{{ remote_dir }}/{{ dest.split("/")[0] }}' + state: absent + +- name: create src file + file: + path: foo.txt + state: touch + +- name: Copy file, dest is a file in non-existing target directory + copy: + src: foo.txt + dest: '{{ remote_dir }}/{{ dest }}' + remote_src: true + register: copy_result + ignore_errors: True + +- name: Assert copy failed + assert: + that: + - 'copy_result is failed' + +- name: Stat dest path + stat: + path: '{{ remote_dir }}/{{ dest.split("/")[0] }}' + register: stat_file_in_dir_result + +- name: assert that dest doesn't exist + assert: + that: + - 'not stat_file_in_dir_result.stat.exists' diff --git a/test/integration/targets/copy/tasks/tests.yml b/test/integration/targets/copy/tasks/tests.yml index c2e4f1e4661..900f86a66a7 100644 --- a/test/integration/targets/copy/tasks/tests.yml +++ b/test/integration/targets/copy/tasks/tests.yml @@ -1466,6 +1466,22 @@ path: 'ansible-testing.txt' state: absent +# src is a file, dest is a non-existent directory (2 levels of directories): +# using remote_src +# checks that dest is created +- include: dest_in_non_existent_directories_remote_src.yml + with_items: + - { src: 'foo.txt', dest: 'new_sub_dir1/sub_dir2/', check: 'new_sub_dir1/sub_dir2/foo.txt' } + +# src is a file, dest is file in a non-existent directory: checks that a failure occurs +# using remote_src +- include: src_file_dest_file_in_non_existent_dir_remote_src.yml + with_items: + - 'new_sub_dir1/sub_dir2/foo.txt' + - 'new_sub_dir1/foo.txt' + loop_control: + loop_var: 'dest' + # src is a file, dest is a non-existent directory (2 levels of directories): # checks that dest is created - include: dest_in_non_existent_directories.yml @@ -1483,7 +1499,6 @@ - 'new_sub_dir1/foo.txt' loop_control: loop_var: 'dest' - # # Recursive copying on remote host #