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/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; } 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 16090ebcf52..51b1635a53a 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 diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 57d5d0e1f6e..bc9a4215fc4 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 + } + } } 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 ab8821d1259..705bfb59d10 100644 --- a/ui/src/views/compute/AutoScaleVmProfile.vue +++ b/ui/src/views/compute/AutoScaleVmProfile.vue @@ -531,7 +531,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' @@ -551,14 +551,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 de29c9e3505..3797077ba6d 100644 --- a/ui/src/views/compute/CreateAutoScaleVmGroup.vue +++ b/ui/src/views/compute/CreateAutoScaleVmGroup.vue @@ -2429,7 +2429,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 @@ -2706,14 +2706,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 d635d3f8e8c..0f63f4e30f0 100644 --- a/ui/src/views/compute/DeployVM.vue +++ b/ui/src/views/compute/DeployVM.vue @@ -860,7 +860,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', @@ -1997,7 +1996,7 @@ export default { deployVmData.nicmultiqueuenumber = values.nicmultiqueuenumber deployVmData.nicpackedvirtqueuesenabled = values.nicpackedvirtqueuesenabled 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 a90f50fc540..4288bc3531b 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 4392125c6dc..a1322a12832 100644 --- a/ui/src/views/compute/RegisterUserData.vue +++ b/ui/src/views/compute/RegisterUserData.vue @@ -187,14 +187,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 @@ -212,7 +204,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 827602150b1..7be1dad8fbd 100644 --- a/ui/src/views/compute/ResetUserData.vue +++ b/ui/src/views/compute/ResetUserData.vue @@ -293,14 +293,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) }, @@ -350,7 +342,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 diff --git a/ui/src/views/infra/AddPrimaryStorage.vue b/ui/src/views/infra/AddPrimaryStorage.vue index eae9abbcd0f..b30b0341ca4 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