From bffee054a1e9d2c3eb100bac319000013223d101 Mon Sep 17 00:00:00 2001
From: huangtianhua <huangtianhua@huawei.com>
Date: Fri, 15 May 2015 14:58:03 +0800
Subject: [PATCH] Should raise error if hook is invalid

If hooks of resource breakpoint are invalid, should
raise error message to user, and then user will know
what happen and why no pause on updation/creation.

Change-Id: I53822ed96fafdd373d20c45cacb5e33ba0306292
Closes-Bug: #1452636
---
 heat/common/exception.py       |  4 ++++
 heat/engine/environment.py     | 16 +++++++++++-----
 heat/tests/test_environment.py | 17 +++++++++++++++++
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/heat/common/exception.py b/heat/common/exception.py
index bccffa45ee..2eda70e164 100644
--- a/heat/common/exception.py
+++ b/heat/common/exception.py
@@ -333,6 +333,10 @@ class InvalidResourceType(HeatException):
     msg_fmt = _("%(message)s")
 
 
+class InvalidBreakPointHook(HeatException):
+    msg_fmt = _("%(message)s")
+
+
 class ResourceNotAvailable(HeatException):
     msg_fmt = _("The Resource (%(resource_name)s) is not available.")
 
diff --git a/heat/engine/environment.py b/heat/engine/environment.py
index b38d369407..678d89920e 100644
--- a/heat/engine/environment.py
+++ b/heat/engine/environment.py
@@ -43,14 +43,20 @@ def valid_hook_type(hook):
 
 
 def is_hook_definition(key, value):
+    is_valid_hook = False
     if key == 'hooks':
         if isinstance(value, six.string_types):
-            return valid_hook_type(value)
+            is_valid_hook = valid_hook_type(value)
         elif isinstance(value, collections.Sequence):
-            return all(valid_hook_type(hook) for hook in value)
-        else:
-            return False
-    return False
+            is_valid_hook = all(valid_hook_type(hook) for hook in value)
+
+        if not is_valid_hook:
+            msg = (_('Invalid hook type "%(value)s" for resource '
+                     'breakpoint, acceptable hook types are: %(types)s') %
+                   {'value': value, 'types': HOOK_TYPES})
+            raise exception.InvalidBreakPointHook(message=msg)
+
+    return is_valid_hook
 
 
 class ResourceInfo(object):
diff --git a/heat/tests/test_environment.py b/heat/tests/test_environment.py
index 4446b28e44..d182780f76 100644
--- a/heat/tests/test_environment.py
+++ b/heat/tests/test_environment.py
@@ -20,6 +20,7 @@ from oslo_config import cfg
 import six
 
 from heat.common import environment_format
+from heat.common import exception
 from heat.engine import environment
 from heat.engine import resources
 from heat.engine.resources.aws.ec2 import instance
@@ -747,6 +748,22 @@ class ResourceRegistryTest(common.HeatTestCase):
         self.assertEqual('pre-create',
                          resources['nested']['res']['hooks'])
 
+    def test_load_registry_invalid_hook_type(self):
+        resources = {
+            u'resources': {
+                u'a': {
+                    u'hooks': 'invalid-type',
+                }
+            }
+        }
+
+        registry = environment.ResourceRegistry(None, {})
+        msg = ('Invalid hook type "invalid-type" for resource breakpoint, '
+               'acceptable hook types are: (\'pre-create\', \'pre-update\')')
+        ex = self.assertRaises(exception.InvalidBreakPointHook,
+                               registry.load, {'resources': resources})
+        self.assertEqual(msg, six.text_type(ex))
+
 
 class HookMatchTest(common.HeatTestCase):