Skip to content

Commit 0986d8e

Browse files
DaanHooglandDaanHoogland
committed
ldap trust map cleanup on domain delete
1 parent 439d70f commit 0986d8e

4 files changed

Lines changed: 105 additions & 58 deletions

File tree

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

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@
2727
import java.util.Map;
2828
import java.util.UUID;
2929

30+
import com.cloud.domain.Domain;
3031
import com.cloud.user.AccountManager;
32+
import com.cloud.user.DomainManager;
3133
import com.cloud.utils.component.ComponentLifecycleBase;
3234
import com.cloud.utils.exception.CloudRuntimeException;
3335
import org.apache.cloudstack.api.LdapValidator;
@@ -107,6 +109,13 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
107109
super.configure(name, params);
108110
LOGGER.debug("Configuring LDAP Manager");
109111

112+
addAccountRemovalListener();
113+
addDomainRemovalListener();
114+
115+
return true;
116+
}
117+
118+
private void addAccountRemovalListener() {
110119
messageBus.subscribe(AccountManager.MESSAGE_REMOVE_ACCOUNT_EVENT, new MessageSubscriber() {
111120
@Override
112121
public void onPublishMessage(String senderAddress, String subject, Object args) {
@@ -125,10 +134,28 @@ public void onPublishMessage(String senderAddress, String subject, Object args)
125134
}
126135
}
127136
});
128-
129-
return true;
130137
}
131138

