diff --git a/heat/common/heat_keystoneclient.py b/heat/common/heat_keystoneclient.py index 6bf1ec8b40..28566d641f 100644 --- a/heat/common/heat_keystoneclient.py +++ b/heat/common/heat_keystoneclient.py @@ -422,10 +422,8 @@ class KeystoneClientV3(object): return user.id - def _check_stack_domain_user(self, user_id, project_id, action): - """Sanity check that domain/project is correct.""" - user = self.domain_admin_client.users.get(user_id) - + @property + def stack_domain_id(self): if not self._stack_domain_id: if self._stack_domain_is_id: self._stack_domain_id = self.stack_domain @@ -433,8 +431,13 @@ class KeystoneClientV3(object): domain = self.domain_admin_client.domains.get( self.stack_domain) self._stack_domain_id = domain.id + return self._stack_domain_id - if user.domain_id != self._stack_domain_id: + def _check_stack_domain_user(self, user_id, project_id, action): + """Sanity check that domain/project is correct.""" + user = self.domain_admin_client.users.get(user_id) + + if user.domain_id != self.stack_domain_id: raise ValueError(_('User %s in invalid domain') % action) if user.default_project_id != project_id: raise ValueError(_('User %s in invalid project') % action) @@ -484,8 +487,25 @@ class KeystoneClientV3(object): LOG.warning(_('Falling back to legacy non-domain project, ' 'configure domain in heat.conf')) return + + # If stacks are created before configuring the heat domain, they + # exist in the default domain, in the user's project, which we + # do *not* want to delete! However, if the keystone v3cloudsample + # policy is used, it's possible that we'll get Forbidden when trying + # to get the project, so again we should do nothing try: - self.domain_admin_client.projects.delete(project=project_id) + project = self.domain_admin_client.projects.get(project=project_id) + except kc_exception.Forbidden: + LOG.warning(_('Unable to get details for project %s, not deleting') + % project_id) + return + + if project.domain_id != self.stack_domain_id: + LOG.warning(_('Not deleting non heat-domain project')) + return + + try: + project.delete() except kc_exception.NotFound: pass diff --git a/heat/tests/test_heatclient.py b/heat/tests/test_heatclient.py index af1591aa19..171bc84b08 100644 --- a/heat/tests/test_heatclient.py +++ b/heat/tests/test_heatclient.py @@ -1241,16 +1241,80 @@ class KeystoneClientTest(HeatTestCase): self._stub_domain_admin_client() self.mock_admin_client.projects = self.m.CreateMockAnything() - self.mock_admin_client.projects.delete(project='aprojectid') - self.mock_admin_client.projects.delete(project='aprojectid').AndRaise( - kc_exception.NotFound) + dummy = self.m.CreateMockAnything() + dummy.id = 'aproject123' + dummy.domain_id = 'adomain123' + dummy.delete().AndReturn(None) + self.mock_admin_client.projects.get(project='aprojectid').AndReturn( + dummy) self.m.ReplayAll() ctx = utils.dummy_context() ctx.trust_id = None heat_ks_client = heat_keystoneclient.KeystoneClient(ctx) heat_ks_client.delete_stack_domain_project(project_id='aprojectid') - # Second delete will raise ignored NotFound + + def test_delete_stack_domain_project_notfound(self): + + """Test the delete_stack_domain_project function.""" + + self._stub_domain_admin_client() + self.mock_admin_client.projects = self.m.CreateMockAnything() + dummy = self.m.CreateMockAnything() + dummy.id = 'aproject123' + dummy.domain_id = 'adomain123' + dummy.delete().AndRaise(kc_exception.NotFound) + self.mock_admin_client.projects.get(project='aprojectid').AndReturn( + dummy) + self.m.ReplayAll() + + ctx = utils.dummy_context() + ctx.trust_id = None + heat_ks_client = heat_keystoneclient.KeystoneClient(ctx) + heat_ks_client.delete_stack_domain_project(project_id='aprojectid') + + def test_delete_stack_domain_project_forbidden(self): + + """Test the delete_stack_domain_project function.""" + + self._stub_domain_admin_client() + self.mock_admin_client.projects = self.m.CreateMockAnything() + self.mock_admin_client.projects.get(project='aprojectid').AndRaise( + kc_exception.Forbidden) + self.m.ReplayAll() + + ctx = utils.dummy_context() + ctx.trust_id = None + heat_ks_client = heat_keystoneclient.KeystoneClient(ctx) + heat_ks_client.delete_stack_domain_project(project_id='aprojectid') + + def test_delete_stack_domain_project_wrongdomain(self): + + """Test the delete_stack_domain_project function.""" + + self._stub_domain_admin_client() + self.mock_admin_client.projects = self.m.CreateMockAnything() + dummy = self.m.CreateMockAnything() + dummy.id = 'aproject123' + dummy.domain_id = 'default' + self.mock_admin_client.projects.get(project='aprojectid').AndReturn( + dummy) + self.m.ReplayAll() + + ctx = utils.dummy_context() + ctx.trust_id = None + heat_ks_client = heat_keystoneclient.KeystoneClient(ctx) + heat_ks_client.delete_stack_domain_project(project_id='aprojectid') + + def test_delete_stack_domain_project_nodomain(self): + + """Test the delete_stack_domain_project function.""" + + self._clear_domain_override() + + ctx = utils.dummy_context() + ctx.trust_id = None + heat_ks_client = heat_keystoneclient.KeystoneClient(ctx) heat_ks_client.delete_stack_domain_project(project_id='aprojectid') def _stub_domain_user_pw_auth(self): @@ -1460,3 +1524,18 @@ class KeystoneClientTestDomainName(KeystoneClientTest): self._stub_domain_admin_client_domain_get() p = super(KeystoneClientTestDomainName, self) p.test_disable_stack_domain_user_error_domain() + + def test_delete_stack_domain_project(self): + self._stub_domain_admin_client_domain_get() + p = super(KeystoneClientTestDomainName, self) + p.test_delete_stack_domain_project() + + def test_delete_stack_domain_project_notfound(self): + self._stub_domain_admin_client_domain_get() + p = super(KeystoneClientTestDomainName, self) + p.test_delete_stack_domain_project_notfound() + + def test_delete_stack_domain_project_wrongdomain(self): + self._stub_domain_admin_client_domain_get() + p = super(KeystoneClientTestDomainName, self) + p.test_delete_stack_domain_project_wrongdomain()