Ensure the whole ResourceDefinition is validated

We used to validate intrinsic functions in the ResourceDefinition by just
passing it to function.validate(). That worked when it also acted as a
CloudFormation template snippet but, since that behaviour was removed by
2c6fc7bcd6, that call now does nothing. This
replaces it with a validate() method on the ResourceDefinition itself.

This also improves path reporting for validation errors in deletion or
update policies.

Change-Id: I58c788002136ddf9bded309366e2072823b3ac77
Closes-Bug: #1732798

heat/engine/resource.py

@@ -1800,19 +1800,19 @@ class Resource(status.ResourceStatus):
             self.stack.context,
             self.t.resource_type
         )
-        path = '.'.join([self.stack.t.RESOURCES, self.name])
-        function.validate(self.t, path)
-        self.validate_deletion_policy(self.t.deletion_policy())
-        self.t.update_policy(self.update_policy_schema,
-                             self.context).validate()
         try:
+            self.t.validate()
+            self.validate_deletion_policy(self.t.deletion_policy())
+            self.t.update_policy(self.update_policy_schema,
+                                 self.context).validate()
             validate = self.properties.validate(
                 with_value=self.stack.strict_validate,
                 template=self.t)
         except exception.StackValidationFailed as ex:
-            path = [self.stack.t.RESOURCES, self.t.name,
-                    self.stack.t.get_section_name(ex.path[0])]
-            path.extend(ex.path[1:])
+            path = [self.stack.t.RESOURCES, self.t.name]
+            if ex.path:
+                path.append(self.stack.t.get_section_name(ex.path[0]))
+                path.extend(ex.path[1:])
             raise exception.StackValidationFailed(
                 error=ex.error,
                 path=path,
@@ -1821,14 +1821,15 @@ class Resource(status.ResourceStatus):
 
     @classmethod
     def validate_deletion_policy(cls, policy):
+        path = rsrc_defn.DELETION_POLICY
         if policy not in rsrc_defn.ResourceDefinition.DELETION_POLICIES:
             msg = _('Invalid deletion policy "%s"') % policy
-            raise exception.StackValidationFailed(message=msg)
+            raise exception.StackValidationFailed(message=msg, path=path)
 
         if policy == rsrc_defn.ResourceDefinition.SNAPSHOT:
             if not callable(getattr(cls, 'handle_snapshot_delete', None)):
                 msg = _('"%s" deletion policy not supported') % policy
-                raise exception.StackValidationFailed(message=msg)
+                raise exception.StackValidationFailed(message=msg, path=path)
 
     def _update_replacement_data(self, template_id):
         # Update the replacement resource's needed_by and replaces

heat/engine/rsrc_defn.py

@@ -204,6 +204,15 @@ class ResourceDefinition(object):
             external_id=reparse_snippet(self._external_id),
             condition=self._condition)
 
+    def validate(self):
+        """Validate intrinsic functions that appear in the definition."""
+        function.validate(self._properties, PROPERTIES)
+        function.validate(self._metadata, METADATA)
+        function.validate(self._depends, DEPENDS_ON)
+        function.validate(self._deletion_policy, DELETION_POLICY)
+        function.validate(self._update_policy, UPDATE_POLICY)
+        function.validate(self._external_id, EXTERNAL_ID)
+
     def dep_attrs(self, resource_name, load_all=False):
         """Iterate over attributes of a given resource that this references.
 

heat/tests/aws/test_volume.py

@@ -168,6 +168,7 @@ class VolumeTest(vt_base.BaseVolumeTest):
         self.m.ReplayAll()
 
         stack = utils.parse_stack(self.t, stack_name=stack_name)
+        stack._update_all_resource_data(True, False)
 
         rsrc = stack['DataVolume']
         self.assertIsNone(rsrc.validate())
@@ -740,7 +741,7 @@ class VolumeTest(vt_base.BaseVolumeTest):
         self.assertEqual((rsrc.UPDATE, rsrc.FAILED), rsrc.state)
         self.m.VerifyAll()
 
-    def test_vaildate_deletion_policy(self):
+    def test_validate_deletion_policy(self):
         cfg.CONF.set_override('backups_enabled', False, group='volumes')
         stack_name = 'test_volume_validate_deletion_policy'
         self.t['Resources']['DataVolume']['DeletionPolicy'] = 'Snapshot'

heat/tests/test_validate.py

@@ -207,6 +207,19 @@ test_template_findinmap_invalid = '''
 }
 '''
 
+test_template_bad_yaql_metadata = '''
+heat_template_version: 2016-10-14
+parameters:
+resources:
+  my_instance:
+    type: OS::Heat::TestResource
+    metadata:
+      test:
+        yaql:
+          expression: {'foo': 'bar'}
+          data: "$.data"
+'''
+
 test_template_invalid_resources = '''
 {
   "AWSTemplateFormatVersion" : "2010-09-09",
@@ -1054,6 +1067,12 @@ class ValidateTest(common.HeatTestCase):
         res = dict(self.engine.validate_template(self.ctx, t, {}))
         self.assertNotEqual(res['Description'], 'Successfully validated')
 
+    def test_validate_bad_yaql_metadata(self):
+        t = template_format.parse(test_template_bad_yaql_metadata)
+        res = dict(self.engine.validate_template(self.ctx, t, {}))
+        self.assertIn('Error', res)
+        self.assertIn('yaql', res['Error'])
+
     def test_validate_parameters(self):
         t = template_format.parse(test_template_ref % 'WikiDatabase')
         res = dict(self.engine.validate_template(self.ctx, t, {}))
@@ -1344,8 +1363,10 @@ class ValidateTest(common.HeatTestCase):
         t = template_format.parse(test_template_snapshot_deletion_policy)
 
         res = dict(self.engine.validate_template(self.ctx, t, {}))
-        self.assertEqual(
-            {'Error': '"Snapshot" deletion policy not supported'}, res)
+        self.assertEqual({'Error': 'Resources.WikiDatabase.DeletionPolicy: '
+                                   '"Snapshot" deletion policy '
+                                   'not supported'},
+                         res)
 
     def test_volume_snapshot_deletion_policy(self):
         t = template_format.parse(test_template_volume_snapshot)