ansible.builtin.copy module - updates pre-existing files' ownership of dest directory if remote_src: true

[angelavuong]

Summary

When using the copy module with remote_src: true, I'm observing unexpected behavior where the pre-existing files' ownership in the destination folder gets modified by the playbook:

Before:

[student@server-1 tmp]$ ll filedest/
total 4
-rw-rw-r--. 1 student student 15 Jan  6 15:04 leavemebe.txt

After: (existing leavemebe.txt got it's owner/group changed)

[root@server-1 ~]# ll /tmp/filedest/
total 8
-rw-r--r--. 1 root root 10 Jan  6 15:27 copyme.txt
-rw-rw-r--. 1 root root 15 Jan  6 15:04 leavemebe.txt

Issue Type

Bug Report

Component Name

ansible.builtin.copy

Ansible Version

$ ansible --version
ansible 2.9.27
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.6/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.6.8 (default, Jan 14 2022, 11:04:20) [GCC 8.5.0 20210514 (Red Hat 8.5.0-7)]

Configuration

# if using a version older than ansible-core 2.12 you should omit the '-t all'
$ ansible-config dump --only-changed -t all

OS / Environment

Red Hat Enterprise Linux release 8.6 (Ootpa)

Steps to Reproduce

1. Create source and destination files and directories on the remote host client:

[student@server-1 ~]$ cd /tmp/
[student@server-1 tmp]$ mkdir filesrc
[student@server-1 tmp]$ mkdir filedest
[student@server-1 tmp]$ vi filesrc/copyme.txt
[student@server-1 tmp]$ vi filedest/leavemebe.txt
[student@server-1 tmp]$ ll filesrc/
total 4
-rw-rw-r--. 1 student student 10 Jan  6 15:04 copyme.txt
[student@server-1 tmp]$ ll filedest/
total 4
-rw-rw-r--. 1 student student 15 Jan  6 15:04 leavemebe.txt

2. Run the playbook on the Ansible host:

---
- hosts: all
  remote_user: student
  gather_facts: false

  tasks:
    - name: copy file
      ansible.builtin.copy:
        remote_src: true
        src: /tmp/filesrc/
        dest: /tmp/filedest/
        owner: root
        group: root
      become: true

3. After the playbook is run (on the client host):

[root@server-1 ~]# ll /tmp/filesrc
total 4
-rw-rw-r--. 1 student student 10 Jan  6 15:04 copyme.txt

[root@server-1 ~]# ll /tmp/filedest/
total 8
-rw-r--r--. 1 root root 10 Jan  6 15:27 copyme.txt
-rw-rw-r--. 1 root root 15 Jan  6 15:04 leavemebe.txt

This behavior only happens with using remote_src: true. When running the copy module on the same Ansible host, this behavior does not occur.

Expected Results

I expected the copy module to only modify the file ownership of the copied files (copyme.txt) and not the pre-existing files' ownerships in the destination directory (leavemebe.txt).

Actual Results

We can see that the playbook changed both the copied files (copyme.txt) and existing files (leavemebe.txt)'s ownership to root:root in the destination directory.

[root@server-1 ~]# ll /tmp/filedest/
total 8
-rw-r--r--. 1 root root 10 Jan  6 15:27 copyme.txt
-rw-rw-r--. 1 root root 15 Jan  6 15:04 leavemebe.txt

Code of Conduct

• I agree to follow the Ansible Code of Conduct

[ansibot]

Files identified in the description:

• lib/ansible/modules/copy.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

[sivel]

Effectively as of now, the module knows what files it need copied through its use of filecmp.dircmp. However, these lists are effectively thrown out, and at the end the module does a recursive os.walk across the dest dir, and sets the perms on everything, not just the files the module was to concern itself with.

We would need to track all of the files that needed to be copied, as well as those that already existed but didn't need to be copied because they match, and then only set file attributes/owner/etc on that list.

A quick filecmp.dircmp(src, dest).left_list might be sufficient instead of tracking everything across multiple function calls.

[h3nd24]

In the current structure, I suspect we actually need to track what files are changed because of the diff files. If a common file is exactly the same between source and destination, then we shouldn't change the ownership as well. However, since it appears in the left_list, I think the ownership will also get changed. Do you think maybe it is easier instead to change the ownership as we copy things across?

[h3nd24]

I created a PR and my local testing for the functionality seems alright