server: prevent adding vm compute details when not applicable (#12637)

This commit is contained in:
Abhishek Kumar 2026-04-15 14:11:20 +05:30 committed by GitHub
parent 8eb162cb99
commit c6936889f5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 202 additions and 85 deletions

View File

@ -54,7 +54,6 @@ import javax.naming.ConfigurationException;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.ParserConfigurationException;
import com.cloud.resourcelimit.ReservationHelper;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
@ -135,8 +134,8 @@ import org.apache.cloudstack.storage.template.VnfTemplateManager;
import org.apache.cloudstack.userdata.UserDataManager;
import org.apache.cloudstack.utils.bytescale.ByteScaleUtils;
import org.apache.cloudstack.utils.security.ParserUtils;
import org.apache.cloudstack.vm.schedule.VMScheduleManager;
import org.apache.cloudstack.vm.UnmanagedVMsManager;
import org.apache.cloudstack.vm.schedule.VMScheduleManager;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.collections.MapUtils;
import org.apache.commons.lang.math.NumberUtils;
@ -297,6 +296,7 @@ import com.cloud.org.Grouping;
import com.cloud.resource.ResourceManager;
import com.cloud.resource.ResourceState;
import com.cloud.resourcelimit.CheckedReservation;
import com.cloud.resourcelimit.ReservationHelper;
import com.cloud.server.ManagementService;
import com.cloud.server.ResourceTag;
import com.cloud.server.StatsCollector;
@ -1285,46 +1285,45 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
@Override
public void validateCustomParameters(ServiceOfferingVO serviceOffering, Map<String, String> customParameters) {
//TODO need to validate custom cpu, and memory against min/max CPU/Memory ranges from service_offering_details table
if (customParameters.size() != 0) {
Map<String, String> offeringDetails = serviceOfferingDetailsDao.listDetailsKeyPairs(serviceOffering.getId());
if (serviceOffering.getCpu() == null) {
int minCPU = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MIN_CPU_NUMBER), 1);
int maxCPU = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MAX_CPU_NUMBER), Integer.MAX_VALUE);
int cpuNumber = NumbersUtil.parseInt(customParameters.get(UsageEventVO.DynamicParameters.cpuNumber.name()), -1);
Integer maxCPUCores = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value() == 0 ? Integer.MAX_VALUE: ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value();
if (cpuNumber < minCPU || cpuNumber > maxCPU || cpuNumber > maxCPUCores) {
throw new InvalidParameterValueException(String.format("Invalid CPU cores value, specify a value between %d and %d", minCPU, Math.min(maxCPUCores, maxCPU)));
}
} else if (customParameters.containsKey(UsageEventVO.DynamicParameters.cpuNumber.name())) {
throw new InvalidParameterValueException("The CPU cores of this offering id:" + serviceOffering.getUuid()
+ " is not customizable. This is predefined in the Template.");
}
if (serviceOffering.getSpeed() == null) {
String cpuSpeed = customParameters.get(UsageEventVO.DynamicParameters.cpuSpeed.name());
if ((cpuSpeed == null) || (NumbersUtil.parseInt(cpuSpeed, -1) <= 0)) {
throw new InvalidParameterValueException("Invalid CPU speed value, specify a value between 1 and " + Integer.MAX_VALUE);
}
} else if (!serviceOffering.isCustomCpuSpeedSupported() && customParameters.containsKey(UsageEventVO.DynamicParameters.cpuSpeed.name())) {
throw new InvalidParameterValueException("The CPU speed of this offering id:" + serviceOffering.getUuid()
+ " is not customizable. This is predefined in the Template.");
}
if (serviceOffering.getRamSize() == null) {
int minMemory = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MIN_MEMORY), 32);
int maxMemory = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MAX_MEMORY), Integer.MAX_VALUE);
int memory = NumbersUtil.parseInt(customParameters.get(UsageEventVO.DynamicParameters.memory.name()), -1);
Integer maxRAMSize = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_RAM_SIZE.value() == 0 ? Integer.MAX_VALUE: ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_RAM_SIZE.value();
if (memory < minMemory || memory > maxMemory || memory > maxRAMSize) {
throw new InvalidParameterValueException(String.format("Invalid memory value, specify a value between %d and %d", minMemory, Math.min(maxRAMSize, maxMemory)));
}
} else if (customParameters.containsKey(UsageEventVO.DynamicParameters.memory.name())) {
throw new InvalidParameterValueException("The memory of this offering id:" + serviceOffering.getUuid() + " is not customizable. This is predefined in the Template.");
}
} else {
if (MapUtils.isEmpty(customParameters) && serviceOffering.isDynamic()) {
throw new InvalidParameterValueException("Need to specify custom parameter values cpu, cpu speed and memory when using custom offering");
}
Map<String, String> offeringDetails = serviceOfferingDetailsDao.listDetailsKeyPairs(serviceOffering.getId());
if (serviceOffering.getCpu() == null) {
int minCPU = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MIN_CPU_NUMBER), 1);
int maxCPU = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MAX_CPU_NUMBER), Integer.MAX_VALUE);
int cpuNumber = NumbersUtil.parseInt(customParameters.get(UsageEventVO.DynamicParameters.cpuNumber.name()), -1);
int maxCPUCores = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value() == 0 ? Integer.MAX_VALUE: ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value();
if (cpuNumber < minCPU || cpuNumber > maxCPU || cpuNumber > maxCPUCores) {
throw new InvalidParameterValueException(String.format("Invalid CPU cores value, specify a value between %d and %d", minCPU, Math.min(maxCPUCores, maxCPU)));
}
} else if (customParameters.containsKey(UsageEventVO.DynamicParameters.cpuNumber.name())) {
throw new InvalidParameterValueException("The CPU cores of this offering id:" + serviceOffering.getUuid()
+ " is not customizable. This is predefined in the Template.");
}
if (serviceOffering.getSpeed() == null) {
String cpuSpeed = customParameters.get(UsageEventVO.DynamicParameters.cpuSpeed.name());
if ((cpuSpeed == null) || (NumbersUtil.parseInt(cpuSpeed, -1) <= 0)) {
throw new InvalidParameterValueException("Invalid CPU speed value, specify a value between 1 and " + Integer.MAX_VALUE);
}
} else if (!serviceOffering.isCustomCpuSpeedSupported() && customParameters.containsKey(UsageEventVO.DynamicParameters.cpuSpeed.name())) {
throw new InvalidParameterValueException(String.format("The CPU speed of this offering id:%s"
+ " is not customizable. This is predefined as %d MHz.",
serviceOffering.getUuid(), serviceOffering.getSpeed()));
}
if (serviceOffering.getRamSize() == null) {
int minMemory = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MIN_MEMORY), 32);
int maxMemory = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MAX_MEMORY), Integer.MAX_VALUE);
int memory = NumbersUtil.parseInt(customParameters.get(UsageEventVO.DynamicParameters.memory.name()), -1);
int maxRAMSize = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_RAM_SIZE.value() == 0 ? Integer.MAX_VALUE: ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_RAM_SIZE.value();
if (memory < minMemory || memory > maxMemory || memory > maxRAMSize) {
throw new InvalidParameterValueException(String.format("Invalid memory value, specify a value between %d and %d", minMemory, Math.min(maxRAMSize, maxMemory)));
}
} else if (customParameters.containsKey(UsageEventVO.DynamicParameters.memory.name())) {
throw new InvalidParameterValueException("The memory of this offering id:" + serviceOffering.getUuid() + " is not customizable. This is predefined in the Template.");
}
}
private UserVm upgradeStoppedVirtualMachine(Long vmId, Long svcOffId, Map<String, String> customParameters) throws ResourceAllocationException {
@ -2785,10 +2784,16 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
Map<String, String> customParameters = new HashMap<>();
customParameters.put(VmDetailConstants.CPU_NUMBER, String.valueOf(newCpu));
customParameters.put(VmDetailConstants.MEMORY, String.valueOf(newMemory));
if (svcOffering.isCustomCpuSpeedSupported()) {
if (details.containsKey(VmDetailConstants.CPU_SPEED)) {
customParameters.put(VmDetailConstants.CPU_SPEED, details.get(VmDetailConstants.CPU_SPEED));
}
validateCustomParameters(svcOffering, customParameters);
} else {
if (details.containsKey(VmDetailConstants.CPU_NUMBER) || details.containsKey(VmDetailConstants.MEMORY) ||
details.containsKey(VmDetailConstants.CPU_SPEED)) {
throw new InvalidParameterValueException("CPU number, Memory and CPU speed cannot be updated for a " +
"non-dynamic offering");
}
}
if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
return;

View File

@ -16,6 +16,25 @@
// under the License.
package com.cloud.hypervisor;
import java.io.UnsupportedEncodingException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.utils.bytescale.ByteScaleUtils;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
import com.cloud.agent.api.to.NicTO;
import com.cloud.agent.api.to.VirtualMachineTO;
import com.cloud.configuration.ConfigurationManagerImpl;
@ -34,23 +53,6 @@ import com.cloud.storage.dao.GuestOSHypervisorDao;
import com.cloud.utils.Pair;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.VirtualMachineProfile;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.utils.bytescale.ByteScaleUtils;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
import java.io.UnsupportedEncodingException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
@RunWith(MockitoJUnitRunner.class)
public class KVMGuruTest {
@ -111,8 +113,15 @@ public class KVMGuruTest {
private static final String detail2Key = "detail2";
private static final String detail2Value = "value2";
private ConfigKey<Integer> originalVmServiceOfferingMaxCpuCores;
private ConfigKey<Integer> originalVmServiceOfferingMaxRAMSize;
@Before
public void setup() throws UnsupportedEncodingException {
// Preserve the original value for restoration in tearDown
originalVmServiceOfferingMaxCpuCores = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES;
originalVmServiceOfferingMaxRAMSize = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_RAM_SIZE;
Mockito.when(vmTO.getLimitCpuUse()).thenReturn(true);
Mockito.when(vmProfile.getVirtualMachine()).thenReturn(vm);
Mockito.when(vm.getHostId()).thenReturn(hostId);
@ -134,6 +143,13 @@ public class KVMGuruTest {
Arrays.asList(detail1, detail2));
}
@After
public void tearDown() {
// Restore the original value
ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES = originalVmServiceOfferingMaxCpuCores;
ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_RAM_SIZE = originalVmServiceOfferingMaxRAMSize;
}
@Test
public void testSetVmQuotaPercentage() {
guru.setVmQuotaPercentage(vmTO, vmProfile);

View File

@ -38,13 +38,16 @@ import static org.mockito.Mockito.when;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import com.cloud.network.NetworkService;
import com.cloud.resourcelimit.CheckedReservation;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
import org.apache.cloudstack.api.command.admin.vm.AssignVMCmd;
import org.apache.cloudstack.api.command.user.vm.DeployVMCmd;
@ -67,6 +70,7 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.MockedConstruction;
import org.mockito.Mockito;
import org.mockito.Spy;
@ -83,6 +87,9 @@ import com.cloud.deploy.DataCenterDeployment;
import com.cloud.deploy.DeployDestination;
import com.cloud.deploy.DeploymentPlanner;
import com.cloud.deploy.DeploymentPlanningManager;
import com.cloud.domain.DomainVO;
import com.cloud.domain.dao.DomainDao;
import com.cloud.event.UsageEventUtils;
import com.cloud.exception.InsufficientAddressCapacityException;
import com.cloud.exception.InsufficientCapacityException;
import com.cloud.exception.InsufficientServerCapacityException;
@ -94,15 +101,34 @@ import com.cloud.host.Host;
import com.cloud.host.HostVO;
import com.cloud.host.dao.HostDao;
import com.cloud.hypervisor.Hypervisor;
import com.cloud.network.Network;
import com.cloud.network.NetworkModel;
import com.cloud.network.NetworkService;
import com.cloud.network.dao.FirewallRulesDao;
import com.cloud.network.dao.IPAddressDao;
import com.cloud.network.dao.IPAddressVO;
import com.cloud.network.dao.LoadBalancerVMMapDao;
import com.cloud.network.dao.LoadBalancerVMMapVO;
import com.cloud.network.dao.NetworkDao;
import com.cloud.network.dao.NetworkVO;
import com.cloud.network.dao.PhysicalNetworkDao;
import com.cloud.network.dao.PhysicalNetworkVO;
import com.cloud.network.guru.NetworkGuru;
import com.cloud.network.rules.FirewallRuleVO;
import com.cloud.network.rules.PortForwardingRule;
import com.cloud.network.rules.dao.PortForwardingRulesDao;
import com.cloud.network.security.SecurityGroupManager;
import com.cloud.network.security.SecurityGroupVO;
import com.cloud.offering.DiskOffering;
import com.cloud.offering.NetworkOffering;
import com.cloud.offering.ServiceOffering;
import com.cloud.offerings.NetworkOfferingVO;
import com.cloud.offerings.dao.NetworkOfferingDao;
import com.cloud.resourcelimit.CheckedReservation;
import com.cloud.server.ManagementService;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.service.dao.ServiceOfferingDetailsDao;
import com.cloud.storage.DiskOfferingVO;
import com.cloud.storage.GuestOSVO;
import com.cloud.storage.ScopeType;
@ -139,31 +165,6 @@ import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.UserVmDetailsDao;
import com.cloud.vm.snapshot.VMSnapshotVO;
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
import org.mockito.MockedStatic;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import com.cloud.domain.DomainVO;
import com.cloud.domain.dao.DomainDao;
import com.cloud.event.UsageEventUtils;
import com.cloud.network.Network;
import com.cloud.network.dao.FirewallRulesDao;
import com.cloud.network.dao.IPAddressDao;
import com.cloud.network.dao.IPAddressVO;
import com.cloud.network.dao.LoadBalancerVMMapDao;
import com.cloud.network.dao.LoadBalancerVMMapVO;
import com.cloud.network.dao.PhysicalNetworkDao;
import com.cloud.network.dao.PhysicalNetworkVO;
import com.cloud.network.guru.NetworkGuru;
import com.cloud.network.rules.FirewallRuleVO;
import com.cloud.network.rules.PortForwardingRule;
import com.cloud.network.rules.dao.PortForwardingRulesDao;
import com.cloud.network.security.SecurityGroupManager;
import com.cloud.offering.NetworkOffering;
import com.cloud.offerings.NetworkOfferingVO;
import com.cloud.offerings.dao.NetworkOfferingDao;
@RunWith(MockitoJUnitRunner.class)
public class UserVmManagerImplTest {
@ -373,6 +374,9 @@ public class UserVmManagerImplTest {
@Mock
NetworkService networkServiceMock;
@Mock
ServiceOfferingDetailsDao serviceOfferingDetailsDao;
private static final long vmId = 1l;
private static final long zoneId = 2L;
private static final long accountId = 3L;
@ -3111,4 +3115,96 @@ public class UserVmManagerImplTest {
Mockito.verify(userVmManagerImpl, Mockito.never()).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
}
}
private ServiceOfferingVO getMockedServiceOffering(boolean custom, boolean customSpeed) {
ServiceOfferingVO serviceOffering = mock(ServiceOfferingVO.class);
when(serviceOffering.getUuid()).thenReturn("offering-uuid");
when(serviceOffering.isDynamic()).thenReturn(custom);
when(serviceOffering.isCustomCpuSpeedSupported()).thenReturn(customSpeed);
if (custom) {
when(serviceOffering.getCpu()).thenReturn(null);
when(serviceOffering.getRamSize()).thenReturn(null);
}
if (customSpeed) {
when(serviceOffering.getSpeed()).thenReturn(null);
} else {
when(serviceOffering.isCustomCpuSpeedSupported()).thenReturn(false);
when(serviceOffering.getSpeed()).thenReturn(1000);
}
return serviceOffering;
}
@Test
public void customOfferingNeedsCustomizationThrowsException() {
ServiceOfferingVO serviceOffering = getMockedServiceOffering(true, true);
InvalidParameterValueException ex = Assert.assertThrows(InvalidParameterValueException.class, () ->
userVmManagerImpl.validateCustomParameters(serviceOffering, Collections.emptyMap()));
assertEquals("Need to specify custom parameter values cpu, cpu speed and memory when using custom offering", ex.getMessage());
}
@Test
public void cpuSpeedCustomizationNotAllowedThrowsException() {
ServiceOfferingVO serviceOffering = getMockedServiceOffering(true, false);
Map<String, String> customParameters = new HashMap<>();
customParameters.put(VmDetailConstants.CPU_NUMBER, "1");
customParameters.put(VmDetailConstants.CPU_SPEED, "2500");
InvalidParameterValueException ex = Assert.assertThrows(InvalidParameterValueException.class, () ->
userVmManagerImpl.validateCustomParameters(serviceOffering, customParameters));
Assert.assertTrue(ex.getMessage().startsWith("The CPU speed of this offering"));
}
@Test
public void cpuSpeedCustomizationAllowedDoesNotThrowException() {
ServiceOfferingVO serviceOffering = getMockedServiceOffering(true, true);
when(serviceOfferingDetailsDao.listDetailsKeyPairs(anyLong())).thenReturn(
Map.of(ApiConstants.MIN_CPU_NUMBER, "1",
ApiConstants.MAX_CPU_NUMBER, "4",
ApiConstants.MIN_MEMORY, "256",
ApiConstants.MAX_MEMORY, "8192"));
Map<String, String> customParameters = new HashMap<>();
customParameters.put(VmDetailConstants.CPU_NUMBER, "1");
customParameters.put(VmDetailConstants.CPU_SPEED, "2500");
customParameters.put(VmDetailConstants.MEMORY, "256");
userVmManagerImpl.validateCustomParameters(serviceOffering, customParameters);
}
@Test
public void verifyVmLimits_fixedOffering_throwsException() {
when(userVmVoMock.getId()).thenReturn(1L);
when(userVmVoMock.getServiceOfferingId()).thenReturn(1L);
when(accountDao.findById(anyLong())).thenReturn(callerAccount);
ServiceOfferingVO serviceOffering = getMockedServiceOffering(false, false);
when(_serviceOfferingDao.findById(anyLong())).thenReturn(serviceOffering);
when(_serviceOfferingDao.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(serviceOffering);
Map<String, String> customParameters = new HashMap<>();
customParameters.put(VmDetailConstants.CPU_SPEED, "2500");
InvalidParameterValueException ex = Assert.assertThrows(InvalidParameterValueException.class, () ->
userVmManagerImpl.verifyVmLimits(userVmVoMock, customParameters));
assertEquals("CPU number, Memory and CPU speed cannot be updated for a non-dynamic offering", ex.getMessage());
}
@Test
public void verifyVmLimits_constrainedOffering_throwsException() {
when(userVmVoMock.getId()).thenReturn(1L);
when(userVmVoMock.getServiceOfferingId()).thenReturn(1L);
when(accountDao.findById(anyLong())).thenReturn(callerAccount);
ServiceOfferingVO serviceOffering = getMockedServiceOffering(true, false);
when(_serviceOfferingDao.findById(anyLong())).thenReturn(serviceOffering);
when(_serviceOfferingDao.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(serviceOffering);
Map<String, String> customParameters = new HashMap<>();
customParameters.put(VmDetailConstants.CPU_NUMBER, "1");
customParameters.put(VmDetailConstants.CPU_SPEED, "2500");
InvalidParameterValueException ex = Assert.assertThrows(InvalidParameterValueException.class, () ->
userVmManagerImpl.verifyVmLimits(userVmVoMock, customParameters));
Assert.assertTrue(ex.getMessage().startsWith("The CPU speed of this offering"));
}
}