From a33634e5558b20e4bd496fe476f6ceb1a2ba79f6 Mon Sep 17 00:00:00 2001 From: Tetsuro Nakamura Date: Fri, 4 Aug 2017 11:29:00 +0900 Subject: [PATCH] fix nova accepting invalid availability zone name with ':' Nova has a legacy hack to allow admins to specify hosts via an availability zone using az:host:node. That means ':' cannot be included in the name of an availability zone itself. However, the aggregate API accepts requests which have availability zone names including ':'. This patch checks the availabilty zone name when aggregate is created or updated and raises an error if it contains ':'. Conflicts: api-ref/source/parameters.yaml NOTE(mriedem): The conflict in the api-ref docs is due to not having change f657efcdc59e6b80f5e96beb7f9fdc59d8aadbec in Pike. Change-Id: I9b0d8e8d4b3ab2cb3d578c22fa259e0e7c0d325b Closes-Bug: #1695861 (cherry picked from commit 38b25397e805dcf7a995666049713304fe4f1af1) --- api-ref/source/parameters.yaml | 3 +- doc/source/user/aggregates.rst | 3 ++ .../openstack/compute/schemas/aggregates.py | 4 +-- nova/api/validation/parameter_types.py | 30 +++++++++++++++++++ nova/api/validation/validators.py | 27 +++++++++++++++++ .../api/openstack/compute/test_aggregates.py | 15 ++++++++++ .../notes/bug-1695861-ebc8a0aa7a87f7e0.yaml | 9 ++++++ 7 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/bug-1695861-ebc8a0aa7a87f7e0.yaml diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 36bc57e7a337..67b9f122154a 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -1302,7 +1302,8 @@ aggregate_az: type: string aggregate_az_optional: description: | - The availability zone of the host aggregate. + The availability zone of the host aggregate. 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 0527c74e4e8e..3e5eecf0ed58 100644 --- a/doc/source/user/aggregates.rst +++ b/doc/source/user/aggregates.rst @@ -55,6 +55,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 15935cbd8265..138d2de1798e 100644 --- a/nova/api/validation/parameter_types.py +++ b/nova/api/validation/parameter_types.py @@ -150,6 +150,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), @@ -245,6 +263,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