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 ':'.

Change-Id: I9b0d8e8d4b3ab2cb3d578c22fa259e0e7c0d325b
Closes-Bug: #1695861
(cherry picked from commit 38b25397e8)
(cherry picked from commit a33634e555)

api-ref/source/parameters.yaml

@@ -934,7 +934,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

doc/source/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 </admin/availability-zones>` for more detail.
 
 Xen Pool Host Aggregates
 ------------------------

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'}]
 }
 

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),
@@ -239,6 +257,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'

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):

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

@@ -246,6 +246,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":
@@ -400,6 +406,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,
@@ -516,6 +527,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"]}})

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