Validation failures now raise StackValidationFailed

This changes the way validation failures are presented
to heat-cfn as per bug 1072939

Change-Id: Iddaaf0c83c5d0df7c0143c6805096a83f8e5dc51

diff --git a/heat/api/aws/exception.py b/heat/api/aws/exception.py
index 2a58212f17c459bb06e8aa71ee14b9c573adf711..b4ece2b34545514fe8abcb7bec388fba431c0418 100644 (file)
--- a/heat/api/aws/exception.py
+++ b/heat/api/aws/exception.py
@@ -253,6 +253,7 @@ def map_remote_error(ex):
             'PhysicalResourceNotFound',
             'WatchRuleNotFound',
             'StackExists',
+            'StackValidationFailed',
         )
         denied_errors = ('Forbidden', 'NotAuthorized')
 

diff --git a/heat/api/openstack/v1/stacks.py b/heat/api/openstack/v1/stacks.py
index bf329b1fa696d99c2f4769ef40cb0f4bc6a829f8..162890a18bc36adc7c24afe74768258d6119c6d1 100644 (file)
--- a/heat/api/openstack/v1/stacks.py
+++ b/heat/api/openstack/v1/stacks.py
@@ -183,9 +183,6 @@ class StackController(object):
         except rpc_common.RemoteError as ex:
             return util.remote_error(ex)
 
-        if 'Description' in result:
-            raise exc.HTTPBadRequest(result['Description'])
-
         raise exc.HTTPCreated(location=util.make_url(req, result))
 
     @util.tenant_local
@@ -259,9 +256,6 @@ class StackController(object):
         except rpc_common.RemoteError as ex:
             return util.remote_error(ex)
 
-        if 'Description' in res:
-            raise exc.HTTPBadRequest(res['Description'])
-
         raise exc.HTTPAccepted()
 
     @util.identified_stack

diff --git a/heat/api/openstack/v1/util.py b/heat/api/openstack/v1/util.py
index ebe7c2cc53555ec8adce16c2e345c0be4a5f41f6..f3aebc1f59beb0812a2fbe4d5a299359b3e1b1e0 100644 (file)
--- a/heat/api/openstack/v1/util.py
+++ b/heat/api/openstack/v1/util.py
@@ -96,6 +96,7 @@ def remote_error(ex):
         'PhysicalResourceNotFound': exc.HTTPNotFound,
         'InvalidTenant': exc.HTTPForbidden,
         'StackExists': exc.HTTPConflict,
+        'StackValidationFailed': exc.HTTPBadRequest,
     }
 
     Exc = error_map.get(ex.exc_type, exc.HTTPInternalServerError)

diff --git a/heat/common/exception.py b/heat/common/exception.py
index 7d61379afb5e83d43e10e3330532e2e3e156ae39..00383559ecb9c621f2f1e196f9ce411a59882f8c 100644 (file)
--- a/heat/common/exception.py
+++ b/heat/common/exception.py
@@ -221,6 +221,10 @@ class StackExists(OpenstackException):
     message = _("The Stack (%(stack_name)s) already exists.")
 
 
+class StackValidationFailed(OpenstackException):
+    message = _("%(message)s")
+
+
 class ResourceNotFound(OpenstackException):
     message = _("The Resource (%(resource_name)s) could not be found "
                 "in Stack %(stack_name)s.")

diff --git a/heat/engine/parser.py b/heat/engine/parser.py
index da0157d2cb12040823ffa420dbacbf84e0620179..4d6274923aff15ad690cb64bd7d989e6dbf22ac1 100644 (file)
--- a/heat/engine/parser.py
+++ b/heat/engine/parser.py
@@ -28,6 +28,7 @@ from heat.engine.clients import Clients
 from heat.db import api as db_api
 
 from heat.openstack.common import log as logging
+from heat.common.exception import StackValidationFailed
 
 logger = logging.getLogger(__name__)
 
@@ -224,11 +225,9 @@ class Stack(object):
             try:
                 result = res.validate()
             except Exception as ex:
-                logger.exception('validate')
-                result = str(ex)
-
+                raise StackValidationFailed(message=str(ex))
             if result:
-                return 'Malformed Query Response %s' % result
+                raise StackValidationFailed(message=result)
 
     def state_set(self, new_status, reason):
         '''Update the stack state in the database'''

