Merge "Drop concept of '?validation' parameter"

nova/api/openstack/compute/flavors_extraspecs.py

@@ -37,7 +37,6 @@ class FlavorExtraSpecsController(wsgi.Controller):
     def _check_extra_specs_value(self, req, specs):
         # TODO(stephenfin): Wire this up to check the API microversion
         validation_supported = False
-        validation_mode = 'strict'
 
         for name, value in specs.items():
             # NOTE(gmann): Max length for numeric value is being checked
@@ -53,7 +52,7 @@ class FlavorExtraSpecsController(wsgi.Controller):
                               explanation=error.format_message())
 
             if validation_supported:
-                validators.validate(name, value, validation_mode)
+                validators.validate(name, value)
 
     @wsgi.expected_errors(404)
     def index(self, req, flavor_id):

nova/api/validation/extra_specs/validators.py

@@ -26,19 +26,16 @@ from nova import exception
 LOG = logging.getLogger(__name__)
 
 VALIDATORS: ty.Dict[str, base.ExtraSpecValidator] = {}
+NAMESPACES: ty.Set[str] = set()
 
 
-def validate(name: str, value: str, mode: str):
+def validate(name: str, value: str):
     """Validate a given extra spec.
 
     :param name: Extra spec name.
     :param value: Extra spec value.
-    :param mode: Validation mode; one of: strict, permissive, disabled
     :raises: exception.ValidationError if validation fails.
     """
-    if mode == 'disabled':
-        return
-
     # attempt a basic lookup for extra specs without embedded parameters
     if name in VALIDATORS:
         VALIDATORS[name].validate(name, value)
@@ -50,7 +47,8 @@ def validate(name: str, value: str, mode: str):
             validator.validate(name, value)
             return
 
-    if mode == 'permissive':  # unregistered extra spec, ignore
+    namespace = name.split(':', 1)[0].split('{')[0] if ':' in name else None
+    if not namespace or namespace not in NAMESPACES:  # unregistered namespace
         return
 
     raise exception.ValidationError(
@@ -74,6 +72,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])
 
 
 load_validators()

nova/tests/unit/api/openstack/compute/test_flavors_extra_specs.py

@@ -268,13 +268,13 @@ class FlavorsExtraSpecsTestV21(test.TestCase):
 
     # TODO(stephenfin): Wire the microversion up
     @unittest.expectedFailure
-    def test_create_invalid_specs_strict(self):
+    def test_create_invalid_known_namespace(self):
-        """Test behavior of strict validator."""
+        """Test behavior of validator with specs from known namespace."""
         invalid_specs = {
             'hw:numa_nodes': 'foo',
             'hw:cpu_policy': 'sharrred',
-            'foo': 'bar',
             'hw:cpu_policyyyyyyy': 'shared',
+            'hw:foo': 'bar',
         }
         for key, value in invalid_specs.items():
             body = {'extra_specs': {key: value}}
@@ -286,68 +286,20 @@ class FlavorsExtraSpecsTestV21(test.TestCase):
             ):
                 self.controller.create(req, 1, body=body)
 
