From 09ae0499b21d680425db1738f60ba12b5e16f3bc Mon Sep 17 00:00:00 2001 From: dahn Date: Tue, 19 Sep 2023 08:01:15 +0200 Subject: [PATCH] ldap trust map cleanup on domain delete (#7915) Co-authored-by: Wei Zhou --- .../cloudstack/ldap/LdapManagerImpl.java | 37 ++++++-- .../com/cloud/user/AccountManagerImpl.java | 48 +++++----- .../com/cloud/user/DomainManagerImpl.java | 90 ++++++++++--------- .../com/cloud/user/DomainManagerImplTest.java | 2 +- 4 files changed, 111 insertions(+), 66 deletions(-) diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java index 90b6e7f0bab..b5b67c0c0a5 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java @@ -28,6 +28,7 @@ import java.util.Map; import java.util.UUID; import com.cloud.user.AccountManager; +import com.cloud.user.DomainManager; import com.cloud.utils.component.ComponentLifecycleBase; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.api.LdapValidator; @@ -107,6 +108,13 @@ public class LdapManagerImpl extends ComponentLifecycleBase implements LdapManag super.configure(name, params); LOGGER.debug("Configuring LDAP Manager"); + addAccountRemovalListener(); + addDomainRemovalListener(); + + return true; + } + + private void addAccountRemovalListener() { messageBus.subscribe(AccountManager.MESSAGE_REMOVE_ACCOUNT_EVENT, new MessageSubscriber() { @Override public void onPublishMessage(String senderAddress, String subject, Object args) { @@ -115,18 +123,37 @@ public class LdapManagerImpl extends ComponentLifecycleBase implements LdapManag long domainId = account.getDomainId(); LdapTrustMapVO ldapTrustMapVO = _ldapTrustMapDao.findByAccount(domainId, account.getAccountId()); if (ldapTrustMapVO != null) { - String msg = String.format("Removing link between LDAP: %s - type: %s and account: %s on domain: %s", - ldapTrustMapVO.getName(), ldapTrustMapVO.getType().name(), account.getAccountId(), domainId); - LOGGER.debug(msg); - _ldapTrustMapDao.remove(ldapTrustMapVO.getId()); + removeTrustmap(ldapTrustMapVO); } } catch (final Exception e) { LOGGER.error("Caught exception while removing account linked to LDAP", e); } } }); + } - return true; + private void addDomainRemovalListener() { + messageBus.subscribe(DomainManager.MESSAGE_REMOVE_DOMAIN_EVENT, new MessageSubscriber() { + @Override + public void onPublishMessage(String senderAddress, String subject, Object args) { + try { + long domainId = ((DomainVO) args).getId(); + List ldapTrustMapVOs = _ldapTrustMapDao.searchByDomainId(domainId); + for (LdapTrustMapVO ldapTrustMapVO : ldapTrustMapVOs) { + removeTrustmap(ldapTrustMapVO); + } + } catch (final Exception e) { + LOGGER.error("Caught exception while removing trust-map for domain linked to LDAP", e); + } + } + }); + } + + private void removeTrustmap(LdapTrustMapVO ldapTrustMapVO) { + String msg = String.format("Removing link between LDAP: %s - type: %s and account: %s on domain: %s", + ldapTrustMapVO.getName(), ldapTrustMapVO.getType().name(), ldapTrustMapVO.getAccountId(), ldapTrustMapVO.getDomainId()); + LOGGER.debug(msg); + _ldapTrustMapDao.remove(ldapTrustMapVO.getId()); } @Override diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 99896dc9827..44711b20e51 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -1812,15 +1812,37 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M // If the user is a System user, return an error. We do not allow this AccountVO account = _accountDao.findById(accountId); - if (account == null || account.getRemoved() != null) { - if (account != null) { - s_logger.info("The account:" + account.getAccountName() + " is already removed"); - } + if (! isDeleteNeeded(account, accountId, caller)) { return true; } + // Account that manages project(s) can't be removed + List managedProjectIds = _projectAccountDao.listAdministratedProjectIds(accountId); + if (!managedProjectIds.isEmpty()) { + StringBuilder projectIds = new StringBuilder(); + for (Long projectId : managedProjectIds) { + projectIds.append(projectId).append(", "); + } + + throw new InvalidParameterValueException("The account id=" + accountId + " manages project(s) with ids " + projectIds + "and can't be removed"); + } + + CallContext.current().putContextParameter(Account.class, account.getUuid()); + + return deleteAccount(account, callerUserId, caller); + } + + private boolean isDeleteNeeded(AccountVO account, long accountId, Account caller) { + if (account == null) { + s_logger.info(String.format("The account, identified by id %d, doesn't exist", accountId )); + return false; + } + if (account.getRemoved() != null) { + s_logger.info("The account:" + account.getAccountName() + " is already removed"); + return false; + } // don't allow removing Project account - if (account == null || account.getType() == Account.Type.PROJECT) { + if (account.getType() == Account.Type.PROJECT) { throw new InvalidParameterValueException("The specified account does not exist in the system"); } @@ -1830,21 +1852,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (account.isDefault()) { throw new InvalidParameterValueException("The account is default and can't be removed"); } - - // Account that manages project(s) can't be removed - List managedProjectIds = _projectAccountDao.listAdministratedProjectIds(accountId); - if (!managedProjectIds.isEmpty()) { - StringBuilder projectIds = new StringBuilder(); - for (Long projectId : managedProjectIds) { - projectIds.append(projectId + ", "); - } - - throw new InvalidParameterValueException("The account id=" + accountId + " manages project(s) with ids " + projectIds + "and can't be removed"); - } - - CallContext.current().putContextParameter(Account.class, account.getUuid()); - - return deleteAccount(account, callerUserId, caller); + return true; } @Override diff --git a/server/src/main/java/com/cloud/user/DomainManagerImpl.java b/server/src/main/java/com/cloud/user/DomainManagerImpl.java index ad056655717..2dd356aca8e 100644 --- a/server/src/main/java/com/cloud/user/DomainManagerImpl.java +++ b/server/src/main/java/com/cloud/user/DomainManagerImpl.java @@ -344,16 +344,8 @@ public class DomainManagerImpl extends ManagerBase implements DomainManager, Dom @Override public boolean deleteDomain(DomainVO domain, Boolean cleanup) { - GlobalLock lock = getGlobalLock("AccountCleanup"); - if (lock == null) { - s_logger.debug("Couldn't get the global lock"); - return false; - } - - if (!lock.lock(30)) { - s_logger.debug("Couldn't lock the db"); - return false; - } + GlobalLock lock = getGlobalLock(); + if (lock == null) return false; try { // mark domain as inactive @@ -361,42 +353,60 @@ public class DomainManagerImpl extends ManagerBase implements DomainManager, Dom domain.setState(Domain.State.Inactive); _domainDao.update(domain.getId(), domain); - try { - long ownerId = domain.getAccountId(); - if (BooleanUtils.toBoolean(cleanup)) { - tryCleanupDomain(domain, ownerId); - } else { - removeDomainWithNoAccountsForCleanupNetworksOrDedicatedResources(domain); - } - - if (!_configMgr.releaseDomainSpecificVirtualRanges(domain.getId())) { - CloudRuntimeException e = new CloudRuntimeException("Can't delete the domain yet because failed to release domain specific virtual ip ranges"); - e.addProxyObject(domain.getUuid(), "domainId"); - throw e; - } else { - s_logger.debug("Domain specific Virtual IP ranges " + " are successfully released as a part of domain id=" + domain.getId() + " cleanup."); - } - - cleanupDomainDetails(domain.getId()); - cleanupDomainOfferings(domain.getId()); - annotationDao.removeByEntityType(AnnotationService.EntityType.DOMAIN.name(), domain.getUuid()); - CallContext.current().putContextParameter(Domain.class, domain.getUuid()); - return true; - } catch (Exception ex) { - s_logger.error("Exception deleting domain with id " + domain.getId(), ex); - if (ex instanceof CloudRuntimeException) { - rollbackDomainState(domain); - throw (CloudRuntimeException)ex; - } - else - return false; - } + return cleanDomain(domain, cleanup); } finally { lock.unlock(); } } + private GlobalLock getGlobalLock() { + GlobalLock lock = getGlobalLock("DomainCleanup"); + if (lock == null) { + s_logger.debug("Couldn't get the global lock"); + return null; + } + + if (!lock.lock(30)) { + s_logger.debug("Couldn't lock the db"); + return null; + } + return lock; + } + + private boolean cleanDomain(DomainVO domain, Boolean cleanup) { + try { + long ownerId = domain.getAccountId(); + if (BooleanUtils.toBoolean(cleanup)) { + tryCleanupDomain(domain, ownerId); + } else { + removeDomainWithNoAccountsForCleanupNetworksOrDedicatedResources(domain); + } + + if (!_configMgr.releaseDomainSpecificVirtualRanges(domain.getId())) { + CloudRuntimeException e = new CloudRuntimeException("Can't delete the domain yet because failed to release domain specific virtual ip ranges"); + e.addProxyObject(domain.getUuid(), "domainId"); + throw e; + } else { + s_logger.debug("Domain specific Virtual IP ranges " + " are successfully released as a part of domain id=" + domain.getId() + " cleanup."); + } + + cleanupDomainDetails(domain.getId()); + cleanupDomainOfferings(domain.getId()); + annotationDao.removeByEntityType(AnnotationService.EntityType.DOMAIN.name(), domain.getUuid()); + CallContext.current().putContextParameter(Domain.class, domain.getUuid()); + return true; + } catch (Exception ex) { + s_logger.error("Exception deleting domain with id " + domain.getId(), ex); + if (ex instanceof CloudRuntimeException) { + rollbackDomainState(domain); + throw (CloudRuntimeException)ex; + } + else + return false; + } + } + /** * Roll back domain state to Active * @param domain domain diff --git a/server/src/test/java/com/cloud/user/DomainManagerImplTest.java b/server/src/test/java/com/cloud/user/DomainManagerImplTest.java index 3b270f30e84..6b0c6121f6b 100644 --- a/server/src/test/java/com/cloud/user/DomainManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/DomainManagerImplTest.java @@ -136,7 +136,7 @@ public class DomainManagerImplTest { public void setup() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException { Mockito.doReturn(adminAccount).when(domainManager).getCaller(); - Mockito.doReturn(lock).when(domainManager).getGlobalLock("AccountCleanup"); + Mockito.doReturn(lock).when(domainManager).getGlobalLock("DomainCleanup"); Mockito.when(lock.lock(Mockito.anyInt())).thenReturn(true); Mockito.when(domainDaoMock.findById(DOMAIN_ID)).thenReturn(domain); Mockito.when(domain.getAccountId()).thenReturn(ACCOUNT_ID);