Handle invalid stack names which are non-string

If we get passed a non-string stack name, e.g a map or list, we
fail with a DB error associated with looking up the existing stack.

So instead force all stack lookups to use string identifiers, and
make the name validation for new stacks robust to fail gracefully
when there is an invalid (non string) argument passed.

Change-Id: I052dc4a715773895d070e1e9f26183c6a1cf3d7f
Closes-Bug: #1533065

heat/engine/stack.py

@@ -134,10 +134,14 @@ class Stack(collections.Mapping):
         """
 
         def _validate_stack_name(name):
-            if not re.match("[a-zA-Z][a-zA-Z0-9_.-]*$", name):
-                message = _('Invalid stack name %s must contain '
-                            'only alphanumeric or \"_-.\" characters, '
-                            'must start with alpha') % name
+            try:
+                if not re.match("[a-zA-Z][a-zA-Z0-9_.-]*$", name):
+                    message = _('Invalid stack name %s must contain '
+                                'only alphanumeric or \"_-.\" characters, '
+                                'must start with alpha') % name
+                    raise exception.StackValidationFailed(message=message)
+            except TypeError:
+                message = _('Invalid stack name %s, must be a string') % name
                 raise exception.StackValidationFailed(message=message)
 
         if owner_id is None:

heat/objects/stack.py

@@ -15,6 +15,8 @@
 
 """Stack object."""
 
+import six
+
 from oslo_versionedobjects import base
 from oslo_versionedobjects import fields
 
@@ -95,7 +97,7 @@ class Stack(
     def get_by_name_and_owner_id(cls, context, stack_name, owner_id):
         db_stack = db_api.stack_get_by_name_and_owner_id(
             context,
-            stack_name,
+            six.text_type(stack_name),
             owner_id
         )
         if not db_stack:
@@ -105,7 +107,7 @@ class Stack(
 
     @classmethod
     def get_by_name(cls, context, stack_name):
-        db_stack = db_api.stack_get_by_name(context, stack_name)
+        db_stack = db_api.stack_get_by_name(context, six.text_type(stack_name))
         if not db_stack:
             return None
         stack = cls._from_db_object(context, cls(context), db_stack)

heat/tests/test_engine_service.py

@@ -469,6 +469,18 @@ class StackServiceTest(common.HeatTestCase):
             ctx2,
             self.stack.name))
 
+    @tools.stack_context('service_badname_test_stack', False)
+    def test_stack_by_name_badname(self):
+        # If a bad name type, such as a map, is passed, we should just return
+        # None, as it's converted to a string, which won't match any name
+        ctx = utils.dummy_context(tenant_id='stack_service_test_tenant')
+        self.assertIsNone(stack_object.Stack.get_by_name(
+            ctx,
+            {'notallowed': self.stack.name}))
+        self.assertIsNone(stack_object.Stack.get_by_name_and_owner_id(
+            ctx,
+            {'notallowed': self.stack.name}, 'owner'))
+
     @tools.stack_context('service_list_all_test_stack')
     def test_stack_list_all(self):
         self.m.StubOutWithMock(parser.Stack, '_from_db')

heat/tests/test_stack.py

@@ -1077,8 +1077,20 @@ class StackTest(common.HeatTestCase):
                        'test/stack', 'test\stack', 'test::stack', 'test;stack',
                        'test~stack', '#test']
         for stack_name in stack_names:
-            self.assertRaises(exception.StackValidationFailed, stack.Stack,
-                              self.ctx, stack_name, self.tmpl)
+            ex = self.assertRaises(
+                exception.StackValidationFailed, stack.Stack,
+                self.ctx, stack_name, self.tmpl)
+            self.assertIn("Invalid stack name %s must contain" % stack_name,
+                          six.text_type(ex))
+
+    def test_stack_name_invalid_type(self):
+        stack_names = [{"bad": 123}, ["no", "lists"]]
+        for stack_name in stack_names:
+            ex = self.assertRaises(
+                exception.StackValidationFailed, stack.Stack,
+                self.ctx, stack_name, self.tmpl)
+            self.assertIn("Invalid stack name %s, must be a string"
+                          % stack_name, six.text_type(ex))
 
     def test_resource_state_get_att(self):
         tmpl = {