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 <[email protected]> --- lib/backend.py | 59 +++++++++++---------------- lib/client/gnt_cluster.py | 8 ++-- lib/client/gnt_node.py | 6 +-- lib/ssh.py | 4 +- test/py/ganeti.client.gnt_cluster_unittest.py | 2 +- 5 files changed, 34 insertions(+), 45 deletions(-) diff --git a/lib/backend.py b/lib/backend.py index 97ff4a1..c8b24f1 100644 --- a/lib/backend.py +++ b/lib/backend.py @@ -2058,24 +2058,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, @@ -2122,11 +2104,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) @@ -2159,11 +2139,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 @@ -2188,6 +2168,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: @@ -2206,11 +2190,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)) @@ -2253,11 +2237,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 @@ -2291,6 +2276,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: diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py index 9833b33..2209d5c 100644 --- a/lib/client/gnt_cluster.py +++ b/lib/client/gnt_cluster.py @@ -1261,10 +1261,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) diff --git a/lib/client/gnt_node.py b/lib/client/gnt_node.py index 36961fc..59b7a77 100644 --- a/lib/client/gnt_node.py +++ b/lib/client/gnt_node.py @@ -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. diff --git a/lib/ssh.py b/lib/ssh.py index 2c92264..0fb592b 100644 --- a/lib/ssh.py +++ b/lib/ssh.py @@ -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 diff --git a/test/py/ganeti.client.gnt_cluster_unittest.py b/test/py/ganeti.client.gnt_cluster_unittest.py index c2cb9f5..a19180c 100755 --- a/test/py/ganeti.client.gnt_cluster_unittest.py +++ b/test/py/ganeti.client.gnt_cluster_unittest.py @@ -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 -- 2.6.0.rc2.230.g3dd15c0
