openstack
/
heat
Code Issues Proposed changes

Don't access `_stored_properties_data` 

It seems we're accessing _stored_properties_data in some
resources. We don't need to do that as self.properties
would have the older values.

Change-Id: I68187c8986d639e1db1ae7fb3f9c2da66c70edc6
This commit is contained in:
Rabi Mishra 2016-04-20 09:02:48 +05:30
parent 5b2a7b7c2b
commit a78303b791
9 changed files with 15 additions and 143 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
heat/engine/resources/openstack/cinder/volume.py
View File

@ -684,7 +684,7 @@ class CinderVolumeAttachment(vb.BaseVolumeAttachment):
# we still first detach the old resource so that
# self.resource_id is not replaced prematurely
volume_id = self.properties[self.VOLUME_ID]
server_id = self._stored_properties_data.get(self.INSTANCE_ID)
server_id = self.properties[self.INSTANCE_ID]
self.client_plugin('nova').detach_volume(server_id,
self.resource_id)
prg_detach = progress.VolumeDetachProgress(

2
heat/engine/resources/openstack/keystone/group.py
View File

@ -98,7 +98,7 @@ class KeystoneGroup(resource.Resource,
description = prop_diff.get(self.DESCRIPTION)
domain = (prop_diff.get(self.DOMAIN) or
self._stored_properties_data.get(self.DOMAIN))
self.properties[self.DOMAIN])
domain_id = self.client_plugin().get_domain_id(domain)
self.client().groups.update(

2
heat/engine/resources/openstack/keystone/project.py
View File

@ -108,7 +108,7 @@ class KeystoneProject(resource.Resource):
description = prop_diff.get(self.DESCRIPTION)
enabled = prop_diff.get(self.ENABLED)
domain = (prop_diff.get(self.DOMAIN) or
self._stored_properties_data.get(self.DOMAIN))
self.properties[self.DOMAIN])
domain_id = self.client_plugin().get_domain_id(domain)
self.client().projects.update(

11
heat/engine/resources/openstack/keystone/role_assignments.py
View File

@ -259,7 +259,7 @@ class KeystoneRoleAssignmentMixin(object):
(new_role_assignments,
removed_role_assignments) = self._find_differences(
prop_diff.get(self.ROLES),
self._stored_properties_data.get(self.ROLES))
self.properties[self.ROLES])
if len(new_role_assignments) > 0:
if user_id is not None:
@ -282,17 +282,16 @@ class KeystoneRoleAssignmentMixin(object):
removed_role_assignments)
def delete_assignment(self, user_id=None, group_id=None):
if self._stored_properties_data.get(self.ROLES) is not None:
self._stored_properties_data
if self.properties[self.ROLES] is not None:
if user_id is not None:
self._remove_role_assignments_from_user(
user_id,
(self._stored_properties_data.
get(self.ROLES)))
(self.properties[self.ROLES]))
elif group_id is not None:
self._remove_role_assignments_from_group(
group_id,
(self._stored_properties_data.
get(self.ROLES)))
(self.properties[self.ROLES]))
def validate_assignment_properties(self):
if self.properties.get(self.ROLES) is not None:

10
heat/engine/resources/openstack/keystone/user.py
View File

@ -214,8 +214,8 @@ class KeystoneUser(resource.Resource,
enabled = prop_diff.get(self.ENABLED)
email = prop_diff.get(self.EMAIL)
password = prop_diff.get(self.PASSWORD)
domain = (prop_diff.get(self.DOMAIN) or
self._stored_properties_data.get(self.DOMAIN))
domain = (prop_diff.get(self.DOMAIN)
or self.properties[self.DOMAIN])
default_project = prop_diff.get(self.DEFAULT_PROJECT)
@ -233,7 +233,7 @@ class KeystoneUser(resource.Resource,
if self.GROUPS in prop_diff:
(new_group_ids, removed_group_ids) = self._find_diff(
prop_diff[self.GROUPS],
self._stored_properties_data.get(self.GROUPS))
self.properties[self.GROUPS])
if new_group_ids:
self._add_user_to_groups(self.resource_id, new_group_ids)
@ -247,12 +247,12 @@ class KeystoneUser(resource.Resource,
def handle_delete(self):
if self.resource_id is not None:
with self.client_plugin().ignore_not_found:
if self._stored_properties_data.get(self.GROUPS) is not None:
if self.properties[self.GROUPS] is not None:
self._remove_user_from_groups(
self.resource_id,
[self.client_plugin().get_group_id(group)
for group in
self._stored_properties_data.get(self.GROUPS)])
self.properties[self.GROUPS]])
self.client().users.delete(self.resource_id)

