]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Support insecure NAS security options in Quobyte
authorSilvan Kaiser <silvan@quobyte.com>
Wed, 14 Oct 2015 11:49:09 +0000 (13:49 +0200)
committerSilvan Kaiser <silvan@quobyte.com>
Tue, 27 Oct 2015 15:26:52 +0000 (16:26 +0100)
So far the nas_secure_file_* options have been hardcoded to
'true' in the Quobyte Cinder driver. This change
implements tests and a change to the set_nas_security_options
method that allows setting these options to false.

Implements: blueprint allow-insecure-conf-in-quobyte

Change-Id: Iffcafd843340059f9b35e9844939cdb004cafbea

cinder/tests/unit/test_quobyte.py
cinder/volume/drivers/quobyte.py

index ed91d4b3c731131d71ddc7d2c6f95d6667860492..16afbbd94b80f17c494286c76d9ddb582073741a 100644 (file)
@@ -86,6 +86,8 @@ class QuobyteDriverTestCase(test.TestCase):
         self._configuration.quobyte_qcow2_volumes = False
         self._configuration.quobyte_mount_point_base = \
             self.TEST_MNT_POINT_BASE
+        self._configuration.nas_secure_file_operations = "auto"
+        self._configuration.nas_secure_file_permissions = "auto"
 
         self._driver =\
             quobyte.QuobyteDriver(configuration=self._configuration,
@@ -342,11 +344,15 @@ class QuobyteDriverTestCase(test.TestCase):
             self.assertEqual(1, len(drv.shares))
             self.assertEqual(0, len(drv._mounted_shares))
 
-    def test_do_setup(self):
+    @mock.patch.object(quobyte.QuobyteDriver, "set_nas_security_options")
+    def test_do_setup(self, qb_snso_mock):
         """do_setup runs successfully."""
         drv = self._driver
+
         drv.do_setup(mock.create_autospec(context.RequestContext))
 
+        qb_snso_mock.assert_called_once_with(is_new_cinder_install=mock.ANY)
+
     def test_check_for_setup_error_throws_quobyte_volume_url_not_set(self):
         """check_for_setup_error throws if 'quobyte_volume_url' is not set."""
         drv = self._driver
@@ -931,3 +937,37 @@ class QuobyteDriverTestCase(test.TestCase):
             mock_upload_volume.assert_called_once_with(
                 mock.ANY, mock.ANY, mock.ANY, upload_path)
             self.assertTrue(mock_create_temporary_file.called)
+
+    def test_set_nas_security_options_default(self):
+        drv = self._driver
+        self.assertTrue(drv.configuration.nas_secure_file_operations ==
+                        "true")
+        self.assertTrue(drv.configuration.nas_secure_file_permissions ==
+                        "true")
+        self.assertFalse(drv._execute_as_root)
+
+    def test_set_nas_security_options_insecure(self):
+        drv = self._driver
+        drv.configuration.nas_secure_file_operations = "false"
+        drv.configuration.nas_secure_file_permissions = "false"
+
+        drv.set_nas_security_options(is_new_cinder_install=True)
+
+        self.assertTrue(drv.configuration.nas_secure_file_operations ==
+                        "false")
+        self.assertTrue(drv.configuration.nas_secure_file_permissions ==
+                        "false")
+        self.assertTrue(drv._execute_as_root)
+
+    def test_set_nas_security_options_explicitly_secure(self):
+        drv = self._driver
+        drv.configuration.nas_secure_file_operations = "true"
+        drv.configuration.nas_secure_file_permissions = "true"
+
+        drv.set_nas_security_options(is_new_cinder_install=True)
+
+        self.assertTrue(drv.configuration.nas_secure_file_operations ==
+                        "true")
+        self.assertTrue(drv.configuration.nas_secure_file_permissions ==
+                        "true")
+        self.assertFalse(drv._execute_as_root)
index 9f6559cf97ce32a1ed86e4733eab7355ef77d63a..76a4559d9d3ceba37b46cc608be7d5dbebc85c39 100644 (file)
@@ -29,7 +29,7 @@ from cinder.image import image_utils
 from cinder import utils
 from cinder.volume.drivers import remotefs as remotefs_drv
 
-VERSION = '1.0'
+VERSION = '1.1'
 
 LOG = logging.getLogger(__name__)
 
@@ -78,6 +78,7 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver):
 
     Version history:
         1.0   - Initial driver.
