CLOUDSTACK-8580 Users are now able to view, expunge and recover their vm's themselves. Two configuration options are added to allow this behaviour on a global or per account scale. Configuration options default to false.

Boris Schrijver · Boris Schrijver · commit dd7dfc3a4e1e · 2015-07-15T11:04:32.000+02:00
diff --git a/api/src/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java b/api/src/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java
@@ -40,7 +40,7 @@
 import com.cloud.uservm.UserVm;
 import com.cloud.vm.VirtualMachine;
 
-@APICommand(name = "destroyVirtualMachine", description = "Destroys a virtual machine. Once destroyed, only the administrator can recover it.", responseObject = UserVmResponse.class, responseView = ResponseView.Restricted, entityType = {VirtualMachine.class},
+@APICommand(name = "destroyVirtualMachine", description = "Destroys a virtual machine.", responseObject = UserVmResponse.class, responseView = ResponseView.Restricted, entityType = {VirtualMachine.class},
             requestHasSensitiveInfo = false,
             responseHasSensitiveInfo = true)
 public class DestroyVMCmd extends BaseAsyncCmd {
@@ -59,7 +59,7 @@ public class DestroyVMCmd extends BaseAsyncCmd {
 
     @Parameter(name = ApiConstants.EXPUNGE,
                type = CommandType.BOOLEAN,
-               description = "If true is passed, the vm is expunged immediately. False by default. Parameter can be passed to the call by ROOT/Domain admin only",
+               description = "If true is passed, the vm is expunged immediately. False by default.",
                since = "4.2.1")
     private Boolean expunge;
 
diff --git a/api/src/org/apache/cloudstack/query/QueryService.java b/api/src/org/apache/cloudstack/query/QueryService.java
@@ -71,6 +71,7 @@
 import org.apache.cloudstack.api.response.UserVmResponse;
 import org.apache.cloudstack.api.response.VolumeResponse;
 import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.framework.config.ConfigKey;
 
 import com.cloud.exception.PermissionDeniedException;
 
@@ -80,6 +81,10 @@
  */
 public interface QueryService {
 
+    // Config keys
+    static final ConfigKey<Boolean> AllowUserViewDestroyedVM = new ConfigKey<Boolean>("Advanced", Boolean.class, "allow.user.view.destroyed.vm", "false",
+            "Determines whether users can view their destroyed or expunging vm ", true, ConfigKey.Scope.Account);
+
     ListResponse<UserResponse> searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException;
 
     ListResponse<EventResponse> searchForEvents(ListEventsCmd cmd);
diff --git a/client/tomcatconf/commands.properties.in b/client/tomcatconf/commands.properties.in
@@ -82,8 +82,8 @@ scaleVirtualMachine=15
 assignVirtualMachine=7
 migrateVirtualMachine=1
 migrateVirtualMachineWithVolume=1
-recoverVirtualMachine=7
-expungeVirtualMachine=7
+recoverVirtualMachine=15
+expungeVirtualMachine=15
 getVirtualMachineUserData=15
 
 #### snapshot commands
diff --git a/server/src/com/cloud/api/query/QueryManagerImpl.java b/server/src/com/cloud/api/query/QueryManagerImpl.java
@@ -103,6 +103,8 @@
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreDriver;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
 import org.apache.cloudstack.engine.subsystem.api.storage.TemplateState;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.query.QueryService;
 import org.apache.log4j.Logger;
@@ -223,7 +225,7 @@
 
 @Component
 @Local(value = {QueryService.class})
-public class QueryManagerImpl extends ManagerBase implements QueryService {
+public class QueryManagerImpl extends ManagerBase implements QueryService, Configurable {
 
     public static final Logger s_logger = Logger.getLogger(QueryManagerImpl.class);
 
@@ -979,8 +981,8 @@ private Pair<List<UserVmJoinVO>, Integer> searchForUserVMsInternal(ListVMsCmd cm
             sc.setParameters("hypervisorType", hypervisor);
         }
 
-        // Don't show Destroyed and Expunging vms to the end user
-        if (!isAdmin) {
+        // Don't show Destroyed and Expunging vms to the end user if the AllowUserViewDestroyedVM flag is not set.
+        if (!isAdmin && !AllowUserViewDestroyedVM.valueIn(caller.getAccountId())) {
             sc.setParameters("stateNIN", "Destroyed", "Expunging");
         }
 
@@ -3685,4 +3687,13 @@ protected ResourceDetailResponse createResourceDetailsResponse(ResourceDetail re
         return resourceDetailResponse;
     }
 
+    @Override
+    public String getConfigComponentName() {
+        return QueryService.class.getSimpleName();
+    }
+
+    @Override
+    public ConfigKey<?>[] getConfigKeys() {
+        return new ConfigKey<?>[] {AllowUserViewDestroyedVM};
+    }
 }
diff --git a/server/src/com/cloud/vm/UserVmManager.java b/server/src/com/cloud/vm/UserVmManager.java
@@ -42,8 +42,11 @@
  */
 public interface UserVmManager extends UserVmService {
     static final String EnableDynamicallyScaleVmCK = "enable.dynamic.scale.vm";
+    static final String AllowUserExpungeRecoverVmCK ="allow.user.expunge.recover.vm";
     static final ConfigKey<Boolean> EnableDynamicallyScaleVm = new ConfigKey<Boolean>("Advanced", Boolean.class, EnableDynamicallyScaleVmCK, "false",
         "Enables/Disables dynamically scaling a vm", true, ConfigKey.Scope.Zone);
+    static final ConfigKey<Boolean> AllowUserExpungeRecoverVm = new ConfigKey<Boolean>("Advanced", Boolean.class, AllowUserExpungeRecoverVmCK, "false",
+        "Determines whether users can expunge or recover their vm", true, ConfigKey.Scope.Account);
 
     static final int MAX_USER_DATA_LENGTH_BYTES = 2048;
 
diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java
@@ -1705,6 +1705,7 @@ public UserVm recoverVirtualMachine(RecoverVMCmd cmd) throws ResourceAllocationE
 
         final Long vmId = cmd.getId();
         Account caller = CallContext.current().getCallingAccount();
+        final Long userId = caller.getAccountId();
 
         // Verify input parameters
         final UserVmVO vm = _vmDao.findById(vmId);
@@ -1713,8 +1714,10 @@ public UserVm recoverVirtualMachine(RecoverVMCmd cmd) throws ResourceAllocationE
             throw new InvalidParameterValueException("unable to find a virtual machine with id " + vmId);
         }
 
-        // check permissions
-        _accountMgr.checkAccess(caller, null, true, vm);
+        // When trying to expunge, permission is denied when the caller is not an admin and the AllowUserExpungeRecoverVm is false for the caller.
+        if (!_accountMgr.isAdmin(userId) && !AllowUserExpungeRecoverVm.valueIn(userId)) {
+            throw new PermissionDeniedException("Recovering a vm can only be done by an Admin. Or when the allow.user.expunge.recover.vm key is set.");
+        }
 
         if (vm.getRemoved() != null) {
             if (s_logger.isDebugEnabled()) {
@@ -2404,8 +2407,9 @@ public UserVm destroyVm(DestroyVMCmd cmd) throws ResourceUnavailableException, C
         long vmId = cmd.getId();
         boolean expunge = cmd.getExpunge();
 
-        if (!_accountMgr.isAdmin(ctx.getCallingAccount().getId()) && expunge) {
-            throw new PermissionDeniedException("Parameter " + ApiConstants.EXPUNGE + " can be passed by Admin only");
+        // When trying to expunge, permission is denied when the caller is not an admin and the AllowUserExpungeRecoverVm is false for the caller.
+        if (expunge && !_accountMgr.isAdmin(ctx.getCallingAccount().getId()) && !AllowUserExpungeRecoverVm.valueIn(cmd.getEntityOwnerId())) {
+            throw new PermissionDeniedException("Parameter " + ApiConstants.EXPUNGE + " can be passed by Admin only. Or when the allow.user.expunge.recover.vm key is set.");
         }
 
         UserVm destroyedVm = destroyVm(vmId);
@@ -4072,7 +4076,10 @@ public UserVm expungeVm(long vmId) throws ResourceUnavailableException, Concurre
             throw ex;
         }
 
-        _accountMgr.checkAccess(caller, null, true, vm);
+        // When trying to expunge, permission is denied when the caller is not an admin and the AllowUserExpungeRecoverVm is false for the caller.
+        if (!_accountMgr.isAdmin(userId) && !AllowUserExpungeRecoverVm.valueIn(userId)) {
+            throw new PermissionDeniedException("Expunging a vm can only be done by an Admin. Or when the allow.user.expunge.recover.vm key is set.");
+        }
 
         boolean status;
 
@@ -5292,7 +5299,7 @@ public String getConfigComponentName() {
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {EnableDynamicallyScaleVm, VmIpFetchWaitInterval, VmIpFetchTrialMax, VmIpFetchThreadPoolMax};
+        return new ConfigKey<?>[] {EnableDynamicallyScaleVm, AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax, VmIpFetchThreadPoolMax};
     }
 
     @Override
