glusterfs_native: make {allow,deny}_access non-destructive
With this patch the allow_access and deny_access methods of glusterfs_native will keep preexisting common names in the affected GlusterFS volume 'auth.ssl-allow' option. Also check in _setup_gluster_vol if 'auth.ssl-allow' is set to a non-empty value to avoid semantically problematic (and from Manila POV, useless) edge cases. Change-Id: I952049d694509a338c7f56b45c5ef0872c3e7d70 Closes-Bug: #1439198
This commit is contained in:
Csaba Henk
parent
482d304e38
commit
0ab34e42b2
|
|
@ -42,6 +42,7 @@ from manila import exception
|
|||
from manila.i18n import _
|
||||
from manila.i18n import _LE
|
||||
from manila.i18n import _LI
|
||||
from manila.i18n import _LW
|
||||
from manila.share import driver
|
||||
from manila.share.drivers import glusterfs
|
||||
from manila import utils
|
||||
|
|
@ -265,6 +266,22 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
|
||||
gluster_mgr = self._glustermanager(vol)
|
||||
|
||||
ssl_allow_opt = gluster_mgr.get_gluster_vol_option(AUTH_SSL_ALLOW)
|
||||
if not ssl_allow_opt:
|
||||
# Not having AUTH_SSL_ALLOW set is a problematic edge case.
|
||||
# - In GlusterFS 3.6, it implies that access is allowed to
|
||||
# noone, including intra-service access, which causes
|
||||
# problems internally in GlusterFS
|
||||
# - In GlusterFS 3.7, it implies that access control is
|
||||
# disabled, which defeats the purpose of this driver --
|
||||
# so to avoid these possiblitiies, we throw an error in this case.
|
||||
msg = (_("Option %(option)s is not defined on gluster volume. "
|
||||
"Volume: %(volname)s") %
|
||||
{'volname': gluster_mgr.volume,
|
||||
'option': AUTH_SSL_ALLOW})
|
||||
LOG.error(msg)
|
||||
raise exception.GlusterfsException(msg)
|
||||
|
||||
for option, value in six.iteritems(
|
||||
{NFS_EXPORT_VOL: 'off', CLIENT_SSL: 'on', SERVER_SSL: 'on'}
|
||||
):
|
||||
|
|
@ -638,6 +655,7 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
'errno': operrno,
|
||||
'errstr': operrstr})
|
||||
|
||||
@utils.synchronized("glusterfs_native_access", external=False)
|
||||
def allow_access(self, context, share, access, share_server=None):
|
||||
"""Allow access to a share using certs.
|
||||
|
||||
|
|
@ -650,17 +668,34 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
exp_locn = share.get('export_location', None)
|
||||
gluster_mgr = self.gluster_used_vols_dict.get(exp_locn)
|
||||
|
||||
ssl_allow_opt = gluster_mgr.get_gluster_vol_option(AUTH_SSL_ALLOW)
|
||||
# wrt. GlusterFS' parsing of auth.ssl-allow, please see code from
|
||||
# https://github.com/gluster/glusterfs/blob/v3.6.2/
|
||||
# xlators/protocol/auth/login/src/login.c#L80
|
||||
# until end of gf_auth() function
|
||||
ssl_allow = re.split('[ ,]', ssl_allow_opt)
|
||||
access_to = access['access_to']
|
||||
if access_to in ssl_allow:
|
||||
LOG.warn(_LW("Access to %(share)s at %(export)s is already "
|
||||
"granted for %(access_to)s. GlusterFS volume "
|
||||
"options might have been changed externally."),
|
||||
{'share': share['id'], 'export': exp_locn,
|
||||
'access_to': access_to})
|
||||
return
|
||||
|
||||
ssl_allow.append(access_to)
|
||||
ssl_allow_opt = ','.join(ssl_allow)
|
||||
try:
|
||||
gluster_mgr.gluster_call(
|
||||
'volume', 'set', gluster_mgr.volume,
|
||||
AUTH_SSL_ALLOW, access['access_to'])
|
||||
AUTH_SSL_ALLOW, ssl_allow_opt)
|
||||
except exception.ProcessExecutionError as exc:
|
||||
msg = (_("Error in gluster volume set during allow access. "
|
||||
"Volume: %(volname)s, Option: %(option)s, "
|
||||
"access_to: %(access_to)s, Error: %(error)s"),
|
||||
{'volname': gluster_mgr.volume,
|
||||
'option': AUTH_SSL_ALLOW,
|
||||
'access_to': access['access_to'], 'error': exc.stderr})
|
||||
'option': AUTH_SSL_ALLOW, 'access_to': access_to,
|
||||
'error': exc.stderr})
|
||||
LOG.error(msg)
|
||||
raise exception.GlusterfsException(msg)
|
||||
|
||||
|
|
@ -668,6 +703,7 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
# set dynamically.
|
||||
self._restart_gluster_vol(gluster_mgr)
|
||||
|
||||
@utils.synchronized("glusterfs_native_access", external=False)
|
||||
def deny_access(self, context, share, access, share_server=None):
|
||||
"""Deny access to a share that's using cert based auth.
|
||||
|
||||
|
|
@ -681,16 +717,30 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
exp_locn = share.get('export_location', None)
|
||||
gluster_mgr = self.gluster_used_vols_dict.get(exp_locn)
|
||||
|
||||
ssl_allow_opt = gluster_mgr.get_gluster_vol_option(AUTH_SSL_ALLOW)
|
||||
ssl_allow = re.split('[ ,]', ssl_allow_opt)
|
||||
access_to = access['access_to']
|
||||
if access_to not in ssl_allow:
|
||||
LOG.warn(_LW("Access to %(share)s at %(export)s is already "
|
||||
"denied for %(access_to)s. GlusterFS volume "
|
||||
"options might have been changed externally."),
|
||||
{'share': share['id'], 'export': exp_locn,
|
||||
'access_to': access_to})
|
||||
return
|
||||
|
||||
ssl_allow.remove(access_to)
|
||||
ssl_allow_opt = ','.join(ssl_allow)
|
||||
try:
|
||||
gluster_mgr.gluster_call(
|
||||
'volume', 'reset', gluster_mgr.volume,
|
||||
AUTH_SSL_ALLOW)
|
||||
'volume', 'set', gluster_mgr.volume,
|
||||
AUTH_SSL_ALLOW, ssl_allow_opt)
|
||||
except exception.ProcessExecutionError as exc:
|
||||
msg = (_("Error in gluster volume reset during deny access. "
|
||||
msg = (_("Error in gluster volume set during deny access. "
|
||||
"Volume: %(volname)s, Option: %(option)s, "
|
||||
"Error: %(error)s"),
|
||||
"access_to: %(access_to)s, Error: %(error)s"),
|
||||
{'volname': gluster_mgr.volume,
|
||||
'option': AUTH_SSL_ALLOW, 'error': exc.stderr})
|
||||
'option': AUTH_SSL_ALLOW, 'access_to': access_to,
|
||||
'error': exc.stderr})
|
||||
LOG.error(msg)
|
||||
raise exception.GlusterfsException(msg)
|
||||
|
||||
|
|
|
|||
|
|
@ -222,8 +222,11 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
gmgr = glusterfs.GlusterManager
|
||||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
self._driver._glustermanager = mock.Mock(return_value=gmgr1)
|
||||
self.mock_object(gmgr1, 'get_gluster_vol_option',
|
||||
mock.Mock(return_value='some.common.name'))
|
||||
|
||||
ret = self._driver._setup_gluster_vol(gmgr1.volume)
|
||||
gmgr1.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow')
|
||||
gmgr1.gluster_call.has_calls(
|
||||
mock.call(*test_args[0]),
|
||||
mock.call(*test_args[1]),
|
||||
|
|
@ -247,15 +250,33 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
gmgr = glusterfs.GlusterManager
|
||||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
self._driver._glustermanager = mock.Mock(return_value=gmgr1)
|
||||
self.mock_object(gmgr1, 'get_gluster_vol_option',
|
||||
mock.Mock(return_value='some.common.name'))
|
||||
self.mock_object(gmgr1, 'gluster_call',
|
||||
mock.Mock(side_effect=raise_exception))
|
||||
|
||||
self.assertRaises(exception.GlusterfsException,
|
||||
self._driver._setup_gluster_vol, gmgr1.volume)
|
||||
gmgr1.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow')
|
||||
self.assertTrue(
|
||||
mock.call(*test_args[idx]) in gmgr1.gluster_call.call_args_list)
|
||||
self.assertFalse(self._driver._restart_gluster_vol.called)
|
||||
|
||||
def test_setup_gluster_vol_no_ssl_allow(self):
|
||||
self._driver._restart_gluster_vol = mock.Mock()
|
||||
|
||||
gmgr = glusterfs.GlusterManager
|
||||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
self._driver._glustermanager = mock.Mock(return_value=gmgr1)
|
||||
self.mock_object(gmgr1, 'get_gluster_vol_option',
|
||||
mock.Mock(return_value=None))
|
||||
|
||||
self.assertRaises(exception.GlusterfsException,
|
||||
self._driver._setup_gluster_vol, gmgr1.volume)
|
||||
gmgr1.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow')
|
||||
self.assertFalse(gmgr1.gluster_call.called)
|
||||
self.assertFalse(self._driver._restart_gluster_vol.called)
|
||||
|
||||
def test_restart_gluster_vol(self):
|
||||
gmgr = glusterfs.GlusterManager
|
||||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
|
|
@ -831,13 +852,33 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
|
||||
gmgr = glusterfs.GlusterManager
|
||||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
self.mock_object(gmgr1, 'get_gluster_vol_option',
|
||||
mock.Mock(return_value='some.common.name'))
|
||||
test_args = ('volume', 'set', 'gv1', 'auth.ssl-allow',
|
||||
access['access_to'])
|
||||
'some.common.name,' + access['access_to'])
|
||||
|
||||
self._driver.gluster_used_vols_dict = {self.glusterfs_target1: gmgr1}
|
||||
|
||||
self._driver.allow_access(self._context, self.share1, access)
|
||||
gmgr1.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow')
|
||||
gmgr1.gluster_call.assert_called_once_with(*test_args)
|
||||
self._driver._restart_gluster_vol.assert_called_once_with(gmgr1)
|
||||
|
||||
def test_allow_access_with_share_having_access(self):
|
||||
self._driver._restart_gluster_vol = mock.Mock()
|
||||
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
|
||||
gmgr = glusterfs.GlusterManager
|
||||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
self.mock_object(
|
||||
gmgr1, 'get_gluster_vol_option',
|
||||
mock.Mock(return_value='some.common.name,' + access['access_to']))
|
||||
|
||||
self._driver.gluster_used_vols_dict = {self.glusterfs_target1: gmgr1}
|
||||
|
||||
self._driver.allow_access(self._context, self.share1, access)
|
||||
gmgr1.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow')
|
||||
self.assertFalse(gmgr1.gluster_call.called)
|
||||
self.assertFalse(self._driver._restart_gluster_vol.called)
|
||||
|
||||
def test_allow_access_invalid_access_type(self):
|
||||
self._driver._restart_gluster_vol = mock.Mock()
|
||||
|
|
@ -853,7 +894,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
def test_allow_access_excp(self):
|
||||
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
|
||||
test_args = ('volume', 'set', 'gv1', 'auth.ssl-allow',
|
||||
access['access_to'])
|
||||
'some.common.name,' + access['access_to'])
|
||||
|
||||
def raise_exception(*args, **kwargs):
|
||||
if (args == test_args):
|
||||
|
|
@ -863,6 +904,8 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
|
||||
gmgr = glusterfs.GlusterManager
|
||||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
self.mock_object(gmgr1, 'get_gluster_vol_option',
|
||||
mock.Mock(return_value='some.common.name'))
|
||||
self._driver.gluster_used_vols_dict = {self.glusterfs_target1: gmgr1}
|
||||
|
||||
self.mock_object(gmgr1, 'gluster_call',
|
||||
|
|
@ -871,22 +914,43 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
self.assertRaises(exception.GlusterfsException,
|
||||
self._driver.allow_access,
|
||||
self._context, self.share1, access)
|
||||
gmgr1.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow')
|
||||
gmgr1.gluster_call.assert_called_once_with(*test_args)
|
||||
self.assertFalse(self._driver._restart_gluster_vol.called)
|
||||
|
||||
def test_deny_access(self):
|
||||
self._driver._restart_gluster_vol = mock.Mock()
|
||||
access = {'access_type': 'cert', 'access_to': 'NotApplicable'}
|
||||
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
|
||||
gmgr = glusterfs.GlusterManager
|
||||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
test_args = ('volume', 'reset', 'gv1', 'auth.ssl-allow')
|
||||
self.mock_object(
|
||||
gmgr1, 'get_gluster_vol_option',
|
||||
mock.Mock(return_value='some.common.name,' + access['access_to']))
|
||||
test_args = ('volume', 'set', 'gv1', 'auth.ssl-allow',
|
||||
'some.common.name')
|
||||
|
||||
self._driver.gluster_used_vols_dict = {self.glusterfs_target1: gmgr1}
|
||||
|
||||
self._driver.deny_access(self._context, self.share1, access)
|
||||
gmgr1.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow')
|
||||
gmgr1.gluster_call.assert_called_once_with(*test_args)
|
||||
self._driver._restart_gluster_vol.assert_called_once_with(gmgr1)
|
||||
|
||||
def test_deny_access_with_share_having_no_access(self):
|
||||
self._driver._restart_gluster_vol = mock.Mock()
|
||||
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
|
||||
gmgr = glusterfs.GlusterManager
|
||||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
self.mock_object(gmgr1, 'get_gluster_vol_option',
|
||||
mock.Mock(return_value='some.common.name'))
|
||||
|
||||
self._driver.gluster_used_vols_dict = {self.glusterfs_target1: gmgr1}
|
||||
|
||||
self._driver.deny_access(self._context, self.share1, access)
|
||||
gmgr1.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow')
|
||||
self.assertFalse(gmgr1.gluster_call.called)
|
||||
self.assertFalse(self._driver._restart_gluster_vol.called)
|
||||
|
||||
def test_deny_access_invalid_access_type(self):
|
||||
self._driver._restart_gluster_vol = mock.Mock()
|
||||
access = {'access_type': 'invalid', 'access_to': 'NotApplicable'}
|
||||
|
|
@ -897,8 +961,9 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
self.assertFalse(self._driver._restart_gluster_vol.called)
|
||||
|
||||
def test_deny_access_excp(self):
|
||||
access = {'access_type': 'cert', 'access_to': 'NotApplicable'}
|
||||
test_args = ('volume', 'reset', 'gv1', 'auth.ssl-allow')
|
||||
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
|
||||
test_args = ('volume', 'set', 'gv1', 'auth.ssl-allow',
|
||||
'some.common.name')
|
||||
|
||||
def raise_exception(*args, **kwargs):
|
||||
if (args == test_args):
|
||||
|
|
@ -908,6 +973,9 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
|
||||
gmgr = glusterfs.GlusterManager
|
||||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
self.mock_object(
|
||||
gmgr1, 'get_gluster_vol_option',
|
||||
mock.Mock(return_value='some.common.name,' + access['access_to']))
|
||||
self._driver.gluster_used_vols_dict = {self.glusterfs_target1: gmgr1}
|
||||
|
||||
self.mock_object(gmgr1, 'gluster_call',
|
||||
|
|
@ -916,6 +984,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
self.assertRaises(exception.GlusterfsException,
|
||||
self._driver.deny_access,
|
||||
self._context, self.share1, access)
|
||||
gmgr1.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow')
|
||||
gmgr1.gluster_call.assert_called_once_with(*test_args)
|
||||
self.assertFalse(self._driver._restart_gluster_vol.called)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue