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

@@ -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}

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'),
     }
 

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 = {

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(),

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
-
-                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])
+            for attr, attr_value in six.iteritems(attrs):
+                cfn_attr = mapping[attr]
+                cfn_object[cfn_attr] = attr_value
+
+            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 {}

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
 
 

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:

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)

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

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))

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

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',