Handle outputs with an OutputDefinition class

This is the last remaining area of the code where we were slinging around
CloudFormation-formatted snippets of template. Replace it with a proper API
along the lines of ResourceDefinition.

The default implementation of the new template.Template.outputs() method is
equivalent to what we have previously done spread throughout the code, so
that third-party Template plugins will not be affected until they have had
time to write their own custom implementations.

Change-Id: Ib65dad6db55ae5dafab473bebba67e841ca9a984

heat/engine/api.py

@@ -170,37 +170,33 @@ def translate_filters(params):
     return params
 
 
-def format_stack_outputs(stack, outputs, resolve_value=False):
+def format_stack_outputs(outputs, resolve_value=False):
     """Return a representation of the given output template.
 
     Return a representation of the given output template for the given stack
     that matches the API output expectations.
     """
-    return [format_stack_output(stack, outputs,
+    return [format_stack_output(outputs[key], resolve_value=resolve_value)
-                                key, resolve_value=resolve_value)
             for key in outputs]
 
 
-def format_stack_output(stack, outputs, k, resolve_value=True):
+def format_stack_output(output_defn, resolve_value=True):
     result = {
-        rpc_api.OUTPUT_KEY: k,
+        rpc_api.OUTPUT_KEY: output_defn.name,
-        rpc_api.OUTPUT_DESCRIPTION: outputs[k].get(stack.t.OUTPUT_DESCRIPTION,
+        rpc_api.OUTPUT_DESCRIPTION: output_defn.description(),
-                                                   'No description given'),
     }
 
     if resolve_value:
+        value = None
         try:
-            value = stack.output(k)
+            value = output_defn.get_value()
         except Exception as ex:
             # We don't need error raising, just adding output_error to
             # resulting dict.
-            value = None
             result.update({rpc_api.OUTPUT_ERROR: six.text_type(ex)})
         finally:
             result.update({rpc_api.OUTPUT_VALUE: value})
 
-    if outputs[k].get('error_msg'):
-        result.update({rpc_api.OUTPUT_ERROR: outputs[k].get('error_msg')})
     return result
 
 
@@ -242,8 +238,7 @@ def format_stack(stack, preview=False, resolve_outputs=True):
 
     # allow users to view the outputs of stacks
     if stack.action != stack.DELETE and resolve_outputs:
-        info[rpc_api.STACK_OUTPUTS] = format_stack_outputs(stack,
+        info[rpc_api.STACK_OUTPUTS] = format_stack_outputs(stack.outputs,
-                                                           stack.outputs,
                                                            resolve_value=True)
 
     return info

heat/engine/output.py

@@ -0,0 +1,51 @@
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import six
+
+from heat.engine import function
+
+
+class OutputDefinition(object):
+    """A definition of a stack output, independent of any template format."""
+
+    def __init__(self, name, value, description=None):
+        self.name = name
+        self._value = value
+        self._resolved_value = None
+        self._description = description
+
+    def validate(self):
+        """Validate the output value without resolving it."""
+        function.validate(self._value)
+
+    def dep_attrs(self, resource_name):
+        """Iterate over attributes of a given resource that this references.
+
+        Return an iterator over dependent attributes for specified
+        resource_name in the output's value field.
+        """
+        return function.dep_attrs(self._value, resource_name)
+
+    def get_value(self):
+        """Resolve the value of the output."""
+        if self._resolved_value is None:
+            self._resolved_value = function.resolve(self._value)
+        return self._resolved_value
+
+    def description(self):
+        """Return a description of the output."""
+        if self._description is None:
+            return 'No description given'
+
+        return six.text_type(self._description)

heat/engine/resources/stack_resource.py

@@ -585,14 +585,12 @@ class StackResource(resource.Resource):
         stack = self.nested()
         if stack is None:
             return None
-        if op not in stack.outputs:
+
+        try:
+            return stack.outputs[op].get_value()
+        except (KeyError, Exception):
             raise exception.InvalidTemplateAttribute(resource=self.name,
                                                      key=op)
-        result = stack.output(op)
-        if result is None and stack.outputs[op].get('error_msg') is not None:
-            raise exception.InvalidTemplateAttribute(resource=self.name,
-                                                     key=op)
-        return result
 
     def _resolve_attribute(self, name):
         return self.get_output(name)

heat/engine/resources/template_resource.py

@@ -298,7 +298,7 @@ class TemplateResource(stack_resource.StackResource):
             return six.text_type(self.name)
 
         if 'OS::stack_id' in self.nested().outputs:
-            return self.nested().output('OS::stack_id')
+            return self.nested().outputs['OS::stack_id'].get_value()
 
         return self.nested().identifier().arn()
 

heat/engine/service.py

@@ -1308,7 +1308,7 @@ class EngineService(service.Service):
         s = self._get_stack(cntx, stack_identity)
         stack = parser.Stack.load(cntx, stack=s)
 
-        return api.format_stack_outputs(stack, stack.t[stack.t.OUTPUTS])
+        return api.format_stack_outputs(stack.outputs)
 
     @context.request_context
     def show_output(self, cntx, stack_identity, output_key):
@@ -1322,14 +1322,13 @@ class EngineService(service.Service):
         s = self._get_stack(cntx, stack_identity)
         stack = parser.Stack.load(cntx, stack=s)
 
-        outputs = stack.t[stack.t.OUTPUTS]
+        outputs = stack.outputs
 
         if output_key not in outputs:
             raise exception.NotFound(_('Specified output key %s not '
                                        'found.') % output_key)
-        output = stack.resolve_outputs_data({output_key: outputs[output_key]})
 
-        return api.format_stack_output(stack, output, output_key)
+        return api.format_stack_output(outputs[output_key])
 
     def _remote_call(self, cnxt, lock_engine_id, call, **kwargs):
         timeout = cfg.CONF.engine_life_check_timeout

heat/engine/stack.py

@@ -42,7 +42,6 @@ from heat.common import timeutils
 from heat.engine import dependencies
 from heat.engine import environment
 from heat.engine import event
-from heat.engine import function
 from heat.engine.notification import stack as notification
 from heat.engine import parameter_groups as param_groups
 from heat.engine import resource
@@ -297,8 +296,7 @@ class Stack(collections.Mapping):
     @property
     def outputs(self):
         if self._outputs is None:
-            self._outputs = self.resolve_outputs_data(self.t[self.t.OUTPUTS],
+            self._outputs = self.t.outputs(self)
-                                                      path=self.t.OUTPUTS)
         return self._outputs
 
     @property
@@ -458,8 +456,7 @@ class Stack(collections.Mapping):
         """
         attr_lists = itertools.chain((res.dep_attrs(resource_name)
                                       for res in resources),
-                                     (function.dep_attrs(
+                                     (out.dep_attrs(resource_name)
-                                         out.get(value_sec, ''), resource_name)
                                       for out in six.itervalues(outputs)))
         return set(itertools.chain.from_iterable(attr_lists))
 
@@ -834,32 +831,16 @@ class Stack(collections.Mapping):
             if result:
                 raise exception.StackValidationFailed(message=result)
 
-        for key, val in self.outputs.items():
+        for op_name, output in six.iteritems(self.outputs):
-            if not isinstance(val, collections.Mapping):
-                message = _('Outputs must contain Output. '
-                            'Found a [%s] instead') % type(val)
-                raise exception.StackValidationFailed(
-                    error='Output validation error',
-                    path=[self.t.OUTPUTS],
-                    message=message)
             try:
-                if not val or self.t.OUTPUT_VALUE not in val:
+                output.validate()
-                    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(self.t.OUTPUT_VALUE))
             except exception.StackValidationFailed as ex:
                 raise
             except AssertionError:
                 raise
             except Exception as ex:
                 raise exception.StackValidationFailed(
-                    error='Output validation error',
+                    error='Validation error in output "%s"' % op_name,
-                    path=[self.t.OUTPUTS, key,
-                          self.t.OUTPUT_VALUE],
                     message=six.text_type(ex))
 
     def requires_deferred_auth(self):
@@ -1900,16 +1881,6 @@ class Stack(collections.Mapping):
                                        action=self.RESTORE)
         updater()
 
-    @profiler.trace('Stack.output', hide_args=False)
-    def output(self, key):
-        """Get the value of the specified stack output."""
-        value = self.outputs[key].get(self.t.OUTPUT_VALUE, '')
-        try:
-            return function.resolve(value)
-        except Exception as ex:
-            self.outputs[key]['error_msg'] = six.text_type(ex)
-            return None
-
     def restart_resource(self, resource_name):
         """Restart the resource specified by resource_name.
 
@@ -1985,16 +1956,11 @@ class Stack(collections.Mapping):
 
     def resolve_static_data(self, snippet, path=''):
         warnings.warn('Stack.resolve_static_data() is deprecated and '
-                      'will be removed in the Ocata release. Use the '
+                      'will be removed in the Ocata release.',
-                      'Stack.resolve_outputs_data() instead.',
                       DeprecationWarning)
 
         return self.t.parse(self, snippet, path=path)
 
-    def resolve_outputs_data(self, outputs, path=''):
-        resolve_outputs = self.t.parse_outputs_conditions(outputs, self)
-        return self.t.parse(self, resolve_outputs, path=path)
-
     def reset_resource_attributes(self):
         # nothing is cached if no resources exist
         if not self._resources:

heat/engine/template.py

@@ -25,6 +25,7 @@ from heat.common import exception
 from heat.common.i18n import _
 from heat.engine import environment
 from heat.engine import function
+from heat.engine import output
 from heat.engine import template_files
 from heat.objects import raw_template as template_object
 
@@ -259,6 +260,36 @@ class Template(collections.Mapping):
         """Return a dictionary of resolved conditions."""
         return {}
 
+    def outputs(self, stack):
+        resolve_outputs = self.parse_outputs_conditions(self[self.OUTPUTS],
+                                                        stack)
+        outputs = self.parse(stack, resolve_outputs, path=self.OUTPUTS)
+
+        def get_outputs():
+            for key, val in outputs.items():
+                if not isinstance(val, collections.Mapping):
+                    message = _('Outputs must contain Output. '
+                                'Found a [%s] instead') % type(val)
+                    raise exception.StackValidationFailed(
+                        error='Output validation error',
+                        path=[self.OUTPUTS, key],
+                        message=message)
+
+                if self.OUTPUT_VALUE not in val:
+                    message = _('Each output must contain '
+                                'a %s key.') % self.OUTPUT_VALUE
+                    raise exception.StackValidationFailed(
+                        error='Output validation error',
+                        path=[self.OUTPUTS, key],
+                        message=message)
+
+                value_def = val[self.OUTPUT_VALUE]
+                description = val.get(self.OUTPUT_DESCRIPTION)
+
+                yield key, output.OutputDefinition(key, value_def, description)
+
+        return dict(get_outputs())
+
     @abc.abstractmethod
     def resource_definitions(self, stack):
         """Return a dictionary of ResourceDefinition objects."""

heat/tests/openstack/heat/test_none_resource.py

@@ -40,7 +40,7 @@ outputs:
         self.assertEqual(self.rsrc.COMPLETE, self.rsrc.status)
         self.assertEqual(self.stack.CREATE, self.stack.action)
         self.assertEqual(self.stack.COMPLETE, self.stack.status)
-        self.assertIsNone(self.stack.output('anything'))
+        self.assertIsNone(self.stack.outputs['anything'].get_value())
 
     def test_none_stack_create(self):
         self._create_none_stack()

heat/tests/openstack/heat/test_value.py

@@ -99,7 +99,7 @@ class TestValueSimple(TestValue):
             stack = self.create_stack(templ_dict, env)
             self.assertEqual(self.param1, stack['my_value'].FnGetAtt('value'))
             self.assertEqual(self.param1, stack['my_value2'].FnGetAtt('value'))
-            self.assertEqual(self.param1, stack.output('myout'))
+            self.assertEqual(self.param1, stack.outputs['myout'].get_value())
 
 
 class TestValueLessSimple(TestValue):

heat/tests/test_engine_api_utils.py

@@ -427,8 +427,7 @@ class FormatTest(common.HeatTestCase):
         stack.status = 'COMPLETE'
         stack['generic'].action = 'CREATE'
         stack['generic'].status = 'COMPLETE'
-        info = api.format_stack_outputs(stack, stack.outputs,
+        info = api.format_stack_outputs(stack.outputs, resolve_value=True)
-                                        resolve_value=True)
         expected = [{'description': 'No description given',
                      'output_error': 'The Referenced Attribute (generic Bar) '
                                      'is incorrect.',
@@ -463,7 +462,7 @@ class FormatTest(common.HeatTestCase):
         stack.status = 'COMPLETE'
         stack['generic'].action = 'CREATE'
         stack['generic'].status = 'COMPLETE'
-        info = api.format_stack_outputs(stack, stack.outputs)
+        info = api.format_stack_outputs(stack.outputs)
         expected = [{'description': 'No description given',
                      'output_key': 'incorrect_output'},
                     {'description': 'Good output',

heat/tests/test_engine_service.py

@@ -1054,7 +1054,7 @@ class StackServiceTest(common.HeatTestCase):
         self.patchobject(self.eng, '_get_stack')
         self.patchobject(parser.Stack, 'load', return_value=stack)
         self.patchobject(
-            stack, 'output',
+            stack.outputs['test'], 'get_value',
             side_effect=[exception.EntityNotFound(entity='one', name='name')])
 
         output = self.eng.show_output(self.ctx, mock.ANY, 'test')

heat/tests/test_hot.py

@@ -741,7 +741,8 @@ class HOTemplateTest(common.HeatTestCase):
         self.stack.create()
         self.assertEqual((parser.Stack.CREATE, parser.Stack.COMPLETE),
                          self.stack.state)
-        self.assertEqual('foo-success', self.stack.output('replaced'))
+        self.assertEqual('foo-success',
+                         self.stack.outputs['replaced'].get_value())
 
     def test_get_file(self):
         """Test get_file function."""

heat/tests/test_stack.py

@@ -34,6 +34,7 @@ from heat.engine.clients.os import keystone
 from heat.engine.clients.os import nova
 from heat.engine import environment
 from heat.engine import function
+from heat.engine import output
 from heat.engine import resource
 from heat.engine import scheduler
 from heat.engine import service
@@ -989,7 +990,8 @@ class StackTest(common.HeatTestCase):
         self.assertEqual('ADOPT', res.action)
         self.assertEqual((self.stack.ADOPT, self.stack.COMPLETE),
                          self.stack.state)
-        self.assertEqual('AResource', self.stack.output('TestOutput'))
+        self.assertEqual('AResource',
+                         self.stack.outputs['TestOutput'].get_value())
 
         loaded_stack = stack.Stack.load(self.ctx, self.stack.id)
         self.assertEqual({}, loaded_stack['AResource']._stored_properties_data)
@@ -1292,13 +1294,16 @@ class StackTest(common.HeatTestCase):
                 (rsrc.UPDATE, rsrc.FAILED),
                 (rsrc.UPDATE, rsrc.COMPLETE)):
             rsrc.state_set(action, status)
-            self.assertEqual('AResource', self.stack.output('TestOutput'))
+            self.stack._outputs = None
+            self.assertEqual('AResource',
+                             self.stack.outputs['TestOutput'].get_value())
         for action, status in (
                 (rsrc.DELETE, rsrc.IN_PROGRESS),
                 (rsrc.DELETE, rsrc.FAILED),
                 (rsrc.DELETE, rsrc.COMPLETE)):
             rsrc.state_set(action, status)
-            self.assertIsNone(self.stack.output('TestOutput'))
+            self.stack._outputs = None
+            self.assertIsNone(self.stack.outputs['TestOutput'].get_value())
 
     def test_resource_required_by(self):
         tmpl = {'HeatTemplateFormatVersion': '2012-12-12',
@@ -1704,7 +1709,8 @@ class StackTest(common.HeatTestCase):
         self.assertEqual('abc', self.stack['AResource'].properties['Foo'])
         # According _resolve_attribute method in GenericResource output
         # value will be equal with name AResource.
-        self.assertEqual('AResource', self.stack.output('Resource_attr'))
+        self.assertEqual('AResource',
+                         self.stack.outputs['Resource_attr'].get_value())
 
         self.stack.delete()
 
@@ -1730,10 +1736,11 @@ class StackTest(common.HeatTestCase):
         self.assertEqual((stack.Stack.CREATE, stack.Stack.COMPLETE),
                          self.stack.state)
 
-        self.assertIsNone(self.stack.output('Resource_attr'))
+        ex = self.assertRaises(exception.InvalidTemplateAttribute,
-        self.assertEqual('The Referenced Attribute (AResource Bar) is '
+                               self.stack.outputs['Resource_attr'].get_value)
-                         'incorrect.',
+        self.assertIn('The Referenced Attribute (AResource Bar) is '
-                         self.stack.outputs['Resource_attr']['error_msg'])
+                      'incorrect.',
+                      six.text_type(ex))
 
         self.stack.delete()
 
@@ -1835,8 +1842,7 @@ class StackTest(common.HeatTestCase):
         ex = self.assertRaises(exception.StackValidationFailed,
                                self.stack.validate)
 
-        self.assertEqual('Output validation error: '
+        self.assertEqual('Validation error in output "Resource_attr": '
-                         'Outputs.Resource_attr.Value: '
                          'The Referenced Attribute '
                          '(AResource Bar) is incorrect.',
                          six.text_type(ex))
@@ -1894,8 +1900,9 @@ class StackTest(common.HeatTestCase):
         ex = self.assertRaises(exception.StackValidationFailed,
                                self.stack.validate)
 
-        self.assertIn('Each Output must contain a Value key.',
+        self.assertIn('Each output must contain a Value key.',
                       six.text_type(ex))
+        self.assertIn('Outputs.Resource_attr', six.text_type(ex))
 
     def test_incorrect_outputs_cfn_empty_value(self):
         tmpl = template_format.parse("""
@@ -1952,6 +1959,7 @@ class StackTest(common.HeatTestCase):
         self.assertIn('Outputs must contain Output. '
                       'Found a [%s] instead' % six.text_type,
                       six.text_type(ex))
+        self.assertIn('Outputs.Resource_attr', six.text_type(ex))
 
     def test_prop_validate_value(self):
         tmpl = template_format.parse("""
@@ -2090,6 +2098,7 @@ class StackTest(common.HeatTestCase):
 
         self.assertIn('Outputs must contain Output. '
                       'Found a [%s] instead' % type([]), six.text_type(ex))
+        self.assertIn('Outputs.Resource_attr', six.text_type(ex))
 
     def test_incorrect_deletion_policy(self):
         tmpl = template_format.parse("""
@@ -2201,8 +2210,7 @@ class StackTest(common.HeatTestCase):
         ex = self.assertRaises(exception.StackValidationFailed,
                                self.stack.validate)
 
-        self.assertEqual('Output validation error: '
+        self.assertEqual('Validation error in output "resource_attr": '
-                         'outputs.resource_attr.value: '
                          'The Referenced Attribute '
                          '(AResource Bar) is incorrect.',
                          six.text_type(ex))
@@ -2717,22 +2725,11 @@ class StackTest(common.HeatTestCase):
             mock_dependency.validate.assert_called_once_with()
 
         stc = stack.Stack(self.ctx, utils.random_name(), self.tmpl)
-        stc._outputs = {'foo': {'Value': 'bar'}}
+        stc._outputs = {'foo': output.OutputDefinition('foo', 'bar')}
         func_val.side_effect = AssertionError(expected_msg)
         expected_exception = self.assertRaises(AssertionError, stc.validate)
         self.assertEqual(expected_msg, six.text_type(expected_exception))
 
-    def test_resolve_static_data_assertion_exception_rethrow(self):
-        tmpl = mock.MagicMock()
-        expected_message = 'Expected Assertion Error'
-        tmpl.parse.side_effect = AssertionError(expected_message)
-
-        stc = stack.Stack(self.ctx, utils.random_name(), tmpl)
-        expected_exception = self.assertRaises(AssertionError,
-                                               stc.resolve_outputs_data,
-                                               None)
-        self.assertEqual(expected_message, six.text_type(expected_exception))
-
     @mock.patch.object(update, 'StackUpdate')
     def test_update_task_exception(self, mock_stack_update):
         class RandomException(Exception):

heat/tests/test_stack_resource.py

@@ -22,6 +22,7 @@ import six
 
 from heat.common import exception
 from heat.common import template_format
+from heat.engine import output
 from heat.engine import resource
 from heat.engine.resources import stack_resource
 from heat.engine import stack as parser
@@ -624,8 +625,7 @@ class StackResourceAttrTest(StackResourceBaseTest):
         nested = self.m.CreateMockAnything()
         self.m.StubOutWithMock(stack_resource.StackResource, 'nested')
         stack_resource.StackResource.nested().AndReturn(nested)
-        nested.outputs = {"key": "value"}
+        nested.outputs = {"key": output.OutputDefinition("key", "value")}
-        nested.output('key').AndReturn("value")
         self.m.ReplayAll()
 
         self.assertEqual("value", self.parent_resource.get_output("key"))
@@ -649,8 +649,7 @@ class StackResourceAttrTest(StackResourceBaseTest):
         nested = self.m.CreateMockAnything()
         self.m.StubOutWithMock(stack_resource.StackResource, 'nested')
         stack_resource.StackResource.nested().AndReturn(nested)
-        nested.outputs = {'key': 'value'}
+        nested.outputs = {'key': output.OutputDefinition('key', 'value')}
-        nested.output('key').AndReturn('value')
         self.m.ReplayAll()
 
         self.assertEqual('value',
@@ -662,8 +661,8 @@ class StackResourceAttrTest(StackResourceBaseTest):
         nested = self.m.CreateMockAnything()
         self.m.StubOutWithMock(stack_resource.StackResource, 'nested')
         stack_resource.StackResource.nested().AndReturn(nested)
-        nested.outputs = {'key': {'a': 1, 'b': 2}}
+        nested.outputs = {'key': output.OutputDefinition('key',
-        nested.output('key').AndReturn({'a': 1, 'b': 2})
+                                                         {'a': 1, 'b': 2})}
         self.m.ReplayAll()
 
         self.assertEqual({'a': 1, 'b': 2},
@@ -675,8 +674,7 @@ class StackResourceAttrTest(StackResourceBaseTest):
         nested = self.m.CreateMockAnything()
         self.m.StubOutWithMock(stack_resource.StackResource, 'nested')
         stack_resource.StackResource.nested().AndReturn(nested)
-        nested.outputs = {"key": [1, 2, 3]}
+        nested.outputs = {'key': output.OutputDefinition('key', [1, 2, 3])}
-        nested.output('key').AndReturn([1, 2, 3])
         self.m.ReplayAll()
 
         self.assertEqual([1, 2, 3],

heat/tests/test_template.py

@@ -387,18 +387,16 @@ class TestTemplateConditionParser(common.HeatTestCase):
                           self.tmpl)
 
         # test condition name is invalid
-        stk.outputs['foo']['condition'] = 'invalid_cd'
+        self.tmpl.t['outputs']['foo']['condition'] = 'invalid_cd'
         ex = self.assertRaises(exception.InvalidConditionReference,
-                               self.tmpl.parse_outputs_conditions,
+                               lambda: stk.outputs)
-                               stk.outputs, stk)
         self.assertIn('Invalid condition "invalid_cd" '
                       '(in outputs.foo.condition)',
                       six.text_type(ex))
         # test condition name is not string
-        stk.outputs['foo']['condition'] = 222
+        self.tmpl.t['outputs']['foo']['condition'] = 222
         ex = self.assertRaises(exception.InvalidConditionReference,
-                               self.tmpl.parse_outputs_conditions,
+                               lambda: stk.outputs)
-                               stk.outputs, stk)
         self.assertIn('Invalid condition "222" (in outputs.foo.condition)',
                       six.text_type(ex))