diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 6e8e65fe30d..b8e5cd06cc7 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -258,6 +258,16 @@ class RBDTestCase(test.TestCase): self.assertRaises(exception.InvalidConfigurationValue, self.driver.check_for_setup_error) + @mock.patch.object(driver, 'rados', mock.Mock()) + @mock.patch.object(driver, 'RADOSClient') + def test_check_for_setup_error_missing_keyring_data(self, mock_client): + self.driver.keyring_file = '/etc/ceph/ceph.client.admin.keyring' + self.driver.keyring_data = None + + self.assertRaises(exception.InvalidConfigurationValue, + self.driver.check_for_setup_error) + mock_client.assert_called_once_with(self.driver) + def test_parse_replication_config_empty(self): self.driver._parse_replication_configs([]) self.assertEqual([], self.driver._replication_targets) @@ -1593,6 +1603,12 @@ class RBDTestCase(test.TestCase): } self._initialize_connection_helper(expected, hosts, ports) + # Check how it will work with keyring data (for cinderlib) + keyring_data = "[client.cinder]\n key = test\n" + self.driver.keyring_data = keyring_data + expected['data']['keyring'] = keyring_data + self._initialize_connection_helper(expected, hosts, ports) + self.driver._active_config = {'name': 'secondary_id', 'user': 'foo', 'conf': 'bar', @@ -1600,6 +1616,68 @@ class RBDTestCase(test.TestCase): expected['data']['secret_uuid'] = 'secondary_secret_uuid' self._initialize_connection_helper(expected, hosts, ports) + def test__set_keyring_attributes_openstack(self): + # OpenStack usage doesn't have the rbd_keyring_conf Oslo Config option + self.assertFalse(hasattr(self.driver.configuration, + 'rbd_keyring_conf')) + # Set initial values so we can confirm that we set them to None + self.driver.keyring_file = mock.sentinel.keyring_file + self.driver.keyring_data = mock.sentinel.keyring_data + + self.driver._set_keyring_attributes() + + self.assertIsNone(self.driver.keyring_file) + self.assertIsNone(self.driver.keyring_data) + + def test__set_keyring_attributes_cinderlib(self): + # OpenStack usage doesn't have the rbd_keyring_conf Oslo Config option + cfg_file = '/etc/ceph/ceph.client.admin.keyring' + self.driver.configuration.rbd_keyring_conf = cfg_file + self.driver._set_keyring_attributes() + self.assertEqual(cfg_file, self.driver.keyring_file) + self.assertIsNone(self.driver.keyring_data) + + @mock.patch('os.path.isfile') + @mock.patch.object(driver, 'open') + def test__set_keyring_attributes_cinderlib_read_file(self, mock_open, + mock_isfile): + cfg_file = '/etc/ceph/ceph.client.admin.keyring' + # This is how cinderlib sets the config option + setattr(self.driver.configuration, 'rbd_keyring_conf', cfg_file) + + keyring_data = "[client.cinder]\n key = test\n" + mock_read = mock_open.return_value.__enter__.return_value.read + mock_read.return_value = keyring_data + + self.assertIsNone(self.driver.keyring_file) + self.assertIsNone(self.driver.keyring_data) + + self.driver._set_keyring_attributes() + + mock_isfile.assert_called_once_with(cfg_file) + mock_open.assert_called_once_with(cfg_file, 'r') + mock_read.assert_called_once_with() + self.assertEqual(cfg_file, self.driver.keyring_file) + self.assertEqual(keyring_data, self.driver.keyring_data) + + @mock.patch('os.path.isfile') + @mock.patch.object(driver, 'open', side_effect=IOError) + def test__set_keyring_attributes_cinderlib_error(self, mock_open, + mock_isfile): + cfg_file = '/etc/ceph/ceph.client.admin.keyring' + # This is how cinderlib sets the config option + setattr(self.driver.configuration, 'rbd_keyring_conf', cfg_file) + + self.assertIsNone(self.driver.keyring_file) + self.driver.keyring_data = mock.sentinel.keyring_data + + self.driver._set_keyring_attributes() + + mock_isfile.assert_called_once_with(cfg_file) + mock_open.assert_called_once_with(cfg_file, 'r') + self.assertEqual(cfg_file, self.driver.keyring_file) + self.assertIsNone(self.driver.keyring_data) + @ddt.data({'rbd_chunk_size': 1, 'order': 20}, {'rbd_chunk_size': 8, 'order': 23}, {'rbd_chunk_size': 32, 'order': 25}) diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index e5c3a89522f..6496bf8c588 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -261,6 +261,30 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, self.RBD_FEATURE_OBJECT_MAP | self.RBD_FEATURE_EXCLUSIVE_LOCK) + self._set_keyring_attributes() + + def _set_keyring_attributes(self): + # The rbd_keyring_conf option is not available for OpenStack usage + # for security reasons (OSSN-0085) and in OpenStack we use + # rbd_secret_uuid or make sure that the keyring files are present on + # the hosts (where os-brick will look for them). + # For cinderlib usage this option is necessary (no security issue, as + # in those cases the contents of the connection are not available to + # users). By using getattr Oslo-conf won't read the option from the + # file even if it's there (because we have removed the conf option + # definition), but cinderlib will find it because it sets the option + # directly as an attribute. + self.keyring_file = getattr(self.configuration, 'rbd_keyring_conf', + None) + + self.keyring_data = None + try: + if self.keyring_file and os.path.isfile(self.keyring_file): + with open(self.keyring_file, 'r') as k_file: + self.keyring_data = k_file.read() + except IOError: + LOG.debug('Cannot read RBD keyring file: %s.', self.keyring_file) + @classmethod def get_driver_options(cls): additional_opts = cls._get_oslo_driver_opts( @@ -388,6 +412,14 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, LOG.error(msg) raise exception.VolumeBackendAPIException(data=msg) + # If the keyring is defined (cinderlib usage), then the contents are + # necessary. + if self.keyring_file and not self.keyring_data: + msg = _('No keyring data found') + LOG.error(msg) + raise exception.InvalidConfigurationValue( + option='rbd_keyring_conf', value=self.keyring_file) + self._start_periodic_tasks() def RBDProxy(self): @@ -1423,6 +1455,8 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, "discard": True, } } + if self.keyring_data: + data['data']['keyring'] = self.keyring_data LOG.debug('connection data: %s', data) return data