diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java index 23541c2431e7..4fd3e729e0d2 100755 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import com.cloud.hypervisor.Hypervisor; import com.cloud.utils.Pair; @@ -192,4 +193,8 @@ List searchRemovedByRemoveDate(final Date startDate, final Date en int getVmCountByOfferingNotInDomain(Long serviceOfferingId, List domainIds); List listByIdsIncludingRemoved(List ids); + + List listDeleteProtectedVmsByAccountId(long accountId); + + List listDeleteProtectedVmsByDomainIds(Set domainIds); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java index 7ebf61366fe2..a38b6af3aa0b 100755 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java @@ -25,11 +25,13 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import javax.annotation.PostConstruct; import javax.inject.Inject; +import org.apache.cloudstack.api.ApiConstants; import org.apache.commons.collections.CollectionUtils; import org.springframework.stereotype.Component; @@ -106,6 +108,8 @@ public class VMInstanceDaoImpl extends GenericDaoBase implem protected SearchBuilder IdsPowerStateSelectSearch; GenericSearchBuilder CountByOfferingId; GenericSearchBuilder CountUserVmNotInDomain; + SearchBuilder DeleteProtectedVmSearchByAccount; + SearchBuilder DeleteProtectedVmSearchByDomainIds; @Inject ResourceTagDao tagsDao; @@ -368,6 +372,19 @@ protected void init() { CountUserVmNotInDomain.and("domainIdsNotIn", CountUserVmNotInDomain.entity().getDomainId(), Op.NIN); CountUserVmNotInDomain.done(); + DeleteProtectedVmSearchByAccount = createSearchBuilder(); + DeleteProtectedVmSearchByAccount.selectFields(DeleteProtectedVmSearchByAccount.entity().getUuid()); + DeleteProtectedVmSearchByAccount.and(ApiConstants.ACCOUNT_ID, DeleteProtectedVmSearchByAccount.entity().getAccountId(), Op.EQ); + DeleteProtectedVmSearchByAccount.and(ApiConstants.DELETE_PROTECTION, DeleteProtectedVmSearchByAccount.entity().isDeleteProtection(), Op.EQ); + DeleteProtectedVmSearchByAccount.and(ApiConstants.REMOVED, DeleteProtectedVmSearchByAccount.entity().getRemoved(), Op.NULL); + DeleteProtectedVmSearchByAccount.done(); + + DeleteProtectedVmSearchByDomainIds = createSearchBuilder(); + DeleteProtectedVmSearchByDomainIds.selectFields(DeleteProtectedVmSearchByDomainIds.entity().getUuid()); + DeleteProtectedVmSearchByDomainIds.and(ApiConstants.DOMAIN_IDS, DeleteProtectedVmSearchByDomainIds.entity().getDomainId(), Op.IN); + DeleteProtectedVmSearchByDomainIds.and(ApiConstants.DELETE_PROTECTION, DeleteProtectedVmSearchByDomainIds.entity().isDeleteProtection(), Op.EQ); + DeleteProtectedVmSearchByDomainIds.and(ApiConstants.REMOVED, DeleteProtectedVmSearchByDomainIds.entity().getRemoved(), Op.NULL); + DeleteProtectedVmSearchByDomainIds.done(); } @Override @@ -1296,4 +1313,22 @@ public List listByIdsIncludingRemoved(List ids) { sc.setParameters("ids", ids.toArray()); return listIncludingRemovedBy(sc); } + + @Override + public List listDeleteProtectedVmsByAccountId(long accountId) { + SearchCriteria sc = DeleteProtectedVmSearchByAccount.create(); + sc.setParameters(ApiConstants.ACCOUNT_ID, accountId); + sc.setParameters(ApiConstants.DELETE_PROTECTION, true); + Filter filter = new Filter(VMInstanceVO.class, null, false, 0L, 10L); + return listBy(sc, filter); + } + + @Override + public List listDeleteProtectedVmsByDomainIds(Set domainIds) { + SearchCriteria sc = DeleteProtectedVmSearchByDomainIds.create(); + sc.setParameters(ApiConstants.DOMAIN_IDS, domainIds.toArray()); + sc.setParameters(ApiConstants.DELETE_PROTECTION, true); + Filter filter = new Filter(VMInstanceVO.class, null, false, 0L, 10L); + return listBy(sc, filter); + } } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index bfe6a1b0a477..2ef866db0b12 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -2097,6 +2097,7 @@ public boolean deleteUserAccount(long accountId) { return true; } + validateNoDeleteProtectedVmsForAccount(account); checkIfAccountManagesProjects(accountId); verifyCallerPrivilegeForUserOrAccountOperations(account); @@ -2138,6 +2139,23 @@ protected boolean isDeleteNeeded(AccountVO account, long accountId, Account call return true; } + private void validateNoDeleteProtectedVmsForAccount(Account account) { + long accountId = account.getId(); + List deleteProtectedVms = _vmDao.listDeleteProtectedVmsByAccountId(accountId); + if (CollectionUtils.isEmpty(deleteProtectedVms)) { + return; + } + + if (logger.isDebugEnabled()) { + List vmUuids = deleteProtectedVms.stream().map(VMInstanceVO::getUuid).collect(Collectors.toList()); + logger.debug("Cannot delete Account {}, delete protection enabled for Instances: {}", account, vmUuids); + } + + throw new InvalidParameterValueException( + String.format("Cannot delete Account '%s'. One or more Instances have delete protection enabled.", + account.getAccountName())); + } + @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 28f9bd3ab391..a590c5ad833d 100644 --- a/server/src/main/java/com/cloud/user/DomainManagerImpl.java +++ b/server/src/main/java/com/cloud/user/DomainManagerImpl.java @@ -25,6 +25,7 @@ import java.util.UUID; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import javax.inject.Inject; @@ -104,6 +105,7 @@ import com.cloud.utils.net.NetUtils; import com.cloud.vm.ReservationContext; import com.cloud.vm.ReservationContextImpl; +import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.VMInstanceDao; import org.apache.commons.lang3.StringUtils; @@ -364,6 +366,8 @@ public boolean deleteDomain(long domainId, Boolean cleanup) { } _accountMgr.checkAccess(caller, domain); + // Check across the domain hierarchy (current + children) for any delete-protected instances + validateNoDeleteProtectedVmsForDomain(domain); return deleteDomain(domain, cleanup); } @@ -724,6 +728,22 @@ protected boolean cleanupDomain(Long domainId, Long ownerId) throws ConcurrentOp return success && deleteDomainSuccess; } + private void validateNoDeleteProtectedVmsForDomain(Domain parentDomain) { + Set allDomainIds = getDomainChildrenIds(parentDomain.getPath()); + List deleteProtectedVms = vmInstanceDao.listDeleteProtectedVmsByDomainIds(allDomainIds); + if (CollectionUtils.isEmpty(deleteProtectedVms)) { + return; + } + if (logger.isDebugEnabled()) { + List vmUuids = deleteProtectedVms.stream().map(VMInstanceVO::getUuid).collect(Collectors.toList()); + logger.debug("Cannot delete Domain {}, it has delete protection enabled for Instances: {}", parentDomain, vmUuids); + } + + throw new InvalidParameterValueException( + String.format("Cannot delete Domain '%s'. One or more Instances have delete protection enabled.", + parentDomain.getName())); + } + @Override public Pair, Integer> searchForDomains(ListDomainsCmd cmd) { Account caller = getCaller(); diff --git a/server/src/test/java/com/cloud/user/DomainManagerImplTest.java b/server/src/test/java/com/cloud/user/DomainManagerImplTest.java index 5a1dba215ae2..06a8eba903b0 100644 --- a/server/src/test/java/com/cloud/user/DomainManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/DomainManagerImplTest.java @@ -68,6 +68,7 @@ import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.NetUtils; +import com.cloud.vm.dao.VMInstanceDao; @RunWith(MockitoJUnitRunner.class) @@ -123,6 +124,8 @@ public class DomainManagerImplTest { Account adminAccount; @Mock GlobalLock lock; + @Mock + VMInstanceDao vmInstanceDao; List domainAccountsForCleanup; List domainNetworkIds; @@ -213,6 +216,7 @@ public void testDeleteDomainRootDomain() { @Test public void testDeleteDomainNoCleanup() { Mockito.when(_configMgr.releaseDomainSpecificVirtualRanges(Mockito.any())).thenReturn(true); + Mockito.doReturn(Collections.emptySet()).when(domainManager).getDomainChildrenIds(Mockito.any()); domainManager.deleteDomain(DOMAIN_ID, testDomainCleanup); Mockito.verify(domainManager).deleteDomain(domain, testDomainCleanup); Mockito.verify(domainManager).removeDomainWithNoAccountsForCleanupNetworksOrDedicatedResources(domain); @@ -278,6 +282,7 @@ public void deleteDomain() { Mockito.when(_dedicatedDao.listByDomainId(Mockito.anyLong())).thenReturn(new ArrayList()); Mockito.when(domainDaoMock.remove(Mockito.anyLong())).thenReturn(true); Mockito.when(_configMgr.releaseDomainSpecificVirtualRanges(Mockito.any())).thenReturn(true); + Mockito.doReturn(Collections.emptySet()).when(domainManager).getDomainChildrenIds(Mockito.any()); try { Assert.assertTrue(domainManager.deleteDomain(20l, false)); @@ -309,7 +314,7 @@ public void deleteDomainCleanup() { Mockito.when(_resourceCountDao.removeEntriesByOwner(Mockito.anyLong(), Mockito.eq(ResourceOwnerType.Domain))).thenReturn(1l); Mockito.when(_resourceLimitDao.removeEntriesByOwner(Mockito.anyLong(), Mockito.eq(ResourceOwnerType.Domain))).thenReturn(1l); Mockito.when(_configMgr.releaseDomainSpecificVirtualRanges(Mockito.any())).thenReturn(true); - + Mockito.when(vmInstanceDao.listDeleteProtectedVmsByDomainIds(Mockito.any())).thenReturn(Collections.emptyList()); try { Assert.assertTrue(domainManager.deleteDomain(20l, true)); } finally {