From ecfa7f696d22d85c58be9a13a39cb533dcfdbf3b Mon Sep 17 00:00:00 2001 From: Mehran Kholdi Date: Sun, 17 Jan 2016 21:43:09 +0330 Subject: [PATCH] Enforce `state='file'` in `copy` module This was causing wrong behaviour when `prev_state` was `hard`-link, since the `file` module tried to apply the same `state` on the new file, causing unexpected errors. Particularly, both `overlay` and `devicemapper` storage drivers in docker use hardlinks to share files between layers. This causes most ansible playbooks to fail when working with files from layers below. --- lib/ansible/modules/files/file.py | 1 + lib/ansible/plugins/action/copy.py | 3 +- test/integration/targets/copy/tasks/tests.yml | 40 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/lib/ansible/modules/files/file.py b/lib/ansible/modules/files/file.py index a250736263..68eddcec66 100644 --- a/lib/ansible/modules/files/file.py +++ b/lib/ansible/modules/files/file.py @@ -250,6 +250,7 @@ def main(): if basename: params['path'] = path = os.path.join(path, basename) b_path = to_bytes(path, errors='surrogate_or_strict') + prev_state = get_state(b_path) # make sure the target path is a directory when we're doing a recursive operation if recurse and state != 'directory': diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index 758ec4d13b..fa7bbf1f54 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -331,7 +331,8 @@ class ActionModule(ActionBase): dict( src=source_rel, dest=dest, - original_basename=source_rel + original_basename=source_rel, + state='file', ) ) if lmode: diff --git a/test/integration/targets/copy/tasks/tests.yml b/test/integration/targets/copy/tasks/tests.yml index 71cbe81dcc..6f8a15305d 100644 --- a/test/integration/targets/copy/tasks/tests.yml +++ b/test/integration/targets/copy/tasks/tests.yml @@ -156,6 +156,18 @@ - "stat_results.stat.checksum == ('modified'|hash('sha1'))" - "stat_results.stat.mode == '0700'" +- name: Create a hardlink to the file + file: + src: '{{ remote_file }}' + dest: '{{ output_dir }}/hard.lnk' + state: hard + +- name: copy the same contents into place + copy: + content: 'modified' + dest: '{{ remote_file }}' + mode: 0404 + - name: Try invalid copy input location fails copy: src: invalid_file_location_does_not_exist @@ -1072,6 +1084,34 @@ - "stat_results.stat.checksum == ('modified'|hash('sha1'))" - "not stat_results.stat.islnk" +- name: setup directory for test + file: state=directory dest={{remote_dir }}/directory mode=0755 + +- name: set file mode when the destination is a directory + copy: src=foo.txt dest={{remote_dir}}/directory/ mode=0705 + +- name: set file mode when the destination is a directory + copy: src=foo.txt dest={{remote_dir}}/directory/ mode=0604 + register: file_result + +- name: check that the file has the correct attributes + stat: path={{ remote_dir }}/directory/foo.txt + register: file_attrs + +- assert: + that: + - "file_attrs.stat.uid == 0" + - "file_attrs.stat.pw_name == 'root'" + - "file_attrs.stat.mode == '0604'" + +- name: check that the containing directory did not change attributes + stat: path={{ remote_dir }}/directory/ + register: dir_attrs + +- assert: + that: + - "dir_attrs.stat.mode == '0755'" + # # I believe the below section is now covered in the recursive copying section. # Hold on for now as an original test case but delete once confirmed that