From 1c84ce4e23e3fd243022c6c533fc14c10439c6f3 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 29 Jan 2025 16:44:16 +0530 Subject: [PATCH 01/14] server: fix attach uploaded volume (#10267) * server: fix attach uploaded volume Fixes #10120 When an uploaded volume is attached to a VM for which no existing volume can be found it was resulting in error. For such volumes, server needs to find a suitable pool first and copy them to the pool from secondary store. Signed-off-by: Abhishek Kumar * fix Signed-off-by: Abhishek Kumar * fix Signed-off-by: Abhishek Kumar * add unit tests Signed-off-by: Abhishek Kumar --------- Signed-off-by: Abhishek Kumar Co-authored-by: Boris Stoyanov - a.k.a Bobby --- .../cloud/storage/VolumeApiServiceImpl.java | 123 ++++++--- .../storage/VolumeApiServiceImplTest.java | 249 ++++++++++++++++++ 2 files changed, 330 insertions(+), 42 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 7f867eb01a9..68546f185b7 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -133,7 +133,9 @@ import com.cloud.dc.ClusterDetailsDao; import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; import com.cloud.dc.Pod; +import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.HostPodDao; import com.cloud.domain.Domain; import com.cloud.domain.dao.DomainDao; import com.cloud.event.ActionEvent; @@ -153,6 +155,7 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.HypervisorCapabilitiesVO; import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao; import com.cloud.offering.DiskOffering; +import com.cloud.org.Cluster; import com.cloud.org.Grouping; import com.cloud.projects.Project; import com.cloud.projects.ProjectManager; @@ -323,6 +326,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic @Inject private VmWorkJobDao _workJobDao; @Inject + ClusterDao clusterDao; + @Inject private ClusterDetailsDao _clusterDetailsDao; @Inject private StorageManager storageMgr; @@ -346,6 +351,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic protected ProjectManager projectManager; @Inject protected StoragePoolDetailsDao storagePoolDetailsDao; + @Inject + HostPodDao podDao; protected Gson _gson; @@ -2380,17 +2387,10 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic return attachVolumeToVM(command.getVirtualMachineId(), command.getId(), command.getDeviceId()); } - private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long deviceId) { - VolumeInfo volumeToAttach = volFactory.getVolume(volumeId); - - if (volumeToAttach.isAttachedVM()) { - throw new CloudRuntimeException("This volume is already attached to a VM."); - } - - UserVmVO vm = _userVmDao.findById(vmId); + protected VolumeVO getVmExistingVolumeForVolumeAttach(UserVmVO vm, VolumeInfo volumeToAttach) { VolumeVO existingVolumeOfVm = null; VMTemplateVO template = _templateDao.findById(vm.getTemplateId()); - List rootVolumesOfVm = _volsDao.findByInstanceAndType(vmId, Volume.Type.ROOT); + List rootVolumesOfVm = _volsDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT); if (rootVolumesOfVm.size() > 1 && template != null && !template.isDeployAsIs()) { throw new CloudRuntimeException("The VM " + vm.getHostName() + " has more than one ROOT volume and is in an invalid state."); } else { @@ -2398,7 +2398,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic existingVolumeOfVm = rootVolumesOfVm.get(0); } else { // locate data volume of the vm - List diskVolumesOfVm = _volsDao.findByInstanceAndType(vmId, Volume.Type.DATADISK); + List diskVolumesOfVm = _volsDao.findByInstanceAndType(vm.getId(), Volume.Type.DATADISK); for (VolumeVO diskVolume : diskVolumesOfVm) { if (diskVolume.getState() != Volume.State.Allocated) { existingVolumeOfVm = diskVolume; @@ -2407,44 +2407,88 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } } } + if (existingVolumeOfVm == null) { + if (s_logger.isTraceEnabled()) { + s_logger.trace(String.format("No existing volume found for VM (%s/%s) to attach volume %s/%s", + vm.getName(), vm.getUuid(), + volumeToAttach.getName(), volumeToAttach.getUuid())); + } + return null; + } if (s_logger.isTraceEnabled()) { String msg = "attaching volume %s/%s to a VM (%s/%s) with an existing volume %s/%s on primary storage %s"; - if (existingVolumeOfVm != null) { - s_logger.trace(String.format(msg, - volumeToAttach.getName(), volumeToAttach.getUuid(), - vm.getName(), vm.getUuid(), - existingVolumeOfVm.getName(), existingVolumeOfVm.getUuid(), - existingVolumeOfVm.getPoolId())); - } + s_logger.trace(String.format(msg, + volumeToAttach.getName(), volumeToAttach.getUuid(), + vm.getName(), vm.getUuid(), + existingVolumeOfVm.getName(), existingVolumeOfVm.getUuid(), + existingVolumeOfVm.getPoolId())); } + return existingVolumeOfVm; + } - HypervisorType rootDiskHyperType = vm.getHypervisorType(); - HypervisorType volumeToAttachHyperType = _volsDao.getHypervisorType(volumeToAttach.getId()); + protected StoragePool getPoolForAllocatedOrUploadedVolumeForAttach(final VolumeInfo volumeToAttach, final UserVmVO vm) { + DataCenter zone = _dcDao.findById(vm.getDataCenterId()); + Pair clusterHostId = virtualMachineManager.findClusterAndHostIdForVm(vm, false); + long podId = vm.getPodIdToDeployIn(); + if (clusterHostId.first() != null) { + Cluster cluster = clusterDao.findById(clusterHostId.first()); + podId = cluster.getPodId(); + } + Pod pod = podDao.findById(podId); + DiskOfferingVO offering = _diskOfferingDao.findById(volumeToAttach.getDiskOfferingId()); + DiskProfile diskProfile = new DiskProfile(volumeToAttach.getId(), volumeToAttach.getVolumeType(), + volumeToAttach.getName(), volumeToAttach.getId(), volumeToAttach.getSize(), offering.getTagsArray(), + offering.isUseLocalStorage(), offering.isRecreatable(), + volumeToAttach.getTemplateId()); + diskProfile.setHyperType(vm.getHypervisorType()); + StoragePool pool = _volumeMgr.findStoragePool(diskProfile, zone, pod, clusterHostId.first(), + clusterHostId.second(), vm, Collections.emptySet()); + if (pool == null) { + throw new CloudRuntimeException(String.format("Failed to find a primary storage for volume in state: %s", volumeToAttach.getState())); + } + return pool; + } + protected VolumeInfo createVolumeOnPrimaryForAttachIfNeeded(final VolumeInfo volumeToAttach, final UserVmVO vm, VolumeVO existingVolumeOfVm) { VolumeInfo newVolumeOnPrimaryStorage = volumeToAttach; - + boolean volumeOnSecondary = volumeToAttach.getState() == Volume.State.Uploaded; + if (!Arrays.asList(Volume.State.Allocated, Volume.State.Uploaded).contains(volumeToAttach.getState())) { + return newVolumeOnPrimaryStorage; + } //don't create volume on primary storage if its being attached to the vm which Root's volume hasn't been created yet - StoragePoolVO destPrimaryStorage = null; + StoragePool destPrimaryStorage = null; if (existingVolumeOfVm != null && !existingVolumeOfVm.getState().equals(Volume.State.Allocated)) { destPrimaryStorage = _storagePoolDao.findById(existingVolumeOfVm.getPoolId()); if (s_logger.isTraceEnabled() && destPrimaryStorage != null) { s_logger.trace(String.format("decided on target storage: %s/%s", destPrimaryStorage.getName(), destPrimaryStorage.getUuid())); } } - - boolean volumeOnSecondary = volumeToAttach.getState() == Volume.State.Uploaded; - - if (destPrimaryStorage != null && (volumeToAttach.getState() == Volume.State.Allocated || volumeOnSecondary)) { - try { - if (volumeOnSecondary && destPrimaryStorage.getPoolType() == Storage.StoragePoolType.PowerFlex) { - throw new InvalidParameterValueException("Cannot attach uploaded volume, this operation is unsupported on storage pool type " + destPrimaryStorage.getPoolType()); - } - newVolumeOnPrimaryStorage = _volumeMgr.createVolumeOnPrimaryStorage(vm, volumeToAttach, rootDiskHyperType, destPrimaryStorage); - } catch (NoTransitionException e) { - s_logger.debug("Failed to create volume on primary storage", e); - throw new CloudRuntimeException("Failed to create volume on primary storage", e); - } + if (destPrimaryStorage == null) { + destPrimaryStorage = getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); } + try { + if (volumeOnSecondary && Storage.StoragePoolType.PowerFlex.equals(destPrimaryStorage.getPoolType())) { + throw new InvalidParameterValueException("Cannot attach uploaded volume, this operation is unsupported on storage pool type " + destPrimaryStorage.getPoolType()); + } + newVolumeOnPrimaryStorage = _volumeMgr.createVolumeOnPrimaryStorage(vm, volumeToAttach, + vm.getHypervisorType(), destPrimaryStorage); + } catch (NoTransitionException e) { + s_logger.debug("Failed to create volume on primary storage", e); + throw new CloudRuntimeException("Failed to create volume on primary storage", e); + } + return newVolumeOnPrimaryStorage; + } + + private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long deviceId) { + VolumeInfo volumeToAttach = volFactory.getVolume(volumeId); + + if (volumeToAttach.isAttachedVM()) { + throw new CloudRuntimeException("This volume is already attached to a VM."); + } + + UserVmVO vm = _userVmDao.findById(vmId); + VolumeVO existingVolumeOfVm = getVmExistingVolumeForVolumeAttach(vm, volumeToAttach); + VolumeInfo newVolumeOnPrimaryStorage = createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, existingVolumeOfVm); // reload the volume from db newVolumeOnPrimaryStorage = volFactory.getVolume(newVolumeOnPrimaryStorage.getId()); @@ -2463,19 +2507,17 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic StoragePoolVO vmRootVolumePool = _storagePoolDao.findById(existingVolumeOfVm.getPoolId()); try { + HypervisorType volumeToAttachHyperType = _volsDao.getHypervisorType(volumeToAttach.getId()); newVolumeOnPrimaryStorage = _volumeMgr.moveVolume(newVolumeOnPrimaryStorage, vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(), vmRootVolumePool.getClusterId(), volumeToAttachHyperType); - } catch (ConcurrentOperationException e) { - s_logger.debug("move volume failed", e); - throw new CloudRuntimeException("move volume failed", e); - } catch (StorageUnavailableException e) { + } catch (ConcurrentOperationException | StorageUnavailableException e) { s_logger.debug("move volume failed", e); throw new CloudRuntimeException("move volume failed", e); } } VolumeVO newVol = _volsDao.findById(newVolumeOnPrimaryStorage.getId()); // Getting the fresh vm object in case of volume migration to check the current state of VM - if (moveVolumeNeeded || volumeOnSecondary) { + if (moveVolumeNeeded) { vm = _userVmDao.findById(vmId); if (vm == null) { throw new InvalidParameterValueException("VM not found."); @@ -2659,9 +2701,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic if (!_volsDao.findByInstanceAndDeviceId(vm.getId(), 0).isEmpty()) { throw new InvalidParameterValueException("Vm already has root volume attached to it"); } - if (volumeToAttach.getState() == Volume.State.Uploaded) { - throw new InvalidParameterValueException("No support for Root volume attach in state " + Volume.State.Uploaded); - } } } diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index a0f89956df5..3e6f9ec63f9 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -45,6 +45,7 @@ import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd; import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd; import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; @@ -86,8 +87,12 @@ import org.springframework.test.util.ReflectionTestUtils; import com.cloud.api.query.dao.ServiceOfferingJoinDao; import com.cloud.configuration.Resource; import com.cloud.configuration.Resource.ResourceType; +import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenterVO; +import com.cloud.dc.HostPodVO; +import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.HostPodDao; import com.cloud.event.EventTypes; import com.cloud.event.UsageEventUtils; import com.cloud.exception.InvalidParameterValueException; @@ -122,10 +127,12 @@ import com.cloud.utils.Pair; import com.cloud.utils.db.TransactionLegacy; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.fsm.NoTransitionException; +import com.cloud.vm.DiskProfile; import com.cloud.vm.UserVmManager; import com.cloud.vm.UserVmVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachine.State; +import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; import com.cloud.vm.snapshot.VMSnapshotVO; @@ -199,6 +206,15 @@ public class VolumeApiServiceImplTest { private DataStoreManager dataStoreMgr; @Mock private SnapshotHelper snapshotHelper; + @Mock + VirtualMachineManager virtualMachineManager; + @Mock + HostPodDao podDao; + @Mock + ClusterDao clusterDao; + @Mock + VolumeOrchestrationService volumeOrchestrationService; + private DetachVolumeCmd detachCmd = new DetachVolumeCmd(); private Class _detachCmdClass = detachCmd.getClass(); @@ -1820,4 +1836,237 @@ public class VolumeApiServiceImplTest { volumeApiServiceImpl.validationsForCheckVolumeOperation(volume); } + + private UserVmVO getMockedVm() { + UserVmVO vm = Mockito.mock(UserVmVO.class); + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(vm.getTemplateId()).thenReturn(10L); + Mockito.when(vm.getHostName()).thenReturn("test-vm"); + return vm; + } + + private VMTemplateVO getMockedTemplate() { + VMTemplateVO template = Mockito.mock(VMTemplateVO.class); + Mockito.when(template.isDeployAsIs()).thenReturn(false); + return template; + } + + @Test(expected = CloudRuntimeException.class) + public void testGetVmExistingVolumeForVolumeAttach_MultipleRootVolumes_ThrowsException() { + UserVmVO vm = getMockedVm(); + VMTemplateVO template = getMockedTemplate(); + when(templateDao.findById(10L)).thenReturn(template); + when(volumeDaoMock.findByInstanceAndType(1L, Volume.Type.ROOT)) + .thenReturn(Arrays.asList(Mockito.mock(VolumeVO.class), Mockito.mock(VolumeVO.class))); + volumeApiServiceImpl.getVmExistingVolumeForVolumeAttach(vm, Mockito.mock(VolumeInfo.class)); + } + + @Test + public void testGetVmExistingVolumeForVolumeAttach_SingleRootVolume() { + UserVmVO vm = getMockedVm(); + VMTemplateVO template = getMockedTemplate(); + VolumeVO rootVolume = Mockito.mock(VolumeVO.class); + Mockito.when(rootVolume.getId()).thenReturn(20L); + Mockito.when(templateDao.findById(10L)).thenReturn(template); + Mockito.when(volumeDaoMock.findByInstanceAndType(1L, Volume.Type.ROOT)) + .thenReturn(Collections.singletonList(rootVolume)); + VolumeVO result = volumeApiServiceImpl.getVmExistingVolumeForVolumeAttach(vm, Mockito.mock(VolumeInfo.class)); + Assert.assertNotNull(result); + Assert.assertEquals(20L, result.getId()); + } + + private VolumeVO getMockedDataVolume() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getId()).thenReturn(30L); + Mockito.when(volume.getState()).thenReturn(Volume.State.Ready); + return volume; + } + + @Test + public void testGetVmExistingVolumeForVolumeAttach_NoRootVolume_DataDiskAvailable() { + UserVmVO vm = getMockedVm(); + VMTemplateVO template = getMockedTemplate(); + VolumeVO dataDisk = getMockedDataVolume(); + List rootVolumes = Collections.emptyList(); + List dataVolumes = Collections.singletonList(dataDisk); + Mockito.when(templateDao.findById(10L)).thenReturn(template); + Mockito.when(volumeDaoMock.findByInstanceAndType(1L, Volume.Type.ROOT)).thenReturn(rootVolumes); + Mockito.when(volumeDaoMock.findByInstanceAndType(1L, Volume.Type.DATADISK)).thenReturn(dataVolumes); + VolumeVO result = volumeApiServiceImpl.getVmExistingVolumeForVolumeAttach(vm, Mockito.mock(VolumeInfo.class)); + Assert.assertNotNull(result); + Assert.assertEquals(30L, result.getId()); + } + + @Test + public void testGetVmExistingVolumeForVolumeAttach_NoVolumesAtAll() { + UserVmVO vm = getMockedVm(); + VMTemplateVO template = getMockedTemplate(); + Mockito.when(templateDao.findById(10L)).thenReturn(template); + Mockito.when(volumeDaoMock.findByInstanceAndType(1L, Volume.Type.ROOT)).thenReturn(Collections.emptyList()); + Mockito.when(volumeDaoMock.findByInstanceAndType(1L, Volume.Type.DATADISK)).thenReturn(Collections.emptyList()); + VolumeVO result = volumeApiServiceImpl.getVmExistingVolumeForVolumeAttach(vm, Mockito.mock(VolumeInfo.class)); + Assert.assertNull(result); + } + + private void mockDiskOffering() { + DiskOfferingVO offering = Mockito.mock(DiskOfferingVO.class); + Mockito.when(_diskOfferingDao.findById(1L)).thenReturn(offering); + Mockito.when(offering.isUseLocalStorage()).thenReturn(true); + Mockito.when(offering.isRecreatable()).thenReturn(false); + } + + private DataCenterVO mockZone() { + DataCenterVO zone = Mockito.mock(DataCenterVO.class); + Mockito.when(_dcDao.findById(1L)).thenReturn(zone); + return zone; + } + + @Test + public void testGetPoolForAllocatedOrUploadedVolumeForAttach_Success() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + ClusterVO cluster = Mockito.mock(ClusterVO.class); + HostPodVO pod = Mockito.mock(HostPodVO.class); + DataCenterVO zone = mockZone(); + mockDiskOffering(); + StoragePool pool = Mockito.mock(StoragePool.class); + when(vm.getDataCenterId()).thenReturn(1L); + when(virtualMachineManager.findClusterAndHostIdForVm(vm, false)).thenReturn(new Pair<>(1L, 2L)); + when(clusterDao.findById(1L)).thenReturn(cluster); + when(cluster.getPodId()).thenReturn(1L); + when(podDao.findById(1L)).thenReturn(pod); + when(volumeToAttach.getDiskOfferingId()).thenReturn(1L); + when(volumeOrchestrationService.findStoragePool(any(DiskProfile.class), eq(zone), eq(pod), eq(1L), eq(2L), eq(vm), eq(Collections.emptySet()))) + .thenReturn(pool); + StoragePool result = volumeApiServiceImpl.getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + Assert.assertNotNull(result); + Assert.assertEquals(pool, result); + } + + @Test(expected = CloudRuntimeException.class) + public void testGetPoolForAllocatedOrUploadedVolumeForAttach_NoPoolFound_ThrowsException() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + DataCenterVO zone = mockZone(); + Pair clusterHostId = new Pair<>(1L, 2L); + ClusterVO cluster = Mockito.mock(ClusterVO.class); + HostPodVO pod = Mockito.mock(HostPodVO.class); + mockDiskOffering(); + when(vm.getDataCenterId()).thenReturn(1L); + when(clusterDao.findById(1L)).thenReturn(cluster); + when(virtualMachineManager.findClusterAndHostIdForVm(vm, false)).thenReturn(clusterHostId); + when(podDao.findById(anyLong())).thenReturn(pod); + when(volumeToAttach.getDiskOfferingId()).thenReturn(1L); + when(volumeOrchestrationService.findStoragePool(any(DiskProfile.class), eq(zone), eq(pod), eq(1L), eq(2L), eq(vm), eq(Collections.emptySet()))) + .thenReturn(null); + volumeApiServiceImpl.getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + } + + @Test + public void testGetPoolForAllocatedOrUploadedVolumeForAttach_NoCluster() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + DataCenterVO zone = mockZone(); + HostPodVO pod = Mockito.mock(HostPodVO.class); + mockDiskOffering(); + StoragePool pool = Mockito.mock(StoragePool.class); + when(vm.getDataCenterId()).thenReturn(1L); + when(vm.getPodIdToDeployIn()).thenReturn(2L); + when(virtualMachineManager.findClusterAndHostIdForVm(vm, false)).thenReturn(new Pair<>(null, 2L)); + when(podDao.findById(2L)).thenReturn(pod); + when(volumeToAttach.getDiskOfferingId()).thenReturn(1L); + when(volumeOrchestrationService.findStoragePool(any(DiskProfile.class), eq(zone), eq(pod), eq(null), eq(2L), eq(vm), eq(Collections.emptySet()))) + .thenReturn(pool); + StoragePool result = volumeApiServiceImpl.getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + Assert.assertNotNull(result); + Assert.assertEquals(pool, result); + } + + + @Test + public void testCreateVolumeOnSecondaryForAttachIfNeeded_VolumeNotAllocatedOrUploaded() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + Mockito.when(volumeToAttach.getState()).thenReturn(Volume.State.Ready); + VolumeInfo result = volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded( + volumeToAttach, Mockito.mock(UserVmVO.class), null); + Assert.assertSame(volumeToAttach, result); + Mockito.verifyNoInteractions(primaryDataStoreDaoMock, volumeOrchestrationService); + } + + @Test + public void testCreateVolumeOnSecondaryForAttachIfNeeded_ExistingVolumeDeterminesStoragePool() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + Mockito.when(volumeToAttach.getState()).thenReturn(Volume.State.Uploaded); + UserVmVO vm = Mockito.mock(UserVmVO.class); + VolumeVO existingVolume = Mockito.mock(VolumeVO.class); + Mockito.when(existingVolume.getState()).thenReturn(Volume.State.Ready); + when(existingVolume.getPoolId()).thenReturn(1L); + StoragePoolVO destPrimaryStorage = Mockito.mock(StoragePoolVO.class); + Mockito.when(destPrimaryStorage.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + Mockito.when(primaryDataStoreDaoMock.findById(1L)).thenReturn(destPrimaryStorage); + VolumeInfo newVolumeOnPrimaryStorage = Mockito.mock(VolumeInfo.class); + try { + Mockito.when(volumeOrchestrationService.createVolumeOnPrimaryStorage(vm, volumeToAttach, vm.getHypervisorType(), destPrimaryStorage)) + .thenReturn(newVolumeOnPrimaryStorage); + } catch (NoTransitionException nte) { + Assert.fail(nte.getMessage()); + } + VolumeInfo result = volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, existingVolume); + Assert.assertSame(newVolumeOnPrimaryStorage, result); + Mockito.verify(primaryDataStoreDaoMock).findById(1L); + } + + @Test + public void testCreateVolumeOnPrimaryForAttachIfNeeded_UsesGetPoolForAttach() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + Mockito.when(volumeToAttach.getState()).thenReturn(Volume.State.Allocated); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePool destPrimaryStorage = Mockito.mock(StoragePool.class); + Mockito.doReturn(destPrimaryStorage).when(volumeApiServiceImpl) + .getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + VolumeInfo newVolumeOnPrimaryStorage = Mockito.mock(VolumeInfo.class); + try { + Mockito.when(volumeOrchestrationService.createVolumeOnPrimaryStorage( + vm, volumeToAttach, vm.getHypervisorType(), destPrimaryStorage)) + .thenReturn(newVolumeOnPrimaryStorage); + } catch (NoTransitionException nte) { + Assert.fail(nte.getMessage()); + } + VolumeInfo result = volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, null); + Assert.assertSame(newVolumeOnPrimaryStorage, result); + verify(volumeApiServiceImpl).getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + } + + @Test(expected = InvalidParameterValueException.class) + public void testCreateVolumeOnPrimaryForAttachIfNeeded_UnsupportedPoolType_ThrowsException() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + when(volumeToAttach.getState()).thenReturn(Volume.State.Uploaded); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePool destPrimaryStorage = Mockito.mock(StoragePool.class); + when(destPrimaryStorage.getPoolType()).thenReturn(Storage.StoragePoolType.PowerFlex); + Mockito.doReturn(destPrimaryStorage).when(volumeApiServiceImpl) + .getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, null); + } + + @Test + public void testCreateVolumeOnSecondaryForAttachIfNeeded_CreateVolumeFails_ThrowsException() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + Mockito.when(volumeToAttach.getState()).thenReturn(Volume.State.Uploaded); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePool destPrimaryStorage = Mockito.mock(StoragePool.class); + Mockito.when(destPrimaryStorage.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + Mockito.doReturn(destPrimaryStorage).when(volumeApiServiceImpl) + .getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + try { + Mockito.when(volumeOrchestrationService.createVolumeOnPrimaryStorage(vm, volumeToAttach, vm.getHypervisorType(), destPrimaryStorage)) + .thenThrow(new NoTransitionException("Mocked exception")); + } catch (NoTransitionException nte) { + Assert.fail(nte.getMessage()); + } + CloudRuntimeException exception = Assert.assertThrows(CloudRuntimeException.class, () -> + volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, null) + ); + Assert.assertTrue(exception.getMessage().contains("Failed to create volume on primary storage")); + } } From 6f03f9e726ef099f63e8c92950282950e83e1c80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Thu, 30 Jan 2025 08:00:32 -0300 Subject: [PATCH 02/14] validate inserted values in numeric global settings (#10279) --- ui/src/views/setting/ConfigurationValue.vue | 23 +++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/ui/src/views/setting/ConfigurationValue.vue b/ui/src/views/setting/ConfigurationValue.vue index 109931a6664..acee0f2eb6a 100644 --- a/ui/src/views/setting/ConfigurationValue.vue +++ b/ui/src/views/setting/ConfigurationValue.vue @@ -39,6 +39,7 @@ @keydown.esc="editableValueKey = null" @pressEnter="updateConfigurationValue(configrecord)" @change="value => setConfigurationEditable(configrecord, value)" + @keydown="e => handleInputNumberKeyDown(e, false)" /> @@ -52,6 +53,7 @@ @keydown.esc="editableValueKey = null" @pressEnter="updateConfigurationValue(configrecord)" @change="value => setConfigurationEditable(configrecord, value)" + @keydown="e => handleInputNumberKeyDown(e, true)" /> @@ -87,6 +89,7 @@ @keydown.esc="editableValueKey = null" @pressEnter="updateConfigurationValue(configrecord)" @change="value => setConfigurationEditable(configrecord, value)" + @keydown="e => handleInputNumberKeyDown(e, true)" /> @@ -350,6 +353,26 @@ export default { } else { this.editableValueKey = null } + }, + handleInputNumberKeyDown (event, isDecimal) { + const allowedCodes = ['Backspace', 'Delete', 'ArrowLeft', 'ArrowRight', 'Minus'] + + if (isDecimal) { + allowedCodes.push('Period') + } + + if ( + event.getModifierState('Control') || + event.getModifierState('Meta') || + event.getModifierState('Alt') + ) { + return + } + + const isValid = allowedCodes.includes(event.code) || !isNaN(event.key) + if (!isValid) { + event.preventDefault() + } } } } From 3313cc58295690968ce9301c583119e8b2a70959 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 30 Jan 2025 20:00:49 +0530 Subject: [PATCH 03/14] ui: fix loading for hypervisor filter in serachview (#10292) Signed-off-by: Abhishek Kumar --- ui/src/components/view/SearchView.vue | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/src/components/view/SearchView.vue b/ui/src/components/view/SearchView.vue index 9cbe1ef0fe6..08f4cd03877 100644 --- a/ui/src/components/view/SearchView.vue +++ b/ui/src/components/view/SearchView.vue @@ -581,6 +581,9 @@ export default { if (accountIndex > -1) { this.fields[accountIndex].loading = false } + if (hypervisorIndex > -1) { + this.fields[hypervisorIndex].loading = false + } if (imageStoreIndex > -1) { this.fields[imageStoreIndex].loading = false } From ee0dc5b2d64516361dbed922fc2f75dec47fd4a0 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Thu, 30 Jan 2025 20:03:25 +0530 Subject: [PATCH 04/14] list hosts API fix, when any stale entries exists on storage_pool_host_ref for the removed pools (#9852) --- .../com/cloud/storage/StorageManagerImpl.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 0a45fd448ad..3a6d804a762 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -503,8 +503,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C public boolean isLocalStorageActiveOnHost(Long hostId) { List storagePoolHostRefs = _storagePoolHostDao.listByHostId(hostId); for (StoragePoolHostVO storagePoolHostRef : storagePoolHostRefs) { - StoragePoolVO PrimaryDataStoreVO = _storagePoolDao.findById(storagePoolHostRef.getPoolId()); - if (PrimaryDataStoreVO.getPoolType() == StoragePoolType.LVM || PrimaryDataStoreVO.getPoolType() == StoragePoolType.EXT) { + StoragePoolVO primaryDataStoreVO = _storagePoolDao.findById(storagePoolHostRef.getPoolId()); + if (primaryDataStoreVO != null && (primaryDataStoreVO.getPoolType() == StoragePoolType.LVM || primaryDataStoreVO.getPoolType() == StoragePoolType.EXT)) { SearchBuilder volumeSB = volumeDao.createSearchBuilder(); volumeSB.and("poolId", volumeSB.entity().getPoolId(), SearchCriteria.Op.EQ); volumeSB.and("removed", volumeSB.entity().getRemoved(), SearchCriteria.Op.NULL); @@ -515,7 +515,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C volumeSB.join("activeVmSB", activeVmSB, volumeSB.entity().getInstanceId(), activeVmSB.entity().getId(), JoinBuilder.JoinType.INNER); SearchCriteria volumeSC = volumeSB.create(); - volumeSC.setParameters("poolId", PrimaryDataStoreVO.getId()); + volumeSC.setParameters("poolId", primaryDataStoreVO.getId()); volumeSC.setParameters("state", Volume.State.Expunging, Volume.State.Destroy); volumeSC.setJoinParameters("activeVmSB", "state", State.Starting, State.Running, State.Stopping, State.Migrating); @@ -2142,9 +2142,9 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C // poolId is null only if volume is destroyed, which has been checked // before. assert poolId != null; - StoragePoolVO PrimaryDataStoreVO = _storagePoolDao.findById(poolId); - assert PrimaryDataStoreVO != null; - return PrimaryDataStoreVO.getUuid(); + StoragePoolVO primaryDataStoreVO = _storagePoolDao.findById(poolId); + assert primaryDataStoreVO != null; + return primaryDataStoreVO.getUuid(); } @Override @@ -2697,8 +2697,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } CapacityVO capacity = new CapacityVO(poolId, zoneId, podId, clusterId, 0, 0, Capacity.CAPACITY_TYPE_STORAGE); - for (StoragePoolVO PrimaryDataStoreVO : pools) { - StorageStats stats = ApiDBUtils.getStoragePoolStatistics(PrimaryDataStoreVO.getId()); + for (StoragePoolVO primaryDataStoreVO : pools) { + StorageStats stats = ApiDBUtils.getStoragePoolStatistics(primaryDataStoreVO.getId()); if (stats == null) { continue; } From 0f544c9a3b01adec4d723907e27577883c361fbf Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 30 Jan 2025 15:35:33 +0100 Subject: [PATCH 05/14] api/ui: add specifyvlan to network response (#10236) --- .../apache/cloudstack/api/response/NetworkResponse.java | 8 ++++++++ .../src/main/java/com/cloud/api/ApiResponseHelper.java | 1 + ui/src/config/section/network.js | 9 ++++++--- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java index 1049740bf55..24f76215d09 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java @@ -187,6 +187,10 @@ public class NetworkResponse extends BaseResponseWithAssociatedNetwork implement @Param(description = "true network requires restart") private Boolean restartRequired; + @SerializedName(ApiConstants.SPECIFY_VLAN) + @Param(description = "true if network supports specifying vlan, false otherwise") + private Boolean specifyVlan; + @SerializedName(ApiConstants.SPECIFY_IP_RANGES) @Param(description = "true if network supports specifying ip ranges, false otherwise") private Boolean specifyIpRanges; @@ -487,6 +491,10 @@ public class NetworkResponse extends BaseResponseWithAssociatedNetwork implement this.restartRequired = restartRequired; } + public void setSpecifyVlan(Boolean specifyVlan) { + this.specifyVlan = specifyVlan; + } + public void setSpecifyIpRanges(Boolean specifyIpRanges) { this.specifyIpRanges = specifyIpRanges; } diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 68761f6eed5..ed7a51d215c 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -2504,6 +2504,7 @@ public class ApiResponseHelper implements ResponseGenerator { response.setIsSystem(networkOffering.isSystemOnly()); response.setNetworkOfferingAvailability(networkOffering.getAvailability().toString()); response.setIsPersistent(networkOffering.isPersistent()); + response.setSpecifyVlan(networkOffering.isSpecifyVlan()); if (Network.GuestType.Isolated.equals(network.getGuestType()) && network.getVpcId() == null) { response.setEgressDefaultPolicy(networkOffering.isEgressDefaultPolicy()); } diff --git a/ui/src/config/section/network.js b/ui/src/config/section/network.js index 986a2c206c7..edbe4bb37b7 100644 --- a/ui/src/config/section/network.js +++ b/ui/src/config/section/network.js @@ -141,7 +141,8 @@ export default { label: 'label.update.network', dataView: true, disabled: (record, user) => { - return !record.projectid && (record.account !== user.userInfo.account && !['Admin', 'DomainAdmin'].includes(user.userInfo.roletype)) + return (!record.projectid && (record.account !== user.userInfo.account && !['Admin', 'DomainAdmin'].includes(user.userInfo.roletype))) || + (record.type === 'Shared' && record.specifyvlan && !['Admin'].includes(user.userInfo.roletype)) }, popup: true, component: shallowRef(defineAsyncComponent(() => import('@/views/network/UpdateNetwork.vue'))) @@ -153,7 +154,8 @@ export default { message: 'message.restart.network', dataView: true, disabled: (record, user) => { - return !record.projectid && (record.account !== user.userInfo.account && !['Admin', 'DomainAdmin'].includes(user.userInfo.roletype)) + return (!record.projectid && (record.account !== user.userInfo.account && !['Admin', 'DomainAdmin'].includes(user.userInfo.roletype))) || + (record.type === 'Shared' && record.specifyvlan && !['Admin'].includes(user.userInfo.roletype)) }, args: (record, store, isGroupAction) => { var fields = [] @@ -194,7 +196,8 @@ export default { message: 'message.action.delete.network', dataView: true, disabled: (record, user) => { - return !record.projectid && (record.account !== user.userInfo.account && !['Admin', 'DomainAdmin'].includes(user.userInfo.roletype)) + return (!record.projectid && (record.account !== user.userInfo.account && !['Admin', 'DomainAdmin'].includes(user.userInfo.roletype))) || + (record.type === 'Shared' && record.specifyvlan && !['Admin'].includes(user.userInfo.roletype)) }, groupAction: true, popup: true, From b9890875cc153d7a2af40c71b975ac32c4559c6c Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 30 Jan 2025 15:49:26 +0100 Subject: [PATCH 06/14] CKS: use --delete-emptydir-data instead of deprecated --delete-local-data (#10234) --- .../cluster/actionworkers/KubernetesClusterScaleWorker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java index 5b496509eeb..d666293d02a 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java @@ -203,7 +203,7 @@ public class KubernetesClusterScaleWorker extends KubernetesClusterResourceModif retryCounter++; try { Pair result = SshHelper.sshExecute(ipAddress, port, getControlNodeLoginUser(), - pkFile, null, String.format("sudo /opt/bin/kubectl drain %s --ignore-daemonsets --delete-local-data", hostName), + pkFile, null, String.format("sudo /opt/bin/kubectl drain %s --ignore-daemonsets --delete-emptydir-data", hostName), 10000, 10000, 60000); if (!result.first()) { LOGGER.warn(String.format("Draining node: %s on VM : %s in Kubernetes cluster : %s unsuccessful", hostName, userVm.getDisplayName(), kubernetesCluster.getName())); From b93589b5bdf52043127a9c898d6aa596b0245c4c Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 30 Jan 2025 20:21:04 +0530 Subject: [PATCH 07/14] server: reset 2fa user configuration on incomplete setup (#10247) Signed-off-by: Abhishek Kumar --- .../java/com/cloud/user/UserAccountVO.java | 7 +++ .../management/MockAccountManager.java | 5 ++ .../main/java/com/cloud/api/ApiServer.java | 3 +- .../java/com/cloud/user/AccountManager.java | 3 ++ .../com/cloud/user/AccountManagerImpl.java | 22 ++++++++ .../cloud/user/AccountManagerImplTest.java | 52 +++++++++++++++++++ .../cloud/user/MockAccountManagerImpl.java | 4 ++ 7 files changed, 95 insertions(+), 1 deletion(-) diff --git a/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java b/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java index c18ca53f7ab..a9d4ca9a2b9 100644 --- a/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java +++ b/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java @@ -35,6 +35,8 @@ import org.apache.cloudstack.api.InternalIdentity; import com.cloud.utils.db.Encrypt; import com.cloud.utils.db.GenericDao; + +import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; import org.apache.commons.lang3.StringUtils; @Entity @@ -368,4 +370,9 @@ public class UserAccountVO implements UserAccount, InternalIdentity { public void setDetails(Map details) { this.details = details; } + + @Override + public String toString() { + return String.format("User %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "id", "name", "uuid")); + } } diff --git a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java index a3fac2c8be9..a63c5b68e57 100644 --- a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java +++ b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java @@ -516,4 +516,9 @@ public class MockAccountManager extends ManagerBase implements AccountManager { public ConfigKey[] getConfigKeys() { return null; } + + @Override + public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) { + return null; + } } diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index d68f42470d5..c78f8e68c2b 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -1159,7 +1159,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer domainId = userDomain.getId(); } - final UserAccount userAcct = accountMgr.authenticateUser(username, password, domainId, loginIpAddress, requestParameters); + UserAccount userAcct = accountMgr.authenticateUser(username, password, domainId, loginIpAddress, requestParameters); if (userAcct != null) { final String timezone = userAcct.getTimezone(); float offsetInHrs = 0f; @@ -1204,6 +1204,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer session.setAttribute("timezoneoffset", Float.valueOf(offsetInHrs).toString()); } + userAcct = accountMgr.clearUserTwoFactorAuthenticationInSetupStateOnLogin(userAcct); boolean is2faEnabled = false; if (userAcct.isUser2faEnabled() || (Boolean.TRUE.equals(AccountManagerImpl.enableUserTwoFactorAuthentication.valueIn(userAcct.getDomainId())) && Boolean.TRUE.equals(AccountManagerImpl.mandateUserTwoFactorAuthentication.valueIn(userAcct.getDomainId())))) { is2faEnabled = true; diff --git a/server/src/main/java/com/cloud/user/AccountManager.java b/server/src/main/java/com/cloud/user/AccountManager.java index c95047a6c42..1d7611d5b54 100644 --- a/server/src/main/java/com/cloud/user/AccountManager.java +++ b/server/src/main/java/com/cloud/user/AccountManager.java @@ -195,6 +195,9 @@ public interface AccountManager extends AccountService, Configurable { UserTwoFactorAuthenticator getUserTwoFactorAuthenticator(final Long domainId, final Long userAccountId); void verifyUsingTwoFactorAuthenticationCode(String code, Long domainId, Long userAccountId); + UserTwoFactorAuthenticationSetupResponse setupUserTwoFactorAuthentication(SetupUserTwoFactorAuthenticationCmd cmd); + UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user); + } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 1e727036d56..7d22a114331 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -3475,4 +3475,26 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M return userTwoFactorAuthenticationProvidersMap.get(name.toLowerCase()); } + @Override + public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) { + return Transaction.execute((TransactionCallback) status -> { + if (!user.isUser2faEnabled() && StringUtils.isBlank(user.getUser2faProvider())) { + return user; + } + UserDetailVO userDetailVO = _userDetailsDao.findDetail(user.getId(), UserDetailVO.Setup2FADetail); + if (userDetailVO != null && UserAccountVO.Setup2FAstatus.VERIFIED.name().equals(userDetailVO.getValue())) { + return user; + } + s_logger.info(String.format("Clearing 2FA configurations for %s as it is still in setup on a new login request", user)); + if (userDetailVO != null) { + _userDetailsDao.remove(userDetailVO.getId()); + } + UserAccountVO userAccountVO = _userAccountDao.findById(user.getId()); + userAccountVO.setUser2faEnabled(false); + userAccountVO.setUser2faProvider(null); + userAccountVO.setKeyFor2fa(null); + _userAccountDao.update(user.getId(), userAccountVO); + return userAccountVO; + }); + } } diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java index 0e8e1df0f3a..e68de194f01 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -39,10 +39,12 @@ import org.apache.cloudstack.auth.UserAuthenticator.ActionOnFailedAuthentication import org.apache.cloudstack.auth.UserTwoFactorAuthenticator; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.resourcedetail.UserDetailVO; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.Mockito; @@ -1218,4 +1220,54 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.when(_projectAccountDao.listAdministratedProjectIds(accountId)).thenReturn(managedProjectIds); accountManagerImpl.checkIfAccountManagesProjects(accountId); } + + @Test + public void testClearUser2FA_When2FADisabled_NoChanges() { + UserAccount user = Mockito.mock(UserAccount.class); + Mockito.when(user.isUser2faEnabled()).thenReturn(false); + Mockito.when(user.getUser2faProvider()).thenReturn(null); + UserAccount result = accountManagerImpl.clearUserTwoFactorAuthenticationInSetupStateOnLogin(user); + Assert.assertSame(user, result); + Mockito.verifyNoInteractions(userDetailsDaoMock, userAccountDaoMock); + } + + @Test + public void testClearUser2FA_When2FAInVerifiedState_NoChanges() { + UserAccount user = Mockito.mock(UserAccount.class); + Mockito.when(user.getId()).thenReturn(1L); + Mockito.when(user.isUser2faEnabled()).thenReturn(true); + UserDetailVO userDetail = new UserDetailVO(); + userDetail.setValue(UserAccountVO.Setup2FAstatus.VERIFIED.name()); + Mockito.when(userDetailsDaoMock.findDetail(1L, UserDetailVO.Setup2FADetail)).thenReturn(userDetail); + UserAccount result = accountManagerImpl.clearUserTwoFactorAuthenticationInSetupStateOnLogin(user); + Assert.assertSame(user, result); + Mockito.verify(userDetailsDaoMock).findDetail(1L, UserDetailVO.Setup2FADetail); + Mockito.verifyNoMoreInteractions(userDetailsDaoMock, userAccountDaoMock); + } + + @Test + public void testClearUser2FA_When2FAInSetupState_Disable2FA() { + UserAccount user = Mockito.mock(UserAccount.class); + Mockito.when(user.getId()).thenReturn(1L); + Mockito.when(user.isUser2faEnabled()).thenReturn(true); + UserDetailVO userDetail = new UserDetailVO(); + userDetail.setValue(UserAccountVO.Setup2FAstatus.ENABLED.name()); + UserAccountVO userAccountVO = new UserAccountVO(); + userAccountVO.setId(1L); + Mockito.when(userDetailsDaoMock.findDetail(1L, UserDetailVO.Setup2FADetail)).thenReturn(userDetail); + Mockito.when(userAccountDaoMock.findById(1L)).thenReturn(userAccountVO); + UserAccount result = accountManagerImpl.clearUserTwoFactorAuthenticationInSetupStateOnLogin(user); + Assert.assertNotNull(result); + Assert.assertFalse(result.isUser2faEnabled()); + Assert.assertNull(result.getUser2faProvider()); + Mockito.verify(userDetailsDaoMock).findDetail(1L, UserDetailVO.Setup2FADetail); + Mockito.verify(userDetailsDaoMock).remove(Mockito.anyLong()); + Mockito.verify(userAccountDaoMock).findById(1L); + ArgumentCaptor captor = ArgumentCaptor.forClass(UserAccountVO.class); + Mockito.verify(userAccountDaoMock).update(Mockito.eq(1L), captor.capture()); + UserAccountVO updatedUser = captor.getValue(); + Assert.assertFalse(updatedUser.isUser2faEnabled()); + Assert.assertNull(updatedUser.getUser2faProvider()); + Assert.assertNull(updatedUser.getKeyFor2fa()); + } } diff --git a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java index f8558992440..96b73cc63dd 100644 --- a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java +++ b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java @@ -484,4 +484,8 @@ public class MockAccountManagerImpl extends ManagerBase implements Manager, Acco return null; } + @Override + public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) { + return null; + } } From d9af9bdb36ed7ea0efcc9d91d30eec1253c54b6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Thu, 30 Jan 2025 11:51:55 -0300 Subject: [PATCH 08/14] fix SQL syntax erros and target 419 (#10273) --- ...cloud.idempotent_update_api_permission.sql | 52 +++++++++++++++++++ .../META-INF/db/schema-41910to41920.sql | 22 ++++++++ 2 files changed, 74 insertions(+) create mode 100644 engine/schema/src/main/resources/META-INF/db/procedures/cloud.idempotent_update_api_permission.sql diff --git a/engine/schema/src/main/resources/META-INF/db/procedures/cloud.idempotent_update_api_permission.sql b/engine/schema/src/main/resources/META-INF/db/procedures/cloud.idempotent_update_api_permission.sql new file mode 100644 index 00000000000..c53e0067061 --- /dev/null +++ b/engine/schema/src/main/resources/META-INF/db/procedures/cloud.idempotent_update_api_permission.sql @@ -0,0 +1,52 @@ +-- 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. + +DROP PROCEDURE IF EXISTS `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`; + +CREATE PROCEDURE `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION` ( + IN role VARCHAR(255), + IN rule VARCHAR(255), + IN permission VARCHAR(255) +) +BEGIN + DECLARE role_id BIGINT(20) UNSIGNED +; DECLARE max_sort_order BIGINT(20) UNSIGNED + +; SELECT `r`.`id` INTO role_id + FROM `cloud`.`roles` `r` + WHERE `r`.`name` = role + AND `r`.`is_default` = 1 + +; SELECT MAX(`rp`.`sort_order`) INTO max_sort_order + FROM `cloud`.`role_permissions` `rp` + WHERE `rp`.`role_id` = role_id + +; IF NOT EXISTS ( + SELECT * FROM `cloud`.`role_permissions` `rp` + WHERE `rp`.`role_id` = role_id + AND `rp`.`rule` = rule + ) THEN + UPDATE `cloud`.`role_permissions` `rp` + SET `rp`.`sort_order` = max_sort_order + 1 + WHERE `rp`.`sort_order` = max_sort_order + AND `rp`.`role_id` = role_id + +; INSERT INTO `cloud`.`role_permissions` + (uuid, role_id, rule, permission, sort_order) + VALUES (uuid(), role_id, rule, permission, max_sort_order) +; END IF +;END; diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41910to41920.sql b/engine/schema/src/main/resources/META-INF/db/schema-41910to41920.sql index 2ce8ea99bd1..12ead739d84 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41910to41920.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41910to41920.sql @@ -21,3 +21,25 @@ -- Add last_id to the volumes table CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.volumes', 'last_id', 'bigint(20) unsigned DEFAULT NULL'); + +-- Grant access to 2FA APIs for the "Read-Only User - Default" role + +CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only User - Default', 'setupUserTwoFactorAuthentication', 'ALLOW'); +CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only User - Default', 'validateUserTwoFactorAuthenticationCode', 'ALLOW'); +CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only User - Default', 'listUserTwoFactorAuthenticatorProviders', 'ALLOW'); + +-- Grant access to 2FA APIs for the "Support User - Default" role + +CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support User - Default', 'setupUserTwoFactorAuthentication', 'ALLOW'); +CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support User - Default', 'validateUserTwoFactorAuthenticationCode', 'ALLOW'); +CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support User - Default', 'listUserTwoFactorAuthenticatorProviders', 'ALLOW'); + +-- Grant access to 2FA APIs for the "Read-Only Admin - Default" role + +CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only Admin - Default', 'setupUserTwoFactorAuthentication', 'ALLOW'); +CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only Admin - Default', 'validateUserTwoFactorAuthenticationCode', 'ALLOW'); + +-- Grant access to 2FA APIs for the "Support Admin - Default" role + +CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support Admin - Default', 'setupUserTwoFactorAuthentication', 'ALLOW'); +CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support Admin - Default', 'validateUserTwoFactorAuthenticationCode', 'ALLOW'); From c70e4e29be41a4ca4f9562e3a11d34eeee905647 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Thu, 30 Jan 2025 11:52:36 -0300 Subject: [PATCH 09/14] fix npe on account creation (#10274) --- .../java/com/cloud/user/AccountManagerImpl.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 7d22a114331..2358830e9a3 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -1341,7 +1341,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M */ private void checkRoleEscalation(Account caller, Account requested) { if (s_logger.isDebugEnabled()) { - s_logger.debug(String.format("checking if user of account %s [%s] with role-id [%d] can create an account of type %s [%s] with role-id [%d]", + s_logger.debug(String.format("Checking if user of account %s [%s] with role-id [%d] can create an account of type %s [%s] with role-id [%d]", caller.getAccountName(), caller.getUuid(), caller.getRoleId(), @@ -1355,12 +1355,13 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M checkApiAccess(apiCheckers, requested, command); } catch (PermissionDeniedException pde) { if (s_logger.isTraceEnabled()) { - s_logger.trace(String.format("checking for permission to \"%s\" is irrelevant as it is not requested for %s [%s]", + s_logger.trace(String.format( + "Checking for permission to \"%s\" is irrelevant as it is not requested for %s [%s]", command, - pde.getAccount().getAccountName(), - pde.getAccount().getUuid(), - pde.getEntitiesInViolation() - )); + requested.getAccountName(), + requested.getUuid() + ) + ); } continue; } From 0fbf6379f987e200e060347ea7a2dda1b4027731 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 30 Jan 2025 20:25:17 +0530 Subject: [PATCH 10/14] UI: Fix domain view when opening details for a specific domainid (#10245) --- ui/src/views/iam/DomainView.vue | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ui/src/views/iam/DomainView.vue b/ui/src/views/iam/DomainView.vue index 997f900bbf7..ed875151a13 100644 --- a/ui/src/views/iam/DomainView.vue +++ b/ui/src/views/iam/DomainView.vue @@ -56,6 +56,7 @@ :loading="loading" :tabs="$route.meta.tabs" /> Date: Thu, 30 Jan 2025 16:08:39 +0100 Subject: [PATCH 11/14] packaging: support both mysql and mariadb on EL8/EL9 (#9941) --- packaging/centos8/cloud.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/centos8/cloud.spec b/packaging/centos8/cloud.spec index f890d303a72..d2aa9baa5c8 100644 --- a/packaging/centos8/cloud.spec +++ b/packaging/centos8/cloud.spec @@ -68,7 +68,7 @@ Requires: (openssh-clients or openssh) Requires: (nfs-utils or nfs-client) Requires: iproute Requires: wget -Requires: mysql +Requires: (mysql or mariadb) Requires: sudo Requires: /sbin/service Requires: /sbin/chkconfig From 641a60670e1029621d341db342b6ff4302b6cb82 Mon Sep 17 00:00:00 2001 From: kiranchavala Date: Thu, 30 Jan 2025 22:01:52 +0530 Subject: [PATCH 12/14] changed the kubernetestool url (#10295) --- scripts/util/create-kubernetes-binaries-iso.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/util/create-kubernetes-binaries-iso.sh b/scripts/util/create-kubernetes-binaries-iso.sh index d5fb014f220..e4364054d2a 100755 --- a/scripts/util/create-kubernetes-binaries-iso.sh +++ b/scripts/util/create-kubernetes-binaries-iso.sh @@ -53,7 +53,7 @@ echo "Downloading Kubernetes tools ${RELEASE}..." k8s_dir="${working_dir}/k8s" mkdir -p "${k8s_dir}" cd "${k8s_dir}" -curl -L --remote-name-all https://storage.googleapis.com/kubernetes-release/release/${RELEASE}/bin/linux/amd64/{kubeadm,kubelet,kubectl} +curl -L --remote-name-all https://dl.k8s.io/release/${RELEASE}/bin/linux/amd64/{kubeadm,kubelet,kubectl} kubeadm_file_permissions=`stat --format '%a' kubeadm` chmod +x kubeadm From a335feab6a366e196b9f92c0a86fbbcefaa8eae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Thu, 30 Jan 2025 14:44:48 -0300 Subject: [PATCH 13/14] fix allocation of vmfs storage pools (#10201) --- .../allocator/AbstractStoragePoolAllocator.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index 2a65fad8e8b..9e0a28875d1 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -126,18 +126,24 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement protected List reorderPoolsByCapacity(DeploymentPlan plan, List pools) { Long zoneId = plan.getDataCenterId(); Long clusterId = plan.getClusterId(); - short capacityType; if (CollectionUtils.isEmpty(pools)) { return null; } - if (pools.get(0).getPoolType().isShared()) { + short capacityType = Capacity.CAPACITY_TYPE_LOCAL_STORAGE; + String storageType = "local"; + StoragePool storagePool = pools.get(0); + if (storagePool.isShared()) { capacityType = Capacity.CAPACITY_TYPE_STORAGE_ALLOCATED; - } else { - capacityType = Capacity.CAPACITY_TYPE_LOCAL_STORAGE; + storageType = "shared"; } + s_logger.debug(String.format( + "Filtering storage pools by capacity type [%s] as the first storage pool of the list, with name [%s] and ID [%s], is a [%s] storage.", + capacityType, storagePool.getName(), storagePool.getUuid(), storageType + )); + List poolIdsByCapacity = capacityDao.orderHostsByFreeCapacity(zoneId, clusterId, capacityType); s_logger.debug(String.format("List of pools in descending order of available capacity [%s].", poolIdsByCapacity)); @@ -223,6 +229,8 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement } List reorderStoragePoolsBasedOnAlgorithm(List pools, DeploymentPlan plan, Account account) { + s_logger.debug(String.format("Using allocation algorithm [%s] to reorder pools.", allocationAlgorithm)); + if (allocationAlgorithm.equals("random") || allocationAlgorithm.equals("userconcentratedpod_random") || (account == null)) { reorderRandomPools(pools); } else if (StringUtils.equalsAny(allocationAlgorithm, "userdispersing", "firstfitleastconsumed")) { From 97be6f2e5d13c4fd2941f30522deebae0ab71827 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 31 Jan 2025 14:53:26 +0530 Subject: [PATCH 14/14] ui: fix column filter for templates, isos (#10288) Signed-off-by: Abhishek Kumar --- ui/src/components/view/ListView.vue | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ui/src/components/view/ListView.vue b/ui/src/components/view/ListView.vue index e67f1c4cc78..0c19ea8edab 100644 --- a/ui/src/components/view/ListView.vue +++ b/ui/src/components/view/ListView.vue @@ -32,7 +32,7 @@ - {{ $t('label.' + String(getColumTitle(column)).toLowerCase()) }} + {{ $t('label.' + String(getColumnTitle(column)).toLowerCase()) }} @@ -911,16 +911,16 @@ export default { return host.state }, getColumnKey (name) { - if (typeof name === 'object') { - name = Object.keys(name).includes('field') ? name.field : name.customTitle + if (typeof name !== 'object' || name === null) { + return name } - return name + return name.field ?? name.customTitle ?? Object.keys(name)[0] }, - getColumTitle (name) { - if (typeof name === 'object') { - name = Object.keys(name).includes('customTitle') ? name.customTitle : name.field + getColumnTitle (name) { + if (typeof name !== 'object' || name === null) { + return name } - return name + return name.customTitle ?? name.field ?? Object.keys(name)[0] }, updateSelectedColumns (name) { this.$emit('update-selected-columns', name)