From a78303b7914b5a5f21870019724a0003756c2d41 Mon Sep 17 00:00:00 2001 From: Rabi Mishra Date: Wed, 20 Apr 2016 09:02:48 +0530 Subject: [PATCH] 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 --- .../resources/openstack/cinder/volume.py | 2 +- .../resources/openstack/keystone/group.py | 2 +- .../resources/openstack/keystone/project.py | 2 +- .../openstack/keystone/role_assignments.py | 11 +- .../resources/openstack/keystone/user.py | 10 +- heat/tests/openstack/keystone/test_group.py | 4 - heat/tests/openstack/keystone/test_project.py | 2 - .../keystone/test_role_assignments.py | 100 ------------------ heat/tests/openstack/keystone/test_user.py | 25 +---- 9 files changed, 15 insertions(+), 143 deletions(-) diff --git a/heat/engine/resources/openstack/cinder/volume.py b/heat/engine/resources/openstack/cinder/volume.py index 264688ae17..0091a47664 100644 --- a/heat/engine/resources/openstack/cinder/volume.py +++ b/heat/engine/resources/openstack/cinder/volume.py @@ -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( diff --git a/heat/engine/resources/openstack/keystone/group.py b/heat/engine/resources/openstack/keystone/group.py index c7d466dd98..0b0d6ae0e4 100644 --- a/heat/engine/resources/openstack/keystone/group.py +++ b/heat/engine/resources/openstack/keystone/group.py @@ -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( diff --git a/heat/engine/resources/openstack/keystone/project.py b/heat/engine/resources/openstack/keystone/project.py index 20950981b0..48bbd40d2e 100644 --- a/heat/engine/resources/openstack/keystone/project.py +++ b/heat/engine/resources/openstack/keystone/project.py @@ -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( diff --git a/heat/engine/resources/openstack/keystone/role_assignments.py b/heat/engine/resources/openstack/keystone/role_assignments.py index 65db09e788..e6bde79eb8 100644 --- a/heat/engine/resources/openstack/keystone/role_assignments.py +++ b/heat/engine/resources/openstack/keystone/role_assignments.py @@ -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: diff --git a/heat/engine/resources/openstack/keystone/user.py b/heat/engine/resources/openstack/keystone/user.py index bf687a1c7f..32a72efacd 100644 --- a/heat/engine/resources/openstack/keystone/user.py +++ b/heat/engine/resources/openstack/keystone/user.py @@ -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) diff --git a/heat/tests/openstack/keystone/test_group.py b/heat/tests/openstack/keystone/test_group.py index ee5d1a7ca8..53a478abb1 100644 --- a/heat/tests/openstack/keystone/test_group.py +++ b/heat/tests/openstack/keystone/test_group.py @@ -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 diff --git a/heat/tests/openstack/keystone/test_project.py b/heat/tests/openstack/keystone/test_project.py index bba8bd67ab..70a1330d53 100644 --- a/heat/tests/openstack/keystone/test_project.py +++ b/heat/tests/openstack/keystone/test_project.py @@ -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, diff --git a/heat/tests/openstack/keystone/test_role_assignments.py b/heat/tests/openstack/keystone/test_role_assignments.py index 8074235202..722b4015ae 100644 --- a/heat/tests/openstack/keystone/test_role_assignments.py +++ b/heat/tests/openstack/keystone/test_role_assignments.py @@ -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 diff --git a/heat/tests/openstack/keystone/test_user.py b/heat/tests/openstack/keystone/test_user.py index f2a1774322..14f75e430b 100644 --- a/heat/tests/openstack/keystone/test_user.py +++ b/heat/tests/openstack/keystone/test_user.py @@ -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