diff --git a/changelogs/fragments/mode-preserve.yaml b/changelogs/fragments/mode-preserve.yaml new file mode 100644 index 00000000000..1fe98cb555e --- /dev/null +++ b/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 diff --git a/lib/ansible/modules/files/copy.py b/lib/ansible/modules/files/copy.py index 7ea1f3c5b8a..b36eb854eb9 100644 --- a/lib/ansible/modules/files/copy.py +++ b/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 diff --git a/lib/ansible/modules/files/template.py b/lib/ansible/modules/files/template.py index 01e812c7a69..28085acb466 100644 --- a/lib/ansible/modules/files/template.py +++ b/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 diff --git a/lib/ansible/modules/windows/win_copy.ps1 b/lib/ansible/modules/windows/win_copy.ps1 index 333bce8e875..a4e4e38c828 100644 --- a/lib/ansible/modules/windows/win_copy.ps1 +++ b/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 diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index b17749a1133..e3bf07f7390 100644 --- a/lib/ansible/plugins/action/template.py +++ b/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) diff --git a/lib/ansible/plugins/action/win_copy.py b/lib/ansible/plugins/action/win_copy.py index 0566b95940c..da709739785 100644 --- a/lib/ansible/plugins/action/win_copy.py +++ b/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'], diff --git a/lib/ansible/utils/module_docs_fragments/files.py b/lib/ansible/utils/module_docs_fragments/files.py index a50dfbce16a..84946f2a49f 100644 --- a/lib/ansible/utils/module_docs_fragments/files.py +++ b/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: diff --git a/test/integration/targets/copy/tasks/tests.yml b/test/integration/targets/copy/tasks/tests.yml index 699561c8096..5f85531b0ce 100644 --- a/test/integration/targets/copy/tasks/tests.yml +++ b/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 # diff --git a/test/integration/targets/template/tasks/main.yml b/test/integration/targets/template/tasks/main.yml index fd0b3c150ba..ee04f0f3c27 100644 --- a/test/integration/targets/template/tasks/main.yml +++ b/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