From 2b1d8ea4a202f24c1b485ccebdbf831c505a4e6a Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Tue, 12 Jun 2018 22:20:35 +0000 Subject: [PATCH] Implement filter validation Enforce validation on filter parameters on list requests. If an API request contains an unknown or unsupported parameter, the server will return a 400 response instead of silently ignoring the invalid input. In resource attributes map, all filter parameters are annotated by the ``is_filter`` keyword. Attributes with is_filter set to True are candidates for validation. Enabling filter validation requires support from core plugin and all service plugins so each plugin need to indicate if it supports the validation by setting ``__filter_validation_support`` to True. If this field is not set, the default is False and validation is turned off. Right now, the ML2 plugin and all the in-tree service plugin support filter validation. Out-of-tree plugins will have filter validation disabled by default. An API extension is introduced to allow API users to discover this new API behavior. This feature can be disabled by cloud operators if they choose to do that. If it is disabled, the extension won't be presented. Depends-On: Ic3ab5b3ffdc378d570678b9c967cb42b0c7a8a9b Depends-On: I4397df1c35463a8b532afdc9c5d28b37224a37b4 Depends-On: I3f2e6e861adaeef81a1a5819a57b28f5c6281d80 Depends-On: I1189bc9a50308df5c7e18c329f3a1262c90b9e12 Depends-On: I057cd917628c77dd20c0ff7747936c3fec7b4844 Depends-On: I0b24a304cc3466a2c05426cdbb6f9d99f1797edd Change-Id: I21bf8a752813802822fd9966dda6ab3b6c4abfdc Partial-Bug: #1749820 --- neutron/api/api_common.py | 30 +++++++++-- neutron/api/v2/base.py | 12 +++-- neutron/common/utils.py | 9 ++++ neutron/conf/common.py | 5 ++ neutron/db/db_base_plugin_v2.py | 4 ++ neutron/extensions/_filter_validation_lib.py | 30 +++++++++++ .../_port_mac_address_regenerate_lib.py | 2 + neutron/extensions/filter_validation.py | 35 +++++++++++++ neutron/extensions/network_ip_availability.py | 13 +++++ neutron/extensions/portbindings_extended.py | 11 ++++ neutron/extensions/rbac.py | 16 ++++-- neutron/extensions/securitygroup.py | 26 +++++++--- neutron/extensions/segment.py | 6 +++ neutron/manager.py | 4 ++ neutron/pecan_wsgi/controllers/utils.py | 2 + neutron/pecan_wsgi/hooks/query_parameters.py | 3 +- neutron/plugins/ml2/plugin.py | 7 +++ neutron/services/auto_allocate/plugin.py | 2 + neutron/services/flavors/flavors_plugin.py | 2 + .../services/l3_router/l3_router_plugin.py | 1 + neutron/services/logapi/logging_plugin.py | 1 + neutron/services/metering/metering_plugin.py | 1 + .../network_ip_availability/plugin.py | 2 + neutron/services/qos/qos_plugin.py | 1 + neutron/services/revisions/revision_plugin.py | 2 + neutron/services/segments/plugin.py | 1 + neutron/services/tag/tag_plugin.py | 2 + .../services/timestamp/timestamp_plugin.py | 2 + neutron/services/trunk/plugin.py | 1 + .../tests/contrib/hooks/api_all_extensions | 1 + neutron/tests/unit/api/v2/test_base.py | 50 ++++++++++++++++++- .../tests/unit/db/test_db_base_plugin_v2.py | 31 ++++++++---- neutron/tests/unit/test_manager.py | 1 + ...rt-filter-validation-fee2cdeedbe8ad76.yaml | 38 ++++++++++++++ 34 files changed, 324 insertions(+), 30 deletions(-) create mode 100644 neutron/extensions/_filter_validation_lib.py create mode 100644 neutron/extensions/filter_validation.py create mode 100644 releasenotes/notes/support-filter-validation-fee2cdeedbe8ad76.yaml diff --git a/neutron/api/api_common.py b/neutron/api/api_common.py index 53ca78f9517..1092601c26a 100644 --- a/neutron/api/api_common.py +++ b/neutron/api/api_common.py @@ -62,13 +62,21 @@ def check_request_for_revision_constraint(request): return revision_number -def get_filters(request, attr_info, skips=None): +def is_filter_validation_enabled(): + return 'filter-validation' in (extensions.PluginAwareExtensionManager. + get_instance().extensions) + + +def get_filters(request, attr_info, skips=None, + is_filter_validation_supported=False): return get_filters_from_dict(request.GET.dict_of_lists(), attr_info, - skips) + skips, + is_filter_validation_supported) -def get_filters_from_dict(data, attr_info, skips=None): +def get_filters_from_dict(data, attr_info, skips=None, + is_filter_validation_supported=False): """Extracts the filters from a dict of query parameters. Returns a dict of lists for the filters: @@ -80,12 +88,19 @@ def get_filters_from_dict(data, attr_info, skips=None): is_empty_string_supported = is_empty_string_filtering_supported() skips = skips or [] res = {} + invalid_keys = [] + check_is_filter = False + if is_filter_validation_supported and is_filter_validation_enabled(): + check_is_filter = True for key, values in data.items(): if key in skips or hasattr(model_base.BASEV2, key): continue values = [v for v in values if v or (v == "" and is_empty_string_supported)] key_attr_info = attr_info.get(key, {}) + if check_is_filter and not key_attr_info.get('is_filter'): + invalid_keys.append(key) + continue if 'convert_list_to' in key_attr_info: values = key_attr_info['convert_list_to'](values) elif 'convert_to' in key_attr_info: @@ -93,6 +108,9 @@ def get_filters_from_dict(data, attr_info, skips=None): values = [convert_to(v) for v in values] if values: res[key] = values + if invalid_keys: + msg = _("%s is invalid attribute for filtering") % invalid_keys + raise exc.HTTPBadRequest(explanation=msg) return res @@ -248,6 +266,12 @@ def is_native_sorting_supported(plugin): return getattr(plugin, native_sorting_attr_name, False) +def is_filter_validation_supported(plugin): + filter_validation_attr_name = ("_%s__filter_validation_support" + % plugin.__class__.__name__) + return getattr(plugin, filter_validation_attr_name, False) + + class PaginationHelper(object): def __init__(self, request, primary_key='id'): diff --git a/neutron/api/v2/base.py b/neutron/api/v2/base.py index d965c277414..6dd6211ca8e 100644 --- a/neutron/api/v2/base.py +++ b/neutron/api/v2/base.py @@ -105,6 +105,7 @@ class Controller(object): self._native_bulk = self._is_native_bulk_supported() self._native_pagination = self._is_native_pagination_supported() self._native_sorting = self._is_native_sorting_supported() + self._filter_validation = self._is_filter_validation_supported() self._policy_attrs = self._init_policy_attrs() self._notifier = n_rpc.get_notifier('network') self._member_actions = member_actions @@ -151,6 +152,9 @@ class Controller(object): def _is_native_sorting_supported(self): return api_common.is_native_sorting_supported(self._plugin) + def _is_filter_validation_supported(self): + return api_common.is_filter_validation_supported(self._plugin) + def _exclude_attributes_by_policy(self, context, data): """Identifies attributes to exclude according to authZ policies. @@ -282,9 +286,11 @@ class Controller(object): # plugin before returning. original_fields, fields_to_add = self._do_field_list( api_common.list_args(request, 'fields')) - filters = api_common.get_filters(request, self._attr_info, - ['fields', 'sort_key', 'sort_dir', - 'limit', 'marker', 'page_reverse']) + filters = api_common.get_filters( + request, self._attr_info, + ['fields', 'sort_key', 'sort_dir', + 'limit', 'marker', 'page_reverse'], + is_filter_validation_supported=self._filter_validation) kwargs = {'filters': filters, 'fields': original_fields} sorting_helper = self._get_sorting_helper(request) diff --git a/neutron/common/utils.py b/neutron/common/utils.py index ad3f0285018..9b1efcc680c 100644 --- a/neutron/common/utils.py +++ b/neutron/common/utils.py @@ -44,6 +44,7 @@ import six import neutron from neutron._i18n import _ +from neutron.api import api_common from neutron.common import exceptions from neutron.db import api as db_api @@ -818,3 +819,11 @@ def get_port_binding_by_status_and_host(bindings, status, host='', return binding if raise_if_not_found: raise exceptions.PortBindingNotFound(port_id=port_id, host=host) + + +def disable_extension_by_service_plugin(core_plugin, service_plugin): + if ('filter-validation' in core_plugin.supported_extension_aliases and + not api_common.is_filter_validation_supported(service_plugin)): + core_plugin.supported_extension_aliases.remove('filter-validation') + LOG.info('Disable filter validation extension by service plugin ' + '%s.', service_plugin.__class__.__name__) diff --git a/neutron/conf/common.py b/neutron/conf/common.py index a1a46570148..026a21eb6a9 100644 --- a/neutron/conf/common.py +++ b/neutron/conf/common.py @@ -117,6 +117,11 @@ core_opts = [ cfg.BoolOpt('vlan_transparent', default=False, help=_('If True, then allow plugins that support it to ' 'create VLAN transparent networks.')), + cfg.BoolOpt('filter_validation', default=True, + help=_('If True, then allow plugins to decide ' + 'whether to perform validations on filter parameters. ' + 'Filter validation is enabled if this config' + 'is turned on and it is supported by all plugins')), cfg.IntOpt('global_physnet_mtu', default=constants.DEFAULT_NETWORK_MTU, deprecated_name='segment_mtu', deprecated_group='ml2', help=_('MTU of the underlying physical network. Neutron uses ' diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index c74c978f9eb..844297a4817 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -140,6 +140,10 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon, __native_bulk_support = True __native_pagination_support = True __native_sorting_support = True + # This attribute specifies whether the plugin supports or not + # filter validations. Name mangling is used in + # order to ensure it is qualified by class + __filter_validation_support = False def has_native_datastore(self): return True diff --git a/neutron/extensions/_filter_validation_lib.py b/neutron/extensions/_filter_validation_lib.py new file mode 100644 index 00000000000..ba6fb4d1639 --- /dev/null +++ b/neutron/extensions/_filter_validation_lib.py @@ -0,0 +1,30 @@ +# 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. + +""" +This module should be deleted once neutron-lib containing +https://review.openstack.org/#/c/580190/ change is released. +""" + + +ALIAS = 'filter-validation' +IS_SHIM_EXTENSION = True +IS_STANDARD_ATTR_EXTENSION = False +NAME = 'Filter parameters validation' +DESCRIPTION = 'Provides validation on filter parameters.' +UPDATED_TIMESTAMP = '2018-03-21T10:00:00-00:00' +RESOURCE_ATTRIBUTE_MAP = {} +SUB_RESOURCE_ATTRIBUTE_MAP = {} +ACTION_MAP = {} +REQUIRED_EXTENSIONS = [] +OPTIONAL_EXTENSIONS = [] +ACTION_STATUS = {} diff --git a/neutron/extensions/_port_mac_address_regenerate_lib.py b/neutron/extensions/_port_mac_address_regenerate_lib.py index 4a75cbcd8c1..601f293c137 100644 --- a/neutron/extensions/_port_mac_address_regenerate_lib.py +++ b/neutron/extensions/_port_mac_address_regenerate_lib.py @@ -46,6 +46,8 @@ RESOURCE_ATTRIBUTE_MAP = { 'convert_to': convert_to_mac_if_none, 'validate': {'type:mac_address': None}, 'enforce_policy': True, + 'is_filter': True, + 'is_sort_key': True, 'is_visible': True}, } } diff --git a/neutron/extensions/filter_validation.py b/neutron/extensions/filter_validation.py new file mode 100644 index 00000000000..a8b42cdf85b --- /dev/null +++ b/neutron/extensions/filter_validation.py @@ -0,0 +1,35 @@ +# Copyright (c) 2017 Huawei Technology, Inc. All rights reserved. +# +# 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. + +from neutron_lib.api import extensions +from oslo_config import cfg +from oslo_log import log as logging + +from neutron.extensions import _filter_validation_lib as apidef + + +LOG = logging.getLogger(__name__) + + +def _disable_extension_by_config(aliases): + if not cfg.CONF.filter_validation: + if 'filter-validation' in aliases: + aliases.remove('filter-validation') + LOG.info('Disabled filter validation extension.') + + +class Filter_validation(extensions.APIExtensionDescriptor): + """Extension class supporting filter validation.""" + + api_definition = apidef diff --git a/neutron/extensions/network_ip_availability.py b/neutron/extensions/network_ip_availability.py index 1636f58c892..e849b2beb3c 100644 --- a/neutron/extensions/network_ip_availability.py +++ b/neutron/extensions/network_ip_availability.py @@ -30,6 +30,9 @@ class Network_ip_availability(api_extensions.APIExtensionDescriptor): """Returns Extended Resource for service type management.""" resource_attributes = apidef.RESOURCE_ATTRIBUTE_MAP[ apidef.RESOURCE_PLURAL] + # TODO(hongbin): Delete _populate_is_filter_keyword once neutron-lib + # containing https://review.openstack.org/#/c/583838/ is released. + cls._populate_is_filter_keyword(resource_attributes) controller = base.create_resource( apidef.RESOURCE_PLURAL, apidef.RESOURCE_NAME, @@ -38,3 +41,13 @@ class Network_ip_availability(api_extensions.APIExtensionDescriptor): return [extensions.ResourceExtension(apidef.COLLECTION_NAME, controller, attr_map=resource_attributes)] + + @classmethod + def _populate_is_filter_keyword(cls, params): + filter_keys = ['network_id', 'network_name', 'tenant_id', + 'project_id'] + for name in params: + if name in filter_keys: + params[name]['is_filter'] = True + params['ip_version'] = {'allow_post': False, 'allow_put': False, + 'is_visible': False, 'is_filter': True} diff --git a/neutron/extensions/portbindings_extended.py b/neutron/extensions/portbindings_extended.py index dab95a2244c..53d7a5ac28f 100644 --- a/neutron/extensions/portbindings_extended.py +++ b/neutron/extensions/portbindings_extended.py @@ -52,6 +52,9 @@ class Portbindings_extended(api_extensions.ExtensionDescriptor): params = pbe_ext.SUB_RESOURCE_ATTRIBUTE_MAP[ pbe_ext.COLLECTION_NAME]['parameters'] + # TODO(hongbin): Delete _populate_is_filter_keyword once neutron-lib + # containing https://review.openstack.org/#/c/583437/ is released. + cls._populate_is_filter_keyword(params) parent = pbe_ext.SUB_RESOURCE_ATTRIBUTE_MAP[ pbe_ext.COLLECTION_NAME]['parent'] controller = base.create_resource( @@ -75,3 +78,11 @@ class Portbindings_extended(api_extensions.ExtensionDescriptor): ] return exts + + @classmethod + def _populate_is_filter_keyword(cls, params): + filter_keys = [pbe_ext.HOST, pbe_ext.VIF_TYPE, pbe_ext.VNIC_TYPE, + pbe_ext.STATUS] + for name in params: + if name in filter_keys: + params[name]['is_filter'] = True diff --git a/neutron/extensions/rbac.py b/neutron/extensions/rbac.py index 7db4347be85..71d4eabbf23 100644 --- a/neutron/extensions/rbac.py +++ b/neutron/extensions/rbac.py @@ -54,22 +54,27 @@ RESOURCE_ATTRIBUTE_MAP = { RESOURCE_COLLECTION: { 'id': {'allow_post': False, 'allow_put': False, 'validate': {'type:uuid': None}, - 'is_visible': True, 'primary_key': True}, + 'is_visible': True, 'primary_key': True, + 'is_filter': True}, 'object_type': {'allow_post': True, 'allow_put': False, 'convert_to': convert_valid_object_type, 'is_visible': True, 'default': None, + 'is_filter': True, 'enforce_policy': True}, 'object_id': {'allow_post': True, 'allow_put': False, 'validate': {'type:uuid': None}, - 'is_visible': True, 'enforce_policy': True}, + 'is_visible': True, 'enforce_policy': True, + 'is_filter': True}, 'target_tenant': {'allow_post': True, 'allow_put': True, 'validate': { 'type:string': db_const.PROJECT_ID_FIELD_SIZE}, - 'is_visible': True, 'enforce_policy': True}, + 'is_visible': True, 'enforce_policy': True, + 'is_filter': True}, 'tenant_id': {'allow_post': True, 'allow_put': False, 'validate': { 'type:string': db_const.PROJECT_ID_FIELD_SIZE}, - 'required_by_policy': True, 'is_visible': True}, + 'required_by_policy': True, 'is_visible': True, + 'is_filter': True}, 'action': {'allow_post': True, 'allow_put': False, # action depends on type so validation has to occur in # the extension @@ -77,7 +82,8 @@ RESOURCE_ATTRIBUTE_MAP = { 'type:string': db_const.DESCRIPTION_FIELD_SIZE}, # we set enforce_policy so operators can define policies # that restrict actions - 'is_visible': True, 'enforce_policy': True} + 'is_visible': True, 'enforce_policy': True, + 'is_filter': True} } } diff --git a/neutron/extensions/securitygroup.py b/neutron/extensions/securitygroup.py index d2411df01b1..6dc09b4ec4b 100644 --- a/neutron/extensions/securitygroup.py +++ b/neutron/extensions/securitygroup.py @@ -221,16 +221,17 @@ RESOURCE_ATTRIBUTE_MAP = { 'id': {'allow_post': False, 'allow_put': False, 'validate': {'type:uuid': None}, 'is_visible': True, + 'is_filter': True, 'primary_key': True}, 'name': {'allow_post': True, 'allow_put': True, - 'is_visible': True, 'default': '', + 'is_visible': True, 'default': '', 'is_filter': True, 'validate': { 'type:name_not_default': db_const.NAME_FIELD_SIZE}}, 'tenant_id': {'allow_post': True, 'allow_put': False, 'required_by_policy': True, 'validate': { 'type:string': db_const.PROJECT_ID_FIELD_SIZE}, - 'is_visible': True}, + 'is_visible': True, 'is_filter': True}, SECURITYGROUPRULES: {'allow_post': False, 'allow_put': False, 'is_visible': True}, }, @@ -238,35 +239,43 @@ RESOURCE_ATTRIBUTE_MAP = { 'id': {'allow_post': False, 'allow_put': False, 'validate': {'type:uuid': None}, 'is_visible': True, + 'is_filter': True, 'primary_key': True}, 'security_group_id': {'allow_post': True, 'allow_put': False, - 'is_visible': True, 'required_by_policy': True}, + 'is_visible': True, 'required_by_policy': True, + 'is_filter': True}, 'remote_group_id': {'allow_post': True, 'allow_put': False, - 'default': None, 'is_visible': True}, + 'default': None, 'is_visible': True, + 'is_filter': True}, 'direction': {'allow_post': True, 'allow_put': False, - 'is_visible': True, + 'is_visible': True, 'is_filter': True, 'validate': {'type:values': ['ingress', 'egress']}}, 'protocol': {'allow_post': True, 'allow_put': False, 'is_visible': True, 'default': None, + 'is_filter': True, 'convert_to': convert_protocol}, 'port_range_min': {'allow_post': True, 'allow_put': False, 'convert_to': convert_validate_port_value, - 'default': None, 'is_visible': True}, + 'default': None, 'is_visible': True, + 'is_filter': True}, 'port_range_max': {'allow_post': True, 'allow_put': False, 'convert_to': convert_validate_port_value, - 'default': None, 'is_visible': True}, + 'default': None, 'is_visible': True, + 'is_filter': True}, 'ethertype': {'allow_post': True, 'allow_put': False, 'is_visible': True, 'default': 'IPv4', + 'is_filter': True, 'convert_to': convert_ethertype_to_case_insensitive, 'validate': {'type:values': sg_supported_ethertypes}}, 'remote_ip_prefix': {'allow_post': True, 'allow_put': False, 'default': None, 'is_visible': True, + 'is_filter': True, 'convert_to': convert_ip_prefix_to_cidr}, 'tenant_id': {'allow_post': True, 'allow_put': False, 'required_by_policy': True, 'validate': { 'type:string': db_const.PROJECT_ID_FIELD_SIZE}, - 'is_visible': True}, + 'is_visible': True, 'is_filter': True}, } } @@ -275,6 +284,7 @@ EXTENDED_ATTRIBUTES_2_0 = { 'ports': {SECURITYGROUPS: {'allow_post': True, 'allow_put': True, 'is_visible': True, + 'is_filter': True, 'convert_to': converters.convert_none_to_empty_list, 'validate': {'type:uuid_list': None}, diff --git a/neutron/extensions/segment.py b/neutron/extensions/segment.py index d27aeb3473b..c1a585c3e23 100644 --- a/neutron/extensions/segment.py +++ b/neutron/extensions/segment.py @@ -44,6 +44,7 @@ RESOURCE_ATTRIBUTE_MAP = { 'id': {'allow_post': False, 'allow_put': False, 'validate': {'type:uuid': None}, + 'is_filter': True, 'is_visible': True, 'primary_key': True}, 'tenant_id': {'allow_post': True, @@ -54,17 +55,20 @@ RESOURCE_ATTRIBUTE_MAP = { 'network_id': {'allow_post': True, 'allow_put': False, 'validate': {'type:uuid': None}, + 'is_filter': True, 'is_visible': True}, PHYSICAL_NETWORK: {'allow_post': True, 'allow_put': False, 'default': constants.ATTR_NOT_SPECIFIED, 'validate': {'type:string': providernet.PHYSICAL_NETWORK_MAX_LEN}, + 'is_filter': True, 'is_visible': True}, NETWORK_TYPE: {'allow_post': True, 'allow_put': False, 'validate': {'type:string': providernet.NETWORK_TYPE_MAX_LEN}, + 'is_filter': True, 'is_visible': True}, SEGMENTATION_ID: {'allow_post': True, 'allow_put': False, @@ -75,6 +79,7 @@ RESOURCE_ATTRIBUTE_MAP = { 'allow_put': True, 'default': constants.ATTR_NOT_SPECIFIED, 'validate': {'type:string_or_none': NAME_LEN}, + 'is_filter': True, 'is_visible': True} }, subnet_def.COLLECTION_NAME: { @@ -82,6 +87,7 @@ RESOURCE_ATTRIBUTE_MAP = { 'allow_put': False, 'default': None, 'validate': {'type:uuid_or_none': None}, + 'is_filter': True, 'is_visible': True, }, }, } diff --git a/neutron/manager.py b/neutron/manager.py index d1bff0f03e0..e35966488c3 100644 --- a/neutron/manager.py +++ b/neutron/manager.py @@ -27,6 +27,7 @@ from osprofiler import profiler import six from neutron._i18n import _ +from neutron.common import utils from neutron.plugins.common import constants @@ -221,6 +222,9 @@ class NeutronManager(object): hasattr(plugin_inst, 'agent_notifiers')): plugin.agent_notifiers.update(plugin_inst.agent_notifiers) + # disable incompatible extensions in core plugin if any + utils.disable_extension_by_service_plugin(plugin, plugin_inst) + LOG.debug("Successfully loaded %(type)s plugin. " "Description: %(desc)s", {"type": plugin_type, diff --git a/neutron/pecan_wsgi/controllers/utils.py b/neutron/pecan_wsgi/controllers/utils.py index 52815a8a722..ffcd552b927 100644 --- a/neutron/pecan_wsgi/controllers/utils.py +++ b/neutron/pecan_wsgi/controllers/utils.py @@ -167,6 +167,8 @@ class NeutronPecanController(object): raise exceptions.Invalid( _("Native pagination depends on native sorting") ) + self.filter_validation = api_common.is_filter_validation_supported( + self.plugin) self.primary_key = self._get_primary_key() self.parent = parent_resource diff --git a/neutron/pecan_wsgi/hooks/query_parameters.py b/neutron/pecan_wsgi/hooks/query_parameters.py index 4a81ca24ecc..800aa6ccde0 100644 --- a/neutron/pecan_wsgi/hooks/query_parameters.py +++ b/neutron/pecan_wsgi/hooks/query_parameters.py @@ -77,7 +77,8 @@ def _set_filters(state, controller): {k: _listify(v) for k, v in params.items()}, controller.resource_info, skips=['fields', 'sort_key', 'sort_dir', - 'limit', 'marker', 'page_reverse']) + 'limit', 'marker', 'page_reverse'], + is_filter_validation_supported=controller.filter_validation) return filters diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index ab32d53aa31..746baab5c85 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -87,6 +87,7 @@ from neutron.db import securitygroups_rpc_base as sg_db_rpc from neutron.db import segments_db from neutron.db import subnet_service_type_mixin from neutron.db import vlantransparent_db +from neutron.extensions import filter_validation from neutron.extensions import providernet as provider from neutron.extensions import vlantransparent from neutron.objects import base as base_obj @@ -148,6 +149,10 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, __native_bulk_support = True __native_pagination_support = True __native_sorting_support = True + # This attribute specifies whether the plugin supports or not + # filter validations. Name mangling is used in + # order to ensure it is qualified by class + __filter_validation_support = True # List of supported extensions _supported_extension_aliases = ["provider", "external-net", "binding", @@ -166,6 +171,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, "ip-substring-filtering", "port-security-groups-filtering", "empty-string-filtering", + "filter-validation", "port-mac-address-regenerate", "binding-extended"] @@ -176,6 +182,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, aliases += self.extension_manager.extension_aliases() sg_rpc.disable_security_group_extension_by_config(aliases) vlantransparent._disable_extension_by_config(aliases) + filter_validation._disable_extension_by_config(aliases) self._aliases = aliases return self._aliases diff --git a/neutron/services/auto_allocate/plugin.py b/neutron/services/auto_allocate/plugin.py index 0d6abe53881..1a90aa514be 100644 --- a/neutron/services/auto_allocate/plugin.py +++ b/neutron/services/auto_allocate/plugin.py @@ -24,6 +24,8 @@ class Plugin(db.AutoAllocatedTopologyMixin): supported_extension_aliases = ["auto-allocated-topology"] + __filter_validation_support = True + @classmethod def get_instance(cls): if cls._instance is None: diff --git a/neutron/services/flavors/flavors_plugin.py b/neutron/services/flavors/flavors_plugin.py index e6644344eb8..3ca6bc8d5d3 100644 --- a/neutron/services/flavors/flavors_plugin.py +++ b/neutron/services/flavors/flavors_plugin.py @@ -25,6 +25,8 @@ class FlavorsPlugin(service_base.ServicePluginBase, supported_extension_aliases = ['flavors', 'service-type'] + __filter_validation_support = True + @classmethod def get_plugin_type(cls): return constants.FLAVORS diff --git a/neutron/services/l3_router/l3_router_plugin.py b/neutron/services/l3_router/l3_router_plugin.py index 2b55924c03e..ba56aa6718b 100644 --- a/neutron/services/l3_router/l3_router_plugin.py +++ b/neutron/services/l3_router/l3_router_plugin.py @@ -88,6 +88,7 @@ class L3RouterPlugin(service_base.ServicePluginBase, __native_pagination_support = True __native_sorting_support = True + __filter_validation_support = True @resource_registry.tracked_resources(router=l3_models.Router, floatingip=l3_models.FloatingIP) diff --git a/neutron/services/logapi/logging_plugin.py b/neutron/services/logapi/logging_plugin.py index 2368ac866eb..19811655fcd 100644 --- a/neutron/services/logapi/logging_plugin.py +++ b/neutron/services/logapi/logging_plugin.py @@ -31,6 +31,7 @@ class LoggingPlugin(log_ext.LoggingPluginBase): __native_pagination_support = True __native_sorting_support = True + __filter_validation_support = True def __init__(self): super(LoggingPlugin, self).__init__() diff --git a/neutron/services/metering/metering_plugin.py b/neutron/services/metering/metering_plugin.py index 76b9f1754c2..cce6b1bea9a 100644 --- a/neutron/services/metering/metering_plugin.py +++ b/neutron/services/metering/metering_plugin.py @@ -26,6 +26,7 @@ class MeteringPlugin(metering_db.MeteringDbMixin): """Implementation of the Neutron Metering Service Plugin.""" supported_extension_aliases = [metering_apidef.ALIAS] path_prefix = "/metering" + __filter_validation_support = True def __init__(self): super(MeteringPlugin, self).__init__() diff --git a/neutron/services/network_ip_availability/plugin.py b/neutron/services/network_ip_availability/plugin.py index 5391763a90c..8eadd513c9a 100644 --- a/neutron/services/network_ip_availability/plugin.py +++ b/neutron/services/network_ip_availability/plugin.py @@ -27,6 +27,8 @@ class NetworkIPAvailabilityPlugin(ip_availability_db.IpAvailabilityMixin, supported_extension_aliases = ["network-ip-availability"] + __filter_validation_support = True + @classmethod def get_instance(cls): if cls._instance is None: diff --git a/neutron/services/qos/qos_plugin.py b/neutron/services/qos/qos_plugin.py index e4288509a83..c870e5acfe9 100644 --- a/neutron/services/qos/qos_plugin.py +++ b/neutron/services/qos/qos_plugin.py @@ -47,6 +47,7 @@ class QoSPlugin(qos.QoSPluginBase): __native_pagination_support = True __native_sorting_support = True + __filter_validation_support = True def __init__(self): super(QoSPlugin, self).__init__() diff --git a/neutron/services/revisions/revision_plugin.py b/neutron/services/revisions/revision_plugin.py index 9115cf5ac9c..fc1ebd8ec7b 100644 --- a/neutron/services/revisions/revision_plugin.py +++ b/neutron/services/revisions/revision_plugin.py @@ -33,6 +33,8 @@ class RevisionPlugin(service_base.ServicePluginBase): supported_extension_aliases = ['standard-attr-revisions', 'revision-if-match'] + __filter_validation_support = True + def __init__(self): super(RevisionPlugin, self).__init__() db_api.sqla_listen(se.Session, 'before_flush', self.bump_revisions) diff --git a/neutron/services/segments/plugin.py b/neutron/services/segments/plugin.py index 1af8996b464..79526dc8083 100644 --- a/neutron/services/segments/plugin.py +++ b/neutron/services/segments/plugin.py @@ -68,6 +68,7 @@ class Plugin(db.SegmentDbMixin, segment.SegmentPluginBase): __native_pagination_support = True __native_sorting_support = True + __filter_validation_support = True def __init__(self): self.nova_updater = NovaSegmentNotifier() diff --git a/neutron/services/tag/tag_plugin.py b/neutron/services/tag/tag_plugin.py index 153da550641..c562eb9c334 100644 --- a/neutron/services/tag/tag_plugin.py +++ b/neutron/services/tag/tag_plugin.py @@ -39,6 +39,8 @@ class TagPlugin(common_db_mixin.CommonDbMixin, tagging.TagPluginBase): supported_extension_aliases = ['standard-attr-tag'] + __filter_validation_support = True + def __new__(cls, *args, **kwargs): inst = super(TagPlugin, cls).__new__(cls, *args, **kwargs) inst._filter_methods = [] # prevent GC of our partial functions diff --git a/neutron/services/timestamp/timestamp_plugin.py b/neutron/services/timestamp/timestamp_plugin.py index cfa8dda5a3a..bad75db929f 100644 --- a/neutron/services/timestamp/timestamp_plugin.py +++ b/neutron/services/timestamp/timestamp_plugin.py @@ -25,6 +25,8 @@ class TimeStampPlugin(service_base.ServicePluginBase, supported_extension_aliases = ['standard-attr-timestamp'] + __filter_validation_support = True + def __init__(self): super(TimeStampPlugin, self).__init__() self.register_db_events() diff --git a/neutron/services/trunk/plugin.py b/neutron/services/trunk/plugin.py index 7df8efd61ae..6270ea0b1a3 100644 --- a/neutron/services/trunk/plugin.py +++ b/neutron/services/trunk/plugin.py @@ -50,6 +50,7 @@ class TrunkPlugin(service_base.ServicePluginBase, __native_pagination_support = True __native_sorting_support = True + __filter_validation_support = True def __init__(self): self._rpc_backend = None diff --git a/neutron/tests/contrib/hooks/api_all_extensions b/neutron/tests/contrib/hooks/api_all_extensions index c8e41788787..10cddd9c8eb 100644 --- a/neutron/tests/contrib/hooks/api_all_extensions +++ b/neutron/tests/contrib/hooks/api_all_extensions @@ -17,6 +17,7 @@ NETWORK_API_EXTENSIONS+=",ext-gw-mode" NETWORK_API_EXTENSIONS+=",external-net" NETWORK_API_EXTENSIONS+=",extra_dhcp_opt" NETWORK_API_EXTENSIONS+=",extraroute" +NETWORK_API_EXTENSIONS+=",filter-validation" NETWORK_API_EXTENSIONS+=",fip-port-details" NETWORK_API_EXTENSIONS+=",flavors" NETWORK_API_EXTENSIONS+=",ip-substring-filtering" diff --git a/neutron/tests/unit/api/v2/test_base.py b/neutron/tests/unit/api/v2/test_base.py index 658aab793c3..902733b36f3 100644 --- a/neutron/tests/unit/api/v2/test_base.py +++ b/neutron/tests/unit/api/v2/test_base.py @@ -86,9 +86,11 @@ class APIv2TestBase(base.BaseTestCase): self._plugin_patcher = mock.patch(plugin, autospec=True) self.plugin = self._plugin_patcher.start() instance = self.plugin.return_value - instance.supported_extension_aliases = ['empty-string-filtering'] + instance.supported_extension_aliases = ['empty-string-filtering', + 'filter-validation'] instance._NeutronPluginBaseV2__native_pagination_support = True instance._NeutronPluginBaseV2__native_sorting_support = True + instance._NeutronPluginBaseV2__filter_validation_support = True tools.make_mock_plugin_json_encodable(instance) api = router.APIRouter() @@ -1612,6 +1614,52 @@ class FiltersTestCase(base.BaseTestCase): 'project_id': {'key': 'val'}} self.assertEqual(expect_attr_info, attr_info) + @mock.patch('neutron.api.api_common.is_filter_validation_enabled', + return_value=True) + def test_attr_info_with_filter_validation(self, mock_validation_enabled): + attr_info = {} + self._test_attr_info(attr_info) + + attr_info = {'foo': {}} + self._test_attr_info(attr_info) + + attr_info = {'foo': {'is_filter': False}} + self._test_attr_info(attr_info) + + attr_info = {'foo': {'is_filter': False}, 'bar': {'is_filter': True}, + 'baz': {'is_filter': True}, 'qux': {'is_filter': True}} + self._test_attr_info(attr_info) + + attr_info = {'foo': {'is_filter': True}, 'bar': {'is_filter': True}, + 'baz': {'is_filter': True}, 'qux': {'is_filter': True}} + expect_val = {'foo': ['4'], 'bar': ['3'], 'baz': ['2'], 'qux': ['1']} + self._test_attr_info(attr_info, expect_val) + + attr_info = {'foo': {'is_filter': True}, 'bar': {'is_filter': True}, + 'baz': {'is_filter': True}, 'qux': {'is_filter': True}, + 'quz': {}} + expect_val = {'foo': ['4'], 'bar': ['3'], 'baz': ['2'], 'qux': ['1']} + self._test_attr_info(attr_info, expect_val) + + attr_info = {'foo': {'is_filter': True}, 'bar': {'is_filter': True}, + 'baz': {'is_filter': True}, 'qux': {'is_filter': True}, + 'quz': {'is_filter': False}} + expect_val = {'foo': ['4'], 'bar': ['3'], 'baz': ['2'], 'qux': ['1']} + self._test_attr_info(attr_info, expect_val) + + def _test_attr_info(self, attr_info, expect_val=None): + path = '/?foo=4&bar=3&baz=2&qux=1' + request = webob.Request.blank(path) + if expect_val: + actual_val = api_common.get_filters( + request, attr_info, + is_filter_validation_supported=True) + self.assertEqual(expect_val, actual_val) + else: + self.assertRaises( + exc.HTTPBadRequest, api_common.get_filters, request, attr_info, + is_filter_validation_supported=True) + def test_attr_info_without_conversion(self): path = '/?foo=4&bar=3&baz=2&qux=1' request = webob.Request.blank(path) diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index 3ff9c0477b3..d9b11897f14 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -158,6 +158,12 @@ class NeutronDbPluginV2TestCase(testlib_api.WebTestCase): self._skip_native_pagination = not _is_native_pagination_support() + def _is_filter_validation_support(): + return 'filter-validation' in (directory.get_plugin(). + supported_extension_aliases) + + self._skip_filter_validation = not _is_filter_validation_support() + def _is_native_sorting_support(): native_sorting_attr_name = ( "_%s__native_sorting_support" % @@ -560,13 +566,13 @@ class NeutronDbPluginV2TestCase(testlib_api.WebTestCase): return self.deserialize(self.fmt, res) def _list(self, resource, fmt=None, neutron_context=None, - query_params=None): + query_params=None, expected_code=webob.exc.HTTPOk.code): fmt = fmt or self.fmt req = self.new_list_request(resource, fmt, query_params) if neutron_context: req.environ['neutron.context'] = neutron_context res = req.get_response(self._api_for_resource(resource)) - self.assertEqual(webob.exc.HTTPOk.code, res.status_int) + self.assertEqual(expected_code, res.status_int) return self.deserialize(fmt, res) def _fail_second_call(self, patched_plugin, orig, *args, **kwargs): @@ -595,13 +601,16 @@ class NeutronDbPluginV2TestCase(testlib_api.WebTestCase): self.assertEqual(items[1]['name'], 'test_1') def _test_list_resources(self, resource, items, neutron_context=None, - query_params=None): + query_params=None, + expected_code=webob.exc.HTTPOk.code): res = self._list('%ss' % resource, neutron_context=neutron_context, - query_params=query_params) - resource = resource.replace('-', '_') - self.assertItemsEqual([i['id'] for i in res['%ss' % resource]], - [i[resource]['id'] for i in items]) + query_params=query_params, + expected_code=expected_code) + if expected_code == webob.exc.HTTPOk.code: + resource = resource.replace('-', '_') + self.assertItemsEqual([i['id'] for i in res['%ss' % resource]], + [i[resource]['id'] for i in items]) @contextlib.contextmanager def network(self, name='net1', @@ -5018,6 +5027,8 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): query_params=query_params) def test_list_subnets_filtering_by_unknown_filter(self): + if self._skip_filter_validation: + self.skipTest("Plugin does not support filter validation") with self.network() as network: with self.subnet(network=network, gateway_ip='10.0.0.1', @@ -5028,11 +5039,13 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): subnets = (v1, v2) query_params = 'admin_state_up=True' self._test_list_resources('subnet', subnets, - query_params=query_params) + query_params=query_params, + expected_code=webob.exc.HTTPClientError.code) # test with other value to check if we have the same results query_params = 'admin_state_up=False' self._test_list_resources('subnet', subnets, - query_params=query_params) + query_params=query_params, + expected_code=webob.exc.HTTPClientError.code) def test_list_subnets_with_parameter(self): with self.network() as network: diff --git a/neutron/tests/unit/test_manager.py b/neutron/tests/unit/test_manager.py index 488e17e5148..ec56ecb2cbc 100644 --- a/neutron/tests/unit/test_manager.py +++ b/neutron/tests/unit/test_manager.py @@ -35,6 +35,7 @@ class MultiServiceCorePlugin(object): class CorePluginWithAgentNotifiers(object): + supported_extension_aliases = [] agent_notifiers = {'l3': 'l3_agent_notifier', 'dhcp': 'dhcp_agent_notifier'} diff --git a/releasenotes/notes/support-filter-validation-fee2cdeedbe8ad76.yaml b/releasenotes/notes/support-filter-validation-fee2cdeedbe8ad76.yaml new file mode 100644 index 00000000000..399d16e7529 --- /dev/null +++ b/releasenotes/notes/support-filter-validation-fee2cdeedbe8ad76.yaml @@ -0,0 +1,38 @@ +--- +prelude: > + Perform validation on filter parameters on listing resources. +features: + - | + Starting from this release, neutron server will perform validation on + filter parameters on list requests. Neutron will return a 400 response + if the request contains invalid filter parameters. + The list of valid parameters is documented in the neutron API reference. + + Add an API extension ``filter-validation`` to indicate this new API + behavior. This extension can be disabled by operators via a config option. +upgrade: + - | + Prior to the upgrade, if a request contains an unknown or unsupported + parameter, the server will silently ignore the invalid input. + After the upgrade, the server will return a 400 Bad Request response + instead. + + API users might observe that requests that received a successful response + now receive a failure response. If they encounter such experience, + they are suggested to confirm if the API extension ``filter-validation`` + is present and validate filter parameters in their requests. + + Operators can disable this feature if they want to maintain + backward-compatibility. If they choose to do that, the API extension + ``filter-validation`` will not be present and the API behavior is + unchanged. +other: + - | + Each plugin can decide if it wants to support filter validation by + setting ``__filter_validation_support`` to True or False. If this field is + not set, the default value is False. + Right now, the ML2 plugin and all the in-tree service plugins support + filter validation. Out-of-tree plugins will have filter validation + disabled by default but they can turn it on if they choose to. + For filter validation to be supported, the core plugin and all the + services plugins in a deployment must support it.