From 0986d8eae36ec0f96939deb705286701d4264184 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 28 Aug 2023 13:27:27 +0200 Subject: [PATCH 1/9] ldap trust map cleanup on domain delete --- .../cloudstack/ldap/LdapManagerImpl.java | 31 ++++++- .../com/cloud/user/AccountManagerImpl.java | 42 +++++---- .../com/cloud/user/DomainManagerImpl.java | 88 +++++++++++-------- .../com/cloud/user/DomainManagerImplTest.java | 2 +- 4 files changed, 105 insertions(+), 58 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 90b6e7f0bab7..4837c8217b8d 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 @@ -27,7 +27,9 @@ import java.util.Map; import java.util.UUID; +import com.cloud.domain.Domain; 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 +109,13 @@ public boolean configure(String name, Map 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) { @@ -125,10 +134,28 @@ public void onPublishMessage(String senderAddress, String subject, Object args) } } }); - - 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 { + final Domain domain = domainDao.findByIdIncludingRemoved((Long) args); + long domainId = domain.getId(); + LdapTrustMapVO ldapTrustMapVO = _ldapTrustMapDao.findByDomainId(domainId); + if (ldapTrustMapVO != null) { + String msg = String.format("Removing link between LDAP: %s - type: %s on domain: %s", + ldapTrustMapVO.getName(), ldapTrustMapVO.getType().name(), domainId); + LOGGER.debug(msg); + _ldapTrustMapDao.remove(ldapTrustMapVO.getId()); + } + } catch (final Exception e) { + LOGGER.error("Caught exception while removing trust-map for domain linked to LDAP", e); + } + } + }); + } @Override public LdapConfigurationResponse addConfiguration(final LdapAddConfigurationCmd cmd) throws InvalidParameterValueException { return addConfigurationInternal(cmd.getHostname(),cmd.getPort(),cmd.getDomainId()); diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 99896dc98272..3b3a42ea4f48 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/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 (checkDeleteNeeded(account, 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 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 checkDeleteNeeded(AccountVO account, Account caller) { + if (account == null) { + s_logger.info("The account:" + account.getAccountName() + " doesn't exist"); + return true; + } + if (account.getRemoved() != null) { + s_logger.info("The account:" + account.getAccountName() + " is already removed"); + return true; + } + // 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 false; + } + @Override @ActionEvent(eventType = EventTypes.EVENT_ACCOUNT_ENABLE, eventDescription = "enabling account", async = true) public AccountVO enableAccount(String accountName, Long domainId, Long accountId) { diff --git a/server/src/main/java/com/cloud/user/DomainManagerImpl.java b/server/src/main/java/com/cloud/user/DomainManagerImpl.java index ad0566557178..ce8a5553ff4b 100644 --- a/server/src/main/java/com/cloud/user/DomainManagerImpl.java +++ b/server/src/main/java/com/cloud/user/DomainManagerImpl.java @@ -48,6 +48,7 @@ import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang.BooleanUtils; import org.apache.log4j.Logger; +import org.jetbrains.annotations.Nullable; import org.springframework.stereotype.Component; import com.cloud.api.query.dao.DiskOfferingJoinDao; @@ -344,56 +345,67 @@ 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(); + } + } + + @Nullable + 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; } } diff --git a/server/src/test/java/com/cloud/user/DomainManagerImplTest.java b/server/src/test/java/com/cloud/user/DomainManagerImplTest.java index 3b270f30e84a..6b0c6121f6bc 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); From 7c6d09f0143eb803b313356e668cc0d0000a39a4 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 28 Aug 2023 13:48:17 +0200 Subject: [PATCH 2/9] delete need check reversed --- .../main/java/com/cloud/user/AccountManagerImpl.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 3b3a42ea4f48..f982669f69ff 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -1812,7 +1812,7 @@ 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 (checkDeleteNeeded(account, caller)) { + if (! isDeleteNeeded(account, caller)) { return true; } @@ -1832,14 +1832,14 @@ public boolean deleteUserAccount(long accountId) { return deleteAccount(account, callerUserId, caller); } - private boolean checkDeleteNeeded(AccountVO account, Account caller) { + private boolean isDeleteNeeded(AccountVO account, Account caller) { if (account == null) { s_logger.info("The account:" + account.getAccountName() + " doesn't exist"); - return true; + return false; } if (account.getRemoved() != null) { s_logger.info("The account:" + account.getAccountName() + " is already removed"); - return true; + return false; } // don't allow removing Project account if (account.getType() == Account.Type.PROJECT) { @@ -1852,7 +1852,7 @@ private boolean checkDeleteNeeded(AccountVO account, Account caller) { if (account.isDefault()) { throw new InvalidParameterValueException("The account is default and can't be removed"); } - return false; + return true; } @Override From 81e67cb27d9987f878bc3552734b7178a2489611 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 28 Aug 2023 14:50:21 +0200 Subject: [PATCH 3/9] iterate through all candidates to remove --- .../org/apache/cloudstack/ldap/LdapManagerImpl.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 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 4837c8217b8d..6b5cf4e8a3e0 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 @@ -67,6 +67,7 @@ @Component public class LdapManagerImpl extends ComponentLifecycleBase implements LdapManager, LdapValidator { private static final Logger LOGGER = Logger.getLogger(LdapManagerImpl.class.getName()); + public static final String REMOVING_LDAP_LINK_ON_DOMAIN = "Removing link between LDAP: %s - type: %s and account: %s on domain: %s"; @Inject private LdapConfigurationDao _ldapConfigurationDao; @@ -124,7 +125,7 @@ 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", + String msg = String.format(REMOVING_LDAP_LINK_ON_DOMAIN, ldapTrustMapVO.getName(), ldapTrustMapVO.getType().name(), account.getAccountId(), domainId); LOGGER.debug(msg); _ldapTrustMapDao.remove(ldapTrustMapVO.getId()); @@ -143,10 +144,10 @@ public void onPublishMessage(String senderAddress, String subject, Object args) try { final Domain domain = domainDao.findByIdIncludingRemoved((Long) args); long domainId = domain.getId(); - LdapTrustMapVO ldapTrustMapVO = _ldapTrustMapDao.findByDomainId(domainId); - if (ldapTrustMapVO != null) { - String msg = String.format("Removing link between LDAP: %s - type: %s on domain: %s", - ldapTrustMapVO.getName(), ldapTrustMapVO.getType().name(), domainId); + List ldapTrustMapVOs = _ldapTrustMapDao.searchByDomainId(domainId); + for (LdapTrustMapVO ldapTrustMapVO : ldapTrustMapVOs) { + String msg = String.format(REMOVING_LDAP_LINK_ON_DOMAIN, + ldapTrustMapVO.getName(), ldapTrustMapVO.getType().name(), ldapTrustMapVO.getAccountId(), domainId); LOGGER.debug(msg); _ldapTrustMapDao.remove(ldapTrustMapVO.getId()); } From 12d5780aed1c5a3e6c5ecdc3960fe357491e5e8e Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 28 Aug 2023 16:14:20 +0200 Subject: [PATCH 4/9] id allready passed --- .../main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 6b5cf4e8a3e0..177286496fbd 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 @@ -142,8 +142,7 @@ private void addDomainRemovalListener() { @Override public void onPublishMessage(String senderAddress, String subject, Object args) { try { - final Domain domain = domainDao.findByIdIncludingRemoved((Long) args); - long domainId = domain.getId(); + long domainId = ((DomainVO) args).getId(); List ldapTrustMapVOs = _ldapTrustMapDao.searchByDomainId(domainId); for (LdapTrustMapVO ldapTrustMapVO : ldapTrustMapVOs) { String msg = String.format(REMOVING_LDAP_LINK_ON_DOMAIN, From d7fb26b6a6655962d23b30355fecdadda63116d4 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 28 Aug 2023 16:20:47 +0200 Subject: [PATCH 5/9] Update plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java --- .../main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java | 1 - 1 file changed, 1 deletion(-) 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 177286496fbd..f7b3bd1cad42 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 @@ -27,7 +27,6 @@ import java.util.Map; import java.util.UUID; -import com.cloud.domain.Domain; import com.cloud.user.AccountManager; import com.cloud.user.DomainManager; import com.cloud.utils.component.ComponentLifecycleBase; From 5d3f400ac299e86ddd02af3f90ced1681b3a5a3e Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 28 Aug 2023 17:38:18 +0200 Subject: [PATCH 6/9] unify logging and remove --- .../cloudstack/ldap/LdapManagerImpl.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 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 f7b3bd1cad42..2ab67008580b 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 @@ -124,10 +124,7 @@ 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_LDAP_LINK_ON_DOMAIN, - 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); @@ -144,10 +141,7 @@ public void onPublishMessage(String senderAddress, String subject, Object args) long domainId = ((DomainVO) args).getId(); List ldapTrustMapVOs = _ldapTrustMapDao.searchByDomainId(domainId); for (LdapTrustMapVO ldapTrustMapVO : ldapTrustMapVOs) { - String msg = String.format(REMOVING_LDAP_LINK_ON_DOMAIN, - ldapTrustMapVO.getName(), ldapTrustMapVO.getType().name(), ldapTrustMapVO.getAccountId(), domainId); - LOGGER.debug(msg); - _ldapTrustMapDao.remove(ldapTrustMapVO.getId()); + removeTrustmap(ldapTrustMapVO); } } catch (final Exception e) { LOGGER.error("Caught exception while removing trust-map for domain linked to LDAP", e); @@ -155,6 +149,14 @@ public void onPublishMessage(String senderAddress, String subject, Object args) } }); } + + private void removeTrustmap(LdapTrustMapVO ldapTrustMapVO) { + String msg = String.format(REMOVING_LDAP_LINK_ON_DOMAIN, + ldapTrustMapVO.getName(), ldapTrustMapVO.getType().name(), ldapTrustMapVO.getAccountId(), ldapTrustMapVO.getDomainId()); + LOGGER.debug(msg); + _ldapTrustMapDao.remove(ldapTrustMapVO.getId()); + } + @Override public LdapConfigurationResponse addConfiguration(final LdapAddConfigurationCmd cmd) throws InvalidParameterValueException { return addConfigurationInternal(cmd.getHostname(),cmd.getPort(),cmd.getDomainId()); From 9fa4267f20fc1ad38c8f3274fbe8d7c7b36c336e Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 28 Aug 2023 17:49:01 +0200 Subject: [PATCH 7/9] remove field --- .../main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 2ab67008580b..b5b67c0c0a53 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 @@ -66,7 +66,6 @@ @Component public class LdapManagerImpl extends ComponentLifecycleBase implements LdapManager, LdapValidator { private static final Logger LOGGER = Logger.getLogger(LdapManagerImpl.class.getName()); - public static final String REMOVING_LDAP_LINK_ON_DOMAIN = "Removing link between LDAP: %s - type: %s and account: %s on domain: %s"; @Inject private LdapConfigurationDao _ldapConfigurationDao; @@ -151,7 +150,7 @@ public void onPublishMessage(String senderAddress, String subject, Object args) } private void removeTrustmap(LdapTrustMapVO ldapTrustMapVO) { - String msg = String.format(REMOVING_LDAP_LINK_ON_DOMAIN, + 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()); From 06ea6202048610f894492409f48c753897267313 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 29 Aug 2023 09:10:03 +0200 Subject: [PATCH 8/9] npe --- server/src/main/java/com/cloud/user/AccountManagerImpl.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index f982669f69ff..44711b20e51e 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -1812,7 +1812,7 @@ 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 (! isDeleteNeeded(account, caller)) { + if (! isDeleteNeeded(account, accountId, caller)) { return true; } @@ -1832,9 +1832,9 @@ public boolean deleteUserAccount(long accountId) { return deleteAccount(account, callerUserId, caller); } - private boolean isDeleteNeeded(AccountVO account, Account caller) { + private boolean isDeleteNeeded(AccountVO account, long accountId, Account caller) { if (account == null) { - s_logger.info("The account:" + account.getAccountName() + " doesn't exist"); + s_logger.info(String.format("The account, identified by id %d, doesn't exist", accountId )); return false; } if (account.getRemoved() != null) { From 28f64da2009134af7fc164f77d4ee21f9229a737 Mon Sep 17 00:00:00 2001 From: dahn Date: Mon, 18 Sep 2023 15:49:59 +0200 Subject: [PATCH 9/9] Apply suggestions from code review --- server/src/main/java/com/cloud/user/DomainManagerImpl.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/main/java/com/cloud/user/DomainManagerImpl.java b/server/src/main/java/com/cloud/user/DomainManagerImpl.java index ce8a5553ff4b..2dd356aca8e2 100644 --- a/server/src/main/java/com/cloud/user/DomainManagerImpl.java +++ b/server/src/main/java/com/cloud/user/DomainManagerImpl.java @@ -48,7 +48,6 @@ import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang.BooleanUtils; import org.apache.log4j.Logger; -import org.jetbrains.annotations.Nullable; import org.springframework.stereotype.Component; import com.cloud.api.query.dao.DiskOfferingJoinDao; @@ -361,7 +360,6 @@ public boolean deleteDomain(DomainVO domain, Boolean cleanup) { } } - @Nullable private GlobalLock getGlobalLock() { GlobalLock lock = getGlobalLock("DomainCleanup"); if (lock == null) {