-    # TODO(stephenfin): Wire the microversion up
+    def test_create_invalid_unknown_namespace(self):
-    @unittest.expectedFailure
+        """Test behavior of validator with specs from unknown namespace."""
-    def test_create_invalid_specs_permissive(self):
+        unknown_specs = {
-        """Test behavior of permissive validator."""
-        invalid_specs = {
-            'hw:numa_nodes': 'foo',
-            'hw:cpu_policy': 'sharrred',
-        }
-        for key, value in invalid_specs.items():
-            body = {'extra_specs': {key: value}}
-            req = self._get_request(
-                '1/os-extra_specs?validation=permissive',
-                use_admin_context=True, version='2.82',
-            )
-            with testtools.ExpectedException(
-                self.bad_request, 'Validation failed; .*',
-            ):
-                self.controller.create(req, 1, body=body)
-
-        valid_specs = {
             'foo': 'bar',
-            'hw:cpu_policyyyyyyy': 'shared',
+            'foo:bar': 'baz',
+            'hww:cpu_policy': 'sharrred',
         }
-        for key, value in valid_specs.items():
+        for key, value in unknown_specs.items():
             body = {'extra_specs': {key: value}}
             req = self._get_request(
-                '1/os-extra_specs?validation=permissive',
+                '1/os-extra_specs', use_admin_context=True, version='2.82',
-                use_admin_context=True, version='2.82',
             )
             self.controller.create(req, 1, body=body)
 
-    def test_create_invalid_specs_disabled(self):
-        """Test behavior of permissive validator."""
-        valid_specs = {
-            'hw:numa_nodes': 'foo',
-            'hw:cpu_policy': 'sharrred',
-            'foo': 'bar',
-            'hw:cpu_policyyyyyyy': 'shared',
-        }
-        for key, value in valid_specs.items():
-            body = {'extra_specs': {key: value}}
-            req = self._get_request(
-                '1/os-extra_specs?validation=disabled',
-                use_admin_context=True, version='2.82',
-            )
-            self.controller.create(req, 1, body=body)
-
-    # TODO(stephenfin): Wire the microversion up
-    @unittest.expectedFailure
-    def test_create_invalid_validator_mode(self):
-        """Test behavior with an invalid validator mode."""
-        body = {'extra_specs': {'hw:numa_nodes': '1'}}
-        req = self._get_request(
-            '1/os-extra_specs?validation=foo',
-            use_admin_context=True, version='2.82',
-        )
-        with testtools.ExpectedException(
-            self.bad_request,
-            'Invalid input for query parameters validation.*',
-        ):
-            self.controller.create(req, 1, body=body)
-
     @mock.patch('nova.objects.flavor._flavor_extra_specs_add')
     def test_create_valid_specs(self, mock_flavor_extra_specs):
         valid_specs = {
@@ -453,18 +405,18 @@ class FlavorsExtraSpecsTestV21(test.TestCase):
 
     # TODO(stephenfin): Wire the microversion up
     @unittest.expectedFailure
-    def test_update_invalid_specs_strict(self):
+    def test_update_invalid_specs_known_namespace(self):
-        """Test behavior of strict validator."""
+        """Test behavior of validator with specs from known namespace."""
         invalid_specs = {
             'hw:numa_nodes': 'foo',
             'hw:cpu_policy': 'sharrred',
-            'foo': 'bar',
             'hw:cpu_policyyyyyyy': 'shared',
+            'hw:foo': 'bar',
         }
         for key, value in invalid_specs.items():
             body = {key: value}
             req = self._get_request(
-                '1/os-extra_specs/{key}?validation=strict',
+                '1/os-extra_specs/{key}',
                 use_admin_context=True, version='2.82',
             )
             with testtools.ExpectedException(
@@ -472,69 +424,21 @@ class FlavorsExtraSpecsTestV21(test.TestCase):
             ):
                 self.controller.update(req, 1, key, body=body)
 
-    # TODO(stephenfin): Wire the microversion up
+    def test_update_invalid_specs_unknown_namespace(self):
-    @unittest.expectedFailure
+        """Test behavior of validator with specs from unknown namespace."""
-    def test_update_invalid_specs_permissive(self):
+        unknown_specs = {
-        """Test behavior of permissive validator."""
-        invalid_specs = {
-            'hw:numa_nodes': 'foo',
-            'hw:cpu_policy': 'sharrred',
-        }
-        for key, value in invalid_specs.items():
-            body = {key: value}
-            req = self._get_request(
-                f'1/os-extra_specs/{key}?validation=permissive',
-                use_admin_context=True, version='2.82',
-            )
-            with testtools.ExpectedException(
-                self.bad_request, 'Validation failed; .*',
-            ):
-                self.controller.update(req, 1, key, body=body)
-
-        valid_specs = {
             'foo': 'bar',
-            'hw:cpu_policyyyyyyy': 'shared',
+            'foo:bar': 'baz',
+            'hww:cpu_policy': 'sharrred',
         }
-        for key, value in valid_specs.items():
+        for key, value in unknown_specs.items():
             body = {key: value}
             req = self._get_request(
-                f'1/os-extra_specs/{key}?validation=permissive',
+                f'1/os-extra_specs/{key}',
                 use_admin_context=True, version='2.82',
             )
             self.controller.update(req, 1, key, body=body)
 
-    def test_update_invalid_specs_disabled(self):
-        """Test behavior of permissive validator."""
-        valid_specs = {
-            'hw:numa_nodes': 'foo',
-            'hw:cpu_policy': 'sharrred',
-            'foo': 'bar',
-            'hw:cpu_policyyyyyyy': 'shared',
-        }
-        for key, value in valid_specs.items():
-            body = {key: value}
-            req = self._get_request(
-                f'1/os-extra_specs/{key}?validation=disabled',
-                use_admin_context=True, version='2.82',
-            )
-            self.controller.update(req, 1, key, body=body)
-
-    # TODO(stephenfin): Wire the microversion up
-    @unittest.expectedFailure
-    def test_update_invalid_validator_mode(self):
-        """Test behavior with an invalid validator mode."""
-        key = 'hw:numa_nodes'
-        body = {'hw:numa_nodes': '1'}
-        req = self._get_request(
-            '1/os-extra_specs/{key}?validation=foo',
-            use_admin_context=True, version='2.82',
-        )
-        with testtools.ExpectedException(
-            self.bad_request,
-            'Invalid input for query parameters validation.*',
-        ):
-            self.controller.update(req, 1, key, body=body)
-
     @mock.patch('nova.objects.flavor._flavor_extra_specs_add')
     def test_update_valid_specs(self, mock_flavor_extra_specs):
         valid_specs = {

nova/tests/unit/api/validation/extra_specs/test_validators.py

@@ -12,7 +12,6 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-import ddt
 import testtools
 
 from nova.api.validation.extra_specs import validators
@@ -20,28 +19,43 @@ from nova import exception
 from nova import test
 
 
-@ddt.ddt
 class TestValidators(test.NoDBTestCase):
 
-    @ddt.data('strict', 'permissive', 'disabled')
+    def test_namespaces(self):
-    def test_spec(self, policy):
+        """Ensure we see at least the in-tree namespaces.
-        invalid_specs = (
+
-            ('hw:cpu_realtime_maskk', '^0'),
+        If we add new namespaces, they should be added to this list.
+        """
+        namespaces = {
+            'accel', 'aggregate_instance_extra_specs', 'capabilities', 'hw',
+            'hw_rng', 'hw_video', 'os', 'pci_passthrough', 'powervm', 'quota',
+            'resources', 'trait', 'vmware',
+        }
+        self.assertTrue(
+            namespaces.issubset(validators.NAMESPACES),
+            f'{namespaces} is not a subset of {validators.NAMESPACES}',
+        )
+
+    def test_spec(self):
+        unknown_namespaces = (
             ('hhw:cpu_realtime_mask', '^0'),
             ('w:cpu_realtime_mask', '^0'),
-            ('hw:cpu_realtime_mas', '^0'),
             ('hw_cpu_realtime_mask', '^0'),
             ('foo', 'bar'),
         )
-        for key, value in invalid_specs:
+        for key, value in unknown_namespaces:
-            if policy == 'strict':
+            validators.validate(key, value)
-                with testtools.ExpectedException(exception.ValidationError):
-                    validators.validate(key, value, policy)
-            else:
-                validators.validate(key, value, policy)
 
-    @ddt.data('strict', 'permissive', 'disabled')
+        known_invalid_namespaces = (
-    def test_value__str(self, policy):
+            ('hw:cpu_realtime_maskk', '^0'),
+            ('hw:cpu_realtime_mas', '^0'),
+            ('hw:foo', 'bar'),
+        )
+        for key, value in known_invalid_namespaces:
+            with testtools.ExpectedException(exception.ValidationError):
+                validators.validate(key, value)
+
+    def test_value__str(self):
         valid_specs = (
             # patterns
             ('hw:cpu_realtime_mask', '^0'),
@@ -55,7 +69,7 @@ class TestValidators(test.NoDBTestCase):
             ('hw:pci_numa_affinity_policy', 'legacy'),
         )
         for key, value in valid_specs:
-            validators.validate(key, value, policy)
+            validators.validate(key, value)
 
         invalid_specs = (
             # patterns
@@ -70,14 +84,10 @@ class TestValidators(test.NoDBTestCase):
             ('hw:pci_numa_affinity_policy', 'lgacy'),
         )
         for key, value in invalid_specs:
-            if policy in ('strict', 'permissive'):
+            with testtools.ExpectedException(exception.ValidationError):
-                with testtools.ExpectedException(exception.ValidationError):
+                validators.validate(key, value)
-                    validators.validate(key, value, policy)
-            else:
-                validators.validate(key, value, policy)
 
-    @ddt.data('strict', 'permissive', 'disabled')
+    def test_value__int(self):
-    def test_value__int(self, policy):
         valid_specs = (
             ('hw:numa_nodes', '1'),
             ('os:monitors', '1'),
@@ -86,7 +96,7 @@ class TestValidators(test.NoDBTestCase):
             ('powervm:shared_weight', '255'),
         )
         for key, value in valid_specs:
-            validators.validate(key, value, 'strict')
+            validators.validate(key, value)
 
         invalid_specs = (
             ('hw:serial_port_count', 'five'),  # NaN
@@ -98,14 +108,10 @@ class TestValidators(test.NoDBTestCase):
             ('powervm:shared_weight', '256'),  # has max
         )
         for key, value in invalid_specs:
-            if policy in ('strict', 'permissive'):
+            with testtools.ExpectedException(exception.ValidationError):
-                with testtools.ExpectedException(exception.ValidationError):
+                validators.validate(key, value)
-                    validators.validate(key, value, policy)
-            else:
-                validators.validate(key, value, policy)
 
-    @ddt.data('strict', 'permissive', 'disabled')
+    def test_value__bool(self):
-    def test_value__bool(self, policy):
         valid_specs = (
             ('hw:cpu_realtime', '1'),
             ('hw:cpu_realtime', '0'),
@@ -113,7 +119,7 @@ class TestValidators(test.NoDBTestCase):
             ('hw:boot_menu', 'y'),
         )
         for key, value in valid_specs:
-            validators.validate(key, value, 'strict')
+            validators.validate(key, value)
 
         invalid_specs = (
             ('hw:cpu_realtime', '2'),
@@ -122,8 +128,5 @@ class TestValidators(test.NoDBTestCase):
             ('hw:boot_menu', 'yah'),
         )
         for key, value in invalid_specs:
-            if policy in ('strict', 'permissive'):
+            with testtools.ExpectedException(exception.ValidationError):
-                with testtools.ExpectedException(exception.ValidationError):
+                validators.validate(key, value)
-                    validators.validate(key, value, policy)
-            else:
-                validators.validate(key, value, policy)