Merge branch 'maint'

Conflicts:
	src/lib/Bcfg2/Server/Plugins/Cfg/CfgPublicKeyCreator.py
	testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py

6 files changed, 150 insertions, 68 deletions

diff --git a/src/lib/Bcfg2/Client/Tools/POSIX/base.py b/src/lib/Bcfg2/Client/Tools/POSIX/base.py
index bce7ba0ca..712620206 100644
--- a/src/lib/Bcfg2/Client/Tools/POSIX/base.py
+++ b/src/lib/Bcfg2/Client/Tools/POSIX/base.py
@@ -232,6 +232,11 @@ class POSIXTool(Bcfg2.Client.Tools.Tool):
         else:
             defacl = None
 
+        if not acls:
+            self.logger.debug("POSIX: Removed ACLs from %s" %
+                              entry.get("name"))
+            return True
+
         for aclkey, perms in acls.items():
             atype, scope, qualifier = aclkey
             if atype == "default":
diff --git a/src/lib/Bcfg2/Server/Plugin/helpers.py b/src/lib/Bcfg2/Server/Plugin/helpers.py
index 73ad24614..98cd250dd 100644
--- a/src/lib/Bcfg2/Server/Plugin/helpers.py
+++ b/src/lib/Bcfg2/Server/Plugin/helpers.py
@@ -547,16 +547,12 @@ class XMLFileBacked(FileBacked):
                 xdata = self.xdata.getroottree()
             else:
                 xdata = lxml.etree.parse(fname)
-        included = [el for el in xdata.findall('//' + xinclude)]
-        for el in included:
+        for el in xdata.findall('//' + xinclude):
             name = el.get("href")
             if name.startswith("/"):
                 fpath = name
             else:
-                if fname:
-                    rel = fname
-                else:
-                    rel = self.name
+                rel = fname or self.name
                 fpath = os.path.join(os.path.dirname(rel), name)
 
             # expand globs in xinclude, a bcfg2-specific extension
@@ -571,12 +567,13 @@ class XMLFileBacked(FileBacked):
             parent = el.getparent()
             parent.remove(el)
             for extra in extras:
-                if extra != self.name and extra not in self.extras:
-                    self.extras.append(extra)
+                if extra != self.name:
                     lxml.etree.SubElement(parent, xinclude, href=extra)
