simplify copy module (#84313)

* simplify redundancy with AnsibleModule set_*_if_different methods

* simplify copying a source directory to a dest directory without modifying behavior

changelogs/fragments/simplify-copy-module.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - copy - refactor copy module for simplicity.

lib/ansible/modules/copy.py

@@ -286,10 +286,8 @@ state:
 
 import errno
 import filecmp
-import grp
 import os
 import os.path
-import pwd
 import shutil
 import stat
 import tempfile
@@ -335,67 +333,24 @@ def adjust_recursive_directory_permissions(pre_existing_dir, new_directory_list,
     return changed
 
 
+def chown_path(module, path, owner, group):
+    """Update the owner/group if specified and different from the current owner/group."""
+    changed = module.set_owner_if_different(path, owner, False)
+    return module.set_group_if_different(path, group, changed)
+
+
 def chown_recursive(path, module):
     changed = False
     owner = module.params['owner']
     group = module.params['group']
 
-    if owner is not None:
+    # TODO: Consolidate with the other methods calling set_*_if_different method, this is inefficient.
-        if not module.check_mode:
-            for dirpath, dirnames, filenames in os.walk(path):
-                owner_changed = module.set_owner_if_different(dirpath, owner, False)
-                if owner_changed is True:
-                    changed = owner_changed
-                for dir in [os.path.join(dirpath, d) for d in dirnames]:
-                    owner_changed = module.set_owner_if_different(dir, owner, False)
-                    if owner_changed is True:
-                        changed = owner_changed
-                for file in [os.path.join(dirpath, f) for f in filenames]:
-                    owner_changed = module.set_owner_if_different(file, owner, False)
-                    if owner_changed is True:
-                        changed = owner_changed
-        else:
-            uid = pwd.getpwnam(owner).pw_uid
-            for dirpath, dirnames, filenames in os.walk(path):
-                owner_changed = (os.stat(dirpath).st_uid != uid)
-                if owner_changed is True:
-                    changed = owner_changed
-                for dir in [os.path.join(dirpath, d) for d in dirnames]:
-                    owner_changed = (os.stat(dir).st_uid != uid)
-                    if owner_changed is True:
-                        changed = owner_changed
-                for file in [os.path.join(dirpath, f) for f in filenames]:
-                    owner_changed = (os.stat(file).st_uid != uid)
-                    if owner_changed is True:
-                        changed = owner_changed
-    if group is not None:
-        if not module.check_mode:
     for dirpath, dirnames, filenames in os.walk(path):
-                group_changed = module.set_group_if_different(dirpath, group, False)
+        changed |= chown_path(module, dirpath, owner, group)
-                if group_changed is True:
+        for subdir in [os.path.join(dirpath, d) for d in dirnames]:
-                    changed = group_changed
+            changed |= chown_path(module, subdir, owner, group)
-                for dir in [os.path.join(dirpath, d) for d in dirnames]:
+        for filepath in [os.path.join(dirpath, f) for f in filenames]:
-                    group_changed = module.set_group_if_different(dir, group, False)
+            changed |= chown_path(module, filepath, owner, group)
-                    if group_changed is True:
-                        changed = group_changed
-                for file in [os.path.join(dirpath, f) for f in filenames]:
-                    group_changed = module.set_group_if_different(file, group, False)
-                    if group_changed is True:
-                        changed = group_changed
-        else:
-            gid = grp.getgrnam(group).gr_gid
-            for dirpath, dirnames, filenames in os.walk(path):
-                group_changed = (os.stat(dirpath).st_gid != gid)
-                if group_changed is True:
-                    changed = group_changed
-                for dir in [os.path.join(dirpath, d) for d in dirnames]:
-                    group_changed = (os.stat(dir).st_gid != gid)
-                    if group_changed is True:
-                        changed = group_changed
-                for file in [os.path.join(dirpath, f) for f in filenames]:
-                    group_changed = (os.stat(file).st_gid != gid)
-                    if group_changed is True:
-                        changed = group_changed
 
     return changed
 
@@ -423,10 +378,7 @@ def copy_diff_files(src, dest, module):
                 shutil.copyfile(b_src_item_path, b_dest_item_path)
                 shutil.copymode(b_src_item_path, b_dest_item_path)
 
-            if owner is not None:
+            chown_path(module, b_dest_item_path, owner, group)
-                module.set_owner_if_different(b_dest_item_path, owner, False)
-            if group is not None:
-                module.set_group_if_different(b_dest_item_path, group, False)
             changed = True
     return changed
 
@@ -458,10 +410,7 @@ def copy_left_only(src, dest, module):
 
             if os.path.islink(b_src_item_path) and os.path.isfile(b_src_item_path) and local_follow is True:
                 shutil.copyfile(b_src_item_path, b_dest_item_path)
-                if owner is not None:
+                chown_path(module, b_dest_item_path, owner, group)
-                    module.set_owner_if_different(b_dest_item_path, owner, False)
-                if group is not None:
-                    module.set_group_if_different(b_dest_item_path, group, False)
 
             if os.path.islink(b_src_item_path) and os.path.isfile(b_src_item_path) and local_follow is False:
                 linkto = os.readlink(b_src_item_path)
@@ -470,11 +419,7 @@ def copy_left_only(src, dest, module):
             if not os.path.islink(b_src_item_path) and os.path.isfile(b_src_item_path):
                 shutil.copyfile(b_src_item_path, b_dest_item_path)
                 shutil.copymode(b_src_item_path, b_dest_item_path)
-
+                chown_path(module, b_dest_item_path, owner, group)
-                if owner is not None:
-                    module.set_owner_if_different(b_dest_item_path, owner, False)
-                if group is not None:
-                    module.set_group_if_different(b_dest_item_path, group, False)
 
             if not os.path.islink(b_src_item_path) and os.path.isdir(b_src_item_path):
                 shutil.copytree(b_src_item_path, b_dest_item_path, symlinks=not local_follow)
@@ -502,6 +447,21 @@ def copy_common_dirs(src, dest, module):
     return changed
 
 
+def copy_directory(src, dest, module):
+    if not os.path.exists(dest):
+        if not module.check_mode:
+            shutil.copytree(src, dest, symlinks=not module.params['local_follow'])
+            chown_recursive(dest, module)
+        changed = True
+    else:
+        diff_files_changed = copy_diff_files(src, dest, module)
+        left_only_changed = copy_left_only(src, dest, module)
+        common_dirs_changed = copy_common_dirs(src, dest, module)
+        owner_group_changed = chown_recursive(dest, module)
+        changed = any([diff_files_changed, left_only_changed, common_dirs_changed, owner_group_changed])
+    return changed
+
+
 def main():
 
     module = AnsibleModule(
@@ -652,12 +612,8 @@ def main():
                 if validate:
                     # if we have a mode, make sure we set it on the temporary
                     # file source as some validations may require it
-                    if mode is not None:
                     module.set_mode_if_different(src, mode, False)
-                    if owner is not None:
+                    chown_path(module, src, owner, group)
-                        module.set_owner_if_different(src, owner, False)
-                    if group is not None:
-                        module.set_group_if_different(src, group, False)
                     if "%s" not in validate:
                         module.fail_json(msg="validate must contain %%s: %s" % (validate))
                     (rc, out, err) = module.run_command(validate % src)
@@ -686,59 +642,18 @@ def main():
         changed = True
 
     # If neither have checksums, both src and dest are directories.
-    if checksum_src is None and checksum_dest is None:
+    checksums_none = checksum_src is None and checksum_dest is None
-        if remote_src and os.path.isdir(module.params['src']):
+    both_directories = os.path.isdir(module.params['src']) and (os.path.isdir(module.params['dest']) or not os.path.exists(module.params['dest']))
+    if checksums_none and remote_src and both_directories:
         b_src = to_bytes(module.params['src'], errors='surrogate_or_strict')
         b_dest = to_bytes(module.params['dest'], errors='surrogate_or_strict')
 
-            if src.endswith(os.path.sep) and os.path.isdir(module.params['dest']):
+        if not b_src.endswith(to_bytes(os.path.sep)):
-                diff_files_changed = copy_diff_files(b_src, b_dest, module)
+            b_basename = os.path.basename(b_src)
-                left_only_changed = copy_left_only(b_src, b_dest, module)
+            b_dest = os.path.join(b_dest, b_basename)
-                common_dirs_changed = copy_common_dirs(b_src, b_dest, module)
+            b_src = os.path.join(b_src, b'')
-                owner_group_changed = chown_recursive(b_dest, module)
-                if diff_files_changed or left_only_changed or common_dirs_changed or owner_group_changed:
-                    changed = True
-
-            if src.endswith(os.path.sep) and not os.path.exists(module.params['dest']):
-                b_basename = to_bytes(os.path.basename(src), errors='surrogate_or_strict')
-                b_dest = to_bytes(os.path.join(b_dest, b_basename), errors='surrogate_or_strict')
-                b_src = to_bytes(os.path.join(module.params['src'], ""), errors='surrogate_or_strict')
-                if not module.check_mode:
-                    shutil.copytree(b_src, b_dest, symlinks=not local_follow)
-                chown_recursive(dest, module)
-                changed = True
 
-            if not src.endswith(os.path.sep) and os.path.isdir(module.params['dest']):
+        changed |= copy_directory(b_src, b_dest, module)
-                b_basename = to_bytes(os.path.basename(src), errors='surrogate_or_strict')
-                b_dest = to_bytes(os.path.join(b_dest, b_basename), errors='surrogate_or_strict')
-                b_src = to_bytes(os.path.join(module.params['src'], ""), errors='surrogate_or_strict')
-                if not module.check_mode and not os.path.exists(b_dest):
-                    shutil.copytree(b_src, b_dest, symlinks=not local_follow)
-                    changed = True
-                    chown_recursive(dest, module)
-                if module.check_mode and not os.path.exists(b_dest):
-                    changed = True
-                if os.path.exists(b_dest):
-                    diff_files_changed = copy_diff_files(b_src, b_dest, module)
-                    left_only_changed = copy_left_only(b_src, b_dest, module)
-                    common_dirs_changed = copy_common_dirs(b_src, b_dest, module)
-                    owner_group_changed = chown_recursive(b_dest, module)
-                    if diff_files_changed or left_only_changed or common_dirs_changed or owner_group_changed:
-                        changed = True
-
-            if not src.endswith(os.path.sep) and not os.path.exists(module.params['dest']):
-                b_basename = to_bytes(os.path.basename(module.params['src']), errors='surrogate_or_strict')
-                b_dest = to_bytes(os.path.join(b_dest, b_basename), errors='surrogate_or_strict')
-                if not module.check_mode and not os.path.exists(b_dest):
-                    os.makedirs(b_dest)
-                    changed = True
-                    b_src = to_bytes(os.path.join(module.params['src'], ""), errors='surrogate_or_strict')
-                    diff_files_changed = copy_diff_files(b_src, b_dest, module)
-                    left_only_changed = copy_left_only(b_src, b_dest, module)
-                    common_dirs_changed = copy_common_dirs(b_src, b_dest, module)
-                    owner_group_changed = chown_recursive(b_dest, module)
-                if module.check_mode and not os.path.exists(b_dest):
-                    changed = True
 
     res_args = dict(
         dest=dest, src=src, md5sum=md5sum_src, checksum=checksum_src, changed=changed