Add any-traits support for listing resource providers

The patch Id908822e8e03b872b204016345fba30b05ff5b1f added support for
any-traits in the DB layer. This patch extends the resource provider
object and API layer to be able to parse the extended 'in:' syntax of
the 'required' query parameter as well as to handle the nested required
trait structure.

This patch refers to microversion 1.39 which has not been added yet so
the changes in this patch cannot be triggered from the REST API. A later
patch will add the microversion bump after the allocation_candidates
code path also gained support for the same query structure.

Story: 2005345
Story: 2005346
Change-Id: I1ef8e31c73ffbc84ecdfed806098ca860c60a396

placement/handlers/resource_provider.py

@@ -239,7 +239,8 @@ def list_resource_providers(req):
             util.normalize_member_of_qs_params(req))
 
     if 'required' in req.GET:
-        filters['required'] = util.normalize_traits_qs_params(req)
+        filters['required_traits'], filters['forbidden_traits'] = (
+            util.normalize_traits_qs_params(req))
 
     qpkeys = ('uuid', 'name', 'in_tree', 'resources')
     for attr in qpkeys:

placement/lib.py

@@ -113,8 +113,11 @@ class RequestGroup(object):
                 request_group.resources = util.normalize_resources_qs_param(
                     val)
             elif prefix == _QS_REQUIRED:
+                # TODO(gibi): switch this to normalize_traits_qs_params when
+                # the data model can handle nested required_traits structure
+                # as part of the any-traits feature
                 request_group.required_traits = (
-                    util.normalize_traits_qs_params(req, suffix))
+                    util.normalize_traits_qs_params_legacy(req, suffix))
             elif prefix == _QS_MEMBER_OF:
                 # special handling of member_of qparam since we allow multiple
                 # member_of params at microversion 1.24.
@@ -392,8 +395,13 @@ class RequestWideParams(object):
                 raise webob.exc.HTTPBadRequest(
                     "Query parameter 'root_required' may be specified only "
                     "once.", comment=errors.ILLEGAL_DUPLICATE_QUERYPARAM)
