Light-weight SSH key removal
authorHelga Velroyen <helgav@google.com>
Tue, 12 Jan 2016 13:57:47 +0000 (14:57 +0100)
committerHelga Velroyen <helgav@google.com>
Tue, 12 Jan 2016 15:53:02 +0000 (16:53 +0100)
This patch adds an RPC call, which is a very light-weight
version of removing an SSH key from the cluster. It simply
only removes it from the public key file of the master.

This is used later to clean up in case the pre-hooks for
adding a node fail. When adding a node with 'gnt-node add',
the client code in gnt_node adds the key to the public
key file. If the hooks fail, so far this key was not
cleaned up and manual intervention was necessary.

To avoid any abuse of the RPC call, it includes a safety
check which makes sure that only keys of nodes that are
not in the cluster anymore (and thus are stray keys).

Signed-off-by: Helga Velroyen <helgav@google.com>
Reviewed-by: Klaus Aehlig <aehlig@google.com>

lib/backend.py
lib/rpc_defs.py
lib/server/noded.py
test/py/ganeti.backend_unittest.py

index 346b900..68880f3 100644 (file)
@@ -1970,6 +1970,40 @@ def RemoveNodeSshKeyBulk(node_list,
 # pylint: enable=R0913
 
 
+def RemoveSshKeyFromPublicKeyFile(node_name,
+                                  pub_key_file=pathutils.SSH_PUB_KEYS,
+                                  ssconf_store=None):
+  """Removes a SSH key from the master's public key file.
+
+  This is an operation that is only used to clean up after failed operations
+  (for example failed hooks before adding a node). To avoid abuse of this
+  function (and the matching RPC call), we add a safety check to make sure
+  that only stray keys can be removed that belong to nodes that are not
+  in the cluster (anymore).
+
+  @type node_name: string
+  @param node_name: the name of the node whose key is removed
+
+  """
+  if not ssconf_store:
+    ssconf_store = ssconf.SimpleStore()
+
+  node_list = ssconf_store.GetNodeList()
+
+  if node_name in node_list:
+    raise errors.SshUpdateError("Cannot remove key of node '%s',"
+                                " because it still belongs to the cluster."
+                                % node_name)
+
+  keys_by_name = ssh.QueryPubKeyFile([node_name], key_file=pub_key_file)
+  if not keys_by_name or node_name not in keys_by_name:
+    logging.info("The node '%s' whose key is supposed to be removed does not"
+                 " have an entry in the public key file. Hence, there is"
+                 " nothing left to do.", node_name)
+
+  ssh.RemovePublicKey(node_name, key_file=pub_key_file)
+
+
 def _GenerateNodeSshKey(node_uuid, node_name, ssh_port_map, ssh_key_type,
                         ssh_key_bits, pub_key_file=pathutils.SSH_PUB_KEYS,
                         ssconf_store=None,
index 9d955e1..8614a03 100644 (file)
@@ -576,6 +576,9 @@ _NODE_CALLS = [
     ("debug", None, "Set logging of SSH update tool to 'debug'."),
     ("verbose", None, "Set logging of SSH update tool to 'info'.")],
     None, None, "Renew all SSH key pairs of all nodes nodes."),
+  ("node_ssh_key_remove_light", MULTI, None, constants.RPC_TMO_FAST, [
+    ("node_name", None, "Name of the node whose key is removed")],
+    None, None, "Remove a node's SSH key from the master's public key file."),
   ]
 
 _MISC_CALLS = [
index 5537bb1..1397fbd 100644 (file)
@@ -976,6 +976,14 @@ class NodeRequestHandler(http.server.HttpServerHandler):
                                     ssh_update_debug=debug,
                                     ssh_update_verbose=verbose)
 
+  @staticmethod
+  def perspective_node_ssh_key_remove_light(params):
+    """Removes a node's SSH key from the master's public key file.
+
+    """
+    (node_name, ) = params
+    return backend.RemoveSshKeyFromPublicKeyFile(node_name)
+
   # cluster --------------------------
 
   @staticmethod
index 2b1d71e..b6af71f 100755 (executable)
@@ -1920,6 +1920,53 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
                      if node == node_name])
 
 
+class TestRemoveSshKeyFromPublicKeyFile(testutils.GanetiTestCase):
+
+  def setUp(self):
+    testutils.GanetiTestCase.setUp(self)
+    self._ssconf_mock = mock.Mock()
+    self._ssconf_mock.GetNodeList = mock.Mock()
+    self._tmpdir = tempfile.mkdtemp()
+    self._pub_keys_file = os.path.join(self._tmpdir, "pub_keys_file")
+
+  def testValidRemoval(self):
+    key = "myKey"
+    name = "myName"
+    ssh.AddPublicKey(name, key, key_file=self._pub_keys_file)
+    self._ssconf_mock.GetNodeList.return_value = \
+        ["myOtherNode1", "myOtherNode2"]
+
+    backend.RemoveSshKeyFromPublicKeyFile(
+        name, pub_key_file=self._pub_keys_file,
+        ssconf_store=self._ssconf_mock)
+
+    result = ssh.QueryPubKeyFile([name], key_file=self._pub_keys_file)
+    self.assertEqual({}, result)
+
+  def testStillClusterNode(self):
+    """Tests the safety check to only remove keys of obsolete nodes."""
+    key = "myKey"
+    name = "myName"
+    ssh.AddPublicKey(name, key, key_file=self._pub_keys_file)
+    self._ssconf_mock.GetNodeList.return_value = ["myName", "myOtherNode"]
+
+    self.assertRaises(
+        errors.SshUpdateError,
+        backend.RemoveSshKeyFromPublicKeyFile,
+        name, pub_key_file=self._pub_keys_file,
+        ssconf_store=self._ssconf_mock)
+
+  def testNoKey(self):
+    name = "myName"
+    # 'clear' file to make sure it exists.
+    ssh.ClearPubKeyFile(key_file=self._pub_keys_file)
+    self._ssconf_mock.GetNodeList.return_value = ["myOtherNode"]
+
+    backend.RemoveSshKeyFromPublicKeyFile(
+        name, pub_key_file=self._pub_keys_file,
+        ssconf_store=self._ssconf_mock)
+
+
 class TestVerifySshSetup(testutils.GanetiTestCase):
 
   _NODE1_UUID = "uuid1"