Skip to content

server: fix url check for storages without a valid url #8353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shwstppr merged 6 commits into from
Dec 15, 2023
Merged

server: fix url check for storages without a valid url #8353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a3c545c
server: fix url check for managed storages
shwstppr Dec 13, 2023
ec1da3c
remove unnecessary imports
shwstppr Dec 13, 2023
33785a8
refactor
shwstppr Dec 13, 2023
af9a284
fix
shwstppr Dec 13, 2023
e8e0d31
checks
shwstppr Dec 14, 2023
cb2442e
refactor
shwstppr Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 67 additions & 36 deletions server/src/main/java/com/cloud/storage/StorageManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@

import static com.cloud.utils.NumbersUtil.toHumanReadableSize;

import java.io.UnsupportedEncodingException;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URLDecoder;
import java.net.UnknownHostException;
import java.nio.file.Files;
import java.sql.PreparedStatement;
Expand Down Expand Up @@ -132,8 +134,9 @@
import org.apache.cloudstack.storage.object.ObjectStoreEntity;
import org.apache.cloudstack.storage.to.VolumeObjectTO;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang3.EnumUtils;
import org.apache.commons.collections.MapUtils;
import org.apache.commons.lang.time.DateUtils;
import org.apache.commons.lang3.EnumUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
Expand Down Expand Up @@ -254,8 +257,6 @@
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.dao.VMInstanceDao;
import com.google.common.collect.Sets;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;


@Component
Expand Down Expand Up @@ -684,12 +685,21 @@ public boolean stop() {
return true;
}