+        1.1   - Adds optional insecure NAS settings
     """
 
     driver_volume_type = 'quobyte'
@@ -94,9 +95,9 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver):
 
     def do_setup(self, context):
         """Any initialization the volume driver does while starting."""
-        self.set_nas_security_options(is_new_cinder_install=False)
         super(QuobyteDriver, self).do_setup(context)
 
+        self.set_nas_security_options(is_new_cinder_install=False)
         self.shares = {}  # address : options
         self._nova = compute.API()
 
@@ -120,10 +121,47 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver):
                 raise
 
     def set_nas_security_options(self, is_new_cinder_install):
-        self.configuration.nas_secure_file_operations = 'true'
-        self.configuration.nas_secure_file_permissions = 'true'
         self._execute_as_root = False
 
+        LOG.debug("nas_secure_file_* settings are %(ops)s and %(perm)s",
+                  {'ops': self.configuration.nas_secure_file_operations,
+                   'perm': self.configuration.nas_secure_file_permissions}
+                  )
+
+        if self.configuration.nas_secure_file_operations == 'auto':
+            """Note (kaisers): All previous Quobyte driver versions ran with
+            secure settings hardcoded to 'True'. Therefore the default 'auto'
+            setting can safely be mapped to the same, secure, setting.
+            """
+            LOG.debug("Mapping 'auto' value to 'true' for"
+                      " nas_secure_file_operations.")
+            self.configuration.nas_secure_file_operations = 'true'
+
+        if self.configuration.nas_secure_file_permissions == 'auto':
+            """Note (kaisers): All previous Quobyte driver versions ran with
+            secure settings hardcoded to 'True'. Therefore the default 'auto'
+            setting can safely be mapped to the same, secure, setting.
+            """
+            LOG.debug("Mapping 'auto' value to 'true' for"
+                      " nas_secure_file_permissions.")
+            self.configuration.nas_secure_file_permissions = 'true'
+
+        if self.configuration.nas_secure_file_operations == 'false':
+            LOG.warning(_LW("The NAS file operations will be run as "
+                            "root, allowing root level access at the storage "
+                            "backend."))
+            self._execute_as_root = True
+        else:
+            LOG.info(_LI("The NAS file operations will be run as"
+                         " non privileged user in secure mode. Please"
+                         " ensure your libvirtd settings have been configured"
+                         " accordingly (see section 'OpenStack' in the Quobyte"
+                         " Manual."))
+
+        if self.configuration.nas_secure_file_permissions == 'false':
+            LOG.warning(_LW("The NAS file permissions mode will be 666 "
+                            "(allowing other/world read & write access)."))
+
     def _qemu_img_info(self, path, volume_name):
         return super(QuobyteDriver, self)._qemu_img_info_base(
             path, volume_name, self.configuration.quobyte_mount_point_base)
@@ -391,7 +429,7 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver):
                         LOG.info(_LI('Fixing previous mount %s which was not'
                                      ' unmounted correctly.'), mount_path)
                         self._execute('umount.quobyte', mount_path,
-                                      run_as_root=False)
+                                      run_as_root=self._execute_as_root)
                     except processutils.ProcessExecutionError as exc:
                         LOG.warning(_LW("Failed to unmount previous mount: "
                                         "%s"), exc)
@@ -411,7 +449,7 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver):
 
             try:
                 LOG.info(_LI('Mounting volume: %s ...'), quobyte_volume)
-                self._execute(*command, run_as_root=False)
+                self._execute(*command, run_as_root=self._execute_as_root)
                 LOG.info(_LI('Mounting volume: %s succeeded'), quobyte_volume)
                 mounted = True
             except processutils.ProcessExecutionError as exc:
@@ -427,7 +465,7 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver):
         """Wraps execute calls for checking validity of a Quobyte volume"""
         command = ['getfattr', "-n", "quobyte.info", mount_path]
         try:
-            self._execute(*command, run_as_root=False)
+            self._execute(*command, run_as_root=self._execute_as_root)
         except processutils.ProcessExecutionError as exc:
             msg = (_("The mount %(mount_path)s is not a valid"
                      " Quobyte USP volume. Error: %(exc)s")