From cee7a713aa2d15372684640de3e8c68ff3916bc6 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 21 Jul 2023 10:55:51 +0530 Subject: [PATCH 1/5] server: clear resource reservation and increment resource count in a transaction (#7724) This PR addresses rare case of potential overlap of resource reservation and resource count. For different resource types there could be some delay between incrementing of the resource count and clearing of the earlier done reservation. This may result in failures when there are parallel deployments happening. Signed-off-by: Abhishek Kumar --- .../cloudstack/context/CallContext.java | 5 +++ .../resourcelimit/CheckedReservation.java | 20 ++++++++---- .../ResourceLimitManagerImpl.java | 31 +++++++++++++------ 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/context/CallContext.java b/api/src/main/java/org/apache/cloudstack/context/CallContext.java index 50049b30a07..ecc109977eb 100644 --- a/api/src/main/java/org/apache/cloudstack/context/CallContext.java +++ b/api/src/main/java/org/apache/cloudstack/context/CallContext.java @@ -92,6 +92,10 @@ public class CallContext { context.put(key, value); } + public void removeContextParameter(Object key) { + context.remove(key); + } + /** * @param key any not null key object * @return the value of the key from context map @@ -234,6 +238,7 @@ public class CallContext { CallContext callContext = register(parent.getCallingUserId(), parent.getCallingAccountId()); callContext.setStartEventId(parent.getStartEventId()); callContext.setEventResourceType(eventResourceType); + callContext.putContextParameters(parent.getContextParameters()); return callContext; } diff --git a/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java b/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java index 0075ef1b4f1..d0c20a3a7af 100644 --- a/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java +++ b/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java @@ -18,17 +18,19 @@ // package com.cloud.resourcelimit; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.reservation.ReservationVO; +import org.apache.cloudstack.reservation.dao.ReservationDao; +import org.apache.cloudstack.user.ResourceReservation; +import org.apache.log4j.Logger; +import org.jetbrains.annotations.NotNull; + import com.cloud.configuration.Resource.ResourceType; import com.cloud.exception.ResourceAllocationException; import com.cloud.user.Account; import com.cloud.user.ResourceLimitService; import com.cloud.utils.db.GlobalLock; -import org.apache.cloudstack.user.ResourceReservation; import com.cloud.utils.exception.CloudRuntimeException; -import org.apache.cloudstack.reservation.ReservationVO; -import org.apache.cloudstack.reservation.dao.ReservationDao; -import org.apache.log4j.Logger; -import org.jetbrains.annotations.NotNull; public class CheckedReservation implements AutoCloseable, ResourceReservation { @@ -42,6 +44,10 @@ public class CheckedReservation implements AutoCloseable, ResourceReservation { private Long amount; private ResourceReservation reservation; + private String getContextParameterKey() { + return String.format("%s-%s", ResourceReservation.class.getSimpleName(), resourceType.getName()); + } + /** * - check if adding a reservation is allowed * - create DB entry for reservation @@ -70,6 +76,7 @@ public class CheckedReservation implements AutoCloseable, ResourceReservation { resourceLimitService.checkResourceLimit(account,resourceType,amount); ReservationVO reservationVO = new ReservationVO(account.getAccountId(), account.getDomainId(), resourceType, amount); this.reservation = reservationDao.persist(reservationVO); + CallContext.current().putContextParameter(getContextParameterKey(), reservationVO.getId()); } catch (NullPointerException npe) { throw new CloudRuntimeException("not enough means to check limits", npe); } finally { @@ -97,7 +104,8 @@ public class CheckedReservation implements AutoCloseable, ResourceReservation { @Override public void close() throws Exception { - if (this.reservation != null){ + if (this.reservation != null) { + CallContext.current().removeContextParameter(getContextParameterKey()); reservationDao.remove(reservation.getId()); reservation = null; } diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 9efeed39ac0..288315d5b62 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -16,6 +16,8 @@ // under the License. package com.cloud.resourcelimit; +import static com.cloud.utils.NumbersUtil.toHumanReadableSize; + import java.util.ArrayList; import java.util.Arrays; import java.util.EnumMap; @@ -103,13 +105,11 @@ import com.cloud.utils.db.TransactionCallbackNoReturn; import com.cloud.utils.db.TransactionCallbackWithExceptionNoReturn; import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.VirtualMachine.State; +import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; -import static com.cloud.utils.NumbersUtil.toHumanReadableSize; - @Component public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLimitService, Configurable { public static final Logger s_logger = Logger.getLogger(ResourceLimitManagerImpl.class); @@ -173,6 +173,23 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim Map domainResourceLimitMap = new EnumMap(ResourceType.class); Map projectResourceLimitMap = new EnumMap(ResourceType.class); + protected void removeResourceReservationIfNeededAndIncrementResourceCount(final long accountId, final ResourceType type, final long numToIncrement) { + Transaction.execute(new TransactionCallbackWithExceptionNoReturn() { + @Override + public void doInTransactionWithoutResult(TransactionStatus status) throws CloudRuntimeException { + + Object obj = CallContext.current().getContextParameter(String.format("%s-%s", ResourceReservation.class.getSimpleName(), type.getName())); + if (obj instanceof Long) { + reservationDao.remove((long)obj); + } + if (!updateResourceCountForAccount(accountId, type, true, numToIncrement)) { + // we should fail the operation (resource creation) when failed to update the resource count + throw new CloudRuntimeException("Failed to increment resource count of type " + type + " for account id=" + accountId); + } + } + }); + } + @Override public boolean start() { if (_resourceCountCheckInterval > 0) { @@ -270,12 +287,8 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim return; } - long numToIncrement = (delta.length == 0) ? 1 : delta[0].longValue(); - - if (!updateResourceCountForAccount(accountId, type, true, numToIncrement)) { - // we should fail the operation (resource creation) when failed to update the resource count - throw new CloudRuntimeException("Failed to increment resource count of type " + type + " for account id=" + accountId); - } + final long numToIncrement = (delta.length == 0) ? 1 : delta[0].longValue(); + removeResourceReservationIfNeededAndIncrementResourceCount(accountId, type, numToIncrement); } @Override From 80ca3acf1574612974a9340dd94954916c2a9aad Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Fri, 21 Jul 2023 12:38:21 +0530 Subject: [PATCH 2/5] Allow encrypted volume migration for PowerFlex volumes (#7757) --- .../cloud/storage/VolumeApiServiceImpl.java | 11 ++-- .../storage/VolumeApiServiceImplTest.java | 52 +++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index d6325b43134..e39fac147e7 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2999,6 +2999,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic throw new InvalidParameterValueException("Volume cannot be migrated, please remove all VM snapshots for VM to which this volume is attached"); } + StoragePoolVO srcStoragePoolVO = _storagePoolDao.findById(vol.getPoolId()); + // OfflineVmwareMigration: extract this block as method and check if it is subject to regression if (vm != null && State.Running.equals(vm.getState())) { // Check if the VM is GPU enabled. @@ -3020,16 +3022,15 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic liveMigrateVolume = capabilities.isStorageMotionSupported(); } - StoragePoolVO storagePoolVO = _storagePoolDao.findById(vol.getPoolId()); - if (liveMigrateVolume && HypervisorType.KVM.equals(host.getHypervisorType()) && !storagePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { + if (liveMigrateVolume && HypervisorType.KVM.equals(host.getHypervisorType()) && !srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { StoragePoolVO destinationStoragePoolVo = _storagePoolDao.findById(storagePoolId); - if (isSourceOrDestNotOnStorPool(storagePoolVO, destinationStoragePoolVo)) { + if (isSourceOrDestNotOnStorPool(srcStoragePoolVO, destinationStoragePoolVo)) { throw new InvalidParameterValueException("KVM does not support volume live migration due to the limited possibility to refresh VM XML domain. " + "Therefore, to live migrate a volume between storage pools, one must migrate the VM to a different host as well to force the VM XML domain update. " + "Use 'migrateVirtualMachineWithVolumes' instead."); } - srcAndDestOnStorPool = isSourceAndDestOnStorPool(storagePoolVO, destinationStoragePoolVo); + srcAndDestOnStorPool = isSourceAndDestOnStorPool(srcStoragePoolVO, destinationStoragePoolVo); } } @@ -3043,7 +3044,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } } - if (vol.getPassphraseId() != null && !srcAndDestOnStorPool) { + if (vol.getPassphraseId() != null && !srcAndDestOnStorPool && !srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { throw new InvalidParameterValueException("Migration of encrypted volumes is unsupported"); } diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index e5b85c829e4..35b69cf82e4 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -17,6 +17,7 @@ package com.cloud.storage; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyObject; @@ -40,8 +41,11 @@ import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker.AccessType; 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.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; @@ -52,6 +56,7 @@ import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext; import org.apache.cloudstack.framework.jobs.AsyncJobManager; import org.apache.cloudstack.framework.jobs.dao.AsyncJobJoinMapDao; import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; +import org.apache.cloudstack.snapshot.SnapshotHelper; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; @@ -114,6 +119,7 @@ import com.cloud.user.ResourceLimitService; import com.cloud.user.User; import com.cloud.user.UserVO; import com.cloud.user.dao.AccountDao; +import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.db.TransactionLegacy; import com.cloud.utils.fsm.NoTransitionException; import com.cloud.vm.UserVmManager; @@ -169,6 +175,8 @@ public class VolumeApiServiceImplTest { @Mock private CreateVolumeCmd createVol; @Mock + private MigrateVolumeCmd migrateVolumeCmd; + @Mock private UserVmManager userVmManager; @Mock private DataCenterDao _dcDao; @@ -188,6 +196,10 @@ public class VolumeApiServiceImplTest { private ServiceOfferingDao serviceOfferingDao; @Mock private DiskOfferingDao _diskOfferingDao; + @Mock + private DataStoreManager dataStoreMgr; + @Mock + private SnapshotHelper snapshotHelper; private DetachVolumeCmd detachCmd = new DetachVolumeCmd(); private Class _detachCmdClass = detachCmd.getClass(); @@ -218,6 +230,9 @@ public class VolumeApiServiceImplTest { @Mock private ProjectManager projectManagerMock; + @Mock + private StorageManager storageMgr; + private long accountMockId = 456l; private long volumeMockId = 12313l; private long vmInstanceMockId = 1123l; @@ -1579,4 +1594,41 @@ public class VolumeApiServiceImplTest { Mockito.when(host.getHypervisorType()).thenReturn(HypervisorType.KVM); Assert.assertFalse(volumeApiServiceImpl.isSendCommandForVmVolumeAttachDetach(host, Mockito.mock(StoragePoolVO.class))); } + + // Below test covers both allowing encrypted volume migration for PowerFlex storage and expect error on storagepool compatibility + @Test + public void testStoragePoolCompatibilityAndAllowEncryptedVolumeMigrationForPowerFlexStorage() { + try { + Mockito.when(migrateVolumeCmd.getVolumeId()).thenReturn(1L); + Mockito.when(migrateVolumeCmd.getStoragePoolId()).thenReturn(2L); + VolumeVO vol = Mockito.mock(VolumeVO.class); + Mockito.when(volumeDaoMock.findById(1L)).thenReturn(vol); + Mockito.when(volumeDaoMock.getHypervisorType(1L)).thenReturn(HypervisorType.KVM); + Mockito.when(vol.getState()).thenReturn(Volume.State.Ready); + Mockito.when(vol.getPoolId()).thenReturn(1L); + Mockito.when(vol.getInstanceId()).thenReturn(null); + Mockito.when(vol.getDiskOfferingId()).thenReturn(1L); + DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class); + Mockito.when(_diskOfferingDao.findById(1L)).thenReturn(diskOffering); + + StoragePoolVO srcStoragePoolVOMock = Mockito.mock(StoragePoolVO.class); + StoragePool destPool = Mockito.mock(StoragePool.class); + PrimaryDataStore dataStore = Mockito.mock(PrimaryDataStore.class); + + Mockito.when(vol.getPassphraseId()).thenReturn(1L); + Mockito.when(primaryDataStoreDaoMock.findById(1L)).thenReturn(srcStoragePoolVOMock); + Mockito.when(srcStoragePoolVOMock.getPoolType()).thenReturn(Storage.StoragePoolType.PowerFlex); + Mockito.when(dataStoreMgr.getDataStore(2L, DataStoreRole.Primary)).thenReturn( dataStore); + Mockito.doNothing().when(snapshotHelper).checkKvmVolumeSnapshotsOnlyInPrimaryStorage(vol, HypervisorType.KVM); + Mockito.when(destPool.getUuid()).thenReturn("bd525970-3d2a-4230-880d-261892129ef3"); + + Mockito.when(storageMgr.storagePoolCompatibleWithVolumePool(destPool, vol)).thenReturn(false); + + volumeApiServiceImpl.migrateVolume(migrateVolumeCmd); + } catch (InvalidParameterValueException e) { + fail("Unexpected InvalidParameterValueException was thrown"); + } catch (CloudRuntimeException e) { + // test passed + } + } } From f057f4b412e0e913448c9ce9f34ee5c7a746c6c4 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 21 Jul 2023 16:08:08 +0530 Subject: [PATCH 3/5] ui: fix userdata base64 encoding (#7749) * ui: fix userdata abse64 encoding Fixes #7748 Signed-off-by: Abhishek Kumar --- ui/src/utils/plugins.js | 8 ++++++++ ui/src/views/compute/AutoScaleVmProfile.vue | 10 +--------- ui/src/views/compute/CreateAutoScaleVmGroup.vue | 10 +--------- ui/src/views/compute/DeployVM.vue | 3 +-- ui/src/views/compute/EditVM.vue | 3 +-- ui/src/views/compute/RegisterUserData.vue | 10 +--------- ui/src/views/compute/ResetUserData.vue | 10 +--------- 7 files changed, 14 insertions(+), 40 deletions(-) diff --git a/ui/src/utils/plugins.js b/ui/src/utils/plugins.js index 2e2c787366e..5cc4bc6269a 100644 --- a/ui/src/utils/plugins.js +++ b/ui/src/utils/plugins.js @@ -484,6 +484,14 @@ export const genericUtilPlugin = { const regexExp = /^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$/gi return regexExp.test(uuid) } + + app.config.globalProperties.$toBase64AndURIEncoded = function (text) { + const base64regex = /^([0-9a-zA-Z+/]{4})*(([0-9a-zA-Z+/]{2}==)|([0-9a-zA-Z+/]{3}=))?$/ + if (base64regex.test(text)) { + return text + } + return encodeURIComponent(btoa(unescape(encodeURIComponent(text)))) + } } } diff --git a/ui/src/views/compute/AutoScaleVmProfile.vue b/ui/src/views/compute/AutoScaleVmProfile.vue index b00a35f04dd..45947753b06 100644 --- a/ui/src/views/compute/AutoScaleVmProfile.vue +++ b/ui/src/views/compute/AutoScaleVmProfile.vue @@ -519,7 +519,7 @@ export default { params.autoscaleuserid = this.autoscaleuserid } if (this.userdata && this.userdata.length > 0) { - params.userdata = encodeURIComponent(btoa(this.sanitizeReverse(this.userdata))) + params.userdata = this.$toBase64AndURIEncoded(this.userdata) } const httpMethod = params.userdata ? 'POST' : 'GET' @@ -539,14 +539,6 @@ export default { this.loading = false }) }, - sanitizeReverse (value) { - const reversedValue = value - .replace(/&/g, '&') - .replace(/</g, '<') - .replace(/>/g, '>') - - return reversedValue - }, decodeUserData (userdata) { const decodedData = Buffer.from(userdata, 'base64') return decodedData.toString('utf-8') diff --git a/ui/src/views/compute/CreateAutoScaleVmGroup.vue b/ui/src/views/compute/CreateAutoScaleVmGroup.vue index c807228f703..4dcb2955274 100644 --- a/ui/src/views/compute/CreateAutoScaleVmGroup.vue +++ b/ui/src/views/compute/CreateAutoScaleVmGroup.vue @@ -2425,7 +2425,7 @@ export default { createVmGroupData.keypairs = this.sshKeyPairs.join(',') createVmGroupData.affinitygroupids = (values.affinitygroupids || []).join(',') if (values.userdata && values.userdata.length > 0) { - createVmGroupData.userdata = encodeURIComponent(btoa(this.sanitizeReverse(values.userdata))) + createVmGroupData.userdata = this.$toBase64AndURIEncoded(values.userdata) } // vm profile details @@ -2702,14 +2702,6 @@ export default { this.params[name].options = { ...this.params[name].options, ...options } this.fetchOptions(this.params[name], name) }, - sanitizeReverse (value) { - const reversedValue = value - .replace(/&/g, '&') - .replace(/</g, '<') - .replace(/>/g, '>') - - return reversedValue - }, fetchTemplateNics (template) { var nics = [] this.nicToNetworkSelection = [] diff --git a/ui/src/views/compute/DeployVM.vue b/ui/src/views/compute/DeployVM.vue index b15fa8bcccf..de74ab8f268 100644 --- a/ui/src/views/compute/DeployVM.vue +++ b/ui/src/views/compute/DeployVM.vue @@ -836,7 +836,6 @@ import UserDataSelection from '@views/compute/wizard/UserDataSelection' import SecurityGroupSelection from '@views/compute/wizard/SecurityGroupSelection' import TooltipLabel from '@/components/widgets/TooltipLabel' import InstanceNicsNetworkSelectListView from '@/components/view/InstanceNicsNetworkSelectListView.vue' -import { sanitizeReverse } from '@/utils/util' export default { name: 'Wizard', @@ -1971,7 +1970,7 @@ export default { deployVmData.iothreadsenabled = values.iothreadsenabled deployVmData.iodriverpolicy = values.iodriverpolicy if (values.userdata && values.userdata.length > 0) { - deployVmData.userdata = encodeURIComponent(btoa(sanitizeReverse(values.userdata))) + deployVmData.userdata = this.$toBase64AndURIEncoded(values.userdata) } // step 2: select template/iso if (this.tabKey === 'templateid') { diff --git a/ui/src/views/compute/EditVM.vue b/ui/src/views/compute/EditVM.vue index b3861d89f46..9aae2f38755 100644 --- a/ui/src/views/compute/EditVM.vue +++ b/ui/src/views/compute/EditVM.vue @@ -123,7 +123,6 @@ import { ref, reactive, toRaw } from 'vue' import { api } from '@/api' import TooltipLabel from '@/components/widgets/TooltipLabel' -import { sanitizeReverse } from '@/utils/util' export default { name: 'EditVM', @@ -317,7 +316,7 @@ export default { params.group = values.group } if (values.userdata && values.userdata.length > 0) { - params.userdata = encodeURIComponent(btoa(sanitizeReverse(values.userdata))) + params.userdata = this.$toBase64AndURIEncoded(values.userdata) } this.loading = true diff --git a/ui/src/views/compute/RegisterUserData.vue b/ui/src/views/compute/RegisterUserData.vue index 36e469f676c..a8e5577d005 100644 --- a/ui/src/views/compute/RegisterUserData.vue +++ b/ui/src/views/compute/RegisterUserData.vue @@ -184,14 +184,6 @@ export default { handleDomainChanged (domain) { this.selectedDomain = domain }, - sanitizeReverse (value) { - const reversedValue = value - .replace(/&/g, '&') - .replace(/</g, '<') - .replace(/>/g, '>') - - return reversedValue - }, handleSubmit (e) { e.preventDefault() if (this.loading) return @@ -209,7 +201,7 @@ export default { if (this.isValidValueForKey(values, 'account') && values.account.length > 0) { params.account = values.account } - params.userdata = encodeURIComponent(btoa(this.sanitizeReverse(values.userdata))) + params.userdata = this.$toBase64AndURIEncoded(values.userdata) if (values.params != null && values.params.length > 0) { var userdataparams = values.params.join(',') diff --git a/ui/src/views/compute/ResetUserData.vue b/ui/src/views/compute/ResetUserData.vue index b7287751032..46561f15c1a 100644 --- a/ui/src/views/compute/ResetUserData.vue +++ b/ui/src/views/compute/ResetUserData.vue @@ -289,14 +289,6 @@ export default { this[type] = key this.userDataParams = [] }, - sanitizeReverse (value) { - const reversedValue = value - .replace(/&/g, '&') - .replace(/</g, '<') - .replace(/>/g, '>') - - return reversedValue - }, isUserAllowedToListUserDatas () { return Boolean('listUserData' in this.$store.getters.apis) }, @@ -346,7 +338,7 @@ export default { id: this.resource.id } if (values.userdata && values.userdata.length > 0) { - params.userdata = encodeURIComponent(btoa(this.sanitizeReverse(values.userdata))) + params.userdata = this.$toBase64AndURIEncoded(values.userdata) } if (values.userdataid) { params.userdataid = values.userdataid From 90baae3dcdf021bacfc526edf0eb9e9e15f1ce4c Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 24 Jul 2023 14:29:01 +0800 Subject: [PATCH 4/5] utils: fix RBD URI if credentials contains slash (#7708) --- ui/src/views/infra/AddPrimaryStorage.vue | 5 --- .../main/java/com/cloud/utils/UriUtils.java | 31 ++++++++++++------- .../java/com/cloud/utils/UriUtilsTest.java | 14 ++++++++- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/ui/src/views/infra/AddPrimaryStorage.vue b/ui/src/views/infra/AddPrimaryStorage.vue index b6f7922d797..c1c733dbb80 100644 --- a/ui/src/views/infra/AddPrimaryStorage.vue +++ b/ui/src/views/infra/AddPrimaryStorage.vue @@ -586,11 +586,6 @@ export default { }, rbdURL (monitor, pool, id, secret) { var url - /* Replace the + and / symbols by - and _ to have URL-safe base64 going to the API - It's hacky, but otherwise we'll confuse java.net.URI which splits the incoming URI - */ - secret = secret.replace(/\+/g, '-') - secret = secret.replace(/\//g, '_') if (id !== null && secret !== null) { monitor = id + ':' + secret + '@' + monitor } diff --git a/utils/src/main/java/com/cloud/utils/UriUtils.java b/utils/src/main/java/com/cloud/utils/UriUtils.java index e68b5307f0e..dffc0106e8a 100644 --- a/utils/src/main/java/com/cloud/utils/UriUtils.java +++ b/utils/src/main/java/com/cloud/utils/UriUtils.java @@ -619,21 +619,30 @@ public class UriUtils { } private static UriInfo getRbdUrlInfo(String url) { - int secondSlash = StringUtils.ordinalIndexOf(url, "/", 2); - int thirdSlash = StringUtils.ordinalIndexOf(url, "/", 3); + if (url == null || !url.toLowerCase().startsWith("rbd://")) { + throw new CloudRuntimeException("RBD URL must start with \"rbd://\""); + } + String schema = StringUtils.substring(url, 0, 6); + url = StringUtils.substring(url, 6, url.length()); int firstAt = StringUtils.indexOf(url, "@"); - int lastColon = StringUtils.lastIndexOf(url,":"); - int lastSquareBracket = StringUtils.lastIndexOf(url,"]"); - int startOfHost = Math.max(secondSlash, firstAt) + 1; - int endOfHost = lastColon < startOfHost ? (thirdSlash > 0 ? thirdSlash : url.length() + 1) : + String credentials = (firstAt == -1) ? null : StringUtils.substring(url, 0, firstAt); + String hostInfo = (firstAt == -1) ? url : StringUtils.substring(url, firstAt + 1, url.length()); + + int firstSlash = StringUtils.indexOf(hostInfo, "/"); + int lastColon = StringUtils.lastIndexOf(hostInfo,":"); + int lastSquareBracket = StringUtils.lastIndexOf(hostInfo,"]"); + int endOfHost = lastColon == -1 ? (firstSlash > 0 ? firstSlash : hostInfo.length() + 1) : (lastSquareBracket > lastColon ? lastSquareBracket + 1 : lastColon); - String storageHosts = StringUtils.substring(url, startOfHost, endOfHost); + String storageHosts = StringUtils.substring(hostInfo, 0, endOfHost); String firstHost = storageHosts.split(",")[0]; - String strBeforeHosts = StringUtils.substring(url, 0, startOfHost); - String strAfterHosts = StringUtils.substring(url, endOfHost); + String strAfterHosts = StringUtils.substring(hostInfo, endOfHost); try { - URI uri = new URI(UriUtils.encodeURIComponent(strBeforeHosts + firstHost + strAfterHosts)); - return new UriInfo(uri.getScheme(), storageHosts, uri.getPath(), uri.getUserInfo(), uri.getPort()); + URI uri = new URI(UriUtils.encodeURIComponent(schema + firstHost + strAfterHosts)); + if (credentials != null) { + credentials = credentials.replace("+", "-"); + credentials = credentials.replace("/", "_"); + } + return new UriInfo(uri.getScheme(), storageHosts, uri.getPath(), credentials, uri.getPort()); } catch (URISyntaxException e) { throw new CloudRuntimeException(url + " is not a valid uri for RBD"); } diff --git a/utils/src/test/java/com/cloud/utils/UriUtilsTest.java b/utils/src/test/java/com/cloud/utils/UriUtilsTest.java index db02e607d63..1a3ae17f584 100644 --- a/utils/src/test/java/com/cloud/utils/UriUtilsTest.java +++ b/utils/src/test/java/com/cloud/utils/UriUtilsTest.java @@ -103,10 +103,14 @@ public class UriUtilsTest { } private void testGetUriInfoInternal(String url, String host) { + testGetUriInfoInternal(url, host, url); + } + + private void testGetUriInfoInternal(String url, String host, String newUrl) { UriUtils.UriInfo uriInfo = UriUtils.getUriInfo(url); Assert.assertEquals(host, uriInfo.getStorageHost()); - Assert.assertEquals(url, uriInfo.toString()); + Assert.assertEquals(newUrl, uriInfo.toString()); } @Test @@ -122,6 +126,10 @@ public class UriUtilsTest { String url6 = String.format("rbd://%s:3300", host); String url7 = String.format("rbd://%s", host); String url8 = String.format("rbd://user@%s", host); + String url9 = String.format("rbd://cloudstack:AQD+hJxklW1RGRAAA56oHGN6d+WPDLss2b05Cw==@%s:3300/cloudstack", host); + String url10 = String.format("rbd://cloudstack:AQDlhZxkgdmiKRAA8uHt/O9jqoBp2Iwdk2MjjQ==@%s:3300/cloudstack", host); + String url11 = String.format("rbd://cloudstack:AQD-hJxklW1RGRAAA56oHGN6d-WPDLss2b05Cw==@%s:3300/cloudstack", host); + String url12 = String.format("rbd://cloudstack:AQDlhZxkgdmiKRAA8uHt_O9jqoBp2Iwdk2MjjQ==@%s:3300/cloudstack", host); testGetUriInfoInternal(url0, host); testGetUriInfoInternal(url1, host); @@ -132,6 +140,10 @@ public class UriUtilsTest { testGetUriInfoInternal(url6, host); testGetUriInfoInternal(url7, host); testGetUriInfoInternal(url8, host); + testGetUriInfoInternal(url9, host, url11); + testGetUriInfoInternal(url10, host, url12); + testGetUriInfoInternal(url11, host); + testGetUriInfoInternal(url12, host); } @Test From 63216425d5beaa3ca25597cd6b5319c189179bab Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Mon, 24 Jul 2023 01:43:46 -0600 Subject: [PATCH 5/5] Set encrypted PowerFlex disk format correctly (#7735) Co-authored-by: Marcus Sorensen --- .../cloud/hypervisor/kvm/storage/KVMStorageProcessor.java | 3 +++ .../datastore/driver/ScaleIOPrimaryDataStoreDriver.java | 8 ++------ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index fd23d23745c..69c28d4350a 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1609,6 +1609,9 @@ public class KVMStorageProcessor implements StorageProcessor { if (vol.getQemuEncryptFormat() != null) { newVol.setEncryptFormat(vol.getQemuEncryptFormat().toString()); } + if (vol.getFormat() != null) { + format = vol.getFormat(); + } } newVol.setSize(volume.getSize()); newVol.setFormat(ImageFormat.valueOf(format.toString().toUpperCase())); diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java index 3a285b49904..cad88dcdd15 100644 --- a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java @@ -504,11 +504,7 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver { volume.setFolder(scaleIOVolume.getVtreeId()); volume.setSize(scaleIOVolume.getSizeInKb() * 1024); volume.setPoolType(Storage.StoragePoolType.PowerFlex); - if (volumeInfo.getVolumeType().equals(Volume.Type.ROOT)) { - volume.setFormat(volumeInfo.getFormat()); - } else { - volume.setFormat(Storage.ImageFormat.RAW); - } + volume.setFormat(volumeInfo.getFormat()); volume.setPoolId(storagePoolId); VolumeObject createdObject = VolumeObject.getVolumeObject(volumeInfo.getDataStore(), volume); createdObject.update(); @@ -1202,7 +1198,7 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver { if (payload.instanceName != null) { VMInstanceVO instance = vmInstanceDao.findVMByInstanceName(payload.instanceName); - if (instance.getState().equals(VirtualMachine.State.Running)) { + if (instance != null && instance.getState().equals(VirtualMachine.State.Running)) { hostId = instance.getHostId(); attachedRunning = true; }