private DataStore createLocalStorage(Map<String, Object> poolInfos) throws ConnectionException{
protected String getValidatedPareForLocalStorage(Object obj, String paramName) {
String result = obj == null ? null : obj.toString();
if (StringUtils.isEmpty(result)) {
throw new InvalidParameterValueException(String.format("Invalid %s provided", paramName));
}
return result;
}

protected DataStore createLocalStorage(Map<String, Object> poolInfos) throws ConnectionException{
Object existingUuid = poolInfos.get("uuid");
if( existingUuid == null ){
poolInfos.put("uuid", UUID.randomUUID().toString());
}
String hostAddress = poolInfos.get("host").toString();
String hostAddress = getValidatedPareForLocalStorage(poolInfos.get("host"), "host");
String hostPath = getValidatedPareForLocalStorage(poolInfos.get("hostPath"), "path");
Host host = _hostDao.findByName(hostAddress);

if( host == null ) {
Expand All @@ -708,8 +718,8 @@ private DataStore createLocalStorage(Map<String, Object> poolInfos) throws Conne

StoragePoolInfo pInfo = new StoragePoolInfo(poolInfos.get("uuid").toString(),
host.getPrivateIpAddress(),
poolInfos.get("hostPath").toString(),
poolInfos.get("hostPath").toString(),
hostPath,
hostPath,
StoragePoolType.Filesystem,
capacityBytes,
0,
Expand Down Expand Up @@ -809,6 +819,7 @@ protected String createLocalStoragePoolName(Host host, StoragePoolInfo storagePo
public PrimaryDataStoreInfo createPool(CreateStoragePoolCmd cmd) throws ResourceInUseException, IllegalArgumentException, UnknownHostException, ResourceUnavailableException {
String providerName = cmd.getStorageProviderName();
Map<String,String> uriParams = extractUriParamsAsMap(cmd.getUrl());
boolean isFileScheme = "file".equals(uriParams.get("scheme"));
DataStoreProvider storeProvider = _dataStoreProviderMgr.getDataStoreProvider(providerName);

if (storeProvider == null) {
Expand All @@ -822,7 +833,10 @@ public PrimaryDataStoreInfo createPool(CreateStoragePoolCmd cmd) throws Resource
Long podId = cmd.getPodId();
Long zoneId = cmd.getZoneId();

ScopeType scopeType = uriParams.get("scheme").toString().equals("file") ? ScopeType.HOST : ScopeType.CLUSTER;
ScopeType scopeType = ScopeType.CLUSTER;
if (isFileScheme) {
scopeType = ScopeType.HOST;
}
String scope = cmd.getScope();
if (scope != null) {
try {
Expand Down Expand Up @@ -889,12 +903,14 @@ public PrimaryDataStoreInfo createPool(CreateStoragePoolCmd cmd) throws Resource
params.put("managed", cmd.isManaged());
params.put("capacityBytes", cmd.getCapacityBytes());
params.put("capacityIops", cmd.getCapacityIops());
params.putAll(uriParams);
if (MapUtils.isNotEmpty(uriParams)) {
params.putAll(uriParams);
}

DataStoreLifeCycle lifeCycle = storeProvider.getDataStoreLifeCycle();
DataStore store = null;
try {
if (params.get("scheme").toString().equals("file")) {
if (isFileScheme) {
store = createLocalStorage(params);
} else {
store = lifeCycle.initialize(params);
Expand Down Expand Up @@ -923,42 +939,55 @@ public PrimaryDataStoreInfo createPool(CreateStoragePoolCmd cmd) throws Resource
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(store.getId(), DataStoreRole.Primary);
}

private Map<String,String> extractUriParamsAsMap(String url){
protected Map<String,String> extractUriParamsAsMap(String url) {
Map<String,String> uriParams = new HashMap<>();
UriUtils.UriInfo uriInfo = UriUtils.getUriInfo(url);
UriUtils.UriInfo uriInfo;
try {
uriInfo = UriUtils.getUriInfo(url);
} catch (CloudRuntimeException cre) {
if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("URI validation for url: %s failed, returning empty uri params", url));
}
return uriParams;
}

String scheme = uriInfo.getScheme();
String storageHost = uriInfo.getStorageHost();
String storagePath = uriInfo.getStoragePath();
try {
if (scheme == null) {
throw new InvalidParameterValueException("scheme is null " + url + ", add nfs:// (or cifs://) as a prefix");
} else if (scheme.equalsIgnoreCase("nfs")) {
if (storageHost == null || storagePath == null || storageHost.trim().isEmpty() || storagePath.trim().isEmpty()) {
throw new InvalidParameterValueException("host or path is null, should be nfs://hostname/path");
}
} else if (scheme.equalsIgnoreCase("cifs")) {
// Don't validate against a URI encoded URI.
if (scheme == null) {
if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("Scheme for url: %s is not found, returning empty uri params", url));
}
return uriParams;
}
boolean isHostOrPathBlank = StringUtils.isAnyBlank(storagePath, storageHost);
if (scheme.equalsIgnoreCase("nfs")) {
if (isHostOrPathBlank) {
throw new InvalidParameterValueException("host or path is null, should be nfs://hostname/path");
}
} else if (scheme.equalsIgnoreCase("cifs")) {
// Don't validate against a URI encoded URI.
try {
URI cifsUri = new URI(url);
String warnMsg = UriUtils.getCifsUriParametersProblems(cifsUri);
if (warnMsg != null) {
throw new InvalidParameterValueException(warnMsg);
}
} else if (scheme.equalsIgnoreCase("sharedMountPoint")) {
if (storagePath == null) {
throw new InvalidParameterValueException("host or path is null, should be sharedmountpoint://localhost/path");
}
} else if (scheme.equalsIgnoreCase("rbd")) {
if (storagePath == null) {
throw new InvalidParameterValueException("host or path is null, should be rbd://hostname/pool");
}
} else if (scheme.equalsIgnoreCase("gluster")) {
if (storageHost == null || storagePath == null || storageHost.trim().isEmpty() || storagePath.trim().isEmpty()) {
throw new InvalidParameterValueException("host or path is null, should be gluster://hostname/volume");
}
} catch (URISyntaxException e) {
throw new InvalidParameterValueException(url + " is not a valid uri");
}
} else if (scheme.equalsIgnoreCase("sharedMountPoint")) {
if (storagePath == null) {
throw new InvalidParameterValueException("host or path is null, should be sharedmountpoint://localhost/path");
}
} else if (scheme.equalsIgnoreCase("rbd")) {
if (storagePath == null) {
throw new InvalidParameterValueException("host or path is null, should be rbd://hostname/pool");
}
} else if (scheme.equalsIgnoreCase("gluster")) {
if (isHostOrPathBlank) {
throw new InvalidParameterValueException("host or path is null, should be gluster://hostname/volume");
}
} catch (URISyntaxException e) {
throw new InvalidParameterValueException(url + " is not a valid uri");
}

String hostPath = null;
Expand All @@ -975,7 +1004,9 @@ private Map<String,String> extractUriParamsAsMap(String url){
uriParams.put("host", storageHost);
uriParams.put("hostPath", hostPath);
uriParams.put("userInfo", uriInfo.getUserInfo());
uriParams.put("port", uriInfo.getPort() + "");
if (uriInfo.getPort() > 0) {
uriParams.put("port", uriInfo.getPort() + "");
}
return uriParams;
}

Expand Down
49 changes: 49 additions & 0 deletions server/src/test/java/com/cloud/storage/StorageManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@
package com.cloud.storage;

import com.cloud.agent.api.StoragePoolInfo;
import com.cloud.exception.ConnectionException;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.host.Host;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.dao.VMInstanceDao;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.commons.collections.MapUtils;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -33,7 +36,9 @@
import org.mockito.junit.MockitoJUnitRunner;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

@RunWith(MockitoJUnitRunner.class)
public class StorageManagerImplTest {
Expand Down Expand Up @@ -157,4 +162,48 @@ public void storagePoolCompatibleWithVolumePoolTestVolumeWithoutPoolIdInAllocate

}

@Test
public void testExtractUriParamsAsMapWithSolidFireUrl() {
String sfUrl = "MVIP=1.2.3.4;SVIP=6.7.8.9;clusterAdminUsername=admin;" +
"clusterAdminPassword=password;clusterDefaultMinIops=1000;" +
"clusterDefaultMaxIops=2000;clusterDefaultBurstIopsPercentOfMaxIops=2";
Map<String,String> uriParams = storageManagerImpl.extractUriParamsAsMap(sfUrl);
Assert.assertTrue(MapUtils.isEmpty(uriParams));
}

@Test
public void testExtractUriParamsAsMapWithNFSUrl() {
String scheme = "nfs";
String host = "HOST";
String path = "/PATH";
String sfUrl = String.format("%s://%s%s", scheme, host, path);
Map<String,String> uriParams = storageManagerImpl.extractUriParamsAsMap(sfUrl);
Assert.assertTrue(MapUtils.isNotEmpty(uriParams));
Assert.assertEquals(scheme, uriParams.get("scheme"));
Assert.assertEquals(host, uriParams.get("host"));
Assert.assertEquals(path, uriParams.get("hostPath"));
}

@Test(expected = InvalidParameterValueException.class)
public void testCreateLocalStorageHostFailure() {
Map<String, Object> test = new HashMap<>();
test.put("host", null);
try {
storageManagerImpl.createLocalStorage(test);
} catch (ConnectionException e) {
throw new RuntimeException(e);
}
}

@Test(expected = InvalidParameterValueException.class)
public void testCreateLocalStoragePathFailure() {
Map<String, Object> test = new HashMap<>();
test.put("host", "HOST");
test.put("hostPath", "");
try {
storageManagerImpl.createLocalStorage(test);
} catch (ConnectionException e) {
throw new RuntimeException(e);
}
}
}