You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ni...@apache.org on 2013/05/13 13:08:08 UTC
[16/55] [abbrv] git commit: updated refs/heads/cs2163 to 8d53b1e
Bug CLOUDSTACK-1390: Allow Root/Domain admin to move a User VM to another user under a different domain
Add unit tests
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/dfad178a
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/dfad178a
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/dfad178a
Branch: refs/heads/cs2163
Commit: dfad178a9e334776e4ba6c056523b5bd627ccace
Parents: e40aba3
Author: Likitha Shetty <li...@citrix.com>
Authored: Mon May 6 09:58:18 2013 +0530
Committer: Likitha Shetty <li...@citrix.com>
Committed: Fri May 10 12:18:43 2013 +0530
----------------------------------------------------------------------
client/tomcatconf/commands.properties.in | 2 +-
server/src/com/cloud/vm/UserVmManagerImpl.java | 14 +--
server/test/com/cloud/vm/UserVmManagerTest.java | 83 +++++++++++++++++-
3 files changed, 88 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dfad178a/client/tomcatconf/commands.properties.in
----------------------------------------------------------------------
diff --git a/client/tomcatconf/commands.properties.in b/client/tomcatconf/commands.properties.in
index cdc1992..0a6ec70 100644
--- a/client/tomcatconf/commands.properties.in
+++ b/client/tomcatconf/commands.properties.in
@@ -67,7 +67,7 @@ getVMPassword=15
restoreVirtualMachine=15
changeServiceForVirtualMachine=15
scaleVirtualMachine=15
-assignVirtualMachine=1
+assignVirtualMachine=7
migrateVirtualMachine=1
migrateVirtualMachineWithVolume=1
recoverVirtualMachine=7
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dfad178a/server/src/com/cloud/vm/UserVmManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java
index 9fbc509..f7f5fc7 100755
--- a/server/src/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/com/cloud/vm/UserVmManagerImpl.java
@@ -3725,19 +3725,14 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Use
+ cmd.getAccountName() + " is disabled.");
}
- // make sure the accounts are under same domain
- if (oldAccount.getDomainId() != newAccount.getDomainId()) {
- throw new InvalidParameterValueException(
- "The account should be under same domain for moving VM between two accounts. Old owner domain ="
- + oldAccount.getDomainId()
- + " New owner domain="
- + newAccount.getDomainId());
- }
+ //check caller has access to both the old and new account
+ _accountMgr.checkAccess(caller, null, true, oldAccount);
+ _accountMgr.checkAccess(caller, null, true, newAccount);
// make sure the accounts are not same
if (oldAccount.getAccountId() == newAccount.getAccountId()) {
throw new InvalidParameterValueException(
- "The account should be same domain for moving VM between two accounts. Account id ="
+ "The new account is the same as the old account. Account id ="
+ oldAccount.getAccountId());
}
@@ -3829,6 +3824,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Use
_resourceLimitMgr.decrementResourceCount(oldAccount.getAccountId(), ResourceType.primary_storage,
new Long(volume.getSize()));
volume.setAccountId(newAccount.getAccountId());
+ volume.setDomainId(newAccount.getDomainId());
_volsDao.persist(volume);
_resourceLimitMgr.incrementResourceCount(newAccount.getAccountId(), ResourceType.volume);
_resourceLimitMgr.incrementResourceCount(newAccount.getAccountId(), ResourceType.primary_storage,
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dfad178a/server/test/com/cloud/vm/UserVmManagerTest.java
----------------------------------------------------------------------
diff --git a/server/test/com/cloud/vm/UserVmManagerTest.java b/server/test/com/cloud/vm/UserVmManagerTest.java
index 08f2a9c..dfd7465 100755
--- a/server/test/com/cloud/vm/UserVmManagerTest.java
+++ b/server/test/com/cloud/vm/UserVmManagerTest.java
@@ -17,19 +17,26 @@
package com.cloud.vm;
+import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyFloat;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.when;
import java.lang.reflect.Field;
import java.util.List;
+import java.util.UUID;
+import org.apache.cloudstack.acl.ControlledEntity;
+import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.admin.vm.AssignVMCmd;
import org.apache.cloudstack.api.command.user.vm.RestoreVMCmd;
import org.apache.cloudstack.api.command.user.vm.ScaleVMCmd;
import org.junit.Before;
@@ -44,9 +51,11 @@ import com.cloud.configuration.dao.ConfigurationDao;
import com.cloud.exception.ConcurrentOperationException;
import com.cloud.exception.InsufficientCapacityException;
import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.PermissionDeniedException;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.hypervisor.Hypervisor;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.offering.ServiceOffering;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.storage.VMTemplateVO;
@@ -57,6 +66,7 @@ import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
+import com.cloud.user.AccountService;
import com.cloud.user.AccountVO;
import com.cloud.user.UserContext;
import com.cloud.user.UserVO;
@@ -73,6 +83,7 @@ public class UserVmManagerTest {
@Mock VolumeManager _storageMgr;
@Mock Account _account;
@Mock AccountManager _accountMgr;
+ @Mock AccountService _accountService;
@Mock ConfigurationManager _configMgr;
@Mock CapacityManager _capacityMgr;
@Mock AccountDao _accountDao;
@@ -91,6 +102,7 @@ public class UserVmManagerTest {
@Mock VMTemplateVO _templateMock;
@Mock VolumeVO _volumeMock;
@Mock List<VolumeVO> _rootVols;
+ @Mock Account _accountMock2;
@Before
public void setup(){
MockitoAnnotations.initMocks(this);
@@ -102,6 +114,7 @@ public class UserVmManagerTest {
_userVmMgr._itMgr = _itMgr;
_userVmMgr.volumeMgr = _storageMgr;
_userVmMgr._accountDao = _accountDao;
+ _userVmMgr._accountService = _accountService;
_userVmMgr._userDao = _userDao;
_userVmMgr._accountMgr = _accountMgr;
_userVmMgr._configMgr = _configMgr;
@@ -370,6 +383,74 @@ public class UserVmManagerTest {
return serviceOffering;
}
-
+ // Test Move VM b/w accounts where caller is not ROOT/Domain admin
+ @Test(expected=InvalidParameterValueException.class)
+ public void testMoveVmToUser1() throws Exception {
+ AssignVMCmd cmd = new AssignVMCmd();
+ Class<?> _class = cmd.getClass();
+
+ Field virtualmachineIdField = _class.getDeclaredField("virtualMachineId");
+ virtualmachineIdField.setAccessible(true);
+ virtualmachineIdField.set(cmd, 1L);
+
+ Field accountNameField = _class.getDeclaredField("accountName");
+ accountNameField.setAccessible(true);
+ accountNameField.set(cmd, "account");
+
+ Field domainIdField = _class.getDeclaredField("domainId");
+ domainIdField.setAccessible(true);
+ domainIdField.set(cmd, 1L);
+
+ // caller is of type 0
+ Account caller = (Account) new AccountVO("testaccount", 1, "networkdomain", (short) 0,
+ UUID.randomUUID().toString());
+ UserContext.registerContext(1, caller, null, true);
+
+ _userVmMgr.moveVMToUser(cmd);
+ }
+
+
+ // Test Move VM b/w accounts where caller doesn't have access to the old or new account
+ @Test(expected=PermissionDeniedException.class)
+ public void testMoveVmToUser2() throws Exception {
+ AssignVMCmd cmd = new AssignVMCmd();
+ Class<?> _class = cmd.getClass();
+
+ Field virtualmachineIdField = _class.getDeclaredField("virtualMachineId");
+ virtualmachineIdField.setAccessible(true);
+ virtualmachineIdField.set(cmd, 1L);
+
+ Field accountNameField = _class.getDeclaredField("accountName");
+ accountNameField.setAccessible(true);
+ accountNameField.set(cmd, "account");
+
+ Field domainIdField = _class.getDeclaredField("domainId");
+ domainIdField.setAccessible(true);
+ domainIdField.set(cmd, 1L);
+
+ // caller is of type 0
+ Account caller = (Account) new AccountVO("testaccount", 1, "networkdomain", (short) 1,
+ UUID.randomUUID().toString());
+ UserContext.registerContext(1, caller, null, true);
+
+ Account oldAccount = (Account) new AccountVO("testaccount", 1, "networkdomain", (short) 0,
+ UUID.randomUUID().toString());
+ Account newAccount = (Account) new AccountVO("testaccount", 1, "networkdomain", (short) 1,
+ UUID.randomUUID().toString());
+
+ UserVmVO vm = new UserVmVO(10L, "test", "test", 1L, HypervisorType.Any, 1L, false, false, 1L, 1L,
+ 5L, "test", "test", 1L);
+ vm.setState(VirtualMachine.State.Stopped);
+ when(_vmDao.findById(anyLong())).thenReturn(vm);
+
+ when(_accountService.getActiveAccountById(anyLong())).thenReturn(oldAccount);
+
+ when(_accountService.getActiveAccountByName(anyString(), anyLong())).thenReturn(newAccount);
+
+ doThrow(new PermissionDeniedException("Access check failed")).when(_accountMgr).checkAccess(any(Account.class), any(AccessType.class),
+ any(Boolean.class), any(ControlledEntity.class));
+
+ _userVmMgr.moveVMToUser(cmd);
+ }
}
\ No newline at end of file