diff --git a/changelogs/fragments/atomic_update_perms_time.yml b/changelogs/fragments/atomic_update_perms_time.yml new file mode 100644 index 00000000000..f776845e380 --- /dev/null +++ b/changelogs/fragments/atomic_update_perms_time.yml @@ -0,0 +1,2 @@ +bugfixes: + - module_utils atomic_move (used by most file based modules), now correctly handles permission copy and setting mtime correctly across all paths diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 1cbb461a2ae..19dbb1d1541 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -1557,7 +1557,7 @@ class AnsibleModule(object): # Similar to shutil.copy(), but metadata is copied as well - in fact, # this is just shutil.copy() followed by copystat(). This is similar # to the Unix command cp -p. - # + # shutil.copystat(src, dst) # Copy the permission bits, last access time, last modification time, # and flags from src to dst. The file contents, owner, and group are @@ -1660,8 +1660,10 @@ class AnsibleModule(object): b_tmp_dest_name, context, False) try: tmp_stat = os.stat(b_tmp_dest_name) - if keep_dest_attrs and dest_stat and (tmp_stat.st_uid != dest_stat.st_uid or tmp_stat.st_gid != dest_stat.st_gid): - os.chown(b_tmp_dest_name, dest_stat.st_uid, dest_stat.st_gid) + if keep_dest_attrs: + if dest_stat and (tmp_stat.st_uid != dest_stat.st_uid or tmp_stat.st_gid != dest_stat.st_gid): + os.chown(b_tmp_dest_name, dest_stat.st_uid, dest_stat.st_gid) + os.utime(b_tmp_dest_name, times=(time.time(), time.time())) except OSError as e: if e.errno != errno.EPERM: raise diff --git a/test/integration/targets/copy/tasks/tests.yml b/test/integration/targets/copy/tasks/tests.yml index 906c441541b..35c4cdf9414 100644 --- a/test/integration/targets/copy/tasks/tests.yml +++ b/test/integration/targets/copy/tasks/tests.yml @@ -2490,3 +2490,45 @@ state: absent loop: - '{{ remote_file }}' + +- name: Verify atime and mtime update on content change (diff partition) + vars: + remote_file: "/var/tmp/foo.txt" + ansible_remote_tmp: "/tmp" + block: + - name: Create a dest file + shell: "echo Test content > {{ remote_file }}" + register: create_dest_result + + - name: Check the stat results of the file before copying + stat: + path: "{{ remote_file }}" + register: stat_results_before_copy + + - name: Overwrite the file using the content system + copy: + content: "modified" + dest: "{{ remote_file }}" + decrypt: no + register: copy_result + + - name: Check the stat results of the file after copying + stat: + path: "{{ remote_file }}" + register: stat_results_after_copy + + - name: Assert that the file has changed + assert: + that: + - "create_dest_result is changed" + - "copy_result is changed" + - "'content' not in copy_result" + - "stat_results_before_copy.stat.atime < stat_results_after_copy.stat.atime" + - "stat_results_before_copy.stat.mtime < stat_results_after_copy.stat.mtime" + always: + - name: clean up dest file + file: + path: '{{ item }}' + state: absent + loop: + - '{{ remote_file }}'