From: Zane Bitter <zbitter@redhat.com>
Date: Tue, 3 Sep 2013 15:22:22 +0000 (+0200)
Subject: Make error message for InvalidTemplateReference sane
X-Git-Tag: 2014.1~80^2
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=b4130affa26ce17b311fea8a23dbe5c22f972ed6;p=openstack-build%2Fheat-build.git

Make error message for InvalidTemplateReference sane

Previously it was more or less incomprehensible. Now it will report the
non-existent resource the uesr tried to reference and the path (starting
with Resources) in the JSON document where the error occurred.

Change-Id: Id98eb4157df16674b8c8a9a190b82ee350c5386f
---

diff --git a/heat/common/exception.py b/heat/common/exception.py
index 1f92baf4..da1db5a5 100644
--- a/heat/common/exception.py
+++ b/heat/common/exception.py
@@ -236,7 +236,7 @@ class InvalidTemplateAttribute(HeatException):
 
 
 class InvalidTemplateReference(HeatException):
-    message = _("The specified reference (%(resource)s %(key)s)"
+    message = _("The specified reference \"%(resource)s\" (in %(key)s)"
                 " is incorrect.")
 
 
diff --git a/heat/engine/resource.py b/heat/engine/resource.py
index c5b3cb70..c3e7b36f 100644
--- a/heat/engine/resource.py
+++ b/heat/engine/resource.py
@@ -269,29 +269,29 @@ class Resource(object):
     def __str__(self):
         return '%s "%s"' % (self.__class__.__name__, self.name)
 
-    def _add_dependencies(self, deps, head, fragment):
+    def _add_dependencies(self, deps, path, fragment):
         if isinstance(fragment, dict):
             for key, value in fragment.items():
                 if key in ('DependsOn', 'Ref', 'Fn::GetAtt'):
                     if key == 'Fn::GetAtt':
-                        value, head = value
+                        value, att = value
 
                     try:
                         target = self.stack.resources[value]
                     except KeyError:
                         raise exception.InvalidTemplateReference(
                             resource=value,
-                            key=head)
+                            key=path)
                     if key == 'DependsOn' or target.strict_dependency:
                         deps += (self, target)
                 else:
-                    self._add_dependencies(deps, key, value)
+                    self._add_dependencies(deps, '%s.%s' % (path, key), value)
         elif isinstance(fragment, list):
-            for item in fragment:
-                self._add_dependencies(deps, head, item)
+            for index, item in enumerate(fragment):
+                self._add_dependencies(deps, '%s[%d]' % (path, index), item)
 
     def add_dependencies(self, deps):
-        self._add_dependencies(deps, None, self.t)
+        self._add_dependencies(deps, self.name, self.t)
         deps += (self, None)
 
     def required_by(self):
diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py
index 345f9030..144889dc 100644
--- a/heat/tests/test_resource.py
+++ b/heat/tests/test_resource.py
@@ -658,6 +658,23 @@ class ResourceDependenciesTest(HeatTestCase):
         self.assertIn(res, graph)
         self.assertIn(stack['foo'], graph[res])
 
+    def test_ref_fail(self):
+        tmpl = template.Template({
+            'Resources': {
+                'foo': {'Type': 'GenericResourceType'},
+                'bar': {
+                    'Type': 'ResourceWithPropsType',
+                    'Properties': {
+                        'Foo': {'Ref': 'baz'},
+                    }
+                }
+            }
+        })
+        ex = self.assertRaises(exception.InvalidTemplateReference,
+                               parser.Stack,
+                               None, 'test', tmpl)
+        self.assertIn('"baz" (in bar.Properties.Foo)', str(ex))
+
     def test_getatt(self):
         tmpl = template.Template({
             'Resources': {
@@ -724,6 +741,45 @@ class ResourceDependenciesTest(HeatTestCase):
         self.assertIn(res, graph)
         self.assertIn(stack['foo'], graph[res])
 
+    def test_getatt_fail(self):
+        tmpl = template.Template({
+            'Resources': {
+                'foo': {'Type': 'GenericResourceType'},
+                'bar': {
+                    'Type': 'ResourceWithPropsType',
+                    'Properties': {
+                        'Foo': {'Fn::GetAtt': ['baz', 'bar']},
+                    }
+                }
+            }
+        })
+        ex = self.assertRaises(exception.InvalidTemplateReference,
+                               parser.Stack,
+                               None, 'test', tmpl)
+        self.assertIn('"baz" (in bar.Properties.Foo)', str(ex))
+
+    def test_getatt_fail_nested_deep(self):
+        tmpl = template.Template({
+            'Resources': {
+                'foo': {'Type': 'GenericResourceType'},
+                'bar': {
+                    'Type': 'ResourceWithPropsType',
+                    'Properties': {
+                        'Foo': {'Fn::Join': [",", ["blarg",
+                                                   {'Fn::GetAtt': ['foo',
+                                                                   'bar']},
+                                                   "wibble",
+                                                   {'Fn::GetAtt': ['baz',
+                                                                   'bar']}]]},
+                    }
+                }
+            }
+        })
+        ex = self.assertRaises(exception.InvalidTemplateReference,
+                               parser.Stack,
+                               None, 'test', tmpl)
+        self.assertIn('"baz" (in bar.Properties.Foo.Fn::Join[1][3])', str(ex))
+
     def test_dependson(self):
         tmpl = template.Template({
             'Resources': {
@@ -743,6 +799,20 @@ class ResourceDependenciesTest(HeatTestCase):
         self.assertIn(res, graph)
         self.assertIn(stack['foo'], graph[res])
 
+    def test_dependson_fail(self):
+        tmpl = template.Template({
+            'Resources': {
+                'foo': {
+                    'Type': 'GenericResourceType',
+                    'DependsOn': 'wibble',
+                }
+            }
+        })
+        ex = self.assertRaises(exception.InvalidTemplateReference,
+                               parser.Stack,
+                               None, 'test', tmpl)
+        self.assertIn('"wibble" (in foo)', str(ex))
+
 
 class MetadataTest(HeatTestCase):
     def setUp(self):
diff --git a/heat/tests/test_vpc.py b/heat/tests/test_vpc.py
index 2e53758b..fff4731c 100644
--- a/heat/tests/test_vpc.py
+++ b/heat/tests/test_vpc.py
@@ -668,7 +668,7 @@ Resources:
             self.test_template_error)
         expected_exception = exception.InvalidTemplateReference(
             resource='INVALID-REF-IN-TEMPLATE',
-            key='GroupSet')
+            key='the_nic.Properties.GroupSet[0]')
 
         self.assertEquals(str(expected_exception), str(real_exception))