Add query parameters white list for server list/detail
This patch enables a white list for query parameter. The parameters, which are out of white list, will be ignored. The sort_key still accepts open value. The later patch will add white list to it. Co-Authored-By: Zhenyu Zheng <zhengzhenyu@huawei.com> partial implement of bp add-whitelist-for-server-list-filter-sort-parameters Change-Id: I7141eef6a1c85ec6d6e8ee170911572535652978
This commit is contained in:
@@ -15,6 +15,7 @@
|
||||
import copy
|
||||
|
||||
from nova.api.validation import parameter_types
|
||||
from nova.api.validation.parameter_types import multi_params
|
||||
|
||||
|
||||
base_create = {
|
||||
@@ -232,3 +233,101 @@ trigger_crash_dump = {
|
||||
'required': ['trigger_crash_dump'],
|
||||
'additionalProperties': False
|
||||
}
|
||||
|
||||
# NOTE: We don't check actual values of queries on params
|
||||
# which are defined as the following common_param.
|
||||
common_param = multi_params({'type': 'string'})
|
||||
common_regex_param = multi_params({'type': 'string', 'format': 'regex'})
|
||||
|
||||
JOINED_TABLE_QUERY_PARAMS_SERVERS = {
|
||||
'block_device_mapping': common_param,
|
||||
'services': common_param,
|
||||
'metadata': common_param,
|
||||
'system_metadata': common_param,
|
||||
'info_cache': common_param,
|
||||
'security_groups': common_param,
|
||||
'pci_devices': common_param
|
||||
}
|
||||
|
||||
query_params_v21 = {
|
||||
'type': 'object',
|
||||
'properties': {
|
||||
'user_id': common_param,
|
||||
'project_id': common_param,
|
||||
# The alias of project_id. It should be removed in the
|
||||
# future with microversion bump.
|
||||
'tenant_id': common_param,
|
||||
'launch_index': common_param,
|
||||
# The alias of image. It should be removed in the
|
||||
# future with microversion bump.
|
||||
'image_ref': common_param,
|
||||
'image': common_param,
|
||||
'kernel_id': common_param,
|
||||
'ramdisk_id': common_param,
|
||||
'hostname': common_param,
|
||||
'key_name': common_param,
|
||||
'power_state': common_param,
|
||||
'vm_state': common_param,
|
||||
'task_state': common_param,
|
||||
'host': common_param,
|
||||
'node': common_param,
|
||||
'flavor': common_param,
|
||||
'reservation_id': common_param,
|
||||
'launched_at': common_param,
|
||||
'terminate_at': common_param,
|
||||
'availability_zone': common_param,
|
||||
# NOTE(alex_xu): This is pattern matching, it didn't get any benefit
|
||||
# from DB index.
|
||||
'name': common_regex_param,
|
||||
# The alias of name. It should be removed in the future
|
||||
# with microversion bump.
|
||||
'display_name': common_regex_param,
|
||||
'description': common_regex_param,
|
||||
# The alias of description. It should be removed in the
|
||||
# future with microversion bump.
|
||||
'display_description': common_regex_param,
|
||||
'locked_by': common_param,
|
||||
'uuid': common_param,
|
||||
'root_device_name': common_param,
|
||||
'config_drive': common_param,
|
||||
'accessIPv4': common_param,
|
||||
'accessIPv6': common_param,
|
||||
'auto_disk_config': common_param,
|
||||
'progress': common_param,
|
||||
'sort_key': common_param,
|
||||
'sort_dir': common_param,
|
||||
'all_tenants': common_param,
|
||||
'deleted': common_param,
|
||||
'status': common_param,
|
||||
'changes-since': multi_params({'type': 'string',
|
||||
'format': 'date-time'}),
|
||||
# NOTE(alex_xu): The ip and ip6 are implemented in the python.
|
||||
'ip': common_regex_param,
|
||||
'ip6': common_regex_param,
|
||||
'created_at': common_param,
|
||||
},
|
||||
# For backward-compatible additionalProperties is set to be True here.
|
||||
# And we will either strip the extra params out or raise HTTP 400
|
||||
# according to the params' value in the later process.
|
||||
'additionalProperties': True,
|
||||
# Prevent internal-attributes that are started with underscore from
|
||||
# being striped out in schema validation, and raise HTTP 400 in API.
|
||||
'patternProperties': {"^_": common_param}
|
||||
}
|
||||
|
||||
# Update the joined-table fields to the list so it will not be
|
||||
# stripped in later process, thus can be handled later in api
|
||||
# to raise HTTP 400.
|
||||
query_params_v21['properties'].update(
|
||||
JOINED_TABLE_QUERY_PARAMS_SERVERS)
|
||||
|
||||
query_params_v21['properties'].update(
|
||||
parameter_types.pagination_parameters)
|
||||
|
||||
query_params_v226 = copy.deepcopy(query_params_v21)
|
||||
query_params_v226['properties'].update({
|
||||
'tags': common_param,
|
||||
'tags-any': common_param,
|
||||
'not-tags': common_param,
|
||||
'not-tags-any': common_param,
|
||||
})
|
||||
|
||||
@@ -14,8 +14,6 @@
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import re
|
||||
|
||||
from oslo_log import log as logging
|
||||
import oslo_messaging as messaging
|
||||
from oslo_utils import strutils
|
||||
@@ -188,6 +186,8 @@ class ServersController(wsgi.Controller):
|
||||
LOG.debug("Did not find any server create schemas")
|
||||
|
||||
@extensions.expected_errors((400, 403))
|
||||
@validation.query_schema(schema_servers.query_params_v226, '2.26')
|
||||
@validation.query_schema(schema_servers.query_params_v21, '2.1', '2.25')
|
||||
def index(self, req):
|
||||
"""Returns a list of server names and ids for a given user."""
|
||||
context = req.environ['nova.context']
|
||||
@@ -199,6 +199,8 @@ class ServersController(wsgi.Controller):
|
||||
return servers
|
||||
|
||||
@extensions.expected_errors((400, 403))
|
||||
@validation.query_schema(schema_servers.query_params_v226, '2.26')
|
||||
@validation.query_schema(schema_servers.query_params_v21, '2.1', '2.25')
|
||||
def detail(self, req):
|
||||
"""Returns a list of server details for a given user."""
|
||||
context = req.environ['nova.context']
|
||||
@@ -219,6 +221,13 @@ class ServersController(wsgi.Controller):
|
||||
remove_invalid_options(context, search_opts,
|
||||
self._get_server_search_options(req))
|
||||
|
||||
for search_opt in search_opts:
|
||||
if (search_opt in
|
||||
schema_servers.JOINED_TABLE_QUERY_PARAMS_SERVERS.keys() or
|
||||
search_opt.startswith('_')):
|
||||
msg = _("Invalid filter field: %s.") % search_opt
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
# Verify search by 'status' contains a valid status.
|
||||
# Convert it to filter by vm_state or task_state for compute_api.
|
||||
# For non-admin user, vm_state and task_state are filtered through
|
||||
@@ -242,12 +251,8 @@ class ServersController(wsgi.Controller):
|
||||
search_opts['task_state'] = task_state
|
||||
|
||||
if 'changes-since' in search_opts:
|
||||
try:
|
||||
parsed = timeutils.parse_isotime(search_opts['changes-since'])
|
||||
except ValueError:
|
||||
msg = _('Invalid changes-since value')
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
search_opts['changes-since'] = parsed
|
||||
search_opts['changes-since'] = timeutils.parse_isotime(
|
||||
search_opts['changes-since'])
|
||||
|
||||
# By default, compute's get_all() will return deleted instances.
|
||||
# If an admin hasn't specified a 'deleted' search option, we need
|
||||
@@ -273,14 +278,6 @@ class ServersController(wsgi.Controller):
|
||||
msg = _("Only administrators may list deleted instances")
|
||||
raise exc.HTTPForbidden(explanation=msg)
|
||||
|
||||
# Verify the value of the 'name' option is a correct regex.
|
||||
if 'name' in search_opts:
|
||||
try:
|
||||
re.compile(search_opts['name'])
|
||||
except re.error:
|
||||
msg = _("The regex for server name is incorrect")
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
if api_version_request.is_supported(req, min_version='2.26'):
|
||||
for tag_filter in TAG_SEARCH_FILTERS:
|
||||
if tag_filter in search_opts:
|
||||
|
||||
@@ -17,6 +17,7 @@ Request Body validating middleware.
|
||||
"""
|
||||
|
||||
import functools
|
||||
import re
|
||||
|
||||
from nova.api.openstack import api_version_request as api_version
|
||||
from nova.api.validation import validators
|
||||
@@ -119,13 +120,23 @@ def _strip_additional_query_parameters(schema, req):
|
||||
after _schema_validation_helper return `True`.
|
||||
"""
|
||||
additional_properties = schema.get('addtionalProperties', True)
|
||||
pattern_regexes = []
|
||||
|
||||
patterns = schema.get('patternProperties', None)
|
||||
if patterns:
|
||||
for regex in patterns:
|
||||
pattern_regexes.append(re.compile(regex))
|
||||
|
||||
if additional_properties:
|
||||
# `req.GET.keys` will return duplicated keys for multiple values
|
||||
# parameters. To get rid of duplicated keys, convert it to set.
|
||||
for param in set(req.GET.keys()):
|
||||
if param not in schema['properties'].keys():
|
||||
del req.GET[param]
|
||||
# keys that can match the patternProperties will be
|
||||
# retained and handle latter.
|
||||
if not (list(regex for regex in pattern_regexes if
|
||||
regex.match(param))):
|
||||
del req.GET[param]
|
||||
|
||||
|
||||
def query_schema(query_params_schema, min_version=None,
|
||||
|
||||
@@ -32,6 +32,17 @@ from nova import exception
|
||||
from nova.i18n import _
|
||||
|
||||
|
||||
@jsonschema.FormatChecker.cls_checks('regex')
|
||||
def _validate_regex_format(instance):
|
||||
if not isinstance(instance, six.text_type):
|
||||
return False
|
||||
try:
|
||||
re.compile(instance)
|
||||
except re.error:
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
@jsonschema.FormatChecker.cls_checks('date-time')
|
||||
def _validate_datetime_format(instance):
|
||||
try:
|
||||
|
||||
@@ -579,7 +579,7 @@ class ServersControllerTest(ControllerTest):
|
||||
|
||||
def test_get_servers_with_limit_bad_value(self):
|
||||
req = self.req('/fake/servers?limit=aaa')
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.controller.index, req)
|
||||
|
||||
def test_get_server_details_empty(self):
|
||||
@@ -594,7 +594,7 @@ class ServersControllerTest(ControllerTest):
|
||||
|
||||
def test_get_server_details_with_bad_name(self):
|
||||
req = self.req('/fake/servers/detail?name=%2Binstance')
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.controller.index, req)
|
||||
|
||||
def test_get_server_details_with_limit(self):
|
||||
@@ -616,7 +616,7 @@ class ServersControllerTest(ControllerTest):
|
||||
|
||||
def test_get_server_details_with_limit_bad_value(self):
|
||||
req = self.req('/fake/servers/detail?limit=aaa')
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.controller.detail, req)
|
||||
|
||||
def test_get_server_details_with_limit_and_other_params(self):
|
||||
@@ -635,7 +635,7 @@ class ServersControllerTest(ControllerTest):
|
||||
href_parts = urlparse.urlparse(servers_links[0]['href'])
|
||||
self.assertEqual('/v2/fake/servers/detail', href_parts.path)
|
||||
params = urlparse.parse_qs(href_parts.query)
|
||||
expected = {'limit': ['3'], 'blah': ['2:t'],
|
||||
expected = {'limit': ['3'],
|
||||
'sort_key': ['id1'], 'sort_dir': ['asc'],
|
||||
'marker': [fakes.get_fake_uuid(2)]}
|
||||
self.assertThat(params, matchers.DictMatches(expected))
|
||||
@@ -647,7 +647,7 @@ class ServersControllerTest(ControllerTest):
|
||||
|
||||
def test_get_servers_with_bad_limit(self):
|
||||
req = self.req('/fake/servers?limit=asdf')
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.controller.index, req)
|
||||
|
||||
def test_get_servers_with_marker(self):
|
||||
@@ -668,6 +668,16 @@ class ServersControllerTest(ControllerTest):
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.index, req)
|
||||
|
||||
def test_get_servers_with_invalid_filter_param(self):
|
||||
req = self.req('/fake/servers?info_cache=asdf',
|
||||
use_admin_context=True)
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.index, req)
|
||||
req = self.req('/fake/servers?__foo__=asdf',
|
||||
use_admin_context=True)
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.index, req)
|
||||
|
||||
def test_get_servers_with_bad_option(self):
|
||||
server_uuid = uuids.fake
|
||||
|
||||
@@ -1067,7 +1077,8 @@ class ServersControllerTest(ControllerTest):
|
||||
def test_get_servers_allows_changes_since_bad_value(self):
|
||||
params = 'changes-since=asdf'
|
||||
req = self.req('/fake/servers?%s' % params)
|
||||
self.assertRaises(webob.exc.HTTPBadRequest, self.controller.index, req)
|
||||
self.assertRaises(exception.ValidationError, self.controller.index,
|
||||
req)
|
||||
|
||||
def test_get_servers_admin_filters_as_user(self):
|
||||
"""Test getting servers by admin-only or unknown options when
|
||||
@@ -1116,7 +1127,7 @@ class ServersControllerTest(ControllerTest):
|
||||
self.assertIn('vm_state', search_opts)
|
||||
# Allowed only by admins with admin API on
|
||||
self.assertIn('ip', search_opts)
|
||||
self.assertIn('unknown_option', search_opts)
|
||||
self.assertNotIn('unknown_option', search_opts)
|
||||
return objects.InstanceList(
|
||||
objects=[fakes.stub_instance_obj(100, uuid=server_uuid)])
|
||||
|
||||
|
||||
@@ -36,6 +36,8 @@ query_schema = {
|
||||
'format': 'uuid'}),
|
||||
'foos': parameter_types.multi_params({'type': 'string'})
|
||||
},
|
||||
'patternProperties': {
|
||||
"^_": parameter_types.multi_params({'type': 'string'})},
|
||||
'additionalProperties': True
|
||||
}
|
||||
|
||||
@@ -47,6 +49,22 @@ class FakeQueryParametersController(object):
|
||||
return list(set(req.GET.keys()))
|
||||
|
||||
|
||||
class RegexFormatFakeController(object):
|
||||
|
||||
schema = {
|
||||
'type': 'object',
|
||||
'properties': {
|
||||
'foo': {
|
||||
'format': 'regex',
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
@validation.schema(request_body_schema=schema)
|
||||
def post(self, req, body):
|
||||
return 'Validation succeeded.'
|
||||
|
||||
|
||||
class FakeRequest(object):
|
||||
api_version_request = api_version.APIVersionRequest("2.1")
|
||||
environ = {}
|
||||
@@ -118,7 +136,7 @@ class APIValidationTestCase(test.NoDBTestCase):
|
||||
if not req:
|
||||
req = FakeRequest()
|
||||
try:
|
||||
method(body=body, req=req,)
|
||||
method(body=body, req=req)
|
||||
except exception.ValidationError as ex:
|
||||
self.assertEqual(400, ex.kwargs['code'])
|
||||
if not re.match(expected_detail, ex.kwargs['detail']):
|
||||
@@ -254,7 +272,7 @@ class QueryParamsSchemaTestCase(test.NoDBTestCase):
|
||||
|
||||
def test_strip_out_additional_properties(self):
|
||||
req = fakes.HTTPRequest.blank(
|
||||
"/tests?foos=abc&foo=%s&bar=123&bar=456" % fakes.FAKE_UUID)
|
||||
"/tests?foos=abc&foo=%s&bar=123&-bar=456" % fakes.FAKE_UUID)
|
||||
req.api_version_request = api_version.APIVersionRequest("2.3")
|
||||
res = self.controller.get(req)
|
||||
res.sort()
|
||||
@@ -271,6 +289,14 @@ class QueryParamsSchemaTestCase(test.NoDBTestCase):
|
||||
res.sort()
|
||||
self.assertEqual(['bar', 'foo', 'foos'], res)
|
||||
|
||||
def test_strip_out_correct_pattern_retained(self):
|
||||
req = fakes.HTTPRequest.blank(
|
||||
"/tests?foos=abc&foo=%s&bar=123&_foo_=456" % fakes.FAKE_UUID)
|
||||
req.api_version_request = api_version.APIVersionRequest("2.3")
|
||||
res = self.controller.get(req)
|
||||
res.sort()
|
||||
self.assertEqual(['_foo_', 'foo', 'foos'], res)
|
||||
|
||||
|
||||
class RequiredDisableTestCase(APIValidationTestCase):
|
||||
|
||||
@@ -1414,3 +1440,24 @@ class Base64TestCase(APIValidationTestCase):
|
||||
"Value: %s. '%s' is not a 'base64'") % (value, value)
|
||||
self.check_validation_error(self.post, body={'foo': value},
|
||||
expected_detail=detail)
|
||||
|
||||
|
||||
class RegexFormatTestCase(APIValidationTestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(APIValidationTestCase, self).setUp()
|
||||
self.controller = RegexFormatFakeController()
|
||||
|
||||
def test_validate_regex(self):
|
||||
req = fakes.HTTPRequest.blank("")
|
||||
self.assertEqual('Validation succeeded.',
|
||||
self.controller.post(req, body={'foo': u'Myserver'}))
|
||||
|
||||
def test_validate_regex_fails(self):
|
||||
value = 1
|
||||
req = fakes.HTTPRequest.blank("")
|
||||
detail = ("Invalid input for field/attribute foo. "
|
||||
"Value: %s. %s is not a 'regex'") % (value, value)
|
||||
self.check_validation_error(self.controller.post, req=req,
|
||||
body={'foo': value},
|
||||
expected_detail=detail)
|
||||
|
||||
Reference in New Issue
Block a user