Skip to content

server: prevent adding of compute details for vm when not applicable #12637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
shwstppr wants to merge 4 commits into
base: 4.20
Choose a base branch
from
+168 −67
Draft

server: prevent adding of compute details for vm when not applicable #12637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
56b2ea1
server: prevent adding of compute details for vm when using fixed off…
shwstppr Feb 12, 2026
3322abf
refactor, test
shwstppr Feb 23, 2026
e28e177
fix
shwstppr Feb 23, 2026
a84d2d7
explicitly set offering bounds
shwstppr Feb 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 46 additions & 41 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.ParserConfigurationException;

import com.cloud.network.NetworkService;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
Expand Down Expand Up @@ -134,8 +133,8 @@
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;
Expand Down Expand Up @@ -254,6 +253,7 @@
import com.cloud.network.Network.Provider;
import com.cloud.network.Network.Service;
import com.cloud.network.NetworkModel;
import com.cloud.network.NetworkService;
import com.cloud.network.Networks.TrafficType;
import com.cloud.network.PhysicalNetwork;
import com.cloud.network.as.AutoScaleManager;
Expand Down Expand Up @@ -1283,46 +1283,45 @@ private void validateOfferingMaxResource(ServiceOfferingVO offering) {

@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 {
Expand Down Expand Up @@ -2765,10 +2764,16 @@ protected void verifyVmLimits(UserVmVO vmInstance, Map<String, String> details)
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;
Expand Down
148 changes: 122 additions & 26 deletions server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,16 @@
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 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;
Expand All @@ -65,6 +69,7 @@
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
Expand All @@ -80,6 +85,9 @@
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;
Expand All @@ -91,15 +99,33 @@
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.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;
Expand Down Expand Up @@ -136,31 +162,6 @@
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 {
Expand Down Expand Up @@ -370,6 +371,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;
Expand Down Expand Up @@ -3121,4 +3125,96 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab
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"));
}
}
Loading