From 94ba31279869d7052ba001e38927f9eecd0a636f Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Tue, 10 Dec 2013 20:58:55 -0500 Subject: Augeas improvements: * Added ability to specify initial content for a file that doesn't exist, to avoid a messy situation where you'd have to probe for file existence and either use a Path type="file" or Path type="augeas" depending, and run Bcfg2 twice. * All commands in an Augeas path are run if *any* of them fail to verify. Previously, only commands that hadn't been run would be installed, but that had issues, particularly with the Clear command, which could pass verification but then be required during the installation phase anyway. * Miscellaneous bug fixes. --- .../TestClient/TestTools/TestPOSIX/TestAugeas.py | 49 ++++++++++------------ 1 file changed, 23 insertions(+), 26 deletions(-) (limited to 'testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX') diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestAugeas.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestAugeas.py index 9b25499fe..b8534f5a8 100644 --- a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestAugeas.py +++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestAugeas.py @@ -74,7 +74,7 @@ if can_skip or HAS_AUGEAS: def tearDown(self): tmpfile = getattr(self, "tmpfile", None) - if tmpfile: + if tmpfile and os.path.exists(tmpfile): os.unlink(tmpfile) def test_fully_specified(self): @@ -83,7 +83,7 @@ if can_skip or HAS_AUGEAS: entry = lxml.etree.Element("Path", name="/test", type="augeas") self.assertFalse(ptool.fully_specified(entry)) - entry.text = "text" + lxml.etree.SubElement(entry, "Set", path="/test", value="test") self.assertTrue(ptool.fully_specified(entry)) def test_install(self): @@ -138,12 +138,14 @@ if can_skip or HAS_AUGEAS: self._verify(self.applied_commands.values()) @patch("Bcfg2.Client.Tools.POSIX.Augeas.POSIXTool.install") - def _install(self, commands, expected, mock_install): + def _install(self, commands, expected, mock_install, **attrs): ptool = self.get_obj() mock_install.return_value = True entry = lxml.etree.Element("Path", name=self.tmpfile, type="augeas", lens="Xml") + for key, val in attrs.items(): + entry.set(key, val) entry.extend(commands) self.assertTrue(ptool.install(entry)) @@ -156,8 +158,7 @@ if can_skip or HAS_AUGEAS: expected = copy.deepcopy(test_xdata) expected.find("Text").text = "Changed content" self._install([lxml.etree.Element("Set", path="Test/Text/#text", - value="Changed content", - verified="false")], + value="Changed content")], expected) def test_install_set_new(self): @@ -166,30 +167,16 @@ if can_skip or HAS_AUGEAS: newtext = lxml.etree.SubElement(expected, "NewText") newtext.text = "new content" self._install([lxml.etree.Element("Set", path="Test/NewText/#text", - value="new content", - verified="false")], + value="new content")], expected) - def test_install_only_verified(self): - """ Test that only unverified commands are installed """ - expected = copy.deepcopy(test_xdata) - newtext = lxml.etree.SubElement(expected, "NewText") - newtext.text = "new content" - self._install( - [lxml.etree.Element("Set", path="Test/NewText/#text", - value="new content", verified="false"), - lxml.etree.Element("Set", path="Test/Bogus/#text", - value="bogus", verified="true")], - expected) - def test_install_remove(self): """ Test removing a node """ expected = copy.deepcopy(test_xdata) expected.remove(expected.find("Attrs")) self._install( [lxml.etree.Element("Remove", - path='Test/*[#attribute/foo = "foo"]', - verified="false")], + path='Test/*[#attribute/foo = "foo"]')], expected) def test_install_move(self): @@ -199,8 +186,7 @@ if can_skip or HAS_AUGEAS: expected.append(foo) self._install( [lxml.etree.Element("Move", source='Test/Children/Foo', - destination='Test/Foo', - verified="false")], + destination='Test/Foo')], expected) def test_install_clear(self): @@ -228,7 +214,7 @@ if can_skip or HAS_AUGEAS: [lxml.etree.Element( "SetMulti", value="same", base='Test/Children[#attribute/identical = "true"]', - sub="Thing/#text", verified="false")], + sub="Thing/#text")], expected) def test_install_insert(self): @@ -242,9 +228,20 @@ if can_skip or HAS_AUGEAS: [lxml.etree.Element( "Insert", path='Test/Children[#attribute/identical = "true"]/Thing[2]', - label="Thing", where="after", verified="false"), + label="Thing", where="after"), lxml.etree.Element( "Set", path='Test/Children[#attribute/identical = "true"]/Thing[3]/#text', - value="three", verified="false")], + value="three")], expected) + + def test_install_initial(self): + """ Test creating initial content and then modifying it """ + os.unlink(self.tmpfile) + expected = copy.deepcopy(test_xdata) + expected.find("Text").text = "Changed content" + initial = lxml.etree.Element("Initial") + initial.text = test_data + modify = lxml.etree.Element("Set", path="Test/Text/#text", + value="Changed content") + self._install([initial, modify], expected, current_exists="false") -- cgit v1.2.3-1-g7c22 From cae2fcc0135c26811b1ce353ea28e4a93900c138 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Mon, 10 Feb 2014 09:02:16 -0500 Subject: POSIX: Fix verification of symlinks * Stat the link itself, not its target * Get SELinux context from the link, not the target * Don't get ACLs at all; symlinks don't have their own ACLs The first issue listed wasn't actually a bug, because none of the information queried from the target by the stat call was actually used in verification, but it's been fixed for completeness. --- .../TestClient/TestTools/TestPOSIX/Testbase.py | 50 +++++++++++++--------- 1 file changed, 30 insertions(+), 20 deletions(-) (limited to 'testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX') diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py index d2f383f42..ab6e2fe54 100644 --- a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py +++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py @@ -563,16 +563,16 @@ class TestPOSIXTool(TestTool): self.assertEqual(6, ptool._norm_acl_perms("rwa")) self.assertEqual(4, ptool._norm_acl_perms("rr")) - @patch('os.stat') - def test__gather_data(self, mock_stat): + @patch('os.lstat') + def test__gather_data(self, mock_lstat): ptool = self.get_obj() path = '/test' - mock_stat.side_effect = OSError + mock_lstat.side_effect = OSError self.assertFalse(ptool._gather_data(path)[0]) - mock_stat.assert_called_with(path) + mock_lstat.assert_called_with(path) - mock_stat.reset_mock() - mock_stat.side_effect = None + mock_lstat.reset_mock() + mock_lstat.side_effect = None # create a return value stat_rv = MagicMock() def stat_getitem(key): @@ -585,7 +585,7 @@ class TestPOSIXTool(TestTool): # and ensure that they're stripped return int('060660', 8) stat_rv.__getitem__ = Mock(side_effect=stat_getitem) - mock_stat.return_value = stat_rv + mock_lstat.return_value = stat_rv # disable selinux and acls for this call -- we test them # separately so that we can skip those tests as appropriate @@ -597,7 +597,7 @@ class TestPOSIXTool(TestTool): (stat_rv, '0', '10', '0660', None, None)) Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX, \ Bcfg2.Client.Tools.POSIX.base.HAS_ACLS = states - mock_stat.assert_called_with(path) + mock_lstat.assert_called_with(path) @skipUnless(HAS_SELINUX, "SELinux not found, skipping") def test__gather_data_selinux(self): @@ -605,12 +605,12 @@ class TestPOSIXTool(TestTool): context = 'system_u:object_r:root_t:s0' path = '/test' - @patch('os.stat') - @patchIf(HAS_SELINUX, "selinux.getfilecon") - def inner(mock_getfilecon, mock_stat): - mock_getfilecon.return_value = [len(context) + 1, context] - mock_stat.return_value = MagicMock() - mock_stat.return_value.__getitem__.return_value = MagicMock() + @patch('os.lstat') + @patchIf(HAS_SELINUX, "selinux.lgetfilecon") + def inner(mock_lgetfilecon, mock_lstat): + mock_lgetfilecon.return_value = [len(context) + 1, context] + mock_lstat.return_value = MagicMock() + mock_lstat.return_value.__getitem__.return_value = MagicMock() # disable acls for this call and test them separately state = (Bcfg2.Client.Tools.POSIX.base.HAS_ACLS, Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX) @@ -619,30 +619,40 @@ class TestPOSIXTool(TestTool): self.assertEqual(ptool._gather_data(path)[4], 'root_t') Bcfg2.Client.Tools.POSIX.base.HAS_ACLS, \ Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = state - mock_getfilecon.assert_called_with(path) + mock_lgetfilecon.assert_called_with(path) inner() @skipUnless(HAS_ACLS, "ACLS not found, skipping") - @patch('os.stat') - def test__gather_data_acls(self, mock_stat): + @patch('os.lstat') + @patch('stat.S_ISLNK') + def test__gather_data_acls(self, mock_S_ISLNK, mock_lstat): ptool = self.get_obj() ptool._list_file_acls = Mock() acls = {("default", posix1e.ACL_USER, "testuser"): "rwx", ("access", posix1e.ACL_GROUP, "testgroup"): "rx"} ptool._list_file_acls.return_value = acls path = '/test' - mock_stat.return_value = MagicMock() - mock_stat.return_value.__getitem__.return_value = MagicMock() + mock_lstat.return_value = MagicMock() + mock_lstat.return_value.__getitem__.return_value = MagicMock() + mock_S_ISLNK.return_value = False # disable selinux for this call and test it separately state = (Bcfg2.Client.Tools.POSIX.base.HAS_ACLS, Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX) Bcfg2.Client.Tools.POSIX.base.HAS_ACLS = True Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = False self.assertItemsEqual(ptool._gather_data(path)[5], acls) + ptool._list_file_acls.assert_called_with(path) + + # symlinks can't have their own ACLs, so ensure that the + # _list_file_acls call is skipped and no ACLs are returned + mock_S_ISLNK.return_value = True + ptool._list_file_acls.reset_mock() + self.assertEqual(ptool._gather_data(path)[5], None) + self.assertFalse(ptool._list_file_acls.called) + Bcfg2.Client.Tools.POSIX.base.HAS_ACLS, \ Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = state - ptool._list_file_acls.assert_called_with(path) @patchIf(HAS_SELINUX, "selinux.matchpathcon") def test_verify_metadata(self, mock_matchpathcon): -- cgit v1.2.3-1-g7c22