From 13f27658634beb2b33412c5d2ffb51f473212da2 Mon Sep 17 00:00:00 2001 From: Daniel Pawlik Date: Tue, 16 May 2017 13:29:04 +0200 Subject: [PATCH] Changed way of providing RBD keyring from keyring_path to client token In created temporary file with RBD configuration there was provided a path for keyring file. It could work only when both system e.g. cinder-volume and host had exact same Ceph configuration, but there was no guarantee for that. Providing Ceph token will help avoid such situations. Partial-Bug: #1668304 Change-Id: I465828dec58ab2110b33743a10e6fc518b5c85ff --- os_brick/initiator/connectors/rbd.py | 30 +++++++++------- .../tests/initiator/connectors/test_rbd.py | 35 ++++++++++++++++--- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/os_brick/initiator/connectors/rbd.py b/os_brick/initiator/connectors/rbd.py index 30220191c..7c6338f50 100644 --- a/os_brick/initiator/connectors/rbd.py +++ b/os_brick/initiator/connectors/rbd.py @@ -69,25 +69,31 @@ class RBDConnector(base.BaseLinuxConnector): return host return list(map(_sanitize_host, hosts)) + def _check_or_get_keyring_contents(self, keyring, cluster_name, user): + try: + if keyring is None: + keyring_path = ("/etc/ceph/%s.client.%s.keyring" % + (cluster_name, user)) + with open(keyring_path, 'r') as keyring_file: + keyring = keyring_file.read() + return keyring + except IOError: + msg = (_("Keyring path %s is not readable.") % (keyring_path)) + raise exception.BrickException(msg=msg) + def _create_ceph_conf(self, monitor_ips, monitor_ports, - cluster_name, user, keyring_path): + cluster_name, user, keyring): monitors = ["%s:%s" % (ip, port) for ip, port in zip(self._sanitize_mon_hosts(monitor_ips), monitor_ports)] mon_hosts = "mon_host = %s" % (','.join(monitors)) - client_section = "[client.%s]" % user - - if keyring_path is None: - keyring = ("keyring = /etc/ceph/%s.client.%s.keyring" % - (cluster_name, user)) - else: - keyring = "keyring = %s" % keyring_path + keyring = self._check_or_get_keyring_contents(keyring, cluster_name, + user) try: fd, ceph_conf_path = tempfile.mkstemp(prefix="brickrbd_") with os.fdopen(fd, 'w') as conf_file: - conf_file.writelines([mon_hosts, "\n", - client_section, "\n", keyring, "\n"]) + conf_file.writelines([mon_hosts, "\n", keyring, "\n"]) return ceph_conf_path except IOError: msg = (_("Failed to write data to %s.") % (ceph_conf_path)) @@ -100,14 +106,14 @@ class RBDConnector(base.BaseLinuxConnector): cluster_name = connection_properties.get('cluster_name') monitor_ips = connection_properties.get('hosts') monitor_ports = connection_properties.get('ports') - keyring_path = connection_properties.get('keyring') + keyring = connection_properties.get('keyring') except IndexError: msg = _("Connect volume failed, malformed connection properties") raise exception.BrickException(msg=msg) conf = self._create_ceph_conf(monitor_ips, monitor_ports, str(cluster_name), user, - keyring_path) + keyring) try: rbd_client = linuxrbd.RBDClient(user, pool, conffile=conf, rbd_cluster_name=str(cluster_name)) diff --git a/os_brick/tests/initiator/connectors/test_rbd.py b/os_brick/tests/initiator/connectors/test_rbd.py index 9a3be81e2..1f11ca6ea 100644 --- a/os_brick/tests/initiator/connectors/test_rbd.py +++ b/os_brick/tests/initiator/connectors/test_rbd.py @@ -34,7 +34,7 @@ class RBDConnectorTestCase(test_connector.ConnectorTestCase): self.clustername = 'fake_ceph' self.hosts = ['192.168.10.2'] self.ports = ['6789'] - self.keyring = '/etc/ceph/ceph.client.cinder.keyring' + self.keyring = "[client.cinder]\n key = test\n" self.connection_properties = { 'auth_username': self.user, @@ -42,6 +42,7 @@ class RBDConnectorTestCase(test_connector.ConnectorTestCase): 'cluster_name': self.clustername, 'hosts': self.hosts, 'ports': self.ports, + 'keyring': self.keyring, } def test_get_search_path(self): @@ -100,16 +101,40 @@ class RBDConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch('os_brick.initiator.linuxrbd.rados') @mock.patch.object(rbd.RBDConnector, '_create_ceph_conf') @mock.patch('os.path.exists') - def test_custom_keyring(self, mock_path, mock_conf, mock_rados, mock_rbd): + def test_provided_keyring(self, mock_path, mock_conf, mock_rados, + mock_rbd): conn = rbd.RBDConnector(None) mock_path.return_value = False mock_conf.return_value = "/tmp/fake_dir/fake_ceph.conf" - custom_keyring_path = "/foo/bar/baz" - self.connection_properties['keyring'] = custom_keyring_path + self.connection_properties['keyring'] = self.keyring conn.connect_volume(self.connection_properties) mock_conf.assert_called_once_with(self.hosts, self.ports, self.clustername, self.user, - custom_keyring_path) + self.keyring) + + def test_keyring_is_none(self): + conn = rbd.RBDConnector(None) + keyring = None + keyring_data = "[client.cinder]\n key = test\n" + mockopen = mock.mock_open(read_data=keyring_data) + mockopen.return_value.__exit__ = mock.Mock() + with mock.patch('os_brick.initiator.connectors.rbd.open', mockopen, + create=True): + self.assertEqual( + conn._check_or_get_keyring_contents(keyring, 'cluster', + 'user'), keyring_data) + + def test_keyring_raise_error(self): + conn = rbd.RBDConnector(None) + keyring = None + mockopen = mock.mock_open() + mockopen.return_value = "" + with mock.patch('os_brick.initiator.connectors.rbd.open', mockopen, + create=True) as mock_keyring_file: + mock_keyring_file.side_effect = IOError + self.assertRaises(exception.BrickException, + conn._check_or_get_keyring_contents, keyring, + 'cluster', 'user') @ddt.data((['192.168.1.1', '192.168.1.2'], ['192.168.1.1', '192.168.1.2']),