diff --git a/heat/engine/service.py b/heat/engine/service.py
index ee8bbcebe265b7198a43374559047d3ff8539050..a58b05126d457dabe65f9c3e2df2fde48a33dc35 100644 (file)
--- a/heat/engine/service.py
+++ b/heat/engine/service.py
@@ -216,9 +216,7 @@ class EngineService(service.Service):
         stack = parser.Stack(context, stack_name, tmpl, template_params,
                              **common_params)
 
-        response = stack.validate()
-        if response:
-            return {'Description': response}
+        stack.validate()
 
         stack_id = stack.store()
 
@@ -256,9 +254,7 @@ class EngineService(service.Service):
         updated_stack = parser.Stack(context, stack_name, tmpl,
                                      template_params, **common_params)
 
-        response = updated_stack.validate()
-        if response:
-            return {'Description': response}
+        updated_stack.validate()
 
         self._start_in_thread(db_stack.id, current_stack.update, updated_stack)
 

diff --git a/heat/tests/test_api_cfn_v1.py b/heat/tests/test_api_cfn_v1.py
index c8a1cbe89e89a8645da06909fc7fc691c2afecf5..911f4167451f16f59c207ddec1f1a968ca965409 100644 (file)
--- a/heat/tests/test_api_cfn_v1.py
+++ b/heat/tests/test_api_cfn_v1.py
@@ -589,7 +589,6 @@ class CfnStackControllerTest(unittest.TestCase):
 
         # Stub out the RPC call to the engine with a pre-canned response
         self.m.StubOutWithMock(rpc, 'call')
-        engine_err = {'Description': 'Something went wrong'}
 
         rpc.call(dummy_req.context, self.topic,
                  {'method': 'create_stack',
@@ -597,15 +596,17 @@ class CfnStackControllerTest(unittest.TestCase):
                   'template': template,
                   'params': engine_parms,
                   'args': engine_args},
-                  'version': self.api_version}, None).AndReturn(engine_err)
+                  'version': self.api_version}, None).AndRaise(
+                      rpc_common.RemoteError(
+                          'StackValidationFailed',
+                          'Something went wrong'))
 
         self.m.ReplayAll()
 
         result = self.controller.create(dummy_req)
 
-        expected = {'CreateStackResponse': {'CreateStackResult': engine_err}}
-
-        self.assertEqual(result, expected)
+        self.assertEqual(type(result),
+                         exception.HeatInvalidParameterValueError)
         self.m.VerifyAll()
 
     def test_update(self):

diff --git a/heat/tests/test_api_openstack_v1.py b/heat/tests/test_api_openstack_v1.py
index aaf0fe715bc227b5fb48c806cd2228236e8adda8..d1a5edebf17288df5816446250661973b35b2219 100644 (file)
--- a/heat/tests/test_api_openstack_v1.py
+++ b/heat/tests/test_api_openstack_v1.py
@@ -419,7 +419,6 @@ class StackControllerTest(ControllerTest, unittest.TestCase):
         req = self._post('/stacks', json.dumps(body))
 
         self.m.StubOutWithMock(rpc, 'call')
-        engine_err = {'Description': 'Something went wrong'}
         rpc.call(req.context, self.topic,
                  {'method': 'create_stack',
                   'args': {'stack_name': stack_name,
@@ -427,7 +426,9 @@ class StackControllerTest(ControllerTest, unittest.TestCase):
                            'params': parameters,
                            'args': {'timeout_mins': 30}},
                   'version': self.api_version},
-                 None).AndReturn(engine_err)
+                 None).AndRaise(rpc_common.RemoteError(
+                     'StackValidationFailed',
+                     'Something went wrong'))
         self.m.ReplayAll()
 
         self.assertRaises(webob.exc.HTTPBadRequest,

diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py
index ff71a25988fccd1c67af18393ffb2aa84c5780b8..81880972455803cfc0d83608113212cb2f0e8ec6 100644 (file)
--- a/heat/tests/test_engine_service.py
+++ b/heat/tests/test_engine_service.py
@@ -29,6 +29,7 @@ from heat.common import identifier
 from heat.common import template_format
 from heat.engine import parser
 from heat.engine import service
+from heat.engine.properties import Properties
 from heat.engine.resources import instance as instances
 from heat.engine import watchrule
 from heat.openstack.common import threadgroup
@@ -215,14 +216,16 @@ class stackServiceCreateUpdateDeleteTest(unittest.TestCase):
                      stack.t, stack.parameters).AndReturn(stack)
 
         self.m.StubOutWithMock(stack, 'validate')
