From faec3a096252acdcd855e64864daaa826e96aa41 Mon Sep 17 00:00:00 2001 From: huangtianhua Date: Mon, 13 Jun 2016 11:04:46 +0800 Subject: [PATCH] Decouple hot and cfn for outputs The changes including: 1. Avoid hard code of resource and output keys 2. Decouple hot and cfn for outputs Change-Id: I1fd7e08ff5c699ddfcf98c81aed5f0d91c4248b3 --- etc/heat/templates/AWS_RDS_DBInstance.yaml | 6 +- heat/engine/api.py | 2 +- heat/engine/cfn/template.py | 6 ++ heat/engine/check_resource.py | 3 +- heat/engine/hot/template.py | 78 +++++++++------------ heat/engine/resources/template_resource.py | 2 +- heat/engine/stack.py | 14 ++-- heat/engine/template.py | 27 +++++++ heat/tests/test_hot.py | 22 +++--- heat/tests/test_stack_collect_attributes.py | 7 +- heat/tests/test_stack_resource.py | 10 +-- heat/tests/test_template.py | 66 +++++++++++++++++ 12 files changed, 169 insertions(+), 74 deletions(-) diff --git a/etc/heat/templates/AWS_RDS_DBInstance.yaml b/etc/heat/templates/AWS_RDS_DBInstance.yaml index 30173442e6..768416f842 100644 --- a/etc/heat/templates/AWS_RDS_DBInstance.yaml +++ b/etc/heat/templates/AWS_RDS_DBInstance.yaml @@ -123,5 +123,7 @@ Resources: Timeout: "600" Outputs: - Endpoint.Address: {'Fn::GetAtt': [DatabaseInstance, PublicIp]} - Endpoint.Port: {Ref: Port} + Endpoint.Address: + Value: {'Fn::GetAtt': [DatabaseInstance, PublicIp]} + Endpoint.Port: + Value: {Ref: Port} diff --git a/heat/engine/api.py b/heat/engine/api.py index a5ef84afde..1f7d279279 100644 --- a/heat/engine/api.py +++ b/heat/engine/api.py @@ -184,7 +184,7 @@ def format_stack_outputs(stack, outputs, resolve_value=False): def format_stack_output(stack, outputs, k, resolve_value=True): result = { rpc_api.OUTPUT_KEY: k, - rpc_api.OUTPUT_DESCRIPTION: outputs[k].get('Description', + rpc_api.OUTPUT_DESCRIPTION: outputs[k].get(stack.t.OUTPUT_DESCRIPTION, 'No description given'), } diff --git a/heat/engine/cfn/template.py b/heat/engine/cfn/template.py index 5526316b02..c802f7294a 100644 --- a/heat/engine/cfn/template.py +++ b/heat/engine/cfn/template.py @@ -44,6 +44,12 @@ class CfnTemplate(template.Template): 'Description', 'Mappings', 'Parameters', 'Resources', 'Outputs' ) + OUTPUT_KEYS = ( + OUTPUT_DESCRIPTION, OUTPUT_VALUE, + ) = ( + 'Description', 'Value', + ) + SECTIONS_NO_DIRECT_ACCESS = set([PARAMETERS, VERSION, ALTERNATE_VERSION]) functions = { diff --git a/heat/engine/check_resource.py b/heat/engine/check_resource.py index 2e7a7e8a7f..37aa56aef9 100644 --- a/heat/engine/check_resource.py +++ b/heat/engine/check_resource.py @@ -282,7 +282,8 @@ def construct_input_data(rsrc, curr_stack): dep_attrs = curr_stack.get_dep_attrs( six.itervalues(curr_stack.resources), curr_stack.outputs, - rsrc.name) + rsrc.name, + curr_stack.t.OUTPUT_VALUE) input_data = {'id': rsrc.id, 'name': rsrc.name, 'reference_id': rsrc.get_reference_id(), diff --git a/heat/engine/hot/template.py b/heat/engine/hot/template.py index da91665008..c8b2bec42f 100644 --- a/heat/engine/hot/template.py +++ b/heat/engine/hot/template.py @@ -27,10 +27,10 @@ from heat.engine import template _RESOURCE_KEYS = ( RES_TYPE, RES_PROPERTIES, RES_METADATA, RES_DEPENDS_ON, - RES_DELETION_POLICY, RES_UPDATE_POLICY, + RES_DELETION_POLICY, RES_UPDATE_POLICY, RES_DESCRIPTION, ) = ( 'type', 'properties', 'metadata', 'depends_on', - 'deletion_policy', 'update_policy', + 'deletion_policy', 'update_policy', 'description', ) @@ -45,6 +45,12 @@ class HOTemplate20130523(template.Template): 'parameters', 'resources', 'outputs', '__undefined__' ) + OUTPUT_KEYS = ( + OUTPUT_DESCRIPTION, OUTPUT_VALUE, + ) = ( + 'description', 'value', + ) + SECTIONS_NO_DIRECT_ACCESS = set([PARAMETERS, VERSION]) _CFN_TO_HOT_SECTIONS = {cfn_template.CfnTemplate.VERSION: VERSION, @@ -54,14 +60,19 @@ class HOTemplate20130523(template.Template): cfn_template.CfnTemplate.RESOURCES: RESOURCES, cfn_template.CfnTemplate.OUTPUTS: OUTPUTS} - _RESOURCE_HOT_TO_CFN_ATTRS = {'type': 'Type', - 'properties': 'Properties', - 'metadata': 'Metadata', - 'depends_on': 'DependsOn', - 'deletion_policy': 'DeletionPolicy', - 'update_policy': 'UpdatePolicy', - 'description': 'Description', - 'value': 'Value'} + _RESOURCE_HOT_TO_CFN_ATTRS = { + RES_TYPE: cfn_template.RES_TYPE, + RES_PROPERTIES: cfn_template.RES_PROPERTIES, + RES_METADATA: cfn_template.RES_METADATA, + RES_DEPENDS_ON: cfn_template.RES_DEPENDS_ON, + RES_DELETION_POLICY: cfn_template.RES_DELETION_POLICY, + RES_UPDATE_POLICY: cfn_template.RES_UPDATE_POLICY, + RES_DESCRIPTION: cfn_template.RES_DESCRIPTION} + + _HOT_TO_CFN_ATTRS = _RESOURCE_HOT_TO_CFN_ATTRS + _HOT_TO_CFN_ATTRS.update( + {OUTPUT_VALUE: cfn_template.CfnTemplate.OUTPUT_VALUE}) + functions = { 'Fn::GetAZs': cfn_funcs.GetAZs, 'get_param': hot_funcs.GetParam, @@ -121,7 +132,8 @@ class HOTemplate20130523(template.Template): return self._translate_resources(the_section) if section == self.OUTPUTS: - return self._translate_outputs(the_section) + self.validate_section(self.OUTPUTS, self.OUTPUT_VALUE, + the_section, self.OUTPUT_KEYS) return the_section @@ -136,57 +148,33 @@ class HOTemplate20130523(template.Template): raise ke def _translate_section(self, section, sub_section, data, mapping): + + self.validate_section(section, sub_section, data, mapping) + cfn_objects = {} - obj_name = section[:-1] - err_msg = _('"%%s" is not a valid keyword inside a %s ' - 'definition') % obj_name for name, attrs in sorted(data.items()): cfn_object = {} - if not attrs: - args = {'object_name': obj_name, 'sub_section': sub_section} - message = _('Each %(object_name)s must contain a ' - '%(sub_section)s key.') % args - raise exception.StackValidationFailed(message=message) - try: - for attr, attr_value in six.iteritems(attrs): - cfn_attr = self._translate(attr, mapping, err_msg) - cfn_object[cfn_attr] = attr_value + for attr, attr_value in six.iteritems(attrs): + cfn_attr = mapping[attr] + cfn_object[cfn_attr] = attr_value - cfn_objects[name] = cfn_object - except AttributeError: - message = _('"%(section)s" must contain a map of ' - '%(obj_name)s maps. Found a [%(_type)s] ' - 'instead') % {'section': section, - '_type': type(attrs), - 'obj_name': obj_name} - raise exception.StackValidationFailed(message=message) - except KeyError as e: - # an invalid keyword was found - raise exception.StackValidationFailed(message=e.args[0]) + cfn_objects[name] = cfn_object return cfn_objects def _translate_resources(self, resources): """Get the resources of the template translated into CFN format.""" - return self._translate_section('resources', 'type', resources, + return self._translate_section(self.RESOURCES, RES_TYPE, resources, self._RESOURCE_HOT_TO_CFN_ATTRS) def get_section_name(self, section): cfn_to_hot_attrs = dict( - zip(six.itervalues(self._RESOURCE_HOT_TO_CFN_ATTRS), - six.iterkeys(self._RESOURCE_HOT_TO_CFN_ATTRS))) + zip(six.itervalues(self._HOT_TO_CFN_ATTRS), + six.iterkeys(self._HOT_TO_CFN_ATTRS))) return cfn_to_hot_attrs.get(section, section) - def _translate_outputs(self, outputs): - """Get the outputs of the template translated into CFN format.""" - HOT_TO_CFN_ATTRS = {'description': 'Description', - 'value': 'Value'} - - return self._translate_section('outputs', 'value', outputs, - HOT_TO_CFN_ATTRS) - def param_schemata(self, param_defaults=None): parameter_section = self.t.get(self.PARAMETERS) or {} pdefaults = param_defaults or {} diff --git a/heat/engine/resources/template_resource.py b/heat/engine/resources/template_resource.py index 8bff92663a..038ced7e49 100644 --- a/heat/engine/resources/template_resource.py +++ b/heat/engine/resources/template_resource.py @@ -37,7 +37,7 @@ def generate_class_from_template(name, data, param_defaults): cls = type(name, (TemplateResource,), {'properties_schema': props, 'attributes_schema': attrs, - '__doc__': tmpl.t.get(tmpl.get_section_name('Description'))}) + '__doc__': tmpl.t.get(tmpl.DESCRIPTION)}) return cls diff --git a/heat/engine/stack.py b/heat/engine/stack.py index fd012528cc..cdcb55b8f1 100644 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -424,7 +424,7 @@ class Stack(collections.Mapping): LOG.warning(_LW("Unable to set parameters StackId identifier")) @staticmethod - def get_dep_attrs(resources, outputs, resource_name): + def get_dep_attrs(resources, outputs, resource_name, value_sec): """Return the attributes of the specified resource that are referenced. Return an iterator over any attributes of the specified resource that @@ -432,8 +432,8 @@ class Stack(collections.Mapping): """ attr_lists = itertools.chain((res.dep_attrs(resource_name) for res in resources), - (function.dep_attrs(out.get('Value', ''), - resource_name) + (function.dep_attrs( + out.get(value_sec, ''), resource_name) for out in six.itervalues(outputs))) return set(itertools.chain.from_iterable(attr_lists)) @@ -796,14 +796,14 @@ class Stack(collections.Mapping): path=[self.t.OUTPUTS], message=message) try: - if not val or 'Value' not in val: + if not val or self.t.OUTPUT_VALUE not in val: message = _('Each Output must contain ' 'a Value key.') raise exception.StackValidationFailed( error='Output validation error', path=[self.t.OUTPUTS, key], message=message) - function.validate(val.get('Value')) + function.validate(val.get(self.t.OUTPUT_VALUE)) except exception.StackValidationFailed as ex: raise except AssertionError: @@ -812,7 +812,7 @@ class Stack(collections.Mapping): raise exception.StackValidationFailed( error='Output validation error', path=[self.t.OUTPUTS, key, - self.t.get_section_name('Value')], + self.t.OUTPUT_VALUE], message=six.text_type(ex)) def requires_deferred_auth(self): @@ -1832,7 +1832,7 @@ class Stack(collections.Mapping): @profiler.trace('Stack.output', hide_args=False) def output(self, key): """Get the value of the specified stack output.""" - value = self.outputs[key].get('Value', '') + value = self.outputs[key].get(self.t.OUTPUT_VALUE, '') try: return function.resolve(value) except Exception as ex: diff --git a/heat/engine/template.py b/heat/engine/template.py index fb941ce13f..1401b8c81e 100644 --- a/heat/engine/template.py +++ b/heat/engine/template.py @@ -221,6 +221,33 @@ class Template(collections.Mapping): """ pass + def validate_section(self, section, sub_section, data, allowed_keys): + obj_name = section[:-1] + err_msg = _('"%%s" is not a valid keyword inside a %s ' + 'definition') % obj_name + args = {'object_name': obj_name, 'sub_section': sub_section} + message = _('Each %(object_name)s must contain a ' + '%(sub_section)s key.') % args + for name, attrs in sorted(data.items()): + if not attrs: + raise exception.StackValidationFailed(message=message) + try: + for attr, attr_value in six.iteritems(attrs): + if attr not in allowed_keys: + raise KeyError(err_msg % attr) + if sub_section not in attrs: + raise exception.StackValidationFailed(message=message) + except AttributeError: + message = _('"%(section)s" must contain a map of ' + '%(obj_name)s maps. Found a [%(_type)s] ' + 'instead') % {'section': section, + '_type': type(attrs), + 'obj_name': obj_name} + raise exception.StackValidationFailed(message=message) + except KeyError as e: + # an invalid keyword was found + raise exception.StackValidationFailed(message=e.args[0]) + def remove_resource(self, name): """Remove a resource from the template.""" self.t.get(self.RESOURCES, {}).pop(name) diff --git a/heat/tests/test_hot.py b/heat/tests/test_hot.py index 63c4dd9e74..b22851433c 100644 --- a/heat/tests/test_hot.py +++ b/heat/tests/test_hot.py @@ -402,8 +402,8 @@ class HOTemplateTest(common.HeatTestCase): 'inside a resource definition', six.text_type(err)) - def test_translate_outputs_good(self): - """Test translation of outputs into internal engine format.""" + def test_get_outputs_good(self): + """Test get outputs.""" hot_tpl = template_format.parse(''' heat_template_version: 2013-05-23 @@ -413,13 +413,13 @@ class HOTemplateTest(common.HeatTestCase): value: value1 ''') - expected = {'output1': {'Description': 'output1', 'Value': 'value1'}} + expected = {'output1': {'description': 'output1', 'value': 'value1'}} tmpl = template.Template(hot_tpl) self.assertEqual(expected, tmpl[tmpl.OUTPUTS]) - def test_translate_outputs_bad_no_data(self): - """Test translation of outputs without any mapping.""" + def test_get_outputs_bad_no_data(self): + """Test get outputs without any mapping.""" hot_tpl = template_format.parse(""" heat_template_version: 2013-05-23 @@ -433,8 +433,8 @@ class HOTemplateTest(common.HeatTestCase): self.assertEqual('Each output must contain a value key.', six.text_type(error)) - def test_translate_outputs_bad_without_name(self): - """Test translation of outputs without name.""" + def test_get_outputs_bad_without_name(self): + """Test get outputs without name.""" hot_tpl = template_format.parse(""" heat_template_version: 2013-05-23 @@ -450,8 +450,8 @@ class HOTemplateTest(common.HeatTestCase): 'Found a [%s] instead' % six.text_type, six.text_type(error)) - def test_translate_outputs_bad_description(self): - """Test translation of outputs into internal engine format.""" + def test_get_outputs_bad_description(self): + """Test get outputs with bad description name.""" hot_tpl = template_format.parse(''' heat_template_version: 2013-05-23 @@ -466,8 +466,8 @@ class HOTemplateTest(common.HeatTestCase): tmpl.__getitem__, tmpl.OUTPUTS) self.assertIn('Description', six.text_type(err)) - def test_translate_outputs_bad_value(self): - """Test translation of outputs into internal engine format.""" + def test_get_outputs_bad_value(self): + """Test get outputs with bad value name.""" hot_tpl = template_format.parse(''' heat_template_version: 2013-05-23 diff --git a/heat/tests/test_stack_collect_attributes.py b/heat/tests/test_stack_collect_attributes.py index b10069d12b..af0e348612 100644 --- a/heat/tests/test_stack_collect_attributes.py +++ b/heat/tests/test_stack_collect_attributes.py @@ -237,5 +237,8 @@ class DepAttrsTest(common.HeatTestCase): outputs = self.stack.outputs resources = six.itervalues(self.stack.resources) self.assertEqual(self.expected[res.name], - self.stack.get_dep_attrs(resources, outputs, - res.name)) + self.stack.get_dep_attrs( + resources, + outputs, + res.name, + self.stack.t.OUTPUT_VALUE)) diff --git a/heat/tests/test_stack_resource.py b/heat/tests/test_stack_resource.py index 25db23f7fe..c52b9c507c 100644 --- a/heat/tests/test_stack_resource.py +++ b/heat/tests/test_stack_resource.py @@ -33,9 +33,8 @@ from heat.tests import generic_resource as generic_rsrc from heat.tests import utils -ws_res_snippet = {"HeatTemplateFormatVersion": "2012-12-12", - "Type": "StackResourceType", - "metadata": { +ws_res_snippet = {"Type": "StackResourceType", + "Metadata": { "key": "value", "some": "more stuff"}} @@ -463,7 +462,10 @@ class StackResourceTest(StackResourceBaseTest): stack_resource.cfg.CONF.set_override('max_resources_per_stack', 2, enforce_type=True) tmpl = {'HeatTemplateFormatVersion': '2012-12-12', - 'Resources': [1]} + 'Resources': { + 'r': { + 'Type': 'OS::Heat::None' + }}} template = stack_resource.template.Template(tmpl) root_resources = mock.Mock(return_value=2) self.parent_resource.stack.total_resources = root_resources diff --git a/heat/tests/test_template.py b/heat/tests/test_template.py index a47ed41e1f..34cae03c35 100644 --- a/heat/tests/test_template.py +++ b/heat/tests/test_template.py @@ -363,6 +363,72 @@ class TestTemplateValidate(common.HeatTestCase): err = tmpl.validate() self.assertIsNone(err) + def test_get_resources_good(self): + """Test get resources successful.""" + + t = template_format.parse(''' + AWSTemplateFormatVersion: 2010-09-09 + Resources: + resource1: + Type: AWS::EC2::Instance + Properties: + property1: value1 + Metadata: + foo: bar + DependsOn: dummy + DeletionPolicy: dummy + UpdatePolicy: + foo: bar + ''') + + expected = {'resource1': {'Type': 'AWS::EC2::Instance', + 'Properties': {'property1': 'value1'}, + 'Metadata': {'foo': 'bar'}, + 'DependsOn': 'dummy', + 'DeletionPolicy': 'dummy', + 'UpdatePolicy': {'foo': 'bar'}}} + + tmpl = template.Template(t) + self.assertEqual(expected, tmpl[tmpl.RESOURCES]) + + def test_get_resources_bad_no_data(self): + """Test get resources without any mapping.""" + + t = template_format.parse(''' + AWSTemplateFormatVersion: 2010-09-09 + Resources: + resource1: + ''') + + tmpl = template.Template(t) + error = self.assertRaises(exception.StackValidationFailed, + tmpl.validate) + self.assertEqual('Each Resource must contain a Type key.', + six.text_type(error)) + + def test_get_resources_no_type(self): + """Test get resources with invalid key.""" + + t = template_format.parse(''' + AWSTemplateFormatVersion: 2010-09-09 + Resources: + resource1: + Properties: + property1: value1 + Metadata: + foo: bar + DependsOn: dummy + DeletionPolicy: dummy + UpdatePolicy: + foo: bar + ''') + + tmpl = template.Template(t) + error = self.assertRaises(exception.StackValidationFailed, + tmpl.validate) + self.assertEqual('Each Resource must contain a Type key.', + six.text_type(error)) + def test_template_validate_hot_check_t_digest(self): t = { 'heat_template_version': '2015-04-30',