From f4058705d7b5bcd838f8dc562052aa1e7ac9a64f Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 25 Mar 2024 11:27:35 +0100 Subject: [PATCH 01/13] UI: add dialog to select a VPC tier when create LB for autoscaling in VPC (#8793) --- ui/src/views/network/LoadBalancing.vue | 170 ++++++++++++++++++++++++- 1 file changed, 167 insertions(+), 3 deletions(-) diff --git a/ui/src/views/network/LoadBalancing.vue b/ui/src/views/network/LoadBalancing.vue index 973a6240d10..eeec232cee5 100644 --- a/ui/src/views/network/LoadBalancing.vue +++ b/ui/src/views/network/LoadBalancing.vue @@ -84,12 +84,18 @@ {{ $t('label.no') }} -
+
{{ $t('label.add.vms') }}
{{ $t('label.add') }}
+
+
{{ $t('label.select.tier') }}
+ + {{ $t('label.add') }} + +
{{ $t('label.add') }}
@@ -519,7 +525,7 @@ @@ -546,6 +552,71 @@
+ +
+ + + + + + + + +
+ {{ $t('label.cancel') }} + {{ $t('label.ok') }} +
+
+ + { + this.networkCount = response.listnetworksresponse.count || 0 + this.networks = response.listnetworksresponse.network || [] + }).catch(error => { + this.$notifyError(error) + }).finally(() => { + this.addNetworkModalLoading = false + }) + this.selectedTierForAutoScaling = null + }, + onNetworkSearch (value) { + this.searchQuery = value + this.fetchNetworks() + }, + handleChangeNetworkPage (page, pageSize) { + this.networkPage = page + this.networkPageSize = pageSize + this.fetchNetworks() + }, + handleChangeNetworkPageSize (currentPage, pageSize) { + this.networkPage = currentPage + this.networkPageSize = pageSize + this.fetchNetworks() + }, handleAssignToLBRule (data) { const vmIDIpMap = {} @@ -1560,7 +1718,8 @@ export default { return } - const networkId = ('vpcid' in this.resource && !('associatednetworkid' in this.resource)) ? this.selectedTier : this.resource.associatednetworkid + const networkId = this.selectedTierForAutoScaling != null ? this.selectedTierForAutoScaling + : ('vpcid' in this.resource && !('associatednetworkid' in this.resource)) ? this.selectedTier : this.resource.associatednetworkid api('createLoadBalancerRule', { openfirewall: false, networkid: networkId, @@ -1573,7 +1732,9 @@ export default { cidrlist: this.newRule.cidrlist }).then(response => { this.addVmModalVisible = false + this.addNetworkModalVisible = false this.handleAssignToLBRule(response.createloadbalancerruleresponse.id) + this.associatednetworkid = networkId }).catch(error => { this.$notifyError(error) this.loading = false @@ -1597,6 +1758,9 @@ export default { this.nics = [] this.addVmModalVisible = false this.newRule.virtualmachineid = [] + this.addNetworkModalLoading = false + this.addNetworkModalVisible = false + this.selectedTierForAutoScaling = null }, handleChangePage (page, pageSize) { this.page = page From 08d9d06d45316edb99ce3aba481de188b2baa7e0 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 25 Mar 2024 14:45:34 +0100 Subject: [PATCH 02/13] api,server,ui: add project ID and name to UserDataResponse (#8656) * api,server,ui: add project ID and name to UserDataResponse * Update: add since --- .../api/response/UserDataResponse.java | 20 ++++++++++++++++++- .../java/com/cloud/api/ApiResponseHelper.java | 7 +------ ui/src/config/section/compute.js | 5 +++++ ui/src/views/AutogenView.vue | 2 +- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/response/UserDataResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/UserDataResponse.java index bbe27f84520..e69094c8f80 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/UserDataResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/UserDataResponse.java @@ -24,7 +24,7 @@ import org.apache.cloudstack.api.BaseResponseWithAnnotations; import org.apache.cloudstack.api.EntityReference; @EntityReference(value = UserData.class) -public class UserDataResponse extends BaseResponseWithAnnotations { +public class UserDataResponse extends BaseResponseWithAnnotations implements ControlledEntityResponse { @SerializedName(ApiConstants.ID) @Param(description = "ID of the ssh keypair") @@ -40,6 +40,14 @@ public class UserDataResponse extends BaseResponseWithAnnotations { @SerializedName(ApiConstants.ACCOUNT) @Param(description="the owner of the userdata") private String accountName; + @SerializedName(ApiConstants.PROJECT_ID) + @Param(description = "the project id of the userdata", since = "4.19.1") + private String projectId; + + @SerializedName(ApiConstants.PROJECT) + @Param(description = "the project name of the userdata", since = "4.19.1") + private String projectName; + @SerializedName(ApiConstants.DOMAIN_ID) @Param(description="the domain id of the userdata owner") private String domainId; @@ -118,6 +126,16 @@ public class UserDataResponse extends BaseResponseWithAnnotations { this.accountName = accountName; } + @Override + public void setProjectId(String projectId) { + this.projectId = projectId; + } + + @Override + public void setProjectName(String projectName) { + this.projectName = projectName; + } + public String getDomainName() { return domain; } diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index e2b72f6175c..6d66da43c37 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -4818,12 +4818,7 @@ public class ApiResponseHelper implements ResponseGenerator { @Override public UserDataResponse createUserDataResponse(UserData userData) { UserDataResponse response = new UserDataResponse(userData.getUuid(), userData.getName(), userData.getUserData(), userData.getParams()); - Account account = ApiDBUtils.findAccountById(userData.getAccountId()); - response.setAccountId(account.getUuid()); - response.setAccountName(account.getAccountName()); - Domain domain = ApiDBUtils.findDomainById(userData.getDomainId()); - response.setDomainId(domain.getUuid()); - response.setDomainName(domain.getName()); + populateOwner(response, userData); response.setHasAnnotation(annotationDao.hasAnnotations(userData.getUuid(), AnnotationService.EntityType.USER_DATA.name(), _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); return response; diff --git a/ui/src/config/section/compute.js b/ui/src/config/section/compute.js index f1b3e1b2c99..bd18535f380 100644 --- a/ui/src/config/section/compute.js +++ b/ui/src/config/section/compute.js @@ -898,7 +898,12 @@ export default { var fields = ['name', 'id'] if (['Admin', 'DomainAdmin'].includes(store.getters.userInfo.roletype)) { fields.push('account') + if (store.getters.listAllProjects) { + fields.push('project') + } fields.push('domain') + } else if (store.getters.listAllProjects) { + fields.push('project') } return fields }, diff --git a/ui/src/views/AutogenView.vue b/ui/src/views/AutogenView.vue index bf1c42d4c05..77ee73d700c 100644 --- a/ui/src/views/AutogenView.vue +++ b/ui/src/views/AutogenView.vue @@ -793,7 +793,7 @@ export default { } this.projectView = Boolean(store.getters.project && store.getters.project.id) - this.hasProjectId = ['vm', 'vmgroup', 'ssh', 'affinitygroup', 'volume', 'snapshot', 'vmsnapshot', 'guestnetwork', + this.hasProjectId = ['vm', 'vmgroup', 'ssh', 'affinitygroup', 'userdata', 'volume', 'snapshot', 'vmsnapshot', 'guestnetwork', 'vpc', 'securitygroups', 'publicip', 'vpncustomergateway', 'template', 'iso', 'event', 'kubernetes', 'autoscalevmgroup', 'vnfapp'].includes(this.$route.name) From 31301f56f649e92c39108b0100200f201f3b348a Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 27 Mar 2024 11:52:44 +0530 Subject: [PATCH 03/13] Fix missing actions on Guest IP ranges for networks (#8777) * Fix missing actions on Guest IP ranges for networks * Show success and error pop on deleting guest ip ranges --- ui/src/views/network/GuestIpRanges.vue | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ui/src/views/network/GuestIpRanges.vue b/ui/src/views/network/GuestIpRanges.vue index 0ecc033119f..815b3d9132f 100644 --- a/ui/src/views/network/GuestIpRanges.vue +++ b/ui/src/views/network/GuestIpRanges.vue @@ -36,14 +36,16 @@ :rowKey="item => item.id" :pagination="false" > - @@ -307,6 +312,10 @@ export default { this.error = this.$t('message.error.provide.setting') return } + if (this.newKey.startsWith('extraconfig')) { + this.error = this.$t('error.unable.to.add.setting.extraconfig') + return + } if (!this.allowEditOfDetail(this.newKey)) { this.error = this.$t('error.unable.to.proceed') return From 67e2061f4b87934c863cbafc5031821ad7ef6d02 Mon Sep 17 00:00:00 2001 From: dahn Date: Tue, 26 Mar 2024 07:13:23 +0100 Subject: [PATCH 10/13] api: client verification in servlet This introduces new global settings to handle how client address checks are handled by the API layer: proxy.header.verify: enables/disables checking of ipaddresses from a proxy set header proxy.header.names: a list of names to check for allowed ipaddresses from a proxy set header. proxy.cidr: a list of cidrs for which \"proxy.header.names\" are honoured if the \"Remote_Addr\" is in this list. (cherry picked from commit b65546636d84a5790e0297b1b0ca8e5a67a48dbc) Signed-off-by: Rohit Yadav (cherry picked from commit b1e0bf9dbd464f8fb7c22f36505dee0148e2d6f4) Signed-off-by: Rohit Yadav --- .../org/apache/cloudstack/ServerDaemon.java | 3 +- .../framework/config/ConfigKey.java | 1 + .../main/java/com/cloud/api/ApiServer.java | 40 ++++++++++++++---- .../main/java/com/cloud/api/ApiServlet.java | 41 +++++++++++++------ .../java/com/cloud/api/ApiServletTest.java | 29 +++++++++---- .../java/com/cloud/utils/StringUtils.java | 2 +- 6 files changed, 85 insertions(+), 31 deletions(-) diff --git a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java index 763c274c7f5..fb84e1297e6 100644 --- a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java +++ b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java @@ -31,7 +31,6 @@ import org.apache.commons.daemon.DaemonContext; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.eclipse.jetty.jmx.MBeanContainer; -import org.eclipse.jetty.server.ForwardedRequestCustomizer; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.NCSARequestLog; @@ -173,7 +172,7 @@ public class ServerDaemon implements Daemon { // HTTP config final HttpConfiguration httpConfig = new HttpConfiguration(); - httpConfig.addCustomizer( new ForwardedRequestCustomizer() ); +// it would be nice to make this dynamic but we take care of this ourselves for now: httpConfig.addCustomizer( new ForwardedRequestCustomizer() ); httpConfig.setSecureScheme("https"); httpConfig.setSecurePort(httpsPort); httpConfig.setOutputBufferSize(32768); diff --git a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java index 192c556bd2a..df93f78fa83 100644 --- a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java +++ b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java @@ -34,6 +34,7 @@ public class ConfigKey { public static final String CATEGORY_ADVANCED = "Advanced"; public static final String CATEGORY_ALERT = "Alert"; + public static final String CATEGORY_NETWORK = "Network"; public enum Scope { Global, Zone, Cluster, StoragePool, Account, ManagementServer, ImageStore, Domain diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index b602ed2edbc..76a436f74d4 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -234,42 +234,42 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer @Inject private MessageBus messageBus; - private static final ConfigKey IntegrationAPIPort = new ConfigKey("Advanced" + private static final ConfigKey IntegrationAPIPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , Integer.class , "integration.api.port" , "0" , "Integration (unauthenticated) API port. To disable set it to 0 or negative." , false , ConfigKey.Scope.Global); - private static final ConfigKey ConcurrentSnapshotsThresholdPerHost = new ConfigKey("Advanced" + private static final ConfigKey ConcurrentSnapshotsThresholdPerHost = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , Long.class , "concurrent.snapshots.threshold.perhost" , null , "Limits number of snapshots that can be handled by the host concurrently; default is NULL - unlimited" , true // not sure if this is to be dynamic , ConfigKey.Scope.Global); - private static final ConfigKey EncodeApiResponse = new ConfigKey("Advanced" + private static final ConfigKey EncodeApiResponse = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , Boolean.class , "encode.api.response" , "false" , "Do URL encoding for the api response, false by default" , false , ConfigKey.Scope.Global); - static final ConfigKey JSONcontentType = new ConfigKey( "Advanced" + static final ConfigKey JSONcontentType = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , String.class , "json.content.type" , "application/json; charset=UTF-8" , "Http response content type for .js files (default is text/javascript)" , false , ConfigKey.Scope.Global); - static final ConfigKey EnableSecureSessionCookie = new ConfigKey("Advanced" + static final ConfigKey EnableSecureSessionCookie = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , Boolean.class , "enable.secure.session.cookie" , "false" , "Session cookie is marked as secure if this is enabled. Secure cookies only work when HTTPS is used." , false , ConfigKey.Scope.Global); - private static final ConfigKey JSONDefaultContentType = new ConfigKey ("Advanced" + private static final ConfigKey JSONDefaultContentType = new ConfigKey<> (ConfigKey.CATEGORY_ADVANCED , String.class , "json.content.type" , "application/json; charset=UTF-8" @@ -277,13 +277,34 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer , false , ConfigKey.Scope.Global); - private static final ConfigKey UseEventAccountInfo = new ConfigKey( "advanced" + private static final ConfigKey UseEventAccountInfo = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , Boolean.class , "event.accountinfo" , "false" , "use account info in event logging" , true , ConfigKey.Scope.Global); + static final ConfigKey useForwardHeader = new ConfigKey<>(ConfigKey.CATEGORY_NETWORK + , Boolean.class + , "proxy.header.verify" + , "false" + , "enables/disables checking of ipaddresses from a proxy set header. See \"proxy.header.names\" for the headers to allow." + , true + , ConfigKey.Scope.Global); + static final ConfigKey listOfForwardHeaders = new ConfigKey<>(ConfigKey.CATEGORY_NETWORK + , String.class + , "proxy.header.names" + , "X-Forwarded-For,HTTP_CLIENT_IP,HTTP_X_FORWARDED_FOR" + , "a list of names to check for allowed ipaddresses from a proxy set header. See \"proxy.cidr\" for the proxies allowed to set these headers." + , true + , ConfigKey.Scope.Global); + static final ConfigKey proxyForwardList = new ConfigKey<>(ConfigKey.CATEGORY_NETWORK + , String.class + , "proxy.cidr" + , "" + , "a list of cidrs for which \"proxy.header.names\" are honoured if the \"Remote_Addr\" is in this list." + , true + , ConfigKey.Scope.Global); @Override public boolean configure(final String name, final Map params) throws ConfigurationException { @@ -1499,7 +1520,10 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer ConcurrentSnapshotsThresholdPerHost, EncodeApiResponse, EnableSecureSessionCookie, - JSONDefaultContentType + JSONDefaultContentType, + proxyForwardList, + useForwardHeader, + listOfForwardHeaders }; } } diff --git a/server/src/main/java/com/cloud/api/ApiServlet.java b/server/src/main/java/com/cloud/api/ApiServlet.java index 626678649d7..f6f46419c04 100644 --- a/server/src/main/java/com/cloud/api/ApiServlet.java +++ b/server/src/main/java/com/cloud/api/ApiServlet.java @@ -21,7 +21,6 @@ import java.net.InetAddress; import java.net.URLDecoder; import java.net.UnknownHostException; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -69,13 +68,9 @@ import com.cloud.utils.db.EntityManager; import com.cloud.utils.net.NetUtils; @Component("apiServlet") -@SuppressWarnings("serial") public class ApiServlet extends HttpServlet { public static final Logger s_logger = Logger.getLogger(ApiServlet.class.getName()); private static final Logger s_accessLogger = Logger.getLogger("apiserver." + ApiServlet.class.getName()); - private final static List s_clientAddressHeaders = Collections - .unmodifiableList(Arrays.asList("X-Forwarded-For", - "HTTP_CLIENT_IP", "HTTP_X_FORWARDED_FOR", "Remote_Addr")); private static final String REPLACEMENT = "_"; private static final String LOG_REPLACEMENTS = "[\n\r\t]"; @@ -570,17 +565,39 @@ public class ApiServlet extends HttpServlet { } return false; } + boolean doUseForwardHeaders() { + return Boolean.TRUE.equals(ApiServer.useForwardHeader.value()); + } + String[] proxyNets() { + return ApiServer.proxyForwardList.value().split(","); + } //This method will try to get login IP of user even if servlet is behind reverseProxy or loadBalancer - public static InetAddress getClientAddress(final HttpServletRequest request) throws UnknownHostException { - for(final String header : s_clientAddressHeaders) { - final String ip = getCorrectIPAddress(request.getHeader(header)); - if (ip != null) { - return InetAddress.getByName(ip); - } + public InetAddress getClientAddress(final HttpServletRequest request) throws UnknownHostException { + String ip = null; + InetAddress pretender = InetAddress.getByName(request.getRemoteAddr()); + if(doUseForwardHeaders()) { + if (NetUtils.isIpInCidrList(pretender, proxyNets())) { + for (String header : getClientAddressHeaders()) { + header = header.trim(); + ip = getCorrectIPAddress(request.getHeader(header)); + if (StringUtils.isNotBlank(ip)) { + s_logger.debug(String.format("found ip %s in header %s ", ip, header)); + break; + } + } // no address found in header so ip is blank and use remote addr + } // else not an allowed proxy address, ip is blank and use remote addr + } + if (StringUtils.isBlank(ip)) { + s_logger.trace(String.format("no ip found in headers, returning remote address %s.", pretender.getHostAddress())); + return pretender; } - return InetAddress.getByName(request.getRemoteAddr()); + return InetAddress.getByName(ip); + } + + private String[] getClientAddressHeaders() { + return ApiServer.listOfForwardHeaders.value().split(","); } private static String getCorrectIPAddress(String ip) { diff --git a/server/src/test/java/com/cloud/api/ApiServletTest.java b/server/src/test/java/com/cloud/api/ApiServletTest.java index cefbbe2d630..4d4f0a12098 100644 --- a/server/src/test/java/com/cloud/api/ApiServletTest.java +++ b/server/src/test/java/com/cloud/api/ApiServletTest.java @@ -97,15 +97,17 @@ public class ApiServletTest { @Mock AccountService accountMgr; + @Mock ConfigKey useForwardHeader; StringWriter responseWriter; ApiServlet servlet; - + ApiServlet spyServlet; @SuppressWarnings("unchecked") @Before public void setup() throws SecurityException, NoSuchFieldException, IllegalArgumentException, IllegalAccessException, IOException, UnknownHostException { servlet = new ApiServlet(); + spyServlet = Mockito.spy(servlet); responseWriter = new StringWriter(); Mockito.when(response.getWriter()).thenReturn( new PrintWriter(responseWriter)); @@ -259,32 +261,43 @@ public class ApiServletTest { @Test public void getClientAddressWithXForwardedFor() throws UnknownHostException { + String[] proxynet = {"127.0.0.0/8"}; + Mockito.when(spyServlet.proxyNets()).thenReturn(proxynet); + Mockito.when(spyServlet.doUseForwardHeaders()).thenReturn(true); Mockito.when(request.getHeader(Mockito.eq("X-Forwarded-For"))).thenReturn("192.168.1.1"); - Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request)); + Assert.assertEquals(InetAddress.getByName("192.168.1.1"), spyServlet.getClientAddress(request)); } @Test public void getClientAddressWithHttpXForwardedFor() throws UnknownHostException { + String[] proxynet = {"127.0.0.0/8"}; + Mockito.when(spyServlet.proxyNets()).thenReturn(proxynet); + Mockito.when(spyServlet.doUseForwardHeaders()).thenReturn(true); Mockito.when(request.getHeader(Mockito.eq("HTTP_X_FORWARDED_FOR"))).thenReturn("192.168.1.1"); - Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request)); + Assert.assertEquals(InetAddress.getByName("192.168.1.1"), spyServlet.getClientAddress(request)); } @Test - public void getClientAddressWithXRemoteAddr() throws UnknownHostException { - Mockito.when(request.getHeader(Mockito.eq("Remote_Addr"))).thenReturn("192.168.1.1"); - Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request)); + public void getClientAddressWithRemoteAddr() throws UnknownHostException { + String[] proxynet = {"127.0.0.0/8"}; + Mockito.when(spyServlet.proxyNets()).thenReturn(proxynet); + Mockito.when(spyServlet.doUseForwardHeaders()).thenReturn(true); + Assert.assertEquals(InetAddress.getByName("127.0.0.1"), spyServlet.getClientAddress(request)); } @Test public void getClientAddressWithHttpClientIp() throws UnknownHostException { + String[] proxynet = {"127.0.0.0/8"}; + Mockito.when(spyServlet.proxyNets()).thenReturn(proxynet); + Mockito.when(spyServlet.doUseForwardHeaders()).thenReturn(true); Mockito.when(request.getHeader(Mockito.eq("HTTP_CLIENT_IP"))).thenReturn("192.168.1.1"); - Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request)); + Assert.assertEquals(InetAddress.getByName("192.168.1.1"), spyServlet.getClientAddress(request)); } @Test public void getClientAddressDefault() throws UnknownHostException { Mockito.when(request.getRemoteAddr()).thenReturn("127.0.0.1"); - Assert.assertEquals(InetAddress.getByName("127.0.0.1"), ApiServlet.getClientAddress(request)); + Assert.assertEquals(InetAddress.getByName("127.0.0.1"), spyServlet.getClientAddress(request)); } @Test diff --git a/utils/src/main/java/com/cloud/utils/StringUtils.java b/utils/src/main/java/com/cloud/utils/StringUtils.java index fd4f7aba698..01b6f833271 100644 --- a/utils/src/main/java/com/cloud/utils/StringUtils.java +++ b/utils/src/main/java/com/cloud/utils/StringUtils.java @@ -31,7 +31,7 @@ import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; -public class StringUtils { +public class StringUtils extends org.apache.commons.lang3.StringUtils { private static final char[] hexChar = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'}; private static Charset preferredACSCharset; From 939d0b9011aef846c2f3a093aed480193d1d45b9 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 27 Mar 2024 10:07:47 +0100 Subject: [PATCH 11/13] 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(); + } } } } From ff3e9bd821f9d3e528b092fa31f338a73d2afc54 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 26 Mar 2024 11:36:01 +0530 Subject: [PATCH 12/13] 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(); + } } } } From 7a9985b42bfdf43efb1a47f02ab58466936472ac Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 4 Apr 2024 11:25:29 +0200 Subject: [PATCH 13/13] upgrade: add unit tests from/to a security release (#8870) * upgrade: add unit tests from/to a security release * upgrade: add unit test from a old security release to a new security release --- .../upgrade/DatabaseUpgradeCheckerTest.java | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java b/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java index d7ef18a4f32..1a9372f73ea 100644 --- a/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java +++ b/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java @@ -223,4 +223,87 @@ public class DatabaseUpgradeCheckerTest { assertEquals("We should have 1 upgrade step", 1, upgrades.length); assertTrue(upgrades[0] instanceof NoopDbUpgrade); } + + @Test + public void testCalculateUpgradePathFrom41800toNextSecurityRelease() { + + final CloudStackVersion dbVersion = CloudStackVersion.parse("4.18.0.0"); + assertNotNull(dbVersion); + + final DatabaseUpgradeChecker checker = new DatabaseUpgradeChecker(); + final CloudStackVersion currentVersion = checker.getLatestVersion(); + assertNotNull(currentVersion); + + final DbUpgrade[] upgrades = checker.calculateUpgradePath(dbVersion, currentVersion); + assertNotNull(upgrades); + + final CloudStackVersion nextSecurityRelease = CloudStackVersion.parse(currentVersion.getMajorRelease() + "." + + currentVersion.getMinorRelease() + "." + + currentVersion.getPatchRelease() + "." + + (currentVersion.getSecurityRelease() + 1)); + assertNotNull(nextSecurityRelease); + + final DbUpgrade[] upgradesToNext = checker.calculateUpgradePath(dbVersion, nextSecurityRelease); + assertNotNull(upgradesToNext); + + assertEquals(upgrades.length + 1, upgradesToNext.length); + assertTrue(upgradesToNext[upgradesToNext.length - 1] instanceof NoopDbUpgrade); + } + + @Test + public void testCalculateUpgradePathFromSecurityReleaseToLatest() { + + final CloudStackVersion dbVersion = CloudStackVersion.parse("4.17.2.0"); // a EOL version + assertNotNull(dbVersion); + + final CloudStackVersion oldSecurityRelease = CloudStackVersion.parse(dbVersion.getMajorRelease() + "." + + dbVersion.getMinorRelease() + "." + + dbVersion.getPatchRelease() + "." + + (dbVersion.getSecurityRelease() + 100)); + assertNotNull(oldSecurityRelease); // fake security release 4.17.2.100 + + final DatabaseUpgradeChecker checker = new DatabaseUpgradeChecker(); + final CloudStackVersion currentVersion = checker.getLatestVersion(); + assertNotNull(currentVersion); + + final DbUpgrade[] upgrades = checker.calculateUpgradePath(dbVersion, currentVersion); + assertNotNull(upgrades); + + final DbUpgrade[] upgradesFromSecurityRelease = checker.calculateUpgradePath(oldSecurityRelease, currentVersion); + assertNotNull(upgradesFromSecurityRelease); + + assertEquals("The upgrade paths should be the same", upgrades.length, upgradesFromSecurityRelease.length); + } + + @Test + public void testCalculateUpgradePathFromSecurityReleaseToNextSecurityRelease() { + + final CloudStackVersion dbVersion = CloudStackVersion.parse("4.17.2.0"); // a EOL version + assertNotNull(dbVersion); + + final CloudStackVersion oldSecurityRelease = CloudStackVersion.parse(dbVersion.getMajorRelease() + "." + + dbVersion.getMinorRelease() + "." + + dbVersion.getPatchRelease() + "." + + (dbVersion.getSecurityRelease() + 100)); + assertNotNull(oldSecurityRelease); // fake security release 4.17.2.100 + + final DatabaseUpgradeChecker checker = new DatabaseUpgradeChecker(); + final CloudStackVersion currentVersion = checker.getLatestVersion(); + assertNotNull(currentVersion); + + final CloudStackVersion nextSecurityRelease = CloudStackVersion.parse(currentVersion.getMajorRelease() + "." + + currentVersion.getMinorRelease() + "." + + currentVersion.getPatchRelease() + "." + + (currentVersion.getSecurityRelease() + 1)); + assertNotNull(nextSecurityRelease); // fake security release + + final DbUpgrade[] upgrades = checker.calculateUpgradePath(dbVersion, currentVersion); + assertNotNull(upgrades); + + final DbUpgrade[] upgradesFromSecurityReleaseToNext = checker.calculateUpgradePath(oldSecurityRelease, nextSecurityRelease); + assertNotNull(upgradesFromSecurityReleaseToNext); + + assertEquals(upgrades.length + 1, upgradesFromSecurityReleaseToNext.length); + assertTrue(upgradesFromSecurityReleaseToNext[upgradesFromSecurityReleaseToNext.length - 1] instanceof NoopDbUpgrade); + } }