Merge pull request #2480 from input-output-hk/djo/2460/fix-acl

aggregator: fix setting public access to files uploaded to GCP storage

Author: Alenar

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "mithril-aggregator"
-version = "0.7.48"
+version = "0.7.49"
 description = "A Mithril Aggregator server"
 authors = { workspace = true }
 edition = { workspace = true }
@@ -36,6 +36,7 @@ mithril-resource-pool = { path = "../internal/mithril-resource-pool" }
 mithril-signed-entity-lock = { path = "../internal/signed-entity/mithril-signed-entity-lock" }
 mithril-signed-entity-preloader = { path = "../internal/signed-entity/mithril-signed-entity-preloader" }
 paste = "1.0.15"
+percent-encoding = "2.3.1"
 rayon = { workspace = true }
 regex = "1.11.1"
 reqwest = { workspace = true, features = [

mithril-aggregator/Cargo.toml

@@ -16,7 +16,7 @@ use tokio_util::codec::{BytesCodec, FramedRead};
 use mithril_common::entities::FileUri;
 use mithril_common::StdResult;
 
-use crate::file_uploaders::cloud_uploader::CloudBackendUploader;
+use crate::file_uploaders::cloud_uploader::{gcp_percent_encode, CloudBackendUploader};
 use crate::file_uploaders::CloudRemotePath;
 
 /// Google Cloud Platform file uploader using `gcloud-storage` crate
@@ -133,13 +133,13 @@ impl CloudBackendUploader for GCloudBackendUploader {
             role: ObjectACLRole::READER,
         };
         info!(
-            self.logger,
-            "Updating acl for {remote_file_path}: {new_bucket_access_control:?}"
+            self.logger, "Updating acl for {remote_file_path}";
+            "inserted_acl" => ?new_bucket_access_control
         );
         self.storage_client
             .insert_object_access_control(&InsertObjectAccessControlRequest {
                 bucket: self.bucket.clone(),
-                object: remote_file_path.to_string(),
+                object: gcp_percent_encode(&remote_file_path.to_string()),
                 acl: new_bucket_access_control,
                 ..Default::default()
             })

mithril-aggregator/src/file_uploaders/cloud_uploader/gcloud_backend.rs

@@ -5,3 +5,16 @@ mod interface;
 pub use api::*;
 pub use gcloud_backend::*;
 pub use interface::*;
+
+use percent_encoding::{utf8_percent_encode, AsciiSet, NON_ALPHANUMERIC};
+
+const ENCODE_SET: &AsciiSet = &NON_ALPHANUMERIC
+    .remove(b'*')
+    .remove(b'-')
+    .remove(b'.')
+    .remove(b'_');
+
+/// Encode a string for use in a GCP URL, satisfying: https://cloud.google.com/storage/docs/request-endpoints#encoding
+pub fn gcp_percent_encode(input: &str) -> String {
+    utf8_percent_encode(input, ENCODE_SET).to_string()
+}

mithril-aggregator/src/file_uploaders/cloud_uploader/mod.rs

@@ -53,11 +53,11 @@ pub trait FileUploader: Sync + Send {
             nb_attempts += 1;
             match self.upload_without_retry(filepath).await {
                 Ok(result) => return Ok(result),
-                Err(_) if nb_attempts >= retry_policy.attempts => {
-                    return Err(anyhow::anyhow!(
-                        "Upload failed after {} attempts",
-                        nb_attempts
-                    ));
+                Err(e) if nb_attempts >= retry_policy.attempts => {
+                    return Err(anyhow::anyhow!(e).context(format!(
+                        "Upload failed after {nb_attempts} attempts. Uploaded file path: {}",
+                        filepath.display()
+                    )));
                 }
                 _ => tokio::time::sleep(retry_policy.delay_between_attempts).await,
             }

mithril-aggregator/src/file_uploaders/interface.rs

@@ -1,6 +1,6 @@
 [package]
 name = "mithril-client"
-version = "0.12.3"
+version = "0.12.4"
 description = "Mithril client library"
 authors = { workspace = true }
 edition = { workspace = true }

mithril-client/Cargo.toml

@@ -81,12 +81,10 @@ impl FileDownloader for RetryDownloader {
                 .await
             {
                 Ok(result) => return Ok(result),
-                Err(_) if nb_attempts >= retry_policy.attempts => {
-                    return Err(anyhow::anyhow!(
-                        "Download of location {:?} failed after {} attempts",
-                        location,
-                        nb_attempts
-                    ));
+                Err(e) if nb_attempts >= retry_policy.attempts => {
+                    return Err(anyhow::anyhow!(e).context(format!(
+                        "Download of location {location:?} failed after {nb_attempts} attempts",
+                    )));
                 }
                 _ => tokio::time::sleep(retry_policy.delay_between_attempts).await,
             }