diff --git a/nova/api/openstack/compute/rest_api_version_history.rst b/nova/api/openstack/compute/rest_api_version_history.rst index 5168890b2346..239c7ac5f0a7 100644 --- a/nova/api/openstack/compute/rest_api_version_history.rst +++ b/nova/api/openstack/compute/rest_api_version_history.rst @@ -1130,7 +1130,7 @@ for the following APIs: * ``POST /flavors/{flavor_id}/os-extra_specs`` * ``PUT /flavors/{flavor_id}/os-extra_specs/{id}`` -Validation is only used for recognized extra spec namespaces, namely: +Validation is only used for recognized extra spec namespaces, currently: ``accel``, ``aggregate_instance_extra_specs``, ``capabilities``, ``hw``, ``hw_rng``, ``hw_video``, ``os``, ``pci_passthrough``, ``powervm``, ``quota``, ``resources``, ``trait``, and ``vmware``. diff --git a/nova/api/validation/extra_specs/traits.py b/nova/api/validation/extra_specs/traits.py index 60ae165955f5..7c07dab92837 100644 --- a/nova/api/validation/extra_specs/traits.py +++ b/nova/api/validation/extra_specs/traits.py @@ -38,10 +38,6 @@ for trait in os_traits.get_traits(): 'name': 'group', 'pattern': r'(_[a-zA-z0-9_]*|\d+)?', }, - { - 'name': 'trait', - 'pattern': r'[a-zA-Z0-9_]+', - }, ], ) ) diff --git a/nova/api/validation/extra_specs/validators.py b/nova/api/validation/extra_specs/validators.py index a1e3d9d50e5f..2163892d71df 100644 --- a/nova/api/validation/extra_specs/validators.py +++ b/nova/api/validation/extra_specs/validators.py @@ -47,8 +47,15 @@ def validate(name: str, value: str): validator.validate(name, value) return - namespace = name.split(':', 1)[0].split('{')[0] if ':' in name else None - if not namespace or namespace not in NAMESPACES: # unregistered namespace + # check if there's a namespace; if not, we've done all we can do + if ':' not in name: # no namespace + return + + # if there is, check if it's one we recognize + for namespace in NAMESPACES: + if re.fullmatch(namespace, name.split(':', 1)[0]): + break + else: return raise exception.ValidationError( @@ -72,8 +79,8 @@ def load_validators(): # TODO(stephenfin): Make 'register' return a dict rather than a list? for validator in ext.plugin.register(): VALIDATORS[validator.name] = validator - if ':' in validator.name: - NAMESPACES.add(validator.name.split(':', 1)[0].split('{')[0]) + if ':' in validator.name_regex: + NAMESPACES.add(validator.name_regex.split(':', 1)[0]) load_validators() diff --git a/nova/tests/functional/test_flavor_extraspecs.py b/nova/tests/functional/test_flavor_extraspecs.py index cb8afe9a9db5..d158067e6d50 100644 --- a/nova/tests/functional/test_flavor_extraspecs.py +++ b/nova/tests/functional/test_flavor_extraspecs.py @@ -14,8 +14,6 @@ """Tests for os-extra_specs API.""" -import testtools - from nova.tests.functional.api import client as api_client from nova.tests.functional import integrated_helpers @@ -43,7 +41,7 @@ class FlavorExtraSpecsTest(integrated_helpers._IntegratedTestBase): This should pass because validation is not enabled in this API microversion. """ - body = {'extra_specs': {'hw:numa_nodes': '1', 'foo': 'bar'}} + body = {'extra_specs': {'hw:numa_nodes': 'foo', 'foo': 'bar'}} self.admin_api.post_extra_spec(self.flavor_id, body) self.assertEqual( body['extra_specs'], self.admin_api.get_extra_specs(self.flavor_id) @@ -64,8 +62,8 @@ class FlavorExtraSpecsTest(integrated_helpers._IntegratedTestBase): This should pass because validation is not enabled in this API microversion. """ - spec_id = 'foo:bar' - body = {'foo:bar': 'baz'} + spec_id = 'hw:foo' + body = {'hw:foo': 'bar'} self.admin_api.put_extra_spec(self.flavor_id, spec_id, body) self.assertEqual( body, self.admin_api.get_extra_spec(self.flavor_id, spec_id) @@ -82,11 +80,12 @@ class FlavorExtraSpecsV286Test(FlavorExtraSpecsTest): # this should fail because 'foo' is not a suitable value for # 'hw:numa_nodes' - with testtools.ExpectedException( - api_client.OpenStackApiException - ) as exc: - self.admin_api.post_extra_spec(self.flavor_id, body) - self.assertEqual(400, exc.response.status_code) + exc = self.assertRaises( + api_client.OpenStackApiException, + self.admin_api.post_extra_spec, + self.flavor_id, body, + ) + self.assertEqual(400, exc.response.status_code) # ...and the extra specs should not be saved self.assertEqual({}, self.admin_api.get_extra_specs(self.flavor_id)) @@ -102,11 +101,12 @@ class FlavorExtraSpecsV286Test(FlavorExtraSpecsTest): body = {'extra_specs': {'hw:numa_nodes': '1', 'hw:foo': 'bar'}} # ...but this should fail because we do recognize the namespace - with testtools.ExpectedException( - api_client.OpenStackApiException - ) as exc: - self.admin_api.post_extra_spec(self.flavor_id, body) - self.assertEqual(400, exc.response.status_code) + exc = self.assertRaises( + api_client.OpenStackApiException, + self.admin_api.post_extra_spec, + self.flavor_id, body, + ) + self.assertEqual(400, exc.response.status_code) def test_update_invalid_spec(self): """Test updating extra specs with invalid specs.""" @@ -114,21 +114,23 @@ class FlavorExtraSpecsV286Test(FlavorExtraSpecsTest): body = {'hw:foo': 'bar'} # this should fail because we don't recognize the extra spec - with testtools.ExpectedException( - api_client.OpenStackApiException - ) as exc: - self.admin_api.put_extra_spec(self.flavor_id, spec_id, body) - self.assertEqual(400, exc.response.status_code) + exc = self.assertRaises( + api_client.OpenStackApiException, + self.admin_api.put_extra_spec, + self.flavor_id, spec_id, body, + ) + self.assertEqual(400, exc.response.status_code) spec_id = 'hw:numa_nodes' body = {'hw:numa_nodes': 'foo'} # ...while this should fail because the value is not valid - with testtools.ExpectedException( - api_client.OpenStackApiException - ) as exc: - self.admin_api.put_extra_spec(self.flavor_id, spec_id, body) - self.assertEqual(400, exc.response.status_code) + exc = self.assertRaises( + api_client.OpenStackApiException, + self.admin_api.put_extra_spec, + self.flavor_id, spec_id, body, + ) + self.assertEqual(400, exc.response.status_code) # ...and neither extra spec should be saved self.assertEqual({}, self.admin_api.get_extra_specs(self.flavor_id)) @@ -141,3 +143,4 @@ class FlavorExtraSpecsV286Test(FlavorExtraSpecsTest): # this should pass because we don't recognize the extra spec but it's # not in a namespace we care about self.admin_api.put_extra_spec(self.flavor_id, spec_id, body) + self.assertEqual(body, self.admin_api.get_extra_specs(self.flavor_id)) diff --git a/nova/tests/unit/api/openstack/compute/test_flavors_extra_specs.py b/nova/tests/unit/api/openstack/compute/test_flavors_extra_specs.py index d973e1d08011..fdf2147329e1 100644 --- a/nova/tests/unit/api/openstack/compute/test_flavors_extra_specs.py +++ b/nova/tests/unit/api/openstack/compute/test_flavors_extra_specs.py @@ -272,6 +272,10 @@ class FlavorsExtraSpecsTestV21(test.TestCase): 'hw:cpu_policy': 'sharrred', 'hw:cpu_policyyyyyyy': 'shared', 'hw:foo': 'bar', + 'trait:STORAGE_DISK_SSD': 'forbiden', + 'trait_foo:HW_CPU_X86_AVX2': 'foo', + 'trait:bar': 'required', + 'trait_foo:bar': 'required', } for key, value in invalid_specs.items(): body = {'extra_specs': {key: value}} @@ -303,6 +307,8 @@ class FlavorsExtraSpecsTestV21(test.TestCase): 'hide_hypervisor_id': 'true', 'hw:numa_nodes': '1', 'hw:numa_cpus.0': '0-3,8-9,11,10', + 'trait:STORAGE_DISK_SSD': 'forbidden', + 'trait_foo:HW_CPU_X86_AVX2': 'required', } mock_flavor_extra_specs.side_effect = return_create_flavor_extra_specs diff --git a/nova/tests/unit/api/validation/extra_specs/test_validators.py b/nova/tests/unit/api/validation/extra_specs/test_validators.py index 2ec5a4304017..c45e9e3773f6 100644 --- a/nova/tests/unit/api/validation/extra_specs/test_validators.py +++ b/nova/tests/unit/api/validation/extra_specs/test_validators.py @@ -29,7 +29,8 @@ class TestValidators(test.NoDBTestCase): namespaces = { 'accel', 'aggregate_instance_extra_specs', 'capabilities', 'hw', 'hw_rng', 'hw_video', 'os', 'pci_passthrough', 'powervm', 'quota', - 'resources', 'trait', 'vmware', + 'resources(?P(_[a-zA-z0-9_]*|\\d+)?)', + 'trait(?P(_[a-zA-z0-9_]*|\\d+)?)', 'vmware', } self.assertTrue( namespaces.issubset(validators.NAMESPACES), diff --git a/releasenotes/notes/flavor-extra-spec-validators-76d1f2e52ba753db.yaml b/releasenotes/notes/flavor-extra-spec-validators-76d1f2e52ba753db.yaml index 76c96e755ac2..0b32b43ee9d1 100644 --- a/releasenotes/notes/flavor-extra-spec-validators-76d1f2e52ba753db.yaml +++ b/releasenotes/notes/flavor-extra-spec-validators-76d1f2e52ba753db.yaml @@ -3,4 +3,19 @@ features: - | The 2.86 microversion adds support for flavor extra spec validation when creating or updating flavor extra specs. Use of an unrecognized or invalid - flavor extra spec will result in a HTTP 400 response. + flavor extra spec in the following namespaces will result in a HTTP 400 + response. + + - ``accel`` + - ``aggregate_instance_extra_specs`` + - ``capabilities`` + - ``hw`` + - ``hw_rng`` + - ``hw_video`` + - ``os`` + - ``pci_passthrough`` + - ``powervm`` + - ``quota`` + - ``resources`` (including ``_{group}`` suffixes) + - ``trait`` (including ``_{group}`` suffixes) + - ``vmware``