diff --git a/cinder/api/openstack/wsgi.py b/cinder/api/openstack/wsgi.py index 671a04bf5c3..f54cfee8c89 100644 --- a/cinder/api/openstack/wsgi.py +++ b/cinder/api/openstack/wsgi.py @@ -21,6 +21,7 @@ import inspect import math import time +from oslo_config import cfg from oslo_log import log as logging from oslo_serialization import jsonutils from oslo_utils import encodeutils @@ -39,6 +40,7 @@ from cinder.i18n import _ from cinder import utils from cinder.wsgi import common as wsgi +CONF = cfg.CONF LOG = logging.getLogger(__name__) SUPPORTED_CONTENT_TYPES = ( @@ -1292,6 +1294,7 @@ class Controller(object, metaclass=ControllerMetaclass): class APIMapper(routes.Mapper): + def routematch(self, url=None, environ=None): if url == "": result = self._match("", environ) @@ -1306,6 +1309,26 @@ class APIMapper(routes.Mapper): kwargs['requirements']['format'] = 'json' return routes.Mapper.connect(self, *args, **kwargs) + def create_route(self, path, method, controller, action): + # NOTE: project_id parameter is only valid if its hex or hex + dashes + # (note, integers are a subset of this). This is required to handle + # our overlapping routes issues. + project_id_regex = CONF.project_id_regex + project_id_token = '{project_id:%s}' % project_id_regex + + self.connect( + '/%s%s' % (project_id_token, path), + conditions={"method": [method]}, + controller=controller, + action=action, + ) + self.connect( + path, + conditions={"method": [method]}, + controller=controller, + action=action, + ) + class Fault(webob.exc.HTTPException): """Wrap webob.exc.HTTPException to provide API friendly response.""" diff --git a/cinder/api/v3/clusters.py b/cinder/api/v3/clusters.py index a54111196b3..d3d40397325 100644 --- a/cinder/api/v3/clusters.py +++ b/cinder/api/v3/clusters.py @@ -15,7 +15,7 @@ from cinder.api import microversions as mv from cinder.api.openstack import wsgi -from cinder.api.schemas import clusters as cluster +from cinder.api.schemas import clusters as schema from cinder.api.v3.views import clusters as clusters_view from cinder.api import validation from cinder.common import constants @@ -90,28 +90,14 @@ class ClusterController(wsgi.Controller): return clusters_view.ViewBuilder.list(clusters, detail, replication_data) - @wsgi.Controller.api_version(mv.CLUSTER_SUPPORT) - def update(self, req, id, body): + def _update( + self, context, name, binary, disabled, disabled_reason=None, + show_replication_data=False, + ): """Enable/Disable scheduling for a cluster.""" # NOTE(geguileo): This method tries to be consistent with services # update endpoint API. - # Let the wsgi middleware convert NotAuthorized exceptions - context = req.environ['cinder.context'] - context.authorize(policy.UPDATE_POLICY) - - if id not in ('enable', 'disable'): - raise exception.NotFound(message=_("Unknown action")) - - disabled = id != 'enable' - disabled_reason = self._disable_cluster( - req, body=body) if disabled else self._enable_cluster( - req, body=body) - - name = body['name'] - - binary = body.get('binary', constants.VOLUME_BINARY) - # Let wsgi handle NotFound exception cluster = objects.Cluster.get_by_id(context, None, binary=binary, name=name) @@ -120,23 +106,56 @@ class ClusterController(wsgi.Controller): cluster.save() # We return summary data plus the disabled reason - replication_data = req.api_version_request.matches( - mv.REPLICATION_CLUSTER) - ret_val = clusters_view.ViewBuilder.summary(cluster, replication_data) + ret_val = clusters_view.ViewBuilder.summary( + cluster, show_replication_data, + ) ret_val['cluster']['disabled_reason'] = disabled_reason return ret_val - @validation.schema(cluster.disable_cluster) - def _disable_cluster(self, req, body): - reason = body.get('disabled_reason') - if reason: - reason = reason.strip() - return reason + @wsgi.Controller.api_version(mv.CLUSTER_SUPPORT) + @validation.schema(schema.disable_cluster) + def disable(self, req, body): + # NOTE(geguileo): This method tries to be consistent with services + # disable endpoint API. - @validation.schema(cluster.enable_cluster) - def _enable_cluster(self, req, body): - pass + # Let the wsgi middleware convert NotAuthorized exceptions + context = req.environ['cinder.context'] + context.authorize(policy.UPDATE_POLICY) + + name = body['name'] + binary = body.get('binary', constants.VOLUME_BINARY) + disabled_reason = body.get('disabled_reason') + if disabled_reason: + disabled_reason = disabled_reason.strip() + + show_replication_data = req.api_version_request.matches( + mv.REPLICATION_CLUSTER) + + return self._update( + context, name=name, binary=binary, disabled=True, + disabled_reason=disabled_reason, + show_replication_data=show_replication_data) + + @wsgi.Controller.api_version(mv.CLUSTER_SUPPORT) + @validation.schema(schema.enable_cluster) + def enable(self, req, body): + # NOTE(geguileo): This method tries to be consistent with services + # enable endpoint API. + + # Let the wsgi middleware convert NotAuthorized exceptions + context = req.environ['cinder.context'] + context.authorize(policy.UPDATE_POLICY) + + name = body['name'] + binary = body.get('binary', constants.VOLUME_BINARY) + + show_replication_data = req.api_version_request.matches( + mv.REPLICATION_CLUSTER) + + return self._update( + context, name=name, binary=binary, disabled=False, + show_replication_data=show_replication_data) def create_resource(): diff --git a/cinder/api/v3/router.py b/cinder/api/v3/router.py index 7baa1ab2bd3..6774cb550d5 100644 --- a/cinder/api/v3/router.py +++ b/cinder/api/v3/router.py @@ -181,9 +181,16 @@ class APIRouter(base_wsgi.Router): controller=self.resources['messages']) self.resources['clusters'] = clusters.create_resource() - mapper.resource('cluster', 'clusters', - controller=self.resources['clusters'], - collection={'detail': 'GET'}) + mapper.create_route( + '/clusters', 'GET', self.resources['clusters'], 'index') + mapper.create_route( + '/clusters/detail', 'GET', self.resources['clusters'], 'detail') + mapper.create_route( + '/clusters/{id}', 'GET', self.resources['clusters'], 'show') + mapper.create_route( + '/clusters/enable', 'PUT', self.resources['clusters'], 'enable') + mapper.create_route( + '/clusters/disable', 'PUT', self.resources['clusters'], 'disable') self.resources['types'] = types.create_resource() mapper.resource("type", "types", diff --git a/cinder/tests/unit/api/v3/test_cluster.py b/cinder/tests/unit/api/v3/test_cluster.py index 86a27f86cd9..ea1278f45b9 100644 --- a/cinder/tests/unit/api/v3/test_cluster.py +++ b/cinder/tests/unit/api/v3/test_cluster.py @@ -305,16 +305,15 @@ class ClustersTestCase(test.TestCase): side_effect=fake_db_api_cluster_update) @mock.patch('cinder.db.sqlalchemy.api.cluster_get', side_effect=fake_db_api_cluster_get) - def test_update_enable(self, get_mock, update_mock): + def test_enable(self, get_mock, update_mock): req = FakeRequest() expected = {'cluster': {'name': 'cluster_name', 'binary': 'cinder-volume', 'state': 'up', 'status': 'enabled', 'disabled_reason': None}} - res = self.controller.update(req, 'enable', - body={'name': 'cluster_name', - 'binary': 'cinder-volume'}) + res = self.controller.enable( + req, body={'name': 'cluster_name', 'binary': 'cinder-volume'}) self.assertEqual(expected, res) ctxt = req.environ['cinder.context'] get_mock.assert_called_once_with(ctxt, @@ -328,7 +327,7 @@ class ClustersTestCase(test.TestCase): side_effect=fake_db_api_cluster_update) @mock.patch('cinder.db.sqlalchemy.api.cluster_get', side_effect=fake_db_api_cluster_get) - def test_update_disable(self, get_mock, update_mock): + def test_disable(self, get_mock, update_mock): req = FakeRequest() disabled_reason = 'For testing' expected = {'cluster': {'name': 'cluster_name', @@ -336,10 +335,9 @@ class ClustersTestCase(test.TestCase): 'binary': 'cinder-volume', 'status': 'disabled', 'disabled_reason': disabled_reason}} - res = self.controller.update(req, 'disable', - body={'name': 'cluster_name', - 'binary': 'cinder-volume', - 'disabled_reason': disabled_reason}) + res = self.controller.disable( + req, body={'name': 'cluster_name', 'binary': 'cinder-volume', + 'disabled_reason': disabled_reason}) self.assertEqual(expected, res) ctxt = req.environ['cinder.context'] get_mock.assert_called_once_with(ctxt, @@ -349,46 +347,66 @@ class ClustersTestCase(test.TestCase): ctxt, 1, {'disabled': True, 'disabled_reason': disabled_reason}) - def test_update_wrong_action(self): + def test_enable_missing_name(self): req = FakeRequest() - self.assertRaises(exception.NotFound, self.controller.update, - req, 'action', body={'name': 'cluster_name'}) + self.assertRaises(exception.ValidationError, self.controller.enable, + req, body={'binary': 'cinder-volume'}) - @ddt.data('enable', 'disable') - def test_update_missing_name(self, action): + def test_disable_missing_name(self): req = FakeRequest() - self.assertRaises(exception.ValidationError, self.controller.update, - req, action, body={'binary': 'cinder-volume'}) + self.assertRaises( + exception.ValidationError, + self.controller.disable, + req, + body={'binary': 'cinder-volume'}) - def test_update_with_binary_more_than_255_characters(self): + def test_enable_with_binary_more_than_255_characters(self): req = FakeRequest() - self.assertRaises(exception.ValidationError, self.controller.update, - req, 'enable', body={'name': 'cluster_name', - 'binary': 'a' * 256}) + self.assertRaises( + exception.ValidationError, + self.controller.enable, + req, + body={'name': 'cluster_name', 'binary': 'a' * 256}) - def test_update_with_name_more_than_255_characters(self): + def test_enable_with_name_more_than_255_characters(self): req = FakeRequest() - self.assertRaises(exception.ValidationError, self.controller.update, - req, 'enable', body={'name': 'a' * 256, - 'binary': 'cinder-volume'}) + self.assertRaises( + exception.ValidationError, + self.controller.enable, + req, + body={'name': 'a' * 256, 'binary': 'cinder-volume'}) @ddt.data('a' * 256, ' ') - def test_update_wrong_disabled_reason(self, disabled_reason): + def test_enable_wrong_disabled_reason(self, disabled_reason): req = FakeRequest() - self.assertRaises(exception.ValidationError, self.controller.update, - req, 'disable', - body={'name': 'cluster_name', - 'disabled_reason': disabled_reason}) + self.assertRaises( + exception.ValidationError, + self.controller.enable, + req, + body={'name': 'cluster_name', 'disabled_reason': disabled_reason}) - @ddt.data('enable', 'disable') - def test_update_unauthorized(self, action): + def test_enable_unauthorized(self): req = FakeRequest(is_admin=False) - self.assertRaises(exception.PolicyNotAuthorized, - self.controller.update, req, action, - body={'name': 'fake_name'}) + self.assertRaises( + exception.PolicyNotAuthorized, + self.controller.enable, + req, body={'name': 'fake_name'}) - @ddt.data('enable', 'disable') - def test_update_wrong_version(self, action): + def test_disable_unauthorized(self): + req = FakeRequest(is_admin=False) + self.assertRaises( + exception.PolicyNotAuthorized, + self.controller.disable, + req, body={'name': 'fake_name'}) + + def test_enable_wrong_version(self): req = FakeRequest(version=mv.get_prior_version(mv.CLUSTER_SUPPORT)) - self.assertRaises(exception.VersionNotFoundForAPIMethod, - self.controller.update, req, action, {}) + self.assertRaises( + exception.VersionNotFoundForAPIMethod, + self.controller.enable, req, {}) + + def test_disable_wrong_version(self): + req = FakeRequest(version=mv.get_prior_version(mv.CLUSTER_SUPPORT)) + self.assertRaises( + exception.VersionNotFoundForAPIMethod, + self.controller.disable, req, {})