4
heat/tests/openstack/keystone/test_group.py
View File

@ -238,7 +238,6 @@ class KeystoneGroupTest(common.HeatTestCase):
def test_group_handle_update(self):
self.test_group.resource_id = '477e8273-60a7-4c41-b683-fdb0bc7cd151'
self.test_group._stored_properties_data = dict(roles=None)
prop_diff = {group.KeystoneGroup.NAME: 'test_group_1_updated',
group.KeystoneGroup.DESCRIPTION: 'Test Group updated',
@ -262,7 +261,6 @@ class KeystoneGroupTest(common.HeatTestCase):
def test_group_handle_update_default(self):
self.test_group.resource_id = '477e8273-60a7-4c41-b683-fdb0bc7cd151'
self.test_group._stored_properties_data = dict(domain='default')
prop_diff = {group.KeystoneGroup.DESCRIPTION: 'Test Project updated'}
@ -281,7 +279,6 @@ class KeystoneGroupTest(common.HeatTestCase):
def test_group_handle_delete(self):
self.test_group.resource_id = '477e8273-60a7-4c41-b683-fdb0bc7cd151'
self.test_group._stored_properties_data = dict(roles=None)
self.groups.delete.return_value = None
self.assertIsNone(self.test_group.handle_delete())
@ -294,7 +291,6 @@ class KeystoneGroupTest(common.HeatTestCase):
self.assertIsNone(self.test_group.handle_delete())
def test_group_handle_delete_not_found(self):
self.test_group._stored_properties_data = dict(roles=None)
exc = self.keystoneclient.NotFound
self.groups.delete.side_effect = exc

2
heat/tests/openstack/keystone/test_project.py
View File

@ -310,7 +310,6 @@ class KeystoneProjectTest(common.HeatTestCase):
def test_project_handle_update_default(self):
self.test_project.resource_id = '477e8273-60a7-4c41-b683-fdb0bc7cd151'
self.test_project._stored_properties_data = dict(domain='default')
prop_diff = {project.KeystoneProject.DESCRIPTION:
'Test Project updated',
@ -332,7 +331,6 @@ class KeystoneProjectTest(common.HeatTestCase):
def test_project_handle_update_only_enabled(self):
self.test_project.resource_id = '477e8273-60a7-4c41-b683-fdb0bc7cd151'
self.test_project._stored_properties_data = dict(domain='default')
prop_diff = {project.KeystoneProject.ENABLED: False}
self.test_project.handle_update(json_snippet=None,

100
heat/tests/openstack/keystone/test_role_assignments.py
View File

@ -188,19 +188,6 @@ class KeystoneRoleAssignmentMixinTest(common.HeatTestCase):
project='project_1')
def test_role_assignment_update_user(self):
self.test_role_assignment._stored_properties_data = {
'roles': [
{
'role': 'role_1',
'project': 'project_1'
},
{
'role': 'role_1',
'domain': 'domain_1'
}
]
}
prop_diff = {
MixinClass.ROLES: [
{
@ -245,19 +232,6 @@ class KeystoneRoleAssignmentMixinTest(common.HeatTestCase):
project='project_1')
def test_role_assignment_update_group(self):
self.test_role_assignment._stored_properties_data = {
'roles': [
{
'role': 'role_1',
'project': 'project_1'
},
{
'role': 'role_1',
'domain': 'domain_1'
}
]
}
prop_diff = {
MixinClass.ROLES: [
{
@ -316,18 +290,6 @@ class KeystoneRoleAssignmentMixinTest(common.HeatTestCase):
self.assertEqual(0, self.roles.revoke.call_count)
def test_role_assignment_delete_user(self):
self.test_role_assignment._stored_properties_data = {
'roles': [
{
'role': 'role_1',
'project': 'project_1'
},
{
'role': 'role_1',
'domain': 'domain_1'
}
]
}
self.assertIsNone(self.test_role_assignment.delete_assignment(
user_id='user_1'))
@ -345,18 +307,6 @@ class KeystoneRoleAssignmentMixinTest(common.HeatTestCase):
project='project_1')
def test_role_assignment_delete_group(self):
self.test_role_assignment._stored_properties_data = {
'roles': [
{
'role': 'role_1',
'project': 'project_1'
},
{
'role': 'role_1',
'domain': 'domain_1'
}
]
}
self.assertIsNone(self.test_role_assignment.delete_assignment(
group_id='group_1'
))
@ -449,19 +399,6 @@ class KeystoneUserRoleAssignmentTest(common.HeatTestCase):
project='project_1')
def test_user_role_assignment_handle_update(self):
self.test_role_assignment._stored_properties_data = {
'roles': [
{
'role': 'role_1',
'project': 'project_1'
},
{
'role': 'role_1',
'domain': 'domain_1'
}
]
}
prop_diff = {
MixinClass.ROLES: [
{
@ -506,18 +443,6 @@ class KeystoneUserRoleAssignmentTest(common.HeatTestCase):
project='project_1')
def test_user_role_assignment_handle_delete(self):
self.test_role_assignment._stored_properties_data = {
'roles': [
{
'role': 'role_1',
'project': 'project_1'
},
{
'role': 'role_1',
'domain': 'domain_1'
}
]
}
self.assertIsNone(self.test_role_assignment.handle_delete())
# Remove role1-project1-domain1
@ -587,19 +512,6 @@ class KeystoneGroupRoleAssignmentTest(common.HeatTestCase):
project='project_1')
def test_group_role_assignment_handle_update(self):
self.test_role_assignment._stored_properties_data = {
'roles': [
{
'role': 'role_1',
'project': 'project_1'
},
{
'role': 'role_1',
'domain': 'domain_1'
}
]
}
prop_diff = {
MixinClass.ROLES: [
{
@ -644,18 +556,6 @@ class KeystoneGroupRoleAssignmentTest(common.HeatTestCase):
project='project_1')
def test_group_role_assignment_handle_delete(self):
self.test_role_assignment._stored_properties_data = {
'roles': [
{
'role': 'role_1',
'project': 'project_1'
},
{
'role': 'role_1',
'domain': 'domain_1'
}
]
}
self.assertIsNone(self.test_role_assignment.handle_delete())
# Remove role1-project1-domain1

25
heat/tests/openstack/keystone/test_user.py
View File

@ -184,13 +184,6 @@ class KeystoneUserTest(common.HeatTestCase):
def test_user_handle_update(self):
self.test_user.resource_id = '477e8273-60a7-4c41-b683-fdb0bc7cd151'
# Make the existing groups as group1 and group2
self.test_user._stored_properties_data = {
'groups': ['group1', 'group2'],
'domain': 'default'
}
# add new group group3 and remove group group2
prop_diff = {user.KeystoneUser.NAME: 'test_user_1_updated',
user.KeystoneUser.DESCRIPTION: 'Test User updated',
@ -207,8 +200,7 @@ class KeystoneUserTest(common.HeatTestCase):
# validate user update
self.users.update.assert_called_once_with(
user=self.test_user.resource_id,
domain=self.test_user._stored_properties_data[
user.KeystoneUser.DOMAIN],
domain=self.test_user.properties[user.KeystoneUser.DOMAIN],
name=prop_diff[user.KeystoneUser.NAME],
description=prop_diff[user.KeystoneUser.DESCRIPTION],
email=prop_diff[user.KeystoneUser.EMAIL],
@ -236,13 +228,6 @@ class KeystoneUserTest(common.HeatTestCase):
def test_user_handle_update_password_only(self):
self.test_user.resource_id = '477e8273-60a7-4c41-b683-fdb0bc7cd151'
# Make the existing groups as group1 and group2
self.test_user._stored_properties_data = {
'groups': ['group1', 'group2'],
'domain': 'default'
}
# Update the password only
prop_diff = {user.KeystoneUser.PASSWORD: 'passWORD'}
@ -253,8 +238,7 @@ class KeystoneUserTest(common.HeatTestCase):
# Validate user update
self.users.update.assert_called_once_with(
user=self.test_user.resource_id,
domain=self.test_user._stored_properties_data[
user.KeystoneUser.DOMAIN],
domain=self.test_user.properties[user.KeystoneUser.DOMAIN],
password=prop_diff[user.KeystoneUser.PASSWORD]
)
@ -264,10 +248,6 @@ class KeystoneUserTest(common.HeatTestCase):
def test_user_handle_delete(self):
self.test_user.resource_id = '477e8273-60a7-4c41-b683-fdb0bc7cd151'
self.test_user._stored_properties_data = {
'groups': ['group1', 'group2'],
'roles': None
}
self.users.delete.return_value = None
self.assertIsNone(self.test_user.handle_delete())
@ -286,7 +266,6 @@ class KeystoneUserTest(common.HeatTestCase):
self.assertIsNone(self.test_user.handle_delete())
def test_user_handle_delete_not_found(self):
self.test_user._stored_properties_data = dict(groups=None, roles=None)
exc = self.keystoneclient.NotFound
self.users.delete.side_effect = exc