From dc31763c582169509ed2f1c3cacd3b6950baa44c Mon Sep 17 00:00:00 2001 From: John Griffith Date: Sat, 11 Mar 2017 17:24:52 +0000 Subject: [PATCH] Add support for generalized filtering on list APIs This patch adds generalized filtering support for these list APIs: 1. list volume 2. list backup 3. list snapshot 4. list group 5. list group-snapshot 6. list attachment 7. list message 8. get pools DocImpact APIImpact Co-Authored-By: TommyLike Change-Id: Icee6c22621489f93614f4adf071329d8d2115637 Partial: blueprint generalized-filtering-for-cinder-list-resource --- cinder/api/common.py | 76 +++++++++++++++++++ cinder/api/contrib/backups.py | 13 +++- cinder/api/contrib/scheduler_stats.py | 20 +++-- cinder/api/openstack/api_version_request.py | 3 +- .../openstack/rest_api_version_history.rst | 4 + cinder/api/v3/attachments.py | 12 ++- cinder/api/v3/group_snapshots.py | 4 + cinder/api/v3/groups.py | 2 + cinder/api/v3/messages.py | 3 + cinder/api/v3/snapshots.py | 20 ++--- cinder/api/v3/volumes.py | 22 ++++-- cinder/tests/unit/api/test_common.py | 55 ++++++++++++++ cinder/tests/unit/api/v3/test_attachments.py | 13 ++++ cinder/tests/unit/api/v3/test_backups.py | 16 ++++ .../tests/unit/api/v3/test_group_snapshots.py | 14 ++++ cinder/tests/unit/api/v3/test_groups.py | 13 ++++ cinder/tests/unit/api/v3/test_messages.py | 16 ++++ cinder/tests/unit/api/v3/test_snapshots.py | 13 ++++ cinder/tests/unit/api/v3/test_volumes.py | 14 +++- cinder/tests/unit/message/test_api.py | 6 +- doc/source/man/generalized_filters.rst | 65 ++++++++++++++++ etc/cinder/resource_filters.json | 12 +++ ...ized-resource-filter-hg598uyvuh119008.yaml | 6 ++ 23 files changed, 390 insertions(+), 32 deletions(-) create mode 100644 doc/source/man/generalized_filters.rst create mode 100644 etc/cinder/resource_filters.json create mode 100644 releasenotes/notes/generalized-resource-filter-hg598uyvuh119008.yaml diff --git a/cinder/api/common.py b/cinder/api/common.py index 307ebdda4df..f67b6eb7be2 100644 --- a/cinder/api/common.py +++ b/cinder/api/common.py @@ -14,6 +14,7 @@ # under the License. +import json import os import re @@ -39,10 +40,16 @@ api_common_opts = [ help='Base URL that will be presented to users in links ' 'to the OpenStack Volume API', deprecated_name='osapi_compute_link_prefix'), + cfg.StrOpt('resource_query_filters_file', + default='/etc/cinder/resource_filters.json', + help="Json file indicating user visible filter " + "parameters for list queries.", + deprecated_name='query_volume_filters'), cfg.ListOpt('query_volume_filters', default=['name', 'status', 'metadata', 'availability_zone', 'bootable', 'group_id'], + deprecated_for_removal=True, help="Volume filter options which " "non-admin user could use to " "query volumes. Default values " @@ -55,6 +62,8 @@ CONF = cfg.CONF CONF.register_opts(api_common_opts) LOG = logging.getLogger(__name__) +_FILTERS_COLLECTION = None +FILTERING_VERSION = '3.31' METADATA_TYPES = enum.Enum('METADATA_TYPES', 'user image') @@ -399,3 +408,70 @@ def get_cluster_host(req, params, cluster_version=None): if bool(cluster_name) == bool(host): raise exception.InvalidInput(reason=msg) return cluster_name, host + + +def _initialize_filters(): + global _FILTERS_COLLECTION + if not _FILTERS_COLLECTION: + with open(CONF.resource_query_filters_file, 'r') as filters_file: + _FILTERS_COLLECTION = json.load(filters_file) + + +def get_enabled_resource_filters(resource=None): + """Get list of configured/allowed filters for the specified resource. + + This method checks resource_query_filters_file and returns dictionary + which contains the specified resource and its allowed filters: + + .. code-block:: json + + { + "resource": ['filter1', 'filter2', 'filter3'] + } + + if resource is not specified, all of the configuration will be returned, + and if the resource is not found, empty dict will be returned. + """ + try: + _initialize_filters() + if not resource: + return _FILTERS_COLLECTION + else: + return {resource: _FILTERS_COLLECTION[resource]} + except Exception: + LOG.debug("Failed to collect resource %s's filters.", resource) + return {} + + +def reject_invalid_filters(context, filters, resource): + if context.is_admin: + # Allow all options + return + # Check the configured filters against those passed in resource + configured_filters = get_enabled_resource_filters(resource) + if configured_filters: + configured_filters = configured_filters[resource] + else: + configured_filters = [] + invalid_filters = [] + for key in filters.copy().keys(): + if key not in configured_filters: + invalid_filters.append(key) + if invalid_filters: + raise webob.exc.HTTPBadRequest( + explanation=_('Invalid filters %s are found in query ' + 'options.') % ','.join(invalid_filters)) + + +def process_general_filtering(resource): + def wrapper(process_non_general_filtering): + def _decorator(*args, **kwargs): + req_version = kwargs.get('req_version') + filters = kwargs.get('filters') + context = kwargs.get('context') + if req_version.matches(FILTERING_VERSION): + reject_invalid_filters(context, filters, resource) + else: + process_non_general_filtering(*args, **kwargs) + return _decorator + return wrapper diff --git a/cinder/api/contrib/backups.py b/cinder/api/contrib/backups.py index 93b50aef290..c5b0075b1bf 100644 --- a/cinder/api/contrib/backups.py +++ b/cinder/api/contrib/backups.py @@ -83,16 +83,23 @@ class BackupsController(wsgi.Controller): """Return volume search options allowed by non-admin.""" return ('name', 'status', 'volume_id') + @common.process_general_filtering('backup') + def _process_backup_filtering(self, context=None, filters=None, + req_version=None): + utils.remove_invalid_filter_options(context, + filters, + self._get_backup_filter_options()) + def _get_backups(self, req, is_detail): """Returns a list of backups, transformed through view builder.""" context = req.environ['cinder.context'] filters = req.params.copy() + req_version = req.api_version_request marker, limit, offset = common.get_pagination_params(filters) sort_keys, sort_dirs = common.get_sort_params(filters) - utils.remove_invalid_filter_options(context, - filters, - self._get_backup_filter_options()) + self._process_backup_filtering(context=context, filters=filters, + req_version=req_version) if 'name' in filters: filters['display_name'] = filters.pop('name') diff --git a/cinder/api/contrib/scheduler_stats.py b/cinder/api/contrib/scheduler_stats.py index c3613734f26..c864538477b 100644 --- a/cinder/api/contrib/scheduler_stats.py +++ b/cinder/api/contrib/scheduler_stats.py @@ -14,6 +14,7 @@ """The Scheduler Stats extension""" +from cinder.api import common from cinder.api import extensions from cinder.api.openstack import wsgi from cinder.api.views import scheduler_stats as scheduler_stats_view @@ -37,6 +38,12 @@ class SchedulerStatsController(wsgi.Controller): self.scheduler_api = rpcapi.SchedulerAPI() super(SchedulerStatsController, self).__init__() + @common.process_general_filtering('pool') + def _process_pool_filtering(self, context=None, filters=None, + req_version=None): + if not req_version.matches(GET_POOL_NAME_FILTER_MICRO_VERSION): + filters.clear() + def get_pools(self, req): """List all active pools in scheduler.""" context = req.environ['cinder.context'] @@ -45,13 +52,14 @@ class SchedulerStatsController(wsgi.Controller): detail = utils.get_bool_param('detail', req.params) req_version = req.api_version_request + filters = req.params.copy() + filters.pop('detail', None) - if req_version.matches(GET_POOL_NAME_FILTER_MICRO_VERSION): - filters = req.params.copy() - filters.pop('detail', None) - pools = self.scheduler_api.get_pools(context, filters=filters) - else: - pools = self.scheduler_api.get_pools(context, filters=None) + self._process_pool_filtering(context=context, + filters=filters, + req_version=req_version) + + pools = self.scheduler_api.get_pools(context, filters=filters) return self._view_builder.pools(req, pools, detail) diff --git a/cinder/api/openstack/api_version_request.py b/cinder/api/openstack/api_version_request.py index a34d58b77ce..ee33efd8222 100644 --- a/cinder/api/openstack/api_version_request.py +++ b/cinder/api/openstack/api_version_request.py @@ -81,6 +81,7 @@ REST_API_VERSION_HISTORY = """ * 3.28 - Add filters support to get_pools * 3.29 - Add filter, sorter and pagination support in group snapshot. * 3.30 - Support sort snapshots with "name". + * 3.31 - Add support for configure resource query filters. """ @@ -89,7 +90,7 @@ REST_API_VERSION_HISTORY = """ # minimum version of the API supported. # Explicitly using /v1 or /v2 enpoints will still work _MIN_API_VERSION = "3.0" -_MAX_API_VERSION = "3.30" +_MAX_API_VERSION = "3.31" _LEGACY_API_VERSION1 = "1.0" _LEGACY_API_VERSION2 = "2.0" diff --git a/cinder/api/openstack/rest_api_version_history.rst b/cinder/api/openstack/rest_api_version_history.rst index 532f5cd9069..2d2dc1a0a11 100644 --- a/cinder/api/openstack/rest_api_version_history.rst +++ b/cinder/api/openstack/rest_api_version_history.rst @@ -296,3 +296,7 @@ user documentation. 3.30 ---- Support sort snapshots with "name". + +3.31 +---- + Add support for configure resource query filters. diff --git a/cinder/api/v3/attachments.py b/cinder/api/v3/attachments.py index 85af1e0e17e..7754eee76da 100644 --- a/cinder/api/v3/attachments.py +++ b/cinder/api/v3/attachments.py @@ -61,17 +61,25 @@ class AttachmentsController(wsgi.Controller): attachments = self._items(req) return attachment_views.ViewBuilder.list(attachments, detail=True) + @common.process_general_filtering('attachment') + def _process_attachment_filtering(self, context=None, filters=None, + req_version=None): + utils.remove_invalid_filter_options(context, filters, + self.allowed_filters) + def _items(self, req): """Return a list of attachments, transformed through view builder.""" context = req.environ['cinder.context'] + req_version = req.api_version_request # Pop out non search_opts and create local variables search_opts = req.GET.copy() sort_keys, sort_dirs = common.get_sort_params(search_opts) marker, limit, offset = common.get_pagination_params(search_opts) - utils.remove_invalid_filter_options(context, search_opts, - self.allowed_filters) + self._process_attachment_filtering(context=context, + filters=search_opts, + req_version=req_version) if search_opts.get('instance_id', None): search_opts['instance_uuid'] = search_opts.pop('instance_id', None) if context.is_admin and 'all_tenants' in search_opts: diff --git a/cinder/api/v3/group_snapshots.py b/cinder/api/v3/group_snapshots.py index 468a08497b8..25d114708a1 100644 --- a/cinder/api/v3/group_snapshots.py +++ b/cinder/api/v3/group_snapshots.py @@ -113,6 +113,10 @@ class GroupSnapshotsController(wsgi.Controller): filters = req.params.copy() marker, limit, offset = common.get_pagination_params(filters) sort_keys, sort_dirs = common.get_sort_params(filters) + + if req.api_version_request.matches(common.FILTERING_VERSION): + common.reject_invalid_filters(context, filters, 'group_snapshot') + group_snapshots = self.group_snapshot_api.get_all_group_snapshots( context, filters=filters, marker=marker, limit=limit, offset=offset, sort_keys=sort_keys, sort_dirs=sort_dirs) diff --git a/cinder/api/v3/groups.py b/cinder/api/v3/groups.py index 36ac11fc204..db8b9fa316b 100644 --- a/cinder/api/v3/groups.py +++ b/cinder/api/v3/groups.py @@ -167,6 +167,8 @@ class GroupsController(wsgi.Controller): sort_keys, sort_dirs = common.get_sort_params(filters) filters.pop('list_volume', None) + if req.api_version_request.matches(common.FILTERING_VERSION): + common.reject_invalid_filters(context, filters, 'group') groups = self.group_api.get_all( context, filters=filters, marker=marker, limit=limit, diff --git a/cinder/api/v3/messages.py b/cinder/api/v3/messages.py index 72e8e0eeb77..01e4518a46f 100644 --- a/cinder/api/v3/messages.py +++ b/cinder/api/v3/messages.py @@ -93,6 +93,9 @@ class MessagesController(wsgi.Controller): marker, limit, offset = common.get_pagination_params(filters) sort_keys, sort_dirs = common.get_sort_params(filters) + if req.api_version_request.matches(common.FILTERING_VERSION): + common.reject_invalid_filters(context, filters, 'message') + messages = self.message_api.get_all(context, filters=filters, marker=marker, limit=limit, offset=offset, diff --git a/cinder/api/v3/snapshots.py b/cinder/api/v3/snapshots.py index 5b5b9984474..dc079868b53 100644 --- a/cinder/api/v3/snapshots.py +++ b/cinder/api/v3/snapshots.py @@ -51,35 +51,37 @@ class SnapshotsController(snapshots_v2.SnapshotsController): LOG.debug('Could not evaluate value %s, assuming string', search_opts['metadata']) - def _process_filters(self, req, context, search_opts): + @common.process_general_filtering('snapshot') + def _process_snapshot_filtering(self, context=None, filters=None, + req_version=None): """Formats allowed filters""" - req_version = req.api_version_request # if the max version is less than or same as 3.21 # metadata based filtering is not supported if req_version.matches(None, "3.21"): - search_opts.pop('metadata', None) + filters.pop('metadata', None) # Filter out invalid options allowed_search_options = self._get_snapshot_filter_options() - utils.remove_invalid_filter_options(context, search_opts, + utils.remove_invalid_filter_options(context, filters, allowed_search_options) - # process snapshot filters to appropriate formats if required - self._format_snapshot_filter_options(search_opts) - def _items(self, req, is_detail=True): """Returns a list of snapshots, transformed through view builder.""" context = req.environ['cinder.context'] - + req_version = req.api_version_request # Pop out non search_opts and create local variables search_opts = req.GET.copy() sort_keys, sort_dirs = common.get_sort_params(search_opts) marker, limit, offset = common.get_pagination_params(search_opts) # process filters - self._process_filters(req, context, search_opts) + self._process_snapshot_filtering(context=context, + filters=search_opts, + req_version=req_version) + # process snapshot filters to appropriate formats if required + self._format_snapshot_filter_options(search_opts) req_version = req.api_version_request if req_version.matches("3.30", None) and 'name' in sort_keys: diff --git a/cinder/api/v3/volumes.py b/cinder/api/v3/volumes.py index fed0346b030..0974d24961b 100644 --- a/cinder/api/v3/volumes.py +++ b/cinder/api/v3/volumes.py @@ -83,6 +83,19 @@ class VolumeController(volumes_v2.VolumeController): return webob.Response(status_int=202) + @common.process_general_filtering('volume') + def _process_volume_filtering(self, context=None, filters=None, + req_version=None): + if req_version.matches(None, "3.3"): + filters.pop('glance_metadata', None) + + if req_version.matches(None, "3.9"): + filters.pop('group_id', None) + + utils.remove_invalid_filter_options( + context, filters, + self._get_volume_filter_options()) + def _get_volumes(self, req, is_detail): """Returns a list of volumes, transformed through view builder.""" @@ -94,14 +107,9 @@ class VolumeController(volumes_v2.VolumeController): sort_keys, sort_dirs = common.get_sort_params(params) filters = params - if req_version.matches(None, "3.3"): - filters.pop('glance_metadata', None) + self._process_volume_filtering(context=context, filters=filters, + req_version=req_version) - if req_version.matches(None, "3.9"): - filters.pop('group_id', None) - - utils.remove_invalid_filter_options(context, filters, - self._get_volume_filter_options()) # NOTE(thingee): v2 API allows name instead of display_name if 'name' in sort_keys: sort_keys[sort_keys.index('name')] = 'display_name' diff --git a/cinder/tests/unit/api/test_common.py b/cinder/tests/unit/api/test_common.py index 01561af2014..cbc5dc60b23 100644 --- a/cinder/tests/unit/api/test_common.py +++ b/cinder/tests/unit/api/test_common.py @@ -358,6 +358,61 @@ class TestCollectionLinks(test.TestCase): should_link_exist) +@ddt.ddt +class GeneralFiltersTest(test.TestCase): + + @ddt.data({'filters': {'volume': ['key1', 'key2']}, + 'resource': 'volume', + 'expected': {'volume': ['key1', 'key2']}}, + {'filters': {'volume': ['key1', 'key2']}, + 'resource': 'snapshot', + 'expected': {}}, + {'filters': {'volume': ['key1', 'key2']}, + 'resource': None, + 'expected': {'volume': ['key1', 'key2']}}) + @ddt.unpack + def test_get_enabled_resource_filters(self, filters, resource, expected): + common._FILTERS_COLLECTION = filters + result = common.get_enabled_resource_filters(resource) + self.assertEqual(expected, result) + + @ddt.data({'filters': {'key1': 'value1'}, + 'is_admin': False, + 'result': {'fake_resource': ['key1']}, + 'expected': {'key1': 'value1'}}, + {'filters': {'key1': 'value1', 'key2': 'value2'}, + 'is_admin': False, + 'result': {'fake_resource': ['key1']}, + 'expected': None}, + {'filters': {'key1': 'value1', + 'all_tenants': 'value2', + 'key3': 'value3'}, + 'is_admin': True, + 'result': {'fake_resource': []}, + 'expected': {'key1': 'value1', + 'all_tenants': 'value2', + 'key3': 'value3'}}) + @ddt.unpack + @mock.patch('cinder.api.common.get_enabled_resource_filters') + def test_reject_invalid_filters(self, mock_get, filters, + is_admin, result, expected): + class FakeContext(object): + def __init__(self, admin): + self.is_admin = admin + + fake_context = FakeContext(is_admin) + mock_get.return_value = result + if expected: + common.reject_invalid_filters(fake_context, + filters, 'fake_resource') + self.assertEqual(expected, filters) + else: + self.assertRaises( + webob.exc.HTTPBadRequest, + common.reject_invalid_filters, fake_context, + filters, 'fake_resource') + + @ddt.ddt class LinkPrefixTest(test.TestCase): diff --git a/cinder/tests/unit/api/v3/test_attachments.py b/cinder/tests/unit/api/v3/test_attachments.py index 528eac5397f..7a573745154 100644 --- a/cinder/tests/unit/api/v3/test_attachments.py +++ b/cinder/tests/unit/api/v3/test_attachments.py @@ -139,6 +139,19 @@ class AttachmentsAPITestCase(test.TestCase): self.controller.delete, req, self.attachment1.id) + @ddt.data('3.30', '3.31') + @mock.patch('cinder.api.common.reject_invalid_filters') + def test_attachment_list_with_general_filter(self, version, mock_update): + url = '/v3/%s/attachments' % fake.PROJECT_ID + req = fakes.HTTPRequest.blank(url, + version=version, + use_admin_context=False) + self.controller.index(req) + + if version != '3.30': + mock_update.assert_called_once_with(req.environ['cinder.context'], + mock.ANY, 'attachment') + @ddt.data('reserved', 'attached') @mock.patch.object(volume_rpcapi.VolumeAPI, 'attachment_delete') def test_delete_attachment(self, status, mock_delete): diff --git a/cinder/tests/unit/api/v3/test_backups.py b/cinder/tests/unit/api/v3/test_backups.py index 37feebdfed8..fa0fbd07db2 100644 --- a/cinder/tests/unit/api/v3/test_backups.py +++ b/cinder/tests/unit/api/v3/test_backups.py @@ -15,6 +15,8 @@ """The backups V3 api.""" +import ddt +import mock import webob from cinder.api.openstack import api_version_request as api_version @@ -29,6 +31,7 @@ from cinder.tests.unit import fake_constants as fake from cinder.tests.unit import utils as test_utils +@ddt.ddt class BackupsControllerAPITestCase(test.TestCase): """Test cases for backups API.""" @@ -82,6 +85,19 @@ class BackupsControllerAPITestCase(test.TestCase): self.controller.update, req, fake.BACKUP_ID, body) + @ddt.data('3.30', '3.31') + @mock.patch('cinder.api.common.reject_invalid_filters') + def test_backup_list_with_general_filter(self, version, mock_update): + url = '/v3/%s/backups' % fake.PROJECT_ID + req = fakes.HTTPRequest.blank(url, + version=version, + use_admin_context=False) + self.controller.index(req) + + if version != '3.30': + mock_update.assert_called_once_with(req.environ['cinder.context'], + mock.ANY, 'backup') + def test_backup_update(self): backup = test_utils.create_backup( self.ctxt, diff --git a/cinder/tests/unit/api/v3/test_group_snapshots.py b/cinder/tests/unit/api/v3/test_group_snapshots.py index 3948298c618..ec73d691d83 100644 --- a/cinder/tests/unit/api/v3/test_group_snapshots.py +++ b/cinder/tests/unit/api/v3/test_group_snapshots.py @@ -183,6 +183,20 @@ class GroupSnapshotsAPITestCase(test.TestCase): res_dict['group_snapshots'][0].keys()) group_snapshot.destroy() + @ddt.data('3.30', '3.31') + @mock.patch('cinder.api.common.reject_invalid_filters') + def test_group_snapshot_list_with_general_filter(self, + version, mock_update): + url = '/v3/%s/group_snapshots' % fake.PROJECT_ID + req = fakes.HTTPRequest.blank(url, + version=version, + use_admin_context=False) + self.controller.index(req) + + if version != '3.30': + mock_update.assert_called_once_with(req.environ['cinder.context'], + mock.ANY, 'group_snapshot') + @ddt.data(False, True) def test_list_group_snapshot_with_filter(self, is_detail): url = ('/v3/%s/group_snapshots?' diff --git a/cinder/tests/unit/api/v3/test_groups.py b/cinder/tests/unit/api/v3/test_groups.py index b4ad2de18a4..65606198b09 100644 --- a/cinder/tests/unit/api/v3/test_groups.py +++ b/cinder/tests/unit/api/v3/test_groups.py @@ -239,6 +239,19 @@ class GroupsAPITestCase(test.TestCase): self.assertRaises(exception.GroupNotFound, self.controller.show, req, fake.WILL_NOT_BE_FOUND_ID) + @ddt.data('3.30', '3.31') + @mock.patch('cinder.api.common.reject_invalid_filters') + def test_group_list_with_general_filter(self, version, mock_update): + url = '/v3/%s/groups' % fake.PROJECT_ID + req = fakes.HTTPRequest.blank(url, + version=version, + use_admin_context=False) + self.controller.index(req) + + if version != '3.30': + mock_update.assert_called_once_with(req.environ['cinder.context'], + mock.ANY, 'group') + def test_list_groups_json(self): self.group2.group_type_id = fake.GROUP_TYPE2_ID # TODO(geguileo): One `volume_type_ids` gets sorted out make proper diff --git a/cinder/tests/unit/api/v3/test_messages.py b/cinder/tests/unit/api/v3/test_messages.py index 6cf17bb3e8b..b758f4c908a 100644 --- a/cinder/tests/unit/api/v3/test_messages.py +++ b/cinder/tests/unit/api/v3/test_messages.py @@ -10,6 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +import ddt +import mock from six.moves import http_client from cinder.api import extensions @@ -26,6 +28,7 @@ from cinder.tests.unit.api.v3 import fakes as v3_fakes NS = '{http://docs.openstack.org/api/openstack-block-storage/3.0/content}' +@ddt.ddt class MessageApiTest(test.TestCase): def setUp(self): super(MessageApiTest, self).setUp() @@ -120,6 +123,19 @@ class MessageApiTest(test.TestCase): self.assertRaises(exception.MessageNotFound, self.controller.delete, req, fakes.FAKE_UUID) + @ddt.data('3.30', '3.31') + @mock.patch('cinder.api.common.reject_invalid_filters') + def test_message_list_with_general_filter(self, version, mock_update): + url = '/v3/%s/messages' % fakes.FAKE_UUID + req = fakes.HTTPRequest.blank(url, + version=version, + use_admin_context=False) + self.controller.index(req) + + if version != '3.30': + mock_update.assert_called_once_with(req.environ['cinder.context'], + mock.ANY, 'message') + def test_index(self): self.mock_object(message_api.API, 'get_all', return_value=[v3_fakes.fake_message(fakes.FAKE_UUID)]) diff --git a/cinder/tests/unit/api/v3/test_snapshots.py b/cinder/tests/unit/api/v3/test_snapshots.py index 8a74df0fe36..7727433e372 100644 --- a/cinder/tests/unit/api/v3/test_snapshots.py +++ b/cinder/tests/unit/api/v3/test_snapshots.py @@ -195,6 +195,19 @@ class SnapshotApiTest(test.TestCase): self.assertDictEqual({"key1": "val1", "key11": "val11"}, res_dict[ 'snapshots'][0]['metadata']) + @ddt.data('3.30', '3.31') + @mock.patch('cinder.api.common.reject_invalid_filters') + def test_snapshot_list_with_general_filter(self, version, mock_update): + url = '/v3/%s/snapshots' % fake.PROJECT_ID + req = fakes.HTTPRequest.blank(url, + version=version, + use_admin_context=False) + self.controller.index(req) + + if version != '3.30': + mock_update.assert_called_once_with(req.environ['cinder.context'], + mock.ANY, 'snapshot') + def test_snapshot_list_with_metadata_unsupported_microversion(self): # Create snapshot with metadata key1: value1 metadata = {"key1": "val1"} diff --git a/cinder/tests/unit/api/v3/test_volumes.py b/cinder/tests/unit/api/v3/test_volumes.py index fdc1fc36238..4ec9be7387b 100644 --- a/cinder/tests/unit/api/v3/test_volumes.py +++ b/cinder/tests/unit/api/v3/test_volumes.py @@ -30,6 +30,7 @@ from cinder.tests.unit.api import fakes from cinder.tests.unit.api.v2 import fakes as v2_fakes from cinder.tests.unit.api.v2 import test_volumes as v2_test_volumes from cinder.tests.unit import fake_constants as fake +from cinder import utils from cinder.volume import api as volume_api from cinder.volume import api as vol_get @@ -73,7 +74,7 @@ class VolumeApiTest(test.TestCase): req.content_type = 'application/json' req.headers = {version_header_name: 'volume 3.2'} req.environ['cinder.context'].is_admin = True - req.api_version_request = api_version.max_api_version() + req.api_version_request = api_version.APIVersionRequest('3.29') self.override_config('query_volume_filters', 'bootable') self.controller.index(req) @@ -388,6 +389,17 @@ class VolumeApiTest(test.TestCase): self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, req, body) + @ddt.data('3.30', '3.31') + @mock.patch.object(volume_api.API, 'check_volume_filters', mock.Mock()) + @mock.patch.object(utils, 'add_visible_admin_metadata', mock.Mock()) + @mock.patch('cinder.api.common.reject_invalid_filters') + def test_list_volume_with_general_filter(self, version, mock_update): + req = fakes.HTTPRequest.blank('/v3/volumes', version=version) + self.controller.index(req) + if version != '3.30': + mock_update.assert_called_once_with(req.environ['cinder.context'], + mock.ANY, 'volume') + @ddt.data({'admin': True, 'version': '3.21'}, {'admin': False, 'version': '3.21'}, {'admin': True, 'version': '3.20'}, diff --git a/cinder/tests/unit/message/test_api.py b/cinder/tests/unit/message/test_api.py index ff5cf41fd69..11ada1d96d6 100644 --- a/cinder/tests/unit/message/test_api.py +++ b/cinder/tests/unit/message/test_api.py @@ -134,7 +134,7 @@ class MessageApiTest(test.TestCase): req.method = 'GET' req.content_type = 'application/json' req.headers = {version_header_name: 'volume 3.5'} - req.api_version_request = api_version.max_api_version() + req.api_version_request = api_version.APIVersionRequest('3.30') req.environ['cinder.context'].is_admin = True res = self.controller.index(req) @@ -145,7 +145,7 @@ class MessageApiTest(test.TestCase): req.method = 'GET' req.content_type = 'application/json' req.headers = {version_header_name: 'volume 3.5'} - req.api_version_request = api_version.max_api_version() + req.api_version_request = api_version.APIVersionRequest('3.30') req.environ['cinder.context'].is_admin = True res = self.controller.index(req) @@ -248,7 +248,7 @@ class MessageApiTest(test.TestCase): req.method = 'GET' req.content_type = 'application/json' req.headers = {version_header_name: 'volume 3.5'} - req.api_version_request = api_version.max_api_version() + req.api_version_request = api_version.APIVersionRequest('3.30') req.environ['cinder.context'].is_admin = True res = self.controller.index(req) diff --git a/doc/source/man/generalized_filters.rst b/doc/source/man/generalized_filters.rst new file mode 100644 index 00000000000..eea704d6c46 --- /dev/null +++ b/doc/source/man/generalized_filters.rst @@ -0,0 +1,65 @@ +Generalized filters +=================== + +Background +---------- + +Cinder introduced generalized resource filters since Pike, it has the +same purpose as ``query_volume_filters`` option, but it's more convenient +and can be applied to more cinder resources, administrator can control the +allowed filter keys for **non-admin** user by editing the filter +configuration file. Also since this feature, cinder will raise +``400 BadRequest`` if any invalid query filter is specified. + +How do I configure the filter keys? +----------------------------------- + +``resource_query_filters_file`` is introduced to cinder to represent the +filter config file path, and the config file accepts the valid filter keys +for **non-admin** user with json format: + +.. code-block:: json + + { + "volume": ["name", "status", "metadata"] + } + + +the key ``volume`` (singular) here stands for the resource you want to apply and the value +accepts an list which contains the allowed filters collection, once the configuration +file is changed and API service is restarted, cinder will only recognize this filter +keys, **NOTE**: the default configuration file will include all the filters we already +enabled. + +Which filter keys are supported? +-------------------------------- + +Not all the attributes are supported at present, so we add this table below to +indicate which filter keys are valid and can be used in the configuration: + ++----------------+-------------------------------------------------------------------------+ +| API | Valid filter keys | ++================+=========================================================================+ +| | id, group_id, name, status, bootable, migration_status, metadata, host, | +| list volume | image_metadata, availability_zone, user_id, volume_type_id, project_id, | +| | size, description, replication_status, multiattach | ++----------------+-------------------------------------------------------------------------+ +| | id, volume_id, user_id, project_id, status, volume_size, name, | +| list snapshot | description, volume_type_id, group_snapshot_id, metadata | ++----------------+-------------------------------------------------------------------------+ +| | id, name, status, container, availability_zone, description, | +| list backup | volume_id, is_incremental, size, host, parent_id | ++----------------+-------------------------------------------------------------------------+ +| | id, user_id, status, availability_zone, group_type, name, description, | +| list group | host | ++----------------+-------------------------------------------------------------------------+ +| list g-snapshot| id, name, description, group_id, group_type_id, status | ++----------------+-------------------------------------------------------------------------+ +| | id, volume_id, instance_id, attach_status, attach_mode, | +| list attachment| connection_info, mountpoint, attached_host | ++----------------+-------------------------------------------------------------------------+ +| | id, event_id, resource_uuid, resource_type, request_id, message_level, | +| list message | project_id | ++----------------+-------------------------------------------------------------------------+ +| get pools | name | ++----------------+-------------------------------------------------------------------------+ diff --git a/etc/cinder/resource_filters.json b/etc/cinder/resource_filters.json new file mode 100644 index 00000000000..68c428ad1f0 --- /dev/null +++ b/etc/cinder/resource_filters.json @@ -0,0 +1,12 @@ +{ + "volume": ["name", "status", "image_metadata", + "bootable", "migration_status"], + "backup": ["name", "status", "volume_id"], + "snapshot": ["name", "status", "volume_id"], + "group": [], + "group_snapshot": ["status", "group_id"], + "attachment": ["volume_id"], + "message": ["resource_uuid", "resource_type", "event_id", + "request_id", "message_level"], + "pool": ["name"] +} diff --git a/releasenotes/notes/generalized-resource-filter-hg598uyvuh119008.yaml b/releasenotes/notes/generalized-resource-filter-hg598uyvuh119008.yaml new file mode 100644 index 00000000000..b2874c81d76 --- /dev/null +++ b/releasenotes/notes/generalized-resource-filter-hg598uyvuh119008.yaml @@ -0,0 +1,6 @@ +--- +features: + - Added generalized resource filter support in + ``list volume``, ``list backup``, ``list snapshot``, + ``list group``, ``list group-snapshot``, ``list attachment``, + ``list message`` and ``list pools`` APIs.