From 8ac814dfafbd7e22840416ee494b040dd9660512 Mon Sep 17 00:00:00 2001
From: Simon <simonleary@umass.edu>
Date: Tue, 25 Jul 2023 16:17:18 -0400
Subject: [PATCH] copy module diff output for multi files

add changelog fragment

add check:true to recursive copy test

typo

add .yml suffix

Update changelogs/fragments/81346-copy-recursive-diff-fix.yml

Co-authored-by: Sloane Hertel <19572925+s-hertel@users.noreply.github.com>

assert multiple diffs

more strict testing

assert is list

accept diff as either list or dict
---
 .../81346-copy-recursive-diff-fix.yml         |  2 ++
 lib/ansible/plugins/action/copy.py            | 17 +++++++--
 test/integration/targets/copy/tasks/main.yml  | 36 +++++++++++++++++++
 3 files changed, 53 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/fragments/81346-copy-recursive-diff-fix.yml

diff --git a/changelogs/fragments/81346-copy-recursive-diff-fix.yml b/changelogs/fragments/81346-copy-recursive-diff-fix.yml
new file mode 100644
index 00000000000..9a10c92ae27
--- /dev/null
+++ b/changelogs/fragments/81346-copy-recursive-diff-fix.yml
@@ -0,0 +1,2 @@
+bugfixes:
+- copy now returns the diff for all content when copying recursively with remote_src=false.
diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py
index 9180d1254cb..333da2e3d44 100644
--- a/lib/ansible/plugins/action/copy.py
+++ b/lib/ansible/plugins/action/copy.py
@@ -200,6 +200,13 @@ def _walk_dirs(topdir, base_path=None, local_follow=False, trailing_slash_detect
 
     return r_files
 
+def combine_result_diffs(dest, source):
+    if not source.get("diff", None):
+        return
+    if isinstance(source["diff"], list):
+        dest["diff"] += source["diff"]
+    else:
+        dest["diff"].append(source["diff"])
 
 class ActionModule(ActionBase):
 
@@ -417,6 +424,7 @@ class ActionModule(ActionBase):
         remote_src = boolean(self._task.args.get('remote_src', False), strict=False)
         local_follow = boolean(self._task.args.get('local_follow', True), strict=False)
 
+        result['diff'] = []
         result['failed'] = True
         if not source and content is None:
             result['msg'] = 'src (or content) is required'
@@ -529,10 +537,10 @@ class ActionModule(ActionBase):
             for dir_component in paths:
                 os.path.join(dir_path, dir_component)
                 implicit_directories.add(dir_path)
-            if 'diff' in result and not result['diff']:
-                del result['diff']
+
             module_executed = True
             changed = changed or module_return.get('changed', False)
+            combine_result_diffs(result, module_return)
 
         for src, dest_path in source_files['directories']:
             # Find directories that are leaves as they might not have been
@@ -556,6 +564,7 @@ class ActionModule(ActionBase):
 
             module_executed = True
             changed = changed or module_return.get('changed', False)
+            combine_result_diffs(result, module_return)
 
         for target_path, dest_path in source_files['symlinks']:
             # Copy symlinks over
@@ -582,6 +591,7 @@ class ActionModule(ActionBase):
                 return self._ensure_invocation(result)
 
             changed = changed or module_return.get('changed', False)
+            combine_result_diffs(result, module_return)
 
         if module_executed and len(source_files['files']) == 1:
             result.update(module_return)
@@ -593,6 +603,9 @@ class ActionModule(ActionBase):
         else:
             result.update(dict(dest=dest, src=source, changed=changed))
 
+        if 'diff' in result and not result['diff']:
+            del result['diff']
+
         # Delete tmp path
         self._remove_tmp_path(self._connection._shell.tmpdir)
 
diff --git a/test/integration/targets/copy/tasks/main.yml b/test/integration/targets/copy/tasks/main.yml
index b86c56acbf2..418afbb5b92 100644
--- a/test/integration/targets/copy/tasks/main.yml
+++ b/test/integration/targets/copy/tasks/main.yml
@@ -96,6 +96,42 @@
           - 'diff_output.diff[0].before == ""'
           - '"Ansible managed" in diff_output.diff[0].after'
 
+    - name: create new directory
+      file:
+        path: "{{ role_path }}/files/two-file-diff-test.d"
+        state: directory
+
+    - name: create 2 files in directory, containing the text "foo" and "bar"
+      copy:
+        dest: "{{ role_path }}/files/two-file-diff-test.d/{{ item }}"
+        content: "{{ item }}"
+      loop:
+        - foo
+        - bar
+
+    - name: copy directory with 2 files in diff mode
+      copy:
+        src: "{{ role_path }}/files/two-file-diff-test.d"
+        dest: "{{ local_temp_dir }}/two-file-diff-test.d"
+      register: two_file_diff_test
+      diff: true
+
+    # right now it returns exactly 2, but if/when the copy module is updated to produce attribute
+    # diffs, there will be more.
+    - name: assert that at least 2 diffs returned when creating 2 files
+      assert:
+        that:
+          - two_file_diff_test.diff is not string and two_file_diff_test.diff is not mapping and two_file_diff_test.diff is iterable
+          - two_file_diff_test.diff | length >= 2
+
+    # right now they all show file contents, but if/when the copy module is updated to produce attribute
+    # diffs, that will not be the case.
+    - name: assert that exactly 2 diffs show file contents, which are "foo" and "bar"
+      assert:
+        that:
+          # diff.after should be a dict if it's not file contents
+          - (two_file_diff_test.diff | selectattr('after', 'string') | map(attribute='after') | sort) == ["bar", "foo"]
+
     - name: tests with remote_src and non files
       import_tasks: src_remote_file_is_not_file.yml