ldap trust map cleanup on domain delete

plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java

@@ -28,6 +28,7 @@
 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 boolean configure(String name, Map<String, Object> params) throws Configu
         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 void onPublishMessage(String senderAddress, String subject, Object args)
                     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<LdapTrustMapVO> 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

server/src/main/java/com/cloud/user/AccountManagerImpl.java

@@ -1812,31 +1812,16 @@ public boolean deleteUserAccount(long accountId) {
         // 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;
         }
 
-        // don't allow removing Project account
-        if (account == null || account.getType() == Account.Type.PROJECT) {
-            throw new InvalidParameterValueException("The specified account does not exist in the system");
-        }
-
-        checkAccess(caller, null, true, account);
-
-        // don't allow to delete default account (system and admin)
-        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<Long> managedProjectIds = _projectAccountDao.listAdministratedProjectIds(accountId);
         if (!managedProjectIds.isEmpty()) {
             StringBuilder projectIds = new StringBuilder();
             for (Long projectId : managedProjectIds) {
-                projectIds.append(projectId + ", ");
+                projectIds.append(projectId).append(", ");
             }
 
             throw new InvalidParameterValueException("The account id=" + accountId + " manages project(s) with ids " + projectIds + "and can't be removed");
@@ -1847,6 +1832,29 @@ public boolean deleteUserAccount(long accountId) {
         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.getType() == Account.Type.PROJECT) {
+            throw new InvalidParameterValueException("The specified account does not exist in the system");
+        }
+
+        checkAccess(caller, null, true, account);
+
+        // don't allow to delete default account (system and admin)
+        if (account.isDefault()) {
+            throw new InvalidParameterValueException("The account is default and can't be removed");
+        }
+        return true;
+    }
+
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_ACCOUNT_ENABLE, eventDescription = "enabling account", async = true)
     public AccountVO enableAccount(String accountName, Long domainId, Long accountId) {

server/src/main/java/com/cloud/user/DomainManagerImpl.java

@@ -344,56 +344,66 @@ public boolean deleteDomain(long domainId, Boolean cleanup) {
 
     @Override
     public boolean deleteDomain(DomainVO domain, Boolean cleanup) {
-        GlobalLock lock = getGlobalLock("AccountCleanup");
+        GlobalLock lock = getGlobalLock();
+        if (lock == null) return false;
+
+        try {
+            // mark domain as inactive
+            s_logger.debug("Marking domain id=" + domain.getId() + " as " + Domain.State.Inactive + " before actually deleting it");
+            domain.setState(Domain.State.Inactive);
+            _domainDao.update(domain.getId(), domain);
+
+            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 false;
+            return null;
         }
 
         if (!lock.lock(30)) {
             s_logger.debug("Couldn't lock the db");
-            return false;
+            return null;
         }
+        return lock;
+    }
 
+    private boolean cleanDomain(DomainVO domain, Boolean cleanup) {
         try {
-            // mark domain as inactive
-            s_logger.debug("Marking domain id=" + domain.getId() + " as " + Domain.State.Inactive + " before actually deleting it");
-            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);
-                }
+            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.");
-                }
+            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;
+            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;
             }
-        }
-        finally {
-            lock.unlock();
+            else
+                return false;
         }
     }
 

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);