On Fri, 13 Nov 2015 at 11:18 'Hrvoje Ribicic' via ganeti-devel < [email protected]> wrote:
> By explicitly specifying the old and new SSH key type in various > renew-crypto operations, this patch allows the switching of SSH key > types to take place during a SSH key renewal operation. > Nit: I find the phrasing "in various renew-crypto operations' kind of odd, as it is only the 'renew-crypto --new-ssh-keys' that is affected, right? > > Signed-off-by: Hrvoje Ribicic <[email protected]> > --- > lib/backend.py | 28 +++++++++++++----------- > lib/client/gnt_cluster.py | 17 +++++++++++---- > lib/cmdlib/cluster/__init__.py | 49 > +++++++++++++++++++++++++----------------- > lib/rpc_defs.py | 5 +++-- > lib/server/noded.py | 7 +++--- > src/Ganeti/OpCodes.hs | 4 +++- > src/Ganeti/OpParams.hs | 20 +++++++++++++---- > src/Ganeti/Rpc.hs | 12 +++++------ > test/hs/Test/Ganeti/OpCodes.hs | 9 ++++++-- > 9 files changed, 97 insertions(+), 54 deletions(-) > > diff --git a/lib/backend.py b/lib/backend.py > index 65cb976..e20d45a 100644 > --- a/lib/backend.py > +++ b/lib/backend.py > @@ -1885,7 +1885,8 @@ def _ReplaceMasterKeyOnMaster(root_keyfiles): > > > def RenewSshKeys(node_uuids, node_names, master_candidate_uuids, > - potential_master_candidates, ssh_key_type, ssh_key_bits, > + potential_master_candidates, old_key_type, new_key_type, > + new_key_bits, > ganeti_pub_keys_file=pathutils.SSH_PUB_KEYS, > ssconf_store=None, > noded_cert_file=pathutils.NODED_CERT_FILE, > @@ -1900,10 +1901,12 @@ def RenewSshKeys(node_uuids, node_names, > master_candidate_uuids, > @type master_candidate_uuids: list of str > @param master_candidate_uuids: list of UUIDs of master candidates or > master node > - @type ssh_key_type: One of L{constants.SSHK_ALL} > - @param ssh_key_type: the type of SSH key to be generated > - @type ssh_key_bits: int > - @param ssh_key_bits: the length of the key to be generated > + @type old_key_type: One of L{constants.SSHK_ALL} > + @param old_key_type: the type of SSH key already present on nodes > + @type new_key_type: One of L{constants.SSHK_ALL} > + @param new_key_type: the type of SSH key to be generated > + @type new_key_bits: int > + @param new_key_bits: the length of the key to be generated > @type ganeti_pub_keys_file: str > @param ganeti_pub_keys_file: file path of the the public key file > @type noded_cert_file: str > @@ -1927,8 +1930,9 @@ def RenewSshKeys(node_uuids, node_names, > master_candidate_uuids, > > (_, root_keyfiles) = \ > ssh.GetAllUserFiles(constants.SSH_LOGIN_USER, mkdir=False, > dircheck=False) > - (_, node_pub_keyfile) = root_keyfiles[ssh_key_type] > - old_master_key = utils.ReadFile(node_pub_keyfile) > + (_, old_pub_keyfile) = root_keyfiles[old_key_type] > + (_, new_pub_keyfile) = root_keyfiles[new_key_type] > + old_master_key = utils.ReadFile(old_pub_keyfile) > > node_uuid_name_map = zip(node_uuids, node_names) > > @@ -1956,7 +1960,7 @@ 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(node_pub_keyfile, > + old_pub_key = ssh.ReadRemoteSshPubKeys(old_pub_keyfile, > node_name, cluster_name, > ssh_port_map[node_name], > False, # ask_key > @@ -1981,15 +1985,15 @@ def RenewSshKeys(node_uuids, node_names, > master_candidate_uuids, > " key. Not deleting that key on the node.", > node_name) > > logging.debug("Generating new SSH key for node '%s'.", node_name) > - _GenerateNodeSshKey(node_uuid, node_name, ssh_port_map, ssh_key_type, > - ssh_key_bits, pub_key_file=ganeti_pub_keys_file, > + _GenerateNodeSshKey(node_uuid, node_name, ssh_port_map, new_key_type, > + new_key_bits, pub_key_file=ganeti_pub_keys_file, > ssconf_store=ssconf_store, > noded_cert_file=noded_cert_file, > run_cmd_fn=run_cmd_fn) > > try: > logging.debug("Fetching newly created SSH key from node '%s'.", > node_name) > - pub_key = ssh.ReadRemoteSshPubKeys(node_pub_keyfile, > + pub_key = ssh.ReadRemoteSshPubKeys(new_pub_keyfile, > node_name, cluster_name, > ssh_port_map[node_name], > False, # ask_key > @@ -2024,7 +2028,7 @@ def RenewSshKeys(node_uuids, node_names, > master_candidate_uuids, > # Generate a new master key with a suffix, don't touch the old one for > now > logging.debug("Generate new ssh key of master.") > _GenerateNodeSshKey(master_node_uuid, master_node_name, ssh_port_map, > - ssh_key_type, ssh_key_bits, > + new_key_type, new_key_bits, > pub_key_file=ganeti_pub_keys_file, > ssconf_store=ssconf_store, > noded_cert_file=noded_cert_file, > diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py > index c7f9048..59f1eb9 100644 > --- a/lib/client/gnt_cluster.py > +++ b/lib/client/gnt_cluster.py > @@ -978,11 +978,12 @@ def _ReadAndVerifyCert(cert_filename, > verify_private_key=False): > return pem > > > +# pylint: disable=R0913 > 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, new_ssh_keys, > - verbose, debug): > + ssh_key_type, ssh_key_bits, verbose, debug): > """Renews cluster certificates, keys and secrets. > > @type new_cluster_cert: bool > @@ -1010,10 +1011,14 @@ def _RenewCrypto(new_cluster_cert, new_rapi_cert, > # pylint: disable=R0911 > @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 > + @type ssh_key_type: One of L{constants.SSHK_ALL} > + @param ssh_key_type: The type of SSH key to be generated > + @type ssh_key_bits: int > + @param ssh_key_bits: The length of the key to be generated > @type verbose: boolean > - @param verbose: show verbose output > + @param verbose: Show verbose output > @type debug: boolean > - @param debug: show debug output > + @param debug: Show debug output > > """ > ToStdout("Updating certificates now. Running \"gnt-cluster verify\" " > @@ -1194,7 +1199,9 @@ def _RenewCrypto(new_cluster_cert, new_rapi_cert, # > pylint: disable=R0911 > cl = GetClient() > renew_op = opcodes.OpClusterRenewCrypto( > node_certificates=new_node_cert or new_cluster_cert, > - ssh_keys=new_ssh_keys) > + renew_ssh_keys=new_ssh_keys, > + ssh_key_type=ssh_key_type, > + ssh_key_bits=ssh_key_bits) > SubmitOpCode(renew_op, cl=cl) > > ToStdout("All requested certificates and keys have been replaced." > @@ -1269,6 +1276,8 @@ def RenewCrypto(opts, args): > opts.force, > opts.new_node_cert, > opts.new_ssh_keys, > + opts.ssh_key_type, > + opts.ssh_key_bits, > opts.verbose, > opts.debug > 0) > > diff --git a/lib/cmdlib/cluster/__init__.py > b/lib/cmdlib/cluster/__init__.py > index ed6a3b8..5658646 100644 > --- a/lib/cmdlib/cluster/__init__.py > +++ b/lib/cmdlib/cluster/__init__.py > @@ -87,17 +87,6 @@ class LUClusterRenewCrypto(NoHooksLU): > self.share_locks = ShareAll() > self.share_locks[locking.LEVEL_NODE] = 0 > > - def CheckPrereq(self): > - """Check prerequisites. > - > - This checks whether the cluster is empty. > - > - Any errors are signaled by raising errors.OpPrereqError. > - > - """ > - self._ssh_renewal_suppressed = \ > - not self.cfg.GetClusterInfo().modify_ssh_setup and self.op.ssh_keys > - > This looks odd to me. Isn't it still necessary to check whether or not renewal is suppressed? Did you run a QA with --no-ssh-init? > def _RenewNodeSslCertificates(self, feedback_fn): > """Renews the nodes' SSL certificates. > > @@ -159,9 +148,12 @@ class LUClusterRenewCrypto(NoHooksLU): > > self.cfg.SetCandidateCerts(digest_map) > > - def _RenewSshKeys(self): > + def _RenewSshKeys(self, feedback_fn): > """Renew all nodes' SSH keys. > > + @type feedback_fn: function > + @param feedback_fn: logging function, see > L{ganeti.cmdlist.base.LogicalUnit} > + > """ > master_uuid = self.cfg.GetMasterNode() > > @@ -175,25 +167,42 @@ class LUClusterRenewCrypto(NoHooksLU): > > cluster_info = self.cfg.GetClusterInfo() > > + new_ssh_key_type = self.op.ssh_key_type > + if new_ssh_key_type is None: > + new_ssh_key_type = cluster_info.ssh_key_type > + > + new_ssh_key_bits = self.op.ssh_key_bits > + if new_ssh_key_bits is None: > + new_ssh_key_bits = cluster_info.ssh_key_bits > + > result = self.rpc.call_node_ssh_keys_renew( > [master_uuid], > node_uuids, node_names, > master_candidate_uuids, > potential_master_candidates, > - cluster_info.ssh_key_type, > - cluster_info.ssh_key_bits) > + cluster_info.ssh_key_type, # Old key type > + new_ssh_key_type, # New key type > + new_ssh_key_bits) # New key bits > result[master_uuid].Raise("Could not renew the SSH keys of all nodes") > > + # After the keys have been successfully swapped, time to commit the > change > + # in key type > + cluster_info.ssh_key_type = new_ssh_key_type > + cluster_info.ssh_key_bits = new_ssh_key_bits > + self.cfg.Update(cluster_info, feedback_fn) > + > def Exec(self, feedback_fn): > if self.op.node_certificates: > feedback_fn("Renewing Node SSL certificates") > self._RenewNodeSslCertificates(feedback_fn) > - if self.op.ssh_keys and not self._ssh_renewal_suppressed: > - feedback_fn("Renewing SSH keys") > - self._RenewSshKeys() > - elif self._ssh_renewal_suppressed: > - feedback_fn("Cannot renew SSH keys if the cluster is configured to > not" > - " modify the SSH setup.") > + > + if self.op.renew_ssh_keys: > see above, we should still respect the modify_ssh_setup option. > + if self.cfg.GetClusterInfo().modify_ssh_setup: > + feedback_fn("Renewing SSH keys") > + self._RenewSshKeys(feedback_fn) > + else: > + 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 88f0ece..6337285 100644 > --- a/lib/rpc_defs.py > +++ b/lib/rpc_defs.py > @@ -567,8 +567,9 @@ _NODE_CALLS = [ > ("node_names", None, "Names of the nodes whose key is renewed"), > ("master_candidate_uuids", None, "List of UUIDs of master > candidates."), > ("potential_master_candidates", None, "Potential master candidates"), > - ("ssh_key_type", None, "The type of key to generate"), > - ("ssh_key_bits", None, "The length of the key to generate")], > + ("old_key_type", None, "The type of key previously used"), > + ("new_key_type", None, "The type of key to generate"), > + ("new_key_bits", None, "The length of the key to generate")], > None, None, "Renew all SSH key pairs of all nodes nodes."), > ] > > diff --git a/lib/server/noded.py b/lib/server/noded.py > index dddf493..eb7eb75 100644 > --- a/lib/server/noded.py > +++ b/lib/server/noded.py > @@ -946,10 +946,11 @@ class > NodeRequestHandler(http.server.HttpServerHandler): > > """ > (node_uuids, node_names, master_candidate_uuids, > - potential_master_candidates, ssh_key_type, ssh_key_bits) = params > + potential_master_candidates, old_key_type, new_key_type, > + new_key_bits) = params > return backend.RenewSshKeys(node_uuids, node_names, > master_candidate_uuids, > - potential_master_candidates, ssh_key_type, > - ssh_key_bits) > + potential_master_candidates, old_key_type, > + new_key_type, new_key_bits) > > @staticmethod > def perspective_node_ssh_key_remove(params): > diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs > index 70fde67..66094a2 100644 > --- a/src/Ganeti/OpCodes.hs > +++ b/src/Ganeti/OpCodes.hs > @@ -281,7 +281,9 @@ $(genOpCode "OpCode" > [t| () |], > OpDoc.opClusterRenewCrypto, > [ pNodeSslCerts > - , pSshKeys > + , pRenewSshKeys > + , pSshKeyType > + , pSshKeyBits > , pVerbose > , pDebug > ], > diff --git a/src/Ganeti/OpParams.hs b/src/Ganeti/OpParams.hs > index 08c4be8..0a793c0 100644 > --- a/src/Ganeti/OpParams.hs > +++ b/src/Ganeti/OpParams.hs > @@ -299,7 +299,9 @@ module Ganeti.OpParams > , pEnabledDataCollectors > , pDataCollectorInterval > , pNodeSslCerts > - , pSshKeys > + , pSshKeyBits > + , pSshKeyType > + , pRenewSshKeys > , pNodeSetup > , pVerifyClutter > , pLongSleep > @@ -1895,11 +1897,21 @@ pNodeSslCerts = > defaultField [| False |] $ > simpleField "node_certificates" [t| Bool |] > > -pSshKeys :: Field > -pSshKeys = > +pSshKeyBits :: Field > +pSshKeyBits = > + withDoc "The number of bits of the SSH key Ganeti uses" . > + optionalField $ simpleField "ssh_key_bits" [t| Positive Int |] > + > +pSshKeyType :: Field > +pSshKeyType = > + withDoc "The type of the SSH key Ganeti uses" . > + optionalField $ simpleField "ssh_key_type" [t| SshKeyType |] > + > +pRenewSshKeys :: Field > +pRenewSshKeys = > withDoc "Whether to renew SSH keys" . > defaultField [| False |] $ > - simpleField "ssh_keys" [t| Bool |] > + simpleField "renew_ssh_keys" [t| Bool |] > > pNodeSetup :: Field > pNodeSetup = > diff --git a/src/Ganeti/Rpc.hs b/src/Ganeti/Rpc.hs > index a43cc52..3fb128a 100644 > --- a/src/Ganeti/Rpc.hs > +++ b/src/Ganeti/Rpc.hs > @@ -650,9 +650,9 @@ instance Rpc RpcCallExportList RpcResultExportList > where > rpcResultFill _ res = fromJSValueToRes res RpcResultExportList > > -- ** Job Queue Replication > - > + > -- | Update a job queue file > - > + > $(buildObject "RpcCallJobqueueUpdate" "rpcCallJobqueueUpdate" > [ simpleField "file_name" [t| String |] > , simpleField "content" [t| String |] > @@ -702,9 +702,9 @@ instance Rpc RpcCallJobqueueRename > RpcResultJobqueueRename where > $ JsonDecodeError ("Expected JSNull, got " ++ show (pp_value > res)) > > -- ** Watcher Status Update > - > + > -- | Set the watcher status > - > + > $(buildObject "RpcCallSetWatcherPause" "rpcCallSetWatcherPause" > [ optionalField $ timeAsDoubleField "time" > ]) > @@ -724,9 +724,9 @@ instance Rpc RpcCallSetWatcherPause > RpcResultSetWatcherPause where > ("Expected JSNull, got " ++ show (pp_value res)) > > -- ** Queue drain status > - > + > -- | Set the queu drain flag > - > + > $(buildObject "RpcCallSetDrainFlag" "rpcCallSetDrainFlag" > [ simpleField "value" [t| Bool |] > ]) > diff --git a/test/hs/Test/Ganeti/OpCodes.hs > b/test/hs/Test/Ganeti/OpCodes.hs > index 17c361a..d529576 100644 > --- a/test/hs/Test/Ganeti/OpCodes.hs > +++ b/test/hs/Test/Ganeti/OpCodes.hs > @@ -168,8 +168,13 @@ instance Arbitrary OpCodes.OpCode where > "OP_TAGS_DEL" -> > arbitraryOpTagsDel > "OP_CLUSTER_POST_INIT" -> pure OpCodes.OpClusterPostInit > - "OP_CLUSTER_RENEW_CRYPTO" -> OpCodes.OpClusterRenewCrypto <$> > - arbitrary <*> arbitrary <*> arbitrary <*> arbitrary > + "OP_CLUSTER_RENEW_CRYPTO" -> OpCodes.OpClusterRenewCrypto > + <$> arbitrary -- Node SSL certificates > + <*> arbitrary -- renew_ssh_keys > + <*> arbitrary -- ssh_key_type > + <*> arbitrary -- ssh_key_bits > + <*> arbitrary -- verbose > + <*> arbitrary -- debug > "OP_CLUSTER_DESTROY" -> pure OpCodes.OpClusterDestroy > "OP_CLUSTER_QUERY" -> pure OpCodes.OpClusterQuery > "OP_CLUSTER_VERIFY" -> > -- > 2.6.0.rc2.230.g3dd15c0 > > -- Helga Velroyen Software Engineer [email protected] Google Germany GmbH Dienerstraße 12 80331 München Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind, leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank. This e-mail is confidential. If you are not the right addressee please do not forward it, please inform the sender, and please erase this e-mail including any attachments. Thanks.
