Deprecate Template.validate_resource_definitions()

This method was added in order to be able to validate the resource
definitions during the Stack.validate() phase, and not have to wait until
the Resource objects are created. (It can't be done during
Template.validate() because the stack is not passed there, which is needed
to call Template.parse().)

However we actually already generate the ResourceDefinitions right below
this in Stack.validate() anyway, by virtue of creating the Resource
objects, so there was really never any reason to put it in a separate
method. It was actually much less efficient because we had to parse the
resource section of the template twice instead of once.

Accordingly, move the validation code into the resource_definitions()
methods and deprecate the validate_resource_definitions() method. (We have
to keep calling it for now, because third-party template formats may be
relying on this validation having been done before calling
resource_definitions().)

Change-Id: I76be34c5cf8500fca1654a066a5cead57b0e7bab

6 files changed, 46 insertions, 33 deletions

diff --git a/heat/engine/cfn/template.py b/heat/engine/cfn/template.py
index 1655433f7..fb123b2b7 100644
--- a/heat/engine/cfn/template.py
+++ b/heat/engine/cfn/template.py
@@ -113,7 +113,12 @@ class CfnTemplateBase(template_common.CommonTemplate):
         resources = self.t.get(self.RESOURCES) or {}
 
         def rsrc_defn_item(name, snippet):
-            data = self.parse(stack, snippet)
+            try:
+                data = self.parse(stack, snippet)
+                self._validate_resource_definition(name, data)
+            except (TypeError, ValueError, KeyError) as ex:
+                msg = six.text_type(ex)
+                raise exception.StackValidationFailed(message=msg)
 
             depends = data.get(self.RES_DEPENDS_ON)
             if isinstance(depends, six.string_types):
@@ -221,8 +226,8 @@ class CfnTemplate(CfnTemplateBase):
 
         return False
 
-    def validate_resource_definition(self, name, data):
-        super(CfnTemplate, self).validate_resource_definition(name, data)
+    def _validate_resource_definition(self, name, data):
+        super(CfnTemplate, self)._validate_resource_definition(name, data)
 
         self.validate_resource_key_type(self.RES_CONDITION,
                                         (six.string_types, bool),
diff --git a/heat/engine/hot/template.py b/heat/engine/hot/template.py
index 0b12ebe09..72789ebcd 100644
--- a/heat/engine/hot/template.py
+++ b/heat/engine/hot/template.py
@@ -218,9 +218,9 @@ class HOTemplate20130523(template_common.CommonTemplate):
                                         user_params=user_params,
                                         param_defaults=param_defaults)
 
-    def validate_resource_definition(self, name, data):
-        super(HOTemplate20130523, self).validate_resource_definition(name,
-                                                                     data)
+    def _validate_resource_definition(self, name, data):
+        super(HOTemplate20130523, self)._validate_resource_definition(name,
+                                                                      data)
 
         invalid_keys = set(data) - set(self._RESOURCE_KEYS)
         if invalid_keys:
@@ -231,7 +231,12 @@ class HOTemplate20130523(template_common.CommonTemplate):
         resources = self.t.get(self.RESOURCES) or {}
 
         def rsrc_defn_from_snippet(name, snippet):
