From d2a75a0b2f0933476d7ea2a80b79f750abee3b2f Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Wed, 26 Jul 2023 14:39:43 +0200 Subject: [PATCH 1/3] clean network offerings for domain on remove --- .../com/cloud/api/query/QueryManagerImpl.java | 4 +- .../com/cloud/user/AccountManagerImpl.java | 11 ++-- .../com/cloud/user/DomainManagerImpl.java | 56 +++++++++++++++---- 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 0968af0e7c0e..306dac4c7be1 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -965,7 +965,7 @@ private Pair, Integer> searchForVmGroupsInternal(ListV @Override public ListResponse searchForUserVMs(ListVMsCmd cmd) { Pair, Integer> result = searchForUserVMsInternal(cmd); - ListResponse response = new ListResponse(); + ListResponse response = new ListResponse<>(); ResponseView respView = ResponseView.Restricted; Account caller = CallContext.current().getCallingAccount(); if (_accountMgr.isRootAdmin(caller.getId())) { @@ -1303,7 +1303,7 @@ private Pair, Integer> searchForUserVMsInternal(ListVMsCmd cm vmIds[i++] = v.getId(); } List vms = _userVmJoinDao.searchByIds(vmIds); - return new Pair, Integer>(vms, count); + return new Pair<>(vms, count); } @Override diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 62a84d0d9bfe..ec5deb34df76 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -2955,18 +2955,17 @@ public void buildACLSearchParameters(Account caller, Long id, String accountName if (projectId != null) { if (!forProjectInvitation) { if (projectId == -1L) { - if (caller.getType() == Account.Type.ADMIN) { - domainIdRecursiveListProject.third(Project.ListProjectResourcesCriteria.ListProjectResourcesOnly); - if (listAll) { - domainIdRecursiveListProject.third(ListProjectResourcesCriteria.ListAllIncludingProjectResources); - } - } else { + domainIdRecursiveListProject.third(Project.ListProjectResourcesCriteria.ListProjectResourcesOnly); + if (caller.getType() != Account.Type.ADMIN) { permittedAccounts.addAll(_projectMgr.listPermittedProjectAccounts(caller.getId())); // permittedAccounts can be empty when the caller is not a part of any project (a domain account) if (permittedAccounts.isEmpty()) { permittedAccounts.add(caller.getId()); } } + if (listAll) { + domainIdRecursiveListProject.third(ListProjectResourcesCriteria.ListAllIncludingProjectResources); + } } else { Project project = _projectMgr.getProject(projectId); if (project == null) { diff --git a/server/src/main/java/com/cloud/user/DomainManagerImpl.java b/server/src/main/java/com/cloud/user/DomainManagerImpl.java index da796d2e48fe..f288f52f2030 100644 --- a/server/src/main/java/com/cloud/user/DomainManagerImpl.java +++ b/server/src/main/java/com/cloud/user/DomainManagerImpl.java @@ -24,7 +24,11 @@ import javax.inject.Inject; +import com.cloud.api.query.dao.NetworkOfferingJoinDao; +import com.cloud.api.query.vo.NetworkOfferingJoinVO; import com.cloud.domain.dao.DomainDetailsDao; +import com.cloud.offerings.dao.NetworkOfferingDao; +import com.cloud.offerings.dao.NetworkOfferingDetailsDao; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.api.ApiConstants; @@ -106,6 +110,12 @@ public class DomainManagerImpl extends ManagerBase implements DomainManager, Dom @Inject private DiskOfferingDetailsDao diskOfferingDetailsDao; @Inject + private NetworkOfferingDao networkOfferingDao; + @Inject + private NetworkOfferingJoinDao networkOfferingJoinDao; + @Inject + private NetworkOfferingDetailsDao networkOfferingDetailsDao; + @Inject private ServiceOfferingJoinDao serviceOfferingJoinDao; @Inject private ServiceOfferingDao serviceOfferingDao; @@ -483,18 +493,31 @@ protected void cleanupDomainOfferings(Long domainId) { } String domainIdString = String.valueOf(domainId); - List diskOfferingsDetailsToRemove = new ArrayList<>(); - List serviceOfferingsDetailsToRemove = new ArrayList<>(); - // delete the service and disk offerings associated with this domain - List diskOfferingsForThisDomain = diskOfferingJoinDao.findByDomainId(domainId); - for (DiskOfferingJoinVO diskOffering : diskOfferingsForThisDomain) { - if (domainIdString.equals(diskOffering.getDomainId())) { - diskOfferingDao.remove(diskOffering.getId()); + removeDiskOfferings(domainId, domainIdString); + + removeServiceOfferings(domainId, domainIdString); + + removeNetworkOfferings(domainId, domainIdString); + } + + private void removeNetworkOfferings(Long domainId, String domainIdString) { + List networkOfferingsDetailsToRemove = new ArrayList<>(); + List networkOfferingsForThisDomain = networkOfferingJoinDao.findByDomainId(domainId, false); + for (NetworkOfferingJoinVO networkOffering : networkOfferingsForThisDomain) { + if (domainIdString.equals(networkOffering.getDomainId())) { + networkOfferingDao.remove(networkOffering.getId()); } else { - diskOfferingsDetailsToRemove.add(diskOffering.getId()); + networkOfferingsDetailsToRemove.add(networkOffering.getId()); } } + for (final Long networkOfferingId : networkOfferingsDetailsToRemove) { + networkOfferingDetailsDao.removeDetail(networkOfferingId, ApiConstants.DOMAIN_ID, domainIdString); + } + } + + private void removeServiceOfferings(Long domainId, String domainIdString) { + List serviceOfferingsDetailsToRemove = new ArrayList<>(); List serviceOfferingsForThisDomain = serviceOfferingJoinDao.findByDomainId(domainId); for (ServiceOfferingJoinVO serviceOffering : serviceOfferingsForThisDomain) { if (domainIdString.equals(serviceOffering.getDomainId())) { @@ -503,14 +526,25 @@ protected void cleanupDomainOfferings(Long domainId) { serviceOfferingsDetailsToRemove.add(serviceOffering.getId()); } } + for (final Long serviceOfferingId : serviceOfferingsDetailsToRemove) { + serviceOfferingDetailsDao.removeDetail(serviceOfferingId, ApiConstants.DOMAIN_ID, domainIdString); + } + } + private void removeDiskOfferings(Long domainId, String domainIdString) { + List diskOfferingsDetailsToRemove = new ArrayList<>(); + List diskOfferingsForThisDomain = diskOfferingJoinDao.findByDomainId(domainId); + for (DiskOfferingJoinVO diskOffering : diskOfferingsForThisDomain) { + if (domainIdString.equals(diskOffering.getDomainId())) { + diskOfferingDao.remove(diskOffering.getId()); + } else { + diskOfferingsDetailsToRemove.add(diskOffering.getId()); + } + } // Remove domain IDs for offerings which may be multi-domain for (final Long diskOfferingId : diskOfferingsDetailsToRemove) { diskOfferingDetailsDao.removeDetail(diskOfferingId, ApiConstants.DOMAIN_ID, domainIdString); } - for (final Long serviceOfferingId : serviceOfferingsDetailsToRemove) { - serviceOfferingDetailsDao.removeDetail(serviceOfferingId, ApiConstants.DOMAIN_ID, domainIdString); - } } protected boolean cleanupDomain(Long domainId, Long ownerId) throws ConcurrentOperationException, ResourceUnavailableException { From e73e64eff707bc43f56b256a922e1ede8da2ec7f Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Wed, 2 Aug 2023 13:22:43 +0200 Subject: [PATCH 2/3] revert change from other PR --- .../main/java/com/cloud/user/AccountManagerImpl.java | 11 ++++++----- 1 file changed, 6 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 ec5deb34df76..62a84d0d9bfe 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -2955,17 +2955,18 @@ public void buildACLSearchParameters(Account caller, Long id, String accountName if (projectId != null) { if (!forProjectInvitation) { if (projectId == -1L) { - domainIdRecursiveListProject.third(Project.ListProjectResourcesCriteria.ListProjectResourcesOnly); - if (caller.getType() != Account.Type.ADMIN) { + if (caller.getType() == Account.Type.ADMIN) { + domainIdRecursiveListProject.third(Project.ListProjectResourcesCriteria.ListProjectResourcesOnly); + if (listAll) { + domainIdRecursiveListProject.third(ListProjectResourcesCriteria.ListAllIncludingProjectResources); + } + } else { permittedAccounts.addAll(_projectMgr.listPermittedProjectAccounts(caller.getId())); // permittedAccounts can be empty when the caller is not a part of any project (a domain account) if (permittedAccounts.isEmpty()) { permittedAccounts.add(caller.getId()); } } - if (listAll) { - domainIdRecursiveListProject.third(ListProjectResourcesCriteria.ListAllIncludingProjectResources); - } } else { Project project = _projectMgr.getProject(projectId); if (project == null) { From d20d2efab7477a25a60c79dcf072677999b96833 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Wed, 2 Aug 2023 13:23:29 +0200 Subject: [PATCH 3/3] clean vpc offerings and fix unit tests --- .../com/cloud/user/DomainManagerImpl.java | 27 +++++++++++++++++++ .../com/cloud/user/DomainManagerImplTest.java | 17 ++++++++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/user/DomainManagerImpl.java b/server/src/main/java/com/cloud/user/DomainManagerImpl.java index f288f52f2030..ad0566557178 100644 --- a/server/src/main/java/com/cloud/user/DomainManagerImpl.java +++ b/server/src/main/java/com/cloud/user/DomainManagerImpl.java @@ -25,8 +25,12 @@ import javax.inject.Inject; import com.cloud.api.query.dao.NetworkOfferingJoinDao; +import com.cloud.api.query.dao.VpcOfferingJoinDao; import com.cloud.api.query.vo.NetworkOfferingJoinVO; +import com.cloud.api.query.vo.VpcOfferingJoinVO; import com.cloud.domain.dao.DomainDetailsDao; +import com.cloud.network.vpc.dao.VpcOfferingDao; +import com.cloud.network.vpc.dao.VpcOfferingDetailsDao; import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.offerings.dao.NetworkOfferingDetailsDao; import org.apache.cloudstack.annotation.AnnotationService; @@ -122,6 +126,12 @@ public class DomainManagerImpl extends ManagerBase implements DomainManager, Dom @Inject private ServiceOfferingDetailsDao serviceOfferingDetailsDao; @Inject + private VpcOfferingDao vpcOfferingDao; + @Inject + private VpcOfferingJoinDao vpcOfferingJoinDao; + @Inject + private VpcOfferingDetailsDao vpcOfferingDetailsDao; + @Inject private ProjectDao _projectDao; @Inject private ProjectManager _projectMgr; @@ -499,6 +509,23 @@ protected void cleanupDomainOfferings(Long domainId) { removeServiceOfferings(domainId, domainIdString); removeNetworkOfferings(domainId, domainIdString); + + removeVpcOfferings(domainId, domainIdString); + } + + private void removeVpcOfferings(Long domainId, String domainIdString) { + List vpcOfferingsDetailsToRemove = new ArrayList<>(); + List vpcOfferingsForThisDomain = vpcOfferingJoinDao.findByDomainId(domainId); + for (VpcOfferingJoinVO vpcOffering : vpcOfferingsForThisDomain) { + if (domainIdString.equals(vpcOffering.getDomainId())) { + vpcOfferingDao.remove(vpcOffering.getId()); + } else { + vpcOfferingsDetailsToRemove.add(vpcOffering.getId()); + } + } + for (final Long vpcOfferingId : vpcOfferingsDetailsToRemove) { + vpcOfferingDetailsDao.removeDetail(vpcOfferingId, ApiConstants.DOMAIN_ID, domainIdString); + } } private void removeNetworkOfferings(Long domainId, String domainIdString) { diff --git a/server/src/test/java/com/cloud/user/DomainManagerImplTest.java b/server/src/test/java/com/cloud/user/DomainManagerImplTest.java index 0290c060667e..3b270f30e84a 100644 --- a/server/src/test/java/com/cloud/user/DomainManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/DomainManagerImplTest.java @@ -22,6 +22,8 @@ import java.util.List; import java.util.UUID; +import com.cloud.api.query.dao.NetworkOfferingJoinDao; +import com.cloud.api.query.dao.VpcOfferingJoinDao; import com.cloud.configuration.ResourceLimit; import com.cloud.domain.dao.DomainDetailsDao; import com.cloud.utils.UuidUtils; @@ -82,7 +84,11 @@ public class DomainManagerImplTest { @Mock DiskOfferingJoinDao _diskOfferingDao; @Mock - ServiceOfferingJoinDao _offeringsDao; + NetworkOfferingJoinDao networkOfferingJoinDao; + @Mock + ServiceOfferingJoinDao serviceOfferingJoinDao; + @Mock + VpcOfferingJoinDao vpcOfferingJoinDao; @Mock ProjectDao _projectDao; @Mock @@ -142,6 +148,11 @@ public void setup() throws NoSuchFieldException, SecurityException, Mockito.when(_accountDao.findCleanupsForRemovedAccounts(DOMAIN_ID)).thenReturn(domainAccountsForCleanup); Mockito.when(_networkDomainDao.listNetworkIdsByDomain(DOMAIN_ID)).thenReturn(domainNetworkIds); Mockito.when(_dedicatedDao.listByDomainId(DOMAIN_ID)).thenReturn(domainDedicatedResources); + + Mockito.when(_diskOfferingDao.findByDomainId(Mockito.anyLong())).thenReturn(Collections.emptyList()); + Mockito.when(networkOfferingJoinDao.findByDomainId(Mockito.anyLong(), Mockito.anyBoolean())).thenReturn(Collections.emptyList()); + Mockito.when(serviceOfferingJoinDao.findByDomainId(Mockito.anyLong())).thenReturn(Collections.emptyList()); + Mockito.when(vpcOfferingJoinDao.findByDomainId(Mockito.anyLong())).thenReturn(Collections.emptyList()); } @Test @@ -266,8 +277,6 @@ 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.anyLong())).thenReturn(true); - Mockito.when(_diskOfferingDao.findByDomainId(Mockito.anyLong())).thenReturn(Collections.emptyList()); - Mockito.when(_offeringsDao.findByDomainId(Mockito.anyLong())).thenReturn(Collections.emptyList()); try { Assert.assertTrue(domainManager.deleteDomain(20l, false)); @@ -299,8 +308,6 @@ 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.anyLong())).thenReturn(true); - Mockito.when(_diskOfferingDao.findByDomainId(Mockito.anyLong())).thenReturn(Collections.emptyList()); - Mockito.when(_offeringsDao.findByDomainId(Mockito.anyLong())).thenReturn(Collections.emptyList()); try { Assert.assertTrue(domainManager.deleteDomain(20l, true));