Don't pass defaults as parameters for TemplateResource
The child_params calculation doesn't differentiate parameters provided via the properties vs those provided via the template defaults (defined in the nested stack template), which leads to confusing results when you look at the parameters for the nested stack, as the defaults are exposed as parameters passed in to the stack, when the parent didn't pass anything via properties. Closes-Bug: #1498107 Change-Id: Ic58cdcd15db09cd8d8d3d52fadfb68aa5709e7d8
This commit is contained in:
@@ -413,12 +413,11 @@ class Properties(collections.Mapping):
|
|||||||
if any(res.action == res.INIT for res in deps):
|
if any(res.action == res.INIT for res in deps):
|
||||||
return True
|
return True
|
||||||
|
|
||||||
def _get_property_value(self, key, validate=False):
|
def get_user_value(self, key, validate=False):
|
||||||
if key not in self:
|
if key not in self:
|
||||||
raise KeyError(_('Invalid Property %s') % key)
|
raise KeyError(_('Invalid Property %s') % key)
|
||||||
|
|
||||||
prop = self.props[key]
|
prop = self.props[key]
|
||||||
|
|
||||||
if key in self.data:
|
if key in self.data:
|
||||||
try:
|
try:
|
||||||
unresolved_value = self.data[key]
|
unresolved_value = self.data[key]
|
||||||
@@ -438,12 +437,18 @@ class Properties(collections.Mapping):
|
|||||||
# so handle this generically
|
# so handle this generically
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
raise ValueError(six.text_type(e))
|
raise ValueError(six.text_type(e))
|
||||||
|
|
||||||
|
def _get_property_value(self, key, validate=False):
|
||||||
|
if key not in self:
|
||||||
|
raise KeyError(_('Invalid Property %s') % key)
|
||||||
|
|
||||||
|
prop = self.props[key]
|
||||||
|
if key in self.data:
|
||||||
|
return self.get_user_value(key, validate)
|
||||||
elif prop.has_default():
|
elif prop.has_default():
|
||||||
return prop.get_value(None, validate)
|
return prop.get_value(None, validate)
|
||||||
elif prop.required():
|
elif prop.required():
|
||||||
raise ValueError(_('Property %s not assigned') % key)
|
raise ValueError(_('Property %s not assigned') % key)
|
||||||
else:
|
|
||||||
return None
|
|
||||||
|
|
||||||
def __getitem__(self, key):
|
def __getitem__(self, key):
|
||||||
return self._get_property_value(key)
|
return self._get_property_value(key)
|
||||||
|
|||||||
@@ -133,7 +133,7 @@ class TemplateResource(stack_resource.StackResource):
|
|||||||
continue
|
continue
|
||||||
|
|
||||||
try:
|
try:
|
||||||
val = self.properties[pname]
|
val = self.properties.get_user_value(pname)
|
||||||
except ValueError:
|
except ValueError:
|
||||||
if self.action == self.INIT:
|
if self.action == self.INIT:
|
||||||
prop = self.properties.props[pname]
|
prop = self.properties.props[pname]
|
||||||
|
|||||||
@@ -1038,9 +1038,25 @@ class PropertiesTest(common.HeatTestCase):
|
|||||||
def test_default_override(self):
|
def test_default_override(self):
|
||||||
self.assertEqual(42, self.props['default_override'])
|
self.assertEqual(42, self.props['default_override'])
|
||||||
|
|
||||||
|
def test_get_user_value(self):
|
||||||
|
self.assertIsNone(self.props.get_user_value('defaulted'))
|
||||||
|
self.assertEqual(42, self.props.get_user_value('default_override'))
|
||||||
|
|
||||||
|
def test_get_user_value_key_error(self):
|
||||||
|
ex = self.assertRaises(KeyError, self.props.get_user_value, 'foo')
|
||||||
|
# Note we have to use args here: https://bugs.python.org/issue2651
|
||||||
|
self.assertEqual('Invalid Property foo',
|
||||||
|
six.text_type(ex.args[0]))
|
||||||
|
|
||||||
def test_bad_key(self):
|
def test_bad_key(self):
|
||||||
self.assertEqual('wibble', self.props.get('foo', 'wibble'))
|
self.assertEqual('wibble', self.props.get('foo', 'wibble'))
|
||||||
|
|
||||||
|
def test_key_error(self):
|
||||||
|
ex = self.assertRaises(KeyError, self.props.__getitem__, 'foo')
|
||||||
|
# Note we have to use args here: https://bugs.python.org/issue2651
|
||||||
|
self.assertEqual('Invalid Property foo',
|
||||||
|
six.text_type(ex.args[0]))
|
||||||
|
|
||||||
def test_none_string(self):
|
def test_none_string(self):
|
||||||
schema = {'foo': {'Type': 'String'}}
|
schema = {'foo': {'Type': 'String'}}
|
||||||
props = properties.Properties(schema, {'foo': None})
|
props = properties.Properties(schema, {'foo': None})
|
||||||
|
|||||||
@@ -178,7 +178,8 @@ class ProviderTemplateTest(common.HeatTestCase):
|
|||||||
# verify Map conversion
|
# verify Map conversion
|
||||||
self.assertEqual(map_prop_val, converted_params.get("AMap"))
|
self.assertEqual(map_prop_val, converted_params.get("AMap"))
|
||||||
|
|
||||||
with mock.patch.object(properties.Properties, '__getitem__') as m_get:
|
with mock.patch.object(properties.Properties,
|
||||||
|
'get_user_value') as m_get:
|
||||||
m_get.side_effect = ValueError('boom')
|
m_get.side_effect = ValueError('boom')
|
||||||
|
|
||||||
# If the property doesn't exist on INIT, return default value
|
# If the property doesn't exist on INIT, return default value
|
||||||
@@ -984,7 +985,7 @@ class TemplateResourceCrudTest(common.HeatTestCase):
|
|||||||
self.res.handle_create()
|
self.res.handle_create()
|
||||||
|
|
||||||
self.res.create_with_template.assert_called_once_with(
|
self.res.create_with_template.assert_called_once_with(
|
||||||
self.provider, {'Foo': 'bar', 'Blarg': 'wibble'})
|
self.provider, {'Foo': 'bar'})
|
||||||
|
|
||||||
def test_handle_adopt(self):
|
def test_handle_adopt(self):
|
||||||
self.res.create_with_template = mock.Mock(return_value=None)
|
self.res.create_with_template = mock.Mock(return_value=None)
|
||||||
@@ -992,7 +993,7 @@ class TemplateResourceCrudTest(common.HeatTestCase):
|
|||||||
self.res.handle_adopt(resource_data={'resource_id': 'fred'})
|
self.res.handle_adopt(resource_data={'resource_id': 'fred'})
|
||||||
|
|
||||||
self.res.create_with_template.assert_called_once_with(
|
self.res.create_with_template.assert_called_once_with(
|
||||||
self.provider, {'Foo': 'bar', 'Blarg': 'wibble'},
|
self.provider, {'Foo': 'bar'},
|
||||||
adopt_data={'resource_id': 'fred'})
|
adopt_data={'resource_id': 'fred'})
|
||||||
|
|
||||||
def test_handle_update(self):
|
def test_handle_update(self):
|
||||||
@@ -1001,7 +1002,7 @@ class TemplateResourceCrudTest(common.HeatTestCase):
|
|||||||
self.res.handle_update(self.defn, None, None)
|
self.res.handle_update(self.defn, None, None)
|
||||||
|
|
||||||
self.res.update_with_template.assert_called_once_with(
|
self.res.update_with_template.assert_called_once_with(
|
||||||
self.provider, {'Foo': 'bar', 'Blarg': 'wibble'})
|
self.provider, {'Foo': 'bar'})
|
||||||
|
|
||||||
def test_handle_delete(self):
|
def test_handle_delete(self):
|
||||||
self.res.rpc_client = mock.MagicMock()
|
self.res.rpc_client = mock.MagicMock()
|
||||||
|
|||||||
Reference in New Issue
Block a user