openstack
/
nova
Code Issues Proposed changes

Merge "[placement] Parse forbidden traits in query strings"

This commit is contained in:
Zuul 2018-04-14 00:47:50 +00:00 committed by Gerrit Code Review
parent 5c2242be03 8bde4042da
commit 930e089b80
4 changed files with 162 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
nova/api/openstack/placement/lib.py
View File

@ -18,7 +18,7 @@ common library that both placement and its consumers can require."""
class RequestGroup(object):
def __init__(self, use_same_provider=True, resources=None,
required_traits=None, member_of=None):
required_traits=None, forbidden_traits=None, member_of=None):
"""Create a grouping of resource and trait requests.
:param use_same_provider:
@ -28,9 +28,11 @@ class RequestGroup(object):
in any resource provider in the same tree, or a sharing provider.
:param resources: A dict of { resource_class: amount, ... }
:param required_traits: A set of { trait_name, ... }
:param forbidden_traits: A set of { trait_name, ... }
:param member_of: A list of [ aggregate_UUID, ... ]
"""
self.use_same_provider = use_same_provider
self.resources = resources or {}
self.required_traits = required_traits or set()
self.forbidden_traits = forbidden_traits or set()
self.member_of = member_of or []

62
nova/api/openstack/placement/util.py
View File

@ -292,7 +292,17 @@ def normalize_resources_qs_param(qs):
return result
def normalize_traits_qs_param(val):
def valid_trait(trait, allow_forbidden):
"""Return True if the provided trait is the expected form.
When allow_forbidden is True, then a leading '!' is acceptable.
"""
if trait.startswith('!') and not allow_forbidden:
return False
return True
def normalize_traits_qs_param(val, allow_forbidden=False):
"""Parse a traits query string parameter value.
Note that this method doesn't know or care about the query parameter key,
@ -304,15 +314,22 @@ def normalize_traits_qs_param(val):
:param val: A traits query parameter value: a comma-separated string of
trait names.
:param allow_forbidden: If True, accept forbidden traits (that is, traits
prefixed by '!') as a valid form when notifying
the caller that the provided value is not properly
formed.
:return: A set of trait names.
:raises `webob.exc.HTTPBadRequest` if the val parameter is not in the
expected format.
"""
ret = set(substr.strip() for substr in val.split(','))
if not all(trait for trait in ret):
expected_form = 'HW_CPU_X86_VMX,CUSTOM_MAGIC'
if allow_forbidden:
expected_form = 'HW_CPU_X86_VMX,!CUSTOM_MAGIC'
if not all(trait and valid_trait(trait, allow_forbidden) for trait in ret):
msg = _("Invalid query string parameters: Expected 'required' "
"parameter value of the form: HW_CPU_X86_VMX,CUSTOM_MAGIC. "
"Got: %s") % val
"parameter value of the form: %(form)s. "
"Got: %(val)s") % {'form': expected_form, 'val': val}
raise webob.exc.HTTPBadRequest(msg)
return ret
@ -346,7 +363,7 @@ def normalize_member_of_qs_param(val):
return ret
def parse_qs_request_groups(qsdict):
def parse_qs_request_groups(qsdict, allow_forbidden=False):
"""Parse numbered resources, traits, and member_of groupings out of a
querystring dict.
@ -365,6 +382,12 @@ def parse_qs_request_groups(qsdict):
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 list of these RequestGroup instances.
As an example, if qsdict represents the query string:
@ -375,7 +398,7 @@ def parse_qs_request_groups(qsdict):
&resources1=SRIOV_NET_VF:2
&required1=CUSTOM_PHYSNET_PUBLIC,CUSTOM_SWITCH_A
&resources2=SRIOV_NET_VF:1
&required2=CUSTOM_PHYSNET_PRIVATE
&required2=!CUSTOM_PHYSNET_PUBLIC
...the return value will be:
@ -410,13 +433,14 @@ def parse_qs_request_groups(qsdict):
resources={
"SRIOV_NET_VF": 1,
},
required_traits=[
"CUSTOM_PHYSNET_PRIVATE",
forbidden_traits=[
"CUSTOM_PHYSNET_PUBLIC",
],
),
]
:param qsdict: The MultiDict representing the querystring on a GET.
:param allow_forbidden: If True, parse for forbidden traits.
: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.
@ -441,7 +465,8 @@ def parse_qs_request_groups(qsdict):
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)
request_group.required_traits = normalize_traits_qs_param(
val, allow_forbidden=allow_forbidden)
elif prefix == _QS_MEMBER_OF:
request_group.member_of = normalize_member_of_qs_param(val)
@ -460,6 +485,25 @@ def parse_qs_request_groups(qsdict):
' values: %s')
raise webob.exc.HTTPBadRequest(msg % ', '.join(orphans))
# 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))
# NOTE(efried): The sorting is not necessary for the API, but it makes
# testing easier.
return [by_suffix[suff] for suff in sorted(by_suffix)]

8
nova/tests/functional/api/openstack/placement/gabbits/allocation-candidates.yaml
View File

