diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 7e58cd01050a..e2df252da81d 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7455,11 +7455,6 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep checkCallerAccessToAccounts(caller, oldAccount, newAccount); - logger.trace("Verifying if the provided domain ID [{}] is valid.", domainId); - if (projectId != null && domainId == null) { - throw new InvalidParameterValueException("Please provide a valid domain ID; cannot assign VM to a project if domain ID is NULL."); - } - validateIfVmHasNoRules(vm, vmId); final List volumes = _volsDao.findByInstance(vmId); @@ -7470,10 +7465,6 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep validateIfNewOwnerHasAccessToTemplate(vm, newAccount, template); - DomainVO domain = _domainDao.findById(domainId); - logger.trace("Verifying if the new account [{}] has access to the specified domain [{}].", newAccount, domain); - _accountMgr.checkAccess(newAccount, domain); - List reservations = new ArrayList<>(); try { verifyResourceLimitsForAccountAndStorage(newAccount, vm, offering, volumes, template, reservations); @@ -7483,7 +7474,7 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep Transaction.execute(new TransactionCallbackNoReturn() { @Override public void doInTransactionWithoutResult(TransactionStatus status) { - executeStepsToChangeOwnershipOfVm(cmd, caller, oldAccount, newAccount, vm, offering, volumes, template, domainId); + executeStepsToChangeOwnershipOfVm(cmd, caller, oldAccount, newAccount, vm, offering, volumes, template); } }); } catch (Exception e) { @@ -7681,10 +7672,9 @@ protected NetworkOfferingVO getOfferingWithRequiredAvailabilityForNetworkCreatio * @param offering The service offering which will be used to decrement and increment resource counts. * @param volumes The volumes of the VM which will be assigned to another user. * @param template The template of the VM which will be assigned to another user. - * @param domainId The ID of the domain where the VM which will be assigned to another user is. */ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller, Account oldAccount, Account newAccount, UserVmVO vm, ServiceOfferingVO offering, - List volumes, VirtualMachineTemplate template, Long domainId) { + List volumes, VirtualMachineTemplate template) { logger.trace("Generating destroy event for VM [{}].", vm); UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_DESTROY, vm.getAccountId(), vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), @@ -7697,7 +7687,7 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller removeInstanceFromInstanceGroup(vm.getId()); Long newAccountId = newAccount.getAccountId(); - updateVmOwner(newAccount, vm, domainId, newAccountId); + updateVmOwner(newAccount, vm); updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId); @@ -7717,11 +7707,11 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller vm.getTemplateId(), vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm()); } - protected void updateVmOwner(Account newAccount, UserVmVO vm, Long domainId, Long newAccountId) { + protected void updateVmOwner(Account newAccount, UserVmVO vm) { logger.debug("Updating VM [{}] owner to [{}].", vm, newAccount); - vm.setAccountId(newAccountId); - vm.setDomainId(domainId); + vm.setAccountId(newAccount.getId()); + vm.setDomainId(newAccount.getDomainId()); _vmDao.persist(vm); } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index b21cd53d743c..e0292b963851 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -618,7 +618,7 @@ private void configureDoNothingForMethodsThatWeDoNotWantToTest() throws Resource Mockito.doNothing().when(userVmManagerImpl).removeInstanceFromInstanceGroup(Mockito.anyLong()); Mockito.doNothing().when(userVmManagerImpl).validateIfNewOwnerHasAccessToTemplate(Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.doNothing().when(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.doNothing().when(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any()); Mockito.doNothing().when(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); Mockito.doNothing().when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); @@ -1958,7 +1958,7 @@ public void validateIfNewOwnerHasAccessToTemplateTestCallCheckAccessWhenTemplate @Test public void updateVmOwnerTestCallsSetAccountIdSetDomainIdAndPersist() { - userVmManagerImpl.updateVmOwner(accountMock, userVmVoMock, 1l, 1l); + userVmManagerImpl.updateVmOwner(accountMock, userVmVoMock); Mockito.verify(userVmVoMock).setAccountId(Mockito.anyLong()); Mockito.verify(userVmVoMock).setDomainId(Mockito.anyLong()); @@ -2913,23 +2913,22 @@ public void moveVmToUserTestValidateAccountsAndCallerAccessToThemThrowsInvalidPa } @Test - public void moveVmToUserTestProjectIdProvidedAndDomainIdIsNullThrowsInvalidParameterValueException() throws ResourceUnavailableException, InsufficientCapacityException, + public void moveVmToUserTestMovesVmWhenProjectIdIsProvidedAndDomainIdIsNull() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { - - String expectedMessage = "Please provide a valid domain ID; cannot assign VM to a project if domain ID is NULL."; - Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); Mockito.doReturn(1l).when(assignVmCmdMock).getProjectId(); Mockito.doReturn(null).when(assignVmCmdMock).getDomainId(); + Mockito.doReturn(null).when(userVmManagerImpl).ensureDestinationNetwork(Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.doNothing().when(userVmManagerImpl).executeStepsToChangeOwnershipOfVm(Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); configureDoNothingForMethodsThatWeDoNotWantToTest(); - InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { - userVmManagerImpl.moveVmToUser(assignVmCmdMock); - }); + userVmManagerImpl.moveVmToUser(assignVmCmdMock); - Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + Mockito.verify(userVmManagerImpl).executeStepsToChangeOwnershipOfVm(Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); } @Test @@ -3003,26 +3002,6 @@ public void moveVmToUserTestVerifyValidateIfNewOwnerHasAccessToTemplateThrowsInv Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); } - @Test - public void moveVmToUserTestAccountManagerCheckAccessThrowsPermissionDeniedException() throws ResourceUnavailableException, InsufficientCapacityException, - ResourceAllocationException { - - LinkedList volumes = new LinkedList(); - - Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); - Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); - Mockito.doReturn(null).when(assignVmCmdMock).getProjectId(); - Mockito.doReturn(volumes).when(volumeDaoMock).findByInstance(Mockito.anyLong()); - Mockito.doReturn(accountMock).when(accountManager).finalizeOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.doReturn(domainVoMock).when(domainDaoMock).findById(Mockito.anyLong()); - - configureDoNothingForMethodsThatWeDoNotWantToTest(); - - Mockito.doThrow(PermissionDeniedException.class).when(accountManager).checkAccess(Mockito.any(Account.class), Mockito.any()); - - Assert.assertThrows(PermissionDeniedException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); - } - @Test public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficientCapacityException() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { @@ -3038,10 +3017,10 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficie Mockito.any()); Assert.assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, - userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock, 1l)); + userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock)); Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any()); Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong()); } } @@ -3061,10 +3040,10 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsResourceAl Mockito.any()); Assert.assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, - userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock, 1l)); + userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock)); Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any()); Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong()); } } @@ -3083,10 +3062,10 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab configureDoNothingForMethodsThatWeDoNotWantToTest(); userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock, volumes, - virtualMachineTemplateMock, 1l); + virtualMachineTemplateMock); Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any()); Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong()); Mockito.verify(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); Mockito.verify(userVmManagerImpl).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); @@ -3106,10 +3085,10 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab configureDoNothingForMethodsThatWeDoNotWantToTest(); userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock, volumes, - virtualMachineTemplateMock, 1l); + virtualMachineTemplateMock); Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any()); Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong()); Mockito.verify(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); Mockito.verify(userVmManagerImpl, Mockito.never()).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());