Check top-level sections when parsing a template

This patch adds validation to template parsing to detect any sections
that do not comply with allowed sections in a template. In the past,
there have been cases where small typos in section names (e.g.
'output' instead of 'outputs') did not get detected and caused issues
at a later point, i.e. by simply not showing any stack outputs.

The place where the template.validate() call has been added to the
parser code only affects new stacks and rejects invalid templates
with an error message. This does not affect stacks that are already
in the database (I did some tests to check this).

Change-Id: Ifed4207badeb364675c905224ba762a9d8eaedd9
Closes-Bug: #1297761

heat/api/aws/exception.py

@@ -278,6 +278,7 @@ def map_remote_error(ex):
             'StackValidationFailed',
             'InvalidTemplateReference',
             'InvalidTemplateVersion',
+            'InvalidTemplateSection',
             'UnknownUserParameter',
             'UserParameterMissing',
             'InvalidTemplateParameter',

heat/api/middleware/fault.py

@@ -73,6 +73,7 @@ class FaultWrapper(wsgi.Middleware):
         'StackValidationFailed': webob.exc.HTTPBadRequest,
         'InvalidTemplateReference': webob.exc.HTTPBadRequest,
         'InvalidTemplateVersion': webob.exc.HTTPBadRequest,
+        'InvalidTemplateSection': webob.exc.HTTPBadRequest,
         'UnknownUserParameter': webob.exc.HTTPBadRequest,
         'RevertFailed': webob.exc.HTTPInternalServerError,
         'StopActionFailed': webob.exc.HTTPInternalServerError,

heat/common/exception.py

@@ -196,6 +196,10 @@ class InvalidTemplateVersion(HeatException):
     msg_fmt = _("The template version is invalid: %(explanation)s")
 
 
+class InvalidTemplateSection(HeatException):
+    msg_fmt = _("The template section is invalid: %(section)s")
+
+
 class InvalidTemplateParameter(HeatException):
     msg_fmt = _("The Parameter (%(key)s) has no attributes.")
 

heat/engine/cfn/template.py

@@ -19,12 +19,13 @@ from heat.engine import template
 class CfnTemplate(template.Template):
     '''A stack template.'''
 