+            # NOTE(gibi): root_required does not support any-traits so here
+            # we continue using the old query parsing function that does not
+            # accept the `in:` prefix and that always returns a flat trait
+            # list
             anchor_required_traits, anchor_forbidden_traits, conflicts = (
-                _fix_one_forbidden(util.normalize_traits_qs_param(
+                _fix_one_forbidden(
+                    util.normalize_traits_qs_param_to_legacy_value(
                         root_required[0], allow_forbidden=True)))
             if conflicts:
                 raise webob.exc.HTTPBadRequest(

placement/objects/resource_provider.py

@@ -921,7 +921,8 @@ def _get_all_by_filters_from_db(context, filters):
     #          'MEMORY_MB': 1024
     #      },
     #      'in_tree': <uuid>,
-    #      'required': [<trait_name>, ...]
+    #      'required_traits': [{<trait_name>, ...}, {...}]
+    #      'forbidden_traits': {<trait_name>, ...}
     #  }
     if not filters:
         filters = {}
@@ -933,11 +934,8 @@ def _get_all_by_filters_from_db(context, filters):
     uuid = filters.pop('uuid', None)
     member_of = filters.pop('member_of', [])
     forbidden_aggs = filters.pop('forbidden_aggs', [])
-    required = set(filters.pop('required', []))
+    required_traits = filters.pop('required_traits', [])
-    forbidden = set([trait for trait in required
+    forbidden_traits = filters.pop('forbidden_traits', {})
-                     if trait.startswith('!')])
-    required = required - forbidden
-    forbidden = set([trait.lstrip('!') for trait in forbidden])
     resources = filters.pop('resources', {})
     in_tree = filters.pop('in_tree', None)
 
@@ -983,15 +981,25 @@ def _get_all_by_filters_from_db(context, filters):
             return []
         root_id = tree_ids.root_id
         query = query.where(rp.c.root_provider_id == root_id)
-    if required:
+    if required_traits:
-        trait_map = trait_obj.ids_from_names(context, required)
+        # translate trait names to trait internal IDs while keeping the nested
-        trait_rps = res_ctx.provider_ids_matching_required_traits(
+        # structure
-            context, trait_map.values())
+        required_traits = [
-        if not trait_rps:
+            {
+                context.trait_cache.id_from_string(trait)
+                for trait in any_traits
+            }
+            for any_traits in required_traits
+        ]
+
+        rps_with_matching_traits = (
+            res_ctx.provider_ids_matching_required_traits(
+                context, required_traits))
+        if not rps_with_matching_traits:
             return []
-        query = query.where(rp.c.id.in_(trait_rps))
+        query = query.where(rp.c.id.in_(rps_with_matching_traits))
-    if forbidden:
+    if forbidden_traits:
-        trait_map = trait_obj.ids_from_names(context, forbidden)
+        trait_map = trait_obj.ids_from_names(context, forbidden_traits)
         trait_rps = res_ctx.get_provider_ids_having_any_trait(
             context, trait_map.values())
         if trait_rps:
@@ -1027,13 +1035,13 @@ def get_all_by_filters(context, filters=None):
 
     :param context: `placement.context.RequestContext` that may be used to
         grab a DB connection.
-    :param filters: Can be `name`, `uuid`, `member_of`, `in_tree` or
+    :param filters: Can be `name`, `uuid`, `member_of`, `in_tree`,
-                    `resources` where `member_of` is a list of list of
+        `required_traits`, `forbidden_traits`, or `resources` where
-                    aggregate UUIDs, `in_tree` is a UUID of a resource
+        `member_of` is a list of list of aggregate UUIDs, `required_traits` is
-                    provider that we can use to find the root provider ID
+        a list of set of trait names, `forbidden_traits` is a set of trait
-                    of the tree of providers to filter results by and
+        names, `in_tree` is a UUID of a resource provider that we can use to
-                    `resources` is a dict of amounts keyed by resource
+        find the root provider ID of the tree of providers to filter results
-                    classes.
+        by and `resources` is a dict of amounts keyed by resource classes.
     :type filters: dict
     """
     resource_providers = _get_all_by_filters_from_db(context, filters)

placement/tests/functional/db/test_resource_provider.py

@@ -1050,13 +1050,16 @@ class ResourceProviderListTestCase(tb.PlacementDbBaseTestCase):
                 tb.set_traits(rp, *traits)
 
         # Three rps (1, 2, 3) should have CUSTOM_TRAIT_A
-        filters = {'required': ['CUSTOM_TRAIT_A']}
+        filters = {'required_traits': [{'CUSTOM_TRAIT_A'}]}
         expected_rps = ['rp_1', 'rp_2', 'rp_3']
         self._run_get_all_by_filters(expected_rps, filters=filters)
 
         # One rp (rp 1) if we forbid CUSTOM_TRAIT_B, with a single trait of
         # CUSTOM_TRAIT_A
-        filters = {'required': ['CUSTOM_TRAIT_A', '!CUSTOM_TRAIT_B']}
+        filters = {
+            'required_traits': [{'CUSTOM_TRAIT_A'}],
+            'forbidden_traits': {'CUSTOM_TRAIT_B'},
+        }
         expected_rps = ['rp_1']
         custom_a_rps = self._run_get_all_by_filters(expected_rps,
                                                     filters=filters)
@@ -1067,6 +1070,22 @@ class ResourceProviderListTestCase(tb.PlacementDbBaseTestCase):
         self.assertEqual(1, len(traits))
         self.assertEqual('CUSTOM_TRAIT_A', traits[0].name)
 
+        # (A or B) and not C
+        filters = {
+            'required_traits': [{'CUSTOM_TRAIT_A', 'CUSTOM_TRAIT_B'}],
+            'forbidden_traits': {'CUSTOM_TRAIT_C'},
+        }
+        expected_rps = ['rp_1', 'rp_2']
+        self._run_get_all_by_filters(expected_rps, filters=filters)
+
+        # A and (B or C)
+        filters = {
+            'required_traits': [
+                {'CUSTOM_TRAIT_A'}, {'CUSTOM_TRAIT_B', 'CUSTOM_TRAIT_C'}],
+        }
+        expected_rps = ['rp_2', 'rp_3']
+        self._run_get_all_by_filters(expected_rps, filters=filters)
+
 
 class TestResourceProviderAggregates(tb.PlacementDbBaseTestCase):
     def test_set_and_get_new_aggregates(self):

placement/tests/functional/gabbits/allocation-candidates-any-traits.yaml

@@ -15,7 +15,7 @@ tests:
       openstack-api-version: placement 1.38
   status: 400
   response_strings:
-      - "No such trait(s): in:CUSTOM_FOO"
+      - "The format 'in:HW_CPU_X86_VMX,CUSTOM_MAGIC' only supported since microversion 1.39."
 
 - name: the 'in:' trait query is not supported yet in named request group
   GET: /allocation_candidates?requiredX=in:CUSTOM_FOO,HW_CPU_X86_MMX&resourcesX=VCPU:1
@@ -23,7 +23,7 @@ tests:
       openstack-api-version: placement 1.38
   status: 400
   response_strings:
-      - "No such trait(s): in:CUSTOM_FOO"
+      - "The format 'in:HW_CPU_X86_VMX,CUSTOM_MAGIC' only supported since microversion 1.39."
 
 - name: the second required field overwrites the first
   # The fixture has one RP for each trait but no RP for both traits.

placement/tests/functional/gabbits/resource-provider-any-traits.yaml

@@ -15,7 +15,7 @@ tests:
       openstack-api-version: placement 1.38
   status: 400
   response_strings:
-      - "No such trait(s): in:CUSTOM_FOO"
+      - "The format 'in:HW_CPU_X86_VMX,CUSTOM_MAGIC' only supported since microversion 1.39."
 
 - name: the second required field overwrites the first
   # The fixture has one RP for each trait but no RP for both traits.

placement/tests/unit/test_util.py

@@ -395,14 +395,16 @@ class TestNormalizeResourceQsParam(testtools.TestCase):
         )
 
 
-class TestNormalizeTraitsQsParam(testtools.TestCase):
+class TestNormalizeTraitsQsParamLegacy(testtools.TestCase):
 
     def test_one(self):
         trait = 'HW_CPU_X86_VMX'
         # Various whitespace permutations
         for fmt in ('%s', ' %s', '%s ', ' %s ', '  %s  '):
-            self.assertEqual(set([trait]),
+            self.assertEqual(
-                             util.normalize_traits_qs_param(fmt % trait))
+                set([trait]),
+                util.normalize_traits_qs_param_to_legacy_value(fmt % trait)
+            )
 
     def test_multiple(self):
         traits = (
@@ -414,12 +416,15 @@ class TestNormalizeTraitsQsParam(testtools.TestCase):
         )
         self.assertEqual(
             set(traits),
-            util.normalize_traits_qs_param('%s, %s,%s , %s ,  %s  ' % traits))
+            util.normalize_traits_qs_param_to_legacy_value(
+                '%s, %s,%s , %s ,  %s  ' % traits)
+        )
 
     def test_400_all_empty(self):
         for qs in ('', ' ', '   ', ',', ' , , '):
             self.assertRaises(
-                webob.exc.HTTPBadRequest, util.normalize_traits_qs_param, qs)
+                webob.exc.HTTPBadRequest,
+                util.normalize_traits_qs_param_to_legacy_value, qs)
 
     def test_400_some_empty(self):
         traits = (
@@ -428,8 +433,107 @@ class TestNormalizeTraitsQsParam(testtools.TestCase):
             'STORAGE_DISK_SSD',
         )
         for fmt in ('%s,,%s,%s', ',%s,%s,%s', '%s,%s,%s,', ' %s , %s ,  , %s'):
-            self.assertRaises(webob.exc.HTTPBadRequest,
+            self.assertRaises(
-                              util.normalize_traits_qs_param, fmt % traits)
+                webob.exc.HTTPBadRequest,
+                util.normalize_traits_qs_param_to_legacy_value, fmt % traits)
+
+
+class TestNormalizeTraitsQsParam(testtools.TestCase):
+
+    def test_one(self):
+        trait = 'HW_CPU_X86_VMX'
+        # Various whitespace permutations
+        for fmt in ('%s', ' %s', '%s ', ' %s ', '  %s  '):
+            self.assertEqual(
+                ([{trait}], set()),
+                util.normalize_traits_qs_param(fmt % trait)
+            )
+
+    def test_multiple(self):
+        traits = (
+            'HW_CPU_X86_VMX',
+            'HW_GPU_API_DIRECT3D_V12_0',
+            'HW_NIC_OFFLOAD_RX',
+            'CUSTOM_GOLD',
+            'STORAGE_DISK_SSD',
+        )
+        self.assertEqual(
+            ([{trait} for trait in traits], set()),
+            util.normalize_traits_qs_param(
+                '%s, %s,%s , %s ,  %s  ' % traits)
+        )
+
+    def test_400_all_empty(self):
+        for qs in ('', ' ', '   ', ',', ' , , '):
+            self.assertRaises(
+                webob.exc.HTTPBadRequest,
+                util.normalize_traits_qs_param, qs)
+
+    def test_400_some_empty(self):
+        traits = (
+            'HW_NIC_OFFLOAD_RX',
+            'CUSTOM_GOLD',
+            'STORAGE_DISK_SSD',
+        )
+        for fmt in (
+            '%s,,%s,%s',
+            ',%s,%s,%s',
+            '%s,%s,%s,',
+            ' %s , %s ,  , %s',
+            '!,%s,%s,%s',
+        ):
+            self.assertRaises(
+                webob.exc.HTTPBadRequest,
+                util.normalize_traits_qs_param,
+                fmt % traits,
+                allow_forbidden=True,
+            )
+
+    def test_multiple_with_forbidden(self):
+        traits = (
+            '!HW_CPU_X86_VMX',
+            'HW_GPU_API_DIRECT3D_V12_0',
+            '!HW_NIC_OFFLOAD_RX',
+            'CUSTOM_GOLD',
+            '!STORAGE_DISK_SSD',
+        )
+
+        self.assertRaises(
+            webob.exc.HTTPBadRequest,
+            util.normalize_traits_qs_param,
+            '%s, %s,%s , %s ,  %s  ' % traits,
+            allow_forbidden=False)
+
+        self.assertEqual(
+            (
+                [{'HW_GPU_API_DIRECT3D_V12_0'}, {'CUSTOM_GOLD'}],
+                {'HW_CPU_X86_VMX', 'HW_NIC_OFFLOAD_RX', 'STORAGE_DISK_SSD'}),
+            util.normalize_traits_qs_param(
+                '%s, %s,%s , %s ,  %s  ' % traits, allow_forbidden=True)
+        )
+
+    def test_any_traits(self):
+        param = 'in:T1 ,T2 , T3'
+        self.assertRaises(
+            webob.exc.HTTPBadRequest,
+            util.normalize_traits_qs_param,
+            param,
+            allow_any_traits=False
+        )
+        self.assertEqual(
+            ([{'T1', 'T2', 'T3'}], set()),
+            util.normalize_traits_qs_param(param, allow_any_traits=True)
+        )
+
+    def test_any_traits_not_mix_with_forbidden(self):
+        param = 'in:T1 ,!T2 , T3'
+        self.assertRaises(
+            webob.exc.HTTPBadRequest,
+            util.normalize_traits_qs_param,
+            param,
+            allow_forbidden=True,
+            allow_any_traits=True,
+        )
 
 
 class TestNormalizeTraitsQsParams(testtools.TestCase):
@@ -451,13 +555,15 @@ class TestNormalizeTraitsQsParams(testtools.TestCase):
     def test_suffix(self):
         req = self._get_req('required=!BAZ&requiredX=FOO,BAR', (1, 38))
 
-        traits = util.normalize_traits_qs_params(req, suffix='')
+        required, forbidden = util.normalize_traits_qs_params(req, suffix='')
 
-        self.assertEqual({'!BAZ'}, traits)
+        self.assertEqual([], required)
+        self.assertEqual({'BAZ'}, forbidden)
 
-        traits = util.normalize_traits_qs_params(req, suffix='X')
+        required, forbidden = util.normalize_traits_qs_params(req, suffix='X')
 
-        self.assertEqual({'FOO', 'BAR'}, traits)
+        self.assertEqual([{'FOO'}, {'BAR'}], required)
+        self.assertEqual(set(), forbidden)
 
     def test_allow_forbidden_1_21(self):
         req = self._get_req('required=!BAZ', (1, 21))
@@ -478,16 +584,67 @@ class TestNormalizeTraitsQsParams(testtools.TestCase):
     def test_allow_forbidden_1_22(self):
         req = self._get_req('required=!BAZ', (1, 22))
 
-        traits = util.normalize_traits_qs_params(req, suffix='')
+        required, forbidden = util.normalize_traits_qs_params(req, suffix='')
 
-        self.assertEqual({'!BAZ'}, traits)
+        self.assertEqual([], required)
+        self.assertEqual({'BAZ'}, forbidden)
 
     def test_repeated_param_1_38(self):
         req = self._get_req('required=FOO,!BAR&required=BAZ', (1, 38))
 
-        traits = util.normalize_traits_qs_params(req, suffix='')
+        required, forbidden = util.normalize_traits_qs_params(req, suffix='')
 
-        self.assertEqual({'BAZ'}, traits)
+        self.assertEqual([{'BAZ'}], required)
+        self.assertEqual(set(), forbidden)
+
+    def test_allow_any_traits_1_38(self):
+        req = self._get_req('required=in:FOO,BAZ', (1, 38))
+
+        ex = self.assertRaises(
+            webob.exc.HTTPBadRequest,
+            util.normalize_traits_qs_params,
+            req,
+            suffix='',
+        )
+
+        self.assertIn(
+            "Invalid query string parameters: "
+            "The format 'in:HW_CPU_X86_VMX,CUSTOM_MAGIC' only supported "
+            "since microversion 1.39. Got: in:FOO,BAZ",
+            str(ex),
+        )
+
+    # TODO(gibi): remove the mock when microversion 1.39 is fully added
+    @mock.patch(
+        'placement.microversion.max_version_string',
+        new=mock.Mock(return_value='1.39'))
+    def test_allow_any_traits_1_39(self):
+        req = self._get_req('required=in:FOO,BAZ', (1, 39))
+
+        required, forbidden = util.normalize_traits_qs_params(req, suffix='')
+
+        self.assertEqual([{'FOO', 'BAZ'}], required)
+        self.assertEqual(set(), forbidden)
+
+    # TODO(gibi): remove the mock when microversion 1.39 is fully added
+    @mock.patch(
+        'placement.microversion.max_version_string',
+        new=mock.Mock(return_value='1.39'))
+    def test_repeated_param_1_39(self):
+        req = self._get_req(
+            'required=in:T1,T2'
+            '&required=T3,!T4'
+            '&required=in:T5,T6'
+            '&required=!T7,T8',
+            (1, 39)
+        )
+
+        required, forbidden = util.normalize_traits_qs_params(req, suffix='')
+
+        self.assertEqual(
+            [{'T1', 'T2'}, {'T3'}, {'T5', 'T6'}, {'T8'}],
+            required)
+        self.assertEqual({'T4', 'T7'}, forbidden)
 
 
 class TestParseQsRequestGroups(testtools.TestCase):

placement/util.py

@@ -12,6 +12,7 @@
 """Utility methods for placement API."""
 
 import functools
+import itertools
 
 import jsonschema
 from oslo_log import log as logging
@@ -295,18 +296,9 @@ def normalize_resources_qs_param(qs):
     return result
 
 
-def valid_trait(trait, allow_forbidden):
+def normalize_traits_qs_param_to_legacy_value(val, allow_forbidden=False):
-    """Return True if the provided trait is the expected form.
+    """Parse a traits query string parameter value into the legacy return
-
+    format.
-    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,
     which may currently be of the form `required`, `required123`, etc., but
@@ -315,29 +307,38 @@ def normalize_traits_qs_param(val, allow_forbidden=False):
     This method currently does no format validation of trait strings, other
     than to ensure they're not zero-length.
 
+    This method only accepts query parameter value without 'in:' prefix support
+
     :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.
+    :return: A set of trait names or trait names prefixed with '!'
     :raises `webob.exc.HTTPBadRequest` if the val parameter is not in the
             expected format.
     """
-    ret = set(substr.strip() for substr in val.split(','))
+    # let's parse the query string to the new internal format
-    expected_form = 'HW_CPU_X86_VMX,CUSTOM_MAGIC'
+    required, forbidden = normalize_traits_qs_param(val, allow_forbidden)
-    if allow_forbidden:
+
-        expected_form = 'HW_CPU_X86_VMX,!CUSTOM_MAGIC'
+    # then reformat that structure to the old format
-    if not all(trait and valid_trait(trait, allow_forbidden) for trait in ret):
+    legacy_traits = set()
-        msg = ("Invalid query string parameters: Expected 'required' "
+    for any_traits in required:
-               "parameter value of the form: %(form)s. "
+        # a legacy request does not have any-trait support so every internal
-               "Got: %(val)s") % {'form': expected_form, 'val': val}
+        # set expressing OR relationship should exactly contain one trait
-        raise webob.exc.HTTPBadRequest(msg)
+        assert len(any_traits) == 1
-    return ret
+        legacy_traits.add(list(any_traits)[0])
+
+    for forbidden_trait in forbidden:
+        legacy_traits.add('!' + forbidden_trait)
+
+    return legacy_traits
 
 
-def normalize_traits_qs_params(req, suffix=''):
+# TODO(gibi): remove this once the allocation candidate code path also
+# supports nested required_traits structure.
+def normalize_traits_qs_params_legacy(req, suffix=''):
     """Given a webob.Request object, validate and collect required querystring
     parameters.
 
@@ -359,11 +360,150 @@ def normalize_traits_qs_params(req, suffix=''):
     # NOTE(gibi): This means if the same query param is repeated then only
     # the last one will be considered
     for value in req.GET.getall('required' + suffix):
-        traits = normalize_traits_qs_param(value, allow_forbidden)
+        traits = normalize_traits_qs_param_to_legacy_value(
-
+            value, allow_forbidden)
     return traits
 
 
+def normalize_traits_qs_param(
+    val, allow_forbidden=False, allow_any_traits=False
+):
+    """Parse a traits query string parameter value.
+
+    Note that this method doesn't know or care about the query parameter key,
+    which may currently be of the form `required`, `required123`, etc., but
+    which may someday also include `preferred`, etc.
+
+    :param val: A traits query parameter value: either a comma-separated string
+        of trait names including trait names with ! prefix, or a string with
+        'in:' prefix and of comma-separated list of trait names. The 'in:'
+        prefixed string does not support trait names with ! prefix
+    :param allow_forbidden:
+        If True, accept forbidden traits (that is, traits prefixed by '!') as a
+        valid form.
+    :param allow_any_traits: if True, accept the 'in:' prefixed format.
+    :return: a two tuple where:
+        The first item is a list of set of traits. Each set of traits
+        represents a set of required traits in an OR relationship, while
+        different sets in the list represent required traits in an AND
+        relationship.
+        The second item is a set of forbidden traits.
+    :raises `webob.exc.HTTPBadRequest` if the val parameter is not in the
+            expected format.
+    """
+    if val.startswith('in:'):
+        if not allow_any_traits:
+            msg = (
+                f"Invalid query string parameters: "
+                f"The format 'in:HW_CPU_X86_VMX,CUSTOM_MAGIC' only supported "
+                f"since microversion 1.39. Got: {val}")
+            raise webob.exc.HTTPBadRequest(msg)
+
+        any_traits = set(substr.strip() for substr in val[3:].split(','))
+
+        if not all(trait for trait in any_traits):
+            msg = (
+                f"Invalid query string parameters: Expected 'required' "
+                f"parameter value of the form: "
+                f"in:HW_CPU_X86_VMX,CUSTOM_MAGIC. Got an empty trait in: "
+                f"{val}")
+            raise webob.exc.HTTPBadRequest(msg)
+
+        if any(trait.startswith('!') for trait in any_traits):
+            msg = (
+                f"Invalid query string parameters: "
+                f"The format 'in:HW_CPU_X86_VMX,CUSTOM_MAGIC' does not "
+                f"support forbidden traits. Got: {val}")
+            raise webob.exc.HTTPBadRequest(msg)
+
+        # the in: prefix means all the traits are in a single OR relationship
+        # so we return [{every trait after the in: prefix}]
+        return [any_traits], set()
+    else:
+        all_traits = [substr.strip() for substr in val.split(',')]
+
+        # NOTE(gibi): lstrip will remove any number of consecutive '!'
+        # characters from the beginning of the trait name. This means !!!!!FOO
+        # is parsed as FOO. This is not a documented behavior of the API but
+        # this is a bug that decided not to be fixed outside a microversion
+        # bump. See
+        # https://review.opendev.org/c/openstack/placement/+/826491/7/placement/util.py#426
+        forbidden_traits = {
+            trait.lstrip('!') for trait in all_traits if trait.startswith('!')}
+
+        if not all(
+                trait
+                for trait in itertools.chain(forbidden_traits, all_traits)
+        ):
+            expected_form = 'HW_CPU_X86_VMX,!CUSTOM_MAGIC'
+            if not allow_forbidden:
+                expected_form = 'HW_CPU_X86_VMX,CUSTOM_MAGIC'
+            msg = (
+                f"Invalid query string parameters: Expected 'required' "
+                f"parameter value of the form: {expected_form}. "
+                f"Got an empty trait in: {val}")
+            raise webob.exc.HTTPBadRequest(msg)
+
+        # NOTE(gibi): we need to wrap each required trait into a one element
+        # set of traits to keep the format of [{}, {}...] where each set of
+        # traits represent OR relationship
+        required_traits = [
+            {trait} for trait in all_traits if not trait.startswith('!')]
+
+        if forbidden_traits and not allow_forbidden:
+            msg = (
+                f"Invalid query string parameters: Expected 'required' "
+                f"parameter value of the form: HW_CPU_X86_VMX,CUSTOM_MAGIC. "
+                f"Got: {val}")
+            raise webob.exc.HTTPBadRequest(msg)
+
+        return required_traits, forbidden_traits
+
+
+def normalize_traits_qs_params(req, suffix=''):
+    """Given a webob.Request object, validate and collect required querystring
+    parameters.
+
+    We begin supporting forbidden traits in microversion 1.22.
+    We begin supporting any-traits and repeating the required param in
+    microversion 1.39.
+
+    :param req: a webob.Request object to read the params from
+    :param suffix: the string suffix of the request group to read from the
+        request. If empty then the unnamed request group is processed.
+    :returns: a two tuple where:
+        The first item is a list of set of traits. Each set of traits
+        represents a set of required traits in an OR relationship, while
+        different sets in the list represent required traits in an AND
+        relationship.
+        The second item is a set of forbidden traits.
+    :raises webob.exc.HTTPBadRequest: if the format of the query param is not
+        valid
+    """
+    want_version = req.environ[placement.microversion.MICROVERSION_ENVIRON]
+    allow_forbidden = want_version.matches((1, 22))
+    allow_any_traits = want_version.matches((1, 39))
+
+    required_traits = []
+    forbidden_traits = set()
+
+    values = req.GET.getall('required' + suffix)
+
+    if not allow_any_traits:
+        # to keep the behavior of <= 1.38 we need to make sure that if
+        # the query param is repeated we only consider the last one from the
+        # request
+        values = values[-1:]
+
+    for value in values:
+        rts, fts = normalize_traits_qs_param(
+            value, allow_forbidden, allow_any_traits)
+        required_traits += rts
+        forbidden_traits |= fts
+
+    return required_traits, forbidden_traits
+
+
 def normalize_member_of_qs_params(req, suffix=''):
     """Given a webob.Request object, validate that the member_of querystring
     parameters are correct. We begin supporting multiple member_of params in