Microversion of Bootable filter in cinder list
Currently cinder treats all non zero values passed to it as True for bootable filter in volume GET call. For ex, if bootable=invalid, it converts it into True being bootable as boolean value in db table 'volume'. Need to update filtering functionality in cinder to address 'True' and 'False' values separately and raise exception if anything than True/False/true/false/0/1 is passed in this filter. But as this will change the API behavior, microversioning is done for this fix. Closes-Bug: #1551535 Change-Id: Idd57014c1504eb4fc9ea0eabe894d2000ea9c364
This commit is contained in:
Sheel Rana
parent
eaeff0eeed
commit
0b6c68a8c1
|
|
@ -47,6 +47,8 @@ REST_API_VERSION_HISTORY = """
|
|||
* 3.0 - Includes all V2 APIs and extensions. V1 API is still supported.
|
||||
* 3.0 - Versions API updated to reflect beginning of microversions epoch.
|
||||
* 3.1 - Adds visibility and protected to _volume_upload_image parameters.
|
||||
* 3.2 - Bootable filters in volume GET call no longer treats all values
|
||||
passed to it as true.
|
||||
|
||||
"""
|
||||
|
||||
|
|
@ -55,7 +57,7 @@ REST_API_VERSION_HISTORY = """
|
|||
# the minimum version of the API supported.
|
||||
# Explicitly using /v1 or /v2 enpoints will still work
|
||||
_MIN_API_VERSION = "3.0"
|
||||
_MAX_API_VERSION = "3.1"
|
||||
_MAX_API_VERSION = "3.2"
|
||||
_LEGACY_API_VERSION1 = "1.0"
|
||||
_LEGACY_API_VERSION2 = "2.0"
|
||||
|
||||
|
|
|
|||
|
|
@ -33,3 +33,21 @@ user documentation.
|
|||
---
|
||||
Added the parameters ``protected`` and ``visibility`` to
|
||||
_volume_upload_image requests.
|
||||
|
||||
3.2
|
||||
---
|
||||
Change in return value of 'GET API request' for fetching cinder volume
|
||||
list on the basis of 'bootable' status of volume as filter.
|
||||
|
||||
Before V3.2, 'GET API request' to fetch volume list returns non-bootable
|
||||
volumes if bootable filter value is any of the false or False.
|
||||
For any other value provided to this filter, it always returns
|
||||
bootable volumes list.
|
||||
|
||||
But in V3.2, this behavior is updated.
|
||||
In V3.2, bootable volume list will be returned for any of the
|
||||
'T/True/1/true' bootable filter values only.
|
||||
Non-bootable volume list will be returned for any of 'F/False/0/false'
|
||||
bootable filter values.
|
||||
But for any other values passed for bootable filter, it will return
|
||||
"Invalid input received: bootable={filter value}' error.
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ from cinder.api.v2 import snapshot_metadata
|
|||
from cinder.api.v2 import snapshots
|
||||
from cinder.api.v2 import types
|
||||
from cinder.api.v2 import volume_metadata
|
||||
from cinder.api.v2 import volumes
|
||||
from cinder.api.v3 import volumes
|
||||
from cinder.api import versions
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,70 @@
|
|||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
"""The volumes V3 api."""
|
||||
|
||||
from cinder.api import common
|
||||
from cinder.api.openstack import wsgi
|
||||
from cinder.api.v2 import volumes as volumes_v2
|
||||
from cinder import utils
|
||||
|
||||
|
||||
class VolumeController(volumes_v2.VolumeController):
|
||||
"""The Volumes API controller for the OpenStack API V3."""
|
||||
|
||||
def _get_volumes(self, req, is_detail):
|
||||
"""Returns a list of volumes, transformed through view builder."""
|
||||
|
||||
context = req.environ['cinder.context']
|
||||
|
||||
params = req.params.copy()
|
||||
marker, limit, offset = common.get_pagination_params(params)
|
||||
sort_keys, sort_dirs = common.get_sort_params(params)
|
||||
filters = params
|
||||
|
||||
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'
|
||||
|
||||
if 'name' in filters:
|
||||
filters['display_name'] = filters['name']
|
||||
del filters['name']
|
||||
|
||||
strict = req.api_version_request.matches("3.2", None)
|
||||
self.volume_api.check_volume_filters(filters, strict)
|
||||
|
||||
volumes = self.volume_api.get_all(context, marker, limit,
|
||||
sort_keys=sort_keys,
|
||||
sort_dirs=sort_dirs,
|
||||
filters=filters,
|
||||
viewable_admin_meta=True,
|
||||
offset=offset)
|
||||
|
||||
for volume in volumes:
|
||||
utils.add_visible_admin_metadata(volume)
|
||||
|
||||
req.cache_db_volumes(volumes.objects)
|
||||
|
||||
if is_detail:
|
||||
volumes = self._view_builder.detail_list(req, volumes)
|
||||
else:
|
||||
volumes = self._view_builder.summary_list(req, volumes)
|
||||
return volumes
|
||||
|
||||
|
||||
def create_resource(ext_mgr):
|
||||
return wsgi.Resource(VolumeController(ext_mgr))
|
||||
|
|
@ -0,0 +1,72 @@
|
|||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
|
||||
import mock
|
||||
|
||||
from cinder.api import extensions
|
||||
from cinder.api.openstack import api_version_request as api_version
|
||||
from cinder.api.v3 import volumes
|
||||
from cinder import context
|
||||
from cinder import test
|
||||
from cinder.tests.unit.api import fakes
|
||||
from cinder.tests.unit import fake_constants as fake
|
||||
from cinder.tests.unit import fake_notifier
|
||||
from cinder.volume.api import API as vol_get
|
||||
|
||||
version_header_name = 'OpenStack-API-Version'
|
||||
|
||||
|
||||
class VolumeApiTest(test.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(VolumeApiTest, self).setUp()
|
||||
self.ext_mgr = extensions.ExtensionManager()
|
||||
self.ext_mgr.extensions = {}
|
||||
self.controller = volumes.VolumeController(self.ext_mgr)
|
||||
|
||||
self.flags(host='fake',
|
||||
notification_driver=[fake_notifier.__name__])
|
||||
self.ctxt = context.RequestContext(fake.user_id, fake.project_id, True)
|
||||
|
||||
def test_check_volume_filters_called(self):
|
||||
with mock.patch.object(vol_get,
|
||||
'check_volume_filters') as volume_get:
|
||||
req = fakes.HTTPRequest.blank('/v3/volumes?bootable=True')
|
||||
req.method = 'GET'
|
||||
req.content_type = 'application/json'
|
||||
req.headers = {version_header_name: 'volume 3.0'}
|
||||
req.environ['cinder.context'].is_admin = True
|
||||
|
||||
self.override_config('query_volume_filters', 'bootable')
|
||||
self.controller.index(req)
|
||||
filters = req.params.copy()
|
||||
|
||||
volume_get.assert_called_with(filters, False)
|
||||
|
||||
def test_check_volume_filters_strict_called(self):
|
||||
|
||||
with mock.patch.object(vol_get,
|
||||
'check_volume_filters') as volume_get:
|
||||
req = fakes.HTTPRequest.blank('/v3/volumes?bootable=True')
|
||||
req.method = 'GET'
|
||||
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()
|
||||
|
||||
self.override_config('query_volume_filters', 'bootable')
|
||||
self.controller.index(req)
|
||||
filters = req.params.copy()
|
||||
|
||||
volume_get.assert_called_with(filters, True)
|
||||
|
|
@ -343,6 +343,7 @@ class AvailabilityZoneTestCase(BaseVolumeTestCase):
|
|||
self.assertEqual(expected, azs)
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class VolumeTestCase(BaseVolumeTestCase):
|
||||
|
||||
def setUp(self):
|
||||
|
|
@ -4167,27 +4168,17 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
fake_new_volume.id)
|
||||
self.assertIsNone(volume.migration_status)
|
||||
|
||||
def test_check_volume_filters_true(self):
|
||||
"""Test bootable as filter for true"""
|
||||
@ddt.data(False, True)
|
||||
def test_check_volume_filters(self, filter_value):
|
||||
"""Test bootable as filter for True or False"""
|
||||
volume_api = cinder.volume.api.API()
|
||||
filters = {'bootable': 'TRUE'}
|
||||
filters = {'bootable': filter_value}
|
||||
|
||||
# To convert filter value to True or False
|
||||
volume_api.check_volume_filters(filters)
|
||||
|
||||
# Confirming converted filter value against True
|
||||
self.assertTrue(filters['bootable'])
|
||||
|
||||
def test_check_volume_filters_false(self):
|
||||
"""Test bootable as filter for false"""
|
||||
volume_api = cinder.volume.api.API()
|
||||
filters = {'bootable': 'false'}
|
||||
|
||||
# To convert filter value to True or False
|
||||
volume_api.check_volume_filters(filters)
|
||||
|
||||
# Confirming converted filter value against False
|
||||
self.assertEqual(False, filters['bootable'])
|
||||
# Confirming converted filter value against True or False
|
||||
self.assertEqual(filter_value, filters['bootable'])
|
||||
|
||||
def test_check_volume_filters_invalid(self):
|
||||
"""Test bootable as filter"""
|
||||
|
|
@ -4200,6 +4191,43 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
# Confirming converted filter value against invalid value
|
||||
self.assertTrue(filters['bootable'])
|
||||
|
||||
@ddt.data('False', 'false', 'f', '0')
|
||||
def test_check_volume_filters_strict_false(self, filter_value):
|
||||
"""Test bootable as filter for False, false, f and 0 values"""
|
||||
volume_api = cinder.volume.api.API()
|
||||
filters = {'bootable': filter_value}
|
||||
|
||||
strict = True
|
||||
# To convert filter value to True or False
|
||||
volume_api.check_volume_filters(filters, strict)
|
||||
|
||||
# Confirming converted filter value against False
|
||||
self.assertFalse(filters['bootable'])
|
||||
|
||||
@ddt.data('True', 'true', 't', '1')
|
||||
def test_check_volume_filters_strict_true(self, filter_value):
|
||||
"""Test bootable as filter for True, true, t, 1 values"""
|
||||
volume_api = cinder.volume.api.API()
|
||||
filters = {'bootable': filter_value}
|
||||
|
||||
strict = True
|
||||
# To convert filter value to True or False
|
||||
volume_api.check_volume_filters(filters, strict)
|
||||
|
||||
# Confirming converted filter value against True
|
||||
self.assertTrue(filters['bootable'])
|
||||
|
||||
def test_check_volume_filters_strict_invalid(self):
|
||||
"""Test bootable as filter for invalid value."""
|
||||
volume_api = cinder.volume.api.API()
|
||||
filters = {'bootable': 'invalid'}
|
||||
|
||||
strict = True
|
||||
# Confirming exception for invalid value in filter
|
||||
self.assertRaises(exception.InvalidInput,
|
||||
volume_api.check_volume_filters,
|
||||
filters, strict)
|
||||
|
||||
def test_update_volume_readonly_flag(self):
|
||||
"""Test volume readonly flag can be updated at API level."""
|
||||
# create a volume and assign to host
|
||||
|
|
|
|||
|
|
@ -1665,24 +1665,19 @@ class API(base.Base):
|
|||
if not self.volume_rpcapi.thaw_host(ctxt, host):
|
||||
return "Backend reported error during thaw_host operation."
|
||||
|
||||
def check_volume_filters(self, filters):
|
||||
"""Sets the user filter value to accepted format."""
|
||||
def check_volume_filters(self, filters, strict=False):
|
||||
"""Sets the user filter value to accepted format"""
|
||||
booleans = self.db.get_booleans_for_table('volume')
|
||||
|
||||
# To translate any true/false equivalent to True/False
|
||||
# which is only acceptable format in database queries.
|
||||
accepted_true = ['True', 'true', 'TRUE']
|
||||
accepted_false = ['False', 'false', 'FALSE']
|
||||
for k, v in filters.items():
|
||||
|
||||
for key, val in filters.items():
|
||||
try:
|
||||
if k in booleans:
|
||||
if v in accepted_false:
|
||||
filters[k] = False
|
||||
elif v in accepted_true:
|
||||
filters[k] = True
|
||||
else:
|
||||
filters[k] = bool(v)
|
||||
elif k == 'display_name':
|
||||
if key in booleans:
|
||||
filters[key] = self._check_boolean_filter_value(
|
||||
key, val, strict)
|
||||
elif key == 'display_name':
|
||||
# Use the raw value of display name as is for the filter
|
||||
# without passing it through ast.literal_eval(). If the
|
||||
# display name is a properly quoted string (e.g. '"foo"')
|
||||
|
|
@ -1690,9 +1685,46 @@ class API(base.Base):
|
|||
# the filter becomes different from the user input.
|
||||
continue
|
||||
else:
|
||||
filters[k] = ast.literal_eval(v)
|
||||
filters[key] = ast.literal_eval(val)
|
||||
except (ValueError, SyntaxError):
|
||||
LOG.debug('Could not evaluate value %s, assuming string', v)
|
||||
LOG.debug('Could not evaluate value %s, assuming string', val)
|
||||
|
||||
def _check_boolean_filter_value(self, key, val, strict=False):
|
||||
"""Boolean filter values in Volume GET.
|
||||
|
||||
Before V3.2, all values other than 'False', 'false', 'FALSE' were
|
||||
trated as True for specific boolean filter parameters in Volume
|
||||
GET request.
|
||||
|
||||
But V3.2 onwards, only true/True/0/1/False/false parameters are
|
||||
supported.
|
||||
All other input values to specific boolean filter parameter will
|
||||
lead to raising exception.
|
||||
|
||||
This changes API behavior. So, micro version introduced for V3.2
|
||||
onwards.
|
||||
"""
|
||||
if strict:
|
||||
# for updated behavior, from V3.2 onwards.
|
||||
# To translate any true/false/t/f/0/1 to True/False
|
||||
# which is only acceptable format in database queries.
|
||||
try:
|
||||
return strutils.bool_from_string(val, strict=True)
|
||||
except ValueError:
|
||||
msg = _('\'%(key)s = %(value)s\'') % {'key': key,
|
||||
'value': val}
|
||||
raise exception.InvalidInput(reason=msg)
|
||||
else:
|
||||
# For existing behavior(before version 3.2)
|
||||
accepted_true = ['True', 'true', 'TRUE']
|
||||
accepted_false = ['False', 'false', 'FALSE']
|
||||
|
||||
if val in accepted_false:
|
||||
return False
|
||||
elif val in accepted_true:
|
||||
return True
|
||||
else:
|
||||
return bool(val)
|
||||
|
||||
|
||||
class HostAPI(base.Base):
|
||||
|
|
|
|||
Loading…
Reference in New Issue