-            data = self.parse(stack, snippet)
+            try:
+                data = self.parse(stack, snippet)
+                self._validate_resource_definition(name, data)
+            except (TypeError, ValueError, KeyError) as ex:
+                msg = six.text_type(ex)
+                raise exception.StackValidationFailed(message=msg)
             return self.rsrc_defn_from_snippet(name, data)
 
         return dict(
@@ -498,8 +503,8 @@ class HOTemplate20161014(HOTemplate20160408):
     def get_condition_definitions(self):
         return self.t.get(self.CONDITIONS, {})
 
-    def validate_resource_definition(self, name, data):
-        super(HOTemplate20161014, self).validate_resource_definition(
+    def _validate_resource_definition(self, name, data):
+        super(HOTemplate20161014, self)._validate_resource_definition(
             name, data)
 
         self.validate_resource_key_type(self.RES_EXTERNAL_ID,
diff --git a/heat/engine/stack.py b/heat/engine/stack.py
index 1ddba6912..3b152d30b 100644
--- a/heat/engine/stack.py
+++ b/heat/engine/stack.py
@@ -301,9 +301,6 @@ class Stack(collections.Mapping):
 
     @property
     def resources(self):
-        return self._find_resources()
-
-    def _find_resources(self):
         if self._resources is None:
             res_defns = self.t.resource_definitions(self)
 
@@ -788,11 +785,25 @@ class Stack(collections.Mapping):
         # Validate condition definition of conditions section
         self.t.validate_condition_definitions(self)
 
-        # Validate types of sections in ResourceDefinitions
+        # Continue to call this function, since old third-party Template
+        # plugins may depend on it being called to validate the resource
+        # definitions before actually generating them.
+        if (type(self.t).validate_resource_definitions !=
+                tmpl.Template.validate_resource_definitions):
+            warnings.warn("The Template.validate_resource_definitions() "
+                          "method is deprecated and will no longer be called "
+                          "in future versions of Heat. Template subclasses "
+                          "should validate resource definitions in the "
+                          "resource_definitions() method.",
+                          DeprecationWarning)
         self.t.validate_resource_definitions(self)
 
+        # Load the resources definitions (success of which implies the
+        # definitions are valid)
+        resources = self.resources
+
         # Check duplicate names between parameters and resources
-        dup_names = set(self.parameters) & set(self.keys())
+        dup_names = set(self.parameters) & set(resources)
 
         if dup_names:
             LOG.debug("Duplicate names %s" % dup_names)
diff --git a/heat/engine/template.py b/heat/engine/template.py
index e3438ff16..1c055ed0a 100644
--- a/heat/engine/template.py
+++ b/heat/engine/template.py
@@ -244,9 +244,15 @@ class Template(collections.Mapping):
         else:
             return False
 
-    @abc.abstractmethod
     def validate_resource_definitions(self, stack):
-        """Check section's type of ResourceDefinitions."""
+        """Check validity of resource definitions.
+
+        This method is deprecated. Subclasses should validate the resource
+        definitions in the process of generating them when calling
+        resource_definitions(). However, for now this method is still called
+        in case any third-party plugins are relying on this for validation and
+        need time to migrate.
+        """
         pass
 
     def validate_condition_definitions(self, stack):
diff --git a/heat/engine/template_common.py b/heat/engine/template_common.py
index 9a4bd0b1f..5c1357c7e 100644
--- a/heat/engine/template_common.py
+++ b/heat/engine/template_common.py
@@ -44,7 +44,9 @@ class CommonTemplate(template.Template):
         else:
             return False
 
-    def validate_resource_definition(self, name, data):
+    def _validate_resource_definition(self, name, data):
+        """Validate a resource definition snippet given the parsed data."""
+
         if not self.validate_resource_key_type(self.RES_TYPE,
                                                six.string_types,
                                                'string',
@@ -79,19 +81,6 @@ class CommonTemplate(template.Template):
             six.string_types,
             'string', name, data)
 
-    def validate_resource_definitions(self, stack):
-        """Check section's type of ResourceDefinitions."""
-
-        resources = self.t.get(self.RESOURCES) or {}
-
-        try:
-            for name, snippet in resources.items():
-                path = '.'.join([self.RESOURCES, name])
-                data = self.parse(stack, snippet, path)
-                self.validate_resource_definition(name, data)
-        except (TypeError, ValueError, KeyError) as ex:
-            raise exception.StackValidationFailed(message=six.text_type(ex))
-
     def validate_condition_definitions(self, stack):
         """Check conditions section."""
 
diff --git a/heat/tests/test_template.py b/heat/tests/test_template.py
index 770f4826d..900013b8e 100644
--- a/heat/tests/test_template.py
+++ b/heat/tests/test_template.py
@@ -136,9 +136,6 @@ class TestTemplatePluginManager(common.HeatTestCase):
                            param_defaults=None):
                 pass
 
-            def validate_resource_definitions(self, stack):
-                pass
-
             def validate_condition_definitions(self, stack):
                 pass