From 60a442fd06c092176a6171b6b5feccf58f2e092b Mon Sep 17 00:00:00 2001 From: Andrija Panic <45762285+andrijapanicsb@users.noreply.github.com> Date: Fri, 6 Nov 2026 23:26:49 +0100 Subject: [PATCH 1/2] CKS: prevent individual offering changes on managed cluster nodes Validate compute offering changes at both VM API entry points and hide individual scaling for CloudManaged Kubernetes cluster members. Keep ExternalManaged and ordinary instances unchanged, and preserve the internal resizing methods used by CKS cluster and node-type scaling. Fixes: #14553 Signed-off-by: Andrija Panic --- .../cluster/KubernetesServiceHelper.java | 1 + .../api/response/UserVmResponse.java | 12 + .../cluster/KubernetesServiceHelperImpl.java | 18 ++ .../KubernetesServiceHelperImplTest.java | 51 ++++ .../api/query/dao/UserVmJoinDaoImpl.java | 20 ++ .../java/com/cloud/vm/UserVmManagerImpl.java | 15 + .../dao/UserVmKubernetesClusterTypeTest.java | 71 +++++ .../cloud/vm/UserVmOfferingChangeTest.java | 259 ++++++++++++++++++ ui/src/config/section/compute.js | 2 +- ui/tests/unit/config/computeScale.spec.js | 55 ++++ 10 files changed, 503 insertions(+), 1 deletion(-) create mode 100644 server/src/test/java/com/cloud/api/query/dao/UserVmKubernetesClusterTypeTest.java create mode 100644 server/src/test/java/com/cloud/vm/UserVmOfferingChangeTest.java create mode 100644 ui/tests/unit/config/computeScale.spec.js diff --git a/api/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelper.java b/api/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelper.java index 5a6eaa3f7b9a..e107b9433436 100644 --- a/api/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelper.java +++ b/api/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelper.java @@ -35,6 +35,7 @@ enum KubernetesClusterNodeType { ControlledEntity findByVmId(long vmId); void checkVmCanBeDestroyed(UserVm userVm); void checkVmAffinityGroupsCanBeUpdated(UserVm userVm); + void checkVmServiceOfferingCanBeChanged(UserVm userVm); boolean isValidNodeType(String nodeType); Map getServiceOfferingNodeTypeMap(Map> serviceOfferingNodeTypeMap); Map getTemplateNodeTypeMap(Map> templateNodeTypeMap); diff --git a/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java index e08c6019f150..594b241055fc 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java @@ -448,6 +448,10 @@ public class UserVmResponse extends BaseResponseWithTagInformation implements Co @Param(description = "User VM type", since = "4.20.0") private String vmType; + @SerializedName("kubernetesclustertype") + @Param(description = "Management type of the Kubernetes cluster containing this instance") + private String kubernetesClusterType; + @SerializedName(ApiConstants.ARCH) @Param(description = "CPU arch of the VM", since = "4.20.1") private String arch; @@ -1326,6 +1330,14 @@ public void setVmType(String vmType) { this.vmType = vmType; } + public void setKubernetesClusterType(String kubernetesClusterType) { + this.kubernetesClusterType = kubernetesClusterType; + } + + public String getKubernetesClusterType() { + return kubernetesClusterType; + } + public String getVmType() { return vmType; } diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java index 9faeb56aea4e..be3919561718 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java @@ -150,6 +150,24 @@ public void checkVmAffinityGroupsCanBeUpdated(UserVm userVm) { throw new CloudRuntimeException(errorMessage); } + @Override + public void checkVmServiceOfferingCanBeChanged(UserVm userVm) { + // Adopted CloudManaged nodes need not have the cksnode VM type. + KubernetesClusterVmMapVO vmMap = kubernetesClusterVmMapDao.findByVmId(userVm.getId()); + if (vmMap == null) { + return; + } + KubernetesCluster cluster = kubernetesClusterDao.findById(vmMap.getClusterId()); + if (cluster != null && KubernetesCluster.ClusterType.ExternalManaged.equals(cluster.getClusterType())) { + return; + } + String message = "Compute offering cannot be changed for an instance belonging to a Kubernetes cluster"; + if (cluster != null) { + message += String.format(": %s", cluster.getName()); + } + throw new CloudRuntimeException(message + ". Use the Kubernetes cluster scale operation instead."); + } + @Override public boolean isValidNodeType(String nodeType) { if (StringUtils.isBlank(nodeType)) { diff --git a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImplTest.java b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImplTest.java index 3994cadc307f..e4fad4fdbcf5 100644 --- a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImplTest.java +++ b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImplTest.java @@ -103,6 +103,57 @@ public void testCheckVmCanBeDestroyedInExternalManagedCluster() { kubernetesServiceHelper.checkVmCanBeDestroyed(vm); } + @Test + public void testOfferingChangeAllowedForOrdinaryVm() { + UserVm vm = Mockito.mock(UserVm.class); + Mockito.when(vm.getId()).thenReturn(1L); + kubernetesServiceHelper.checkVmServiceOfferingCanBeChanged(vm); + Mockito.verify(kubernetesClusterDao, Mockito.never()).findById(Mockito.anyLong()); + } + + @Test + public void testOfferingChangeAllowedForUnmappedCksVm() { + UserVm vm = Mockito.mock(UserVm.class); + Mockito.when(vm.getId()).thenReturn(1L); + kubernetesServiceHelper.checkVmServiceOfferingCanBeChanged(vm); + } + + private UserVm mappedVm(KubernetesCluster.ClusterType type) { + UserVm vm = Mockito.mock(UserVm.class); + Mockito.when(vm.getId()).thenReturn(1L); + KubernetesClusterVmMapVO map = Mockito.mock(KubernetesClusterVmMapVO.class); + Mockito.when(map.getClusterId()).thenReturn(2L); + Mockito.when(kubernetesClusterVmMapDao.findByVmId(1L)).thenReturn(map); + if (type != null) { + KubernetesClusterVO cluster = Mockito.mock(KubernetesClusterVO.class); + Mockito.when(cluster.getClusterType()).thenReturn(type); + Mockito.when(kubernetesClusterDao.findById(2L)).thenReturn(cluster); + } + return vm; + } + + @Test(expected = CloudRuntimeException.class) + public void testOfferingChangeBlockedForCloudManagedNode() { + UserVm vm = mappedVm(KubernetesCluster.ClusterType.CloudManaged); + // The VM type is deliberately not consulted: membership is authoritative. + kubernetesServiceHelper.checkVmServiceOfferingCanBeChanged(vm); + } + + @Test(expected = CloudRuntimeException.class) + public void testOfferingChangeBlockedForAdoptedCloudManagedNode() { + kubernetesServiceHelper.checkVmServiceOfferingCanBeChanged(mappedVm(KubernetesCluster.ClusterType.CloudManaged)); + } + + @Test + public void testOfferingChangeAllowedForExternalManagedNode() { + kubernetesServiceHelper.checkVmServiceOfferingCanBeChanged(mappedVm(KubernetesCluster.ClusterType.ExternalManaged)); + } + + @Test(expected = CloudRuntimeException.class) + public void testOfferingChangeBlockedForMissingMappedCluster() { + kubernetesServiceHelper.checkVmServiceOfferingCanBeChanged(mappedVm(null)); + } + @Test public void testIsValidNodeTypeEmptyNodeType() { Assert.assertFalse(kubernetesServiceHelper.isValidNodeType(null)); diff --git a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java index ebc2326a72d5..999f899b45b5 100644 --- a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java @@ -55,6 +55,7 @@ import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.stereotype.Component; import com.cloud.api.ApiDBUtils; @@ -69,6 +70,8 @@ import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; import com.cloud.host.dao.HostDetailsDao; +import com.cloud.kubernetes.cluster.KubernetesCluster; +import com.cloud.kubernetes.cluster.KubernetesServiceHelper; import com.cloud.network.IpAddress; import com.cloud.network.vpc.VpcVO; import com.cloud.network.vpc.dao.VpcDao; @@ -92,6 +95,7 @@ import com.cloud.user.dao.UserDao; import com.cloud.user.dao.UserStatisticsDao; import com.cloud.uservm.UserVm; +import com.cloud.utils.component.ComponentContext; import com.cloud.utils.db.Filter; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; @@ -232,6 +236,7 @@ public UserVmResponse newUserVmResponse(ResponseView view, String objectName, Us userVmResponse.setLastUpdated(userVm.getLastUpdated()); userVmResponse.setDisplayVm(userVm.isDisplayVm()); userVmResponse.setVmType(userVm.getUserVmType()); + populateKubernetesClusterType(userVm.getId(), userVmResponse); if (userVm.getState() != null) { userVmResponse.setState(userVm.getState().toString()); @@ -580,6 +585,21 @@ private long computeLeaseDurationFromExpiryDate(Date created, Date leaseExpiryDa return ChronoUnit.DAYS.between(createdDate, expiryDate); } + void populateKubernetesClusterType(long vmId, UserVmResponse response) { + try { + KubernetesServiceHelper helper = ComponentContext.getDelegateComponentOfType(KubernetesServiceHelper.class); + Object cluster = helper.findByVmId(vmId); + if (cluster instanceof KubernetesCluster) { + KubernetesCluster.ClusterType type = ((KubernetesCluster) cluster).getClusterType(); + if (type != null) { + response.setKubernetesClusterType(type.name()); + } + } + } catch (NoSuchBeanDefinitionException ignored) { + logger.debug("No KubernetesClusterHelper bean found"); + } + } + int effectiveCdromMaxCount(UserVmJoinVO userVm) { Long hostId = userVm.getHostId() != null && userVm.getHostId() > 0 ? userVm.getHostId() : userVm.getLastHostId(); diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index d93dae9ebdec..1d80bcd6e714 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -1303,6 +1303,8 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE _accountMgr.checkAccess(caller, null, true, vmInstance); + checkPluginsIfVmServiceOfferingCanBeChanged(_vmDao.findById(vmId)); + upgradeStoppedVirtualMachine(vmId, svcOffId, cmd.getDetails()); // Generate usage event for VM upgrade @@ -2123,6 +2125,8 @@ public UserVm upgradeVirtualMachine(ScaleVMCmd cmd) throws ResourceUnavailableEx throw new InvalidParameterValueException(String.format("Operation not supported for instance: %s", vm.getName())); } + _accountMgr.checkAccess(CallContext.current().getCallingAccount(), null, true, vm); + checkPluginsIfVmServiceOfferingCanBeChanged(_vmDao.findById(vmId)); CallContext.current().setEventDetails("Vm Id: " + vm.getUuid()); Map cmdDetails = cmd.getDetails(); @@ -3676,6 +3680,17 @@ protected void checkExpungeVmPermission(Account callingAccount, String apiKey) { } } + // API boundary only: CKS scaling calls the internal upgradeVirtualMachine overloads. + protected void checkPluginsIfVmServiceOfferingCanBeChanged(UserVm vm) { + try { + KubernetesServiceHelper kubernetesServiceHelper = + ComponentContext.getDelegateComponentOfType(KubernetesServiceHelper.class); + kubernetesServiceHelper.checkVmServiceOfferingCanBeChanged(vm); + } catch (NoSuchBeanDefinitionException ignored) { + logger.debug("No KubernetesClusterHelper bean found"); + } + } + protected void checkPluginsIfVmCanBeDestroyed(UserVm vm) { try { KubernetesServiceHelper kubernetesServiceHelper = diff --git a/server/src/test/java/com/cloud/api/query/dao/UserVmKubernetesClusterTypeTest.java b/server/src/test/java/com/cloud/api/query/dao/UserVmKubernetesClusterTypeTest.java new file mode 100644 index 000000000000..f453f1c09ac4 --- /dev/null +++ b/server/src/test/java/com/cloud/api/query/dao/UserVmKubernetesClusterTypeTest.java @@ -0,0 +1,71 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.api.query.dao; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +import com.cloud.kubernetes.cluster.KubernetesCluster; +import com.cloud.kubernetes.cluster.KubernetesServiceHelper; +import com.cloud.utils.component.ComponentContext; +import org.apache.cloudstack.api.response.UserVmResponse; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; + +public class UserVmKubernetesClusterTypeTest { + @Test + public void reportsManagementTypeForUiIncludingAdoptedNodes() { + KubernetesServiceHelper helper = mock(KubernetesServiceHelper.class); + KubernetesCluster cluster = mock(KubernetesCluster.class); + when(helper.findByVmId(1L)).thenReturn(cluster); + try (MockedStatic components = mockStatic(ComponentContext.class)) { + components.when(() -> ComponentContext.getDelegateComponentOfType(KubernetesServiceHelper.class)).thenReturn(helper); + for (KubernetesCluster.ClusterType type : KubernetesCluster.ClusterType.values()) { + when(cluster.getClusterType()).thenReturn(type); + UserVmResponse response = new UserVmResponse(); + new UserVmJoinDaoImpl().populateKubernetesClusterType(1L, response); + assertEquals(type.name(), response.getKubernetesClusterType()); + } + } + } + + @Test + public void ordinaryVmHasNoClusterType() { + KubernetesServiceHelper helper = mock(KubernetesServiceHelper.class); + try (MockedStatic components = mockStatic(ComponentContext.class)) { + components.when(() -> ComponentContext.getDelegateComponentOfType(KubernetesServiceHelper.class)).thenReturn(helper); + UserVmResponse response = new UserVmResponse(); + new UserVmJoinDaoImpl().populateKubernetesClusterType(1L, response); + assertNull(response.getKubernetesClusterType()); + } + } + + @Test + public void absentPluginHasNoClusterType() { + try (MockedStatic components = mockStatic(ComponentContext.class)) { + components.when(() -> ComponentContext.getDelegateComponentOfType(KubernetesServiceHelper.class)) + .thenThrow(new NoSuchBeanDefinitionException(KubernetesServiceHelper.class)); + UserVmResponse response = new UserVmResponse(); + new UserVmJoinDaoImpl().populateKubernetesClusterType(1L, response); + assertNull(response.getKubernetesClusterType()); + } + } +} diff --git a/server/src/test/java/com/cloud/vm/UserVmOfferingChangeTest.java b/server/src/test/java/com/cloud/vm/UserVmOfferingChangeTest.java new file mode 100644 index 000000000000..a4dfafb87218 --- /dev/null +++ b/server/src/test/java/com/cloud/vm/UserVmOfferingChangeTest.java @@ -0,0 +1,259 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.vm; + +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyFloat; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.HashMap; + +import org.apache.cloudstack.api.command.admin.vm.ScaleVMCmdByAdmin; +import org.apache.cloudstack.api.command.admin.vm.UpgradeVMCmdByAdmin; +import org.apache.cloudstack.api.command.user.vm.ScaleVMCmd; +import org.apache.cloudstack.api.command.user.vm.UpgradeVMCmd; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; + +import com.cloud.api.ApiDBUtils; +import com.cloud.capacity.CapacityManager; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; +import com.cloud.user.dao.AccountDao; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.impl.ConfigDepotImpl; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.kubernetes.cluster.KubernetesServiceHelper; +import com.cloud.service.ServiceOfferingVO; +import com.cloud.service.dao.ServiceOfferingDao; +import com.cloud.storage.dao.DiskOfferingDao; +import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.user.AccountVO; +import com.cloud.user.AccountManager; +import com.cloud.user.ResourceLimitService; +import com.cloud.user.User; +import com.cloud.utils.component.ComponentContext; +import com.cloud.utils.db.EntityManager; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.dao.UserVmDao; +import com.cloud.vm.dao.VMInstanceDao; + +@RunWith(MockitoJUnitRunner.class) +public class UserVmOfferingChangeTest { + @Spy @InjectMocks private UserVmManagerImpl manager = new UserVmManagerImpl(); + @Mock private EntityManager _entityMgr; + @Mock private UserVmDao _vmDao; + @Mock private VMInstanceDao _vmInstanceDao; + @Mock private AccountManager _accountMgr; + @Mock private ServiceOfferingDao serviceOfferingDao; + @Mock private VirtualMachineManager _itMgr; + @Mock private ResourceLimitService _resourceLimitMgr; + @Mock private VMTemplateDao _templateDao; + @Mock private DataCenterDao _dcDao; + @Mock private DiskOfferingDao _diskOfferingDao; + @Mock private VolumeDao _volsDao; + @Mock private VolumeService _volService; + @Mock private AccountDao _accountDao; + @Mock private HostDao _hostDao; + @Mock private CapacityManager _capacityMgr; + @Mock private KubernetesServiceHelper helper; + @Mock private UserVmVO vm; + @Mock private AccountVO account; + private MockedStatic components; + + @Before + public void setup() { + CallContext.register(mock(User.class), account); + components = mockStatic(ComponentContext.class); + components.when(() -> ComponentContext.getDelegateComponentOfType(KubernetesServiceHelper.class)).thenReturn(helper); + } + + @After + public void cleanup() { + if (components != null) { + components.close(); + } + CallContext.unregister(); + } + + private void scaleApiBlocked(ScaleVMCmd cmd, VirtualMachine.State state) throws Exception { + lenient().when(vm.getState()).thenReturn(state); + when(cmd.getId()).thenReturn(1L); + when(_entityMgr.findById(VirtualMachine.class, 1L)).thenReturn(vm); + when(vm.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(_vmDao.findById(1L)).thenReturn(vm); + doThrow(new CloudRuntimeException("Use cluster scaling")).when(helper).checkVmServiceOfferingCanBeChanged(vm); + assertThrows(CloudRuntimeException.class, () -> manager.upgradeVirtualMachine(cmd)); + verify(_accountMgr).checkAccess(account, null, true, vm); + verify(manager, never()).updateInstanceDetailsMapWithCurrentValuesForAbsentDetails(anyMap(), any(), any()); + verify(manager, never()).upgradeVirtualMachine(anyLong(), anyLong(), anyMap()); + verifyNoInteractions(_itMgr, serviceOfferingDao); + } + + @Test public void blocksRunningScaleApi() throws Exception { + scaleApiBlocked(mock(ScaleVMCmd.class), VirtualMachine.State.Running); + } + @Test public void blocksStoppedScaleApi() throws Exception { + scaleApiBlocked(mock(ScaleVMCmd.class), VirtualMachine.State.Stopped); + } + @Test public void blocksAdminScaleApi() throws Exception { + scaleApiBlocked(mock(ScaleVMCmdByAdmin.class), VirtualMachine.State.Running); + } + + private void deprecatedApiBlocked(UpgradeVMCmd cmd) throws Exception { + when(cmd.getId()).thenReturn(1L); + when(_vmInstanceDao.findById(1L)).thenReturn(vm); + when(vm.getState()).thenReturn(VirtualMachine.State.Stopped); + when(_vmDao.findById(1L)).thenReturn(vm); + doThrow(new CloudRuntimeException("Use cluster scaling")).when(helper).checkVmServiceOfferingCanBeChanged(vm); + assertThrows(CloudRuntimeException.class, () -> manager.upgradeVirtualMachine(cmd)); + verify(_accountMgr).checkAccess(account, null, true, vm); + verifyNoInteractions(_itMgr, serviceOfferingDao); + } + + @Test public void blocksDeprecatedApi() throws Exception { + deprecatedApiBlocked(mock(UpgradeVMCmd.class)); + } + @Test public void blocksAdminDeprecatedApi() throws Exception { + deprecatedApiBlocked(mock(UpgradeVMCmdByAdmin.class)); + } + + @Test public void allowsScaleApiWhenPluginPermits() throws Exception { + ScaleVMCmd cmd = mock(ScaleVMCmd.class); + HashMap details = new HashMap<>(); + when(cmd.getId()).thenReturn(1L); + when(cmd.getServiceOfferingId()).thenReturn(2L); + when(cmd.getDetails()).thenReturn(details); + when(_entityMgr.findById(VirtualMachine.class, 1L)).thenReturn(vm); + when(vm.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(_vmDao.findById(1L)).thenReturn(vm); + doNothing().when(manager).updateInstanceDetailsMapWithCurrentValuesForAbsentDetails(details, vm, 2L); + doReturn(true).when(manager).upgradeVirtualMachine(1L, 2L, details); + when(vm.getState()).thenReturn(VirtualMachine.State.Running); + assertSame(vm, manager.upgradeVirtualMachine(cmd)); + verify(helper).checkVmServiceOfferingCanBeChanged(vm); + } + + @Test public void allowsMissingKubernetesPlugin() { + components.when(() -> ComponentContext.getDelegateComponentOfType(KubernetesServiceHelper.class)) + .thenThrow(new NoSuchBeanDefinitionException(KubernetesServiceHelper.class)); + manager.checkPluginsIfVmServiceOfferingCanBeChanged(vm); + } + + @Test public void internalStoppedResizeDoesNotInvokeApiGuard() throws Exception { + when(_vmInstanceDao.findById(1L)).thenReturn(vm); + when(vm.getId()).thenReturn(1L); + when(vm.getState()).thenReturn(VirtualMachine.State.Stopped); + when(vm.getServiceOfferingId()).thenReturn(2L); + ServiceOfferingVO oldOffering = mock(ServiceOfferingVO.class); + ServiceOfferingVO newOffering = mock(ServiceOfferingVO.class); + when(serviceOfferingDao.findById(3L)).thenReturn(newOffering); + when(serviceOfferingDao.findByIdIncludingRemoved(1L, 2L)).thenReturn(oldOffering); + when(newOffering.getCpu()).thenReturn(3); + when(newOffering.getRamSize()).thenReturn(3072); + when(oldOffering.getCpu()).thenReturn(2); + when(oldOffering.getRamSize()).thenReturn(2048); + when(_accountMgr.getActiveAccountById(anyLong())).thenReturn(account); + doNothing().when(manager).validateDiskOfferingChecks(oldOffering, newOffering); + lenient().when(_volsDao.findReadyAndAllocatedRootVolumesByInstance(1L)).thenReturn(Collections.emptyList()); + assertTrue(manager.upgradeVirtualMachine(1L, 3L, new HashMap<>())); + verify(_itMgr).upgradeVmDb(1L, newOffering, oldOffering); + verifyNoInteractions(helper); + components.verifyNoInteractions(); + } + + @Test public void internalLiveResizeDoesNotInvokeApiGuard() throws Exception { + Field depotField = ConfigKey.class.getDeclaredField("s_depot"); + depotField.setAccessible(true); + ConfigDepotImpl originalDepot = (ConfigDepotImpl) depotField.get(null); + ConfigDepotImpl depot = mock(ConfigDepotImpl.class); + when(depot.getConfigStringValue(eq(UserVmManager.EnableDynamicallyScaleVm.key()), any(), any())) + .thenReturn("true"); + Field retryField = UserVmManagerImpl.class.getDeclaredField("_scaleRetry"); + retryField.setAccessible(true); + retryField.set(manager, 1); + ConfigKey.init(depot); + try (MockedStatic apiDb = mockStatic(ApiDBUtils.class)) { + HostVO host = mock(HostVO.class); + when(_vmInstanceDao.findById(1L)).thenReturn(vm); + when(vm.getId()).thenReturn(1L); + when(vm.getState()).thenReturn(VirtualMachine.State.Running); + when(vm.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(vm.getServiceOfferingId()).thenReturn(2L); + when(vm.isDynamicallyScalable()).thenReturn(true); + when(_accountDao.findById(anyLong())).thenReturn(account); + when(_hostDao.findById(anyLong())).thenReturn(host); + apiDb.when(() -> ApiDBUtils.findHostById(vm.getHostId())).thenReturn(host); + ServiceOfferingVO oldOffering = mock(ServiceOfferingVO.class); + ServiceOfferingVO newOffering = mock(ServiceOfferingVO.class); + when(serviceOfferingDao.findById(3L)).thenReturn(newOffering); + when(serviceOfferingDao.findByIdIncludingRemoved(1L, 2L)).thenReturn(oldOffering); + when(newOffering.getCpu()).thenReturn(3); + when(newOffering.getRamSize()).thenReturn(3072); + when(newOffering.getSpeed()).thenReturn(1000); + when(newOffering.getDetails()).thenReturn(Collections.emptyMap()); + when(oldOffering.getCpu()).thenReturn(2); + when(oldOffering.getRamSize()).thenReturn(2048); + when(oldOffering.getSpeed()).thenReturn(1000); + when(oldOffering.getDetails()).thenReturn(Collections.emptyMap()); + when(host.checkHostServiceOfferingAndTemplateTags(eq(newOffering), any(), any())).thenReturn(true); + when(_capacityMgr.checkIfHostHasCpuCapability(host, 3, 1000)).thenReturn(true); + when(_capacityMgr.checkIfHostHasCapacity(eq(host), any(), anyLong(), eq(false), anyFloat(), anyFloat(), eq(false))) + .thenReturn(true); + doReturn(true).when(manager).checkEnforceStrictHostTagCheck(vm, host); + doNothing().when(manager).validateDiskOfferingChecks(oldOffering, newOffering); + lenient().when(_volsDao.findReadyAndAllocatedRootVolumesByInstance(1L)).thenReturn(Collections.emptyList()); + HashMap details = new HashMap<>(); + assertTrue(manager.upgradeVirtualMachine(1L, 3L, details)); + verify(_itMgr).reConfigureVm(vm.getUuid(), oldOffering, newOffering, details, true); + verifyNoInteractions(helper); + components.verifyNoInteractions(); + } finally { + ConfigKey.init(originalDepot); + } + } + +} diff --git a/ui/src/config/section/compute.js b/ui/src/config/section/compute.js index 7c2c9f09eae4..7e2a36117a98 100644 --- a/ui/src/config/section/compute.js +++ b/ui/src/config/section/compute.js @@ -356,7 +356,7 @@ export default { label: 'label.scale.vm', docHelp: 'adminguide/virtual_machines.html#how-to-dynamically-scale-cpu-and-ram', dataView: true, - show: (record) => { return record.hypervisor !== 'External' && (['Stopped'].includes(record.state) || (['Running'].includes(record.state) && record.hypervisor !== 'LXC')) && record.vmtype !== 'sharedfsvm' }, + show: (record) => { return record.hypervisor !== 'External' && (['Stopped'].includes(record.state) || (['Running'].includes(record.state) && record.hypervisor !== 'LXC')) && record.vmtype !== 'sharedfsvm' && record.kubernetesclustertype !== 'CloudManaged' && (record.kubernetesclustertype === 'ExternalManaged' || record.vmtype !== 'cksnode') }, disabled: (record) => { return record.state === 'Running' && !record.isdynamicallyscalable }, popup: true, component: shallowRef(defineAsyncComponent(() => import('@/views/compute/ScaleVM.vue'))) diff --git a/ui/tests/unit/config/computeScale.spec.js b/ui/tests/unit/config/computeScale.spec.js new file mode 100644 index 000000000000..584bae139ce2 --- /dev/null +++ b/ui/tests/unit/config/computeScale.spec.js @@ -0,0 +1,55 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import compute from '@/config/section/compute' + +jest.mock('@/store', () => ({})) +jest.mock('@/api', () => ({})) +jest.mock('@/utils/zone', () => ({})) +jest.mock('@/utils/acsrepo', () => ({})) +jest.mock('@/utils/util', () => ({})) +jest.mock('@/assets/icons/kubernetes.svg?inline', () => ({})) + +const action = compute.children.find(section => section.name === 'vm').actions + .find(action => action.api === 'scaleVirtualMachine') + +describe('instance compute offering changes', () => { + const vm = { hypervisor: 'KVM', state: 'Stopped' } + + it.each(['Stopped', 'Running'])('hides CloudManaged nodes in %s state', state => { + expect(action.show({ ...vm, state, vmtype: 'cksnode', kubernetesclustertype: 'CloudManaged' })).toBe(false) + expect(action.show({ ...vm, state, kubernetesclustertype: 'CloudManaged' })).toBe(false) + }) + + it('keeps ExternalManaged and ordinary instance actions', () => { + expect(action.show(vm)).toBe(true) + expect(action.show({ ...vm, kubernetesclustertype: 'ExternalManaged' })).toBe(true) + expect(action.show({ ...vm, vmtype: 'cksnode', kubernetesclustertype: 'ExternalManaged' })).toBe(true) + }) + + it('falls back to the existing VM type for older servers', () => { + expect(action.show({ ...vm, vmtype: 'cksnode' })).toBe(false) + }) + + it('retains existing hypervisor and state restrictions', () => { + expect(action.show({ ...vm, hypervisor: 'External' })).toBe(false) + expect(action.show({ ...vm, state: 'Running', hypervisor: 'LXC' })).toBe(false) + expect(action.show({ ...vm, state: 'Destroyed' })).toBe(false) + expect(action.show({ ...vm, vmtype: 'sharedfsvm' })).toBe(false) + expect(action.disabled({ ...vm, state: 'Running', isdynamicallyscalable: false })).toBe(true) + }) +}) From a263ee2f41d2d64c8cc1ed07e92fc9419c311d0d Mon Sep 17 00:00:00 2001 From: Andrija Panic Date: Sun, 8 Nov 2026 00:04:52 +0100 Subject: [PATCH 2/2] CKS: show the node offering change error to the instance owner The deprecated changeServiceForVirtualMachine API is synchronous, so the CloudRuntimeException from the guard reached the generic API handler, and non-admin owners got "Internal error executing command" instead of the reason. Throw InvalidParameterValueException instead: the API shows its message to every caller. The guard still runs at the same place, before any change, and internal cluster scaling is not affected. --- .../kubernetes/cluster/KubernetesServiceHelperImpl.java | 3 ++- .../kubernetes/cluster/KubernetesServiceHelperImplTest.java | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java index be3919561718..e2cccd3e2e25 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java @@ -165,7 +165,8 @@ public void checkVmServiceOfferingCanBeChanged(UserVm userVm) { if (cluster != null) { message += String.format(": %s", cluster.getName()); } - throw new CloudRuntimeException(message + ". Use the Kubernetes cluster scale operation instead."); + // A parameter error, so the API also shows the reason to non-admin callers. + throw new InvalidParameterValueException(message + ". Use the Kubernetes cluster scale operation instead."); } @Override diff --git a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImplTest.java b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImplTest.java index e4fad4fdbcf5..456117c321f7 100644 --- a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImplTest.java +++ b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImplTest.java @@ -132,14 +132,14 @@ private UserVm mappedVm(KubernetesCluster.ClusterType type) { return vm; } - @Test(expected = CloudRuntimeException.class) + @Test(expected = InvalidParameterValueException.class) public void testOfferingChangeBlockedForCloudManagedNode() { UserVm vm = mappedVm(KubernetesCluster.ClusterType.CloudManaged); // The VM type is deliberately not consulted: membership is authoritative. kubernetesServiceHelper.checkVmServiceOfferingCanBeChanged(vm); } - @Test(expected = CloudRuntimeException.class) + @Test(expected = InvalidParameterValueException.class) public void testOfferingChangeBlockedForAdoptedCloudManagedNode() { kubernetesServiceHelper.checkVmServiceOfferingCanBeChanged(mappedVm(KubernetesCluster.ClusterType.CloudManaged)); } @@ -149,7 +149,7 @@ public void testOfferingChangeAllowedForExternalManagedNode() { kubernetesServiceHelper.checkVmServiceOfferingCanBeChanged(mappedVm(KubernetesCluster.ClusterType.ExternalManaged)); } - @Test(expected = CloudRuntimeException.class) + @Test(expected = InvalidParameterValueException.class) public void testOfferingChangeBlockedForMissingMappedCluster() { kubernetesServiceHelper.checkVmServiceOfferingCanBeChanged(mappedVm(null)); }