Bugfix/769/mode option does not behave the same way that it does in the community module

changelogs/fragments/795_overwrite_permissions_on_copy.yml

@@ -0,0 +1,4 @@
+bugfixes:
+- zos_copy - kept permissions on target directory when copy overwrote
+  files. The fix now set permissions when mode is given.
+  (https://github.com/ansible-collections/ibm_zos_core/pull/795)

plugins/modules/zos_copy.py

@@ -1241,27 +1241,30 @@ def _get_changed_files(self, src, dest, copy_directory):
                      for the files and directories already present on the
                      destination.
         """
-        copied_files = self._walk_uss_tree(src)
+        files_to_copy = self._walk_uss_tree(src)
 
         # It's not needed to normalize the path because it was already normalized
         # on _copy_to_dir.
         parent_dir = os.path.basename(src) if copy_directory else ''
 
-        changed_files = []
-        original_files = []
-        for relative_path in copied_files:
+        files_to_change = []
+        existing_files = []
+        for relative_path in files_to_copy:
             if os.path.exists(os.path.join(dest, parent_dir, relative_path)):
-                original_files.append(relative_path)
+                existing_files.append(relative_path)
             else:
-                changed_files.append(relative_path)
+                files_to_change.append(relative_path)
 
+        # This change adds to the files_to_change variable any file that accord with
+        # a name found in the source copy.
+        files_to_change.extend(existing_files)
         # Creating tuples with (filename, permissions).
         original_permissions = [
             (filepath, os.stat(os.path.join(dest, parent_dir, filepath)).st_mode)
-            for filepath in original_files
+            for filepath in existing_files
         ]
 
-        return changed_files, original_permissions
+        return files_to_change, original_permissions
 
     def _walk_uss_tree(self, dir):
         """Walks the tree directory for dir and returns all relative paths

tests/functional/modules/test_zos_copy_func.py

@@ -17,6 +17,7 @@
 import os
 import shutil
 import re
+import time
 import tempfile
 from tempfile import mkstemp
 
@@ -933,7 +934,7 @@ def test_copy_local_dir_and_change_mode(ansible_zos_module, copy_directory):
         for result in stat_overwritten_file_res.contacted.values():
             assert result.get("stat").get("exists") is True
             assert result.get("stat").get("isdir") is False
-            assert result.get("stat").get("mode") == dest_mode
+            assert result.get("stat").get("mode") == mode
 
         for result in stat_new_file_res.contacted.values():
             assert result.get("stat").get("exists") is True
@@ -1027,7 +1028,7 @@ def test_copy_uss_dir_and_change_mode(ansible_zos_module, copy_directory):
         for result in stat_overwritten_file_res.contacted.values():
             assert result.get("stat").get("exists") is True
             assert result.get("stat").get("isdir") is False
-            assert result.get("stat").get("mode") == dest_mode
+            assert result.get("stat").get("mode") == mode
 
         for result in stat_new_file_res.contacted.values():
             assert result.get("stat").get("exists") is True
@@ -1110,52 +1111,32 @@ def test_copy_non_existent_file_fails(ansible_zos_module, is_remote):
 def test_ensure_copy_file_does_not_change_permission_on_dest(ansible_zos_module, src):
     hosts = ansible_zos_module
     dest_path = "/tmp/test/"
+    mode = "750"
+    other_mode = "744"
+    mode_overwrite = "0777"
+    full_path = "{0}/profile".format(dest_path)
     try:
-        hosts.all.file(path=dest_path, state="directory", mode="750")
-        permissions_before = hosts.all.shell(cmd="ls -la {0}".format(dest_path))
-        hosts.all.zos_copy(content=src["src"], dest=dest_path)
-        permissions = hosts.all.shell(cmd="ls -la {0}".format(dest_path))
+        hosts.all.file(path=dest_path, state="directory", mode=mode)
+        permissions_before = hosts.all.stat(path=dest_path)
+        hosts.all.zos_copy(src=src["src"], dest=dest_path, mode=other_mode)
+        permissions = hosts.all.stat(path=dest_path)
 
         for before in permissions_before.contacted.values():
-            permissions_be_copy = before.get("stdout")
-
-        for after in permissions.contacted.values():
-            permissions_af_copy = after.get("stdout") 
-
-        permissions_be_copy = permissions_be_copy.splitlines()[1].split()[0]
-        permissions_af_copy = permissions_af_copy.splitlines()[1].split()[0]
-
-        assert permissions_be_copy == permissions_af_copy
-    finally:
-        hosts.all.file(path=dest_path, state="absent")
-
-
-@pytest.mark.uss
-@pytest.mark.parametrize("src", [
-    dict(src="/etc/", is_remote=False),
-    dict(src="/etc/", is_remote=True),])
-def test_ensure_copy_directory_does_not_change_permission_on_dest(ansible_zos_module, src):
-    hosts = ansible_zos_module
-    dest_path = "/tmp/test/"
-    try:
-        hosts.all.file(path=dest_path, state="directory", mode="750")
-        permissions_before = hosts.all.shell(cmd="ls -la {0}".format(dest_path))
-        hosts.all.zos_copy(content=src["src"], dest=dest_path)
-        permissions = hosts.all.shell(cmd="ls -la {0}".format(dest_path))
-
-        for before in permissions_before.contacted.values():
-            permissions_be_copy = before.get("stdout")
+            permissions_be_copy = before.get("stat").get("mode")
 
         for after in permissions.contacted.values():
-            permissions_af_copy = after.get("stdout") 
+            permissions_af_copy = after.get("stat").get("mode")
 
-        permissions_be_copy = permissions_be_copy.splitlines()[1].split()[0]
-        permissions_af_copy = permissions_af_copy.splitlines()[1].split()[0]
-
         assert permissions_be_copy == permissions_af_copy
+
+        # Extra asserts to ensure change mode rewrite a copy
+        hosts.all.zos_copy(src=src["src"], dest=dest_path, mode=mode_overwrite)
+        permissions_overwriten = hosts.all.stat(path = full_path)
+        for over in permissions_overwriten.contacted.values():
+            assert over.get("stat").get("mode") == mode_overwrite
     finally:
         hosts.all.file(path=dest_path, state="absent")
-        
+
 
 @pytest.mark.uss
 @pytest.mark.seq