From 769881198f3c81db9546be0c4b4bbcdc74de5c4b Mon Sep 17 00:00:00 2001 From: Strahinja Kustudic Date: Sat, 4 Nov 2017 01:13:58 +0100 Subject: [PATCH] Sets sane defaults for follow in file modules (#31430) * Sets sane defaults for follow in file modules * Add a note in replace for removal of `follow` option --- lib/ansible/modules/files/blockinfile.py | 13 ++++--------- lib/ansible/modules/files/file.py | 4 +++- lib/ansible/modules/files/replace.py | 11 +++-------- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/lib/ansible/modules/files/blockinfile.py b/lib/ansible/modules/files/blockinfile.py index b3ef4e149d8..9683c95a51e 100644 --- a/lib/ansible/modules/files/blockinfile.py +++ b/lib/ansible/modules/files/blockinfile.py @@ -77,16 +77,11 @@ options: get the original file back if you somehow clobbered it incorrectly. type: bool default: 'no' - follow: - description: - - 'This flag indicates that filesystem links, if they exist, should be followed.' - type: bool - default: 'no' - version_added: "2.1" notes: - This module supports check mode. - When using 'with_*' loops be aware that if you do not set a unique mark the block will be overwritten on each iteration. - As of Ansible 2.3, the I(dest) option has been changed to I(path) as default, but I(dest) still works as well. + - Option I(follow) has been removed in version 2.5, because this module modifies the contents of the file so I(follow=no) doesn't make sense. """ EXAMPLES = r""" @@ -203,8 +198,6 @@ def main(): params = module.params path = params['path'] - if module.boolean(params.get('follow', None)): - path = os.path.realpath(path) if os.path.isdir(path): module.fail_json(rc=256, @@ -315,7 +308,9 @@ def main(): if changed and not module.check_mode: if module.boolean(params['backup']) and path_exists: module.backup_local(path) - write_changes(module, result, path) + # We should always follow symlinks so that we change the real file + real_path = os.path.realpath(params['path']) + write_changes(module, result, real_path) if module.check_mode and not path_exists: module.exit_json(changed=changed, msg=msg, diff=diff) diff --git a/lib/ansible/modules/files/file.py b/lib/ansible/modules/files/file.py index f048572d5c2..06831a62b11 100644 --- a/lib/ansible/modules/files/file.py +++ b/lib/ansible/modules/files/file.py @@ -71,8 +71,9 @@ options: follow: description: - 'This flag indicates that filesystem links, if they exist, should be followed.' + - 'Previous to Ansible 2.5, this was C(no) by default.' type: bool - default: "no" + default: 'yes' version_added: "1.8" ''' @@ -176,6 +177,7 @@ def main(): original_basename=dict(required=False), # Internal use only, for recursive ops recurse=dict(default=False, type='bool'), force=dict(required=False, default=False, type='bool'), + follow=dict(required=False, default=True, type='bool'), diff_peek=dict(default=None), # Internal use only, for internal checks in the action plugins validate=dict(required=False, default=None), # Internal use only, for template and copy src=dict(required=False, default=None, type='path'), diff --git a/lib/ansible/modules/files/replace.py b/lib/ansible/modules/files/replace.py index f2bf6da84ed..1a652b6b45e 100644 --- a/lib/ansible/modules/files/replace.py +++ b/lib/ansible/modules/files/replace.py @@ -78,12 +78,6 @@ options: others: description: - All arguments accepted by the M(file) module also work here. - follow: - description: - - 'This flag indicates that filesystem links, if they exist, should be followed.' - type: bool - default: "no" - version_added: "1.9" encoding: description: - "The character encoding for reading and writing the file." @@ -91,6 +85,7 @@ options: version_added: "2.4" notes: - As of Ansible 2.3, the I(dest) option has been changed to I(path) as default, but I(dest) still works as well. + - Option I(follow) has been removed in version 2.5, because this module modifies the contents of the file so I(follow=no) doesn't make sense. """ EXAMPLES = r""" @@ -269,8 +264,8 @@ def main(): if changed and not module.check_mode: if params['backup'] and os.path.exists(path): res_args['backup_file'] = module.backup_local(path) - if params['follow'] and os.path.islink(path): - path = os.path.realpath(path) + # We should always follow symlinks so that we change the real file + path = os.path.realpath(path) write_changes(module, to_bytes(result[0], encoding=encoding), path) res_args['msg'], res_args['changed'] = check_file_attrs(module, changed, msg)