LGTM, thanks On Thu, Sep 25, 2014 at 11:05 AM, 'Helga Velroyen' via ganeti-devel < [email protected]> wrote:
> This patch initializes the "ganeti_pub_keys" file on > cluster initialization and adds the master's key to it. > On node-add, the key file is queried for the keys of > the master candidates and those are transferred to the > new node and added to its "authorized_keys" file. > > Signed-off-by: Helga Velroyen <[email protected]> > --- > lib/bootstrap.py | 4 ++ > lib/client/gnt_node.py | 15 ++++++- > lib/ssh.py | 48 > ++++++++++++++++++---- > lib/tools/prepare_node_join.py | 40 +++++++++++------- > src/Ganeti/Constants.hs | 3 ++ > test/py/ganeti.ssh_unittest.py | 47 > +++++++++++++++++++++ > test/py/ganeti.tools.prepare_node_join_unittest.py | 14 ++++++- > 7 files changed, 145 insertions(+), 26 deletions(-) > > diff --git a/lib/bootstrap.py b/lib/bootstrap.py > index aca3d78..dba7dd4 100644 > --- a/lib/bootstrap.py > +++ b/lib/bootstrap.py > @@ -36,6 +36,7 @@ import os > import os.path > import re > import logging > +import tempfile > import time > import tempfile > > @@ -863,6 +864,9 @@ def InitCluster(cluster_name, mac_prefix, # pylint: > disable=R0913, R0914 > cfg.Update(cfg.GetClusterInfo(), logging.error) > ssconf.WriteSsconfFiles(cfg.GetSsconfValues()) > > + master_uuid = cfg.GetMasterNode() > + if modify_ssh_setup: > + ssh.InitPubKeyFile(master_uuid) > # set up the inter-node password and certificate > _InitGanetiServerSetup(hostname.name) > > diff --git a/lib/client/gnt_node.py b/lib/client/gnt_node.py > index 4f89da0..6031361 100644 > --- a/lib/client/gnt_node.py > +++ b/lib/client/gnt_node.py > @@ -192,7 +192,7 @@ def _ReadSshKeys(keyfiles, _tostderr_fn=ToStderr): > return result > > > -def _SetupSSH(options, cluster_name, node, ssh_port): > +def _SetupSSH(options, cluster_name, node, ssh_port, cl): > """Configures a destination node's SSH daemon. > > @param options: Command line options > @@ -202,8 +202,18 @@ def _SetupSSH(options, cluster_name, node, ssh_port): > @param node: Destination node name > @type ssh_port: int > @param ssh_port: Destination node ssh port > + @param cl: luxi client > > """ > + # Retrieve the list of master and master candidates > + candidate_filter = ["|", ["=", "role", "M"], ["=", "role", "C"]] > + result = cl.Query(constants.QR_NODE, ["uuid"], candidate_filter) > + if len(result.data) < 1: > + raise errors.OpPrereqError("No master or master candidate nodes are" > + " found.") > + candidates = [uuid for (_, uuid) in result.data[0]] > + candidate_keys = ssh.QueryPubKeyFile(candidates) > + > if options.force_join: > ToStderr("The \"--force-join\" option is no longer supported and will > be" > " ignored.") > @@ -223,6 +233,7 @@ def _SetupSSH(options, cluster_name, node, ssh_port): > constants.SSHS_NODE_DAEMON_CERTIFICATE: cert_pem, > constants.SSHS_SSH_HOST_KEY: host_keys, > constants.SSHS_SSH_ROOT_KEY: root_keys, > + constants.SSHS_SSH_AUTHORIZED_KEYS: candidate_keys, > } > > bootstrap.RunNodeSetupCmd(cluster_name, node, > pathutils.PREPARE_NODE_JOIN, > @@ -298,7 +309,7 @@ def AddNode(opts, args): > "and grant full intra-cluster ssh root access to/from it\n", > node) > > if opts.node_setup: > - _SetupSSH(opts, cluster_name, node, ssh_port) > + _SetupSSH(opts, cluster_name, node, ssh_port, cl) > > bootstrap.SetupNodeDaemon(opts, cluster_name, node, ssh_port) > > diff --git a/lib/ssh.py b/lib/ssh.py > index b611f75..5e81f7b 100644 > --- a/lib/ssh.py > +++ b/lib/ssh.py > @@ -141,16 +141,16 @@ def _SplitSshKey(key): > return (True, parts) > > > -def AddAuthorizedKey(file_obj, key): > - """Adds an SSH public key to an authorized_keys file. > +def AddAuthorizedKeys(file_obj, keys): > + """Adds a list of SSH public key to an authorized_keys file. > > @type file_obj: str or file handle > @param file_obj: path to authorized_keys file > - @type key: str > - @param key: string containing key > + @type keys: list of str > + @param keys: list of strings containing keys > > """ > - key_fields = _SplitSshKey(key) > + key_field_list = [(key, _SplitSshKey(key)) for key in keys] > > if isinstance(file_obj, basestring): > f = open(file_obj, "a+") > @@ -161,19 +161,34 @@ def AddAuthorizedKey(file_obj, key): > nl = True > for line in f: > # Ignore whitespace changes > - if _SplitSshKey(line) == key_fields: > - break > + line_key = _SplitSshKey(line) > + key_field_list[:] = [(key, split_key) for (key, split_key) > + in key_field_list > + if split_key != line_key] > nl = line.endswith("\n") > else: > if not nl: > f.write("\n") > - f.write(key.rstrip("\r\n")) > - f.write("\n") > + for (key, _) in key_field_list: > + f.write(key.rstrip("\r\n")) > + f.write("\n") > f.flush() > finally: > f.close() > > > +def AddAuthorizedKey(file_obj, key): > + """Adds an SSH public key to an authorized_keys file. > + > + @type file_obj: str or file handle > + @param file_obj: path to authorized_keys file > + @type key: str > + @param key: string containing key > + > + """ > + AddAuthorizedKeys(file_obj, [key]) > + > + > def RemoveAuthorizedKey(file_name, key): > """Removes an SSH public key from an authorized_keys file. > > @@ -578,6 +593,21 @@ def InitSSHSetup(error_fn=errors.OpPrereqError): > AddAuthorizedKey(auth_keys, utils.ReadFile(pub_key)) > > > +def InitPubKeyFile(master_uuid, key_file=pathutils.SSH_PUB_KEYS): > + """Creates the public key file and adds the master node's SSH key. > + > + @type master_uuid: str > + @param master_uuid: the master node's UUID > + @type key_file: str > + @param key_file: name of the file containing the public keys > + > + """ > + _, pub_key, _ = GetUserFiles(constants.SSH_LOGIN_USER) > + utils.WriteFile(key_file, data="", mode=0600) > + key = utils.ReadFile(pub_key) > + AddPublicKey(master_uuid, key, key_file=key_file) > + > + > class SshRunner: > """Wrapper for SSH commands. > > diff --git a/lib/tools/prepare_node_join.py > b/lib/tools/prepare_node_join.py > index d913f81..e772f8e 100644 > --- a/lib/tools/prepare_node_join.py > +++ b/lib/tools/prepare_node_join.py > @@ -64,6 +64,8 @@ _DATA_CHECK = ht.TStrictDict(False, True, { > constants.SSHS_NODE_DAEMON_CERTIFICATE: ht.TNonEmptyString, > constants.SSHS_SSH_HOST_KEY: _SSH_KEY_LIST, > constants.SSHS_SSH_ROOT_KEY: _SSH_KEY_LIST, > + constants.SSHS_SSH_AUTHORIZED_KEYS: > + ht.TDictOf(ht.TNonEmptyString, ht.TListOf(ht.TNonEmptyString)), > }) > > > @@ -222,20 +224,30 @@ def UpdateSshRoot(data, dry_run, _homedir_fn=None): > > """ > keys = data.get(constants.SSHS_SSH_ROOT_KEY) > - if not keys: > - return > - > - (auth_keys_file, keyfiles) = \ > - ssh.GetAllUserFiles(constants.SSH_LOGIN_USER, mkdir=True, > - _homedir_fn=_homedir_fn) > - > - _UpdateKeyFiles(keys, dry_run, keyfiles) > - > - if dry_run: > - logging.info("This is a dry run, not modifying %s", auth_keys_file) > - else: > - for (_, _, public_key) in keys: > - ssh.AddAuthorizedKey(auth_keys_file, public_key) > + authorized_keys = data.get(constants.SSHS_SSH_AUTHORIZED_KEYS) > + > + if keys or authorized_keys: > + (auth_keys_file, keyfiles) = \ > + ssh.GetAllUserFiles(constants.SSH_LOGIN_USER, mkdir=True, > + _homedir_fn=_homedir_fn) > + > + if keys: > + _UpdateKeyFiles(keys, dry_run, keyfiles) > + > + if dry_run: > + logging.info("This is a dry run, not modifying %s", > auth_keys_file) > + else: > + for (_, _, public_key) in keys: > + ssh.AddAuthorizedKey(auth_keys_file, public_key) > + > + if authorized_keys: > + if dry_run: > + logging.info("This is a dry run, not modifying %s", > auth_keys_file) > + else: > + all_authorized_keys = [] > + for keys in authorized_keys.values(): > + all_authorized_keys += keys > + ssh.AddAuthorizedKeys(auth_keys_file, all_authorized_keys) > > > def LoadData(raw): > diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs > index e233349..fe96281 100644 > --- a/src/Ganeti/Constants.hs > +++ b/src/Ganeti/Constants.hs > @@ -4523,6 +4523,9 @@ sshsSshHostKey = "ssh_host_key" > sshsSshRootKey :: String > sshsSshRootKey = "ssh_root_key" > > +sshsSshAuthorizedKeys :: String > +sshsSshAuthorizedKeys = "authorized_keys" > + > sshsNodeDaemonCertificate :: String > sshsNodeDaemonCertificate = "node_daemon_certificate" > > diff --git a/test/py/ganeti.ssh_unittest.py b/test/py/ > ganeti.ssh_unittest.py > index cc9f543..5ce6074 100755 > --- a/test/py/ganeti.ssh_unittest.py > +++ b/test/py/ganeti.ssh_unittest.py > @@ -184,6 +184,33 @@ class TestSshKeys(testutils.GanetiTestCase): > " ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22 root@key-b\n" > "ssh-dss AAAAB3NzaC1kc3MAAACB root@test\n") > > + def testAddingDuplicateKeys(self): > + ssh.AddAuthorizedKey(self.tmpname, > + "ssh-dss AAAAB3NzaC1kc3MAAACB root@test") > + ssh.AddAuthorizedKeys(self.tmpname, > + ["ssh-dss AAAAB3NzaC1kc3MAAACB root@test", > + "ssh-dss AAAAB3NzaC1kc3MAAACB root@test"]) > + > + self.assertFileContent(self.tmpname, > + "ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@key-a\n" > + 'command="/usr/bin/fooserver -t --verbose",from="198.51.100.4"' > + " ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22 root@key-b\n" > + "ssh-dss AAAAB3NzaC1kc3MAAACB root@test\n") > + > + def testAddingSeveralKeysAtOnce(self): > + ssh.AddAuthorizedKeys(self.tmpname, ["aaa", "bbb", "ccc"]) > + self.assertFileContent(self.tmpname, > + "ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@key-a\n" > + 'command="/usr/bin/fooserver -t --verbose",from="198.51.100.4"' > + " ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22 root@key-b\n" > + "aaa\nbbb\nccc\n") > + ssh.AddAuthorizedKeys(self.tmpname, ["bbb", "ddd", "eee"]) > + self.assertFileContent(self.tmpname, > + "ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@key-a\n" > + 'command="/usr/bin/fooserver -t --verbose",from="198.51.100.4"' > + " ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22 root@key-b\n" > + "aaa\nbbb\nccc\nddd\neee\n") > + > def testAddingAlmostButNotCompletelyTheSameKey(self): > ssh.AddAuthorizedKey(self.tmpname, > "ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@test") > @@ -224,6 +251,26 @@ class TestSshKeys(testutils.GanetiTestCase): > 'command="/usr/bin/fooserver -t --verbose",from="198.51.100.4"' > " ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22 root@key-b\n") > > + def testAddingNewKeys(self): > + ssh.AddAuthorizedKeys(self.tmpname, > + ["ssh-dss AAAAB3NzaC1kc3MAAACB root@test"]) > + self.assertFileContent(self.tmpname, > + "ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@key-a\n" > + 'command="/usr/bin/fooserver -t --verbose",from="198.51.100.4"' > + " ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22 root@key-b\n" > + "ssh-dss AAAAB3NzaC1kc3MAAACB root@test\n") > + > + ssh.AddAuthorizedKeys(self.tmpname, > + ["ssh-dss AAAAB3asdfasdfaYTUCB laracroft@test", > + "ssh-dss AasdfliuobaosfMAAACB frodo@test"]) > + self.assertFileContent(self.tmpname, > + "ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@key-a\n" > + 'command="/usr/bin/fooserver -t --verbose",from="198.51.100.4"' > + " ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22 root@key-b\n" > + "ssh-dss AAAAB3NzaC1kc3MAAACB root@test\n" > + "ssh-dss AAAAB3asdfasdfaYTUCB laracroft@test\n" > + "ssh-dss AasdfliuobaosfMAAACB frodo@test\n") > + > > class TestPublicSshKeys(testutils.GanetiTestCase): > """Test case for the handling of the list of public ssh keys.""" > diff --git a/test/py/ganeti.tools.prepare_node_join_unittest.py b/test/py/ > ganeti.tools.prepare_node_join_unittest.py > index e0c60a4..8b6c7ee 100755 > --- a/test/py/ganeti.tools.prepare_node_join_unittest.py > +++ b/test/py/ganeti.tools.prepare_node_join_unittest.py > @@ -64,10 +64,22 @@ class TestLoadData(unittest.TestCase): > raw = serializer.DumpJson([]) > self.assertRaises(errors.ParseError, prepare_node_join.LoadData, raw) > > - def testValidData(self): > + def testEmptyDict(self): > raw = serializer.DumpJson({}) > self.assertEqual(prepare_node_join.LoadData(raw), {}) > > + def testValidData(self): > + key_list = [[constants.SSHK_DSA, "private foo", "public bar"]] > + data_dict = { > + constants.SSHS_CLUSTER_NAME: "Skynet", > + constants.SSHS_SSH_HOST_KEY: key_list, > + constants.SSHS_SSH_ROOT_KEY: key_list, > + constants.SSHS_SSH_AUTHORIZED_KEYS: > + {"nodeuuid01234": ["foo"], > + "nodeuuid56789": ["bar"]}} > + raw = serializer.DumpJson(data_dict) > + self.assertEqual(prepare_node_join.LoadData(raw), data_dict) > + > > class TestVerifyCertificate(testutils.GanetiTestCase): > def setUp(self): > -- > 2.1.0.rc2.206.gedb03e5 > >
