Merge branch 'eichhorl/default-redirects' into 'master'

chore(http_utils): CON-1181 Remove manual redirect logic

`reqwest` follows up to 10 redirects by default 

See merge request dfinity-lab/public/ic!16622

rs/http_utils/src/file_downloader.rs

@@ -19,32 +19,21 @@ pub struct FileDownloader {
     /// This is a timeout that is applied to the downloading each chunk that is
     /// yielded, not to the entire downloading of the file.
     timeout: Duration,
-    allow_redirects: bool,
 }
 
 impl FileDownloader {
     pub fn new(logger: Option<ReplicaLogger>) -> Self {
-        Self::new_with_timeout(logger, Duration::from_secs(15), false)
+        Self::new_with_timeout(logger, Duration::from_secs(15))
     }
 
-    pub fn new_with_timeout(
-        logger: Option<ReplicaLogger>,
-        timeout: Duration,
-        allow_redirects: bool,
-    ) -> Self {
+    pub fn new_with_timeout(logger: Option<ReplicaLogger>, timeout: Duration) -> Self {
         Self {
             client: Client::new(),
             logger,
             timeout,
-            allow_redirects,
         }
     }
 
-    pub fn follow_redirects(mut self) -> FileDownloader {
-        self.allow_redirects = true;
-        self
-    }
-
     /// Download a .tar.gz file from `url`, verify its hash if given, and
     /// extract the file into `target_dir`
     pub async fn download_and_extract_tar_gz(
@@ -139,38 +128,13 @@ impl FileDownloader {
         let response = self.client.get(url).timeout(self.timeout).send().await?;
 
         if response.status().is_success() {
-            return Ok(response);
-        }
-
-        if response.status().is_redirection() && self.allow_redirects {
-            match response.headers().get(http::header::LOCATION) {
-                Some(url) => {
-                    let url = url.to_str().unwrap();
-                    let response = self.client.get(url).timeout(self.timeout).send().await?;
-                    if response.status().is_success() {
-                        return Ok(response);
-                    }
-                    return Err(FileDownloadError::HttpError(HttpError::NonSuccessResponse(
-                        Method::GET,
-                        response,
-                    )));
-                }
-                None => {
-                    return Err(FileDownloadError::HttpError(
-                        HttpError::RedirectMissingHeader(
-                            Method::GET,
-                            http::header::LOCATION,
-                            response.status(),
-                        ),
-                    ))
-                }
-            }
+            Ok(response)
+        } else {
+            Err(FileDownloadError::HttpError(HttpError::NonSuccessResponse(
+                Method::GET,
+                response,
+            )))
         }
-
-        Err(FileDownloadError::HttpError(HttpError::NonSuccessResponse(
-            Method::GET,
-            response,
-        )))
     }
 
     /// Stream the bytes of a given HTTP response body into the given file
@@ -341,11 +305,6 @@ impl fmt::Display for FileDownloadError {
                 "Received non-success response from endpoint: method: {}, uri: {}, remote_addr: {:?}, status_code: {}, headers: {:?}",
                 method.as_str(), response.url(), response.remote_addr(), response.status(), response.headers()
             ),
-            FileDownloadError::HttpError(HttpError::RedirectMissingHeader(method, header, status_code)) => write!(
-                f,
-                "Received a redirect response from endpoint but a header is missing: method: {}, header: {}, status_code: {}",
-                method.as_str(), header, status_code
-            ),
             FileDownloadError::FileHashMismatchError { computed_hash, expected_hash, file_path } =>
                 write!(
                     f,
@@ -380,9 +339,6 @@ pub enum HttpError {
 
     /// A non-success HTTP response was received from the given URI
     NonSuccessResponse(http::Method, Response),
-
-    /// A redirect response without a required header
-    RedirectMissingHeader(http::Method, http::HeaderName, http::StatusCode),
 }
 
 #[cfg(test)]

rs/orchestrator/image_upgrader/src/image_upgrader.rs

@@ -158,11 +158,8 @@ pub trait ImageUpgrader<V: Clone + Debug + PartialEq + Eq + Send + Sync, R: Send
                 "Request to download image {:?} from {}",
                 version, release_package_url
             );
-            let file_downloader = FileDownloader::new_with_timeout(
-                Some(self.log().clone()),
-                Duration::from_secs(60),
-                /* allow_redirects = */ true,
-            );
+            let file_downloader =
+                FileDownloader::new_with_timeout(Some(self.log().clone()), Duration::from_secs(60));
             let start_time = std::time::Instant::now();
             let download_result = file_downloader
                 .download_file(release_package_url, self.image_path(), hash.clone())

rs/orchestrator/src/upgrade.rs

@@ -276,7 +276,7 @@ impl Upgrade {
                     registry_store_uri.uri,
                     registry_store_uri.hash,
                 );
-                let downloader = FileDownloader::new(Some(self.logger.clone())).follow_redirects();
+                let downloader = FileDownloader::new(Some(self.logger.clone()));
                 let local_store_location = tempfile::tempdir()
                     .expect("temporary location for local store download could not be created")
                     .into_path();

rs/registry/admin/src/main.rs

@@ -4731,7 +4731,7 @@ async fn main() {
 
             // Download the IC-OS upgrade, do not check sha256 yet, we will do that
             // explicitly later
-            let file_downloader = FileDownloader::new(None).follow_redirects();
+            let file_downloader = FileDownloader::new(None);
 
             let mut result = Err(anyhow!("Download of release package failed."));
             for url in version.release_package_urls.iter() {

rs/tests/src/orchestrator/utils/upgrade.rs

@@ -76,7 +76,7 @@ pub(crate) async fn fetch_update_file_sha256(
     let mut tmp_file = tmp_dir.clone();
     tmp_file.push("SHA256.txt");
 
-    let file_downloader = FileDownloader::new(None).follow_redirects();
+    let file_downloader = FileDownloader::new(None);
     file_downloader
         .download_file(&sha_url, &tmp_file, None)
         .await