-    SECTIONS = (VERSION, DESCRIPTION, MAPPINGS,
+    SECTIONS = (VERSION, ALTERNATE_VERSION, DESCRIPTION, MAPPINGS,
                 PARAMETERS, RESOURCES, OUTPUTS) = \
-               ('AWSTemplateFormatVersion', 'Description', 'Mappings',
+               ('AWSTemplateFormatVersion', 'HeatTemplateFormatVersion',
-                'Parameters', 'Resources', 'Outputs')
+                'Description', 'Mappings', 'Parameters', 'Resources', 'Outputs'
+                )
 
-    SECTIONS_NO_DIRECT_ACCESS = set([PARAMETERS, VERSION])
+    SECTIONS_NO_DIRECT_ACCESS = set([PARAMETERS, VERSION, ALTERNATE_VERSION])
 
     def __getitem__(self, section):
         '''Get the relevant section in the template.'''

heat/engine/parser.py

@@ -336,6 +336,9 @@ class Stack(collections.Mapping):
         '''
         # TODO(sdake) Should return line number of invalid reference
 
+        # validate overall template (top-level structure)
+        self.t.validate()
+
         # Validate Parameter Groups
         parameter_groups = ParameterGroups(self.t)
         parameter_groups.validate()

heat/engine/service.py

@@ -600,6 +600,9 @@ class EngineService(service.Service):
         except KeyError as ex:
             return {'Error': str(ex)}
 
+        # validate overall template (top-level structure)
+        tmpl.validate()
+
         if not tmpl_resources:
             return {'Error': 'At least one Resources member must be defined.'}
 

heat/engine/template.py

@@ -165,6 +165,18 @@ class Template(collections.Mapping):
     def parse(self, stack, snippet):
         return parse(self.functions(), stack, snippet)
 
+    def validate(self):
+        '''Validate the template.
+
+        Only validates the top-level sections of the template. Syntax inside
+        sections is not checked here but in code parts that are responsible
+        for working with the respective sections.
+        '''
+
+        for k in self.t.keys():
+            if k not in self.SECTIONS:
+                raise exception.InvalidTemplateSection(section=k)
+
 
 def parse(functions, stack, snippet):
     recurse = functools.partial(parse, functions, stack)

heat/tests/test_template.py

@@ -90,3 +90,76 @@ class TestTemplateVersion(HeatTestCase):
         }
         self.assertRaises(exception.InvalidTemplateVersion,
                           template.get_version, tmpl, self.versions)
+
+
+class TestTemplateValidate(HeatTestCase):
+
+    def test_template_validate_cfn_good(self):
+        t = {
+            'AWSTemplateFormatVersion': '2010-09-09',
+            'Description': 'foo',
+            'Parameters': {},
+            'Mappings': {},
+            'Resources': {},
+            'Outputs': {},
+        }
+
+        tmpl = template.Template(t)
+        err = tmpl.validate()
+        self.assertIsNone(err)
+
+        # test with alternate version key
+        t = {
+            'HeatTemplateFormatVersion': '2012-12-12',
+            'Description': 'foo',
+            'Parameters': {},
+            'Mappings': {},
+            'Resources': {},
+            'Outputs': {},
+        }
+
+        tmpl = template.Template(t)
+        err = tmpl.validate()
+        self.assertIsNone(err)
+
+    def test_template_validate_cfn_bad_section(self):
+        t = {
+            'AWSTemplateFormatVersion': '2010-09-09',
+            'Description': 'foo',
+            'Parameteers': {},
+            'Mappings': {},
+            'Resources': {},
+            'Outputs': {},
+        }
+
+        tmpl = template.Template(t)
+        err = self.assertRaises(exception.InvalidTemplateSection,
+                                tmpl.validate)
+        self.assertIn('Parameteers', str(err))
+
+    def test_template_validate_hot_good(self):
+        t = {
+            'heat_template_version': '2013-05-23',
+            'description': 'foo',
+            'parameters': {},
+            'resources': {},
+            'outputs': {},
+        }
+
+        tmpl = template.Template(t)
+        err = tmpl.validate()
+        self.assertIsNone(err)
+
+    def test_template_validate_hot_bad_section(self):
+        t = {
+            'heat_template_version': '2013-05-23',
+            'description': 'foo',
+            'parameteers': {},
+            'resources': {},
+            'outputs': {},
+        }
+
+        tmpl = template.Template(t)
+        err = self.assertRaises(exception.InvalidTemplateSection,
+                                tmpl.validate)
+        self.assertIn('parameteers', str(err))

heat/tests/test_validate.py

@@ -25,6 +25,7 @@ from heat.engine import resources
 from heat.engine.resources import instance as instances
 from heat.engine import service
 from heat.openstack.common.importutils import try_import
+from heat.openstack.common.rpc import common as rpc_common
 from heat.tests.common import HeatTestCase
 from heat.tests import utils
 from heat.tests.v1_1 import fakes
@@ -975,6 +976,56 @@ class validateTest(HeatTestCase):
                           'Found a [string] instead'},
                          res)
 
+    def test_invalid_section_cfn(self):
+        t = template_format.parse(
+            """
+            {
+                'AWSTemplateFormatVersion': '2010-09-09',
+                'Resources': {
+                    'server': {
+                        'Type': 'OS::Nova::Server'
+                    }
+                },
+                'Output': {}
+            }
+            """)
+
+        self.m.StubOutWithMock(instances.Instance, 'nova')
+        instances.Instance.nova().AndReturn(self.fc)
+        self.m.StubOutWithMock(service.EngineListener, 'start')
+        service.EngineListener.start().AndReturn(None)
+        self.m.ReplayAll()
+
+        engine = service.EngineService('a', 't')
+        ex = self.assertRaises(rpc_common.ClientException,
+                               engine.validate_template, None, t)
+        self.assertEqual(ex._exc_info[0], exception.InvalidTemplateSection)
+        self.assertEqual('The template section is invalid: Output',
+                         str(ex._exc_info[1]))
+
+    def test_invalid_section_hot(self):
+        t = template_format.parse(
+            """
+            heat_template_version: 2013-05-23
+            resources:
+              server:
+                type: OS::Nova::Server
+            output:
+            """)
+
+        self.m.StubOutWithMock(instances.Instance, 'nova')
+        instances.Instance.nova().AndReturn(self.fc)
+        self.m.StubOutWithMock(service.EngineListener, 'start')
+        service.EngineListener.start().AndReturn(None)
+        self.m.ReplayAll()
+
+        engine = service.EngineService('a', 't')
+        ex = self.assertRaises(rpc_common.ClientException,
+                               engine.validate_template, None, t)
+        self.assertEqual(ex._exc_info[0], exception.InvalidTemplateSection)
+        self.assertEqual('The template section is invalid: output',
+                         str(ex._exc_info[1]))
+
     def test_unimplemented_property(self):
         t = template_format.parse(test_template_unimplemented_property)
         self.m.StubOutWithMock(instances.Instance, 'nova')