Refactor resource definition parsing in HOT/cfn

Parse each field in a resource definition individually instead of all in
one go. This allows the HOT and cfn formats to apply different parsing to
different fields - specifically in the future we want to parse the
condition field like a condition, using only the valid condition functions.

Change-Id: I5b864d241a5e16c09fcce30c40d634d9bb72e173

heat/engine/cfn/template.py

@@ -16,7 +16,6 @@ import six
 from heat.common import exception
 from heat.common.i18n import _
 from heat.engine.cfn import functions as cfn_funcs
-from heat.engine import function
 from heat.engine import parameters
 from heat.engine import rsrc_defn
 from heat.engine import template_common
@@ -110,47 +109,18 @@ class CfnTemplateBase(template_common.CommonTemplate):
     def resource_definitions(self, stack):
         resources = self.t.get(self.RESOURCES) or {}
 
-        def build_rsrc_defn(name, data):
-            depends = data.get(self.RES_DEPENDS_ON)
-            if isinstance(depends, six.string_types):
-                depends = [depends]
-
-            deletion_policy = function.resolve(
-                data.get(self.RES_DELETION_POLICY))
-            if deletion_policy is not None:
-                if deletion_policy not in self.deletion_policies:
-                    msg = _('Invalid deletion policy "%s"') % deletion_policy
-                    raise exception.StackValidationFailed(message=msg)
-                else:
-                    deletion_policy = self.deletion_policies[deletion_policy]
-
-            kwargs = {
-                'resource_type': data.get(self.RES_TYPE),
-                'properties': data.get(self.RES_PROPERTIES),
-                'metadata': data.get(self.RES_METADATA),
-                'depends': depends,
-                'deletion_policy': deletion_policy,
-                'update_policy': data.get(self.RES_UPDATE_POLICY),
-                'description': data.get(self.RES_DESCRIPTION) or ''
-            }
-
-            if hasattr(self, 'RES_CONDITION'):
-                kwargs['condition'] = data.get(self.RES_CONDITION)
-
-            return rsrc_defn.ResourceDefinition(name, **kwargs)
-
         conditions = self.conditions(stack)
 
         def defns():
             for name, snippet in resources.items():
                 try:
-                    data = self.parse(stack, snippet)
+                    defn_data = dict(self._rsrc_defn_args(stack, name,
-                    self._validate_resource_definition(name, data)
+                                                          snippet))
                 except (TypeError, ValueError, KeyError) as ex:
                     msg = six.text_type(ex)
                     raise exception.StackValidationFailed(message=msg)
 
-                defn = build_rsrc_defn(name, data)
+                defn = rsrc_defn.ResourceDefinition(name, **defn_data)
                 cond_name = defn.condition_name()
 
                 if cond_name is not None:
@@ -231,12 +201,18 @@ class CfnTemplate(CfnTemplateBase):
     def _get_condition_definitions(self):
         return self.t.get(self.CONDITIONS, {})
 
-    def _validate_resource_definition(self, name, data):
+    def _rsrc_defn_args(self, stack, name, data):
-        super(CfnTemplate, self)._validate_resource_definition(name, data)
+        for arg in super(CfnTemplate, self)._rsrc_defn_args(stack, name, data):
+            yield arg
 
-        self.validate_resource_key_type(self.RES_CONDITION,
+        def no_parse(field, path):
-                                        (six.string_types, bool),
+            return field
-                                        'string or boolean', name, data)
+
+        yield ('condition',
+               self._parse_resource_field(self.RES_CONDITION,
+                                          (six.string_types, bool),
+                                          'string or boolean',
+                                          name, data, no_parse))
 
 
 class HeatTemplate(CfnTemplateBase):

heat/engine/hot/template.py

@@ -10,6 +10,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import functools
+
 import six
 
 from heat.common import exception
@@ -217,29 +219,28 @@ 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)
-
-        invalid_keys = set(data) - set(self._RESOURCE_KEYS)
-        if invalid_keys:
-            raise ValueError(_('Invalid keyword(s) inside a resource '
-                               'definition: %s') % ', '.join(invalid_keys))
-
     def resource_definitions(self, stack):
         resources = self.t.get(self.RESOURCES) or {}
         conditions = self.conditions(stack)
 
+        valid_keys = frozenset(self._RESOURCE_KEYS)
+
         def defns():
             for name, snippet in six.iteritems(resources):
                 try:
-                    data = self.parse(stack, snippet)
+                    invalid_keys = set(snippet) - valid_keys
-                    self._validate_resource_definition(name, data)
+                    if invalid_keys:
+                        raise ValueError(_('Invalid keyword(s) inside a '
+                                           'resource definition: '
+                                           '%s') % ', '.join(invalid_keys))
+
+                    defn_data = dict(self._rsrc_defn_args(stack, name,
+                                                          snippet))
                 except (TypeError, ValueError, KeyError) as ex:
                     msg = six.text_type(ex)
                     raise exception.StackValidationFailed(message=msg)
 