139+
private void addDomainRemovalListener() {
140+
messageBus.subscribe(DomainManager.MESSAGE_REMOVE_DOMAIN_EVENT, new MessageSubscriber() {
141+
@Override
142+
public void onPublishMessage(String senderAddress, String subject, Object args) {
143+
try {
144+
final Domain domain = domainDao.findByIdIncludingRemoved((Long) args);
145+
long domainId = domain.getId();
146+
LdapTrustMapVO ldapTrustMapVO = _ldapTrustMapDao.findByDomainId(domainId);
147+
if (ldapTrustMapVO != null) {
148+
String msg = String.format("Removing link between LDAP: %s - type: %s on domain: %s",
149+
ldapTrustMapVO.getName(), ldapTrustMapVO.getType().name(), domainId);
150+
LOGGER.debug(msg);
151+
_ldapTrustMapDao.remove(ldapTrustMapVO.getId());
152+
}
153+
} catch (final Exception e) {
154+
LOGGER.error("Caught exception while removing trust-map for domain linked to LDAP", e);
155+
}
156+
}
157+
});
158+
}
132159
@Override
133160
public LdapConfigurationResponse addConfiguration(final LdapAddConfigurationCmd cmd) throws InvalidParameterValueException {
134161
return addConfigurationInternal(cmd.getHostname(),cmd.getPort(),cmd.getDomainId());

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

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1812,31 +1812,16 @@ public boolean deleteUserAccount(long accountId) {
18121812
// If the user is a System user, return an error. We do not allow this
18131813
AccountVO account = _accountDao.findById(accountId);
18141814

1815-
if (account == null || account.getRemoved() != null) {
1816-
if (account != null) {
1817-
s_logger.info("The account:" + account.getAccountName() + " is already removed");
1818-
}
1815+
if (checkDeleteNeeded(account, caller)) {
18191816
return true;
18201817
}
18211818

1822-
// don't allow removing Project account
1823-
if (account == null || account.getType() == Account.Type.PROJECT) {
1824-
throw new InvalidParameterValueException("The specified account does not exist in the system");
1825-
}
1826-
1827-
checkAccess(caller, null, true, account);
1828-
1829-
// don't allow to delete default account (system and admin)
1830-
if (account.isDefault()) {
1831-
throw new InvalidParameterValueException("The account is default and can't be removed");
1832-
}
1833-
18341819
// Account that manages project(s) can't be removed
18351820
List<Long> managedProjectIds = _projectAccountDao.listAdministratedProjectIds(accountId);
18361821
if (!managedProjectIds.isEmpty()) {
18371822
StringBuilder projectIds = new StringBuilder();
18381823
for (Long projectId : managedProjectIds) {
1839-
projectIds.append(projectId + ", ");
1824+
projectIds.append(projectId).append(", ");
18401825
}
18411826

18421827
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) {
18471832
return deleteAccount(account, callerUserId, caller);
18481833
}
18491834

1835+
private boolean checkDeleteNeeded(AccountVO account, Account caller) {
1836+
if (account == null) {
1837+
s_logger.info("The account:" + account.getAccountName() + " doesn't exist");
1838+
return true;
1839+
}
1840+
if (account.getRemoved() != null) {
1841+
s_logger.info("The account:" + account.getAccountName() + " is already removed");
1842+
return true;
1843+
}
1844+
// don't allow removing Project account
1845+
if (account.getType() == Account.Type.PROJECT) {
1846+
throw new InvalidParameterValueException("The specified account does not exist in the system");
1847+
}
1848+
1849+
checkAccess(caller, null, true, account);
1850+
1851+
// don't allow to delete default account (system and admin)
1852+
if (account.isDefault()) {
1853+
throw new InvalidParameterValueException("The account is default and can't be removed");
1854+
}
1855+
return false;
1856+
}
1857+
18501858
@Override
18511859
@ActionEvent(eventType = EventTypes.EVENT_ACCOUNT_ENABLE, eventDescription = "enabling account", async = true)
18521860
public AccountVO enableAccount(String accountName, Long domainId, Long accountId) {

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

Lines changed: 50 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import org.apache.commons.collections.CollectionUtils;
4949
import org.apache.commons.lang.BooleanUtils;
5050
import org.apache.log4j.Logger;
51+
import org.jetbrains.annotations.Nullable;
5152
import org.springframework.stereotype.Component;
5253

5354
import com.cloud.api.query.dao.DiskOfferingJoinDao;
@@ -344,56 +345,67 @@ public boolean deleteDomain(long domainId, Boolean cleanup) {
344345

345346
@Override
346347
public boolean deleteDomain(DomainVO domain, Boolean cleanup) {
347-
GlobalLock lock = getGlobalLock("AccountCleanup");
348+
GlobalLock lock = getGlobalLock();
349+
if (lock == null) return false;
350+
351+
try {
352+
// mark domain as inactive
353+
s_logger.debug("Marking domain id=" + domain.getId() + " as " + Domain.State.Inactive + " before actually deleting it");
354+
domain.setState(Domain.State.Inactive);
355+
_domainDao.update(domain.getId(), domain);
356+
357+
return cleanDomain(domain, cleanup);
358+
}
359+
finally {
360+
lock.unlock();
361+
}
362+
}
363+
364+
@Nullable
365+
private GlobalLock getGlobalLock() {
366+
GlobalLock lock = getGlobalLock("DomainCleanup");
348367
if (lock == null) {
349368
s_logger.debug("Couldn't get the global lock");
350-
return false;
369+
return null;
351370
}
352371

353372
if (!lock.lock(30)) {
354373
s_logger.debug("Couldn't lock the db");
355-
return false;
374+
return null;
356375
}
376+
return lock;
377+
}
357378

379+
private boolean cleanDomain(DomainVO domain, Boolean cleanup) {
358380
try {
359-
// mark domain as inactive
360-
s_logger.debug("Marking domain id=" + domain.getId() + " as " + Domain.State.Inactive + " before actually deleting it");
361-
domain.setState(Domain.State.Inactive);
362-
_domainDao.update(domain.getId(), domain);
363-
364-
try {
365-
long ownerId = domain.getAccountId();
366-
if (BooleanUtils.toBoolean(cleanup)) {
367-
tryCleanupDomain(domain, ownerId);
368-
} else {
369-
removeDomainWithNoAccountsForCleanupNetworksOrDedicatedResources(domain);
370-
}
381+
long ownerId = domain.getAccountId();
382+
if (BooleanUtils.toBoolean(cleanup)) {
383+
tryCleanupDomain(domain, ownerId);
384+
} else {
385+
removeDomainWithNoAccountsForCleanupNetworksOrDedicatedResources(domain);
386+
}
371387

372-
if (!_configMgr.releaseDomainSpecificVirtualRanges(domain.getId())) {
373-
CloudRuntimeException e = new CloudRuntimeException("Can't delete the domain yet because failed to release domain specific virtual ip ranges");
374-
e.addProxyObject(domain.getUuid(), "domainId");
375-
throw e;
376-
} else {
377-
s_logger.debug("Domain specific Virtual IP ranges " + " are successfully released as a part of domain id=" + domain.getId() + " cleanup.");
378-
}
388+
if (!_configMgr.releaseDomainSpecificVirtualRanges(domain.getId())) {
389+
CloudRuntimeException e = new CloudRuntimeException("Can't delete the domain yet because failed to release domain specific virtual ip ranges");
390+
e.addProxyObject(domain.getUuid(), "domainId");
391+
throw e;
392+
} else {
393+
s_logger.debug("Domain specific Virtual IP ranges " + " are successfully released as a part of domain id=" + domain.getId() + " cleanup.");
394+
}
379395

380-
cleanupDomainDetails(domain.getId());
381-
cleanupDomainOfferings(domain.getId());
382-
annotationDao.removeByEntityType(AnnotationService.EntityType.DOMAIN.name(), domain.getUuid());
383-
CallContext.current().putContextParameter(Domain.class, domain.getUuid());
384-
return true;
385-
} catch (Exception ex) {
386-
s_logger.error("Exception deleting domain with id " + domain.getId(), ex);
387-
if (ex instanceof CloudRuntimeException) {
388-
rollbackDomainState(domain);
389-
throw (CloudRuntimeException)ex;
390-
}
391-
else
392-
return false;
396+
cleanupDomainDetails(domain.getId());
397+
cleanupDomainOfferings(domain.getId());
398+
annotationDao.removeByEntityType(AnnotationService.EntityType.DOMAIN.name(), domain.getUuid());
399+
CallContext.current().putContextParameter(Domain.class, domain.getUuid());
400+
return true;
401+
} catch (Exception ex) {
402+
s_logger.error("Exception deleting domain with id " + domain.getId(), ex);
403+
if (ex instanceof CloudRuntimeException) {
404+
rollbackDomainState(domain);
405+
throw (CloudRuntimeException)ex;
393406
}
394-
}
395-
finally {
396-
lock.unlock();
407+
else
408+
return false;
397409
}
398410
}
399411

‎server/src/test/java/com/cloud/user/DomainManagerImplTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ public class DomainManagerImplTest {
136136
public void setup() throws NoSuchFieldException, SecurityException,
137137
IllegalArgumentException, IllegalAccessException {
138138
Mockito.doReturn(adminAccount).when(domainManager).getCaller();
139-
Mockito.doReturn(lock).when(domainManager).getGlobalLock("AccountCleanup");
139+
Mockito.doReturn(lock).when(domainManager).getGlobalLock("DomainCleanup");
140140
Mockito.when(lock.lock(Mockito.anyInt())).thenReturn(true);
141141
Mockito.when(domainDaoMock.findById(DOMAIN_ID)).thenReturn(domain);
142142
Mockito.when(domain.getAccountId()).thenReturn(ACCOUNT_ID);

0 commit comments

Comments
 (0)