From 684e18f838b2d1070259a309396f55daa0533515 Mon Sep 17 00:00:00 2001 From: Jithin Raju Date: Mon, 19 Oct 2026 12:59:52 +0530 Subject: [PATCH] importVm: make clusterid optional clusterid is inherited from importUnmanagedInstance as a required parameter, but importVm with importsource external, local or shared never reads it: the deployment plan for disk imports is derived from the storage pool (and host for local storage). Make it optional at the API layer. The VMware and unmanaged import paths still reject a missing cluster in basicAccessChecks, now with a message naming the parameter. Co-Authored-By: Claude Opus 5.5 --- .../admin/vm/ImportUnmanagedInstanceCmd.java | 4 ++-- .../cloudstack/vm/UnmanagedVMsManagerImpl.java | 2 +- .../vm/UnmanagedVMsManagerImplTest.java | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java index fd18e44754a4..8d7db1294503 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java @@ -75,8 +75,8 @@ public class ImportUnmanagedInstanceCmd extends BaseAsyncCmd { @Parameter(name = ApiConstants.CLUSTER_ID, type = CommandType.UUID, entityType = ClusterResponse.class, - required = true, - description = "The cluster ID") + description = "The cluster ID. Required when importing an unmanaged Instance or importing from VMware. " + + "Not used when importing a KVM Instance from an external host, local storage or shared storage") private Long clusterId; @Parameter(name = ApiConstants.NAME, diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 585f00d80eb3..96ac43103a4d 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -1262,7 +1262,7 @@ protected Cluster basicAccessChecks(Long clusterId) { throw new PermissionDeniedException(String.format("Cannot perform this operation, caller account [%s] is not ROOT Admin.", caller.getUuid())); } if (clusterId == null) { - throw new InvalidParameterValueException("Cluster ID cannot be null."); + throw new InvalidParameterValueException(String.format("Cluster ID cannot be null, please specify the [%s] parameter.", ApiConstants.CLUSTER_ID)); } final Cluster cluster = clusterDao.findById(clusterId); if (cluster == null) { diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index 30bc769d5d4f..c4225056e003 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -1051,6 +1051,24 @@ private void importFromDisk(String source, Storage.StoragePoolType poolType, Str verify(deploymentPlanningManager).planDeployment(any(), planCaptor.capture(), any(), any()); Assert.assertEquals(poolPodId, planCaptor.getValue().getPodId()); Assert.assertEquals(poolClusterId, planCaptor.getValue().getClusterId()); + // the clusterid parameter is not used, the cluster is derived from the storage pool + Mockito.verify(cmd, Mockito.never()).getClusterId(); + } + + @Test(expected = InvalidParameterValueException.class) + public void testImportVmFromVmwareMissingClusterId() { + ImportVmCmd cmd = Mockito.mock(ImportVmCmd.class); + when(cmd.getName()).thenReturn("testVm"); + when(cmd.getImportSource()).thenReturn("vmware"); + unmanagedVMsManager.importVm(cmd); + } + + @Test(expected = InvalidParameterValueException.class) + public void testImportVmFromUnmanagedMissingClusterId() { + ImportVmCmd cmd = Mockito.mock(ImportVmCmd.class); + when(cmd.getName()).thenReturn("testVm"); + when(cmd.getImportSource()).thenReturn("unmanaged"); + unmanagedVMsManager.importVm(cmd); } @Test