From 89a648d7ad9d10634c93e3f1ff8a9a8c72dba572 Mon Sep 17 00:00:00 2001 From: Liang Chen Date: Tue, 16 Jul 2013 23:24:16 +0800 Subject: [PATCH] Check missing parameters during stack create Add missing parameters checking logic to Parameter validation. It has to be optional, since template validation makes use of the same Parameter validation code. Fixes bug #1198670 Change-Id: I4c85ebf496b19999e47cf3838e6ca160aa194f20 --- heat/api/aws/exception.py | 1 + heat/api/openstack/v1/util.py | 1 + heat/engine/parameters.py | 19 ++++++--- heat/engine/resources/loadbalancer.py | 1 + heat/engine/service.py | 2 +- heat/tests/test_api_cfn_v1.py | 15 +++++++ heat/tests/test_api_openstack_v1.py | 13 ++++++ heat/tests/test_autoscaling.py | 61 ++++++++++++++------------- heat/tests/test_clouddatabase.py | 14 +++--- heat/tests/test_parameters.py | 34 +++++++++++---- heat/tests/test_parser.py | 2 +- 11 files changed, 108 insertions(+), 55 deletions(-) diff --git a/heat/api/aws/exception.py b/heat/api/aws/exception.py index a75463e2..c1090770 100644 --- a/heat/api/aws/exception.py +++ b/heat/api/aws/exception.py @@ -264,6 +264,7 @@ def map_remote_error(ex): 'StackValidationFailed', 'InvalidTemplateReference', 'UnknownUserParameter', + 'UserParameterMissing', ) denied_errors = ('Forbidden', 'NotAuthorized') already_exists_errors = ('StackExists') diff --git a/heat/api/openstack/v1/util.py b/heat/api/openstack/v1/util.py index b578e1cb..7e6d1491 100644 --- a/heat/api/openstack/v1/util.py +++ b/heat/api/openstack/v1/util.py @@ -86,6 +86,7 @@ def remote_error(ex): 'InvalidTemplateReference': exc.HTTPBadRequest, 'UnknownUserParameter': exc.HTTPBadRequest, 'MissingCredentialError': exc.HTTPBadRequest, + 'UserParameterMissing': exc.HTTPBadRequest, } Exc = error_map.get(ex.exc_type, exc.HTTPInternalServerError) diff --git a/heat/engine/parameters.py b/heat/engine/parameters.py index 64cc2ced..e21a3690 100644 --- a/heat/engine/parameters.py +++ b/heat/engine/parameters.py @@ -44,7 +44,7 @@ PSEUDO_PARAMETERS = ( class Parameter(object): '''A template parameter.''' - def __new__(cls, name, schema, value=None): + def __new__(cls, name, schema, value=None, validate_value=True): '''Create a new Parameter of the appropriate type.''' if cls is not Parameter: return super(Parameter, cls).__new__(cls) @@ -61,9 +61,9 @@ class Parameter(object): else: raise ValueError('Invalid Parameter type "%s"' % param_type) - return ParamClass(name, schema, value) + return ParamClass(name, schema, value, validate_value) - def __init__(self, name, schema, value=None): + def __init__(self, name, schema, value=None, validate_value=True): ''' Initialise the Parameter with a name, schema and optional user-supplied value. @@ -76,8 +76,11 @@ class Parameter(object): if self.has_default(): self._validate(self.default()) - if self.user_value is not None: - self._validate(self.user_value) + if validate_value: + if self.user_value is not None: + self._validate(self.user_value) + elif not self.has_default(): + raise exception.UserParameterMissing(key=self.name) def _error_msg(self, message): return '%s %s' % (self.name, self._constraint_error or message) @@ -272,7 +275,8 @@ class Parameters(collections.Mapping): The parameters of a stack, with type checking, defaults &c. specified by the stack's template. ''' - def __init__(self, stack_name, tmpl, user_params={}, stack_id=None): + def __init__(self, stack_name, tmpl, user_params={}, stack_id=None, + validate_value=True): ''' Create the parameter container for a stack from the stack name and template, optionally setting the user-supplied parameter values. @@ -298,7 +302,8 @@ class Parameters(collections.Mapping): 'ap-northeast-1']}) for name, schema in tmpl[template.PARAMETERS].iteritems(): - yield Parameter(name, schema, user_params.get(name)) + yield Parameter(name, schema, user_params.get(name), + validate_value) self.tmpl = tmpl self._validate(user_params) diff --git a/heat/engine/resources/loadbalancer.py b/heat/engine/resources/loadbalancer.py index 5b6b7cd0..f0c90b9c 100644 --- a/heat/engine/resources/loadbalancer.py +++ b/heat/engine/resources/loadbalancer.py @@ -346,6 +346,7 @@ class LoadBalancer(stack_resource.StackResource): param = {'KeyName': self.stack.parameters['KeyName']} except KeyError: del templ['Resources']['LB_instance']['Properties']['KeyName'] + del templ['Parameters']['KeyName'] param = {} return self.create_with_template(templ, param) diff --git a/heat/engine/service.py b/heat/engine/service.py index 9e80242c..e1de9a60 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -310,7 +310,7 @@ class EngineService(service.Service): except Exception as ex: return {'Error': str(ex)} - tmpl_params = parser.Parameters(None, tmpl) + tmpl_params = parser.Parameters(None, tmpl, validate_value=False) format_validate_parameter = lambda p: dict(p.schema) is_real_param = lambda p: p.name not in parameters.PSEUDO_PARAMETERS params = tmpl_params.map(format_validate_parameter, is_real_param) diff --git a/heat/tests/test_api_cfn_v1.py b/heat/tests/test_api_cfn_v1.py index df6a2aa8..57cfe8f2 100644 --- a/heat/tests/test_api_cfn_v1.py +++ b/heat/tests/test_api_cfn_v1.py @@ -555,6 +555,16 @@ class CfnStackControllerTest(HeatTestCase): 'args': engine_args}, 'version': self.api_version}, None ).AndRaise(rpc_common.RemoteError("UnknownUserParameter")) + rpc.call(dummy_req.context, self.topic, + {'namespace': None, + 'method': 'create_stack', + 'args': {'stack_name': stack_name, + 'template': template, + 'params': engine_parms, + 'files': {}, + 'args': engine_args}, + 'version': self.api_version}, None + ).AndRaise(rpc_common.RemoteError("UserParameterMissing")) self.m.ReplayAll() @@ -565,6 +575,11 @@ class CfnStackControllerTest(HeatTestCase): result = self.controller.create(dummy_req) + self.assertEqual(type(result), + exception.HeatInvalidParameterValueError) + + result = self.controller.create(dummy_req) + self.assertEqual(type(result), exception.HeatInvalidParameterValueError) diff --git a/heat/tests/test_api_openstack_v1.py b/heat/tests/test_api_openstack_v1.py index f6c413f5..2af4cbe8 100644 --- a/heat/tests/test_api_openstack_v1.py +++ b/heat/tests/test_api_openstack_v1.py @@ -507,6 +507,16 @@ class StackControllerTest(ControllerTest, HeatTestCase): 'args': {'timeout_mins': 30}}, 'version': self.api_version}, None).AndRaise(rpc_common.RemoteError("UnknownUserParameter")) + rpc.call(req.context, self.topic, + {'namespace': None, + 'method': 'create_stack', + 'args': {'stack_name': stack_name, + 'template': template, + 'params': {'parameters': parameters}, + 'files': {}, + 'args': {'timeout_mins': 30}}, + 'version': self.api_version}, + None).AndRaise(rpc_common.RemoteError("UserParameterMissing")) self.m.ReplayAll() @@ -516,6 +526,9 @@ class StackControllerTest(ControllerTest, HeatTestCase): self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, req, tenant_id=self.tenant, body=body) + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.create, + req, tenant_id=self.tenant, body=body) self.m.VerifyAll() diff --git a/heat/tests/test_autoscaling.py b/heat/tests/test_autoscaling.py index 7f9bb66d..ff239488 100644 --- a/heat/tests/test_autoscaling.py +++ b/heat/tests/test_autoscaling.py @@ -94,6 +94,7 @@ as_template = ''' class AutoScalingTest(HeatTestCase): dummy_instance_id = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' + params = {'KeyName': 'test'} def setUp(self): super(AutoScalingTest, self).setUp() @@ -167,7 +168,7 @@ class AutoScalingTest(HeatTestCase): properties = t['Resources']['WebServerGroup']['Properties'] properties['MinSize'] = '0' properties['MaxSize'] = '0' - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) now = timeutils.utcnow() self.m.ReplayAll() @@ -182,7 +183,7 @@ class AutoScalingTest(HeatTestCase): properties = t['Resources']['WebServerGroup']['Properties'] properties['MinSize'] = '1' properties['MaxSize'] = '1' - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(1) now = timeutils.utcnow() @@ -210,7 +211,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_update_replace(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(1) now = timeutils.utcnow() @@ -231,7 +232,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_suspend(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(1) now = timeutils.utcnow() @@ -262,7 +263,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_resume(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(1) now = timeutils.utcnow() @@ -297,7 +298,7 @@ class AutoScalingTest(HeatTestCase): t = template_format.parse(as_template) properties = t['Resources']['WebServerGroup']['Properties'] properties['DesiredCapacity'] = '2' - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(2) now = timeutils.utcnow() @@ -332,7 +333,7 @@ class AutoScalingTest(HeatTestCase): t = template_format.parse(as_template) properties = t['Resources']['WebServerGroup']['Properties'] properties['DesiredCapacity'] = '2' - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(2) now = timeutils.utcnow() @@ -367,7 +368,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_suspend_fail(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(1) now = timeutils.utcnow() @@ -398,7 +399,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_resume_fail(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(1) now = timeutils.utcnow() @@ -431,7 +432,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_create_error(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self.m.StubOutWithMock(instance.Instance, 'handle_create') self.m.StubOutWithMock(instance.Instance, 'check_create_complete') @@ -456,7 +457,7 @@ class AutoScalingTest(HeatTestCase): properties = t['Resources']['WebServerGroup']['Properties'] properties['MinSize'] = '1' properties['MaxSize'] = '3' - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(1) now = timeutils.utcnow() @@ -482,7 +483,7 @@ class AutoScalingTest(HeatTestCase): properties = t['Resources']['WebServerGroup']['Properties'] properties['MinSize'] = '1' properties['MaxSize'] = '3' - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(1) now = timeutils.utcnow() @@ -513,7 +514,7 @@ class AutoScalingTest(HeatTestCase): properties = t['Resources']['WebServerGroup']['Properties'] properties['MinSize'] = '1' properties['MaxSize'] = '3' - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(1) now = timeutils.utcnow() @@ -544,7 +545,7 @@ class AutoScalingTest(HeatTestCase): t = template_format.parse(as_template) properties = t['Resources']['WebServerGroup']['Properties'] properties['DesiredCapacity'] = '2' - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(2) now = timeutils.utcnow() @@ -572,7 +573,7 @@ class AutoScalingTest(HeatTestCase): t = template_format.parse(as_template) properties = t['Resources']['WebServerGroup']['Properties'] properties['Cooldown'] = '60' - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) self._stub_lb_reload(1) now = timeutils.utcnow() @@ -614,7 +615,7 @@ class AutoScalingTest(HeatTestCase): self._stub_meta_expected(now, 'ExactCapacity : 1') self._stub_create(1) self.m.ReplayAll() - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) rsrc = self.create_scaling_group(t, stack, 'WebServerGroup') self.assertEqual('WebServerGroup', rsrc.FnGetRefId()) @@ -628,7 +629,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_adjust(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # start with 3 properties = t['Resources']['WebServerGroup']['Properties'] @@ -669,7 +670,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_scale_up_failure(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group self._stub_lb_reload(1) @@ -698,7 +699,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_nochange(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group, 2 instances properties = t['Resources']['WebServerGroup']['Properties'] @@ -732,7 +733,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_percent(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group, 2 instances properties = t['Resources']['WebServerGroup']['Properties'] @@ -768,7 +769,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_cooldown_toosoon(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group, 2 instances, Cooldown 60s properties = t['Resources']['WebServerGroup']['Properties'] @@ -821,7 +822,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_cooldown_ok(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group, 2 instances, Cooldown 60s properties = t['Resources']['WebServerGroup']['Properties'] @@ -873,7 +874,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_group_cooldown_zero(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group, 2 instances, Cooldown 0 properties = t['Resources']['WebServerGroup']['Properties'] @@ -922,7 +923,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_policy_up(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group self._stub_lb_reload(1) @@ -950,7 +951,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_policy_down(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group, 2 instances properties = t['Resources']['WebServerGroup']['Properties'] @@ -979,7 +980,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_policy_cooldown_toosoon(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group self._stub_lb_reload(1) @@ -1030,7 +1031,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_policy_cooldown_ok(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group self._stub_lb_reload(1) @@ -1080,7 +1081,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_policy_cooldown_zero(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group self._stub_lb_reload(1) @@ -1130,7 +1131,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_policy_cooldown_none(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group self._stub_lb_reload(1) @@ -1182,7 +1183,7 @@ class AutoScalingTest(HeatTestCase): def test_scaling_policy_update(self): t = template_format.parse(as_template) - stack = parse_stack(t) + stack = parse_stack(t, params=self.params) # Create initial group self._stub_lb_reload(1) diff --git a/heat/tests/test_clouddatabase.py b/heat/tests/test_clouddatabase.py index 510d244c..e135b805 100644 --- a/heat/tests/test_clouddatabase.py +++ b/heat/tests/test_clouddatabase.py @@ -48,9 +48,9 @@ wp_template = ''' "MySqlCloudDB": { "Type": "Rackspace::Cloud::DBInstance", "Properties" : { - "InstanceName" : {"testsqlinstance"}, - "FlavorRef" : {"test-flavor"}, - "VolumeSize" : {"test-volume-size"}, + "InstanceName" : {"Ref": "InstanceName"}, + "FlavorRef" : {"Ref": "FlavorRef"}, + "VolumeSize" : {"Ref": VolumeSize}, "Users" : [{"name":"testuser", "password":"testpass123"}] , "Databases" : [{"name":"testdbonetwo"}] } @@ -87,13 +87,11 @@ class CloudDBInstanceTest(HeatTestCase): stack = parser.Stack(None, stack_name, template, - environment.Environment({'InstanceName': 'test'}), + environment.Environment({'InstanceName': 'Test', + 'FlavorRef': '1GB', + 'VolumeSize': '30'}), stack_id=uuidutils.generate_uuid()) - t['Resources']['MySqlCloudDB']['Properties']['InstanceName'] = 'Test' - t['Resources']['MySqlCloudDB']['Properties']['FlavorRef'] = '1GB' - t['Resources']['MySqlCloudDB']['Properties']['VolumeSize'] = '30' - if inject_property_error: # database name given in users list is not a valid database t['Resources']['MySqlCloudDB']['Properties']['Databases'] = \ diff --git a/heat/tests/test_parameters.py b/heat/tests/test_parameters.py index d8e4c5a1..0fe738b6 100644 --- a/heat/tests/test_parameters.py +++ b/heat/tests/test_parameters.py @@ -22,19 +22,20 @@ from heat.engine import parameters class ParameterTest(testtools.TestCase): def test_new_string(self): - p = parameters.Parameter('p', {'Type': 'String'}) + p = parameters.Parameter('p', {'Type': 'String'}, validate_value=False) self.assertTrue(isinstance(p, parameters.StringParam)) def test_new_number(self): - p = parameters.Parameter('p', {'Type': 'Number'}) + p = parameters.Parameter('p', {'Type': 'Number'}, validate_value=False) self.assertTrue(isinstance(p, parameters.NumberParam)) def test_new_list(self): - p = parameters.Parameter('p', {'Type': 'CommaDelimitedList'}) + p = parameters.Parameter('p', {'Type': 'CommaDelimitedList'}, + validate_value=False) self.assertTrue(isinstance(p, parameters.CommaDelimitedListParam)) def test_new_json(self): - p = parameters.Parameter('p', {'Type': 'Json'}) + p = parameters.Parameter('p', {'Type': 'Json'}, validate_value=False) self.assertTrue(isinstance(p, parameters.JsonParam)) def test_new_bad_type(self): @@ -101,11 +102,12 @@ class ParameterTest(testtools.TestCase): def test_description(self): description = 'Description of the parameter' p = parameters.Parameter('p', {'Type': 'String', - 'Description': description}) + 'Description': description}, + validate_value=False) self.assertEqual(p.description(), description) def test_no_description(self): - p = parameters.Parameter('p', {'Type': 'String'}) + p = parameters.Parameter('p', {'Type': 'String'}, validate_value=False) self.assertEqual(p.description(), '') def test_string_len_good(self): @@ -349,6 +351,12 @@ class ParameterTest(testtools.TestCase): else: self.fail("Value error not raised") + def test_missing_param(self): + '''Test missing user parameter.''' + self.assertRaises(exception.UserParameterMissing, + parameters.Parameter, 'p', + {'Type': 'String'}) + params_schema = json.loads('''{ "Parameters" : { @@ -379,10 +387,12 @@ class ParametersTest(testtools.TestCase): def test_schema_invariance(self): params1 = parameters.Parameters('test', params_schema, - {'Defaulted': 'wibble'}) + {'User': 'foo', + 'Defaulted': 'wibble'}) self.assertEqual(params1['Defaulted'], 'wibble') - params2 = parameters.Parameters('test', params_schema) + params2 = parameters.Parameters('test', params_schema, + {'User': 'foo'}) self.assertEqual(params2['Defaulted'], 'foobar') def test_to_dict(self): @@ -430,3 +440,11 @@ class ParametersTest(testtools.TestCase): 'test', params_schema, user_params) + + def test_missing_params(self): + user_params = {} + self.assertRaises(exception.UserParameterMissing, + parameters.Parameters, + 'test', + params_schema, + user_params) diff --git a/heat/tests/test_parser.py b/heat/tests/test_parser.py index d1d1f0d6..0e2f45b0 100644 --- a/heat/tests/test_parser.py +++ b/heat/tests/test_parser.py @@ -199,7 +199,7 @@ Mappings: def test_param_ref_missing(self): tmpl = {'Parameters': {'foo': {'Type': 'String', 'Required': True}}} - params = parameters.Parameters('test', tmpl) + params = parameters.Parameters('test', tmpl, validate_value=False) snippet = {"Ref": "foo"} self.assertRaises(exception.UserParameterMissing, parser.Template.resolve_param_refs, -- 2.45.2