Fixes for mode=preserve (#39343)

* Fixes for mode=preserve

* Document mode=preserve for template and copy
* Make mode=preserve work with remote_src for copy
* Make mode=preserve work for template
* Integration tests for copy & template mode=preserve

Fixes #39279

* Changed mode option in win_copy to hidden option as it doesn't reflect copy mode

changelogs/fragments/mode-preserve.yaml

@@ -0,0 +1,5 @@
+---
+bugfixes:
+- Implement mode=preserve for the template module
+- Fix mode=preserve with remote_src=True for the copy module
+- Document mode=preserve for both the copy and template module

lib/ansible/modules/files/copy.py

@@ -57,6 +57,17 @@ options:
     default: 'yes'
     aliases: [ thirsty ]
     version_added: "1.1"
+  mode:
+    description:
+      - "Mode the file or directory should be. For those used to I(/usr/bin/chmod) remember that
+        modes are actually octal numbers.  You must either specify the leading zero so that
+        Ansible's YAML parser knows it is an octal number (like C(0644) or C(01777)) or quote it
+        (like C('644') or C('0644') so Ansible receives a string and can do its own conversion from
+        string into number.  Giving Ansible a number without following one of these rules will end
+        up with a decimal number which will have unexpected results.  As of version 1.8, the mode
+        may be specified as a symbolic mode (for example, C(u+rwx) or C(u=rw,g=r,o=r)).  As of
+        version 2.3, the mode may also be the special string C(preserve).  C(preserve) means that
+        the file will be given the same permissions as the source file."
   directory_mode:
     description:
       - When doing a recursive copy set the mode for the directories. If this is not set we will use the system
@@ -68,6 +79,7 @@ options:
       - If C(no), it will search for I(src) at originating/master machine.
       - If C(yes) it will go to the remote/target machine for the I(src). Default is C(no).
       - Currently I(remote_src) does not support recursive copying.
+      - I(remote_src) only works with C(mode=preserve) as of version 2.6.
     type: bool
     default: 'no'
     version_added: "2.0"
@@ -220,6 +232,7 @@ state:
 import os
 import os.path
 import shutil
+import stat
 import tempfile
 import traceback
 
@@ -289,7 +302,6 @@ def main():
     original_basename = module.params.get('original_basename', None)
     validate = module.params.get('validate', None)
     follow = module.params['follow']
-    mode = module.params['mode']
     remote_src = module.params['remote_src']
     checksum = module.params['checksum']
 
@@ -300,6 +312,12 @@ def main():
     if os.path.isdir(b_src):
         module.fail_json(msg="Remote copy does not support recursive copy of directory: %s" % (src))
 
+    # Preserve is usually handled in the action plugin but mode + remote_src has to be done on the
+    # remote host
+    if module.params['mode'] == 'preserve':
+        module.params['mode'] = '0%03o' % stat.S_IMODE(os.stat(b_src).st_mode)
+    mode = module.params['mode']
+
     checksum_src = module.sha1(src)
     checksum_dest = None
     # Backwards compat only.  This will be None in FIPS mode

lib/ansible/modules/files/template.py

@@ -95,6 +95,17 @@ options:
     type: bool
     default: 'no'
     version_added: "2.4"
+  mode:
+    description:
+      - "Mode the file or directory should be. For those used to I(/usr/bin/chmod) remember that
+        modes are actually octal numbers.  You must either specify the leading zero so that
+        Ansible's YAML parser knows it is an octal number (like C(0644) or C(01777)) or quote it
+        (like C('644') or C('0644') so Ansible receives a string and can do its own conversion from
+        string into number.  Giving Ansible a number without following one of these rules will end
+        up with a decimal number which will have unexpected results.  As of version 1.8, the mode
+        may be specified as a symbolic mode (for example, C(u+rwx) or C(u=rw,g=r,o=r)).  As of
+        version 2.6, the mode may also be the special string C(preserve).  C(preserve) means that
+        the file will be given the same permissions as the source file."
 notes:
   - For Windows you can use M(win_template) which uses '\\r\\n' as C(newline_sequence).
   - Including a string that uses a date in the template will result in the template being marked 'changed' each time

lib/ansible/modules/windows/win_copy.ps1

@@ -18,10 +18,10 @@ $diff_mode = Get-AnsibleParam -obj $params -name "_ansible_diff" -type "bool" -d
 #   query: win_copy action plugin wants to get the state of remote files to check whether it needs to send them
 #   remote: all copy action is happening remotely (remote_src=True)
 #   single: a single file has been copied, also used with template
-$mode = Get-AnsibleParam -obj $params -name "mode" -type "str" -default "single" -validateset "explode","query","remote","single"
+$copy_mode = Get-AnsibleParam -obj $params -name "_copy_mode" -type "str" -default "single" -validateset "explode","query","remote","single"
 
 # used in explode, remote and single mode
-$src = Get-AnsibleParam -obj $params -name "src" -type "path" -failifempty ($mode -in @("explode","process","single"))
+$src = Get-AnsibleParam -obj $params -name "src" -type "path" -failifempty ($copy_mode -in @("explode","process","single"))
 $dest = Get-AnsibleParam -obj $params -name "dest" -type "path" -failifempty $true
 
 # used in single mode
@@ -224,7 +224,7 @@ Function Extract-ZipLegacy($src, $dest) {
     }
 }
 
-if ($mode -eq "query") {
+if ($copy_mode -eq "query") {
     # we only return a list of files/directories that need to be copied over
     # the source of the local file will be the key used
     $changed_files = @()
@@ -271,7 +271,7 @@ if ($mode -eq "query") {
     $result.files = $changed_files
     $result.directories = $changed_directories
     $result.symlinks = $changed_symlinks
-} elseif ($mode -eq "explode") {
+} elseif ($copy_mode -eq "explode") {
     # a single zip file containing the files and directories needs to be
     # expanded this will always result in a change as the calculation is done
     # on the win_copy action plugin and is only run if a change needs to occur
@@ -294,7 +294,7 @@ if ($mode -eq "query") {
     }
 
     $result.changed = $true
-} elseif ($mode -eq "remote") {
+} elseif ($copy_mode -eq "remote") {
     # all copy actions are happening on the remote side (windows host), need
     # too copy source and dest using PS code
     $result.src = $src
@@ -359,7 +359,7 @@ if ($mode -eq "query") {
     if ($diff_mode) {
         $result.diff.prepared = $diff
     }
-} elseif ($mode -eq "single") {
+} elseif ($copy_mode -eq "single") {
     # a single file is located in src and we need to copy to dest, this will
     # always result in a change as the calculation is done on the Ansible side
     # before this is run. This should also never run in check mode

lib/ansible/plugins/action/template.py

@@ -19,6 +19,7 @@ __metaclass__ = type
 
 import os
 import shutil
+import stat
 import tempfile
 
 from ansible import constants as C
@@ -105,6 +106,10 @@ class ActionModule(ActionBase):
                 except AnsibleError as e:
                     raise AnsibleActionFail(to_text(e))
 
+            mode = self._task.args.get('mode', None)
+            if mode == 'preserve':
+                mode = '0%03o' % stat.S_IMODE(os.stat(source).st_mode)
+
             # Get vault decrypted tmp file
             try:
                 tmp_source = self._loader.get_real_file(source)
@@ -157,6 +162,9 @@ class ActionModule(ActionBase):
                 self._loader.cleanup_tmp_file(tmp_source)
 
             new_task = self._task.copy()
+            # mode is either the mode from task.args or the mode of the source file if the task.args
+            # mode == 'preserve'
+            new_task.args['mode'] = mode
             new_task.args.pop('newline_sequence', None)
             new_task.args.pop('block_start_string', None)
             new_task.args.pop('block_end_string', None)

lib/ansible/plugins/action/win_copy.py

@@ -275,7 +275,7 @@ class ActionModule(ActionBase):
                 dest=dest,
                 src=tmp_src,
                 original_basename=source_rel,
-                mode="single"
+                _copy_mode="single"
             )
         )
         copy_args.pop('content', None)
@@ -317,7 +317,7 @@ class ActionModule(ActionBase):
             dict(
                 src=tmp_src,
                 dest=dest,
-                mode="explode"
+                _copy_mode="explode"
             )
         )
         copy_args.pop('content', None)
@@ -380,7 +380,7 @@ class ActionModule(ActionBase):
             new_module_args = self._task.args.copy()
             new_module_args.update(
                 dict(
-                    mode="remote",
+                    _copy_mode="remote",
                     dest=dest,
                     src=source,
                     force=force
@@ -467,7 +467,7 @@ class ActionModule(ActionBase):
         query_args = self._task.args.copy()
         query_args.update(
             dict(
-                mode="query",
+                _copy_mode="query",
                 dest=check_dest,
                 force=force,
                 files=source_files['files'],

lib/ansible/utils/module_docs_fragments/files.py

@@ -19,6 +19,9 @@
 class ModuleDocFragment(object):
 
     # Standard files documentation fragment
+
+    # Note: mode is overridden by the copy and template modules so if you change the description
+    # here, you should also change it there.
     DOCUMENTATION = """
 options:
   mode:

test/integration/targets/copy/tasks/tests.yml

@@ -406,6 +406,27 @@
        - "stat_results.stat.checksum == ('foo.txt\n'|hash('sha1'))"
        - "stat_results.stat.mode == '0547'"
 
+- name: Test copy with mode=preserve and remote_src=True
+  copy:
+    src: '{{ remote_dir }}/copy-foo.txt'
+    dest: '{{ remote_dir }}/copy-foo2.txt'
+    mode: 'preserve'
+    remote_src: True
+  register: copy_results2
+
+- name: Check the stat results of the file
+  stat:
+    path: '{{ remote_dir }}/copy-foo2.txt'
+  register: stat_results2
+
+- name: Assert that the file has changed and has correct mode
+  assert:
+     that:
+       - "copy_results2 is changed"
+       - "copy_results2.mode == '0547'"
+       - "stat_results2.stat.checksum == ('foo.txt\n'|hash('sha1'))"
+       - "stat_results2.stat.mode == '0547'"
+
 #
 # test recursive copy local_follow=False, no trailing slash
 #

test/integration/targets/template/tasks/main.yml

@@ -574,5 +574,32 @@
         - 'diff_result.stdout == ""'
         - "diff_result.rc == 0"
 
+# Check that mode=preserve works with template
+- name: Create a template which has strange permissions
+  copy:
+    content: !unsafe '{{ ansible_managed }}\n'
+    dest: '{{ output_dir }}/foo-template.j2'
+    mode: 0547
+  delegate_to: localhost
+
+- name: Use template with mode=preserve
+  template:
+    src: '{{ output_dir }}/foo-template.j2'
+    dest: '{{ output_dir }}/foo-templated.txt'
+    mode: 'preserve'
+  register: template_results
+
+- name: Get permissions from the templated file
+  stat:
+    path: '{{ output_dir }}/foo-templated.txt'
+  register: stat_results
+
+- name: Check that the resulting file has the correct permissions
+  assert:
+    that:
+      - 'template_results is changed'
+      - 'template_results.mode == "0547"'
+      - 'stat_results.stat["mode"] == "0547"'
+
 # aliases file requires root for template tests so this should be safe
 - include: backup_test.yml