On Tue, Sep 02, 2014 at 04:19:39PM +0200, 'Helga Velroyen' via
ganeti-devel wrote:
This patch adjusts the SSH connectivity test that
'gnt-cluster verify' does and introduces a couple of
sanity checks for the new SSH setup with individual
keys.
Note that it won't be possible for this to always hold
through the entire patch series. I decided to put it in
anyway, because it a great debugging tool during the
development itself as keeping track of the states of
various key files is tedious manual work.
Signed-off-by: Helga Velroyen <[email protected]>
---
lib/backend.py | 114 ++++++++++++++++++++++++++++++
+++++--
lib/cmdlib/cluster.py | 50 +++++++++++++++-
lib/cmdlib/node.py | 2 +-
lib/ssh.py | 28 +++++++++
src/Ganeti/Constants.hs | 3 +
test/py/cmdlib/cluster_unittest.py | 67 ++++++++++++++--------
test/py/ganeti.backend_unittest.py | 88 ++++++++++++++++++++++++++++
test/py/ganeti.ssh_unittest.py | 5 ++
8 files changed, 326 insertions(+), 31 deletions(-)
diff --git a/lib/backend.py b/lib/backend.py
index d9526c4..717298c 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -948,6 +948,100 @@ def _VerifyClientCertificate(cert_
file=pathutils.NODED_CLIENT_CERT_FILE):
return (None, utils.GetCertificateDigest(cert_filename=cert_file))
+def _VerifySshSetup(node_status_list, my_name):
+ """Verifies the state of the SSH key files.
+
+ @type node_status_list: list of tuples
+ @param node_status_list: list of nodes of the cluster associated with a
+ couple of flags: (uuid, name, is_master_candidate,
+ is_potential_master_candidate, online)
+ @type my_name: str
+ @param my_name: name of this node
+
+ """
+ if node_status_list is None:
+ return ["No node list to check against the pub_key_file received.."]
+
+ my_status_list = [(my_uuid, name, mc, pot_mc) for (my_uuid, name, mc,
pot_mc)
+ in node_status_list if name == my_name]
+ if len(my_status_list) < 0:
This should be ... `== 0:`
+ return ["Cannot find node information for node '%s'." % my_name]
+ (my_uuid, _, _, potential_master_candidate) = \
+ my_status_list[0]
+
+ result = []
+ pot_mc_uuids = [uuid for (uuid, _, _, _) in node_status_list]
+ pub_keys = ssh.QueryPubKeyFile(None)
+
+ if potential_master_candidate:
+ # Check that the set of potential master candidates matches the
+ # public key file
+ pub_uuids_set = set(pub_keys.keys())
+ pot_mc_uuids_set = set(pot_mc_uuids)
+ missing_uuids = set([])
+ if pub_uuids_set != pot_mc_uuids_set:
+ unknown_uuids = pub_uuids_set - pot_mc_uuids_set
+ if unknown_uuids:
+ result.append("The following node UUIDs are listed in the public
key"
+ " file on node '%s', but are not potential master"
+ " candidates: %s."
+ % (my_name, ", ".join(list(unknown_uuids))))
+ missing_uuids = pot_mc_uuids_set - pub_uuids_set
+ if missing_uuids:
+ result.append("The following node UUIDs of potential master
candidates"
+ " are missing in the public key file on node %s:
%s."
+ % (my_name, ", ".join(list(missing_uuids))))
+
+ (_, key_files) = \
+ ssh.GetAllUserFiles(constants.SSH_LOGIN_USER, mkdir=False,
dircheck=False)
+ my_keys = pub_keys[my_uuid]
+ num_keys = 0
+ for (key_type, (_, pub_key_file)) in key_files.items():
+ try:
+ pub_key = utils.ReadFile(pub_key_file)
+ if pub_key.strip() not in my_keys:
+ result.append("The %s key of node %s does not match this
node's keys"
+ " in the pub key file." % (key_type, my_name))
+ num_keys += 1
+ except IOError:
+ # There might not be keys of every type.
+ pass
+ if num_keys != len(my_keys):
+ result.append("The number of keys for node %s in the public key
file"
+ " (%s) does not match the number of keys on the node"
+ " (%s)." % (my_name, len(my_keys), len(key_files)))
+ else:
+ if len(pub_keys.keys()) > 0:
+ result.append("The public key file of node '%s' is not empty,
although"
+ " the node is not a potential master candidate."
+ % my_name)
+
+ # Check that all master candidate keys are in the authorized_keys file
+ (auth_key_file, _) = \
+ ssh.GetAllUserFiles(constants.SSH_LOGIN_USER, mkdir=False,
dircheck=False)
+ for (uuid, name, mc, _) in node_status_list:
+ if uuid in missing_uuids:
+ continue
+ if mc:
+ for key in pub_keys[uuid]:
+ if not ssh.HasAuthorizedKey(auth_key_file, key):
+ result.append("A SSH key of master candidate '%s' (UUID: '%s')
is"
+ " not in the 'authorized_keys' file of node
'%s'."
+ % (name, uuid, my_name))
+ else:
+ for key in pub_keys[uuid]:
+ if name != my_name and ssh.HasAuthorizedKey(auth_key_file, key):
+ result.append("A SSH key of normal node '%s' (UUID: '%s') is
in the"
+ " 'authorized_keys' file of node '%s'."
+ % (name, uuid, my_name))
+ if name == my_name and not ssh.HasAuthorizedKey(auth_key_file,
key):
+ result.append("A SSH key of normal node '%s' (UUID: '%s') is
not"
+ " in the 'authorized_keys' file of itself."
+ % (my_name, uuid))
+
+ return result
+
+
def VerifyNode(what, cluster_name, all_hvparams, node_groups, groups_cfg):
"""Verify the status of the local node.
@@ -1004,8 +1098,12 @@ def VerifyNode(what, cluster_name, all_hvparams,
node_groups, groups_cfg):
if constants.NV_CLIENT_CERT in what:
result[constants.NV_CLIENT_CERT] = _VerifyClientCertificate()
+ if constants.NV_SSH_SETUP in what:
+ result[constants.NV_SSH_SETUP] = \
+ _VerifySshSetup(what[constants.NV_SSH_SETUP], my_name)
+
if constants.NV_NODELIST in what:
- (nodes, bynode) = what[constants.NV_NODELIST]
+ (nodes, bynode, mcs) = what[constants.NV_NODELIST]
# Add nodes from other groups (different for each node)
try:
@@ -1023,10 +1121,16 @@ def VerifyNode(what, cluster_name, all_hvparams,
node_groups, groups_cfg):
ssh_port = params["ndparams"].get(constants.ND_SSH_PORT)
logging.debug("Ssh port %s (None = default) for node %s",
str(ssh_port), node)
- success, message = _GetSshRunner(cluster_name). \
- VerifyNodeHostname(node, ssh_port)
- if not success:
- val[node] = message
+
+ # We only test if master candidates can communicate to other nodes.
+ # We cannot test if normal nodes cannot communicate with other
nodes,
+ # because the administrator might have installed additional SSH
keys,
+ # over which Ganeti has no power.
+ if my_name in mcs:
+ success, message = _GetSshRunner(cluster_name). \
+ VerifyNodeHostname(node, ssh_port)
+ if not success:
+ val[node] = message
result[constants.NV_NODELIST] = val
diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py
index 915c418..d0c54de 100644
--- a/lib/cmdlib/cluster.py
+++ b/lib/cmdlib/cluster.py
@@ -2682,6 +2682,27 @@ class LUClusterVerifyGroup(LogicalUnit,
_VerifyErrors):
" certificate of another node which is master candidate.",
node.uuid)
+ def _VerifySshSetup(self, nodes, all_nvinfo):
+ """Evaluates the verification results of the SSH setup.
+
+ @param nodes: List of L{objects.Node} objects
+ @param all_nvinfo: RPC results
+
+ """
+ for node in nodes:
+ if not node.offline:
+ nresult = all_nvinfo[node.uuid]
+ if nresult.fail_msg or not nresult.payload:
+ self._ErrorIf(True, constants.CV_ENODESSH, node.name,
+ "Could not verify the SSH setup of this node..")
+ return
+ result = nresult.payload.get(constants.NV_SSH_SETUP, None)
+ error_msg = ""
+ if isinstance(result, list):
+ error_msg = " ".join(result)
+ self._ErrorIf(result,
+ constants.CV_ENODESSH, None, error_msg)
+
def _VerifyFiles(self, nodes, master_node_uuid, all_nvinfo,
(files_all, files_opt, files_mc, files_vm)):
"""Verifies file checksums collected from all nodes.
@@ -3286,17 +3307,38 @@ class LUClusterVerifyGroup(LogicalUnit,
_VerifyErrors):
We will make nodes contact all nodes in their group, and one node from
every other group.
+ @rtype: tuple of (string, dict of strings to list of strings, string)
+ @return: a tuple containing the list of all online nodes, a
dictionary
+ mapping node names to additional nodes of other node groups to
which
+ connectivity should be tested, and a list of all online master
+ candidates
+
@warning: This algorithm has a known issue if one node group is much
smaller than others (e.g. just one node). In such a case all other
nodes will talk to the single node.
"""
online_nodes = sorted(node.name for node in group_nodes if not
node.offline)
+ online_mcs = sorted(node.name for node in group_nodes
+ if (node.master_candidate and not node.offline))
sel = cls._SshNodeSelector(group_uuid, all_nodes)
return (online_nodes,
dict((name, sorted([i.next() for i in sel]))
- for name in online_nodes))
+ for name in online_nodes),
+ online_mcs)
+
+ def _PrepareSshSetupCheck(self):
+ """Prepare the input data for the SSH setup verification.
+
+ """
+ all_nodes_info = self.cfg.GetAllNodesInfo()
+ potential_master_candidates = self.cfg.
GetPotentialMasterCandidates()
+ node_status = [
+ (uuid, node_info.name, node_info.master_candidate,
+ node_info.name in potential_master_candidates)
+ for (uuid, node_info) in all_nodes_info.items()]
+ return node_status
def BuildHooksEnv(self):
"""Build hooks env.
@@ -3391,6 +3433,9 @@ class LUClusterVerifyGroup(LogicalUnit,
_VerifyErrors):
constants.NV_CLIENT_CERT: None,
}
+ if self.cfg.GetClusterInfo().modify_ssh_setup:
+ node_verify_param[constants.NV_SSH_SETUP] =
self._PrepareSshSetupCheck()
+
if vg_name is not None:
node_verify_param[constants.NV_VGLIST] = None
node_verify_param[constants.NV_LVLIST] = vg_name
@@ -3573,7 +3618,8 @@ class LUClusterVerifyGroup(LogicalUnit,
_VerifyErrors):
feedback_fn("* Verifying configuration file consistency")
self._VerifyClientCertificates(self.my_node_info.values(),
all_nvinfo)
-
+ if self.cfg.GetClusterInfo().modify_ssh_setup:
+ self._VerifySshSetup(self.my_node_info.values(), all_nvinfo)
self._VerifyFiles(vf_node_info, master_node_uuid, vf_nvinfo, filemap)
feedback_fn("* Verifying node status")
diff --git a/lib/cmdlib/node.py b/lib/cmdlib/node.py
index 9badcba..823ebb6 100644
--- a/lib/cmdlib/node.py
+++ b/lib/cmdlib/node.py
@@ -420,7 +420,7 @@ class LUNodeAdd(LogicalUnit):
node_verifier_uuids = [self.cfg.GetMasterNode()]
node_verify_param = {
- constants.NV_NODELIST: ([self.new_node.name], {}),
+ constants.NV_NODELIST: ([self.new_node.name], {}, []),
# TODO: do a node-net-test as well?
}
diff --git a/lib/ssh.py b/lib/ssh.py
index 78adce6..7e2f64c 100644
--- a/lib/ssh.py
+++ b/lib/ssh.py
@@ -173,6 +173,34 @@ def AddAuthorizedKeys(file_obj, keys):
f.close()
+def HasAuthorizedKey(file_obj, key):
+ """Check if a particular key is in the '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
+
+ """
+ key_fields = _SplitSshKey(key)
+
+ if isinstance(file_obj, basestring):
+ f = open(file_obj, "r")
+ else:
+ f = file_obj
+
+ try:
+ for line in f:
+ # Ignore whitespace changes
+ line_key = _SplitSshKey(line)
+ if line_key == key_fields:
+ return True
+ finally:
+ f.close()
+
+ return False
+
+
def AddAuthorizedKey(file_obj, key):
"""Adds an SSH public key to an authorized_keys file.
diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
index 36227e6..4a2db43 100644
--- a/src/Ganeti/Constants.hs
+++ b/src/Ganeti/Constants.hs
@@ -3209,6 +3209,9 @@ nvVglist = "vglist"
nvVmnodes :: String
nvVmnodes = "vmnodes"
+nvSshSetup :: String
+nvSshSetup = "ssh-setup"
+
-- * Instance status
inststAdmindown :: String
diff --git a/test/py/cmdlib/cluster_unittest.py b/test/py/cmdlib/cluster_
unittest.py
index d0a6542..fa0e655 100644
--- a/test/py/cmdlib/cluster_unittest.py
+++ b/test/py/cmdlib/cluster_unittest.py
@@ -50,29 +50,46 @@ class TestClusterVerifySsh(unittest.TestCase):
def testMultipleGroups(self):
fn = cluster.LUClusterVerifyGroup._SelectSshCheckNodes
mygroupnodes = [
- objects.Node(name="node20", group="my", offline=False),
- objects.Node(name="node21", group="my", offline=False),
- objects.Node(name="node22", group="my", offline=False),
- objects.Node(name="node23", group="my", offline=False),
- objects.Node(name="node24", group="my", offline=False),
- objects.Node(name="node25", group="my", offline=False),
- objects.Node(name="node26", group="my", offline=True),
+ objects.Node(name="node20", group="my", offline=False,
+ master_candidate=True),
+ objects.Node(name="node21", group="my", offline=False,
+ master_candidate=True),
+ objects.Node(name="node22", group="my", offline=False,
+ master_candidate=False),
+ objects.Node(name="node23", group="my", offline=False,
+ master_candidate=True),
+ objects.Node(name="node24", group="my", offline=False,
+ master_candidate=True),
+ objects.Node(name="node25
diff --git a/lib/backend.py b/lib/backend.py
index 509e99b..8f48f9f 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -973,7 +973,7 @@ def _VerifySshSetup(node_status_list, my_name):
my_status_list = [(my_uuid, name, mc, pot_mc) for (my_uuid, name, mc,
pot_mc)
in node_status_list if name == my_name]
- if len(my_status_list) < 0:
+ if len(my_status_list) == 0:
return ["Cannot find node information for node '%s'." % my_name]
(my_uuid, _, _, potential_master_candidate) = \
my_status_list[0]
", group="my", offline=False,
+ master_candidate=False),
+ objects.Node(name="node26", group="my", offline=True,
+ master_candidate=True),
]
nodes = [
- objects.Node(name="node1", group="g1", offline=True),
- objects.Node(name="node2", group="g1", offline=False),
- objects.Node(name="node3", group="g1", offline=False),
- objects.Node(name="node4", group="g1", offline=True),
- objects.Node(name="node5", group="g1", offline=False),
- objects.Node(name="node10", group="xyz", offline=False),
- objects.Node(name="node11", group="xyz", offline=False),
- objects.Node(name="node40", group="alloff", offline=True),
- objects.Node(name="node41", group="alloff", offline=True),
- objects.Node(name="node50", group="aaa", offline=False),
+ objects.Node(name="node1", group="g1", offline=True,
+ master_candidate=True),
+ objects.Node(name="node2", group="g1", offline=False,
+ master_candidate=False),
+ objects.Node(name="node3", group="g1", offline=False,
+ master_candidate=True),
+ objects.Node(name="node4", group="g1", offline=True,
+ master_candidate=True),
+ objects.Node(name="node5", group="g1", offline=False,
+ master_candidate=True),
+ objects.Node(name="node10", group="xyz", offline=False,
+ master_candidate=True),
+ objects.Node(name="node11", group="xyz", offline=False,
+ master_candidate=True),
+ objects.Node(name="node40", group="alloff", offline=True,
+ master_candidate=True),
+ objects.Node(name="node41", group="alloff", offline=True,
+ master_candidate=True),
+ objects.Node(name="node50", group="aaa", offline=False,
+ master_candidate=True),
] + mygroupnodes
assert not utils.FindDuplicates(map(operator.attrgetter("name"),
nodes))
- (online, perhost) = fn(mygroupnodes, "my", nodes)
+ (online, perhost, _) = fn(mygroupnodes, "my", nodes)
self.assertEqual(online, ["node%s" % i for i in range(20, 26)])
self.assertEqual(set(perhost.keys()), set(online))
@@ -88,14 +105,18 @@ class TestClusterVerifySsh(unittest.TestCase):
def testSingleGroup(self):
fn = cluster.LUClusterVerifyGroup._SelectSshCheckNodes
nodes = [
- objects.Node(name="node1", group="default", offline=True),
- objects.Node(name="node2", group="default", offline=False),
- objects.Node(name="node3", group="default", offline=False),
- objects.Node(name="node4", group="default", offline=True),
+ objects.Node(name="node1", group="default", offline=True,
+ master_candidate=True),
+ objects.Node(name="node2", group="default", offline=False,
+ master_candidate=True),
+ objects.Node(name="node3", group="default", offline=False,
+ master_candidate=True),
+ objects.Node(name="node4", group="default", offline=True,
+ master_candidate=True),
]
assert not utils.FindDuplicates(map(operator.attrgetter("name"),
nodes))
- (online, perhost) = fn(nodes, "default", nodes)
+ (online, perhost, _) = fn(nodes, "default", nodes)
self.assertEqual(online, ["node2", "node3"])
self.assertEqual(set(perhost.keys()), set(online))
diff --git a/test/py/ganeti.backend_unittest.py b/test/py/ganeti.backend_
unittest.py
index 6e54273..9b18763 100755
--- a/test/py/ganeti.backend_unittest.py
+++ b/test/py/ganeti.backend_unittest.py
@@ -21,6 +21,7 @@
"""Script for testing ganeti.backend"""
+import copy
import mock
import os
import shutil
@@ -1275,7 +1276,94 @@ class TestVerifySshSetup(testutils.
GanetiTestCase):
_NODE2_NAME = "name2"
_NODE3_NAME = "name3"
_NODE1_KEYS = ["key11", "key12"]
+ _NODE2_KEYS = ["key21"]
+ _NODE3_KEYS = ["key31"]
+
+ _NODE_STATUS_LIST = [
+ (_NODE1_UUID, _NODE1_NAME, True, True),
+ (_NODE2_UUID, _NODE2_NAME, False, True),
+ (_NODE3_UUID, _NODE3_NAME, False, False),
+ ]
+
+ _PUB_KEY_RESULT = {
+ _NODE1_UUID: _NODE1_KEYS,
+ _NODE2_UUID: _NODE2_KEYS,
+ _NODE3_UUID: _NODE3_KEYS,
+ }
+
+ _AUTH_RESULT = {
+ _NODE1_KEYS[0]: True,
+ _NODE1_KEYS[1]: True,
+ _NODE2_KEYS[0]: False,
+ _NODE3_KEYS[0]: False,
+ }
+ def setUp(self):
+ testutils.GanetiTestCase.setUp(self)
+ self._has_authorized_patcher = testutils \
+ .patch_object(ssh, "HasAuthorizedKey")
+ self._has_authorized_mock = self._has_authorized_patcher.start()
+ self._query_patcher = testutils \
+ .patch_object(ssh, "QueryPubKeyFile")
+ self._query_mock = self._query_patcher.start()
+ self._read_file_patcher = testutils \
+ .patch_object(utils, "ReadFile")
+ self._read_file_mock = self._read_file_patcher.start()
+ self._read_file_mock.return_value = self._NODE1_KEYS[0]
+
+ def tearDown(self):
+ super(testutils.GanetiTestCase, self).tearDown()
+ self._has_authorized_patcher.stop()
+ self._query_patcher.stop()
+ self._read_file_patcher.stop()
+
+ def testValidData(self):
+ self._has_authorized_mock.side_effect = \
+ lambda _, key : self._AUTH_RESULT[key]
+ self._query_mock.return_value = self._PUB_KEY_RESULT
+ result = backend._VerifySshSetup(self._NODE_STATUS_LIST,
+ self._NODE1_NAME)
+ self.assertEqual(result, [])
+
+ def testMissingKey(self):
+ self._has_authorized_mock.side_effect = \
+ lambda _, key : self._AUTH_RESULT[key]
+ pub_key_missing = copy.deepcopy(self._PUB_KEY_RESULT)
+ del pub_key_missing[self._NODE2_UUID]
+ self._query_mock.return_value = pub_key_missing
+ result = backend._VerifySshSetup(self._NODE_STATUS_LIST,
+ self._NODE1_NAME)
+ self.assertTrue(self._NODE2_UUID in result[0])
+
+ def testUnknownKey(self):
+ self._has_authorized_mock.side_effect = \
+ lambda _, key : self._AUTH_RESULT[key]
+ pub_key_missing = copy.deepcopy(self._PUB_KEY_RESULT)
+ pub_key_missing["unkownnodeuuid"] = "pinkbunny"
+ self._query_mock.return_value = pub_key_missing
+ result = backend._VerifySshSetup(self._NODE_STATUS_LIST,
+ self._NODE1_NAME)
+ self.assertTrue("unkownnodeuuid" in result[0])
+
+ def testMissingMasterCandidate(self):
+ auth_result = copy.deepcopy(self._AUTH_RESULT)
+ auth_result["key12"] = False
+ self._has_authorized_mock.side_effect = \
+ lambda _, key : auth_result[key]
+ self._query_mock.return_value = self._PUB_KEY_RESULT
+ result = backend._VerifySshSetup(self._NODE_STATUS_LIST,
+ self._NODE1_NAME)
+ self.assertTrue(self._NODE1_UUID in result[0])
+
+ def testSuperfluousNormalNode(self):
+ auth_result = copy.deepcopy(self._AUTH_RESULT)
+ auth_result["key31"] = True
+ self._has_authorized_mock.side_effect = \
+ lambda _, key : auth_result[key]
+ self._query_mock.return_value = self._PUB_KEY_RESULT
+ result = backend._VerifySshSetup(self._NODE_STATUS_LIST,
+ self._NODE1_NAME)
+ self.assertTrue(self._NODE3_UUID in result[0])
if __name__ == "__main__":
diff --git a/test/py/ganeti.ssh_unittest.py b/test/py/
ganeti.ssh_unittest.py
index 9abc629..b0ba9dd 100755
--- a/test/py/ganeti.ssh_unittest.py
+++ b/test/py/ganeti.ssh_unittest.py
@@ -165,6 +165,11 @@ class TestSshKeys(testutils.GanetiTestCase):
finally:
handle.close()
+ def testHasAuthorizedKey(self):
+ self.assertTrue(ssh.HasAuthorizedKey(self.tmpname, self.KEY_A))
+ self.assertFalse(ssh.HasAuthorizedKey(
+ self.tmpname, "I am the key of the pink bunny!"))
+
def testAddingNewKey(self):
ssh.AddAuthorizedKey(self.tmpname,
"ssh-dss AAAAB3NzaC1kc3MAAACB root@test")
--
2.1.0.rc2.206.gedb03e5
Rest LGTM
