From 190ce16babb29b39af233c26b8408250b1fe5d34 Mon Sep 17 00:00:00 2001 From: David Simon Date: Thu, 9 May 2013 15:35:05 -0400 Subject: [PATCH] Dealing with invalid symlinks and symlink permissions --- lib/ansible/module_common.py | 22 +++++++++++++++------- test/TestRunner.py | 8 ++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/lib/ansible/module_common.py b/lib/ansible/module_common.py index c97521a4bc7..31cf8df0be3 100644 --- a/lib/ansible/module_common.py +++ b/lib/ansible/module_common.py @@ -316,7 +316,7 @@ class AnsibleModule(object): def user_and_group(self, filename): filename = os.path.expanduser(filename) - st = os.stat(filename) + st = os.lstat(filename) uid = st.st_uid gid = st.st_gid return (uid, gid) @@ -370,7 +370,7 @@ class AnsibleModule(object): return True if orig_uid != uid: try: - os.chown(path, uid, -1) + os.lchown(path, uid, -1) except OSError: self.fail_json(path=path, msg='chown failed') changed = True @@ -392,7 +392,7 @@ class AnsibleModule(object): return True if orig_gid != gid: try: - os.chown(path, -1, gid) + os.lchown(path, -1, gid) except OSError: self.fail_json(path=path, msg='chgrp failed') changed = True @@ -409,7 +409,7 @@ class AnsibleModule(object): except Exception, e: self.fail_json(path=path, msg='mode needs to be something octalish', details=str(e)) - st = os.stat(path) + st = os.lstat(path) prev_mode = stat.S_IMODE(st[stat.ST_MODE]) if prev_mode != mode: @@ -418,11 +418,19 @@ class AnsibleModule(object): # FIXME: comparison against string above will cause this to be executed # every time try: - os.chmod(path, mode) + if 'lchmod' in dir(os): + os.lchmod(path, mode) + else: + os.chmod(path, mode) + except OSError, e: + if e.errno == errno.ENOENT: # Can't set mode on broken symbolic links + pass + else: + raise e except Exception, e: self.fail_json(path=path, msg='chmod failed', details=str(e)) - st = os.stat(path) + st = os.lstat(path) new_mode = stat.S_IMODE(st[stat.ST_MODE]) if new_mode != prev_mode: @@ -483,7 +491,7 @@ class AnsibleModule(object): group = str(gid) kwargs['owner'] = user kwargs['group'] = group - st = os.stat(path) + st = os.lstat(path) kwargs['mode'] = oct(stat.S_IMODE(st[stat.ST_MODE])) # secontext not yet supported if os.path.islink(path): diff --git a/test/TestRunner.py b/test/TestRunner.py index 63f89dc3f3c..cf17e669986 100644 --- a/test/TestRunner.py +++ b/test/TestRunner.py @@ -230,6 +230,14 @@ class TestRunner(unittest.TestCase): assert self._run('file', ['dest=' + filedemo, 'state=absent'])['changed'] assert not os.path.exists(filedemo) assert not self._run('file', ['dest=' + filedemo, 'state=absent'])['changed'] + + # Make sure that we can deal safely with bad symlinks + os.symlink('/tmp/non_existent_target', filedemo) + assert self._run('file', ['dest=' + tmp_dir, 'state=directory recurse=yes mode=701'])['changed'] + assert not self._run('file', ['dest=' + tmp_dir, 'state=directory', 'recurse=yes', 'owner=' + str(os.getuid())])['changed'] + assert os.path.islink(filedemo) + assert self._run('file', ['dest=' + filedemo, 'state=absent'])['changed'] + assert not os.path.exists(filedemo) os.rmdir(tmp_dir) def test_large_output(self):