From 04e68f87ca0cdb94f47ca156d2363de4932ec911 Mon Sep 17 00:00:00 2001 From: Angus Salkeld Date: Wed, 25 Feb 2015 21:01:12 +1000 Subject: [PATCH] Add ability to pass 0, "", {} or [] as a parameter Normal issues with using values as booleans. Closes-Bug: #1423946 Closes-Bug: #1425238 Co-Authored-by: Sergey Kraynev Co-Authored-by: Angus Salkeld Change-Id: I9c6cd01ca722a65d3de6f28732ae07caefaa6cd8 --- heat/engine/parameters.py | 16 +++- heat/engine/resources/resource_group.py | 3 +- heat/tests/test_parameters.py | 20 +++++ heat/tests/test_resource_group.py | 75 ++++++++++--------- .../functional/test_resource_group.py | 75 +++++++++++++++++++ 5 files changed, 149 insertions(+), 40 deletions(-) diff --git a/heat/engine/parameters.py b/heat/engine/parameters.py index 27fbd44568..1e62aba2ed 100644 --- a/heat/engine/parameters.py +++ b/heat/engine/parameters.py @@ -250,7 +250,7 @@ class Parameter(object): def has_value(self): '''Parameter has a user or default value.''' - return self.user_value or self.has_default() + return self.user_value is not None or self.has_default() def hidden(self): ''' @@ -274,7 +274,9 @@ class Parameter(object): def default(self): '''Return the default value of the parameter.''' - return self.user_default or self.schema.default + if self.user_default is not None: + return self.user_default + return self.schema.default def set_default(self, value): self.user_default = value @@ -341,7 +343,10 @@ class CommaDelimitedListParam(Parameter, collections.Sequence): def __init__(self, name, schema, value=None): super(CommaDelimitedListParam, self).__init__(name, schema, value) if self.has_value(): - self.parsed = self.parse(self.user_value or self.default()) + if self.user_value is not None: + self.parsed = self.parse(self.user_value) + else: + self.parsed = self.parse(self.default()) else: self.parsed = [] @@ -389,7 +394,10 @@ class JsonParam(Parameter): def __init__(self, name, schema, value=None): super(JsonParam, self).__init__(name, schema, value) if self.has_value(): - self.parsed = self.parse(self.user_value or self.default()) + if self.user_value is not None: + self.parsed = self.parse(self.user_value) + else: + self.parsed = self.parse(self.default()) else: self.parsed = {} diff --git a/heat/engine/resources/resource_group.py b/heat/engine/resources/resource_group.py index b77ae7fea7..ce5f087fb9 100644 --- a/heat/engine/resources/resource_group.py +++ b/heat/engine/resources/resource_group.py @@ -282,7 +282,8 @@ class ResourceGroup(stack_resource.StackResource): res_def[self.RESOURCE_DEF_PROPERTIES] = {} if not include_all: resource_def_props = res_def[self.RESOURCE_DEF_PROPERTIES] - clean = dict((k, v) for k, v in resource_def_props.items() if v) + clean = dict((k, v) for k, v in resource_def_props.items() + if v is not None) res_def[self.RESOURCE_DEF_PROPERTIES] = clean return res_def diff --git a/heat/tests/test_parameters.py b/heat/tests/test_parameters.py index a28622a52f..573b5fb325 100644 --- a/heat/tests/test_parameters.py +++ b/heat/tests/test_parameters.py @@ -39,30 +39,35 @@ class ParameterTestCommon(common.HeatTestCase): value='test', expected='test', allowed_value=['foo'], + zero='', default='default')), ('type_number', dict(p_type='Number', inst=parameters.NumberParam, value=10, expected='10', allowed_value=[42], + zero=0, default=13)), ('type_list', dict(p_type='CommaDelimitedList', inst=parameters.CommaDelimitedListParam, value=['a', 'b', 'c'], expected='a,b,c', allowed_value=['foo'], + zero=[], default=['d', 'e', 'f'])), ('type_json', dict(p_type='Json', inst=parameters.JsonParam, value={'a': 1, 'b': '2'}, expected='{"a": 1, "b": "2"}', allowed_value=[{'foo': 'bar'}], + zero={}, default={'d': 1, 'e': 'f'})), ('type_boolean', dict(p_type='Boolean', inst=parameters.BooleanParam, value=True, expected='True', allowed_value=[False], + zero=False, default=True)) ] @@ -136,6 +141,21 @@ class ParameterTestCommon(common.HeatTestCase): self.assertFalse(p.hidden()) self.assertEqual(self.expected, str(p)) + def test_default_empty(self): + p = new_parameter('defaulted', {'Type': self.p_type, + 'Default': self.zero}) + self.assertTrue(p.has_default()) + self.assertEqual(self.zero, p.default()) + self.assertEqual(self.zero, p.value()) + + def test_default_no_empty_user_value_empty(self): + p = new_parameter('defaulted', {'Type': self.p_type, + 'Default': self.default}, + self.zero) + self.assertTrue(p.has_default()) + self.assertEqual(self.default, p.default()) + self.assertEqual(self.zero, p.value()) + class ParameterTestSpecific(testtools.TestCase): def test_new_bad_type(self): diff --git a/heat/tests/test_resource_group.py b/heat/tests/test_resource_group.py index 4f894c89c4..042a4036a0 100644 --- a/heat/tests/test_resource_group.py +++ b/heat/tests/test_resource_group.py @@ -143,41 +143,6 @@ class ResourceGroupTest(common.HeatTestCase): AttributeResource) self.m.StubOutWithMock(stackm.Stack, 'validate') - def test_build_resource_definition(self): - stack = utils.parse_stack(template) - snip = stack.t.resource_definitions(stack)['group1'] - resg = resource_group.ResourceGroup('test', snip, stack) - expect = { - "type": "dummy.resource", - "properties": { - "Foo": "Bar" - }, - } - self.assertEqual( - expect, resg._build_resource_definition()) - self.assertEqual( - expect, resg._build_resource_definition(include_all=True)) - - def test_build_resource_definition_include(self): - templ = copy.deepcopy(template) - res_def = templ["resources"]["group1"]["properties"]['resource_def'] - res_def['properties']['Foo'] = None - stack = utils.parse_stack(templ) - snip = stack.t.resource_definitions(stack)['group1'] - resg = resource_group.ResourceGroup('test', snip, stack) - expect = { - "type": "dummy.resource", - "properties": {} - } - self.assertEqual( - expect, resg._build_resource_definition()) - expect = { - "type": "dummy.resource", - "properties": {"Foo": None} - } - self.assertEqual( - expect, resg._build_resource_definition(include_all=True)) - def test_assemble_nested(self): """ Tests that the nested stack that implements the group is created @@ -467,6 +432,46 @@ class ResourceGroupBlackList(common.HeatTestCase): ','.join(self.expected)) +class ResourceGroupEmptyParams(common.HeatTestCase): + """This class tests ResourceGroup._build_resource_definition().""" + + scenarios = [ + ('non_empty', dict(value='Bar', expected={'Foo': 'Bar'}, + expected_include={'Foo': 'Bar'})), + ('empty_None', dict(value=None, expected={}, + expected_include={'Foo': None})), + ('empty_boolean', dict(value=False, expected={'Foo': False}, + expected_include={'Foo': False})), + ('empty_string', dict(value='', expected={'Foo': ''}, + expected_include={'Foo': ''})), + ('empty_number', dict(value=0, expected={'Foo': 0}, + expected_include={'Foo': 0})), + ('empty_json', dict(value={}, expected={'Foo': {}}, + expected_include={'Foo': {}})), + ('empty_list', dict(value=[], expected={'Foo': []}, + expected_include={'Foo': []})) + ] + + def test_definition(self): + templ = copy.deepcopy(template) + res_def = templ["resources"]["group1"]["properties"]['resource_def'] + res_def['properties']['Foo'] = self.value + stack = utils.parse_stack(templ) + snip = stack.t.resource_definitions(stack)['group1'] + resg = resource_group.ResourceGroup('test', snip, stack) + exp1 = { + "type": "dummy.resource", + "properties": self.expected, + } + exp2 = { + "type": "dummy.resource", + "properties": self.expected_include, + } + self.assertEqual(exp1, resg._build_resource_definition()) + self.assertEqual( + exp2, resg._build_resource_definition(include_all=True)) + + class ResourceGroupNameListTest(common.HeatTestCase): """This class tests ResourceGroup._resource_names().""" diff --git a/heat_integrationtests/functional/test_resource_group.py b/heat_integrationtests/functional/test_resource_group.py index 1811c29909..7da20a1d29 100644 --- a/heat_integrationtests/functional/test_resource_group.py +++ b/heat_integrationtests/functional/test_resource_group.py @@ -287,6 +287,81 @@ outputs: self.assertNotEqual(initial_rand, updated_rand) +class ResourceGroupTestNullParams(test.HeatIntegrationTest): + template = ''' +heat_template_version: 2013-05-23 +parameters: + param: + type: empty +resources: + random_group: + type: OS::Heat::ResourceGroup + properties: + count: 1 + resource_def: + type: My::RandomString + properties: + param: {get_param: param} +outputs: + val: + value: {get_attr: [random_group, val]} +''' + + nested_template_file = ''' +heat_template_version: 2013-05-23 +parameters: + param: + type: empty +outputs: + val: + value: {get_param: param} +''' + + scenarios = [ + ('string_empty', dict( + param='', + p_type='string', + )), + ('boolean_false', dict( + param=False, + p_type='boolean', + )), + ('number_zero', dict( + param=0, + p_type='number', + )), + ('comma_delimited_list', dict( + param=[], + p_type='comma_delimited_list', + )), + ('json_empty', dict( + param={}, + p_type='json', + )), + ] + + def setUp(self): + super(ResourceGroupTestNullParams, self).setUp() + self.client = self.orchestration_client + + def test_create_pass_zero_parameter(self): + templ = self.template.replace('type: empty', + 'type: %s' % self.p_type) + n_t_f = self.nested_template_file.replace('type: empty', + 'type: %s' % self.p_type) + files = {'provider.yaml': n_t_f} + env = {'resource_registry': + {'My::RandomString': 'provider.yaml'}} + stack_identifier = self.stack_create( + template=templ, + files=files, + environment=env, + parameters={'param': self.param} + ) + stack = self.client.stacks.get(stack_identifier) + self.assertEqual(self.param, self._stack_output(stack, 'val')[0]) + + class ResourceGroupAdoptTest(test.HeatIntegrationTest): """Prove that we can do resource group adopt."""