Make backend.RenewCrypto more testable
authorHelga Velroyen <helgav@google.com>
Tue, 19 Jan 2016 14:19:02 +0000 (15:19 +0100)
committerHelga Velroyen <helgav@google.com>
Fri, 22 Jan 2016 09:39:11 +0000 (10:39 +0100)
In order to improve the testability of backend.RenewCrypto,
this patch does two things:
* It uses the previously introduced SSH utility functions.
  Those are easier to consistently mock during unit tests
  and they consistenly abstract the lower layer of file
  operations on SSH keys.
* When calling the subfunctions to add and remove keys,
  some of the optional parameters were not propagated,
  which in tests will prevent the mocks from being
  propagated.

Besides that, it also renames ReadRemoteSshPubKeys to
ReadRemoteSshPubKey, because that actually only fetches one
key.

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

lib/backend.py
lib/client/gnt_cluster.py
lib/client/gnt_node.py
lib/ssh.py
test/py/ganeti.client.gnt_cluster_unittest.py

index 0d4a789..18d3431 100644 (file)
@@ -2059,24 +2059,6 @@ def _GetOldMasterKeys(master_node_uuid, pub_key_file):
   return old_master_keys_by_uuid
 
 
-def _GetNewMasterKey(root_keyfiles, master_node_uuid):
-  new_master_keys = []
-  for (_, (_, public_key_file)) in root_keyfiles.items():
-    public_key_dir = os.path.dirname(public_key_file)
-    public_key_file_tmp_filename = \
-        os.path.splitext(os.path.basename(public_key_file))[0] \
-        + constants.SSHS_MASTER_SUFFIX + ".pub"
-    public_key_path_tmp = os.path.join(public_key_dir,
-                                       public_key_file_tmp_filename)
-    if os.path.exists(public_key_path_tmp):
-      # for some key types, there might not be any keys
-      key = utils.ReadFile(public_key_path_tmp)
-      new_master_keys.append(key)
-  if not new_master_keys:
-    raise errors.SshUpdateError("Cannot find any type of temporary SSH key.")
-  return {master_node_uuid: new_master_keys}
-
-
 def RenewSshKeys(node_uuids, node_names, master_candidate_uuids,
                  potential_master_candidates, old_key_type, new_key_type,
                  new_key_bits,
@@ -2123,11 +2105,9 @@ def RenewSshKeys(node_uuids, node_names, master_candidate_uuids,
     raise errors.ProgrammerError("List of nodes UUIDs and node names"
                                  " does not match in length.")
 
-  (_, root_keyfiles) = \
-    ssh.GetAllUserFiles(constants.SSH_LOGIN_USER, mkdir=False, dircheck=False)
-  (_, old_pub_keyfile) = root_keyfiles[old_key_type]
-  (_, new_pub_keyfile) = root_keyfiles[new_key_type]
-  old_master_key = utils.ReadFile(old_pub_keyfile)
+  old_pub_keyfile = ssh.GetSshPubKeyFilename(old_key_type)
+  new_pub_keyfile = ssh.GetSshPubKeyFilename(new_key_type)
+  old_master_key = ssh.ReadLocalSshPubKeys([old_key_type])
 
   node_uuid_name_map = zip(node_uuids, node_names)
 
@@ -2160,11 +2140,11 @@ def RenewSshKeys(node_uuids, node_names, master_candidate_uuids,
 
     if master_candidate:
       logging.debug("Fetching old SSH key from node '%s'.", node_name)
-      old_pub_key = ssh.ReadRemoteSshPubKeys(old_pub_keyfile,
-                                             node_name, cluster_name,
-                                             ssh_port_map[node_name],
-                                             False, # ask_key
-                                             False) # key_check
+      old_pub_key = ssh.ReadRemoteSshPubKey(old_pub_keyfile,
+                                            node_name, cluster_name,
+                                            ssh_port_map[node_name],
+                                            False, # ask_key
+                                            False) # key_check
       if old_pub_key != old_master_key:
         # If we are already in a multi-key setup (that is past Ganeti 2.12),
         # we can safely remove the old key of the node. Otherwise, we cannot
@@ -2189,6 +2169,10 @@ def RenewSshKeys(node_uuids, node_names, master_candidate_uuids,
         master_candidate_uuids,
         potential_master_candidates,
         master_uuid=master_node_uuid,
+        pub_key_file=ganeti_pub_keys_file,
+        ssconf_store=ssconf_store,
+        noded_cert_file=noded_cert_file,
+        run_cmd_fn=run_cmd_fn,
         ssh_update_debug=ssh_update_debug,
         ssh_update_verbose=ssh_update_verbose)
     if node_errors:
@@ -2207,11 +2191,11 @@ def RenewSshKeys(node_uuids, node_names, master_candidate_uuids,
 
     try:
       logging.debug("Fetching newly created SSH key from node '%s'.", node_name)
-      pub_key = ssh.ReadRemoteSshPubKeys(new_pub_keyfile,
-                                         node_name, cluster_name,
-                                         ssh_port_map[node_name],
-                                         False, # ask_key
-                                         False) # key_check
+      pub_key = ssh.ReadRemoteSshPubKey(new_pub_keyfile,
+                                        node_name, cluster_name,
+                                        ssh_port_map[node_name],
+                                        False, # ask_key
+                                        False) # key_check
     except:
       raise errors.SshUpdateError("Could not fetch key of node %s"
                                   " (UUID %s)" % (node_name, node_uuid))
@@ -2254,11 +2238,12 @@ def RenewSshKeys(node_uuids, node_names, master_candidate_uuids,
                       ssh_update_debug=ssh_update_debug,
                       ssh_update_verbose=ssh_update_verbose)
   # Read newly created master key
-  new_master_key_dict = _GetNewMasterKey(root_keyfiles, master_node_uuid)
+  new_master_keys = ssh.ReadLocalSshPubKeys(
+      [new_key_type], suffix=constants.SSHS_MASTER_SUFFIX)
 
   # Replace master key in the master nodes' public key file
   ssh.RemovePublicKey(master_node_uuid, key_file=ganeti_pub_keys_file)
-  for pub_key in new_master_key_dict[master_node_uuid]:
+  for pub_key in new_master_keys:
     ssh.AddPublicKey(master_node_uuid, pub_key, key_file=ganeti_pub_keys_file)
 
   # Add new master key to all node's public and authorized keys
@@ -2292,6 +2277,10 @@ def RenewSshKeys(node_uuids, node_names, master_candidate_uuids,
       keys_to_remove=old_master_keys_by_uuid, from_authorized_keys=True,
       from_public_keys=False, clear_authorized_keys=False,
       clear_public_keys=False,
+      pub_key_file=ganeti_pub_keys_file,
+      ssconf_store=ssconf_store,
+      noded_cert_file=noded_cert_file,
+      run_cmd_fn=run_cmd_fn,
       ssh_update_debug=ssh_update_debug,
       ssh_update_verbose=ssh_update_verbose)
   if node_errors:
index 1109fe7..fc8c5dd 100644 (file)
@@ -1269,10 +1269,10 @@ def _BuildGanetiPubKeys(options, pub_key_file=pathutils.SSH_PUB_KEYS, cl=None,
 
   # get the key files of all non-master nodes
   for node in nonmaster_nodes:
-    pub_key = ssh.ReadRemoteSshPubKeys(pub_key_filename, node, cluster_name,
-                                       ssh_port_map[node],
-                                       options.ssh_key_check,
-                                       options.ssh_key_check)
+    pub_key = ssh.ReadRemoteSshPubKey(pub_key_filename, node, cluster_name,
+                                      ssh_port_map[node],
+                                      options.ssh_key_check,
+                                      options.ssh_key_check)
     ssh.AddPublicKey(node_uuid_map[node], pub_key, key_file=pub_key_file)
 
 
index 36961fc..59b7a77 100644 (file)
@@ -250,9 +250,9 @@ def _SetupSSH(options, cluster_name, node, ssh_port, cl):
                          strict_host_check=options.ssh_key_check)
 
   (_, pub_keyfile) = root_keyfiles[ssh_key_type]
-  pub_key = ssh.ReadRemoteSshPubKeys(pub_keyfile, node, cluster_name, ssh_port,
-                                     options.ssh_key_check,
-                                     options.ssh_key_check)
+  pub_key = ssh.ReadRemoteSshPubKey(pub_keyfile, node, cluster_name, ssh_port,
+                                    options.ssh_key_check,
+                                    options.ssh_key_check)
   # Unfortunately, we have to add the key with the node name rather than
   # the node's UUID here, because at this point, we do not have a UUID yet.
   # The entry will be corrected in noded later.
index 2c92264..0fb592b 100644 (file)
@@ -1074,8 +1074,8 @@ def RunSshCmdWithStdin(cluster_name, node, basecmd, port, data,
                              (result.cmd, result.fail_reason))
 
 
-def ReadRemoteSshPubKeys(pub_key_file, node, cluster_name, port, ask_key,
-                         strict_host_check):
+def ReadRemoteSshPubKey(pub_key_file, node, cluster_name, port, ask_key,
+                        strict_host_check):
   """Fetches a public SSH key from a node via SSH.
 
   @type pub_key_file: string
index c2cb9f5..a19180c 100755 (executable)
@@ -405,7 +405,7 @@ class TestBuildGanetiPubKeys(testutils.GanetiTestCase):
     self._setUpFakeKeys()
 
     self._ssh_read_remote_ssh_pub_keys_patcher = testutils \
-      .patch_object(ssh, "ReadRemoteSshPubKeys")
+      .patch_object(ssh, "ReadRemoteSshPubKey")
     self._ssh_read_remote_ssh_pub_keys_mock = \
       self._ssh_read_remote_ssh_pub_keys_patcher.start()
     self._ssh_read_remote_ssh_pub_keys_mock.return_value = self._SOME_KEY_DICT