-                defn = self.rsrc_defn_from_snippet(name, data)
+                defn = rsrc_defn.ResourceDefinition(name, **defn_data)
                 cond_name = defn.condition_name()
 
                 if cond_name is not None:
@@ -257,37 +258,6 @@ class HOTemplate20130523(template_common.CommonTemplate):
 
         return dict(defns())
 
-    @classmethod
-    def rsrc_defn_from_snippet(cls, name, data):
-        depends = data.get(cls.RES_DEPENDS_ON)
-        if isinstance(depends, six.string_types):
-            depends = [depends]
-
-        deletion_policy = function.resolve(
-            data.get(cls.RES_DELETION_POLICY))
-        if deletion_policy is not None:
-            if deletion_policy not in cls.deletion_policies:
-                msg = _('Invalid deletion policy "%s"') % deletion_policy
-                raise exception.StackValidationFailed(message=msg)
-            else:
-                deletion_policy = cls.deletion_policies[deletion_policy]
-
-        kwargs = {
-            'resource_type': data.get(cls.RES_TYPE),
-            'properties': data.get(cls.RES_PROPERTIES),
-            'metadata': data.get(cls.RES_METADATA),
-            'depends': depends,
-            'deletion_policy': deletion_policy,
-            'update_policy': data.get(cls.RES_UPDATE_POLICY),
-            'description': None
-        }
-        if hasattr(cls, 'RES_EXTERNAL_ID'):
-            kwargs['external_id'] = data.get(cls.RES_EXTERNAL_ID)
-        if hasattr(cls, 'RES_CONDITION'):
-            kwargs['condition'] = data.get(cls.RES_CONDITION)
-
-        return rsrc_defn.ResourceDefinition(name, **kwargs)
-
     def add_resource(self, definition, name=None):
         if name is None:
             name = definition.name
@@ -517,13 +487,26 @@ class HOTemplate20161014(HOTemplate20160408):
     def _get_condition_definitions(self):
         return self.t.get(self.CONDITIONS, {})
 
-    def _validate_resource_definition(self, name, data):
+    def _rsrc_defn_args(self, stack, name, data):
-        super(HOTemplate20161014, self)._validate_resource_definition(
+        for arg in super(HOTemplate20161014, self)._rsrc_defn_args(stack,
-            name, data)
+                                                                   name,
+                                                                   data):
+            yield arg
 
-        self.validate_resource_key_type(self.RES_EXTERNAL_ID,
+        parse = functools.partial(self.parse, stack)
-                                        (six.string_types, function.Function),
+
-                                        'string', name, data)
+        def no_parse(field, path):
-        self.validate_resource_key_type(self.RES_CONDITION,
+            return field
-                                        (six.string_types, bool),
+
-                                        'string or boolean', name, data)
+        yield ('external_id',
+               self._parse_resource_field(self.RES_EXTERNAL_ID,
+                                          (six.string_types,
+                                           function.Function),
+                                          'string',
+                                          name, data, parse))
+
+        yield ('condition',
+               self._parse_resource_field(self.RES_CONDITION,
+                                          (six.string_types, bool),
+                                          'string or boolean',
+                                          name, data, no_parse))

heat/engine/resources/openstack/heat/autoscaling_group.py

@@ -16,10 +16,26 @@ from heat.common import grouputils
 from heat.common.i18n import _
 from heat.engine import attributes
 from heat.engine import constraints
+from heat.engine.hot import template
 from heat.engine import properties
 from heat.engine.resources.aws.autoscaling import autoscaling_group as aws_asg
+from heat.engine import rsrc_defn
 from heat.engine import support
-from heat.engine import template
+
+
+class HOTInterpreter(template.HOTemplate20150430):
+    def __new__(cls):
+        return object.__new__(cls)
+
+    def __init__(self):
+        version = {'heat_template_version': '2015-04-30'}
+        super(HOTInterpreter, self).__init__(version)
+
+    def parse(self, stack, snippet, path=''):
+        return snippet
+
+    def parse_conditions(self, stack, snippet, path=''):
+        return snippet
 
 
 class AutoScalingResourceGroup(aws_asg.AutoScalingGroup):
@@ -155,12 +171,11 @@ class AutoScalingResourceGroup(aws_asg.AutoScalingGroup):
     }
     update_policy_schema = {}
 
-    def _get_resource_definition(self,
+    def _get_resource_definition(self):
-                                 template_version=('heat_template_version',
+        resource_def = self.properties[self.RESOURCE]
-                                                   '2015-04-30')):
+        defn_data = dict(HOTInterpreter()._rsrc_defn_args(None, 'member',
-        tmpl = template.Template(dict([template_version]))
+                                                          resource_def))
-        return tmpl.rsrc_defn_from_snippet(None,
+        return rsrc_defn.ResourceDefinition(None, **defn_data)
-                                           self.properties[self.RESOURCE])
 
     def _try_rolling_update(self, prop_diff):
         if self.RESOURCE in prop_diff:

heat/engine/template_common.py

@@ -12,6 +12,7 @@
 #    under the License.
 
 import collections
+import functools
 import weakref
 
 import six
@@ -37,60 +38,92 @@ class CommonTemplate(template.Template):
         self._conditions_cache = None, None
 
     @classmethod
-    def validate_resource_key_type(cls, key, valid_types, typename,
+    def _parse_resource_field(cls, key, valid_types, typename,
-                                   rsrc_name, rsrc_data):
+                              rsrc_name, rsrc_data, parse_func):
-        """Validate the type of the value provided for a specific resource key.
+        """Parse a field in a resource definition.
 
-        Used in _validate_resource_definition() to validate correctness of
+        :param key: The name of the key
-        user input data.
+        :param valid_types: Valid types for the parsed output
+        :param typename: Description of valid type to include in error output
+        :param rsrc_name: The resource name
+        :param rsrc_data: The unparsed resource definition data
+        :param parse_func: A function to parse the data, which takes the
+            contents of the field and its path in the template as arguments.
         """
         if key in rsrc_data:
-            if not isinstance(rsrc_data[key], valid_types):
+            data = parse_func(rsrc_data[key], '.'.join([cls.RESOURCES,
+                                                        rsrc_name,
+                                                        key]))
+            if not isinstance(data, valid_types):
                 args = {'name': rsrc_name, 'key': key,
                         'typename': typename}
                 message = _('Resource %(name)s %(key)s type '
                             'must be %(typename)s') % args
                 raise TypeError(message)
-            return True
+            return data
         else:
-            return False
+            return None
 
-    def _validate_resource_definition(self, name, data):
+    def _rsrc_defn_args(self, stack, name, data):
-        """Validate a resource definition snippet given the parsed data."""
+        if self.RES_TYPE not in data:
-
-        if not self.validate_resource_key_type(self.RES_TYPE,
-                                               six.string_types,
-                                               'string',
-                                               name,
-                                               data):
             args = {'name': name, 'type_key': self.RES_TYPE}
             msg = _('Resource %(name)s is missing "%(type_key)s"') % args
             raise KeyError(msg)
 
-        self.validate_resource_key_type(
+        parse = functools.partial(self.parse, stack)
-            self.RES_PROPERTIES,
+
-            (collections.Mapping, function.Function),
+        def no_parse(field, path):
-            'object', name, data)
+            return field
-        self.validate_resource_key_type(
+
-            self.RES_METADATA,
+        yield ('resource_type',
-            (collections.Mapping, function.Function),
+               self._parse_resource_field(self.RES_TYPE,
-            'object', name, data)
+                                          six.string_types, 'string',
-        self.validate_resource_key_type(
+                                          name, data, parse))
-            self.RES_DEPENDS_ON,
+
-            collections.Sequence,
+        yield ('properties',
-            'list or string', name, data)
+               self._parse_resource_field(self.RES_PROPERTIES,
-        self.validate_resource_key_type(
+                                          (collections.Mapping,
-            self.RES_DELETION_POLICY,
+                                           function.Function), 'object',
-            (six.string_types, function.Function),
+                                          name, data, parse))
-            'string', name, data)
+
-        self.validate_resource_key_type(
+        yield ('metadata',
-            self.RES_UPDATE_POLICY,
+               self._parse_resource_field(self.RES_METADATA,
-            (collections.Mapping, function.Function),
+                                          (collections.Mapping,
-            'object', name, data)
+                                           function.Function), 'object',
-        self.validate_resource_key_type(
+                                          name, data, parse))
-            self.RES_DESCRIPTION,
+
-            six.string_types,
+        depends = self._parse_resource_field(self.RES_DEPENDS_ON,
-            'string', name, data)
+                                             collections.Sequence,
+                                             'list or string',
+                                             name, data, no_parse)
+        if isinstance(depends, six.string_types):
+            depends = [depends]
+        yield 'depends', depends
+
+        del_policy = self._parse_resource_field(self.RES_DELETION_POLICY,
+                                                (six.string_types,
+                                                 function.Function),
+                                                'string',
+                                                name, data, parse)
+        deletion_policy = function.resolve(del_policy)
+        if deletion_policy is not None:
+            if deletion_policy not in self.deletion_policies:
+                msg = _('Invalid deletion policy "%s"') % deletion_policy
+                raise exception.StackValidationFailed(message=msg)
+            else:
+                deletion_policy = self.deletion_policies[deletion_policy]
+        yield 'deletion_policy', deletion_policy
+
+        yield ('update_policy',
+               self._parse_resource_field(self.RES_UPDATE_POLICY,
+                                          (collections.Mapping,
+                                           function.Function), 'object',
+                                          name, data, parse))
+
+        yield ('description',
+               self._parse_resource_field(self.RES_DESCRIPTION,
+                                          six.string_types, 'string',
+                                          name, data, no_parse))
 
     def _get_condition_definitions(self):
         """Return the condition definitions of template."""