-        error = 'fubar'
-        stack.validate().AndReturn(error)
+        stack.validate().AndRaise(exception.StackValidationFailed(
+            message='fubar'))
 
         self.m.ReplayAll()
 
-        result = self.man.create_stack(self.ctx, stack_name,
-                                       template, params, {})
-        self.assertEqual(result, {'Description': error})
+        self.assertRaises(
+            exception.StackValidationFailed,
+            self.man.create_stack,
+            self.ctx, stack_name,
+            template, params, {})
         self.m.VerifyAll()
 
     def test_stack_create_invalid_stack_name(self):
@@ -245,6 +248,34 @@ class stackServiceCreateUpdateDeleteTest(unittest.TestCase):
                           self.ctx, stack_name,
                           stack.t, {}, {})
 
+    def test_stack_validate(self):
+        stack_name = 'service_create_test_validate'
+        stack = get_wordpress_stack(stack_name, self.ctx)
+        setup_mocks(self.m, stack)
+
+        template = dict(stack.t)
+        template['Parameters']['KeyName']['Default'] = 'test'
+        resource = stack['WebServer']
+
+        self.m.ReplayAll()
+
+        resource.properties = Properties(
+            resource.properties_schema,
+            {
+                'ImageId': 'foo',
+                'KeyName': 'test',
+                'InstanceType': 'm1.large'
+            })
+        stack.validate()
+
+        resource.properties = Properties(
+            resource.properties_schema,
+            {
+                'KeyName': 'test',
+                'InstanceType': 'm1.large'
+            })
+        self.assertRaises(exception.StackValidationFailed, stack.validate)
+
     def test_stack_delete(self):
         stack_name = 'service_delete_test_stack'
         stack = get_wordpress_stack(stack_name, self.ctx)
@@ -336,14 +367,16 @@ class stackServiceCreateUpdateDeleteTest(unittest.TestCase):
                      stack.t, stack.parameters).AndReturn(stack)
 
         self.m.StubOutWithMock(stack, 'validate')
-        error = 'fubar'
-        stack.validate().AndReturn(error)
+        stack.validate().AndRaise(exception.StackValidationFailed(
+            message='fubar'))
 
         self.m.ReplayAll()
 
-        result = self.man.update_stack(self.ctx, old_stack.identifier(),
-                                       template, params, {})
-        self.assertEqual(result, {'Description': error})
+        self.assertRaises(
+            exception.StackValidationFailed,
+            self.man.update_stack,
+            self.ctx, old_stack.identifier(),
+            template, params, {})
         self.m.VerifyAll()
 
     def test_stack_update_nonexist(self):

diff --git a/heat/tests/test_properties.py b/heat/tests/test_properties.py
index 435ab78744d1f7573ef6eeac2d26e5fd3d0f05ed..ac7dd6d9c1119177d89a4cb60400345bceffc5cb 100644 (file)
--- a/heat/tests/test_properties.py
+++ b/heat/tests/test_properties.py
@@ -316,7 +316,7 @@ class PropertiesValidationTest(unittest.TestCase):
     def test_missing_required(self):
         schema = {'foo': {'Type': 'String', 'Required': True}}
         props = properties.Properties(schema, {})
-        self.assertNotEqual(props.validate(), None)
+        self.assertEqual(props.validate(), 'Property foo not assigned')
 
     def test_missing_unimplemented(self):
         schema = {'foo': {'Type': 'String', 'Implemented': False}}
@@ -326,7 +326,7 @@ class PropertiesValidationTest(unittest.TestCase):
     def test_present_unimplemented(self):
         schema = {'foo': {'Type': 'String', 'Implemented': False}}
         props = properties.Properties(schema, {'foo': 'bar'})
-        self.assertNotEqual(props.validate(), None)
+        self.assertEqual(props.validate(), 'foo Property not implemented yet')
 
     def test_missing(self):
         schema = {'foo': {'Type': 'String'}}
@@ -336,4 +336,4 @@ class PropertiesValidationTest(unittest.TestCase):
     def test_bad_data(self):
         schema = {'foo': {'Type': 'String'}}
         props = properties.Properties(schema, {'foo': 42})
-        self.assertNotEqual(props.validate(), None)
+        self.assertEqual(props.validate(), 'foo Value must be a string')