From b376e208c7d9e2fddd19ed964628c5d99bb71150 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Tue, 16 Sep 2014 12:03:40 -0500 Subject: [PATCH] Adding "follow" param for file/copy options Also modifies the template action plugin to use this new param when executing the file/copy modules for templating so that links are preserved correctly. Fixes #8998 --- lib/ansible/module_utils/basic.py | 6 ++++ lib/ansible/runner/action_plugins/template.py | 22 ++++++++------ .../utils/module_docs_fragments/files.py | 8 +++++ library/files/copy | 3 ++ library/files/file | 14 ++++++--- .../roles/test_copy/tasks/main.yml | 30 +++++++++++++++++++ .../roles/test_file/tasks/main.yml | 26 ++++++++++++++++ 7 files changed, 96 insertions(+), 13 deletions(-) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 17e2773e5b6..655464d40fd 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -151,6 +151,7 @@ FILE_COMMON_ARGUMENTS=dict( serole = dict(), selevel = dict(), setype = dict(), + follow = dict(type='bool', default=False), # not taken by the file module, but other modules call file so it must ignore them. content = dict(no_log=True), backup = dict(), @@ -295,6 +296,11 @@ class AnsibleModule(object): else: path = os.path.expanduser(path) + # if the path is a symlink, and we're following links, get + # the target of the link instead for testing + if params.get('follow', False) and os.path.islink(path): + path = os.path.realpath(path) + mode = params.get('mode', None) owner = params.get('owner', None) group = params.get('group', None) diff --git a/lib/ansible/runner/action_plugins/template.py b/lib/ansible/runner/action_plugins/template.py index 9bfd66d3ff6..11e37b4815a 100644 --- a/lib/ansible/runner/action_plugins/template.py +++ b/lib/ansible/runner/action_plugins/template.py @@ -33,9 +33,6 @@ class ActionModule(object): def run(self, conn, tmp, module_name, module_args, inject, complex_args=None, **kwargs): ''' handler for template operations ''' - # note: since this module just calls the copy module, the --check mode support - # can be implemented entirely over there - if not self.runner.is_playbook: raise errors.AnsibleError("in current versions of ansible, templates are only usable in playbooks") @@ -121,6 +118,7 @@ class ActionModule(object): src=xfered, dest=dest, original_basename=os.path.basename(source), + follow=True, ) module_args_tmp = utils.merge_module_args(module_args, new_module_args) @@ -132,12 +130,18 @@ class ActionModule(object): res.diff = dict(before=dest_contents, after=resultant) return res else: - # if we're running in check mode, we still want the file module - # to execute, since we can't know if anything would be changed here, - # so we inject the check mode param into the module args and rely on - # the file module to report its changed status + # when running the file module based on the template data, we do + # not want the source filename (the name of the template) to be used, + # since this would mess up links, so we clear the src param and tell + # the module to follow links + new_module_args = dict( + src=None, + follow=True, + ) + # be sure to inject the check mode param into the module args and + # rely on the file module to report its changed status if self.runner.noop_on_check(inject): - new_module_args = dict(CHECKMODE=True) - module_args = utils.merge_module_args(module_args, new_module_args) + new_module_args['CHECKMODE'] = True + module_args = utils.merge_module_args(module_args, new_module_args) return self.runner._execute_module(conn, tmp, 'file', module_args, inject=inject, complex_args=complex_args) diff --git a/lib/ansible/utils/module_docs_fragments/files.py b/lib/ansible/utils/module_docs_fragments/files.py index d54d0ece5ab..adff1f2f1bf 100644 --- a/lib/ansible/utils/module_docs_fragments/files.py +++ b/lib/ansible/utils/module_docs_fragments/files.py @@ -67,4 +67,12 @@ options: - level part of the SELinux file context. This is the MLS/MCS attribute, sometimes known as the C(range). C(_default) feature works as for I(seuser). + follow: + required: false + default: "no" + choices: [ "yes", "no" ] + version_added: "1.8" + description: + - 'This flag indicates that filesystem links, if they exist, should be followed.' + """ diff --git a/library/files/copy b/library/files/copy index 5aef3d72221..eff46dae982 100644 --- a/library/files/copy +++ b/library/files/copy @@ -160,6 +160,7 @@ def main(): force = module.params['force'] original_basename = module.params.get('original_basename',None) validate = module.params.get('validate',None) + follow = module.params['follow'] if not os.path.exists(src): module.fail_json(msg="Source %s failed to transfer" % (src)) @@ -187,6 +188,8 @@ def main(): adjust_recursive_directory_permissions(pre_existing_dir, new_directory_list, module, directory_args, changed) if os.path.exists(dest): + if os.path.islink(dest) and follow: + dest = os.path.realpath(dest) if not force: module.exit_json(msg="file already exists", src=src, dest=dest, changed=False) if (os.path.isdir(dest)): diff --git a/library/files/file b/library/files/file index 8bfd94dd98a..e25278fded7 100644 --- a/library/files/file +++ b/library/files/file @@ -125,6 +125,7 @@ def main(): force = params['force'] diff_peek = params['diff_peek'] src = params['src'] + follow = params['follow'] # modify source as we later reload and pass, specially relevant when used by other modules. params['path'] = path = os.path.expanduser(params['path']) @@ -177,15 +178,20 @@ def main(): params['path'] = path = os.path.join(path, basename) else: if state in ['link','hard']: - module.fail_json(msg='src and dest are required for creating links') - - file_args = module.load_file_common_arguments(params) - changed = False + if follow: + # use the current target of the link as the source + src = os.readlink(path) + else: + module.fail_json(msg='src and dest are required for creating links') + # make sure the target path is a directory when we're doing a recursive operation recurse = params['recurse'] if recurse and state != 'directory': module.fail_json(path=path, msg="recurse option requires state to be 'directory'") + file_args = module.load_file_common_arguments(params) + changed = False + if state == 'absent': if state != prev_state: if not module.check_mode: diff --git a/test/integration/roles/test_copy/tasks/main.yml b/test/integration/roles/test_copy/tasks/main.yml index 8c4892bea80..47ed5166578 100644 --- a/test/integration/roles/test_copy/tasks/main.yml +++ b/test/integration/roles/test_copy/tasks/main.yml @@ -207,3 +207,33 @@ - name: clean up file: dest=/tmp/worldwritable state=absent +# test overwritting a link using "follow=yes" so that the link +# is preserved and the link target is updated + +- name: create a test file to symlink to + copy: dest={{output_dir}}/follow_test content="this is the follow test file\n" + +- name: create a symlink to the test file + file: path={{output_dir}}/follow_link src='./follow_test' state=link + +- name: update the test file using follow=True to preserve the link + copy: dest={{output_dir}}/follow_link content="this is the new content\n" follow=yes + register: replace_follow_result + +- name: stat the link path + stat: path={{output_dir}}/follow_link + register: stat_link_result + +- name: assert that the link is still a link + assert: + that: + - stat_link_result.stat.islnk + +- name: get the md5 of the link target + shell: md5sum {{output_dir}}/follow_test | cut -f1 -sd ' ' + register: target_file_result + +- name: assert that the link target was updated + assert: + that: + - replace_follow_result.md5sum == target_file_result.stdout diff --git a/test/integration/roles/test_file/tasks/main.yml b/test/integration/roles/test_file/tasks/main.yml index 775c173f34f..3c3bc81ea1e 100644 --- a/test/integration/roles/test_file/tasks/main.yml +++ b/test/integration/roles/test_file/tasks/main.yml @@ -405,3 +405,29 @@ that: - result.mode == '0444' +# test the file module using follow=yes, so that the target of a +# symlink is modified, rather than the link itself + +- name: create a test file + copy: dest={{output_dir}}/test_follow content="this is a test file\n" mode=0666 + +- name: create a symlink to the test file + file: path={{output_dir}}/test_follow_link src="./test_follow" state=link + +- name: modify the permissions on the link using follow=yes + file: path={{output_dir}}/test_follow_link mode=0644 follow=yes + register: result + +- name: assert that the chmod worked + assert: + that: + - result.changed + +- name: stat the link target + stat: path={{output_dir}}/test_follow + register: result + +- name: assert that the link target was modified correctly + assert: + that: + - result.stat.mode == '0644'