From b67605de2478b303f1fa8a40c768698ede35630a Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Sat, 10 Sep 2016 15:22:44 -0400 Subject: [PATCH] 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 | 52 +++----- heat/engine/hot/template.py | 87 ++++++-------- .../openstack/heat/autoscaling_group.py | 29 +++-- heat/engine/template_common.py | 113 +++++++++++------- 4 files changed, 144 insertions(+), 137 deletions(-) diff --git a/heat/engine/cfn/template.py b/heat/engine/cfn/template.py index 5993347ee7..0b43cdff10 100644 --- a/heat/engine/cfn/template.py +++ b/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) - self._validate_resource_definition(name, data) + 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 = 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): - super(CfnTemplate, self)._validate_resource_definition(name, data) + def _rsrc_defn_args(self, stack, name, data): + for arg in super(CfnTemplate, self)._rsrc_defn_args(stack, name, data): + yield arg - self.validate_resource_key_type(self.RES_CONDITION, - (six.string_types, bool), - 'string or boolean', name, data) + def no_parse(field, path): + return field + + yield ('condition', + self._parse_resource_field(self.RES_CONDITION, + (six.string_types, bool), + 'string or boolean', + name, data, no_parse)) class HeatTemplate(CfnTemplateBase): diff --git a/heat/engine/hot/template.py b/heat/engine/hot/template.py index 2aa608e700..af280a2e8b 100644 --- a/heat/engine/hot/template.py +++ b/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) - self._validate_resource_definition(name, data) + invalid_keys = set(snippet) - valid_keys + 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): - super(HOTemplate20161014, self)._validate_resource_definition( - name, data) + def _rsrc_defn_args(self, stack, name, data): + for arg in super(HOTemplate20161014, self)._rsrc_defn_args(stack, + name, + data): + yield arg - self.validate_resource_key_type(self.RES_EXTERNAL_ID, - (six.string_types, function.Function), - 'string', name, data) - self.validate_resource_key_type(self.RES_CONDITION, - (six.string_types, bool), - 'string or boolean', name, data) + parse = functools.partial(self.parse, stack) + + def no_parse(field, path): + return field + + 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)) diff --git a/heat/engine/resources/openstack/heat/autoscaling_group.py b/heat/engine/resources/openstack/heat/autoscaling_group.py index fecd5e43d2..23e2b263d2 100644 --- a/heat/engine/resources/openstack/heat/autoscaling_group.py +++ b/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, - template_version=('heat_template_version', - '2015-04-30')): - tmpl = template.Template(dict([template_version])) - return tmpl.rsrc_defn_from_snippet(None, - self.properties[self.RESOURCE]) + def _get_resource_definition(self): + resource_def = self.properties[self.RESOURCE] + defn_data = dict(HOTInterpreter()._rsrc_defn_args(None, 'member', + resource_def)) + return rsrc_defn.ResourceDefinition(None, **defn_data) def _try_rolling_update(self, prop_diff): if self.RESOURCE in prop_diff: diff --git a/heat/engine/template_common.py b/heat/engine/template_common.py index c6354e9d32..f421d18f90 100644 --- a/heat/engine/template_common.py +++ b/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, - rsrc_name, rsrc_data): - """Validate the type of the value provided for a specific resource key. + def _parse_resource_field(cls, key, valid_types, typename, + rsrc_name, rsrc_data, parse_func): + """Parse a field in a resource definition. - Used in _validate_resource_definition() to validate correctness of - user input data. + :param key: The name of the key + :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): - """Validate a resource definition snippet given the parsed data.""" - - if not self.validate_resource_key_type(self.RES_TYPE, - six.string_types, - 'string', - name, - data): + def _rsrc_defn_args(self, stack, name, data): + if self.RES_TYPE not in 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( - self.RES_PROPERTIES, - (collections.Mapping, function.Function), - 'object', name, data) - self.validate_resource_key_type( - self.RES_METADATA, - (collections.Mapping, function.Function), - 'object', name, data) - self.validate_resource_key_type( - self.RES_DEPENDS_ON, - collections.Sequence, - 'list or string', name, data) - self.validate_resource_key_type( - self.RES_DELETION_POLICY, - (six.string_types, function.Function), - 'string', name, data) - self.validate_resource_key_type( - self.RES_UPDATE_POLICY, - (collections.Mapping, function.Function), - 'object', name, data) - self.validate_resource_key_type( - self.RES_DESCRIPTION, - six.string_types, - 'string', name, data) + parse = functools.partial(self.parse, stack) + + def no_parse(field, path): + return field + + yield ('resource_type', + self._parse_resource_field(self.RES_TYPE, + six.string_types, 'string', + name, data, parse)) + + yield ('properties', + self._parse_resource_field(self.RES_PROPERTIES, + (collections.Mapping, + function.Function), 'object', + name, data, parse)) + + yield ('metadata', + self._parse_resource_field(self.RES_METADATA, + (collections.Mapping, + function.Function), 'object', + name, data, parse)) + + depends = self._parse_resource_field(self.RES_DEPENDS_ON, + 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."""