On Fri, Sep 26, 2014 at 4:26 PM, Petr Pudlak <[email protected]> wrote:
> On Tue, Sep 02, 2014 at 04:19:42PM +0200, 'Helga Velroyen' via > ganeti-devel wrote: > >> This adds an option to 'InitSSHSetup' to not override >> the SSH key, but create an additional one with a suffix. >> This will be used to replace the master node's SSH key, >> but keeping the old one a little longer to distribute the >> new one. >> >> Signed-off-by: Helga Velroyen <[email protected]> >> --- >> lib/ssh.py | 13 +++++++++---- >> test/py/ganeti.ssh_unittest.py | 8 ++++++++ >> 2 files changed, 17 insertions(+), 4 deletions(-) >> >> diff --git a/lib/ssh.py b/lib/ssh.py >> index 7e2f64c..3ede50e 100644 >> --- a/lib/ssh.py >> +++ b/lib/ssh.py >> @@ -636,7 +636,8 @@ def QueryPubKeyFile(target_uuids, >> key_file=pathutils.SSH_PUB_KEYS, >> return result >> >> >> -def InitSSHSetup(error_fn=errors.OpPrereqError, _homedir_fn=None): >> +def InitSSHSetup(error_fn=errors.OpPrereqError, _homedir_fn=None, >> + _suffix=""): >> """Setup the SSH configuration for the node. >> >> This generates a dsa keypair for root, adds the pub key to the >> @@ -649,16 +650,20 @@ def InitSSHSetup(error_fn=errors.OpPrereqError, >> _homedir_fn=None): >> for name in priv_key, pub_key: >> if os.path.exists(name): >> utils.CreateBackup(name) >> - utils.RemoveFile(name) >> + if len(_suffix) == 0: >> + utils.RemoveFile(name) >> > > I'm merely curious, why are we removing the files in the first place, and > only when we have no suffix? They'll get overwritten in any case, and how > are these two cases (yes/no suffix) different? I am sorry I did not elaborate on this before, this is actually a part that is not yet used, but needed for creating a new master key. The idea is that if we create a new master key, we need to keep the old one long enough to distribute the new one (otherwise we would have locked the master out). In this case, we create the new key with a suffix, but keep the old one until all nodes have the new key in their 'authorized_keys' file. Only after that has happened we can remove the old key. Therefore, in the function that creates the new key, we cannot remove the old key (yet), which is why this removal line is only executed if we don't have a suffix (thus if we are replacing other keys than the master key). I hope this clarifies the intention here. > > > + >> + new_priv_key_name = priv_key + _suffix >> + new_pub_key_name = priv_key + _suffix + ".pub" >> >> result = utils.RunCmd(["ssh-keygen", "-t", "dsa", >> - "-f", priv_key, >> + "-f", new_priv_key_name, >> "-q", "-N", ""]) >> if result.failed: >> raise error_fn("Could not generate ssh keypair, error %s" % >> result.output) >> >> - AddAuthorizedKey(auth_keys, utils.ReadFile(pub_key)) >> + AddAuthorizedKey(auth_keys, utils.ReadFile(new_pub_key_name)) >> >> >> def InitPubKeyFile(master_uuid, key_file=pathutils.SSH_PUB_KEYS): >> diff --git a/test/py/ganeti.ssh_unittest.py b/test/py/ >> ganeti.ssh_unittest.py >> index 635426f..1826588 100755 >> --- a/test/py/ganeti.ssh_unittest.py >> +++ b/test/py/ganeti.ssh_unittest.py >> @@ -447,6 +447,14 @@ class TestGetUserFiles(testutils.GanetiTestCase): >> self.assertFileContentNotEqual(self.priv_filename, self._PRIV_KEY) >> self.assertFileContentNotEqual(self.pub_filename, self._PUB_KEY) >> >> + def testSuffix(self): >> + suffix = "_pinkbunny" >> + ssh.InitSSHSetup(_homedir_fn=self._GetTempHomedir, _suffix=suffix) >> + self.assertFileContent(self.priv_filename, self._PRIV_KEY) >> + self.assertFileContent(self.pub_filename, self._PUB_KEY) >> + self.assertTrue(os.path.exists(self.priv_filename + suffix)) >> + self.assertTrue(os.path.exists(self.priv_filename + suffix + >> ".pub")) >> + >> >> if __name__ == "__main__": >> testutils.GanetiTestProgram() >> -- >> 2.1.0.rc2.206.gedb03e5 >> >> > Rest LGTM > Cheers, Helga -- Helga Velroyen | Software Engineer | [email protected] | 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
