From 939d0b9011aef846c2f3a093aed480193d1d45b9 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 27 Mar 2024 10:07:47 +0100 Subject: [PATCH] engine-storage: control download redirection Add a global setting to control whether redirection is allowed while downloading templates and volumes core: some changes on SimpleHttpMultiFileDownloader similar as HttpTemplateDownloader Signed-off-by: Abhishek Kumar (cherry picked from commit b1642bc3bf58ccde9f56f632b5a9fe46a3eb5356) Signed-off-by: Rohit Yadav --- .../template/HttpTemplateDownloader.java | 18 +++++++- .../template/MetalinkTemplateDownloader.java | 10 ++++- .../template/S3TemplateDownloader.java | 19 ++++++-- .../SimpleHttpMultiFileDownloader.java | 22 +++++++-- .../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 | 45 ++++++++++--------- .../DirectTemplateDownloaderImpl.java | 13 +++++- .../HttpDirectTemplateDownloader.java | 21 +++++---- .../HttpsDirectTemplateDownloader.java | 26 +++++++---- .../MetalinkDirectTemplateDownloader.java | 20 ++++----- .../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 | 3 +- .../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 | 4 +- .../kvm/storage/KVMStorageProcessor.java | 2 +- .../com/cloud/storage/StorageManagerImpl.java | 29 +++++++++++- .../cloud/storage/VolumeApiServiceImpl.java | 12 +++-- .../template/HypervisorTemplateAdapter.java | 19 +++++--- .../download/DirectDownloadManagerImpl.java | 17 ++++--- .../cloud/storage/StorageManagerImplTest.java | 43 ++++++++++++++++++ .../storage/template/DownloadManager.java | 16 ++++--- .../storage/template/DownloadManagerImpl.java | 25 +++++++---- .../main/java/com/cloud/utils/UriUtils.java | 3 +- .../com/cloud/utils/storage/QCOW2Utils.java | 20 ++++++--- 46 files changed, 457 insertions(+), 123 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 d55c387d820..7ad8070b6e5 100755 --- a/core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java +++ b/core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java @@ -28,6 +28,7 @@ import java.io.RandomAccessFile; import java.net.URI; import java.net.URISyntaxException; import java.util.Date; +import java.util.List; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.storage.command.DownloadCommand.ResourceType; @@ -80,6 +81,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) { @@ -111,7 +113,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; } @@ -337,6 +339,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; @@ -538,4 +546,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/SimpleHttpMultiFileDownloader.java b/core/src/main/java/com/cloud/storage/template/SimpleHttpMultiFileDownloader.java index 7a0ce47ec99..56cf76fb9f6 100644 --- a/core/src/main/java/com/cloud/storage/template/SimpleHttpMultiFileDownloader.java +++ b/core/src/main/java/com/cloud/storage/template/SimpleHttpMultiFileDownloader.java @@ -25,6 +25,7 @@ import java.io.InputStream; import java.io.RandomAccessFile; import java.util.Date; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.apache.cloudstack.managed.context.ManagedContextRunnable; @@ -73,6 +74,7 @@ public class SimpleHttpMultiFileDownloader extends ManagedContextRunnable implem private final HttpMethodRetryHandler retryHandler; private HashMap urlFileMap; + private boolean followRedirects = false; public SimpleHttpMultiFileDownloader(StorageLayer storageLayer, String[] downloadUrls, String toDir, DownloadCompleteCallback callback, long maxTemplateSizeInBytes, @@ -94,7 +96,7 @@ public class SimpleHttpMultiFileDownloader extends ManagedContextRunnable implem private GetMethod createRequest(String downloadUrl) { GetMethod request = new GetMethod(downloadUrl); request.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, retryHandler); - request.setFollowRedirects(true); + request.setFollowRedirects(followRedirects); return request; } @@ -170,7 +172,7 @@ public class SimpleHttpMultiFileDownloader extends ManagedContextRunnable implem urlFileMap.put(downloadUrl, currentToFile); file = new File(currentToFile); long localFileSize = checkLocalFileSizeForResume(resume, file); - if (checkServerResponse(localFileSize)) return 0; + if (checkServerResponse(localFileSize, downloadUrl)) return 0; if (!tryAndGetRemoteSize()) return 0; if (!canHandleDownloadSize()) return 0; checkAndSetDownloadSize(); @@ -317,7 +319,7 @@ public class SimpleHttpMultiFileDownloader extends ManagedContextRunnable implem return true; } - private boolean checkServerResponse(long localFileSize) throws IOException { + private boolean checkServerResponse(long localFileSize, String downloadUrl) throws IOException { int responseCode = 0; if (localFileSize > 0) { @@ -331,6 +333,12 @@ public class SimpleHttpMultiFileDownloader extends ManagedContextRunnable implem } else if ((responseCode = client.executeMethod(request)) != HttpStatus.SC_OK) { currentStatus = 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; @@ -478,4 +486,12 @@ public class SimpleHttpMultiFileDownloader extends ManagedContextRunnable implem public Map getDownloadedFilesMap() { return urlFileMap; } + + @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/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..325f61427a9 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,19 +44,25 @@ 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; this.connectTimeout = connectTimeout; this.socketTimeout = socketTimeout; this.connectionRequestTimeout = connectionRequestTimeout; + this.followRedirects = followRedirects; } @Override 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..a00274e259e 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,9 @@ 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 +75,27 @@ 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..068f6b08418 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,14 @@ 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 +69,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 +112,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 +131,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..3a48ade4cd8 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,26 @@ 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 +96,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 +171,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..86b97880a85 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,14 @@ 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 4032ac0b632..f44220f31d0 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 @@ -49,6 +49,8 @@ public class DownloadCommand extends AbstractDownloadCommand implements Internal private DataStoreTO _store; private DataStoreTO cacheStore; + private boolean followRedirects = false; + protected DownloadCommand() { } @@ -65,6 +67,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) { @@ -80,6 +83,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) { @@ -95,6 +99,7 @@ public class DownloadCommand extends AbstractDownloadCommand implements Internal _store = volume.getDataStore(); this.maxDownloadSizeInBytes = maxDownloadSizeInBytes; resourceType = ResourceType.VOLUME; + this.followRedirects = volume.isFollowRedirects(); } public DownloadCommand(SnapshotObjectTO snapshot, Long maxDownloadSizeInBytes, String url) { @@ -194,4 +199,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 70cb6d155b0..76b93909b8c 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..2fc0f7b00a0 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,8 +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() { metalinkDownloader.downloader = httpsDownloader; 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 f5cf443211c..b1594e315bb 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 @@ -199,6 +199,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); ConfigKey HEURISTICS_SCRIPT_TIMEOUT = new ConfigKey<>("Advanced", Long.class, "heuristics.script.timeout", "3000", "The maximum runtime, in milliseconds, to execute the heuristic rule; if it is reached, a timeout will happen.", true); 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 9e41e0d4d0e..2e49698d0da 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..0e1bf57a974 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 @@ -42,9 +42,9 @@ public class LibvirtCheckUrlCommand extends CommandWrapper _discoverers; public List getDiscoverers() { @@ -442,6 +449,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); @@ -673,7 +697,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } _executor.scheduleWithFixedDelay(new DownloadURLGarbageCollector(), _downloadUrlCleanupInterval, _downloadUrlCleanupInterval, TimeUnit.SECONDS); - + enableDefaultDatastoreDownloadRedirectionForExistingInstallations(); return true; } @@ -3719,7 +3743,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C MountDisabledStoragePool, VmwareCreateCloneFull, VmwareAllowParallelExecution, - ConvertVmwareInstanceToKvmTimeout + ConvertVmwareInstanceToKvmTimeout, + DataStoreDownloadFollowRedirects }; } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 2e8b36da446..88d45d54aa5 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -552,12 +552,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); } @@ -659,8 +661,10 @@ 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)); - volume.setSize(UriUtils.getRemoteSize(url)); + long remoteSize = UriUtils.getRemoteSize(url, StorageManager.DataStoreDownloadFollowRedirects.value()); + _resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.secondary_storage, + remoteSize); + volume.setSize(remoteSize); } 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 b886f0868f6..d8132df6d28 100644 --- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java +++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java @@ -89,6 +89,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; @@ -161,7 +162,7 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { * @param url url */ private Long performDirectDownloadUrlValidation(final String format, final Hypervisor.HypervisorType hypervisor, - final String url, final List zoneIds) { + final String url, final List zoneIds, final boolean followRedirects) { HostVO host = null; if (zoneIds != null && !zoneIds.isEmpty()) { for (Long zoneId : zoneIds) { @@ -180,7 +181,7 @@ 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()) { @@ -204,6 +205,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; @@ -212,12 +214,14 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { zoneIds.add(cmd.getZoneId()); } Long templateSize = performDirectDownloadUrlValidation(ImageFormat.ISO.getFileExtension(), - Hypervisor.HypervisorType.KVM, url, zoneIds); + Hypervisor.HypervisorType.KVM, 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; } @@ -236,15 +240,18 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { String url = profile.getUrl(); UriUtils.validateUrl(cmd.getFormat(), url, cmd.isDirectDownload()); Hypervisor.HypervisorType hypervisor = Hypervisor.HypervisorType.getType(cmd.getHypervisor()); + boolean followRedirects = StorageManager.DataStoreDownloadFollowRedirects.value(); if (cmd.isDirectDownload()) { DigestHelper.validateChecksumString(cmd.getChecksum()); Long templateSize = performDirectDownloadUrlValidation(cmd.getFormat(), - hypervisor, url, cmd.getZoneIds()); + hypervisor, 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 ba5f2baf932..dbceac9efb1 100644 --- a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java @@ -17,12 +17,17 @@ package com.cloud.storage; import com.cloud.agent.api.StoragePoolInfo; +import com.cloud.dc.DataCenterVO; +import com.cloud.dc.dao.DataCenterDao; 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.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.apache.commons.collections.MapUtils; @@ -48,6 +53,12 @@ public class StorageManagerImplTest { @Mock VMInstanceDao vmInstanceDao; + @Mock + ConfigDepot configDepot; + @Mock + ConfigurationDao configurationDao; + @Mock + DataCenterDao dataCenterDao; @Spy @InjectMocks @@ -206,4 +217,36 @@ public class StorageManagerImplTest { throw new RuntimeException(e); } } + + @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 0396f96f094..e1e69028a9b 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 @@ -661,8 +661,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(); @@ -682,6 +683,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); @@ -717,8 +719,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; @@ -765,6 +769,7 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager 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 @@ -901,12 +906,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..a36b01197ee 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; @@ -112,16 +113,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 getVirtualSize(httpConn.getInputStream(), UriUtils.isUrlForCompressedFile(urlStr)); + } 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(); + } } } }