diff --git a/placement/handlers/allocation_candidate.py b/placement/handlers/allocation_candidate.py index ed12bc934..86e68d608 100644 --- a/placement/handlers/allocation_candidate.py +++ b/placement/handlers/allocation_candidate.py @@ -22,6 +22,7 @@ import webob from placement import exception from placement.i18n import _ +from placement import lib from placement import microversion from placement.objects import resource_provider as rp_obj from placement.policies import allocation_candidate as policies @@ -291,7 +292,7 @@ def list_allocation_candidates(req): get_schema = schema.GET_SCHEMA_1_16 util.validate_query_params(req, get_schema) - requests = util.parse_qs_request_groups(req) + requests = lib.RequestGroup.dict_from_request(req) limit = req.GET.getall('limit') # JSONschema has already confirmed that limit has the form # of an integer. diff --git a/placement/lib.py b/placement/lib.py index 58c230db7..92d1c628a 100644 --- a/placement/lib.py +++ b/placement/lib.py @@ -14,6 +14,22 @@ """Symbols intended to be imported by both placement code and placement API consumers. When placement is separated out, this module should be part of a common library that both placement and its consumers can require.""" +import re + +import webob + +from placement.i18n import _ +from placement import microversion +from placement import util + + +# Querystring-related constants +_QS_RESOURCES = 'resources' +_QS_REQUIRED = 'required' +_QS_MEMBER_OF = 'member_of' +_QS_KEY_PATTERN = re.compile( + r"^(%s)([1-9][0-9]*)?$" % '|'.join( + (_QS_RESOURCES, _QS_REQUIRED, _QS_MEMBER_OF))) class RequestGroup(object): @@ -51,3 +67,177 @@ class RequestGroup(object): for agglist in sorted(self.member_of))) ret += ')' return ret + + @staticmethod + def dict_from_request(req): + """Parse numbered resources, traits, and member_of groupings out of a + querystring dict found in a webob Request. + + The input req contains a query string of the form: + + ?resources=$RESOURCE_CLASS_NAME:$AMOUNT,$RESOURCE_CLASS_NAME:$AMOUNT + &required=$TRAIT_NAME,$TRAIT_NAME&member_of=in:$AGG1_UUID,$AGG2_UUID + &resources1=$RESOURCE_CLASS_NAME:$AMOUNT,RESOURCE_CLASS_NAME:$AMOUNT + &required1=$TRAIT_NAME,$TRAIT_NAME&member_of1=$AGG_UUID + &resources2=$RESOURCE_CLASS_NAME:$AMOUNT,RESOURCE_CLASS_NAME:$AMOUNT + &required2=$TRAIT_NAME,$TRAIT_NAME&member_of2=$AGG_UUID + + These are parsed in groups according to the numeric suffix of the key. + For each group, a RequestGroup instance is created containing that + group's resources, required traits, and member_of. For the (single) + group with no suffix, the RequestGroup.use_same_provider attribute is + False; for the numbered groups it is True. + + If a trait in the required parameter is prefixed with ``!`` this + indicates that that trait must not be present on the resource + providers in the group. That is, the trait is forbidden. Forbidden + traits are only processed if ``allow_forbidden`` is True. This allows + the caller to control processing based on microversion handling. + + The return is a dict, keyed by the numeric suffix of these RequestGroup + instances (or the empty string for the unnumbered group). + + As an example, if qsdict represents the query string: + + ?resources=VCPU:2,MEMORY_MB:1024,DISK_GB=50 + &required=HW_CPU_X86_VMX,CUSTOM_STORAGE_RAID + &member_of=in:9323b2b1-82c9-4e91-bdff-e95e808ef954,8592a199-7d73-4465-8df6-ab00a6243c82 # noqa + &resources1=SRIOV_NET_VF:2 + &required1=CUSTOM_PHYSNET_PUBLIC,CUSTOM_SWITCH_A + &resources2=SRIOV_NET_VF:1 + &required2=!CUSTOM_PHYSNET_PUBLIC + + ...the return value will be: + + { '': RequestGroup( + use_same_provider=False, + resources={ + "VCPU": 2, + "MEMORY_MB": 1024, + "DISK_GB" 50, + }, + required_traits=[ + "HW_CPU_X86_VMX", + "CUSTOM_STORAGE_RAID", + ], + member_of=[ + [9323b2b1-82c9-4e91-bdff-e95e808ef954], + [8592a199-7d73-4465-8df6-ab00a6243c82, + ddbd9226-d6a6-475e-a85f-0609914dd058], + ], + ), + '1': RequestGroup( + use_same_provider=True, + resources={ + "SRIOV_NET_VF": 2, + }, + required_traits=[ + "CUSTOM_PHYSNET_PUBLIC", + "CUSTOM_SWITCH_A", + ], + ), + '2': RequestGroup( + use_same_provider=True, + resources={ + "SRIOV_NET_VF": 1, + }, + forbidden_traits=[ + "CUSTOM_PHYSNET_PUBLIC", + ], + ), + } + + :param req: webob.Request object + :return: A dict, keyed by suffix, of RequestGroup instances. + :raises `webob.exc.HTTPBadRequest` if any value is malformed, or if a + trait list is given without corresponding resources. + """ + want_version = req.environ[microversion.MICROVERSION_ENVIRON] + # Control whether we handle forbidden traits. + allow_forbidden = want_version.matches((1, 22)) + # dict of the form: { suffix: RequestGroup } to be returned + by_suffix = {} + + def get_request_group(suffix): + if suffix not in by_suffix: + rq_grp = RequestGroup(use_same_provider=bool(suffix)) + by_suffix[suffix] = rq_grp + return by_suffix[suffix] + + for key, val in req.GET.items(): + match = _QS_KEY_PATTERN.match(key) + if not match: + continue + # `prefix` is 'resources', 'required', or 'member_of' + # `suffix` is an integer string, or None + prefix, suffix = match.groups() + suffix = suffix or '' + request_group = get_request_group(suffix) + if prefix == _QS_RESOURCES: + request_group.resources = util.normalize_resources_qs_param( + val) + elif prefix == _QS_REQUIRED: + request_group.required_traits = util.normalize_traits_qs_param( + val, allow_forbidden=allow_forbidden) + elif prefix == _QS_MEMBER_OF: + # special handling of member_of qparam since we allow multiple + # member_of params at microversion 1.24. + # NOTE(jaypipes): Yes, this is inefficient to do this when + # there are multiple member_of query parameters, but we do this + # so we can error out if someone passes an "orphaned" member_of + # request group. + # TODO(jaypipes): Do validation of query parameters using + # JSONSchema + request_group.member_of = util.normalize_member_of_qs_params( + req, suffix) + + # Ensure any group with 'required' or 'member_of' also has 'resources'. + orphans = [('required%s' % suff) for suff, group in by_suffix.items() + if group.required_traits and not group.resources] + if orphans: + msg = _( + 'All traits parameters must be associated with resources. ' + 'Found the following orphaned traits keys: %s') + raise webob.exc.HTTPBadRequest(msg % ', '.join(orphans)) + orphans = [('member_of%s' % suff) for suff, group in by_suffix.items() + if group.member_of and not group.resources] + if orphans: + msg = _('All member_of parameters must be associated with ' + 'resources. Found the following orphaned member_of ' + 'keys: %s') + raise webob.exc.HTTPBadRequest(msg % ', '.join(orphans)) + # All request groups must have resources (which is almost, but not + # quite, verified by the orphan checks above). + if not all(grp.resources for grp in by_suffix.values()): + msg = _("All request groups must specify resources.") + raise webob.exc.HTTPBadRequest(msg) + # The above would still pass if there were no request groups + if not by_suffix: + msg = _( + "At least one request group (`resources` or `resources{N}`) " + "is required.") + raise webob.exc.HTTPBadRequest(msg) + + # Make adjustments for forbidden traits by stripping forbidden out + # of required. + if allow_forbidden: + conflicting_traits = [] + for suff, group in by_suffix.items(): + forbidden = [trait for trait in group.required_traits + if trait.startswith('!')] + group.required_traits = ( + group.required_traits - set(forbidden)) + group.forbidden_traits = set([trait.lstrip('!') for trait in + forbidden]) + conflicts = group.forbidden_traits & group.required_traits + if conflicts: + conflicting_traits.append('required%s: (%s)' + % (suff, ', '.join(conflicts))) + if conflicting_traits: + msg = _( + 'Conflicting required and forbidden traits found in the ' + 'following traits keys: %s') + raise webob.exc.HTTPBadRequest( + msg % ', '.join(conflicting_traits)) + + return by_suffix diff --git a/placement/tests/unit/test_util.py b/placement/tests/unit/test_util.py index e983db1c0..c75b28733 100644 --- a/placement/tests/unit/test_util.py +++ b/placement/tests/unit/test_util.py @@ -439,7 +439,7 @@ class TestParseQsRequestGroups(testtools.TestCase): @staticmethod def do_parse(qstring, version=(1, 18)): """Converts a querystring to a MultiDict, mimicking request.GET, and - runs parse_qs_request_groups on it. + runs dict_from_request on it. """ req = webob.Request.blank('?' + qstring) mv_parsed = microversion_parse.Version(*version) @@ -448,7 +448,7 @@ class TestParseQsRequestGroups(testtools.TestCase): mv_parsed.min_version = microversion_parse.parse_version_string( microversion.min_version_string()) req.environ['placement.microversion'] = mv_parsed - d = util.parse_qs_request_groups(req) + d = pl.RequestGroup.dict_from_request(req) # Sort for easier testing return [d[suff] for suff in sorted(d)] diff --git a/placement/util.py b/placement/util.py index a06a8b35a..83e5d3bd1 100644 --- a/placement/util.py +++ b/placement/util.py @@ -13,7 +13,6 @@ import contextlib import functools -import re import shutil import tempfile @@ -29,7 +28,6 @@ import webob from placement import errors from placement import exception from placement.i18n import _ -from placement import lib as placement_lib # NOTE(cdent): avoid cyclical import conflict between util and # microversion import placement.microversion @@ -44,14 +42,6 @@ LOG = logging.getLogger(__name__) ENV_ERROR_CODE = 'placement.error_code' ERROR_CODE_MICROVERSION = (1, 23) -# Querystring-related constants -_QS_RESOURCES = 'resources' -_QS_REQUIRED = 'required' -_QS_MEMBER_OF = 'member_of' -_QS_KEY_PATTERN = re.compile( - r"^(%s)([1-9][0-9]*)?$" % '|'.join( - (_QS_RESOURCES, _QS_REQUIRED, _QS_MEMBER_OF))) - # NOTE(cdent): This registers a FormatChecker on the jsonschema # module. Do not delete this code! Although it appears that nothing @@ -412,174 +402,6 @@ def normalize_member_of_qs_param(value): return value -def parse_qs_request_groups(req): - """Parse numbered resources, traits, and member_of groupings out of a - querystring dict. - - The input qsdict represents a query string of the form: - - ?resources=$RESOURCE_CLASS_NAME:$AMOUNT,$RESOURCE_CLASS_NAME:$AMOUNT - &required=$TRAIT_NAME,$TRAIT_NAME&member_of=in:$AGG1_UUID,$AGG2_UUID - &resources1=$RESOURCE_CLASS_NAME:$AMOUNT,RESOURCE_CLASS_NAME:$AMOUNT - &required1=$TRAIT_NAME,$TRAIT_NAME&member_of1=$AGG_UUID - &resources2=$RESOURCE_CLASS_NAME:$AMOUNT,RESOURCE_CLASS_NAME:$AMOUNT - &required2=$TRAIT_NAME,$TRAIT_NAME&member_of2=$AGG_UUID - - These are parsed in groups according to the numeric suffix of the key. - For each group, a RequestGroup instance is created containing that group's - resources, required traits, and member_of. For the (single) group with no - suffix, the RequestGroup.use_same_provider attribute is False; for the - numbered groups it is True. - - If a trait in the required parameter is prefixed with ``!`` this - indicates that that trait must not be present on the resource - providers in the group. That is, the trait is forbidden. Forbidden traits - are only processed if ``allow_forbidden`` is True. This allows the - caller to control processing based on microversion handling. - - The return is a dict, keyed by the numeric suffix of these RequestGroup - instances (or the empty string for the unnumbered group). - - As an example, if qsdict represents the query string: - - ?resources=VCPU:2,MEMORY_MB:1024,DISK_GB=50 - &required=HW_CPU_X86_VMX,CUSTOM_STORAGE_RAID - &member_of=in:9323b2b1-82c9-4e91-bdff-e95e808ef954,8592a199-7d73-4465-8df6-ab00a6243c82 # noqa - &resources1=SRIOV_NET_VF:2 - &required1=CUSTOM_PHYSNET_PUBLIC,CUSTOM_SWITCH_A - &resources2=SRIOV_NET_VF:1 - &required2=!CUSTOM_PHYSNET_PUBLIC - - ...the return value will be: - - { '': RequestGroup( - use_same_provider=False, - resources={ - "VCPU": 2, - "MEMORY_MB": 1024, - "DISK_GB" 50, - }, - required_traits=[ - "HW_CPU_X86_VMX", - "CUSTOM_STORAGE_RAID", - ], - member_of=[ - [9323b2b1-82c9-4e91-bdff-e95e808ef954], - [8592a199-7d73-4465-8df6-ab00a6243c82, - ddbd9226-d6a6-475e-a85f-0609914dd058], - ], - ), - '1': RequestGroup( - use_same_provider=True, - resources={ - "SRIOV_NET_VF": 2, - }, - required_traits=[ - "CUSTOM_PHYSNET_PUBLIC", - "CUSTOM_SWITCH_A", - ], - ), - '2': RequestGroup( - use_same_provider=True, - resources={ - "SRIOV_NET_VF": 1, - }, - forbidden_traits=[ - "CUSTOM_PHYSNET_PUBLIC", - ], - ), - } - - :param req: webob.Request object - :return: A list of RequestGroup instances. - :raises `webob.exc.HTTPBadRequest` if any value is malformed, or if a - trait list is given without corresponding resources. - """ - want_version = req.environ[placement.microversion.MICROVERSION_ENVIRON] - # Control whether we handle forbidden traits. - allow_forbidden = want_version.matches((1, 22)) - # Temporary dict of the form: { suffix: RequestGroup } - by_suffix = {} - - def get_request_group(suffix): - if suffix not in by_suffix: - rq_grp = placement_lib.RequestGroup(use_same_provider=bool(suffix)) - by_suffix[suffix] = rq_grp - return by_suffix[suffix] - - for key, val in req.GET.items(): - match = _QS_KEY_PATTERN.match(key) - if not match: - continue - # `prefix` is 'resources', 'required', or 'member_of' - # `suffix` is an integer string, or None - prefix, suffix = match.groups() - suffix = suffix or '' - request_group = get_request_group(suffix) - if prefix == _QS_RESOURCES: - request_group.resources = normalize_resources_qs_param(val) - elif prefix == _QS_REQUIRED: - request_group.required_traits = normalize_traits_qs_param( - val, allow_forbidden=allow_forbidden) - elif prefix == _QS_MEMBER_OF: - # special handling of member_of qparam since we allow multiple - # member_of params at microversion 1.24. - # NOTE(jaypipes): Yes, this is inefficient to do this when there - # are multiple member_of query parameters, but we do this so we can - # error out if someone passes an "orphaned" member_of request - # group. - # TODO(jaypipes): Do validation of query parameters using - # JSONSchema - request_group.member_of = normalize_member_of_qs_params( - req, suffix) - - # Ensure any group with 'required' or 'member_of' also has 'resources'. - orphans = [('required%s' % suff) for suff, group in by_suffix.items() - if group.required_traits and not group.resources] - if orphans: - msg = _('All traits parameters must be associated with resources. ' - 'Found the following orphaned traits keys: %s') - raise webob.exc.HTTPBadRequest(msg % ', '.join(orphans)) - orphans = [('member_of%s' % suff) for suff, group in by_suffix.items() - if group.member_of and not group.resources] - if orphans: - msg = _('All member_of parameters must be associated with ' - 'resources. Found the following orphaned member_of ' - 'keys: %s') - raise webob.exc.HTTPBadRequest(msg % ', '.join(orphans)) - # All request groups must have resources (which is almost, but not quite, - # verified by the orphan checks above). - if not all(grp.resources for grp in by_suffix.values()): - msg = _("All request groups must specify resources.") - raise webob.exc.HTTPBadRequest(msg) - # The above would still pass if there were no request groups - if not by_suffix: - msg = _("At least one request group (`resources` or `resources{N}`) " - "is required.") - raise webob.exc.HTTPBadRequest(msg) - - # Make adjustments for forbidden traits by stripping forbidden out - # of required. - if allow_forbidden: - conflicting_traits = [] - for suff, group in by_suffix.items(): - forbidden = [trait for trait in group.required_traits - if trait.startswith('!')] - group.required_traits = (group.required_traits - set(forbidden)) - group.forbidden_traits = set([trait.lstrip('!') for trait in - forbidden]) - conflicts = group.forbidden_traits & group.required_traits - if conflicts: - conflicting_traits.append('required%s: (%s)' - % (suff, ', '.join(conflicts))) - if conflicting_traits: - msg = _('Conflicting required and forbidden traits found in the ' - 'following traits keys: %s') - raise webob.exc.HTTPBadRequest(msg % ', '.join(conflicting_traits)) - - return by_suffix - - def ensure_consumer(ctx, consumer_uuid, project_id, user_id, consumer_generation, want_version): """Ensures there are records in the consumers, projects and users table for