glusterfs_native: fix parsing of the dynamic-auth option
The full name of the 'dynamic-auth' GlusterFS option is 'server.dynamic-auth'. When we started to make use of it in https://review.openstack.org/229409, we just used its short form, 'dynamic-auth'. That actually *should* work but due to some weakness in the GlusterFS instrumentation code, it doesn't. Fix the situation for now by using the full option name. We also take the opportunity to remove some redundant checks from the respective option handling code. Change-Id: I6f30060cec38733afa1211cb8e7bc57b66dead34 Closes-Bug: #1506577
This commit is contained in:
Csaba Henk
parent
cb4db916d3
commit
c38d496953
|
|
@ -45,7 +45,7 @@ AUTH_SSL_ALLOW = 'auth.ssl-allow'
|
|||
CLIENT_SSL = 'client.ssl'
|
||||
NFS_EXPORT_VOL = 'nfs.export-volumes'
|
||||
SERVER_SSL = 'server.ssl'
|
||||
DYNAMIC_AUTH = 'dynamic-auth'
|
||||
DYNAMIC_AUTH = 'server.dynamic-auth'
|
||||
|
||||
|
||||
class GlusterfsNativeShareDriver(driver.ExecuteMixin,
|
||||
|
|
@ -245,19 +245,13 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin,
|
|||
try:
|
||||
dynauth = gluster_mgr.get_gluster_vol_option(DYNAMIC_AUTH) or 'off'
|
||||
except exception.ProcessExecutionError as exc:
|
||||
if exc.exit_code == 1:
|
||||
# dynamic auth is not supported by gluster backend
|
||||
dynauth = 'off'
|
||||
else:
|
||||
# unexpected failure
|
||||
msg = (_("Error in querying gluster volume."
|
||||
"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_to,
|
||||
'error': exc.stderr})
|
||||
LOG.error(msg)
|
||||
raise exception.GlusterfsException(msg)
|
||||
msg = (_("Error in querying gluster volume. "
|
||||
"Volume: %(volname)s, Option: %(option)s, "
|
||||
"Error: %(error)s") %
|
||||
{'volname': gluster_mgr.volume,
|
||||
'option': DYNAMIC_AUTH, 'error': exc.stderr})
|
||||
LOG.error(msg)
|
||||
raise exception.GlusterfsException(msg)
|
||||
# TODO(csaba): boolean option processing shoud be done in common
|
||||
if dynauth.lower() not in ('on', '1', 'true', 'yes', 'enable'):
|
||||
common._restart_gluster_vol(gluster_mgr)
|
||||
|
|
|
|||
|
|
@ -115,7 +115,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
('volume', 'set', 'fakevol', 'nfs.export-volumes', 'off'),
|
||||
('volume', 'set', 'fakevol', 'client.ssl', 'on'),
|
||||
('volume', 'set', 'fakevol', 'server.ssl', 'on'),
|
||||
('volume', 'set', 'fakevol', 'dynamic-auth', 'on'),
|
||||
('volume', 'set', 'fakevol', 'server.dynamic-auth', 'on'),
|
||||
('volume', 'stop', 'fakevol', '--mode=script'),
|
||||
('volume', 'start', 'fakevol'))
|
||||
gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args])
|
||||
|
|
@ -145,7 +145,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
('volume', 'set', 'fakevol', 'nfs.export-volumes', 'off'),
|
||||
('volume', 'set', 'fakevol', 'client.ssl', 'on'),
|
||||
('volume', 'set', 'fakevol', 'server.ssl', 'on'),
|
||||
('volume', 'set', 'fakevol', 'dynamic-auth', 'on'))
|
||||
('volume', 'set', 'fakevol', 'server.dynamic-auth', 'on'))
|
||||
gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args])
|
||||
self.assertEqual(ret, gmgr.export)
|
||||
|
||||
|
|
@ -262,7 +262,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
def _get_gluster_vol_option(opt):
|
||||
if opt == 'auth.ssl-allow':
|
||||
return('some.common.name,' + access['access_to'])
|
||||
elif opt == 'dynamic-auth':
|
||||
elif opt == 'server.dynamic-auth':
|
||||
return trueish
|
||||
|
||||
self.mock_object(
|
||||
|
|
@ -274,13 +274,13 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
self.share1, access)
|
||||
|
||||
gmgr1.get_gluster_vol_option.assert_has_calls(
|
||||
[mock.call(a) for a in ('auth.ssl-allow', 'dynamic-auth')])
|
||||
[mock.call(a) for a in ('auth.ssl-allow', 'server.dynamic-auth')])
|
||||
test_args = ('volume', 'set', 'gv1', 'auth.ssl-allow',
|
||||
'some.common.name')
|
||||
gmgr1.gluster_call.assert_called_once_with(*test_args)
|
||||
self.assertFalse(common._restart_gluster_vol.called)
|
||||
|
||||
@ddt.data('off', None, 'strangelove', '!')
|
||||
@ddt.data('off', None, 'strangelove')
|
||||
def test_deny_access_via_manager_no_dyn_auth(self, falseish):
|
||||
self.mock_object(common, '_restart_gluster_vol', mock.Mock())
|
||||
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
|
||||
|
|
@ -290,9 +290,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
def _get_gluster_vol_option(opt):
|
||||
if opt == 'auth.ssl-allow':
|
||||
return('some.common.name,' + access['access_to'])
|
||||
elif opt == 'dynamic-auth':
|
||||
if falseish == '!':
|
||||
raise exception.ProcessExecutionError(exit_code=1)
|
||||
elif opt == 'server.dynamic-auth':
|
||||
return falseish
|
||||
|
||||
self.mock_object(
|
||||
|
|
@ -304,7 +302,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
self.share1, access)
|
||||
|
||||
gmgr1.get_gluster_vol_option.assert_has_calls(
|
||||
[mock.call(a) for a in ('auth.ssl-allow', 'dynamic-auth')])
|
||||
[mock.call(a) for a in ('auth.ssl-allow', 'server.dynamic-auth')])
|
||||
test_args = ('volume', 'set', 'gv1', 'auth.ssl-allow',
|
||||
'some.common.name')
|
||||
gmgr1.gluster_call.assert_called_once_with(*test_args)
|
||||
|
|
@ -368,9 +366,9 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
gmgr1.gluster_call.assert_called_once_with(*test_args)
|
||||
self.assertFalse(common._restart_gluster_vol.called)
|
||||
|
||||
@ddt.data({'trouble': (exception.ProcessExecutionError, {'exit_code': 2}),
|
||||
@ddt.data({'trouble': exception.ProcessExecutionError,
|
||||
'_exception': exception.GlusterfsException},
|
||||
{'trouble': (RuntimeError, {}), '_exception': RuntimeError})
|
||||
{'trouble': RuntimeError, '_exception': RuntimeError})
|
||||
@ddt.unpack
|
||||
def test_deny_access_via_manager_dyn_auth_fail(self, trouble, _exception):
|
||||
self.mock_object(common, '_restart_gluster_vol', mock.Mock())
|
||||
|
|
@ -381,8 +379,8 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
def _get_gluster_vol_option(opt):
|
||||
if opt == 'auth.ssl-allow':
|
||||
return('some.common.name,' + access['access_to'])
|
||||
elif opt == 'dynamic-auth':
|
||||
raise trouble[0](**trouble[1])
|
||||
elif opt == 'server.dynamic-auth':
|
||||
raise trouble
|
||||
|
||||
self.mock_object(
|
||||
gmgr1, 'get_gluster_vol_option',
|
||||
|
|
@ -394,7 +392,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
self._context, self.share1, access)
|
||||
|
||||
gmgr1.get_gluster_vol_option.assert_has_calls(
|
||||
[mock.call(a) for a in ('auth.ssl-allow', 'dynamic-auth')])
|
||||
[mock.call(a) for a in ('auth.ssl-allow', 'server.dynamic-auth')])
|
||||
test_args = ('volume', 'set', 'gv1', 'auth.ssl-allow',
|
||||
'some.common.name')
|
||||
gmgr1.gluster_call.assert_called_once_with(*test_args)
|
||||
|
|
|
|||
Loading…
Reference in New Issue