Merge "Ignore replicas in error state during allow/deny access."
This commit is contained in:
commit
a69bccefe5
@ -188,6 +188,8 @@ REST_API_VERSION_HISTORY = """
|
|||||||
* 2.71 - Added 'updated_at' field in share instance show API output.
|
* 2.71 - Added 'updated_at' field in share instance show API output.
|
||||||
* 2.72 - Added new option ``share-network`` to share replica creare API.
|
* 2.72 - Added new option ``share-network`` to share replica creare API.
|
||||||
* 2.73 - Added Share Snapshot Metadata to Metadata API
|
* 2.73 - Added Share Snapshot Metadata to Metadata API
|
||||||
|
* 2.74 - Allow/deny share access rule even if share replicas are in
|
||||||
|
'error' state.
|
||||||
|
|
||||||
"""
|
"""
|
||||||
|
|
||||||
@ -195,7 +197,7 @@ REST_API_VERSION_HISTORY = """
|
|||||||
# The default api version request is defined to be the
|
# The default api version request is defined to be the
|
||||||
# minimum version of the API supported.
|
# minimum version of the API supported.
|
||||||
_MIN_API_VERSION = "2.0"
|
_MIN_API_VERSION = "2.0"
|
||||||
_MAX_API_VERSION = "2.73"
|
_MAX_API_VERSION = "2.74"
|
||||||
DEFAULT_API_VERSION = _MIN_API_VERSION
|
DEFAULT_API_VERSION = _MIN_API_VERSION
|
||||||
|
|
||||||
|
|
||||||
|
@ -406,3 +406,7 @@ ____
|
|||||||
---------------------
|
---------------------
|
||||||
Added Metadata API methods (GET, PUT, POST, DELETE)
|
Added Metadata API methods (GET, PUT, POST, DELETE)
|
||||||
to Share Snapshots
|
to Share Snapshots
|
||||||
|
|
||||||
|
2.74
|
||||||
|
----
|
||||||
|
Allow/deny share access rule even if share replicas are in 'error' state.
|
||||||
|
@ -450,7 +450,7 @@ class ShareMixin(object):
|
|||||||
@wsgi.Controller.authorize('allow_access')
|
@wsgi.Controller.authorize('allow_access')
|
||||||
def _allow_access(self, req, id, body, enable_ceph=False,
|
def _allow_access(self, req, id, body, enable_ceph=False,
|
||||||
allow_on_error_status=False, enable_ipv6=False,
|
allow_on_error_status=False, enable_ipv6=False,
|
||||||
enable_metadata=False):
|
enable_metadata=False, allow_on_error_state=False):
|
||||||
"""Add share access rule."""
|
"""Add share access rule."""
|
||||||
context = req.environ['manila.context']
|
context = req.environ['manila.context']
|
||||||
access_data = body.get('allow_access', body.get('os-allow_access'))
|
access_data = body.get('allow_access', body.get('os-allow_access'))
|
||||||
@ -488,7 +488,8 @@ class ShareMixin(object):
|
|||||||
try:
|
try:
|
||||||
access = self.share_api.allow_access(
|
access = self.share_api.allow_access(
|
||||||
context, share, access_type, access_to,
|
context, share, access_type, access_to,
|
||||||
access_data.get('access_level'), access_data.get('metadata'))
|
access_data.get('access_level'), access_data.get('metadata'),
|
||||||
|
allow_on_error_state)
|
||||||
except exception.ShareAccessExists as e:
|
except exception.ShareAccessExists as e:
|
||||||
raise webob.exc.HTTPBadRequest(explanation=e.msg)
|
raise webob.exc.HTTPBadRequest(explanation=e.msg)
|
||||||
|
|
||||||
@ -501,7 +502,7 @@ class ShareMixin(object):
|
|||||||
return self._access_view_builder.view(req, access)
|
return self._access_view_builder.view(req, access)
|
||||||
|
|
||||||
@wsgi.Controller.authorize('deny_access')
|
@wsgi.Controller.authorize('deny_access')
|
||||||
def _deny_access(self, req, id, body):
|
def _deny_access(self, req, id, body, allow_on_error_state=False):
|
||||||
"""Remove share access rule."""
|
"""Remove share access rule."""
|
||||||
context = req.environ['manila.context']
|
context = req.environ['manila.context']
|
||||||
|
|
||||||
@ -528,7 +529,8 @@ class ShareMixin(object):
|
|||||||
share = self.share_api.get(context, id)
|
share = self.share_api.get(context, id)
|
||||||
except exception.NotFound as error:
|
except exception.NotFound as error:
|
||||||
raise webob.exc.HTTPNotFound(explanation=error.message)
|
raise webob.exc.HTTPNotFound(explanation=error.message)
|
||||||
self.share_api.deny_access(context, share, access)
|
self.share_api.deny_access(context, share, access,
|
||||||
|
allow_on_error_state)
|
||||||
return webob.Response(status_int=http_client.ACCEPTED)
|
return webob.Response(status_int=http_client.ACCEPTED)
|
||||||
|
|
||||||
def _access_list(self, req, id, body):
|
def _access_list(self, req, id, body):
|
||||||
|
@ -472,6 +472,8 @@ class ShareController(wsgi.Controller,
|
|||||||
kwargs['enable_ipv6'] = True
|
kwargs['enable_ipv6'] = True
|
||||||
if req.api_version_request >= api_version.APIVersionRequest("2.45"):
|
if req.api_version_request >= api_version.APIVersionRequest("2.45"):
|
||||||
kwargs['enable_metadata'] = True
|
kwargs['enable_metadata'] = True
|
||||||
|
if req.api_version_request >= api_version.APIVersionRequest("2.74"):
|
||||||
|
kwargs['allow_on_error_state'] = True
|
||||||
return self._allow_access(*args, **kwargs)
|
return self._allow_access(*args, **kwargs)
|
||||||
|
|
||||||
@wsgi.Controller.api_version('2.0', '2.6')
|
@wsgi.Controller.api_version('2.0', '2.6')
|
||||||
@ -484,7 +486,11 @@ class ShareController(wsgi.Controller,
|
|||||||
@wsgi.action('deny_access')
|
@wsgi.action('deny_access')
|
||||||
def deny_access(self, req, id, body):
|
def deny_access(self, req, id, body):
|
||||||
"""Remove share access rule."""
|
"""Remove share access rule."""
|
||||||
return self._deny_access(req, id, body)
|
args = (req, id, body)
|
||||||
|
kwargs = {}
|
||||||
|
if req.api_version_request >= api_version.APIVersionRequest("2.74"):
|
||||||
|
kwargs['allow_on_error_state'] = True
|
||||||
|
return self._deny_access(*args, **kwargs)
|
||||||
|
|
||||||
@wsgi.Controller.api_version('2.0', '2.6')
|
@wsgi.Controller.api_version('2.0', '2.6')
|
||||||
@wsgi.action('os-access_list')
|
@wsgi.action('os-access_list')
|
||||||
|
@ -132,7 +132,7 @@ TRANSITIONAL_STATUSES = (
|
|||||||
)
|
)
|
||||||
|
|
||||||
INVALID_SHARE_INSTANCE_STATUSES_FOR_ACCESS_RULE_UPDATES = (
|
INVALID_SHARE_INSTANCE_STATUSES_FOR_ACCESS_RULE_UPDATES = (
|
||||||
TRANSITIONAL_STATUSES + (STATUS_ERROR,)
|
TRANSITIONAL_STATUSES
|
||||||
)
|
)
|
||||||
|
|
||||||
SUPPORTED_SHARE_PROTOCOLS = (
|
SUPPORTED_SHARE_PROTOCOLS = (
|
||||||
|
@ -2199,13 +2199,20 @@ class API(base.Base):
|
|||||||
return self.db.share_snapshot_get_latest_for_share(context, share_id)
|
return self.db.share_snapshot_get_latest_for_share(context, share_id)
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _is_invalid_share_instance(instance):
|
def _any_invalid_share_instance(share, allow_on_error_state=False):
|
||||||
return (instance['host'] is None
|
invalid_states = (
|
||||||
or instance['status'] in constants.
|
constants.INVALID_SHARE_INSTANCE_STATUSES_FOR_ACCESS_RULE_UPDATES)
|
||||||
INVALID_SHARE_INSTANCE_STATUSES_FOR_ACCESS_RULE_UPDATES)
|
if not allow_on_error_state:
|
||||||
|
invalid_states += (constants.STATUS_ERROR,)
|
||||||
|
|
||||||
|
for instance in share.instances:
|
||||||
|
if (not instance['host'] or instance['status'] in invalid_states):
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
def allow_access(self, ctx, share, access_type, access_to,
|
def allow_access(self, ctx, share, access_type, access_to,
|
||||||
access_level=None, metadata=None):
|
access_level=None, metadata=None,
|
||||||
|
allow_on_error_state=False):
|
||||||
"""Allow access to share."""
|
"""Allow access to share."""
|
||||||
|
|
||||||
# Access rule validation:
|
# Access rule validation:
|
||||||
@ -2221,9 +2228,7 @@ class API(base.Base):
|
|||||||
raise exception.ShareAccessExists(access_type=access_type,
|
raise exception.ShareAccessExists(access_type=access_type,
|
||||||
access=access_to)
|
access=access_to)
|
||||||
|
|
||||||
# Share instance validation
|
if self._any_invalid_share_instance(share, allow_on_error_state):
|
||||||
if any(instance for instance in share.instances
|
|
||||||
if self._is_invalid_share_instance(instance)):
|
|
||||||
msg = _("New access rules cannot be applied while the share or "
|
msg = _("New access rules cannot be applied while the share or "
|
||||||
"any of its replicas or migration copies lacks a valid "
|
"any of its replicas or migration copies lacks a valid "
|
||||||
"host or is in an invalid state.")
|
"host or is in an invalid state.")
|
||||||
@ -2258,11 +2263,10 @@ class API(base.Base):
|
|||||||
context, conditionally_change=conditionally_change,
|
context, conditionally_change=conditionally_change,
|
||||||
share_instance_id=share_instance['id'])
|
share_instance_id=share_instance['id'])
|
||||||
|
|
||||||
def deny_access(self, ctx, share, access):
|
def deny_access(self, ctx, share, access, allow_on_error_state=False):
|
||||||
"""Deny access to share."""
|
"""Deny access to share."""
|
||||||
|
|
||||||
if any(instance for instance in share.instances if
|
if self._any_invalid_share_instance(share, allow_on_error_state):
|
||||||
self._is_invalid_share_instance(instance)):
|
|
||||||
msg = _("Access rules cannot be denied while the share, "
|
msg = _("Access rules cannot be denied while the share, "
|
||||||
"any of its replicas or migration copies lacks a valid "
|
"any of its replicas or migration copies lacks a valid "
|
||||||
"host or is in an invalid state.")
|
"host or is in an invalid state.")
|
||||||
|
@ -2451,7 +2451,7 @@ class ShareActionsTest(test.TestCase):
|
|||||||
self.assertEqual(expected_access, access['access'])
|
self.assertEqual(expected_access, access['access'])
|
||||||
share_api.API.allow_access.assert_called_once_with(
|
share_api.API.allow_access.assert_called_once_with(
|
||||||
req.environ['manila.context'], share, 'user',
|
req.environ['manila.context'], share, 'user',
|
||||||
'clemsontigers', 'rw', None)
|
'clemsontigers', 'rw', None, False)
|
||||||
|
|
||||||
@ddt.data(*itertools.product(
|
@ddt.data(*itertools.product(
|
||||||
set(['2.28', api_version._MAX_API_VERSION]),
|
set(['2.28', api_version._MAX_API_VERSION]),
|
||||||
@ -2498,10 +2498,17 @@ class ShareActionsTest(test.TestCase):
|
|||||||
{
|
{
|
||||||
'metadata': {},
|
'metadata': {},
|
||||||
})
|
})
|
||||||
|
|
||||||
|
if api_version.APIVersionRequest(version) >= (
|
||||||
|
api_version.APIVersionRequest("2.74")):
|
||||||
|
allow_on_error_state = True
|
||||||
|
else:
|
||||||
|
allow_on_error_state = False
|
||||||
|
|
||||||
self.assertEqual(expected_access, access['access'])
|
self.assertEqual(expected_access, access['access'])
|
||||||
share_api.API.allow_access.assert_called_once_with(
|
share_api.API.allow_access.assert_called_once_with(
|
||||||
req.environ['manila.context'], share, 'user',
|
req.environ['manila.context'], share, 'user',
|
||||||
'clemsontigers', 'rw', None)
|
'clemsontigers', 'rw', None, allow_on_error_state)
|
||||||
|
|
||||||
def test_deny_access(self):
|
def test_deny_access(self):
|
||||||
def _stub_deny_access(*args, **kwargs):
|
def _stub_deny_access(*args, **kwargs):
|
||||||
|
@ -0,0 +1,8 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
With replication setup at least one backend is working and serving the
|
||||||
|
shares. Starting from API version 2.74, allowing and denying access to
|
||||||
|
shares will only fail if any of the instances is in a transitional state.
|
||||||
|
Please refer to
|
||||||
|
`Launchpad bug 1965561 <https://bugs.launchpad.net/manila/+bug/1965561>`_
|
Loading…
Reference in New Issue
Block a user