From ff3e9bd821f9d3e528b092fa31f338a73d2afc54 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 26 Mar 2024 11:36:01 +0530 Subject: [PATCH] engine-storage: control download redirection Add a global setting to control whether redirection is allowed while downloading templates and volumes Signed-off-by: Abhishek Kumar --- .../template/HttpTemplateDownloader.java | 18 ++++++- .../template/MetalinkTemplateDownloader.java | 10 +++- .../template/S3TemplateDownloader.java | 19 +++++-- .../storage/template/TemplateDownloader.java | 2 + .../template/TemplateDownloaderBase.java | 6 +++ .../agent/directdownload/CheckUrlCommand.java | 11 ++++- .../directdownload/DirectDownloadCommand.java | 15 +++++- .../HttpDirectDownloadCommand.java | 6 ++- .../HttpsDirectDownloadCommand.java | 7 ++- .../MetalinkDirectDownloadCommand.java | 5 +- .../NfsDirectDownloadCommand.java | 5 +- .../direct/download/DirectDownloadHelper.java | 49 +++++++++++-------- .../DirectTemplateDownloaderImpl.java | 13 ++++- .../HttpDirectTemplateDownloader.java | 22 +++++---- .../HttpsDirectTemplateDownloader.java | 28 +++++++---- .../MetalinkDirectTemplateDownloader.java | 21 ++++---- .../download/NfsDirectTemplateDownloader.java | 5 +- .../storage/command/DownloadCommand.java | 13 +++++ .../storage/to/DownloadableObjectTO.java | 30 ++++++++++++ .../storage/to/SnapshotObjectTO.java | 2 +- .../storage/to/TemplateObjectTO.java | 3 +- .../cloudstack/storage/to/VolumeObjectTO.java | 3 +- .../BaseDirectTemplateDownloaderTest.java | 2 +- .../MetalinkDirectTemplateDownloaderTest.java | 2 +- .../api/storage/DownloadableDataInfo.java | 24 +++++++++ .../subsystem/api/storage/TemplateInfo.java | 2 +- .../subsystem/api/storage/VolumeInfo.java | 2 +- .../com/cloud/storage/StorageManager.java | 4 ++ .../storage/image/TemplateServiceImpl.java | 5 +- .../storage/image/store/TemplateObject.java | 8 +++ .../storage/volume/VolumeObject.java | 8 +++ .../framework/config/ConfigDepot.java | 1 + .../config/impl/ConfigDepotImpl.java | 7 +++ .../config/impl/ConfigDepotImplTest.java | 17 +++++++ .../wrapper/LibvirtCheckUrlCommand.java | 5 +- .../kvm/storage/KVMStorageProcessor.java | 2 +- .../com/cloud/storage/StorageManagerImpl.java | 29 ++++++++++- .../cloud/storage/VolumeApiServiceImpl.java | 9 ++-- .../template/HypervisorTemplateAdapter.java | 23 ++++++--- .../download/DirectDownloadManagerImpl.java | 17 ++++--- .../cloud/storage/StorageManagerImplTest.java | 41 ++++++++++++++++ .../storage/template/DownloadManager.java | 16 +++--- .../storage/template/DownloadManagerImpl.java | 25 +++++++--- .../main/java/com/cloud/utils/UriUtils.java | 3 +- .../com/cloud/utils/storage/QCOW2Utils.java | 21 +++++--- 45 files changed, 447 insertions(+), 119 deletions(-) create mode 100644 core/src/main/java/org/apache/cloudstack/storage/to/DownloadableObjectTO.java create mode 100644 engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DownloadableDataInfo.java diff --git a/core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java b/core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java index 2e331cab227..25f177eb5ff 100755 --- a/core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java +++ b/core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java @@ -26,6 +26,7 @@ import java.io.RandomAccessFile; import java.net.URI; import java.net.URISyntaxException; import java.util.Date; +import java.util.List; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.utils.imagestore.ImageStoreUtil; @@ -81,6 +82,7 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te private long maxTemplateSizeInBytes; private ResourceType resourceType = ResourceType.TEMPLATE; private final HttpMethodRetryHandler myretryhandler; + private boolean followRedirects = false; public HttpTemplateDownloader(StorageLayer storageLayer, String downloadUrl, String toDir, DownloadCompleteCallback callback, long maxTemplateSizeInBytes, String user, String password, Proxy proxy, ResourceType resourceType) { @@ -112,7 +114,7 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te private GetMethod createRequest(String downloadUrl) { GetMethod request = new GetMethod(downloadUrl); request.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, myretryhandler); - request.setFollowRedirects(true); + request.setFollowRedirects(followRedirects); return request; } @@ -336,6 +338,12 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te } else if ((responseCode = client.executeMethod(request)) != HttpStatus.SC_OK) { status = Status.UNRECOVERABLE_ERROR; errorString = " HTTP Server returned " + responseCode + " (expected 200 OK) "; + if (List.of(HttpStatus.SC_MOVED_PERMANENTLY, HttpStatus.SC_MOVED_TEMPORARILY).contains(responseCode) + && !followRedirects) { + errorString = String.format("Failed to download %s due to redirection, response code: %d", + downloadUrl, responseCode); + s_logger.error(errorString); + } return true; //FIXME: retry? } return false; @@ -537,4 +545,12 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te return this; } } + + @Override + public void setFollowRedirects(boolean followRedirects) { + this.followRedirects = followRedirects; + if (this.request != null) { + this.request.setFollowRedirects(followRedirects); + } + } } diff --git a/core/src/main/java/com/cloud/storage/template/MetalinkTemplateDownloader.java b/core/src/main/java/com/cloud/storage/template/MetalinkTemplateDownloader.java index dd452f2e2d9..a118a9ac40f 100644 --- a/core/src/main/java/com/cloud/storage/template/MetalinkTemplateDownloader.java +++ b/core/src/main/java/com/cloud/storage/template/MetalinkTemplateDownloader.java @@ -60,7 +60,7 @@ public class MetalinkTemplateDownloader extends TemplateDownloaderBase implement protected GetMethod createRequest(String downloadUrl) { GetMethod request = new GetMethod(downloadUrl); request.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, myretryhandler); - request.setFollowRedirects(true); + request.setFollowRedirects(followRedirects); if (!toFileSet) { String[] parts = downloadUrl.split("/"); String filename = parts[parts.length - 1]; @@ -173,4 +173,12 @@ public class MetalinkTemplateDownloader extends TemplateDownloaderBase implement public void setStatus(Status status) { this.status = status; } + + @Override + public void setFollowRedirects(boolean followRedirects) { + super.setFollowRedirects(followRedirects); + if (this.request != null) { + this.request.setFollowRedirects(followRedirects); + } + } } diff --git a/core/src/main/java/com/cloud/storage/template/S3TemplateDownloader.java b/core/src/main/java/com/cloud/storage/template/S3TemplateDownloader.java index 44565c4416c..a259e79fa42 100644 --- a/core/src/main/java/com/cloud/storage/template/S3TemplateDownloader.java +++ b/core/src/main/java/com/cloud/storage/template/S3TemplateDownloader.java @@ -34,6 +34,7 @@ import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.storage.command.DownloadCommand.ResourceType; import org.apache.commons.httpclient.Header; import org.apache.commons.httpclient.HttpClient; +import org.apache.commons.httpclient.HttpStatus; import org.apache.commons.httpclient.URIException; import org.apache.commons.httpclient.methods.GetMethod; import org.apache.commons.httpclient.params.HttpMethodParams; @@ -44,6 +45,7 @@ import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; import java.util.Date; +import java.util.List; import static com.cloud.utils.NumbersUtil.toHumanReadableSize; import static java.util.Arrays.asList; @@ -72,8 +74,8 @@ public class S3TemplateDownloader extends ManagedContextRunnable implements Temp private long downloadTime; private long totalBytes; private long maxTemplateSizeInByte; - private boolean resume = false; + private boolean followRedirects = false; public S3TemplateDownloader(S3TO s3TO, String downloadUrl, String installPath, DownloadCompleteCallback downloadCompleteCallback, long maxTemplateSizeInBytes, String username, String password, Proxy proxy, ResourceType resourceType) { @@ -91,7 +93,7 @@ public class S3TemplateDownloader extends ManagedContextRunnable implements Temp this.getMethod.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, HTTPUtils.getHttpMethodRetryHandler(5)); // Follow redirects - this.getMethod.setFollowRedirects(true); + this.getMethod.setFollowRedirects(followRedirects); // Set file extension. this.fileExtension = StringUtils.substringAfterLast(StringUtils.substringAfterLast(downloadUrl, "/"), "."); @@ -124,10 +126,11 @@ public class S3TemplateDownloader extends ManagedContextRunnable implements Temp return 0; } - if (!HTTPUtils.verifyResponseCode(responseCode)) { + boolean failedDueToRedirection = List.of(HttpStatus.SC_MOVED_PERMANENTLY, + HttpStatus.SC_MOVED_TEMPORARILY).contains(responseCode) && !followRedirects; + if (!HTTPUtils.verifyResponseCode(responseCode) || failedDueToRedirection) { errorString = "Response code for GetMethod of " + downloadUrl + " is incorrect, responseCode: " + responseCode; LOGGER.warn(errorString); - status = Status.UNRECOVERABLE_ERROR; return 0; } @@ -373,4 +376,12 @@ public class S3TemplateDownloader extends ManagedContextRunnable implements Temp public String getFileExtension() { return fileExtension; } + + @Override + public void setFollowRedirects(boolean followRedirects) { + this.followRedirects = followRedirects; + if (this.getMethod != null) { + this.getMethod.setFollowRedirects(followRedirects); + } + } } diff --git a/core/src/main/java/com/cloud/storage/template/TemplateDownloader.java b/core/src/main/java/com/cloud/storage/template/TemplateDownloader.java index 5db3d2425a5..9fb1ca42442 100644 --- a/core/src/main/java/com/cloud/storage/template/TemplateDownloader.java +++ b/core/src/main/java/com/cloud/storage/template/TemplateDownloader.java @@ -92,4 +92,6 @@ public interface TemplateDownloader extends Runnable { boolean isInited(); long getMaxTemplateSizeInBytes(); + + void setFollowRedirects(boolean followRedirects); } diff --git a/core/src/main/java/com/cloud/storage/template/TemplateDownloaderBase.java b/core/src/main/java/com/cloud/storage/template/TemplateDownloaderBase.java index f56e4911ca7..66058bbf82e 100644 --- a/core/src/main/java/com/cloud/storage/template/TemplateDownloaderBase.java +++ b/core/src/main/java/com/cloud/storage/template/TemplateDownloaderBase.java @@ -43,6 +43,7 @@ public abstract class TemplateDownloaderBase extends ManagedContextRunnable impl protected long _start; protected StorageLayer _storage; protected boolean _inited = false; + protected boolean followRedirects = false; private long maxTemplateSizeInBytes; public TemplateDownloaderBase(StorageLayer storage, String downloadUrl, String toDir, long maxTemplateSizeInBytes, DownloadCompleteCallback callback) { @@ -149,4 +150,9 @@ public abstract class TemplateDownloaderBase extends ManagedContextRunnable impl public boolean isInited() { return _inited; } + + @Override + public void setFollowRedirects(boolean followRedirects) { + this.followRedirects = followRedirects; + } } diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/CheckUrlCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/CheckUrlCommand.java index b1b76da8211..fe941b32ee3 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/CheckUrlCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/CheckUrlCommand.java @@ -28,6 +28,7 @@ public class CheckUrlCommand extends Command { private Integer connectTimeout; private Integer connectionRequestTimeout; private Integer socketTimeout; + private boolean followRedirects; public String getFormat() { return format; @@ -43,13 +44,19 @@ public class CheckUrlCommand extends Command { public Integer getSocketTimeout() { return socketTimeout; } - public CheckUrlCommand(final String format,final String url) { + public boolean isFollowRedirects() { + return followRedirects; + } + + public CheckUrlCommand(final String format, final String url, final boolean followRedirects) { super(); this.format = format; this.url = url; + this.followRedirects = followRedirects; } - public CheckUrlCommand(final String format,final String url, Integer connectTimeout, Integer connectionRequestTimeout, Integer socketTimeout) { + public CheckUrlCommand(final String format,final String url, Integer connectTimeout, + Integer connectionRequestTimeout, Integer socketTimeout, final boolean followRedirects) { super(); this.format = format; this.url = url; diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java index 7e1ff0b34c4..b6748dca434 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java @@ -45,7 +45,11 @@ public abstract class DirectDownloadCommand extends StorageSubSystemCommand { private Long templateSize; private Storage.ImageFormat format; - protected DirectDownloadCommand (final String url, final Long templateId, final PrimaryDataStoreTO destPool, final String checksum, final Map headers, final Integer connectTimeout, final Integer soTimeout, final Integer connectionRequestTimeout) { + private boolean followRedirects; + + protected DirectDownloadCommand (final String url, final Long templateId, final PrimaryDataStoreTO destPool, + final String checksum, final Map headers, final Integer connectTimeout, + final Integer soTimeout, final Integer connectionRequestTimeout, final boolean followRedirects) { this.url = url; this.templateId = templateId; this.destData = destData; @@ -55,6 +59,7 @@ public abstract class DirectDownloadCommand extends StorageSubSystemCommand { this.connectTimeout = connectTimeout; this.soTimeout = soTimeout; this.connectionRequestTimeout = connectionRequestTimeout; + this.followRedirects = followRedirects; } public String getUrl() { @@ -137,4 +142,12 @@ public abstract class DirectDownloadCommand extends StorageSubSystemCommand { public int getWaitInMillSeconds() { return getWait() * 1000; } + + public boolean isFollowRedirects() { + return followRedirects; + } + + public void setFollowRedirects(boolean followRedirects) { + this.followRedirects = followRedirects; + } } diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java index f131b3b0a7f..bdc00b3bc47 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java @@ -24,8 +24,10 @@ import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; public class HttpDirectDownloadCommand extends DirectDownloadCommand { - public HttpDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map headers, int connectTimeout, int soTimeout) { - super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, null); + public HttpDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, + Map headers, int connectTimeout, int soTimeout, boolean followRedirects) { + super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, + null, followRedirects); } } diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java index dd88ad2a155..a7e16eac91a 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java @@ -25,7 +25,10 @@ import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; public class HttpsDirectDownloadCommand extends DirectDownloadCommand { - public HttpsDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map headers, int connectTimeout, int soTimeout, int connectionRequestTimeout) { - super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, connectionRequestTimeout); + public HttpsDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, + Map headers, int connectTimeout, int soTimeout, int connectionRequestTimeout, + boolean followRedirects) { + super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, + connectionRequestTimeout, followRedirects); } } diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java index a3edcebe7de..7742994c76b 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java @@ -24,8 +24,9 @@ import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; public class MetalinkDirectDownloadCommand extends DirectDownloadCommand { - public MetalinkDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map headers, int connectTimeout, int soTimeout) { - super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, null); + public MetalinkDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, + Map headers, int connectTimeout, int soTimeout, boolean followRedirects) { + super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, null, followRedirects); } } diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/NfsDirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/NfsDirectDownloadCommand.java index 0bf9c4d934b..0e51d786230 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/NfsDirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/NfsDirectDownloadCommand.java @@ -24,8 +24,9 @@ import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; public class NfsDirectDownloadCommand extends DirectDownloadCommand { - public NfsDirectDownloadCommand(final String url, final Long templateId, final PrimaryDataStoreTO destPool, final String checksum, final Map headers) { - super(url, templateId, destPool, checksum, headers, null, null, null); + public NfsDirectDownloadCommand(final String url, final Long templateId, final PrimaryDataStoreTO destPool, + final String checksum, final Map headers) { + super(url, templateId, destPool, checksum, headers, null, null, null, false); } } diff --git a/core/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadHelper.java b/core/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadHelper.java index 27e35b7074b..12fd5ad7162 100644 --- a/core/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadHelper.java +++ b/core/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadHelper.java @@ -34,27 +34,30 @@ public class DirectDownloadHelper { * Get direct template downloader from direct download command and destination pool */ public static DirectTemplateDownloader getDirectTemplateDownloaderFromCommand(DirectDownloadCommand cmd, - String destPoolLocalPath, - String temporaryDownloadPath) { + String destPoolLocalPath, String temporaryDownloadPath) { if (cmd instanceof HttpDirectDownloadCommand) { - return new HttpDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPoolLocalPath, cmd.getChecksum(), cmd.getHeaders(), - cmd.getConnectTimeout(), cmd.getSoTimeout(), temporaryDownloadPath); + return new HttpDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPoolLocalPath, + cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout(), + temporaryDownloadPath, cmd.isFollowRedirects()); } else if (cmd instanceof HttpsDirectDownloadCommand) { - return new HttpsDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPoolLocalPath, cmd.getChecksum(), cmd.getHeaders(), - cmd.getConnectTimeout(), cmd.getSoTimeout(), cmd.getConnectionRequestTimeout(), temporaryDownloadPath); + return new HttpsDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPoolLocalPath, + cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout(), + cmd.getConnectionRequestTimeout(), temporaryDownloadPath, cmd.isFollowRedirects()); } else if (cmd instanceof NfsDirectDownloadCommand) { - return new NfsDirectTemplateDownloader(cmd.getUrl(), destPoolLocalPath, cmd.getTemplateId(), cmd.getChecksum(), temporaryDownloadPath); + return new NfsDirectTemplateDownloader(cmd.getUrl(), destPoolLocalPath, cmd.getTemplateId(), + cmd.getChecksum(), temporaryDownloadPath); } else if (cmd instanceof MetalinkDirectDownloadCommand) { - return new MetalinkDirectTemplateDownloader(cmd.getUrl(), destPoolLocalPath, cmd.getTemplateId(), cmd.getChecksum(), cmd.getHeaders(), - cmd.getConnectTimeout(), cmd.getSoTimeout(), temporaryDownloadPath); + return new MetalinkDirectTemplateDownloader(cmd.getUrl(), destPoolLocalPath, cmd.getTemplateId(), + cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout(), + temporaryDownloadPath, cmd.isFollowRedirects()); } else { throw new IllegalArgumentException("Unsupported protocol, please provide HTTP(S), NFS or a metalink"); } } - public static boolean checkUrlExistence(String url) { + public static boolean checkUrlExistence(String url, boolean followRedirects) { try { - DirectTemplateDownloader checker = getCheckerDownloader(url, null, null, null); + DirectTemplateDownloader checker = getCheckerDownloader(url, null, null, null, followRedirects); return checker.checkUrl(url); } catch (CloudRuntimeException e) { LOGGER.error(String.format("Cannot check URL %s is reachable due to: %s", url, e.getMessage()), e); @@ -62,9 +65,11 @@ public class DirectDownloadHelper { } } - public static boolean checkUrlExistence(String url, Integer connectTimeout, Integer connectionRequestTimeout, Integer socketTimeout) { + public static boolean checkUrlExistence(String url, Integer connectTimeout, Integer connectionRequestTimeout, + Integer socketTimeout, boolean followRedirects) { try { - DirectTemplateDownloader checker = getCheckerDownloader(url, connectTimeout, connectionRequestTimeout, socketTimeout); + DirectTemplateDownloader checker = getCheckerDownloader(url, connectTimeout, connectionRequestTimeout, + socketTimeout, followRedirects); return checker.checkUrl(url); } catch (CloudRuntimeException e) { LOGGER.error(String.format("Cannot check URL %s is reachable due to: %s", url, e.getMessage()), e); @@ -72,27 +77,29 @@ public class DirectDownloadHelper { } } - private static DirectTemplateDownloader getCheckerDownloader(String url, Integer connectTimeout, Integer connectionRequestTimeout, Integer socketTimeout) { + private static DirectTemplateDownloader getCheckerDownloader(String url, Integer connectTimeout, + Integer connectionRequestTimeout, Integer socketTimeout, boolean followRedirects) { if (url.toLowerCase().startsWith("https:")) { - return new HttpsDirectTemplateDownloader(url, connectTimeout, connectionRequestTimeout, socketTimeout); + return new HttpsDirectTemplateDownloader(url, connectTimeout, connectionRequestTimeout, socketTimeout, followRedirects); } else if (url.toLowerCase().startsWith("http:")) { - return new HttpDirectTemplateDownloader(url, connectTimeout, socketTimeout); + return new HttpDirectTemplateDownloader(url, connectTimeout, socketTimeout, followRedirects); } else if (url.toLowerCase().startsWith("nfs:")) { return new NfsDirectTemplateDownloader(url); } else if (url.toLowerCase().endsWith(".metalink")) { - return new MetalinkDirectTemplateDownloader(url, connectTimeout, socketTimeout); + return new MetalinkDirectTemplateDownloader(url, connectTimeout, socketTimeout, followRedirects); } else { throw new CloudRuntimeException(String.format("Cannot find a download checker for url: %s", url)); } } - public static Long getFileSize(String url, String format) { - DirectTemplateDownloader checker = getCheckerDownloader(url, null, null, null); + public static Long getFileSize(String url, String format, boolean followRedirects) { + DirectTemplateDownloader checker = getCheckerDownloader(url, null, null, null, followRedirects); return checker.getRemoteFileSize(url, format); } - public static Long getFileSize(String url, String format, Integer connectTimeout, Integer connectionRequestTimeout, Integer socketTimeout) { - DirectTemplateDownloader checker = getCheckerDownloader(url, connectTimeout, connectionRequestTimeout, socketTimeout); + public static Long getFileSize(String url, String format, Integer connectTimeout, Integer connectionRequestTimeout, + Integer socketTimeout, boolean followRedirects) { + DirectTemplateDownloader checker = getCheckerDownloader(url, connectTimeout, connectionRequestTimeout, socketTimeout, followRedirects); return checker.getRemoteFileSize(url, format); } } diff --git a/core/src/main/java/org/apache/cloudstack/direct/download/DirectTemplateDownloaderImpl.java b/core/src/main/java/org/apache/cloudstack/direct/download/DirectTemplateDownloaderImpl.java index 9476dbaa5ce..9431b8209b3 100644 --- a/core/src/main/java/org/apache/cloudstack/direct/download/DirectTemplateDownloaderImpl.java +++ b/core/src/main/java/org/apache/cloudstack/direct/download/DirectTemplateDownloaderImpl.java @@ -42,16 +42,19 @@ public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDown private String checksum; private boolean redownload = false; protected String temporaryDownloadPath; + private boolean followRedirects; public static final Logger s_logger = Logger.getLogger(DirectTemplateDownloaderImpl.class.getName()); protected DirectTemplateDownloaderImpl(final String url, final String destPoolPath, final Long templateId, - final String checksum, final String temporaryDownloadPath) { + final String checksum, final String temporaryDownloadPath, + final boolean followRedirects) { this.url = url; this.destPoolPath = destPoolPath; this.templateId = templateId; this.checksum = checksum; this.temporaryDownloadPath = temporaryDownloadPath; + this.followRedirects = followRedirects; } private static String directDownloadDir = "template"; @@ -111,6 +114,14 @@ public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDown return redownload; } + public boolean isFollowRedirects() { + return followRedirects; + } + + public void setFollowRedirects(boolean followRedirects) { + this.followRedirects = followRedirects; + } + /** * Create download directory (if it does not exist) */ diff --git a/core/src/main/java/org/apache/cloudstack/direct/download/HttpDirectTemplateDownloader.java b/core/src/main/java/org/apache/cloudstack/direct/download/HttpDirectTemplateDownloader.java index e1b2f1fe429..a43ce9ba8e3 100644 --- a/core/src/main/java/org/apache/cloudstack/direct/download/HttpDirectTemplateDownloader.java +++ b/core/src/main/java/org/apache/cloudstack/direct/download/HttpDirectTemplateDownloader.java @@ -50,13 +50,15 @@ public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl { protected GetMethod request; protected Map reqHeaders = new HashMap<>(); - protected HttpDirectTemplateDownloader(String url, Integer connectTimeout, Integer socketTimeout) { - this(url, null, null, null, null, connectTimeout, socketTimeout, null); + protected HttpDirectTemplateDownloader(String url, Integer connectTimeout, Integer socketTimeout, + boolean followRedirects) { + this(url, null, null, null, null, connectTimeout, socketTimeout, null, followRedirects); } public HttpDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, - Map headers, Integer connectTimeout, Integer soTimeout, String downloadPath) { - super(url, destPoolPath, templateId, checksum, downloadPath); + Map headers, Integer connectTimeout, Integer soTimeout, String downloadPath, + boolean followRedirects) { + super(url, destPoolPath, templateId, checksum, downloadPath, followRedirects); s_httpClientManager.getParams().setConnectionTimeout(connectTimeout == null ? 5000 : connectTimeout); s_httpClientManager.getParams().setSoTimeout(soTimeout == null ? 5000 : soTimeout); client = new HttpClient(s_httpClientManager); @@ -68,7 +70,7 @@ public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl { protected GetMethod createRequest(String downloadUrl, Map headers) { GetMethod request = new GetMethod(downloadUrl); - request.setFollowRedirects(true); + request.setFollowRedirects(this.isFollowRedirects()); if (MapUtils.isNotEmpty(headers)) { for (String key : headers.keySet()) { request.setRequestHeader(key, headers.get(key)); @@ -111,9 +113,11 @@ public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl { @Override public boolean checkUrl(String url) { HeadMethod httpHead = new HeadMethod(url); + httpHead.setFollowRedirects(this.isFollowRedirects()); try { - if (client.executeMethod(httpHead) != HttpStatus.SC_OK) { - s_logger.error(String.format("Invalid URL: %s", url)); + int responseCode = client.executeMethod(httpHead); + if (responseCode != HttpStatus.SC_OK) { + s_logger.error(String.format("HTTP HEAD request to URL: %s failed, response code: %d", url, responseCode)); return false; } return true; @@ -128,9 +132,9 @@ public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl { @Override public Long getRemoteFileSize(String url, String format) { if ("qcow2".equalsIgnoreCase(format)) { - return QCOW2Utils.getVirtualSize(url); + return QCOW2Utils.getVirtualSizeFromUrl(url, this.isFollowRedirects()); } else { - return UriUtils.getRemoteSize(url); + return UriUtils.getRemoteSize(url, this.isFollowRedirects()); } } diff --git a/core/src/main/java/org/apache/cloudstack/direct/download/HttpsDirectTemplateDownloader.java b/core/src/main/java/org/apache/cloudstack/direct/download/HttpsDirectTemplateDownloader.java index 70a3eb29bc7..524c87b988c 100644 --- a/core/src/main/java/org/apache/cloudstack/direct/download/HttpsDirectTemplateDownloader.java +++ b/core/src/main/java/org/apache/cloudstack/direct/download/HttpsDirectTemplateDownloader.java @@ -68,20 +68,28 @@ public class HttpsDirectTemplateDownloader extends DirectTemplateDownloaderImpl protected CloseableHttpClient httpsClient; private HttpUriRequest req; - protected HttpsDirectTemplateDownloader(String url, Integer connectTimeout, Integer connectionRequestTimeout, Integer socketTimeout) { - this(url, null, null, null, null, connectTimeout, socketTimeout, connectionRequestTimeout, null); + protected HttpsDirectTemplateDownloader(String url, Integer connectTimeout, Integer connectionRequestTimeout, + Integer socketTimeout, boolean followRedirects) { + this(url, null, null, null, null, connectTimeout, socketTimeout, + connectionRequestTimeout, null, followRedirects); } - public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, - Integer connectTimeout, Integer soTimeout, Integer connectionRequestTimeout, String temporaryDownloadPath) { - super(url, destPoolPath, templateId, checksum, temporaryDownloadPath); + public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, + Map headers, Integer connectTimeout, Integer soTimeout, + Integer connectionRequestTimeout, String temporaryDownloadPath, boolean followRedirects) { + super(url, destPoolPath, templateId, checksum, temporaryDownloadPath, followRedirects); SSLContext sslcontext = getSSLContext(); SSLConnectionSocketFactory factory = new SSLConnectionSocketFactory(sslcontext, SSLConnectionSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER); RequestConfig config = RequestConfig.custom() .setConnectTimeout(connectTimeout == null ? 5000 : connectTimeout) .setConnectionRequestTimeout(connectionRequestTimeout == null ? 5000 : connectionRequestTimeout) - .setSocketTimeout(soTimeout == null ? 5000 : soTimeout).build(); - httpsClient = HttpClients.custom().setSSLSocketFactory(factory).setDefaultRequestConfig(config).build(); + .setSocketTimeout(soTimeout == null ? 5000 : soTimeout) + .setRedirectsEnabled(followRedirects) + .build(); + httpsClient = HttpClients.custom() + .setSSLSocketFactory(factory) + .setDefaultRequestConfig(config) + .build(); createUriRequest(url, headers); String downloadDir = getDirectDownloadTempPath(templateId); File tempFile = createTemporaryDirectoryAndFile(downloadDir); @@ -90,6 +98,7 @@ public class HttpsDirectTemplateDownloader extends DirectTemplateDownloaderImpl protected void createUriRequest(String downloadUrl, Map headers) { req = new HttpGet(downloadUrl); + setFollowRedirects(this.isFollowRedirects()); if (MapUtils.isNotEmpty(headers)) { for (String headerKey: headers.keySet()) { req.setHeader(headerKey, headers.get(headerKey)); @@ -164,8 +173,9 @@ public class HttpsDirectTemplateDownloader extends DirectTemplateDownloaderImpl HttpHead httpHead = new HttpHead(url); try { CloseableHttpResponse response = httpsClient.execute(httpHead); - if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) { - s_logger.error(String.format("Invalid URL: %s", url)); + int responseCode = response.getStatusLine().getStatusCode(); + if (responseCode != HttpStatus.SC_OK) { + s_logger.error(String.format("HTTP HEAD request to URL: %s failed, response code: %d", url, responseCode)); return false; } return true; diff --git a/core/src/main/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloader.java b/core/src/main/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloader.java index 06578d8c2b2..f6f7f7e48c2 100644 --- a/core/src/main/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloader.java +++ b/core/src/main/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloader.java @@ -42,16 +42,15 @@ public class MetalinkDirectTemplateDownloader extends DirectTemplateDownloaderIm private static final Logger s_logger = Logger.getLogger(MetalinkDirectTemplateDownloader.class.getName()); protected DirectTemplateDownloader createDownloaderForMetalinks(String url, Long templateId, - String destPoolPath, String checksum, - Map headers, - Integer connectTimeout, Integer soTimeout, - Integer connectionRequestTimeout, String temporaryDownloadPath) { + String destPoolPath, String checksum, Map headers, Integer connectTimeout, + Integer soTimeout, Integer connectionRequestTimeout, String temporaryDownloadPath) { if (url.toLowerCase().startsWith("https:")) { return new HttpsDirectTemplateDownloader(url, templateId, destPoolPath, checksum, headers, - connectTimeout, soTimeout, connectionRequestTimeout, temporaryDownloadPath); + connectTimeout, soTimeout, connectionRequestTimeout, temporaryDownloadPath, + this.isFollowRedirects()); } else if (url.toLowerCase().startsWith("http:")) { return new HttpDirectTemplateDownloader(url, templateId, destPoolPath, checksum, headers, - connectTimeout, soTimeout, temporaryDownloadPath); + connectTimeout, soTimeout, temporaryDownloadPath, this.isFollowRedirects()); } else if (url.toLowerCase().startsWith("nfs:")) { return new NfsDirectTemplateDownloader(url); } else { @@ -60,13 +59,15 @@ public class MetalinkDirectTemplateDownloader extends DirectTemplateDownloaderIm } } - protected MetalinkDirectTemplateDownloader(String url, Integer connectTimeout, Integer socketTimeout) { - this(url, null, null, null, null, connectTimeout, socketTimeout, null); + protected MetalinkDirectTemplateDownloader(String url, Integer connectTimeout, Integer socketTimeout, + boolean followRedirects) { + this(url, null, null, null, null, connectTimeout, socketTimeout, null, followRedirects); } public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long templateId, String checksum, - Map headers, Integer connectTimeout, Integer soTimeout, String downloadPath) { - super(url, destPoolPath, templateId, checksum, downloadPath); + Map headers, Integer connectTimeout, Integer soTimeout, String downloadPath, + boolean followRedirects) { + super(url, destPoolPath, templateId, checksum, downloadPath, followRedirects); this.headers = headers; this.connectTimeout = connectTimeout; this.soTimeout = soTimeout; diff --git a/core/src/main/java/org/apache/cloudstack/direct/download/NfsDirectTemplateDownloader.java b/core/src/main/java/org/apache/cloudstack/direct/download/NfsDirectTemplateDownloader.java index d606136e297..e5ff533cc97 100644 --- a/core/src/main/java/org/apache/cloudstack/direct/download/NfsDirectTemplateDownloader.java +++ b/core/src/main/java/org/apache/cloudstack/direct/download/NfsDirectTemplateDownloader.java @@ -57,8 +57,9 @@ public class NfsDirectTemplateDownloader extends DirectTemplateDownloaderImpl { this(url, null, null, null, null); } - public NfsDirectTemplateDownloader(String url, String destPool, Long templateId, String checksum, String downloadPath) { - super(url, destPool, templateId, checksum, downloadPath); + public NfsDirectTemplateDownloader(String url, String destPool, Long templateId, String checksum, + String downloadPath) { + super(url, destPool, templateId, checksum, downloadPath, false); parseUrl(); } diff --git a/core/src/main/java/org/apache/cloudstack/storage/command/DownloadCommand.java b/core/src/main/java/org/apache/cloudstack/storage/command/DownloadCommand.java index 29d737fcce9..7b41bd949e5 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/command/DownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/storage/command/DownloadCommand.java @@ -48,6 +48,8 @@ public class DownloadCommand extends AbstractDownloadCommand implements Internal private DataStoreTO _store; private DataStoreTO cacheStore; + private boolean followRedirects = false; + protected DownloadCommand() { } @@ -64,6 +66,7 @@ public class DownloadCommand extends AbstractDownloadCommand implements Internal installPath = that.installPath; _store = that._store; _proxy = that._proxy; + followRedirects = that.followRedirects; } public DownloadCommand(TemplateObjectTO template, Long maxDownloadSizeInBytes) { @@ -79,6 +82,7 @@ public class DownloadCommand extends AbstractDownloadCommand implements Internal setSecUrl(((NfsTO)_store).getUrl()); } this.maxDownloadSizeInBytes = maxDownloadSizeInBytes; + this.followRedirects = template.isFollowRedirects(); } public DownloadCommand(TemplateObjectTO template, String user, String passwd, Long maxDownloadSizeInBytes) { @@ -94,6 +98,7 @@ public class DownloadCommand extends AbstractDownloadCommand implements Internal _store = volume.getDataStore(); this.maxDownloadSizeInBytes = maxDownloadSizeInBytes; resourceType = ResourceType.VOLUME; + this.followRedirects = volume.isFollowRedirects(); } @Override @@ -181,4 +186,12 @@ public class DownloadCommand extends AbstractDownloadCommand implements Internal public DataStoreTO getCacheStore() { return cacheStore; } + + public boolean isFollowRedirects() { + return followRedirects; + } + + public void setFollowRedirects(boolean followRedirects) { + this.followRedirects = followRedirects; + } } diff --git a/core/src/main/java/org/apache/cloudstack/storage/to/DownloadableObjectTO.java b/core/src/main/java/org/apache/cloudstack/storage/to/DownloadableObjectTO.java new file mode 100644 index 00000000000..70db8fafffc --- /dev/null +++ b/core/src/main/java/org/apache/cloudstack/storage/to/DownloadableObjectTO.java @@ -0,0 +1,30 @@ +// 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. + +package org.apache.cloudstack.storage.to; + +public class DownloadableObjectTO { + protected boolean followRedirects = false; + + public boolean isFollowRedirects() { + return followRedirects; + } + + public void setFollowRedirects(boolean followRedirects) { + this.followRedirects = followRedirects; + } +} diff --git a/core/src/main/java/org/apache/cloudstack/storage/to/SnapshotObjectTO.java b/core/src/main/java/org/apache/cloudstack/storage/to/SnapshotObjectTO.java index c62110b179e..72e6492aa52 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/to/SnapshotObjectTO.java +++ b/core/src/main/java/org/apache/cloudstack/storage/to/SnapshotObjectTO.java @@ -30,7 +30,7 @@ import com.cloud.agent.api.to.DataStoreTO; import com.cloud.agent.api.to.DataTO; import com.cloud.hypervisor.Hypervisor.HypervisorType; -public class SnapshotObjectTO implements DataTO { +public class SnapshotObjectTO extends DownloadableObjectTO implements DataTO { private String path; private VolumeObjectTO volume; private String parentSnapshotPath; diff --git a/core/src/main/java/org/apache/cloudstack/storage/to/TemplateObjectTO.java b/core/src/main/java/org/apache/cloudstack/storage/to/TemplateObjectTO.java index a405785bf64..eafe8f83269 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/to/TemplateObjectTO.java +++ b/core/src/main/java/org/apache/cloudstack/storage/to/TemplateObjectTO.java @@ -28,7 +28,7 @@ import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.Storage.ImageFormat; import com.cloud.template.VirtualMachineTemplate; -public class TemplateObjectTO implements DataTO { +public class TemplateObjectTO extends DownloadableObjectTO implements DataTO { private String path; private String origUrl; private String uuid; @@ -87,6 +87,7 @@ public class TemplateObjectTO implements DataTO { this.deployAsIs = template.isDeployAsIs(); this.deployAsIsConfiguration = template.getDeployAsIsConfiguration(); this.directDownload = template.isDirectDownload(); + this.followRedirects = template.isFollowRedirects(); } @Override diff --git a/core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java b/core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java index 8473ea7a49e..2bb67c80ce4 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java +++ b/core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java @@ -33,7 +33,7 @@ import com.cloud.storage.Volume; import java.util.Arrays; -public class VolumeObjectTO implements DataTO { +public class VolumeObjectTO extends DownloadableObjectTO implements DataTO { private String uuid; private Volume.Type volumeType; private DataStoreTO dataStore; @@ -119,6 +119,7 @@ public class VolumeObjectTO implements DataTO { this.vSphereStoragePolicyId = volume.getvSphereStoragePolicyId(); this.passphrase = volume.getPassphrase(); this.encryptFormat = volume.getEncryptFormat(); + this.followRedirects = volume.isFollowRedirects(); } public String getUuid() { diff --git a/core/src/test/java/org/apache/cloudstack/direct/download/BaseDirectTemplateDownloaderTest.java b/core/src/test/java/org/apache/cloudstack/direct/download/BaseDirectTemplateDownloaderTest.java index 2c7245662a2..b74dc9460d6 100644 --- a/core/src/test/java/org/apache/cloudstack/direct/download/BaseDirectTemplateDownloaderTest.java +++ b/core/src/test/java/org/apache/cloudstack/direct/download/BaseDirectTemplateDownloaderTest.java @@ -56,7 +56,7 @@ public class BaseDirectTemplateDownloaderTest { private HttpEntity httpEntity; @InjectMocks - protected HttpsDirectTemplateDownloader httpsDownloader = new HttpsDirectTemplateDownloader(httpUrl, 1000, 1000, 1000); + protected HttpsDirectTemplateDownloader httpsDownloader = new HttpsDirectTemplateDownloader(httpUrl, 1000, 1000, 1000, false); @Before public void init() throws IOException { diff --git a/core/src/test/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloaderTest.java b/core/src/test/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloaderTest.java index 68982fb915f..81e504ec512 100644 --- a/core/src/test/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloaderTest.java +++ b/core/src/test/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloaderTest.java @@ -25,7 +25,7 @@ import org.mockito.InjectMocks; public class MetalinkDirectTemplateDownloaderTest extends BaseDirectTemplateDownloaderTest { @InjectMocks - protected MetalinkDirectTemplateDownloader metalinkDownloader = new MetalinkDirectTemplateDownloader(httpsUrl, 1000, 1000); + protected MetalinkDirectTemplateDownloader metalinkDownloader = new MetalinkDirectTemplateDownloader(httpsUrl, 1000, 1000, false); @Test public void testCheckUrlMetalink() { diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DownloadableDataInfo.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DownloadableDataInfo.java new file mode 100644 index 00000000000..63b0867d1ab --- /dev/null +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DownloadableDataInfo.java @@ -0,0 +1,24 @@ +// 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. + +package org.apache.cloudstack.engine.subsystem.api.storage; + +public interface DownloadableDataInfo extends DataObject { + default public boolean isFollowRedirects() { + return true; + } +} diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateInfo.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateInfo.java index 1d49e19d2da..3bd3100e84e 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateInfo.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateInfo.java @@ -21,7 +21,7 @@ package org.apache.cloudstack.engine.subsystem.api.storage; import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.UserData; -public interface TemplateInfo extends DataObject, VirtualMachineTemplate { +public interface TemplateInfo extends DownloadableDataInfo, VirtualMachineTemplate { @Override String getUniqueName(); diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java index be16c20d173..19b852bd912 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java @@ -26,7 +26,7 @@ import com.cloud.storage.Storage; import com.cloud.storage.Volume; import com.cloud.vm.VirtualMachine; -public interface VolumeInfo extends DataObject, Volume { +public interface VolumeInfo extends DownloadableDataInfo, Volume { boolean isAttachedVM(); diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index 28e7c89419a..7fdec905242 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -191,6 +191,10 @@ public interface StorageManager extends StorageService { true, ConfigKey.Scope.Global, null); + static final ConfigKey DataStoreDownloadFollowRedirects = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + Boolean.class, "store.download.follow.redirects", "false", + "Whether HTTP redirect is followed during store downloads for objects such as template, volume etc.", + true, ConfigKey.Scope.Global); /** * should we execute in sequence not involving any storages? diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java index e6235a63c77..6c4fcab2f17 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java @@ -91,6 +91,7 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.TemplateType; +import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.VMTemplateStorageResourceAssoc.Status; @@ -366,6 +367,7 @@ public class TemplateServiceImpl implements TemplateService { toBeDownloaded.addAll(allTemplates); final StateMachine2 stateMachine = VirtualMachineTemplate.State.getStateMachine(); + Boolean followRedirect = StorageManager.DataStoreDownloadFollowRedirects.value(); for (VMTemplateVO tmplt : allTemplates) { String uniqueName = tmplt.getUniqueName(); TemplateDataStoreVO tmpltStore = _vmTemplateStoreDao.findByStoreTemplate(storeId, tmplt.getId()); @@ -446,7 +448,8 @@ public class TemplateServiceImpl implements TemplateService { try { _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(accountId), com.cloud.configuration.Resource.ResourceType.secondary_storage, - tmpltInfo.getSize() - UriUtils.getRemoteSize(tmplt.getUrl())); + tmpltInfo.getSize() - UriUtils.getRemoteSize(tmplt.getUrl(), + followRedirect)); } catch (ResourceAllocationException e) { s_logger.warn(e.getMessage()); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_RESOURCE_LIMIT_EXCEEDED, zoneId, null, e.getMessage(), e.getMessage()); diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/store/TemplateObject.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/store/TemplateObject.java index b688197bfb9..3883637cd07 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/store/TemplateObject.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/store/TemplateObject.java @@ -23,6 +23,7 @@ import java.util.Map; import javax.inject.Inject; +import com.cloud.storage.StorageManager; import com.cloud.user.UserData; import org.apache.log4j.Logger; @@ -73,8 +74,10 @@ public class TemplateObject implements TemplateInfo { VMTemplatePoolDao templatePoolDao; @Inject TemplateDataStoreDao templateStoreDao; + final private boolean followRedirects; public TemplateObject() { + this.followRedirects = StorageManager.DataStoreDownloadFollowRedirects.value(); } protected void configure(VMTemplateVO template, DataStore dataStore) { @@ -573,4 +576,9 @@ public class TemplateObject implements TemplateInfo { // TODO Auto-generated method stub return null; } + + @Override + public boolean isFollowRedirects() { + return followRedirects; + } } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java index 5ebee87acd4..b0f426a54c4 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java @@ -23,6 +23,7 @@ import javax.inject.Inject; import com.cloud.configuration.Resource.ResourceType; import com.cloud.dc.VsphereStoragePolicyVO; import com.cloud.dc.dao.VsphereStoragePolicyDao; +import com.cloud.storage.StorageManager; import com.cloud.utils.db.Transaction; import com.cloud.utils.db.TransactionCallbackNoReturn; import com.cloud.utils.db.TransactionStatus; @@ -117,6 +118,7 @@ public class VolumeObject implements VolumeInfo { private MigrationOptions migrationOptions; private boolean directDownload; private String vSphereStoragePolicyId; + private boolean followRedirects; private final List volumeStatesThatShouldNotTransitWhenDataStoreRoleIsImage = Arrays.asList(Volume.State.Migrating, Volume.State.Uploaded, Volume.State.Copying, Volume.State.Expunged); @@ -127,6 +129,7 @@ public class VolumeObject implements VolumeInfo { public VolumeObject() { _volStateMachine = Volume.State.getStateMachine(); + this.followRedirects = StorageManager.DataStoreDownloadFollowRedirects.value(); } protected void configure(DataStore dataStore, VolumeVO volumeVO) { @@ -930,4 +933,9 @@ public class VolumeObject implements VolumeInfo { public void setEncryptFormat(String encryptFormat) { volumeVO.setEncryptFormat(encryptFormat); } + + @Override + public boolean isFollowRedirects() { + return followRedirects; + } } diff --git a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigDepot.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigDepot.java index 1ed37ab9969..b38b30e88b8 100644 --- a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigDepot.java +++ b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigDepot.java @@ -31,4 +31,5 @@ public interface ConfigDepot { void set(ConfigKey key, T value); void createOrUpdateConfigObject(String componentName, ConfigKey key, String value); + boolean isNewConfig(ConfigKey configKey); } diff --git a/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java index 75a3ea4d947..46a1de950dd 100644 --- a/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java +++ b/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java @@ -81,6 +81,7 @@ public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin { List _configurables; List _scopedStorages; Set _configured = Collections.synchronizedSet(new HashSet()); + Set newConfigs = Collections.synchronizedSet(new HashSet<>()); private HashMap>> _allKeys = new HashMap>>(1007); @@ -193,6 +194,7 @@ public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin { } _configDao.persist(vo); + newConfigs.add(vo.getName()); } else { boolean configUpdated = false; if (vo.isDynamic() != key.isDynamic() || !ObjectUtils.equals(vo.getDescription(), key.description()) || !ObjectUtils.equals(vo.getDefaultValue(), key.defaultValue()) || @@ -343,4 +345,9 @@ public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin { return new Pair<>(groupId, subGroupId); } + + @Override + public boolean isNewConfig(ConfigKey configKey) { + return newConfigs.contains(configKey.key()); + } } diff --git a/framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java b/framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java index fed784cfe2d..8dd6f71af3c 100644 --- a/framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java +++ b/framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java @@ -18,9 +18,14 @@ // package org.apache.cloudstack.framework.config.impl; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + import org.apache.cloudstack.framework.config.ConfigKey; import org.junit.Assert; import org.junit.Test; +import org.springframework.test.util.ReflectionTestUtils; public class ConfigDepotImplTest { @@ -40,4 +45,16 @@ public class ConfigDepotImplTest { } } + @Test + public void testIsNewConfig() { + String validNewConfigKey = "CONFIG"; + ConfigKey validNewConfig = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Boolean.class, "CONFIG", "true", "", true); + ConfigKey invalidNewConfig = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Boolean.class, "CONFIG1", "true", "", true); + Set newConfigs = Collections.synchronizedSet(new HashSet<>()); + newConfigs.add(validNewConfigKey); + ReflectionTestUtils.setField(configDepotImpl, "newConfigs", newConfigs); + Assert.assertTrue(configDepotImpl.isNewConfig(validNewConfig)); + Assert.assertFalse(configDepotImpl.isNewConfig(invalidNewConfig)); + } + } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckUrlCommand.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckUrlCommand.java index 5faad5633f3..9d003eb2096 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckUrlCommand.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckUrlCommand.java @@ -38,13 +38,14 @@ public class LibvirtCheckUrlCommand extends CommandWrapper _discoverers; public List getDiscoverers() { @@ -407,6 +414,23 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } } + protected void enableDefaultDatastoreDownloadRedirectionForExistingInstallations() { + if (!configDepot.isNewConfig(DataStoreDownloadFollowRedirects)) { + if (s_logger.isTraceEnabled()) { + s_logger.trace(String.format("%s is not a new configuration, skipping updating its value", + DataStoreDownloadFollowRedirects.key())); + } + return; + } + List zones = + _dcDao.listAll(new Filter(1)); + if (CollectionUtils.isNotEmpty(zones)) { + s_logger.debug(String.format("Updating value for configuration: %s to true", + DataStoreDownloadFollowRedirects.key())); + configurationDao.update(DataStoreDownloadFollowRedirects.key(), "true"); + } + } + @Override public List ListByDataCenterHypervisor(long datacenterId, HypervisorType type) { List pools = _storagePoolDao.listByDataCenterId(datacenterId); @@ -638,7 +662,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } _executor.scheduleWithFixedDelay(new DownloadURLGarbageCollector(), _downloadUrlCleanupInterval, _downloadUrlCleanupInterval, TimeUnit.SECONDS); - + enableDefaultDatastoreDownloadRedirectionForExistingInstallations(); return true; } @@ -3417,7 +3441,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C SecStorageVMAutoScaleDown, MountDisabledStoragePool, VmwareCreateCloneFull, - VmwareAllowParallelExecution + VmwareAllowParallelExecution, + DataStoreDownloadFollowRedirects }; } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 06f6fcc8c2a..0907a847c03 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -535,12 +535,14 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic throw new InvalidParameterValueException("File:// type urls are currently unsupported"); } UriUtils.validateUrl(format, url); + boolean followRedirects = StorageManager.DataStoreDownloadFollowRedirects.value(); if (VolumeUrlCheck.value()) { // global setting that can be set when their MS does not have internet access s_logger.debug("Checking url: " + url); - DirectDownloadHelper.checkUrlExistence(url); + DirectDownloadHelper.checkUrlExistence(url, followRedirects); } // Check that the resource limit for secondary storage won't be exceeded - _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(ownerId), ResourceType.secondary_storage, UriUtils.getRemoteSize(url)); + _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(ownerId), ResourceType.secondary_storage, + UriUtils.getRemoteSize(url, followRedirects)); } else { _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(ownerId), ResourceType.secondary_storage); } @@ -642,7 +644,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic _resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.volume); //url can be null incase of postupload if (url != null) { - _resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.secondary_storage, UriUtils.getRemoteSize(url)); + _resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.secondary_storage, + UriUtils.getRemoteSize(url, StorageManager.DataStoreDownloadFollowRedirects.value())); } return volume; diff --git a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java index 1633cd8a360..2a536fa7a79 100644 --- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java +++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java @@ -87,6 +87,7 @@ import com.cloud.server.StatsCollector; import com.cloud.storage.ScopeType; import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.TemplateType; +import com.cloud.storage.StorageManager; import com.cloud.storage.TemplateProfile; import com.cloud.storage.VMTemplateStorageResourceAssoc.Status; import com.cloud.storage.VMTemplateVO; @@ -156,7 +157,8 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { * Validate on random running KVM host that URL is reachable * @param url url */ - private Long performDirectDownloadUrlValidation(final String format, final String url, final List zoneIds) { + private Long performDirectDownloadUrlValidation(final String format, final String url, final List zoneIds, + boolean followRedirects) { HostVO host = null; if (zoneIds != null && !zoneIds.isEmpty()) { for (Long zoneId : zoneIds) { @@ -175,7 +177,8 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { Integer socketTimeout = DirectDownloadManager.DirectDownloadSocketTimeout.value(); Integer connectRequestTimeout = DirectDownloadManager.DirectDownloadConnectionRequestTimeout.value(); Integer connectTimeout = DirectDownloadManager.DirectDownloadConnectTimeout.value(); - CheckUrlCommand cmd = new CheckUrlCommand(format, url, connectTimeout, connectRequestTimeout, socketTimeout); + CheckUrlCommand cmd = new CheckUrlCommand(format, url, connectTimeout, connectRequestTimeout, socketTimeout, + followRedirects); s_logger.debug("Performing URL " + url + " validation on host " + host.getId()); Answer answer = _agentMgr.easySend(host.getId(), cmd); if (answer == null || !answer.getResult()) { @@ -199,6 +202,7 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { TemplateProfile profile = super.prepare(cmd); String url = profile.getUrl(); UriUtils.validateUrl(ImageFormat.ISO.getFileExtension(), url); + boolean followRedirects = StorageManager.DataStoreDownloadFollowRedirects.value(); if (cmd.isDirectDownload()) { DigestHelper.validateChecksumString(cmd.getChecksum()); List zoneIds = null; @@ -206,12 +210,15 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { zoneIds = new ArrayList<>(); zoneIds.add(cmd.getZoneId()); } - Long templateSize = performDirectDownloadUrlValidation(ImageFormat.ISO.getFileExtension(), url, zoneIds); + Long templateSize = performDirectDownloadUrlValidation(ImageFormat.ISO.getFileExtension(), url, zoneIds, + followRedirects); profile.setSize(templateSize); } profile.setUrl(url); // Check that the resource limit for secondary storage won't be exceeded - _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(cmd.getEntityOwnerId()), ResourceType.secondary_storage, UriUtils.getRemoteSize(url)); + _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(cmd.getEntityOwnerId()), + ResourceType.secondary_storage, + UriUtils.getRemoteSize(url, followRedirects)); return profile; } @@ -229,14 +236,18 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { TemplateProfile profile = super.prepare(cmd); String url = profile.getUrl(); UriUtils.validateUrl(cmd.getFormat(), url, cmd.isDirectDownload()); + boolean followRedirects = StorageManager.DataStoreDownloadFollowRedirects.value(); if (cmd.isDirectDownload()) { DigestHelper.validateChecksumString(cmd.getChecksum()); - Long templateSize = performDirectDownloadUrlValidation(cmd.getFormat(), url, cmd.getZoneIds()); + Long templateSize = performDirectDownloadUrlValidation(cmd.getFormat(), url, cmd.getZoneIds(), + followRedirects); profile.setSize(templateSize); } profile.setUrl(url); // Check that the resource limit for secondary storage won't be exceeded - _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(cmd.getEntityOwnerId()), ResourceType.secondary_storage, UriUtils.getRemoteSize(url)); + _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(cmd.getEntityOwnerId()), + ResourceType.secondary_storage, + UriUtils.getRemoteSize(url, followRedirects)); return profile; } diff --git a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index af543c6c798..0a21815789d 100644 --- a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -272,7 +272,8 @@ public class DirectDownloadManagerImpl extends ManagerBase implements DirectDown PrimaryDataStoreTO to = (PrimaryDataStoreTO) primaryDataStore.getTO(); DownloadProtocol protocol = getProtocolFromUrl(url); - DirectDownloadCommand cmd = getDirectDownloadCommandFromProtocol(protocol, url, templateId, to, checksum, headers); + DirectDownloadCommand cmd = getDirectDownloadCommandFromProtocol(protocol, url, templateId, to, checksum, + headers); cmd.setTemplateSize(template.getSize()); cmd.setFormat(template.getFormat()); @@ -393,19 +394,23 @@ public class DirectDownloadManagerImpl extends ManagerBase implements DirectDown /** * Return DirectDownloadCommand according to the protocol */ - private DirectDownloadCommand getDirectDownloadCommandFromProtocol(DownloadProtocol protocol, String url, Long templateId, PrimaryDataStoreTO destPool, - String checksum, Map httpHeaders) { + private DirectDownloadCommand getDirectDownloadCommandFromProtocol(DownloadProtocol protocol, String url, + Long templateId, PrimaryDataStoreTO destPool, String checksum, Map httpHeaders) { int connectTimeout = DirectDownloadConnectTimeout.value(); int soTimeout = DirectDownloadSocketTimeout.value(); int connectionRequestTimeout = DirectDownloadConnectionRequestTimeout.value(); + boolean followRedirects = StorageManager.DataStoreDownloadFollowRedirects.value(); if (protocol.equals(DownloadProtocol.HTTP)) { - return new HttpDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders, connectTimeout, soTimeout); + return new HttpDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders, connectTimeout, + soTimeout, followRedirects); } else if (protocol.equals(DownloadProtocol.HTTPS)) { - return new HttpsDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders, connectTimeout, soTimeout, connectionRequestTimeout); + return new HttpsDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders, connectTimeout, + soTimeout, connectionRequestTimeout, followRedirects); } else if (protocol.equals(DownloadProtocol.NFS)) { return new NfsDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders); } else if (protocol.equals(DownloadProtocol.METALINK)) { - return new MetalinkDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders, connectTimeout, soTimeout); + return new MetalinkDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders, connectTimeout, + soTimeout, followRedirects); } else { return null; } diff --git a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java index 20e1be95e52..ecbc9505e04 100644 --- a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java @@ -19,6 +19,8 @@ package com.cloud.storage; import java.util.ArrayList; import java.util.List; +import org.apache.cloudstack.framework.config.ConfigDepot; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.junit.Assert; @@ -31,6 +33,8 @@ import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; import com.cloud.agent.api.StoragePoolInfo; +import com.cloud.dc.DataCenterVO; +import com.cloud.dc.dao.DataCenterDao; import com.cloud.host.Host; import com.cloud.storage.dao.VolumeDao; import com.cloud.vm.VMInstanceVO; @@ -44,6 +48,12 @@ public class StorageManagerImplTest { @Mock VMInstanceDao vmInstanceDao; + @Mock + ConfigDepot configDepot; + @Mock + ConfigurationDao configurationDao; + @Mock + DataCenterDao dataCenterDao; @Spy @InjectMocks @@ -155,7 +165,38 @@ public class StorageManagerImplTest { PrimaryDataStoreDao storagePoolDao = Mockito.mock(PrimaryDataStoreDao.class); storageManagerImpl._storagePoolDao = storagePoolDao; Assert.assertTrue(storageManagerImpl.storagePoolCompatibleWithVolumePool(storagePool, volume)); + } + @Test + public void testEnableDefaultDatastoreDownloadRedirectionForExistingInstallationsNoChange() { + Mockito.when(configDepot.isNewConfig(StorageManager.DataStoreDownloadFollowRedirects)) + .thenReturn(false); + storageManagerImpl.enableDefaultDatastoreDownloadRedirectionForExistingInstallations(); + Mockito.verify(configurationDao, Mockito.never()).update(Mockito.anyString(), Mockito.anyString()); + } + + @Test + public void testEnableDefaultDatastoreDownloadRedirectionForExistingInstallationsOldInstall() { + Mockito.when(configDepot.isNewConfig(StorageManager.DataStoreDownloadFollowRedirects)) + .thenReturn(true); + Mockito.when(dataCenterDao.listAll(Mockito.any())) + .thenReturn(List.of(Mockito.mock(DataCenterVO.class))); + Mockito.doReturn(true).when(configurationDao).update(Mockito.anyString(), Mockito.anyString()); + storageManagerImpl.enableDefaultDatastoreDownloadRedirectionForExistingInstallations(); + Mockito.verify(configurationDao, Mockito.times(1)) + .update(StorageManager.DataStoreDownloadFollowRedirects.key(), "true"); + } + + @Test + public void testEnableDefaultDatastoreDownloadRedirectionForExistingInstallationsNewInstall() { + Mockito.when(configDepot.isNewConfig(StorageManager.DataStoreDownloadFollowRedirects)) + .thenReturn(true); + Mockito.when(dataCenterDao.listAll(Mockito.any())) + .thenReturn(new ArrayList<>()); //new installation + storageManagerImpl.enableDefaultDatastoreDownloadRedirectionForExistingInstallations(); + Mockito.verify(configurationDao, Mockito.never()) + .update(StorageManager.DataStoreDownloadFollowRedirects.key(), + StorageManager.DataStoreDownloadFollowRedirects.defaultValue()); } } diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManager.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManager.java index a0417704028..5526835b8f2 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManager.java +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManager.java @@ -41,17 +41,21 @@ public interface DownloadManager extends Manager { * @param hvm whether the template is a hardware virtual machine * @param accountId the accountId of the iso owner (null if public iso) * @param descr description of the template - * @param user username used for authentication to the server - * @param password password used for authentication to the server + * @param userName username used for authentication to the server + * @param passwd password used for authentication to the server * @param maxDownloadSizeInBytes (optional) max download size for the template, in bytes. * @param resourceType signifying the type of resource like template, volume etc. + * @param followRedirects whether downloader follows redirections * @return job-id that can be used to interrogate the status of the download. */ - public String downloadPublicTemplate(long id, String url, String name, ImageFormat format, boolean hvm, Long accountId, String descr, String cksum, - String installPathPrefix, String templatePath, String userName, String passwd, long maxDownloadSizeInBytes, Proxy proxy, ResourceType resourceType); + public String downloadPublicTemplate(long id, String url, String name, ImageFormat format, boolean hvm, + Long accountId, String descr, String cksum, String installPathPrefix, String templatePath, + String userName, String passwd, long maxDownloadSizeInBytes, Proxy proxy, ResourceType resourceType, + boolean followRedirects); - public String downloadS3Template(S3TO s3, long id, String url, String name, ImageFormat format, boolean hvm, Long accountId, String descr, String cksum, - String installPathPrefix, String user, String password, long maxTemplateSizeInBytes, Proxy proxy, ResourceType resourceType); + public String downloadS3Template(S3TO s3, long id, String url, String name, ImageFormat format, boolean hvm, + Long accountId, String descr, String cksum, String installPathPrefix, String user, String password, + long maxTemplateSizeInBytes, Proxy proxy, ResourceType resourceType, boolean followRedirects); Map getProcessors(); diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java index f647b497f58..e14977682b6 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java @@ -549,8 +549,9 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager } @Override - public String downloadS3Template(S3TO s3, long id, String url, String name, ImageFormat format, boolean hvm, Long accountId, String descr, String cksum, - String installPathPrefix, String user, String password, long maxTemplateSizeInBytes, Proxy proxy, ResourceType resourceType) { + public String downloadS3Template(S3TO s3, long id, String url, String name, ImageFormat format, boolean hvm, + Long accountId, String descr, String cksum, String installPathPrefix, String user, String password, + long maxTemplateSizeInBytes, Proxy proxy, ResourceType resourceType, boolean followRedirects) { UUID uuid = UUID.randomUUID(); String jobId = uuid.toString(); @@ -570,6 +571,7 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager } else { throw new CloudRuntimeException("Unable to download from URL: " + url); } + td.setFollowRedirects(followRedirects); DownloadJob dj = new DownloadJob(td, jobId, id, name, format, hvm, accountId, descr, cksum, installPathPrefix, resourceType); dj.setTmpltPath(installPathPrefix); jobs.put(jobId, dj); @@ -579,8 +581,10 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager } @Override - public String downloadPublicTemplate(long id, String url, String name, ImageFormat format, boolean hvm, Long accountId, String descr, String cksum, - String installPathPrefix, String templatePath, String user, String password, long maxTemplateSizeInBytes, Proxy proxy, ResourceType resourceType) { + public String downloadPublicTemplate(long id, String url, String name, ImageFormat format, boolean hvm, + Long accountId, String descr, String cksum, String installPathPrefix, String templatePath, String user, + String password, long maxTemplateSizeInBytes, Proxy proxy, ResourceType resourceType, + boolean followRedirects) { UUID uuid = UUID.randomUUID(); String jobId = uuid.toString(); String tmpDir = installPathPrefix; @@ -632,6 +636,7 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager } else { throw new CloudRuntimeException("Unable to download from URL: " + url); } + td.setFollowRedirects(followRedirects); // NOTE the difference between installPathPrefix and templatePath // here. instalPathPrefix is the absolute path for template // including mount directory @@ -768,12 +773,16 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager String jobId = null; if (dstore instanceof S3TO) { jobId = - downloadS3Template((S3TO)dstore, cmd.getId(), cmd.getUrl(), cmd.getName(), cmd.getFormat(), cmd.isHvm(), cmd.getAccountId(), cmd.getDescription(), - cmd.getChecksum(), installPathPrefix, user, password, maxDownloadSizeInBytes, cmd.getProxy(), resourceType); + downloadS3Template((S3TO)dstore, cmd.getId(), cmd.getUrl(), cmd.getName(), cmd.getFormat(), + cmd.isHvm(), cmd.getAccountId(), cmd.getDescription(), cmd.getChecksum(), + installPathPrefix, user, password, maxDownloadSizeInBytes, cmd.getProxy(), resourceType, + cmd.isFollowRedirects()); } else { jobId = - downloadPublicTemplate(cmd.getId(), cmd.getUrl(), cmd.getName(), cmd.getFormat(), cmd.isHvm(), cmd.getAccountId(), cmd.getDescription(), - cmd.getChecksum(), installPathPrefix, cmd.getInstallPath(), user, password, maxDownloadSizeInBytes, cmd.getProxy(), resourceType); + downloadPublicTemplate(cmd.getId(), cmd.getUrl(), cmd.getName(), cmd.getFormat(), cmd.isHvm(), + cmd.getAccountId(), cmd.getDescription(), cmd.getChecksum(), installPathPrefix, + cmd.getInstallPath(), user, password, maxDownloadSizeInBytes, cmd.getProxy(), resourceType, + cmd.isFollowRedirects()); } sleep(); if (jobId == null) { diff --git a/utils/src/main/java/com/cloud/utils/UriUtils.java b/utils/src/main/java/com/cloud/utils/UriUtils.java index a2bfa9eaa6f..7327218a897 100644 --- a/utils/src/main/java/com/cloud/utils/UriUtils.java +++ b/utils/src/main/java/com/cloud/utils/UriUtils.java @@ -213,7 +213,7 @@ public class UriUtils { } // Get the size of a file from URL response header. - public static long getRemoteSize(String url) { + public static long getRemoteSize(String url, Boolean followRedirect) { long remoteSize = 0L; final String[] methods = new String[]{"HEAD", "GET"}; IllegalArgumentException exception = null; @@ -228,6 +228,7 @@ public class UriUtils { httpConn.setRequestMethod(method); httpConn.setConnectTimeout(2000); httpConn.setReadTimeout(5000); + httpConn.setInstanceFollowRedirects(Boolean.TRUE.equals(followRedirect)); String contentLength = httpConn.getHeaderField("content-length"); if (contentLength != null) { remoteSize = Long.parseLong(contentLength); diff --git a/utils/src/main/java/com/cloud/utils/storage/QCOW2Utils.java b/utils/src/main/java/com/cloud/utils/storage/QCOW2Utils.java index 4daf138bb8c..300fc4da1fe 100644 --- a/utils/src/main/java/com/cloud/utils/storage/QCOW2Utils.java +++ b/utils/src/main/java/com/cloud/utils/storage/QCOW2Utils.java @@ -22,8 +22,9 @@ package com.cloud.utils.storage; import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; -import java.net.MalformedURLException; -import java.net.URL; +import java.net.HttpURLConnection; +import java.net.URI; +import java.net.URISyntaxException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; @@ -32,7 +33,6 @@ import org.apache.commons.compress.compressors.CompressorStreamFactory; import org.apache.log4j.Logger; import com.cloud.utils.NumbersUtil; -import com.cloud.utils.UriUtils; public final class QCOW2Utils { public static final Logger LOGGER = Logger.getLogger(QCOW2Utils.class.getName()); @@ -112,16 +112,23 @@ public final class QCOW2Utils { } } - public static long getVirtualSize(String urlStr) { + public static long getVirtualSizeFromUrl(String urlStr, boolean followRedirects) { + HttpURLConnection httpConn = null; try { - URL url = new URL(urlStr); - return getVirtualSize(url.openStream(), UriUtils.isUrlForCompressedFile(urlStr)); - } catch (MalformedURLException e) { + URI url = new URI(urlStr); + httpConn = (HttpURLConnection)url.toURL().openConnection(); + httpConn.setInstanceFollowRedirects(followRedirects); + return getVirtualSizeFromInputStream(httpConn.getInputStream()); + } catch (URISyntaxException e) { LOGGER.warn("Failed to validate for qcow2, malformed URL: " + urlStr + ", error: " + e.getMessage()); throw new IllegalArgumentException("Invalid URL: " + urlStr); } catch (IOException e) { LOGGER.warn("Failed to validate for qcow2, error: " + e.getMessage()); throw new IllegalArgumentException("Failed to connect URL: " + urlStr); + } finally { + if (httpConn != null) { + httpConn.disconnect(); + } } } }