diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 7fc17b3e029d..cc97ca96cb32 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -1305,7 +1305,8 @@ aggregate_az_optional: description: | The availability zone of the host aggregate. You should use a custom availability zone rather than the default returned by the - os-availability-zone API. + os-availability-zone API. The availability zone must not include ':' + in its name. in: body required: false type: string diff --git a/doc/source/user/aggregates.rst b/doc/source/user/aggregates.rst index b58a73068112..4b9db6e73134 100644 --- a/doc/source/user/aggregates.rst +++ b/doc/source/user/aggregates.rst @@ -65,6 +65,9 @@ between aggregates and availability zones: moved to another aggregate or when the user would like to migrate the instance. +.. note:: Availablity zone name must NOT contain ':' since it is used by admin + users to specify hosts where instances are launched in server creation. + See :doc:`Select hosts where instances are launched ` for more detail. Xen Pool Host Aggregates ------------------------ diff --git a/nova/api/openstack/compute/schemas/aggregates.py b/nova/api/openstack/compute/schemas/aggregates.py index 5283429e78ed..396e4fe31c08 100644 --- a/nova/api/openstack/compute/schemas/aggregates.py +++ b/nova/api/openstack/compute/schemas/aggregates.py @@ -16,9 +16,9 @@ import copy from nova.api.validation import parameter_types -availability_zone = {'oneOf': [parameter_types.name, {'type': 'null'}]} +availability_zone = {'oneOf': [parameter_types.az_name, {'type': 'null'}]} availability_zone_with_leading_trailing_spaces = { - 'oneOf': [parameter_types.name_with_leading_trailing_spaces, + 'oneOf': [parameter_types.az_name_with_leading_trailing_spaces, {'type': 'null'}] } diff --git a/nova/api/validation/parameter_types.py b/nova/api/validation/parameter_types.py index d87d2d8269c1..462ea88af117 100644 --- a/nova/api/validation/parameter_types.py +++ b/nova/api/validation/parameter_types.py @@ -159,6 +159,24 @@ valid_name_leading_trailing_spaces_regex_base = ( "^[%(ws)s]*[%(no_ws)s][%(no_ws)s%(ws)s]+[%(no_ws)s][%(ws)s]*$") +valid_az_name_regex = ValidationRegex( + valid_name_regex_base % ( + _build_regex_range(ws=False, invert=True), + _build_regex_range(exclude=[':']), + _build_regex_range(ws=False, invert=True)), + _("printable characters except :." + "Can not start or end with whitespace.")) + + +# az's name disallow ':'. +valid_az_name_leading_trailing_spaces_regex = ValidationRegex( + valid_name_leading_trailing_spaces_regex_base % { + 'ws': _build_regex_range(exclude=[':']), + 'no_ws': _build_regex_range(ws=False, exclude=[':'])}, + _("printable characters except :, " + "with at least one non space character")) + + valid_cell_name_regex = ValidationRegex( valid_name_regex_base % ( _build_regex_range(ws=False, invert=True), @@ -254,6 +272,18 @@ name = { } +az_name = { + 'type': 'string', 'minLength': 1, 'maxLength': 255, + 'format': 'az_name' +} + + +az_name_with_leading_trailing_spaces = { + 'type': 'string', 'minLength': 1, 'maxLength': 255, + 'format': 'az_name_with_leading_trailing_spaces' +} + + cell_name = { 'type': 'string', 'minLength': 1, 'maxLength': 255, 'format': 'cell_name' diff --git a/nova/api/validation/validators.py b/nova/api/validation/validators.py index bfe98b1eeb75..de3225a0507e 100644 --- a/nova/api/validation/validators.py +++ b/nova/api/validation/validators.py @@ -120,6 +120,33 @@ def _validate_name(instance): raise exception.InvalidName(reason=regex.reason) +@jsonschema.FormatChecker.cls_checks('az_name_with_leading_trailing_spaces', + exception.InvalidName) +def _validate_az_name_with_leading_trailing_spaces(instance): + regex = parameter_types.valid_az_name_leading_trailing_spaces_regex + try: + if re.search(regex.regex, instance): + return True + except TypeError: + # The name must be string type. If instance isn't string type, the + # TypeError will be raised at here. + pass + raise exception.InvalidName(reason=regex.reason) + + +@jsonschema.FormatChecker.cls_checks('az_name', exception.InvalidName) +def _validate_az_name(instance): + regex = parameter_types.valid_az_name_regex + try: + if re.search(regex.regex, instance): + return True + except TypeError: + # The name must be string type. If instance isn't string type, the + # TypeError will be raised at here. + pass + raise exception.InvalidName(reason=regex.reason) + + @jsonschema.FormatChecker.cls_checks('cell_name_with_leading_trailing_spaces', exception.InvalidName) def _validate_cell_name_with_leading_trailing_spaces(instance): diff --git a/nova/tests/unit/api/openstack/compute/test_aggregates.py b/nova/tests/unit/api/openstack/compute/test_aggregates.py index e09bfc6d97b9..ce06f9788546 100644 --- a/nova/tests/unit/api/openstack/compute/test_aggregates.py +++ b/nova/tests/unit/api/openstack/compute/test_aggregates.py @@ -232,6 +232,12 @@ class AggregateTestCaseV21(test.NoDBTestCase): {"name": "test", "availability_zone": "x" * 256}}) + def test_create_with_availability_zone_invalid(self): + self.assertRaises(self.bad_request, self.controller.create, + self.req, body={"aggregate": + {"name": "test", + "availability_zone": "bad:az"}}) + def test_create_availability_zone_with_leading_trailing_spaces(self): self.assertRaises(self.bad_request, self.controller.create, self.req, body={"aggregate": @@ -372,6 +378,11 @@ class AggregateTestCaseV21(test.NoDBTestCase): self.assertRaises(self.bad_request, self.controller.update, self.req, "2", body=test_metadata) + def test_update_with_availability_zone_invalid(self): + test_metadata = {"aggregate": {"availability_zone": "bad:az"}} + self.assertRaises(self.bad_request, self.controller.update, + self.req, "2", body=test_metadata) + def test_update_with_empty_availability_zone(self): test_metadata = {"aggregate": {"availability_zone": ""}} self.assertRaises(self.bad_request, self.controller.update, @@ -482,6 +493,10 @@ class AggregateTestCaseV21(test.NoDBTestCase): self.assertRaises(self.bad_request, eval(self.add_host), self.req, "1", body={"add_host": {"host": "a" * 300}}) + def test_add_host_with_invalid_name_host(self): + self.assertRaises(self.bad_request, eval(self.add_host), + self.req, "1", body={"add_host": {"host": "bad:host"}}) + def test_add_host_with_multiple_hosts(self): self.assertRaises(self.bad_request, eval(self.add_host), self.req, "1", body={"add_host": {"host": ["host1", "host2"]}}) diff --git a/releasenotes/notes/bug-1695861-ebc8a0aa7a87f7e0.yaml b/releasenotes/notes/bug-1695861-ebc8a0aa7a87f7e0.yaml new file mode 100644 index 000000000000..57303608aa21 --- /dev/null +++ b/releasenotes/notes/bug-1695861-ebc8a0aa7a87f7e0.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixes `bug 1695861`_ in which the aggregate API accepted requests that + have availability zone names including ':'. With this fix, a creation + of an availabilty zone whose name includes ':' results in a + ``400 BadRequest`` error response. + + .. _bug 1695861: https://bugs.launchpad.net/nova/+bug/1695861