server: do not deploy or upgrade vm with inactive service offering (#7063)

Co-authored-by: Stephan Krug <stekrug@icloud.com>

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -3741,6 +3741,10 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
             throw new InvalidParameterValueException("Invalid parameter, newServiceOffering can't be null");
         }
 
+        if (ServiceOffering.State.Inactive.equals(newServiceOffering.getState())) {
+            throw new InvalidParameterValueException(String.format("New service offering is inactive: [%s].", newServiceOffering.getUuid()));
+        }
+
         if (!(vmInstance.getState().equals(State.Stopped) || vmInstance.getState().equals(State.Running))) {
             s_logger.warn("Unable to upgrade virtual machine " + vmInstance.toString() + " in state " + vmInstance.getState());
             throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString() + " " + " in state " + vmInstance.getState() +

engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java

@@ -57,6 +57,7 @@ import com.cloud.deploy.DeploymentPlanner.ExcludeList;
 import com.cloud.host.HostVO;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.hypervisor.HypervisorGuru;
+import com.cloud.offering.ServiceOffering;
 import com.cloud.service.ServiceOfferingVO;
 import com.cloud.service.dao.ServiceOfferingDao;
 import com.cloud.storage.DiskOfferingVO;
@@ -234,6 +235,13 @@ public class VirtualMachineManagerImplTest {
         virtualMachineManagerImpl.checkIfCanUpgrade(vmInstanceMock, serviceOfferingMock);
     }
 
+    @Test(expected = InvalidParameterValueException.class)
+    public void testCheckIfCanUpgradeFail() {
+        when(serviceOfferingMock.getState()).thenReturn(ServiceOffering.State.Inactive);
+
+        virtualMachineManagerImpl.checkIfCanUpgrade(vmInstanceMock, serviceOfferingMock);
+    }
+
     @Test
     public void isStorageCrossClusterMigrationTestStorageTypeEqualsCluster() {
         Mockito.doReturn(2L).when(storagePoolVoMock).getClusterId();

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -5658,6 +5658,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
             throw new InvalidParameterValueException("Unable to find service offering: " + serviceOfferingId);
         }
 
+        if (ServiceOffering.State.Inactive.equals(serviceOffering.getState())) {
+            throw new InvalidParameterValueException(String.format("Service offering is inactive: [%s].", serviceOffering.getUuid()));
+        }
+
         if (serviceOffering.getDiskOfferingStrictness() && overrideDiskOfferingId != null) {
             throw new InvalidParameterValueException(String.format("Cannot override disk offering id %d since provided service offering is strictly mapped to its disk offering", overrideDiskOfferingId));
         }

server/src/main/java/org/apache/cloudstack/network/router/deployment/RouterDeploymentDefinition.java

@@ -407,7 +407,7 @@ public class RouterDeploymentDefinition {
     private void verifyServiceOfferingByUuid(String offeringUuid) {
         logger.debug("Verifying router service offering with uuid : " + offeringUuid);
         ServiceOfferingVO serviceOffering = serviceOfferingDao.findByUuid(offeringUuid);
-        if (serviceOffering != null && serviceOffering.isSystemUse()) {
+        if (serviceOffering != null && serviceOffering.isSystemUse() && ServiceOffering.State.Active.equals(serviceOffering.getState())) {
             DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
             boolean isLocalStorage = ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dest.getDataCenter().getId());
             if (isLocalStorage == diskOffering.isUseLocalStorage()) {

server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java

@@ -23,6 +23,7 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.anyMap;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.nullable;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
@@ -34,6 +35,7 @@ import java.util.List;
 import java.util.Map;
 
 import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
+import org.apache.cloudstack.api.command.user.vm.DeployVMCmd;
 import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd;
 import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
 import org.apache.cloudstack.context.CallContext;
@@ -49,13 +51,16 @@ import org.mockito.Mockito;
 import org.mockito.Spy;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.springframework.test.util.ReflectionTestUtils;
 
 import com.cloud.configuration.Resource;
+import com.cloud.dc.DataCenter;
 import com.cloud.dc.DataCenterVO;
 import com.cloud.dc.dao.DataCenterDao;
 import com.cloud.exception.InsufficientAddressCapacityException;
 import com.cloud.exception.InsufficientCapacityException;
 import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.ResourceAllocationException;
 import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.hypervisor.Hypervisor;
 import com.cloud.network.NetworkModel;
@@ -74,11 +79,13 @@ import com.cloud.storage.dao.GuestOSDao;
 import com.cloud.storage.dao.VMTemplateDao;
 import com.cloud.user.Account;
 import com.cloud.user.AccountManager;
+import com.cloud.user.AccountService;
 import com.cloud.user.AccountVO;
 import com.cloud.user.ResourceLimitService;
 import com.cloud.user.UserVO;
 import com.cloud.user.dao.AccountDao;
 import com.cloud.uservm.UserVm;
+import com.cloud.utils.db.EntityManager;
 import com.cloud.vm.dao.NicDao;
 import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.dao.UserVmDetailsDao;
@@ -125,6 +132,12 @@ public class UserVmManagerImplTest {
     @Mock
     private AccountManager accountManager;
 
+    @Mock
+    private AccountService accountService;
+
+    @Mock
+    private EntityManager entityManager;
+
     @Mock
     private UserVmDetailsDao userVmDetailVO;
 
@@ -155,7 +168,16 @@ public class UserVmManagerImplTest {
     @Mock
     VolumeApiService volumeApiService;
 
-    private long vmId = 1l;
+    @Mock
+    AccountVO account;
+
+    @Mock
+    private ServiceOfferingVO serviceOffering;
+
+    private static final long vmId = 1l;
+    private static final long zoneId = 2L;
+    private static final long accountId = 3L;
+    private static final long serviceOfferingId = 10L;
 
     private static final long GiB_TO_BYTES = 1024 * 1024 * 1024;
 
@@ -171,7 +193,7 @@ public class UserVmManagerImplTest {
 
         when(_dcDao.findById(anyLong())).thenReturn(_dcMock);
 
-        Mockito.when(userVmDao.findById(Mockito.eq(vmId))).thenReturn(userVmVoMock);
+        Mockito.when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
 
         Mockito.when(callerAccount.getType()).thenReturn(Account.Type.ADMIN);
         CallContext.register(callerUser, callerAccount);
@@ -202,14 +224,14 @@ public class UserVmManagerImplTest {
     @Test
     public void validateGuestOsIdForUpdateVirtualMachineCommandTestOsTypeFound() {
         Mockito.when(updateVmCommand.getOsTypeId()).thenReturn(1l);
-        Mockito.when(guestOSDao.findById(Mockito.eq(1l))).thenReturn(Mockito.mock(GuestOSVO.class));
+        Mockito.when(guestOSDao.findById(1l)).thenReturn(Mockito.mock(GuestOSVO.class));
 
         userVmManagerImpl.validateGuestOsIdForUpdateVirtualMachineCommand(updateVmCommand);
     }
 
     @Test(expected = InvalidParameterValueException.class)
     public void validateInputsAndPermissionForUpdateVirtualMachineCommandTestVmNotFound() {
-        Mockito.when(userVmDao.findById(Mockito.eq(vmId))).thenReturn(null);
+        Mockito.when(userVmDao.findById(vmId)).thenReturn(null);
 
         userVmManagerImpl.validateInputsAndPermissionForUpdateVirtualMachineCommand(updateVmCommand);
     }
@@ -573,4 +595,20 @@ public class UserVmManagerImplTest {
         Mockito.when(newRootDiskOffering.getName()).thenReturn("OfferingName");
         return newRootDiskOffering;
     }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void createVirtualMachineWithInactiveServiceOffering() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException {
+        DeployVMCmd deployVMCmd = new DeployVMCmd();
+        ReflectionTestUtils.setField(deployVMCmd, "zoneId", zoneId);
+        ReflectionTestUtils.setField(deployVMCmd, "serviceOfferingId", serviceOfferingId);
+        deployVMCmd._accountService = accountService;
+
+        when(accountService.finalyzeAccountId(nullable(String.class), nullable(Long.class), nullable(Long.class), eq(true))).thenReturn(accountId);
+        when(accountService.getActiveAccountById(accountId)).thenReturn(account);
+        when(entityManager.findById(DataCenter.class, zoneId)).thenReturn(_dcMock);
+        when(entityManager.findById(ServiceOffering.class, serviceOfferingId)).thenReturn(serviceOffering);
+        when(serviceOffering.getState()).thenReturn(ServiceOffering.State.Inactive);
+
+        userVmManagerImpl.createVirtualMachine(deployVMCmd);
+    }
 }