From a6a29aa01744cc893741ddf558f415b7c705d3f6 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Thu, 15 Nov 2012 15:25:21 -0500 Subject: POSIX: fixed removal of symlinked directories --- src/lib/Bcfg2/Client/Tools/POSIX/Directory.py | 16 +--- src/lib/Bcfg2/Client/Tools/POSIX/Nonexistent.py | 23 +++--- src/lib/Bcfg2/Client/Tools/POSIX/base.py | 18 ++-- .../TestTools/TestPOSIX/TestDirectory.py | 36 ++++---- .../TestTools/TestPOSIX/TestNonexistent.py | 53 ++++-------- .../TestClient/TestTools/TestPOSIX/Testbase.py | 95 ++++++++++++++-------- 6 files changed, 120 insertions(+), 121 deletions(-) diff --git a/src/lib/Bcfg2/Client/Tools/POSIX/Directory.py b/src/lib/Bcfg2/Client/Tools/POSIX/Directory.py index 9b0b998bb..9d0fe05e0 100644 --- a/src/lib/Bcfg2/Client/Tools/POSIX/Directory.py +++ b/src/lib/Bcfg2/Client/Tools/POSIX/Directory.py @@ -3,7 +3,6 @@ import os import sys import stat -import shutil import Bcfg2.Client.XML from Bcfg2.Client.Tools.POSIX.base import POSIXTool @@ -67,25 +66,14 @@ class POSIXDirectory(POSIXTool): rv &= self._makedirs(entry) if entry.get('prune', 'false') == 'true': - ulfailed = False for pent in entry.findall('Prune'): pname = pent.get('path') - ulfailed = False - if os.path.isdir(pname): - remove = shutil.rmtree - else: - remove = os.unlink try: self.logger.debug("POSIX: Removing %s" % pname) - remove(pname) + self._remove(pent) except OSError: err = sys.exc_info()[1] self.logger.error("POSIX: Failed to unlink %s: %s" % (pname, err)) - ulfailed = True - if ulfailed: - # even if prune failed, we still want to install the - # entry to make sure that we get permissions and - # whatnot set - rv = False + rv = False return POSIXTool.install(self, entry) and rv diff --git a/src/lib/Bcfg2/Client/Tools/POSIX/Nonexistent.py b/src/lib/Bcfg2/Client/Tools/POSIX/Nonexistent.py index 5f1fbbe7c..0606d47f9 100644 --- a/src/lib/Bcfg2/Client/Tools/POSIX/Nonexistent.py +++ b/src/lib/Bcfg2/Client/Tools/POSIX/Nonexistent.py @@ -2,7 +2,6 @@ import os import sys -import shutil from Bcfg2.Client.Tools.POSIX.base import POSIXTool @@ -19,25 +18,25 @@ class POSIXNonexistent(POSIXTool): def install(self, entry): ename = entry.get('name') - if entry.get('recursive', '').lower() == 'true': + recursive = entry.get('recursive', '').lower() == 'true' + if recursive: + print "here" # ensure that configuration spec is consistent first for struct in self.config.getchildren(): - for entry in struct.getchildren(): - if (entry.tag == 'Path' and - entry.get('type') != 'nonexistent' and - entry.get('name').startswith(ename)): + print "checking struct" + for el in struct.getchildren(): + import lxml.etree + print "checking entry: %s" % lxml.etree.tostring(el) + if (el.tag == 'Path' and + el.get('type') != 'nonexistent' and + el.get('name').startswith(ename)): self.logger.error('POSIX: Not removing %s. One or ' 'more files in this directory are ' 'specified in your configuration.' % ename) return False - remove = shutil.rmtree - elif os.path.isdir(ename): - remove = os.rmdir - else: - remove = os.remove try: - remove(ename) + self._remove(entry, recursive=recursive) return True except OSError: err = sys.exc_info()[1] diff --git a/src/lib/Bcfg2/Client/Tools/POSIX/base.py b/src/lib/Bcfg2/Client/Tools/POSIX/base.py index a9566b698..6388f6731 100644 --- a/src/lib/Bcfg2/Client/Tools/POSIX/base.py +++ b/src/lib/Bcfg2/Client/Tools/POSIX/base.py @@ -66,18 +66,26 @@ class POSIXTool(Bcfg2.Client.Tools.Tool): rv &= self._set_perms(entry, path=os.path.join(root, path)) return rv + def _remove(self, entry, recursive=True): + """ Remove a Path entry, whatever that takes """ + if os.path.islink(entry.get('name')): + os.unlink(entry.get('name')) + elif os.path.isdir(entry.get('name')): + if recursive: + shutil.rmtree(entry.get('name')) + else: + os.rmdir(entry.get('name')) + else: + os.unlink(entry.get('name')) + def _exists(self, entry, remove=False): """ check for existing paths and optionally remove them. if the path exists, return the lstat of it """ try: ondisk = os.lstat(entry.get('name')) if remove: - if os.path.isdir(entry.get('name')): - remove = shutil.rmtree - else: - remove = os.unlink try: - remove(entry.get('name')) + self._remove(entry) return None except OSError: err = sys.exc_info()[1] diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestDirectory.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestDirectory.py index 6dd130bee..16490808e 100644 --- a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestDirectory.py +++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestDirectory.py @@ -83,34 +83,35 @@ class TestPOSIXDirectory(TestPOSIXTool): mock_verify.assert_called_with(self.ptool, entry, modlist) mock_listdir.assert_called_with(entry.get("name")) self.assertEqual(len(entry.findall("Prune")), 0) - + @patch("os.unlink") - @patch("os.path.isdir") - @patch("shutil.rmtree") @patch("Bcfg2.Client.Tools.POSIX.base.POSIXTool.install") @patch("Bcfg2.Client.Tools.POSIX.Directory.%s._exists" % test_obj.__name__) @patch("Bcfg2.Client.Tools.POSIX.Directory.%s._makedirs" % test_obj.__name__) def test_install(self, mock_makedirs, mock_exists, mock_install, - mock_rmtree, mock_isdir, mock_unlink): + mock_unlink): entry = lxml.etree.Element("Path", name="/test/foo/bar", type="directory", mode='0644', owner='root', group='root') - + + self.ptool._makedirs = Mock() + self.ptool._remove = Mock() + def reset(): mock_exists.reset_mock() mock_install.reset_mock() mock_unlink.reset_mock() - mock_rmtree.reset_mock() - mock_rmtree.mock_makedirs() + self.ptool._makedirs.reset_mock() + self.ptool._remove.reset_mock() - mock_makedirs.return_value = True + self.ptool._makedirs.return_value = True mock_exists.return_value = False mock_install.return_value = True self.assertTrue(self.ptool.install(entry)) mock_exists.assert_called_with(entry) mock_install.assert_called_with(self.ptool, entry) - mock_makedirs.assert_called_with(entry) + self.ptool._makedirs.assert_called_with(entry) reset() exists_rv = MagicMock() @@ -119,7 +120,7 @@ class TestPOSIXDirectory(TestPOSIXTool): self.assertTrue(self.ptool.install(entry)) mock_unlink.assert_called_with(entry.get("name")) mock_exists.assert_called_with(entry) - mock_makedirs.assert_called_with(entry) + self.ptool._makedirs.assert_called_with(entry) mock_install.assert_called_with(self.ptool, entry) reset() @@ -138,20 +139,13 @@ class TestPOSIXDirectory(TestPOSIXTool): prune = ["/test/foo/bar/prune1", "/test/foo/bar/prune2"] for path in prune: lxml.etree.SubElement(entry, "Prune", path=path) - + reset() mock_install.return_value = True - def isdir_rv(path): - if path.endswith("prune2"): - return True - else: - return False - mock_isdir.side_effect = isdir_rv self.assertTrue(self.ptool.install(entry)) mock_exists.assert_called_with(entry) mock_install.assert_called_with(self.ptool, entry) - self.assertItemsEqual(mock_isdir.call_args_list, - [call(p) for p in prune]) - mock_unlink.assert_called_with("/test/foo/bar/prune1") - mock_rmtree.assert_called_with("/test/foo/bar/prune2") + self.assertItemsEqual([c[0][0].get("path") + for c in self.ptool._remove.call_args_list], + prune) diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestNonexistent.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestNonexistent.py index 676b18f5d..43242afb7 100644 --- a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestNonexistent.py +++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestNonexistent.py @@ -18,6 +18,7 @@ from Test__init import get_config, get_posix_object from Testbase import TestPOSIXTool from common import * + class TestPOSIXNonexistent(TestPOSIXTool): test_obj = POSIXNonexistent @@ -31,59 +32,41 @@ class TestPOSIXNonexistent(TestPOSIXTool): self.assertEqual(self.ptool.verify(entry, []), not val) mock_lexists.assert_called_with(entry.get("name")) - @patch("os.rmdir") - @patch("os.remove") - @patch("os.path.isdir") - @patch("shutil.rmtree") - def test_install(self, mock_rmtree, mock_isdir, mock_remove, mock_rmdir): + def test_install(self): entry = lxml.etree.Element("Path", name="/test", type="nonexistent") - def reset(): - mock_isdir.reset_mock() - mock_remove.reset_mock() - mock_rmdir.reset_mock() - mock_rmtree.reset_mock() + self.ptool._remove = Mock() - mock_isdir.return_value = False - self.assertTrue(self.ptool.install(entry)) - mock_remove.assert_called_with(entry.get("name")) - - reset() - mock_remove.side_effect = OSError - self.assertFalse(self.ptool.install(entry)) - mock_remove.assert_called_with(entry.get("name")) + def reset(): + self.ptool._remove.reset_mock() - reset() - mock_isdir.return_value = True self.assertTrue(self.ptool.install(entry)) - mock_rmdir.assert_called_with(entry.get("name")) - - reset() - mock_rmdir.side_effect = OSError - self.assertFalse(self.ptool.install(entry)) - mock_rmdir.assert_called_with(entry.get("name")) + self.ptool._remove.assert_called_with(entry, recursive=False) reset() entry.set("recursive", "true") self.assertTrue(self.ptool.install(entry)) - mock_rmtree.assert_called_with(entry.get("name")) - - reset() - mock_rmtree.side_effect = OSError - self.assertFalse(self.ptool.install(entry)) - mock_rmtree.assert_called_with(entry.get("name")) + self.ptool._remove.assert_called_with(entry, recursive=True) + print "about to start" reset() child_entry = lxml.etree.Element("Path", name="/test/foo", type="nonexistent") ptool = self.get_obj(posix=get_posix_object(config=get_config([child_entry]))) - mock_rmtree.side_effect = None + ptool._remove = Mock() self.assertTrue(ptool.install(entry)) - mock_rmtree.assert_called_with(entry.get("name")) + ptool._remove.assert_called_with(entry, recursive=True) reset() child_entry = lxml.etree.Element("Path", name="/test/foo", type="file") ptool = self.get_obj(posix=get_posix_object(config=get_config([child_entry]))) - mock_rmtree.side_effect = None + ptool._remove = Mock() self.assertFalse(ptool.install(entry)) + self.assertFalse(ptool._remove.called) + + reset() + entry.set("recursive", "false") + self.ptool._remove.side_effect = OSError + self.assertFalse(self.ptool.install(entry)) + self.ptool._remove.assert_called_with(entry, recursive=False) diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py index ec194d401..b3599db83 100644 --- a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py +++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py @@ -123,57 +123,84 @@ class TestPOSIXTool(Bcfg2TestCase): mock_walk.assert_called_with(entry.get("name")) self.assertItemsEqual(mock_set_perms.call_args_list, all_set_perms) + @patch('os.rmdir') + @patch('os.unlink') + @patch('shutil.rmtree') + @patch('os.path.isdir') + @patch('os.path.islink') + def test_remove(self, mock_islink, mock_isdir, mock_rmtree, mock_unlink, + mock_rmdir): + entry = lxml.etree.Element("Path", name="/etc/foo") + + def reset(): + mock_islink.reset_mock() + mock_isdir.reset_mock() + mock_rmtree.reset_mock() + mock_unlink.reset_mock() + mock_rmdir.reset_mock() + + mock_islink.return_value = True + mock_isdir.return_value = False + self.ptool._remove(entry) + mock_unlink.assert_called_with(entry.get('name')) + self.assertFalse(mock_rmtree.called) + self.assertFalse(mock_rmdir.called) + + reset() + mock_islink.return_value = False + mock_isdir.return_value = True + self.ptool._remove(entry) + mock_rmtree.assert_called_with(entry.get('name')) + self.assertFalse(mock_unlink.called) + self.assertFalse(mock_rmdir.called) + + reset() + self.ptool._remove(entry, recursive=False) + mock_rmdir.assert_called_with(entry.get('name')) + self.assertFalse(mock_unlink.called) + self.assertFalse(mock_rmtree.called) + + reset() + mock_islink.return_value = False + mock_isdir.return_value = False + self.ptool._remove(entry, recursive=False) + mock_unlink.assert_called_with(entry.get('name')) + self.assertFalse(mock_rmtree.called) + self.assertFalse(mock_rmdir.called) + @patch('os.lstat') - @patch("os.unlink") - @patch("os.path.isdir") - @patch("shutil.rmtree") - def test_exists(self, mock_rmtree, mock_isdir, mock_unlink, mock_lstat): + def test_exists(self, mock_lstat): entry = lxml.etree.Element("Path", name="/etc/foo", type="file") + self.ptool._remove = Mock() + + def reset(): + mock_lstat.reset_mock() + self.ptool._remove.reset_mock() + mock_lstat.side_effect = OSError self.assertFalse(self.ptool._exists(entry)) mock_lstat.assert_called_with(entry.get('name')) - self.assertFalse(mock_unlink.called) + self.assertFalse(self.ptool._remove.called) - mock_lstat.reset_mock() - mock_unlink.reset_mock() + reset() rv = MagicMock() mock_lstat.return_value = rv mock_lstat.side_effect = None self.assertEqual(self.ptool._exists(entry), rv) mock_lstat.assert_called_with(entry.get('name')) - self.assertFalse(mock_unlink.called) - - mock_lstat.reset_mock() - mock_unlink.reset_mock() - mock_isdir.return_value = False - self.assertFalse(self.ptool._exists(entry, remove=True)) - mock_isdir.assert_called_with(entry.get('name')) - mock_lstat.assert_called_with(entry.get('name')) - mock_unlink.assert_called_with(entry.get('name')) - self.assertFalse(mock_rmtree.called) + self.assertFalse(self.ptool._remove.called) - mock_lstat.reset_mock() - mock_isdir.reset_mock() - mock_unlink.reset_mock() - mock_rmtree.reset_mock() - mock_isdir.return_value = True - self.assertFalse(self.ptool._exists(entry, remove=True)) - mock_isdir.assert_called_with(entry.get('name')) + reset() + self.assertEqual(self.ptool._exists(entry, remove=True), None) mock_lstat.assert_called_with(entry.get('name')) - mock_rmtree.assert_called_with(entry.get('name')) - self.assertFalse(mock_unlink.called) + self.ptool._remove.assert_called_with(entry) - mock_isdir.reset_mock() - mock_lstat.reset_mock() - mock_unlink.reset_mock() - mock_rmtree.reset_mock() - mock_rmtree.side_effect = OSError + reset() + self.ptool._remove.side_effect = OSError self.assertEqual(self.ptool._exists(entry, remove=True), rv) - mock_isdir.assert_called_with(entry.get('name')) mock_lstat.assert_called_with(entry.get('name')) - mock_rmtree.assert_called_with(entry.get('name')) - self.assertFalse(mock_unlink.called) + self.ptool._remove.assert_called_with(entry) @patch("os.chown") @patch("os.chmod") -- cgit v1.2.3-1-g7c22