-                    self._follow_xincludes(fname=extra)
-                    if extra not in self.extra_monitors:
-                        self.add_monitor(extra)
+                    if extra not in self.extras:
+                        self.extras.append(extra)
+                        self._follow_xincludes(fname=extra)
+                        if extra not in self.extra_monitors:
+                            self.add_monitor(extra)
 
     def Index(self):
         self.xdata = lxml.etree.XML(self.data, base_url=self.name,
diff --git a/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPrivateKeyCreator.py b/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPrivateKeyCreator.py
index e5611d50b..e9698f526 100644
--- a/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPrivateKeyCreator.py
+++ b/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPrivateKeyCreator.py
@@ -98,7 +98,7 @@ class CfgPrivateKeyCreator(XMLCfgCreator):
             raise
 
     # pylint: disable=W0221
-    def create_data(self, entry, metadata, return_pair=False):
+    def create_data(self, entry, metadata):
         """ Create data for the given entry on the given client
 
         :param entry: The abstract entry to create data for.  This
@@ -106,15 +106,7 @@ class CfgPrivateKeyCreator(XMLCfgCreator):
         :type entry: lxml.etree._Element
         :param metadata: The client metadata to create data for
         :type metadata: Bcfg2.Server.Plugins.Metadata.ClientMetadata
-        :param return_pair: Return a tuple of ``(public key, private
-                            key)`` instead of just the private key.
-                            This is used by
-                            :class:`Bcfg2.Server.Plugins.Cfg.CfgPublicKeyCreator.CfgPublicKeyCreator`
-                            to create public keys as requested.
-        :type return_pair: bool
         :returns: string - The private key data
-        :returns: tuple - Tuple of ``(public key, private key)``, if
-                  ``return_pair`` is set to True
         """
         spec = self.XMLMatch(metadata)
         specificity = self.get_specificity(metadata)
@@ -132,11 +124,7 @@ class CfgPrivateKeyCreator(XMLCfgCreator):
             # return it
             privkey = open(filename).read()
             self.write_data(privkey, **specificity)
-
-            if return_pair:
-                return (pubkey, privkey)
-            else:
-                return privkey
+            return privkey
         finally:
             shutil.rmtree(os.path.dirname(filename))
     # pylint: enable=W0221
diff --git a/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPublicKeyCreator.py b/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPublicKeyCreator.py
index de1848159..3f2d1030b 100644
--- a/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPublicKeyCreator.py
+++ b/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPublicKeyCreator.py
@@ -2,7 +2,11 @@
 :class:`Bcfg2.Server.Plugins.Cfg.CfgPrivateKeyCreator.CfgPrivateKeyCreator`
 to create SSH keys on the fly. """
 
+import os
+import sys
+import tempfile
 import lxml.etree
+from Bcfg2.Utils import Executor
 from Bcfg2.Server.Plugin import StructFile, PluginExecutionError
 from Bcfg2.Server.Plugins.Cfg import CfgCreator, CfgCreationError, get_cfg
 
@@ -30,7 +34,8 @@ class CfgPublicKeyCreator(CfgCreator, StructFile):
         CfgCreator.__init__(self, fname)
         StructFile.__init__(self, fname)
         self.cfg = get_cfg()
-    __init__.__doc__ = CfgCreator.__init__.__doc__
+        self.core = self.cfg.core
+        self.cmd = Executor()
 
     def create_data(self, entry, metadata):
         if entry.get("name").endswith(".pub"):
@@ -40,25 +45,51 @@ class CfgPublicKeyCreator(CfgCreator, StructFile):
                                    "%s: Filename does not end in .pub" %
                                    entry.get("name"))
 
-        if privkey not in self.cfg.entries:
-            raise CfgCreationError("Cfg: Could not find Cfg entry for %s "
-                                   "(private key for %s)" % (privkey,
-                                                             self.name))
-        eset = self.cfg.entries[privkey]
+        privkey_entry = lxml.etree.Element("Path", name=privkey)
         try:
+            self.core.Bind(privkey_entry, metadata)
+        except PluginExecutionError:
+            raise CfgCreationError("Cfg: Could not bind %s (private key for "
+                                   "%s): %s" % (privkey, self.name,
+                                                sys.exc_info()[1]))
+
+        try:
+            eset = self.cfg.entries[privkey]
             creator = eset.best_matching(metadata,
                                          eset.get_handlers(metadata,
                                                            CfgCreator))
+        except KeyError:
+            raise CfgCreationError("Cfg: No private key defined for %s (%s)" %
+                                   (self.name, privkey))
         except PluginExecutionError:
             raise CfgCreationError("Cfg: No privkey.xml defined for %s "
                                    "(private key for %s)" % (privkey,
                                                              self.name))
 
-        privkey_entry = lxml.etree.Element("Path", name=privkey)
-        pubkey = creator.create_data(privkey_entry, metadata,
-                                     return_pair=True)[0]
-        return pubkey
-    create_data.__doc__ = CfgCreator.create_data.__doc__
+        specificity = creator.get_specificity(metadata)
+        fname = self.get_filename(**specificity)
+
+        # if the private key didn't exist, then creating it may have
+        # created the private key, too.  check for it first.
+        if os.path.exists(fname):
+            return open(fname).read()
+        else:
+            # generate public key from private key
+            fd, privfile = tempfile.mkstemp()
+            try:
+                os.fdopen(fd, 'w').write(privkey_entry.text)
+                cmd = ["ssh-keygen", "-y", "-f", privfile]
+                self.debug_log("Cfg: Extracting SSH public key from %s: %s" %
+                               (privkey, " ".join(cmd)))
+                result = self.cmd.run(cmd)
+                if not result.success:
+                    raise CfgCreationError("Cfg: Failed to extract public key "
+                                           "from %s: %s" % (privkey,
+                                                            result.error))
+                self.write_data(result.stdout, **specificity)
+                return result.stdout
+            finally:
+                os.unlink(privfile)
 
     def handle_event(self, event):
         CfgCreator.handle_event(self, event)
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py
index 2967a23b6..d64bbaabf 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py
@@ -29,6 +29,7 @@ class TestCfgPrivateKeyCreator(TestXMLCfgCreator):
     test_obj = CfgPrivateKeyCreator
     should_monitor = False
 
+    @patch("Bcfg2.Server.Plugins.Cfg.CfgPublicKeyCreator.get_cfg", Mock())
     def get_obj(self, name=None, fam=None):
         return TestXMLCfgCreator.get_obj(self, name=name)
 
@@ -122,28 +123,11 @@ class TestCfgPrivateKeyCreator(TestXMLCfgCreator):
         pkc.XMLMatch.assert_called_with(metadata)
         pkc.get_specificity.assert_called_with(metadata)
         pkc._gen_keypair.assert_called_with(metadata,
- pkc.XMLMatch.return_value)
-        self.assertItemsEqual(mock_open.call_args_list,
-                              [call(privkey + ".pub"), call(privkey)])
-        pkc.pubkey_creator.get_filename.assert_called_with(group="foo")
-        pkc.pubkey_creator.write_data.assert_called_with(
-            "ssh-rsa publickey pubkey.filename\n", group="foo")
-        pkc.write_data.assert_called_with("privatekey", group="foo")
-        mock_rmtree.assert_called_with(datastore)
-
-        reset()
-        self.assertEqual(pkc.create_data(entry, metadata, return_pair=True),
-                         ("ssh-rsa publickey pubkey.filename\n",
-                          "privatekey"))
-        pkc.XMLMatch.assert_called_with(metadata)
-        pkc.get_specificity.assert_called_with(metadata)
-        pkc._gen_keypair.assert_called_with(metadata,
                                             pkc.XMLMatch.return_value)
         self.assertItemsEqual(mock_open.call_args_list,
                               [call(privkey + ".pub"), call(privkey)])
         pkc.pubkey_creator.get_filename.assert_called_with(group="foo")
         pkc.pubkey_creator.write_data.assert_called_with(
-            "ssh-rsa publickey pubkey.filename\n",
-            group="foo")
+            "ssh-rsa publickey pubkey.filename\n", group="foo")
         pkc.write_data.assert_called_with("privatekey", group="foo")
         mock_rmtree.assert_called_with(datastore)
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPublicKeyCreator.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPublicKeyCreator.py
index 04772cf9a..f512a6803 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPublicKeyCreator.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPublicKeyCreator.py
@@ -25,6 +25,7 @@ class TestCfgPublicKeyCreator(TestCfgCreator, TestStructFile):
     test_obj = CfgPublicKeyCreator
     should_monitor = False
 
+    @patch("Bcfg2.Server.Plugins.Cfg.CfgPublicKeyCreator.get_cfg", Mock())
     def get_obj(self, name=None, fam=None):
         return TestCfgCreator.get_obj(self, name=name)
 
@@ -37,41 +38,117 @@ class TestCfgPublicKeyCreator(TestCfgCreator, TestStructFile):
         mock_HandleEvent.assert_called_with(pkc, evt)
         mock_handle_event.assert_called_with(pkc, evt)
 
-    def test_create_data(self):
+    @patch("os.unlink")
+    @patch("os.path.exists")
+    @patch("tempfile.mkstemp")
+    @patch("os.fdopen", Mock())
+    @patch("%s.open" % builtins)
+    def test_create_data(self, mock_open, mock_mkstemp, mock_exists,
+                         mock_unlink):
         metadata = Mock()
         pkc = self.get_obj()
         pkc.cfg = Mock()
+        pkc.core = Mock()
+        pkc.cmd = Mock()
+        pkc.write_data = Mock()
 
+        pubkey = "public key data"
         privkey_entryset = Mock()
         privkey_creator = Mock()
-        pubkey = Mock()
-        privkey = Mock()
-        privkey_creator.create_data.return_value = (pubkey, privkey)
-        privkey_entryset.best_matching.return_value = privkey_creator
+        privkey_creator.get_specificity = Mock()
+        privkey_creator.get_specificity.return_value = dict()
+        fileloc = pkc.get_filename()
         pkc.cfg.entries = {"/home/foo/.ssh/id_rsa": privkey_entryset}
 
+        def reset():
+            privkey_creator.reset_mock()
+            pkc.cmd.reset_mock()
+            pkc.core.reset_mock()
+            pkc.write_data.reset_mock()
+            mock_exists.reset_mock()
+            mock_unlink.reset_mock()
+            mock_mkstemp.reset_mock()
+            mock_open.reset_mock()
+
         # public key doesn't end in .pub
         entry = lxml.etree.Element("Path", name="/home/bar/.ssh/bogus")
         self.assertRaises(CfgCreationError,
                           pkc.create_data, entry, metadata)
+        self.assertFalse(pkc.write_data.called)
+
+        # cannot bind private key
+        reset()
+        pkc.core.Bind.side_effect = PluginExecutionError
+        entry = lxml.etree.Element("Path", name="/home/foo/.ssh/id_rsa.pub")
+        self.assertRaises(CfgCreationError,
+                          pkc.create_data, entry, metadata)
+        self.assertFalse(pkc.write_data.called)
 
         # private key not in cfg.entries
+        reset()
+        pkc.core.Bind.side_effect = None
+        pkc.core.Bind.return_value = "private key data"
         entry = lxml.etree.Element("Path", name="/home/bar/.ssh/id_rsa.pub")
         self.assertRaises(CfgCreationError,
                           pkc.create_data, entry, metadata)
+        self.assertFalse(pkc.write_data.called)
 
-        # successful operation
+        # no privkey.xml defined
+        reset()
+        privkey_entryset.best_matching.side_effect = PluginExecutionError
+        entry = lxml.etree.Element("Path", name="/home/foo/.ssh/id_rsa.pub")
+        self.assertRaises(CfgCreationError,
+                          pkc.create_data, entry, metadata)
+        self.assertFalse(pkc.write_data.called)
+
+        # successful operation, create new key
+        reset()
+        pkc.cmd.run.return_value = Mock()
+        pkc.cmd.run.return_value.success = True
+        pkc.cmd.run.return_value.stdout = pubkey
+        mock_mkstemp.return_value = (Mock(), str(Mock()))
+        mock_exists.return_value = False
+        privkey_entryset.best_matching.side_effect = None
+        privkey_entryset.best_matching.return_value = privkey_creator
         entry = lxml.etree.Element("Path", name="/home/foo/.ssh/id_rsa.pub")
         self.assertEqual(pkc.create_data(entry, metadata), pubkey)
+        self.assertTrue(pkc.core.Bind.called)
+        (privkey_entry, md) = pkc.core.Bind.call_args[0]
+        self.assertXMLEqual(privkey_entry,
+                            lxml.etree.Element("Path",
+                                               name="/home/foo/.ssh/id_rsa"))
+        self.assertEqual(md, metadata)
+
         privkey_entryset.get_handlers.assert_called_with(metadata, CfgCreator)
-        privkey_entryset.best_matching.assert_called_with(metadata,
-                                                          privkey_entryset.get_handlers.return_value)
-        self.assertXMLEqual(privkey_creator.create_data.call_args[0][0],
+        privkey_entryset.best_matching.assert_called_with(
+            metadata,
+            privkey_entryset.get_handlers.return_value)
+        mock_exists.assert_called_with(fileloc)
+        pkc.cmd.run.assert_called_with(["ssh-keygen", "-y", "-f",
+                                        mock_mkstemp.return_value[1]])
+        self.assertEqual(pkc.write_data.call_args[0][0], pubkey)
+        mock_unlink.assert_called_with(mock_mkstemp.return_value[1])
+        self.assertFalse(mock_open.called)
+
+        # successful operation, no need to create new key
+        reset()
+        mock_exists.return_value = True
+        mock_open.return_value = Mock()
+        mock_open.return_value.read.return_value = pubkey
+        pkc.cmd.run.return_value.stdout = None
+        self.assertEqual(pkc.create_data(entry, metadata), pubkey)
+        self.assertTrue(pkc.core.Bind.called)
+        (privkey_entry, md) = pkc.core.Bind.call_args[0]
+        self.assertXMLEqual(privkey_entry,
                             lxml.etree.Element("Path",
                                                name="/home/foo/.ssh/id_rsa"))
-        self.assertEqual(privkey_creator.create_data.call_args[0][1], metadata)
+        self.assertEqual(md, metadata)
 
-        # no privkey.xml
-        privkey_entryset.best_matching.side_effect = PluginExecutionError
-        self.assertRaises(CfgCreationError,
-                          pkc.create_data, entry, metadata)
+        privkey_entryset.get_handlers.assert_called_with(metadata, CfgCreator)
+        privkey_entryset.best_matching.assert_called_with(
+            metadata,
+            privkey_entryset.get_handlers.return_value)
+        mock_exists.assert_called_with(fileloc)
+        mock_open.assert_called_with(fileloc)
+        self.assertFalse(mock_mkstemp.called)
+        self.assertFalse(pkc.write_data.called)