diff --git a/keystone/common/validation/__init__.py b/keystone/common/validation/__init__.py index 8284bac666..0311de5dad 100644 --- a/keystone/common/validation/__init__.py +++ b/keystone/common/validation/__init__.py @@ -40,6 +40,14 @@ def nullable(property_schema): # do that yet so I'm not wasting time on it new_schema = property_schema.copy() new_schema['type'] = [property_schema['type'], 'null'] + # NOTE(kmalloc): If enum is specified (such as our boolean case) ensure we + # add null to the enum as well so that null can be passed/validated as + # expected. Without adding to the enum, null will not validate as enum is + # explicitly listing valid values. According to the JSON Schema + # specification, the values must be unique in the enum array. + if 'enum' in new_schema and None not in new_schema['enum']: + # In the enum the 'null' is NoneType + new_schema['enum'].append(None) return new_schema diff --git a/keystone/tests/unit/test_validation.py b/keystone/tests/unit/test_validation.py index a40bf527f9..485c0ba1c2 100644 --- a/keystone/tests/unit/test_validation.py +++ b/keystone/tests/unit/test_validation.py @@ -11,6 +11,7 @@ # License for the specific language governing permissions and limitations # under the License. +import copy import uuid from keystone.application_credential import schema as app_cred_schema @@ -109,6 +110,43 @@ _INVALID_FILTERS = ['some string', 1, 0, True, False] _INVALID_NAMES = [True, 24, ' ', ''] +class CommonValidationTestCase(unit.BaseTestCase): + + def test_nullable_type_only(self): + bool_without_enum = copy.deepcopy(parameter_types.boolean) + bool_without_enum.pop('enum') + schema_type_only = { + 'type': 'object', + 'properties': {'test': validation.nullable(bool_without_enum)}, + 'additionalProperties': False, + 'required': ['test']} + + # Null should be in the types + self.assertIn('null', schema_type_only['properties']['test']['type']) + # No Enum, and nullable should not have added it. + self.assertNotIn('enum', schema_type_only['properties']['test'].keys()) + validator = validators.SchemaValidator(schema_type_only) + reqs_to_validate = [{'test': val} for val in [True, False, None]] + for req in reqs_to_validate: + validator.validate(req) + + def test_nullable_with_enum(self): + schema_with_enum = { + 'type': 'object', + 'properties': { + 'test': validation.nullable(parameter_types.boolean)}, + 'additionalProperties': False, + 'required': ['test']} + + # Null should be in enum and type + self.assertIn('null', schema_with_enum['properties']['test']['type']) + self.assertIn(None, schema_with_enum['properties']['test']['enum']) + validator = validators.SchemaValidator(schema_with_enum) + reqs_to_validate = [{'test': val} for val in [True, False, None]] + for req in reqs_to_validate: + validator.validate(req) + + class EntityValidationTestCase(unit.BaseTestCase): def setUp(self): @@ -1945,9 +1983,7 @@ class UserValidationTestCase(unit.BaseTestCase): ro.IGNORE_CHANGE_PASSWORD_OPT.option_name: None } } - self.assertRaises(exception.SchemaValidationError, - self.create_user_validator.validate, - request_to_validate) + self.create_user_validator.validate(request_to_validate) def test_user_update_with_options_change_password_required(self): request_to_validate = { diff --git a/releasenotes/notes/bug-1763824-3d2f5169af9d42f.yaml b/releasenotes/notes/bug-1763824-3d2f5169af9d42f.yaml new file mode 100644 index 0000000000..7fb8be5cfd --- /dev/null +++ b/releasenotes/notes/bug-1763824-3d2f5169af9d42f.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + [`bug 1763824 `_] + JSON Schema implementation ``nullable`` in keystone.common.validation now + properly adds ``None`` to the enum if the enum exists.