@ -213,6 +213,14 @@ tests:
response_strings:
- "Invalid query string parameters: Expected 'required' parameter value of the form: HW_CPU_X86_VMX,CUSTOM_MAGIC."
- name: get allocation candidates with forbidden trait pre-forbidden
GET: /allocation_candidates?resources=VCPU:1,MEMORY_MB:1024,DISK_GB:100&required=!CUSTOM_MAGIC
status: 400
request_headers:
openstack-api-version: placement 1.17
response_strings:
- "Invalid query string parameters: Expected 'required' parameter value of the form: HW_CPU_X86_VMX,CUSTOM_MAGIC."
- name: get allocation candidates with required trait
GET: /allocation_candidates?resources=VCPU:1,MEMORY_MB:1024,DISK_GB:100&required=HW_CPU_X86_SSE
status: 200

100
nova/tests/unit/api/openstack/placement/test_util.py
View File

@ -430,12 +430,12 @@ class TestNormalizeTraitsQsParam(test.NoDBTestCase):
class TestParseQsResourcesAndTraits(test.NoDBTestCase):
@staticmethod
def do_parse(qstring):
def do_parse(qstring, allow_forbidden=False):
"""Converts a querystring to a MultiDict, mimicking request.GET, and
runs parse_qs_request_groups on it.
"""
return util.parse_qs_request_groups(webob.multidict.MultiDict(
urlparse.parse_qsl(qstring)))
urlparse.parse_qsl(qstring)), allow_forbidden=allow_forbidden)
def assertRequestGroupsEqual(self, expected, observed):
self.assertEqual(len(expected), len(observed))
@ -617,6 +617,102 @@ class TestParseQsResourcesAndTraits(test.NoDBTestCase):
'&resources3=CUSTOM_MAGIC:123')
self.assertRaises(webob.exc.HTTPBadRequest, self.do_parse, qs)
def test_forbidden_one_group(self):
"""When forbidden are allowed this will parse, but otherwise will
indicate an invalid trait.
"""
qs = ('resources=VCPU:2,MEMORY_MB:2048'
'&required=CUSTOM_PHYSNET1,!CUSTOM_SWITCH_BIG')
expected_forbidden = [
pl.RequestGroup(
use_same_provider=False,
resources={
'VCPU': 2,
'MEMORY_MB': 2048,
},
required_traits={
'CUSTOM_PHYSNET1',
},
forbidden_traits={
'CUSTOM_SWITCH_BIG',
}
),
]
expected_message = (
"Invalid query string parameters: Expected 'required' parameter "
"value of the form: HW_CPU_X86_VMX,CUSTOM_MAGIC. Got: "
"CUSTOM_PHYSNET1,!CUSTOM_SWITCH_BIG")
exc = self.assertRaises(webob.exc.HTTPBadRequest, self.do_parse, qs)
self.assertEqual(expected_message, six.text_type(exc))
self.assertRequestGroupsEqual(
expected_forbidden, self.do_parse(qs, allow_forbidden=True))
def test_forbidden_conflict(self):
qs = ('resources=VCPU:2,MEMORY_MB:2048'
'&required=CUSTOM_PHYSNET1,!CUSTOM_PHYSNET1')
expected_message = (
'Conflicting required and forbidden traits found '
'in the following traits keys: required: (CUSTOM_PHYSNET1)')
exc = self.assertRaises(webob.exc.HTTPBadRequest, self.do_parse, qs,
allow_forbidden=True)
self.assertEqual(expected_message, six.text_type(exc))
def test_forbidden_two_groups(self):
qs = ('resources=VCPU:2,MEMORY_MB:2048&resources1=CUSTOM_MAGIC:1'
'&required1=CUSTOM_PHYSNET1,!CUSTOM_PHYSNET2')
expected = [
pl.RequestGroup(
use_same_provider=False,
resources={
'VCPU': 2,
'MEMORY_MB': 2048,
},
),
pl.RequestGroup(
resources={
'CUSTOM_MAGIC': 1,
},
required_traits={
'CUSTOM_PHYSNET1',
},
forbidden_traits={
'CUSTOM_PHYSNET2',
}
),
]
self.assertRequestGroupsEqual(
expected, self.do_parse(qs, allow_forbidden=True))
def test_forbidden_separate_groups_no_conflict(self):
qs = ('resources1=CUSTOM_MAGIC:1&required1=CUSTOM_PHYSNET1'
'&resources2=CUSTOM_MAGIC:1&required2=!CUSTOM_PHYSNET1')
expected = [
pl.RequestGroup(
use_same_provider=True,
resources={
'CUSTOM_MAGIC': 1,
},
required_traits={
'CUSTOM_PHYSNET1',
}
),
pl.RequestGroup(
use_same_provider=True,
resources={
'CUSTOM_MAGIC': 1,
},
forbidden_traits={
'CUSTOM_PHYSNET1',
}
),
]
self.assertRequestGroupsEqual(
expected, self.do_parse(qs, allow_forbidden=True))
class TestPickLastModified(test.NoDBTestCase):