Hi!
On Tue, Sep 30, 2014 at 4:14 PM, Petr Pudlak <pud...@google.com> wrote:
> On Tue, Sep 02, 2014 at 04:19:45PM +0200, 'Helga Velroyen' via
> ganeti-devel wrote:
>
>> This patch adds the '--new-ssh-keys' option
>> to 'gnt-cluster renew-crypto'. In the client, it retrieves
>> all current ssh keys and (re-)writes the 'ganeti_pub_key'
>> file with it, then in the backend, the new keys are
>> generated and distributed.
>>
>> Signed-off-by: Helga Velroyen <hel...@google.com>
>>
>> Conflicts:
>> src/Ganeti/OpParams.hs
>>
>> Signed-off-by: Helga Velroyen <hel...@google.com>
>> ---
>> UPGRADE | 21 +++++
>> lib/backend.py | 113
>> +++++++++++++++++++++++++-
>> lib/cli.py | 24 ++++++
>> lib/client/gnt_cluster.py | 72 ++++++++++++++--
>> lib/cmdlib/cluster.py | 52 ++++++++++++
>> lib/rpc_defs.py | 7 ++
>> lib/server/noded.py | 11 +++
>> lib/tools/ssh_update.py | 28 ++++++-
>> qa/qa_cluster.py | 7 +-
>> src/Ganeti/Constants.hs | 12 ++-
>> src/Ganeti/OpCodes.hs | 1 +
>> src/Ganeti/OpParams.hs | 7 ++
>> test/hs/Test/Ganeti/OpCodes.hs | 3 +-
>> test/py/ganeti.backend_unittest.py | 24 +++++-
>> test/py/ganeti.client.gnt_cluster_unittest.py | 108
>> ++++++++++++++++++++++++
>> test/py/ganeti.mcpu_unittest.py | 1 -
>> tools/post-upgrade | 8 ++
>> 17 files changed, 483 insertions(+), 16 deletions(-)
>>
>> diff --git a/UPGRADE b/UPGRADE
>> index 62624ff..2186298 100644
>> --- a/UPGRADE
>> +++ b/UPGRADE
>> @@ -39,6 +39,27 @@ the Ganeti binaries should happen in the same way as
>> for all other binaries on
>> your system.
>>
>>
>> +2.13
>> +----
>> +
>> +When upgrading to 2.13, first apply the instructions of ``2.11 and
>> +above``. 2.13 comes with the new feature of enhanced SSH security
>> +through individual SSH keys. This features needs to be enabled
>> +after the upgrade by::
>> +
>> + $ gnt-cluster renew-crypt --new-ssh-keys --no-ssh-key-check
>> +
>> +Note that new SSH keys are generated automatically without warning when
>> +upgrading with ``gnt-cluster upgrade``.
>> +
>> +If you instructed Ganeti to not touch the SSH setup (by using the
>> +``--no-ssh-init`` option of ``gnt-cluster init``, the changes in the
>> +handling of SSH keys will not affect your cluster.
>> +
>> +If you want to be prompted for each newly created SSH key, leave out
>> +the ``--no-ssh-key-check`` option in the command listed above.
>> +
>> +
>> 2.11
>> ----
>>
>> diff --git a/lib/backend.py b/lib/backend.py
>> index efdc00a..4a6ad04 100644
>> --- a/lib/backend.py
>> +++ b/lib/backend.py
>> @@ -1471,7 +1471,7 @@ def AddNodeSshKey(node_uuid, node_name,
>> pot_mc_data = copy.deepcopy(base_data)
>> if to_public_keys:
>> pot_mc_data[constants.SSHS_SSH_PUBLIC_KEYS] = \
>> - (constants.SSHS_ADD, keys_by_uuid)
>> + (constants.SSHS_REPLACE_OR_ADD, keys_by_uuid)
>>
>> all_nodes = ssconf_store.GetNodeList()
>> master_node = ssconf_store.GetMasterNode()
>> @@ -1626,6 +1626,117 @@ def RemoveNodeSshKey(node_uuid, node_name,
>> from_authorized_keys,
>> node_name, e)
>>
>>
>> +def _GenerateNodeSshKey(node_uuid, node_name, ssh_port_map,
>> + pub_key_file=pathutils.SSH_PUB_KEYS,
>> + ssconf_store=None,
>> + noded_cert_file=pathutils.NODED_CERT_FILE,
>> + run_cmd_fn=ssh.RunSshCmdWithStdin):
>> + """Generates the root SSH key pair on the node.
>> +
>> + @type node_uuid: str
>> + @param node_uuid: UUID of the node whose key is removed
>> + @type node_name: str
>> + @param node_name: name of the node whose key is remove
>> + @type ssh_port_map: dict of str to int
>> + @param ssh_port_map: mapping of node names to their SSH port
>> +
>> + """
>> + if not ssconf_store:
>> + ssconf_store = ssconf.SimpleStore()
>> +
>> + keys_by_uuid = ssh.QueryPubKeyFile([node_uuid], key_file=pub_key_file)
>> + if not keys_by_uuid or node_uuid not in keys_by_uuid:
>> + raise errors.SshUpdateError("Node %s (UUID: %s) whose key is
>> requested to"
>> + " be regenerated is not registered in
>> the"
>> + " public keys file." % (node_name,
>> node_uuid))
>> +
>> + data = {}
>> + _InitSshUpdateData(data, noded_cert_file, ssconf_store)
>> + cluster_name = data[constants.SSHS_CLUSTER_NAME]
>> + data[constants.SSHS_GENERATE] = True
>> +
>> + run_cmd_fn(cluster_name, node_name, pathutils.SSH_UPDATE,
>> + True, True, False, False, False,
>> + ssh_port_map.get(node_name), data, ssconf_store)
>> +
>> +
>> +def RenewSshKeys(node_uuids, node_names, ssh_port_map,
>> + master_candidate_uuids,
>> + potential_master_candidates,
>> + pub_key_file=pathutils.SSH_PUB_KEYS,
>> + ssconf_store=None,
>> + noded_cert_file=pathutils.NODED_CERT_FILE,
>> + run_cmd_fn=ssh.RunSshCmdWithStdin):
>> + """Renews all SSH keys and updates authorized_keys and ganeti_pub_keys.
>> +
>> + """
>> + if not ssconf_store:
>> + ssconf_store = ssconf.SimpleStore()
>> + cluster_name = ssconf_store.GetClusterName()
>> +
>> + if not len(node_uuids) == len(node_names):
>> + 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)
>> +
>> + node_uuid_name_map = zip(node_uuids, node_names)
>> +
>> + master_node_name = ssconf_store.GetMasterNode()
>> + # process non-master nodes
>> + for node_uuid, node_name in node_uuid_name_map:
>> + if node_name == master_node_name:
>> + continue
>> + master_candidate = node_uuid in master_candidate_uuids
>> + potential_master_candidate = node_name in potential_master_candidates
>> +
>> + keys_by_uuid = ssh.QueryPubKeyFile([node_uuid],
>> key_file=pub_key_file)
>> + if not keys_by_uuid:
>> + raise errors.SshUpdateError("No public key of node %s (UUID %s)
>> found,"
>> + " not generating a new key."
>> + % (node_name, node_uuid))
>> +
>> + RemoveNodeSshKey(node_uuid, node_name,
>> + master_candidate, # from authorized keys
>> + False, # Don't remove (yet) from public keys
>> + False, # Don't clear authorized_keys
>> + ssh_port_map,
>> + master_candidate_uuids,
>> + potential_master_candidates)
>> +
>> + _GenerateNodeSshKey(node_uuid, node_name, ssh_port_map,
>> + pub_key_file=pub_key_file,
>> + ssconf_store=ssconf_store,
>> + noded_cert_file=noded_cert_file,
>> + run_cmd_fn=run_cmd_fn)
>> +
>> + fetched_keys = ssh.ReadRemoteSshPubKeys(root_keyfiles, node_name,
>> + cluster_name,
>> + ssh_port_map[node_name],
>> + False, # ask_key
>> + False) # key_check
>> + if not fetched_keys:
>> + raise errors.SshUpdateError("Could not fetch key of node %s"
>> + " (UUID %s)" % (node_name, node_uuid))
>> +
>> + if potential_master_candidate:
>> + ssh.RemovePublicKey(node_uuid, key_file=pub_key_file)
>> + for pub_key in fetched_keys.values():
>> + ssh.AddPublicKey(node_uuid, pub_key, key_file=pub_key_file)
>> +
>> + AddNodeSshKey(node_uuid, node_name,
>> + master_candidate, # Add to authorized_keys file
>> + potential_master_candidate, # Add to public_keys
>> + True, # Get public keys
>> + ssh_port_map, potential_master_candidates,
>> + pub_key_file=pub_key_file, ssconf_store=ssconf_store,
>> + noded_cert_file=noded_cert_file,
>> + run_cmd_fn=run_cmd_fn)
>> +
>> + # FIXME: Update master key as well
>> +
>> +
>> def GetBlockDevSizes(devices):
>> """Return the size of the given block devices
>>
>> diff --git a/lib/cli.py b/lib/cli.py
>> index e939b5c..77c9323 100644
>> --- a/lib/cli.py
>> +++ b/lib/cli.py
>> @@ -136,6 +136,7 @@ __all__ = [
>> "NETWORK6_OPT",
>> "NEW_CLUSTER_CERT_OPT",
>> "NEW_NODE_CERT_OPT",
>> + "NEW_SSH_KEY_OPT",
>> "NEW_CLUSTER_DOMAIN_SECRET_OPT",
>> "NEW_CONFD_HMAC_KEY_OPT",
>> "NEW_RAPI_CERT_OPT",
>> @@ -255,6 +256,7 @@ __all__ = [
>> "GetClient",
>> "GetOnlineNodes",
>> "GetNodesSshPorts",
>> + "GetNodeUUIDs",
>> "JobExecutor",
>> "JobSubmittedException",
>> "ParseTimespec",
>> @@ -1491,6 +1493,10 @@ NEW_NODE_CERT_OPT = cli_option(
>> "--new-node-certificates", dest="new_node_cert", default=False,
>> action="store_true", help="Generate new node certificates (for all
>> nodes)")
>>
>> +NEW_SSH_KEY_OPT = cli_option(
>> + "--new-ssh-keys", dest="new_ssh_keys", default=False,
>> + action="store_true", help="Generate new node SSH keys (for all nodes)")
>> +
>> RAPI_CERT_OPT = cli_option("--rapi-certificate", dest="rapi_cert",
>> default=None,
>> help="File containing new RAPI certificate")
>> @@ -3729,6 +3735,7 @@ def GetNodesSshPorts(nodes, cl):
>> @type cl: L{ganeti.luxi.Client}
>> @return: the list of SSH ports corresponding to the nodes
>> @rtype: a list of tuples
>> +
>> """
>> return map(lambda t: t[0],
>> cl.QueryNodes(names=nodes,
>> @@ -3736,6 +3743,23 @@ def GetNodesSshPorts(nodes, cl):
>> use_locking=False))
>>
>>
>> +def GetNodeUUIDs(nodes, cl):
>> + """Retrieves the UUIDs of given nodes.
>> +
>> + @param nodes: the names of nodes
>> + @type nodes: a list of string
>> + @param cl: a client to use for the query
>> + @type cl: L{ganeti.luxi.Client}
>> + @return: the list of UUIDs corresponding to the nodes
>> + @rtype: a list of tuples
>> +
>> + """
>> + return map(lambda t: t[0],
>> + cl.QueryNodes(names=nodes,
>> + fields=["uuid"],
>> + use_locking=False))
>> +
>> +
>> def _ToStream(stream, txt, *args):
>> """Write a message to a stream, bypassing the logging system
>>
>> diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
>> index b6c7975..2c0de7f 100644
>> --- a/lib/client/gnt_cluster.py
>> +++ b/lib/client/gnt_cluster.py
>> @@ -938,7 +938,7 @@ def _ReadAndVerifyCert(cert_filename,
>> verify_private_key=False):
>> def _RenewCrypto(new_cluster_cert, new_rapi_cert, # pylint: disable=R0911
>> rapi_cert_filename, new_spice_cert, spice_cert_filename,
>> spice_cacert_filename, new_confd_hmac_key, new_cds,
>> - cds_filename, force, new_node_cert):
>> + cds_filename, force, new_node_cert, new_ssh_keys):
>> """Renews cluster certificates, keys and secrets.
>>
>> @type new_cluster_cert: bool
>> @@ -962,8 +962,10 @@ def _RenewCrypto(new_cluster_cert, new_rapi_cert, #
>> pylint: disable=R0911
>> @param cds_filename: Path to file containing new cluster domain secret
>> @type force: bool
>> @param force: Whether to ask user for confirmation
>> - @type new_node_cert: string
>> + @type new_node_cert: bool
>> @param new_node_cert: Whether to generate new node certificates
>> + @type new_ssh_keys: bool
>> + @param new_ssh_keys: Whether to generate new node SSH keys
>>
>> """
>> if new_rapi_cert and rapi_cert_filename:
>> @@ -1058,18 +1060,75 @@ def _RenewCrypto(new_cluster_cert, new_rapi_cert,
>> # pylint: disable=R0911
>> ToStdout("All requested certificates and keys have been replaced."
>> " Running \"gnt-cluster verify\" now is recommended.")
>>
>> - if new_node_cert:
>> + if new_node_cert or new_ssh_keys:
>> cl = GetClient()
>> - renew_op = opcodes.OpClusterRenewCrypto(node_certificates=new_node_
>> cert)
>> + renew_op = opcodes.OpClusterRenewCrypto(node_certificates=new_node_
>> cert,
>> + ssh_keys=new_ssh_keys)
>> SubmitOpCode(renew_op, cl=cl)
>>
>> return 0
>>
>>
>> +def _BuildGanetiPubKeys(options, pub_key_file=pathutils.SSH_PUB_KEYS,
>> cl=None,
>> + get_online_nodes_fn=GetOnlineNodes,
>> + get_nodes_ssh_ports_fn=GetNodesSshPorts,
>> + get_node_uuids_fn=GetNodeUUIDs,
>> + homedir_fn=None):
>> + """Recreates the 'ganeti_pub_key' file by polling all nodes.
>> +
>> + """
>> + if os.path.exists(pub_key_file):
>> + utils.CreateBackup(pub_key_file)
>> + utils.RemoveFile(pub_key_file)
>> +
>> + ssh.ClearPubKeyFile(pub_key_file)
>> +
>> + if not cl:
>> + cl = GetClient()
>> +
>> + (cluster_name, master_node) = \
>> + cl.QueryConfigValues(["cluster_name", "master_node"])
>> +
>> + online_nodes = get_online_nodes_fn([], cl=cl)
>> + ssh_ports = get_nodes_ssh_ports_fn(online_nodes + [master_node], cl)
>> + ssh_port_map = dict(zip(online_nodes + [master_node], ssh_ports))
>> +
>> + node_uuids = get_node_uuids_fn(online_nodes + [master_node], cl)
>> + node_uuid_map = dict(zip(online_nodes + [master_node], node_uuids))
>> +
>> + nonmaster_nodes = [name for name in online_nodes
>> + if name != master_node]
>> +
>> + (_, root_keyfiles) = \
>> + ssh.GetAllUserFiles(constants.SSH_LOGIN_USER, mkdir=False,
>> dircheck=False,
>> + _homedir_fn=homedir_fn)
>> +
>> + # get the key file of the master node
>> + for (_, (_, public_key_file)) in root_keyfiles.items():
>> + try:
>> + pub_key = utils.ReadFile(public_key_file)
>> + ssh.AddPublicKey(node_uuid_map[master_node], pub_key,
>> + key_file=pub_key_file)
>> + except IOError:
>> + # Not all types of keys might be existing
>> + pass
>> +
>> + # get the key files of all non-master nodes
>> + for node in nonmaster_nodes:
>> + fetched_keys = ssh.ReadRemoteSshPubKeys(root_keyfiles, node,
>> cluster_name,
>> + ssh_port_map[node],
>> + options.ssh_key_check,
>> + options.ssh_key_check)
>> + for pub_key in fetched_keys.values():
>> + ssh.AddPublicKey(node_uuid_map[node], pub_key,
>> key_file=pub_key_file)
>> +
>> +
>> def RenewCrypto(opts, args):
>> """Renews cluster certificates, keys and secrets.
>>
>> """
>> + if opts.new_ssh_keys:
>> + _BuildGanetiPubKeys(opts)
>> return _RenewCrypto(opts.new_cluster_cert,
>> opts.new_rapi_cert,
>> opts.rapi_cert,
>> @@ -1080,7 +1139,8 @@ def RenewCrypto(opts, args):
>> opts.new_cluster_domain_secret,
>> opts.cluster_domain_secret,
>> opts.force,
>> - opts.new_node_cert)
>> + opts.new_node_cert,
>> + opts.new_ssh_keys)
>>
>>
>> def _GetEnabledDiskTemplates(opts):
>> @@ -2312,7 +2372,7 @@ commands = {
>> NEW_CONFD_HMAC_KEY_OPT, FORCE_OPT,
>> NEW_CLUSTER_DOMAIN_SECRET_OPT, CLUSTER_DOMAIN_SECRET_OPT,
>> NEW_SPICE_CERT_OPT, SPICE_CERT_OPT, SPICE_CACERT_OPT,
>> - NEW_NODE_CERT_OPT],
>> + NEW_NODE_CERT_OPT, NEW_SSH_KEY_OPT, NOSSH_KEYCHECK_OPT],
>> "[opts...]",
>> "Renews cluster certificates, keys and secrets"),
>> "epo": (
>> diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py
>> index 5674b07..493ae87 100644
>> --- a/lib/cmdlib/cluster.py
>> +++ b/lib/cmdlib/cluster.py
>> @@ -99,6 +99,32 @@ class LUClusterRenewCrypto(NoHooksLU):
>>
>> """
>>
>> + REQ_BGL = False
>> +
>> + def ExpandNames(self):
>> + self.needed_locks = {
>> + locking.LEVEL_NODE: locking.ALL_SET,
>> + locking.LEVEL_NODE_RES: locking.ALL_SET,
>> + locking.LEVEL_NODE_ALLOC: locking.ALL_SET,
>> + }
>> + self.share_locks = ShareAll()
>> + self.share_locks[locking.LEVEL_NODE] = 0
>> + self.share_locks[locking.LEVEL_NODE_RES] = 0
>> + self.share_locks[locking.LEVEL_NODE_ALLOC] = 0
>>
>
> I'm not sure if NODE_RES is needed here, AFAIK it should be used for
> resource-intensive operations (but since nobody really knows where NODE_RES
> should be used, my opinion isn't strong about it).
Good point, I think I did not pay too much attention here,but I agree that
it is most probably not justified as changing the SSH keys is not that
IO-load-intensive.
>
>
> +
>> + def CheckPrereq(self):
>> + """Check prerequisites.
>> +
>> + This checks whether the cluster is empty.
>> +
>> + Any errors are signaled by raising errors.OpPrereqError.
>> +
>> + """
>> + self._ssh_renewal_suppressed = False
>> + if not self.cfg.GetClusterInfo().modify_ssh_setup:
>> + if self.op.ssh_keys:
>> + self._ssh_renewal_suppressed = True
>>
>
> I'd suggest
>
> self._ssh_renewal_suppressed = \
> not self.cfg.GetClusterInfo().modify_ssh_setup and self.op.ssh_keys
ACK
>
> +
>> def _RenewNodeSslCertificates(self):
>> """Renews the nodes' SSL certificates.
>>
>> @@ -132,9 +158,35 @@ class LUClusterRenewCrypto(NoHooksLU):
>> self.cfg.RemoveNodeFromCandidateCerts("%s-SERVER" % master_uuid)
>> self.cfg.RemoveNodeFromCandidateCerts("%s-OLDMASTER" % master_uuid)
>>
>> + def _RenewSshKeys(self):
>> + """Renew all nodes' SSH keys.
>> +
>> + """
>> + master_uuid = self.cfg.GetMasterNode()
>> +
>> + nodes = self.cfg.GetAllNodesInfo()
>> + nodes_uuid_names = [(node_uuid, node_info.name) for (node_uuid,
>> node_info)
>> + in nodes.items() if not node_info.offline]
>> + node_names = [name for (_, name) in nodes_uuid_names]
>> + node_uuids = [uuid for (uuid, _) in nodes_uuid_names]
>> + port_map = ssh.GetSshPortMap(node_names, self.cfg)
>> + potential_master_candidates = self.cfg.
>> GetPotentialMasterCandidates()
>> + master_candidate_uuids = self.cfg.GetMasterCandidateUuids()
>> + result = self.rpc.call_node_ssh_keys_renew(
>> + [master_uuid],
>> + node_uuids, node_names, port_map,
>> + master_candidate_uuids,
>> + potential_master_candidates)
>> + result[master_uuid].Raise("Could not renew the SSH keys of all
>> nodes")
>> +
>> def Exec(self, feedback_fn):
>> if self.op.node_certificates:
>> self._RenewNodeSslCertificates()
>> + if self.op.ssh_keys and not self._ssh_renewal_suppressed:
>> + self._RenewSshKeys()
>> + elif self._ssh_renewal_suppressed:
>> + feedback_fn("Cannot renew SSH keys if the cluster is configured to
>> not"
>> + " modify the SSH setup.")
>>
>>
>> class LUClusterActivateMasterIp(NoHooksLU):
>> diff --git a/lib/rpc_defs.py b/lib/rpc_defs.py
>> index ee66da0..b944373 100644
>> --- a/lib/rpc_defs.py
>> +++ b/lib/rpc_defs.py
>> @@ -549,6 +549,13 @@ _NODE_CALLS = [
>> ("master_candidate_uuids", None, "List of UUIDs of master
>> candidates."),
>> ("potential_master_candidates", None, "Potential master
>> candidates")],
>> None, None, "Remove a node's SSH key from the other nodes' key
>> files."),
>> + ("node_ssh_keys_renew", MULTI, None, constants.RPC_TMO_URGENT, [
>> + ("node_uuids", None, "UUIDs of the nodes whose key is renewed"),
>> + ("node_names", None, "Names of the nodes whose key is renewed"),
>> + ("ssh_port_map", None, "Map of nodes' SSH ports to be used for
>> transfers"),
>> + ("master_candidate_uuids", None, "List of UUIDs of master
>> candidates."),
>> + ("potential_master_candidates", None, "Potential master
>> candidates")],
>> + None, None, "Renew all SSH key pairs of all nodes nodes."),
>> ]
>>
>> _MISC_CALLS = [
>> diff --git a/lib/server/noded.py b/lib/server/noded.py
>> index 05af067..05bbc04 100644
>> --- a/lib/server/noded.py
>> +++ b/lib/server/noded.py
>> @@ -922,6 +922,17 @@ class NodeRequestHandler(http.
>> server.HttpServerHandler):
>> ssh_port_map,
>> potential_master_candidates)
>>
>> @staticmethod
>> + def perspective_node_ssh_keys_renew(params):
>> + """Generates a new root SSH key pair on the node.
>> +
>> + """
>> + (node_uuids, node_names, ssh_port_map,
>> + master_candidate_uuids, potential_master_candidates) = params
>> + return backend.RenewSshKeys(node_uuids, node_names, ssh_port_map,
>> + master_candidate_uuids,
>> + potential_master_candidates)
>> +
>> + @staticmethod
>> def perspective_node_ssh_key_remove(params):
>> """Removes a node's SSH key from the other nodes' SSH files.
>>
>> diff --git a/lib/tools/ssh_update.py b/lib/tools/ssh_update.py
>> index 41a9de8..052a2e4 100644
>> --- a/lib/tools/ssh_update.py
>> +++ b/lib/tools/ssh_update.py
>> @@ -53,6 +53,7 @@ _DATA_CHECK = ht.TStrictDict(False, True, {
>> ht.TItems(
>> [ht.TElemOf(constants.SSHS_ACTIONS),
>> ht.TDictOf(ht.TNonEmptyString, ht.TListOf(ht.TNonEmptyString))]),
>> + constants.SSHS_GENERATE: ht.TBool,
>> })
>>
>>
>> @@ -143,11 +144,14 @@ def UpdatePubKeyFile(data, dry_run,
>> key_file=pathutils.SSH_PUB_KEYS):
>> logging.info("This is a dry run, not overriding %s", key_file)
>> else:
>> ssh.OverridePubKeyFile(public_keys, key_file=key_file)
>> - elif action == constants.SSHS_ADD:
>> + elif action in [constants.SSHS_ADD, constants.SSHS_REPLACE_OR_ADD]:
>> if dry_run:
>> - logging.info("This is a dry run, not adding a key to %s",
>> key_file)
>> + logging.info("This is a dry run, not adding or replacing a key to
>> %s",
>> + key_file)
>> else:
>> for uuid, keys in public_keys.items():
>> + if action == constants.SSHS_REPLACE_OR_ADD:
>> + ssh.RemovePublicKey(uuid, key_file=key_file)
>> for key in keys:
>> ssh.AddPublicKey(uuid, key, key_file=key_file)
>> elif action == constants.SSHS_REMOVE:
>> @@ -166,6 +170,23 @@ def UpdatePubKeyFile(data, dry_run,
>> key_file=pathutils.SSH_PUB_KEYS):
>> % action)
>>
>>
>> +def GenerateRootSshKeys(data, dry_run):
>> + """(Re-)generates the root SSH keys.
>> +
>> + @type data: dict
>> + @param data: Input data
>> + @type dry_run: boolean
>> + @param dry_run: Whether to perform a dry run
>> +
>> + """
>> + generate = data.get(constants.SSHS_GENERATE)
>> + if generate:
>> + if dry_run:
>> + logging.info("This is a dry run, not generating any files.")
>> + else:
>> + common.GenerateRootSshKeys(SshUpdateError)
>> +
>> +
>> def Main():
>> """Main routine.
>>
>> @@ -181,9 +202,10 @@ def Main():
>> common.VerifyClusterName(data, SshUpdateError)
>> common.VerifyCertificate(data, SshUpdateError)
>>
>> - # Update SSH files
>> + # Update / Generate SSH files
>> UpdateAuthorizedKeys(data, opts.dry_run)
>> UpdatePubKeyFile(data, opts.dry_run)
>> + GenerateRootSshKeys(data, opts.dry_run)
>>
>> logging.info("Setup finished successfully")
>> except Exception, err: # pylint: disable=W0703
>> diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
>> index 4d6e09a..712d991 100644
>> --- a/qa/qa_cluster.py
>> +++ b/qa/qa_cluster.py
>> @@ -1220,12 +1220,17 @@ def TestClusterRenewCrypto():
>> AssertCommand(["gnt-cluster", "renew-crypto", "--force",
>> "--new-cluster-certificate", "--new-confd-hmac-key",
>> "--new-rapi-certificate",
>> "--new-cluster-domain-secret",
>> - "--new-node-certificates"])
>> + "--new-node-certificates", "--new-ssh-keys",
>> + "--no-ssh-key-check"])
>>
>> # Only renew node certificates
>> AssertCommand(["gnt-cluster", "renew-crypto", "--force",
>> "--new-node-certificates"])
>>
>> + # Only renew SSH keys
>> + AssertCommand(["gnt-cluster", "renew-crypto", "--force",
>> + "--new-ssh-keys", "--no-ssh-key-check"])
>> +
>> # Restore RAPI certificate
>> AssertCommand(["gnt-cluster", "renew-crypto", "--force",
>> "--rapi-certificate=%s" % rapi_cert_backup])
>> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
>> index 4a2db43..6c9f75f 100644
>> --- a/src/Ganeti/Constants.hs
>> +++ b/src/Ganeti/Constants.hs
>> @@ -4487,6 +4487,9 @@ sshsNodeDaemonCertificate =
>> "node_daemon_certificate"
>> sshsAdd :: String
>> sshsAdd = "add"
>>
>> +sshsReplaceOrAdd :: String
>> +sshsReplaceOrAdd = "replace_or_add"
>> +
>> sshsRemove :: String
>> sshsRemove = "remove"
>>
>> @@ -4496,8 +4499,15 @@ sshsOverride = "override"
>> sshsClear :: String
>> sshsClear = "clear"
>>
>> +sshsGenerate :: String
>> +sshsGenerate = "generate"
>> +
>> sshsActions :: FrozenSet String
>> -sshsActions = ConstantUtils.mkSet [sshsAdd, sshsRemove, sshsOverride,
>> sshsClear]
>> +sshsActions = ConstantUtils.mkSet [ sshsAdd
>> + , sshsRemove
>> + , sshsOverride
>> + , sshsClear
>> + , sshsReplaceOrAdd]
>>
>> -- * Key files for SSH daemon
>>
>> diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs
>> index 4ebb222..66c5285 100644
>> --- a/src/Ganeti/OpCodes.hs
>> +++ b/src/Ganeti/OpCodes.hs
>> @@ -269,6 +269,7 @@ $(genOpCode "OpCode"
>> [t| () |],
>> OpDoc.opClusterRenewCrypto,
>> [ pNodeSslCerts
>> + , pSshKeys
>> ],
>> [])
>> , ("OpQuery",
>> diff --git a/src/Ganeti/OpParams.hs b/src/Ganeti/OpParams.hs
>> index f277065..9cac511 100644
>> --- a/src/Ganeti/OpParams.hs
>> +++ b/src/Ganeti/OpParams.hs
>> @@ -285,6 +285,7 @@ module Ganeti.OpParams
>> , pEnableDataCollectors
>> , pDisableDataCollectors
>> , pNodeSslCerts
>> + , pSshKeys
>> ) where
>>
>> import Control.Monad (liftM, mplus)
>> @@ -1850,3 +1851,9 @@ pNodeSslCerts =
>> withDoc "Whether to renew node SSL certificates" .
>> defaultField [| False |] $
>> simpleField "node_certificates" [t| Bool |]
>> +
>> +pSshKeys :: Field
>> +pSshKeys =
>> + withDoc "Whether to renew SSH keys" .
>> + defaultField [| False |] $
>> + simpleField "ssh_keys" [t| Bool |]
>> diff --git a/test/hs/Test/Ganeti/OpCodes.hs
>> b/test/hs/Test/Ganeti/OpCodes.hs
>> index f881731..517b332 100644
>> --- a/test/hs/Test/Ganeti/OpCodes.hs
>> +++ b/test/hs/Test/Ganeti/OpCodes.hs
>> @@ -153,7 +153,8 @@ instance Arbitrary OpCodes.OpCode where
>> "OP_TAGS_DEL" ->
>> arbitraryOpTagsDel
>> "OP_CLUSTER_POST_INIT" -> pure OpCodes.OpClusterPostInit
>> - "OP_CLUSTER_RENEW_CRYPTO" -> pure OpCodes.OpClusterRenewCrypto
>> + "OP_CLUSTER_RENEW_CRYPTO" ->
>> + OpCodes.OpClusterRenewCrypto <$> arbitrary <*> arbitrary
>> "OP_CLUSTER_DESTROY" -> pure OpCodes.OpClusterDestroy
>> "OP_CLUSTER_QUERY" -> pure OpCodes.OpClusterQuery
>> "OP_CLUSTER_VERIFY" ->
>> diff --git a/test/py/ganeti.backend_unittest.py b/test/py/ganeti.backend_
>> unittest.py
>> index 9b18763..5fcc6b5 100755
>> --- a/test/py/ganeti.backend_unittest.py
>> +++ b/test/py/ganeti.backend_unittest.py
>> @@ -941,7 +941,7 @@ class TestSpaceReportingConstants(unittest.TestCase):
>> self.assertEqual(None, backend._STORAGE_TYPE_INFO_FN[storage_type])
>>
>>
>> -class TestAddAndRemoveNodeSshKey(testutils.GanetiTestCase):
>> +class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
>>
>> _CLUSTER_NAME = "mycluster"
>> _SSH_PORT = 22
>> @@ -1024,7 +1024,8 @@ class TestAddAndRemoveNodeSshKey(
>> testutils.GanetiTestCase):
>> expected_key):
>> return self._KeyOperationExecuted(
>> key_data, node_name, expected_type, expected_key,
>> - [constants.SSHS_ADD, constants.SSHS_OVERRIDE])
>> + [constants.SSHS_ADD, constants.SSHS_OVERRIDE,
>> + constants.SSHS_REPLACE_OR_ADD])
>>
>> def _KeyRemoved(self, key_data, node_name, expected_type,
>> expected_key):
>> @@ -1051,6 +1052,25 @@ class TestAddAndRemoveNodeSshKey(
>> testutils.GanetiTestCase):
>> calls_per_node[node].append(data)
>> return calls_per_node
>>
>> + def testGenerateKey(self):
>> + test_node_name = "node_name_7"
>> + test_node_uuid = "node_uuid_7"
>> +
>> + self._SetupTestData()
>> +
>> + backend._GenerateNodeSshKey(test_node_uuid, test_node_name,
>> + self._ssh_port_map,
>> + pub_key_file=self._pub_key_file,
>> + ssconf_store=self._ssconf_mock,
>> + noded_cert_file=self.noded_cert_file,
>> + run_cmd_fn=self._run_cmd_mock)
>> +
>> + calls_per_node = self._GetCallsPerNode()
>> + for node, calls in calls_per_node.items():
>> + self.assertEquals(node, test_node_name)
>> + for call in calls:
>> + self.assertTrue(constants.SSHS_GENERATE in call)
>> +
>> def testAddNodeSshKeyValid(self):
>> new_node_name = "new_node_name"
>> new_node_uuid = "new_node_uuid"
>> diff --git a/test/py/ganeti.client.gnt_cluster_unittest.py b/test/py/
>> ganeti.client.gnt_cluster_unittest.py
>> index b2ce012..7589baa 100755
>> --- a/test/py/ganeti.client.gnt_cluster_unittest.py
>> +++ b/test/py/ganeti.client.gnt_cluster_unittest.py
>> @@ -23,12 +23,17 @@
>>
>> import unittest
>> import optparse
>> +import os
>> +import shutil
>> +import tempfile
>>
>> from ganeti import errors
>> from ganeti.client import gnt_cluster
>> from ganeti import utils
>> from ganeti import compat
>> from ganeti import constants
>> +from ganeti import ssh
>> +from ganeti import cli
>>
>> import mock
>> import testutils
>> @@ -354,5 +359,108 @@ class GetDrbdHelper(DrbdHelperTestCase):
>> self.assertEquals(opts.drbd_helper, helper)
>>
>>
>> +class TestBuildGanetiPubKeys(testutils.GanetiTestCase):
>> +
>> + _SOME_KEY_DICT = {"rsa": "key_rsa",
>> + "dsa": "key_dsa"}
>> + _MASTER_NODE_NAME = "master_node"
>> + _MASTER_NODE_UUID = "master_uuid"
>> + _NUM_NODES = 2 # excluding master node
>> + _ONLINE_NODE_NAMES = ["node%s_name" % i for i in range(_NUM_NODES)]
>> + _ONLINE_NODE_UUIDS = ["node%s_uuid" % i for i in range(_NUM_NODES)]
>> + _CLUSTER_NAME = "cluster_name"
>> + _PRIV_KEY = "master_private_key"
>> + _PUB_KEY = "master_public_key"I think the only thing I told him is
>> that I am not the only one who is annoyh
>> + _AUTH_KEYS = "a\nb\nc"
>> +
>> + def _setUpFakeKeys(self):
>> + os.makedirs(os.path.join(self.tmpdir, ".ssh"))
>> +
>> + for key_type in ["rsa", "dsa"]:
>> + self.priv_filename = os.path.join(self.tmpdir, ".ssh", "id_%s" %
>> key_type)
>> + priv_fd = open(self.priv_filename, 'w')
>> + priv_fd.write(self._PRIV_KEY)
>> + priv_fd.close()
>> +
>> + self.pub_filename = os.path.join(
>> + self.tmpdir, ".ssh", "id_%s.pub" % key_type)
>> + pub_fd = open(self.pub_filename, 'w')
>> + pub_fd.write(self._PUB_KEY)
>> + pub_fd.close()
>> +
>> + self.auth_filename = os.path.join(self.tmpdir, ".ssh",
>> "authorized_keys")
>> + auth_fd = open(self.auth_filename, 'w')
>> + auth_fd.write(self._AUTH_KEYS)
>> + auth_fd.close()
>>
>
> Just a minor point, again we could simplify these (and similar statements
> in the module) with utils.WriteFile (as the module uses utils.ReadFile
> already).
ACK
>
>
> +
>> + def setUp(self):
>> + testutils.GanetiTestCase.setUp(self)
>> + self.tmpdir = tempfile.mkdtemp()
>> + self.pub_key_filename = os.path.join(self.tmpdir,
>> "ganeti_test_pub_keys")
>> + self._setUpFakeKeys()
>> +
>> + self._ssh_read_remote_ssh_pub_keys_patcher = testutils \
>> + .patch_object(ssh, "ReadRemoteSshPubKeys")
>> + 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
>> +
>> + self.mock_cl = mock.Mock()
>> + self.mock_cl.QueryConfigValues = mock.Mock()
>> + self.mock_cl.QueryConfigValues.return_value = \
>> + (self._CLUSTER_NAME, self._MASTER_NODE_NAME)
>> +
>> + self._get_online_nodes_mock = mock.Mock()
>> + self._get_online_nodes_mock.return_value = \
>> + self._ONLINE_NODE_NAMES
>> +
>> + self._get_nodes_ssh_ports_mock = mock.Mock()
>> + self._get_nodes_ssh_ports_mock.return_value = \
>> + [22 for i in range(self._NUM_NODES + 1)]
>> +
>> + self._get_node_uuids_mock = mock.Mock()
>> + self._get_node_uuids_mock.return_value = \
>> + self._ONLINE_NODE_UUIDS + [self._MASTER_NODE_UUID]
>> +
>> + self._options = mock.Mock()
>> + self._options.ssh_key_check = False
>> +
>> + def _GetTempHomedir(self, _):
>> + return self.tmpdir
>> +
>> + def tearDown(self):
>> + super(testutils.GanetiTestCase, self).tearDown()
>> + shutil.rmtree(self.tmpdir)
>> + self._ssh_read_remote_ssh_pub_keys_patcher.stop()
>> +
>> + def testNewPubKeyFile(self):
>> + gnt_cluster._BuildGanetiPubKeys(
>> + self._options,
>> + pub_key_file=self.pub_key_filename,
>> + cl=self.mock_cl,
>> + get_online_nodes_fn=self._get_online_nodes_mock,
>> + get_nodes_ssh_ports_fn=self._get_nodes_ssh_ports_mock,
>> + get_node_uuids_fn=self._get_node_uuids_mock,
>> + homedir_fn=self._GetTempHomedir)
>> + key_file_result = utils.ReadFile(self.pub_key_filename)
>> + for node_uuid in self._ONLINE_NODE_UUIDS + [self._MASTER_NODE_UUID]:
>> + self.assertTrue(node_uuid in key_file_result)
>> + self.assertTrue(self._PUB_KEY in key_file_result)
>> +
>> + def testOverridePubKeyFile(self):
>> + fd = open(self.pub_key_filename, "w")
>> + fd.write("Pink Bunny")
>> + fd.close()
>> + gnt_cluster._BuildGanetiPubKeys(
>> + self._options,
>> + pub_key_file=self.pub_key_filename,
>> + cl=self.mock_cl,
>> + get_online_nodes_fn=self._get_online_nodes_mock,
>> + get_nodes_ssh_ports_fn=self._get_nodes_ssh_ports_mock,
>> + get_node_uuids_fn=self._get_node_uuids_mock,
>> + homedir_fn=self._GetTempHomedir)
>> + self.assertFalse("Pink" in self.pub_key_filename)
>>
>
> This just tests that "Pink" isn't in the filename, doesn't it? Probably it
> should be similar to the previous test - read the file and check the
> content.
It tests whether the "Pink Bunny" key has disappeared. I think checking if
"Pink" is not in there is sufficient. I deliberately did not check the
entire key file, because I want to avoid too pedantic tests that are hard
to maintain, because they would need to be change with any small change in
the code. However, for better readability, I changed it to check for the
entire line "Pink Bunny".
>
>
> +
>> +
>> if __name__ == "__main__":
>> testutils.GanetiTestProgram()
>> diff --git a/test/py/ganeti.mcpu_unittest.py b/test/py/ganeti.mcpu_
>> unittest.py
>> index 02a97ef..27549d8 100755
>> --- a/test/py/ganeti.mcpu_unittest.py
>> +++ b/test/py/ganeti.mcpu_unittest.py
>> @@ -46,7 +46,6 @@ REQ_BGL_WHITELIST = compat.UniqueFrozenset([
>> opcodes.OpClusterDestroy,
>> opcodes.OpClusterPostInit,
>> opcodes.OpClusterRename,
>> - opcodes.OpClusterRenewCrypto,
>>
>
> +1!
>
> opcodes.OpNodeAdd,
>> opcodes.OpNodeRemove,
>> opcodes.OpTestAllocator,
>> diff --git a/tools/post-upgrade b/tools/post-upgrade
>> index 6aec3ff..92ad7b9 100644
>> --- a/tools/post-upgrade
>> +++ b/tools/post-upgrade
>> @@ -48,6 +48,14 @@ def main():
>> if result.failed:
>> cli.ToStderr("Failed to create node certificates: %s; Output %s" %
>> (result.fail_reason, result.output))
>> +
>> + if utils.version.IsBefore(version, 2, 13, 0):
>> + result = utils.RunCmd(["gnt-cluster", "renew-crypto",
>> + "--new-ssh-keys", "--no-ssh-key-check", "-f"])
>> + if result.failed:
>> + cli.ToStderr("Failed to create node certificates: %s; Output %s" %
>> + (result.fail_reason, result.output))
>> +
>> return 0
>>
>> if __name__ == "__main__":
>> --
>> 2.1.0.rc2.206.gedb03e5
>>
>>
>
> Rest LGTM, no need to resend.
>
Even though you said no need to resend, here the interdiff FYI:
diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py
index 35f1d93..0defb61 100644
--- a/lib/cmdlib/cluster.py
+++ b/lib/cmdlib/cluster.py
@@ -113,12 +113,10 @@ class LUClusterRenewCrypto(NoHooksLU):
def ExpandNames(self):
self.needed_locks = {
locking.LEVEL_NODE: locking.ALL_SET,
- locking.LEVEL_NODE_RES: locking.ALL_SET,
locking.LEVEL_NODE_ALLOC: locking.ALL_SET,
}
self.share_locks = ShareAll()
self.share_locks[locking.LEVEL_NODE] = 0
- self.share_locks[locking.LEVEL_NODE_RES] = 0
self.share_locks[locking.LEVEL_NODE_ALLOC] = 0
def CheckPrereq(self):
@@ -129,10 +127,8 @@ class LUClusterRenewCrypto(NoHooksLU):
Any errors are signaled by raising errors.OpPrereqError.
"""
- self._ssh_renewal_suppressed = False
- if not self.cfg.GetClusterInfo().modify_ssh_setup:
- if self.op.ssh_keys:
- self._ssh_renewal_suppressed = True
+ self._ssh_renewal_suppressed = \
+ not self.cfg.GetClusterInfo().modify_ssh_setup and self.op.ssh_keys
def _RenewNodeSslCertificates(self):
"""Renews the nodes' SSL certificates.
diff --git a/test/py/ganeti.client.gnt_cluster_unittest.py b/test/py/
ganeti.client.gnt_cluster_unittest.py
index 95b29fa..be28eb2 100755
--- a/test/py/ganeti.client.gnt_cluster_unittest.py
+++ b/test/py/ganeti.client.gnt_cluster_unittest.py
@@ -387,20 +387,14 @@ class
TestBuildGanetiPubKeys(testutils.GanetiTestCase):
for key_type in ["rsa", "dsa"]:
self.priv_filename = os.path.join(self.tmpdir, ".ssh", "id_%s" %
key_type)
- priv_fd = open(self.priv_filename, 'w')
- priv_fd.write(self._PRIV_KEY)
- priv_fd.close()
+ utils.WriteFile(self.priv_filename, data=self._PRIV_KEY)
self.pub_filename = os.path.join(
self.tmpdir, ".ssh", "id_%s.pub" % key_type)
- pub_fd = open(self.pub_filename, 'w')
- pub_fd.write(self._PUB_KEY)
- pub_fd.close()
+ utils.WriteFile(self.pub_filename, data=self._PUB_KEY)
self.auth_filename = os.path.join(self.tmpdir, ".ssh",
"authorized_keys")
- auth_fd = open(self.auth_filename, 'w')
- auth_fd.write(self._AUTH_KEYS)
- auth_fd.close()
+ utils.WriteFile(self.auth_filename, data=self._AUTH_KEYS)
def setUp(self):
testutils.GanetiTestCase.setUp(self)
@@ -468,7 +462,7 @@ class TestBuildGanetiPubKeys(testutils.GanetiTestCase):
get_nodes_ssh_ports_fn=self._get_nodes_ssh_ports_mock,
get_node_uuids_fn=self._get_node_uuids_mock,
homedir_fn=self._GetTempHomedir)
- self.assertFalse("Pink" in self.pub_key_filename)
+ self.assertFalse("Pink Bunny" in self.pub_key_filename)
if __name__ == "__main__":
--
Helga Velroyen | Software Engineer | hel...@google.com |
Google Germany GmbH
Dienerstr. 12
80331 München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
