Drop concept of '?validation' parameter
We had planned to introduce a 'validation' parameter to the various extra spec APIs in change If67f0d924ea372746a6dc440ea7bdc655e4f0bea to configure a policy for extra spec validation. Following reviews, we've decided to instead go with a strict policy for all known namespaces and no validation for any unknown namespaces. Update the tests we had in place for this. Part of blueprint flavor-extra-spec-validators Change-Id: If30990ec1c43177b7d13bd8ee1c5dc481265e47b Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane
parent
1b606da3f1
commit
c87b75e008
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -268,13 +268,13 @@ class FlavorsExtraSpecsTestV21(test.TestCase):
|
|||
|
||||
# TODO(stephenfin): Wire the microversion up
|
||||
@unittest.expectedFailure
|
||||
def test_create_invalid_specs_strict(self):
|
||||
"""Test behavior of strict validator."""
|
||||
def test_create_invalid_known_namespace(self):
|
||||
"""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
|
||||
@unittest.expectedFailure
|
||||
def test_create_invalid_specs_permissive(self):
|
||||
"""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 = {
|
||||
def test_create_invalid_unknown_namespace(self):
|
||||
"""Test behavior of validator with specs from unknown namespace."""
|
||||
unknown_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',
|
||||
use_admin_context=True, version='2.82',
|
||||
'1/os-extra_specs', 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):
|
||||
"""Test behavior of strict validator."""
|
||||
def test_update_invalid_specs_known_namespace(self):
|
||||
"""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
|
||||
@unittest.expectedFailure
|
||||
def test_update_invalid_specs_permissive(self):
|
||||
"""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 = {
|
||||
def test_update_invalid_specs_unknown_namespace(self):
|
||||
"""Test behavior of validator with specs from unknown namespace."""
|
||||
unknown_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 = {
|
||||
|
|
|
|||
|
|
@ -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_spec(self, policy):
|
||||
invalid_specs = (
|
||||
('hw:cpu_realtime_maskk', '^0'),
|
||||
def test_namespaces(self):
|
||||
"""Ensure we see at least the in-tree namespaces.
|
||||
|
||||
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:
|
||||
if policy == 'strict':
|
||||
with testtools.ExpectedException(exception.ValidationError):
|
||||
validators.validate(key, value, policy)
|
||||
else:
|
||||
validators.validate(key, value, policy)
|
||||
for key, value in unknown_namespaces:
|
||||
validators.validate(key, value)
|
||||
|
||||
@ddt.data('strict', 'permissive', 'disabled')
|
||||
def test_value__str(self, policy):
|
||||
known_invalid_namespaces = (
|
||||
('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):
|
||||
validators.validate(key, value, policy)
|
||||
else:
|
||||
validators.validate(key, value, policy)
|
||||
with testtools.ExpectedException(exception.ValidationError):
|
||||
validators.validate(key, value)
|
||||
|
||||
@ddt.data('strict', 'permissive', 'disabled')
|
||||
def test_value__int(self, policy):
|
||||
def test_value__int(self):
|
||||
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):
|
||||
validators.validate(key, value, policy)
|
||||
else:
|
||||
validators.validate(key, value, policy)
|
||||
with testtools.ExpectedException(exception.ValidationError):
|
||||
validators.validate(key, value)
|
||||
|
||||
@ddt.data('strict', 'permissive', 'disabled')
|
||||
def test_value__bool(self, policy):
|
||||
def test_value__bool(self):
|
||||
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):
|
||||
validators.validate(key, value, policy)
|
||||
else:
|
||||
validators.validate(key, value, policy)
|
||||
with testtools.ExpectedException(exception.ValidationError):
|
||||
validators.validate(key, value)
|
||||
|
|
|
|||
Loading…
Reference in New Issue