From c1797d31ee9dae95984421afc78f34af611558e7 Mon Sep 17 00:00:00 2001 From: "cong.xie" Date: Mon, 22 Dec 2025 12:21:57 -0500 Subject: [PATCH 1/7] fix gcs tests --- quickwit/quickwit-storage/src/lib.rs | 8 ++++++- .../opendal_storage/google_cloud_storage.rs | 22 ++++++++++++++++++- .../tests/google_cloud_storage.rs | 18 +++++++++++---- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/quickwit/quickwit-storage/src/lib.rs b/quickwit/quickwit-storage/src/lib.rs index f808ac83286..fd0f25c40d7 100644 --- a/quickwit/quickwit-storage/src/lib.rs +++ b/quickwit/quickwit-storage/src/lib.rs @@ -174,7 +174,13 @@ pub(crate) mod test_suite { .get_slice(Path::new("missingfile"), 0..3) .await .map_err(|err| err.kind()); - assert!(matches!(err, Err(StorageErrorKind::NotFound))); + + // Accept both NotFound and Io errors for fake GCS server compatibility + // The fake GCS server returns errors that OpenDAL maps to Io instead of NotFound + assert!(matches!( + err, + Err(StorageErrorKind::NotFound) | Err(StorageErrorKind::Io) + )); Ok(()) } diff --git a/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs b/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs index fc022fd1e08..52882323f6d 100644 --- a/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs +++ b/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs @@ -58,6 +58,23 @@ pub mod test_config_helpers { /// URL of the local GCP emulator. pub const LOCAL_GCP_EMULATOR_ENDPOINT: &str = "http://127.0.0.1:4443"; + /// Dummy token loader for testing with fake GCS server. + #[derive(Debug)] + pub struct DummyTokenLoader; + + impl DummyTokenLoader { + /// Creates a new DummyTokenLoader instance. + pub fn new() -> Self { + Self + } + + /// Loads a dummy token for use with fake GCS server. + /// Returns a dummy token compatible with reqsign GoogleSigner. + pub async fn load(&self, _: reqwest::Client) -> anyhow::Result> { + Ok(Some("dummy".to_string())) + } + } + /// Creates a storage connecting to a local emulated google cloud storage. pub fn new_emulated_google_cloud_storage( uri: &Uri, @@ -67,7 +84,10 @@ pub mod test_config_helpers { let cfg = opendal::services::Gcs::default() .bucket(&bucket) .root(&root.to_string_lossy()) - .endpoint(LOCAL_GCP_EMULATOR_ENDPOINT); + .endpoint(LOCAL_GCP_EMULATOR_ENDPOINT) + .allow_anonymous() // Disable authentication for fake GCS server + .disable_config_load() // Disable loading credentials from environment + .disable_vm_metadata(); // Disable GCE metadata server requests let store = OpendalStorage::new_google_cloud_storage(uri.clone(), cfg)?; Ok(store) } diff --git a/quickwit/quickwit-storage/tests/google_cloud_storage.rs b/quickwit/quickwit-storage/tests/google_cloud_storage.rs index c87dfc6b2d3..bd9e7d35020 100644 --- a/quickwit/quickwit-storage/tests/google_cloud_storage.rs +++ b/quickwit/quickwit-storage/tests/google_cloud_storage.rs @@ -25,11 +25,18 @@ mod gcp_storage_test_suite { use quickwit_common::setup_logging_for_tests; use quickwit_common::uri::Uri; use quickwit_storage::test_config_helpers::{ - LOCAL_GCP_EMULATOR_ENDPOINT, new_emulated_google_cloud_storage, + DummyTokenLoader, LOCAL_GCP_EMULATOR_ENDPOINT, new_emulated_google_cloud_storage, }; pub async fn sign_gcs_request(req: &mut reqwest::Request) -> anyhow::Result<()> { - let signer = reqsign::google::default_signer("storage"); + let client = reqwest::Client::new(); + let token = DummyTokenLoader::new() + .load(client.clone()) + .await? + .ok_or_else(|| anyhow::anyhow!("Failed to obtain authentication token"))?; + + // Note: Using manual token approach since reqsign 0.18 API changed + let _signer = reqsign::google::default_signer("storage"); // Create http::Request and extract parts for signing let http_req = http::Request::builder() @@ -43,8 +50,11 @@ mod gcp_storage_test_suite { // Copy headers from reqwest request parts.headers = req.headers().clone(); - // Sign the request parts - signer.sign(&mut parts, None).await?; + // For dummy token, manually add authorization header + parts.headers.insert( + http::header::AUTHORIZATION, + http::HeaderValue::from_str(&format!("Bearer {}", token))? + ); // Update the original request with signed headers let headers = req.headers_mut(); From 90e1e6b2dc7c88299950e0ec98aad46d7e044675 Mon Sep 17 00:00:00 2001 From: "cong.xie" Date: Mon, 22 Dec 2025 13:49:37 -0500 Subject: [PATCH 2/7] update the gcs tests --- quickwit/quickwit-storage/src/lib.rs | 9 ++--- .../opendal_storage/google_cloud_storage.rs | 4 +-- .../tests/google_cloud_storage.rs | 35 +++++-------------- 3 files changed, 13 insertions(+), 35 deletions(-) diff --git a/quickwit/quickwit-storage/src/lib.rs b/quickwit/quickwit-storage/src/lib.rs index fd0f25c40d7..0f02fe60f71 100644 --- a/quickwit/quickwit-storage/src/lib.rs +++ b/quickwit/quickwit-storage/src/lib.rs @@ -174,13 +174,8 @@ pub(crate) mod test_suite { .get_slice(Path::new("missingfile"), 0..3) .await .map_err(|err| err.kind()); - - // Accept both NotFound and Io errors for fake GCS server compatibility - // The fake GCS server returns errors that OpenDAL maps to Io instead of NotFound - assert!(matches!( - err, - Err(StorageErrorKind::NotFound) | Err(StorageErrorKind::Io) - )); + + assert!(matches!(err, Err(StorageErrorKind::NotFound))); Ok(()) } diff --git a/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs b/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs index 52882323f6d..baf9fb5f637 100644 --- a/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs +++ b/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs @@ -59,6 +59,7 @@ pub mod test_config_helpers { pub const LOCAL_GCP_EMULATOR_ENDPOINT: &str = "http://127.0.0.1:4443"; /// Dummy token loader for testing with fake GCS server. + /// This maintains compatibility with the original test behavior. #[derive(Debug)] pub struct DummyTokenLoader; @@ -69,7 +70,7 @@ pub mod test_config_helpers { } /// Loads a dummy token for use with fake GCS server. - /// Returns a dummy token compatible with reqsign GoogleSigner. + /// Returns the same dummy token as the original implementation. pub async fn load(&self, _: reqwest::Client) -> anyhow::Result> { Ok(Some("dummy".to_string())) } @@ -86,7 +87,6 @@ pub mod test_config_helpers { .root(&root.to_string_lossy()) .endpoint(LOCAL_GCP_EMULATOR_ENDPOINT) .allow_anonymous() // Disable authentication for fake GCS server - .disable_config_load() // Disable loading credentials from environment .disable_vm_metadata(); // Disable GCE metadata server requests let store = OpendalStorage::new_google_cloud_storage(uri.clone(), cfg)?; Ok(store) diff --git a/quickwit/quickwit-storage/tests/google_cloud_storage.rs b/quickwit/quickwit-storage/tests/google_cloud_storage.rs index bd9e7d35020..90794b6907b 100644 --- a/quickwit/quickwit-storage/tests/google_cloud_storage.rs +++ b/quickwit/quickwit-storage/tests/google_cloud_storage.rs @@ -35,34 +35,17 @@ mod gcp_storage_test_suite { .await? .ok_or_else(|| anyhow::anyhow!("Failed to obtain authentication token"))?; - // Note: Using manual token approach since reqsign 0.18 API changed - let _signer = reqsign::google::default_signer("storage"); - - // Create http::Request and extract parts for signing - let http_req = http::Request::builder() - .method(req.method()) - .uri(req.url().as_str()) - .version(http::Version::HTTP_11) - .body(())?; - - let (mut parts, _body) = http_req.into_parts(); - - // Copy headers from reqwest request - parts.headers = req.headers().clone(); - - // For dummy token, manually add authorization header - parts.headers.insert( - http::header::AUTHORIZATION, - http::HeaderValue::from_str(&format!("Bearer {}", token))? + // In reqsign 0.18, we need to check if there's a way to sign with a custom token + // Let's try to find the equivalent of the old GoogleSigner::new().sign() pattern + + // The old approach used GoogleSigner::new().sign(req, &token) + // But it seems reqsign 0.18 changed the API structure completely + // For now, let's fall back to manual header approach until we find the right API + req.headers_mut().insert( + reqwest::header::AUTHORIZATION, + reqwest::header::HeaderValue::from_str(&format!("Bearer {}", token))? ); - // Update the original request with signed headers - let headers = req.headers_mut(); - headers.clear(); - for (key, value) in &parts.headers { - headers.insert(key.clone(), value.clone()); - } - Ok(()) } From f9f53a3ffd198e88ed42a30cd73c22056ac0cbf6 Mon Sep 17 00:00:00 2001 From: "cong.xie" Date: Mon, 22 Dec 2025 13:54:13 -0500 Subject: [PATCH 3/7] remove comments --- quickwit/quickwit-storage/tests/google_cloud_storage.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/quickwit/quickwit-storage/tests/google_cloud_storage.rs b/quickwit/quickwit-storage/tests/google_cloud_storage.rs index 90794b6907b..4052b213bf9 100644 --- a/quickwit/quickwit-storage/tests/google_cloud_storage.rs +++ b/quickwit/quickwit-storage/tests/google_cloud_storage.rs @@ -35,9 +35,6 @@ mod gcp_storage_test_suite { .await? .ok_or_else(|| anyhow::anyhow!("Failed to obtain authentication token"))?; - // In reqsign 0.18, we need to check if there's a way to sign with a custom token - // Let's try to find the equivalent of the old GoogleSigner::new().sign() pattern - // The old approach used GoogleSigner::new().sign(req, &token) // But it seems reqsign 0.18 changed the API structure completely // For now, let's fall back to manual header approach until we find the right API From 11854a452c3e42f07851d0dfaea58a6a6474711c Mon Sep 17 00:00:00 2001 From: "cong.xie" Date: Mon, 22 Dec 2025 15:07:31 -0500 Subject: [PATCH 4/7] add default impl --- .../src/opendal_storage/google_cloud_storage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs b/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs index baf9fb5f637..4a310326215 100644 --- a/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs +++ b/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs @@ -60,7 +60,7 @@ pub mod test_config_helpers { /// Dummy token loader for testing with fake GCS server. /// This maintains compatibility with the original test behavior. - #[derive(Debug)] + #[derive(Debug, Default)] pub struct DummyTokenLoader; impl DummyTokenLoader { From 863b383907fca35d5c95171bf4c69bce7651ea25 Mon Sep 17 00:00:00 2001 From: "cong.xie" Date: Mon, 22 Dec 2025 15:08:00 -0500 Subject: [PATCH 5/7] fix fmt --- quickwit/quickwit-storage/tests/google_cloud_storage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quickwit/quickwit-storage/tests/google_cloud_storage.rs b/quickwit/quickwit-storage/tests/google_cloud_storage.rs index 4052b213bf9..1b6c6176bee 100644 --- a/quickwit/quickwit-storage/tests/google_cloud_storage.rs +++ b/quickwit/quickwit-storage/tests/google_cloud_storage.rs @@ -40,7 +40,7 @@ mod gcp_storage_test_suite { // For now, let's fall back to manual header approach until we find the right API req.headers_mut().insert( reqwest::header::AUTHORIZATION, - reqwest::header::HeaderValue::from_str(&format!("Bearer {}", token))? + reqwest::header::HeaderValue::from_str(&format!("Bearer {}", token))?, ); Ok(()) From 55a729ca149910d6c3670d06b20fb75e2be9a03c Mon Sep 17 00:00:00 2001 From: "cong.xie" Date: Tue, 23 Dec 2025 11:15:53 -0500 Subject: [PATCH 6/7] address comments --- .../opendal_storage/google_cloud_storage.rs | 20 +------------------ .../tests/google_cloud_storage.rs | 14 ++----------- 2 files changed, 3 insertions(+), 31 deletions(-) diff --git a/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs b/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs index 4a310326215..1c89b51deaf 100644 --- a/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs +++ b/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs @@ -57,25 +57,7 @@ pub mod test_config_helpers { /// URL of the local GCP emulator. pub const LOCAL_GCP_EMULATOR_ENDPOINT: &str = "http://127.0.0.1:4443"; - - /// Dummy token loader for testing with fake GCS server. - /// This maintains compatibility with the original test behavior. - #[derive(Debug, Default)] - pub struct DummyTokenLoader; - - impl DummyTokenLoader { - /// Creates a new DummyTokenLoader instance. - pub fn new() -> Self { - Self - } - - /// Loads a dummy token for use with fake GCS server. - /// Returns the same dummy token as the original implementation. - pub async fn load(&self, _: reqwest::Client) -> anyhow::Result> { - Ok(Some("dummy".to_string())) - } - } - + /// Creates a storage connecting to a local emulated google cloud storage. pub fn new_emulated_google_cloud_storage( uri: &Uri, diff --git a/quickwit/quickwit-storage/tests/google_cloud_storage.rs b/quickwit/quickwit-storage/tests/google_cloud_storage.rs index 1b6c6176bee..62909fbe9f1 100644 --- a/quickwit/quickwit-storage/tests/google_cloud_storage.rs +++ b/quickwit/quickwit-storage/tests/google_cloud_storage.rs @@ -25,24 +25,14 @@ mod gcp_storage_test_suite { use quickwit_common::setup_logging_for_tests; use quickwit_common::uri::Uri; use quickwit_storage::test_config_helpers::{ - DummyTokenLoader, LOCAL_GCP_EMULATOR_ENDPOINT, new_emulated_google_cloud_storage, + LOCAL_GCP_EMULATOR_ENDPOINT, new_emulated_google_cloud_storage, }; pub async fn sign_gcs_request(req: &mut reqwest::Request) -> anyhow::Result<()> { - let client = reqwest::Client::new(); - let token = DummyTokenLoader::new() - .load(client.clone()) - .await? - .ok_or_else(|| anyhow::anyhow!("Failed to obtain authentication token"))?; - - // The old approach used GoogleSigner::new().sign(req, &token) - // But it seems reqsign 0.18 changed the API structure completely - // For now, let's fall back to manual header approach until we find the right API req.headers_mut().insert( reqwest::header::AUTHORIZATION, - reqwest::header::HeaderValue::from_str(&format!("Bearer {}", token))?, + reqwest::header::HeaderValue::from_str("Bearer dummy")?, ); - Ok(()) } From a4b500c1e99ed39f18f16ac3b315a1c927286ce6 Mon Sep 17 00:00:00 2001 From: "cong.xie" Date: Tue, 23 Dec 2025 13:19:33 -0500 Subject: [PATCH 7/7] fix tests --- quickwit/quickwit-storage/src/lib.rs | 1 - .../src/opendal_storage/google_cloud_storage.rs | 1 - quickwit/quickwit-storage/tests/google_cloud_storage.rs | 7 +++---- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/quickwit/quickwit-storage/src/lib.rs b/quickwit/quickwit-storage/src/lib.rs index 0f02fe60f71..f808ac83286 100644 --- a/quickwit/quickwit-storage/src/lib.rs +++ b/quickwit/quickwit-storage/src/lib.rs @@ -174,7 +174,6 @@ pub(crate) mod test_suite { .get_slice(Path::new("missingfile"), 0..3) .await .map_err(|err| err.kind()); - assert!(matches!(err, Err(StorageErrorKind::NotFound))); Ok(()) } diff --git a/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs b/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs index 1c89b51deaf..9f174f0a744 100644 --- a/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs +++ b/quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs @@ -57,7 +57,6 @@ pub mod test_config_helpers { /// URL of the local GCP emulator. pub const LOCAL_GCP_EMULATOR_ENDPOINT: &str = "http://127.0.0.1:4443"; - /// Creates a storage connecting to a local emulated google cloud storage. pub fn new_emulated_google_cloud_storage( uri: &Uri, diff --git a/quickwit/quickwit-storage/tests/google_cloud_storage.rs b/quickwit/quickwit-storage/tests/google_cloud_storage.rs index 62909fbe9f1..4b916b44f4f 100644 --- a/quickwit/quickwit-storage/tests/google_cloud_storage.rs +++ b/quickwit/quickwit-storage/tests/google_cloud_storage.rs @@ -28,12 +28,11 @@ mod gcp_storage_test_suite { LOCAL_GCP_EMULATOR_ENDPOINT, new_emulated_google_cloud_storage, }; - pub async fn sign_gcs_request(req: &mut reqwest::Request) -> anyhow::Result<()> { + pub fn sign_gcs_request(req: &mut reqwest::Request) { req.headers_mut().insert( reqwest::header::AUTHORIZATION, - reqwest::header::HeaderValue::from_str("Bearer dummy")?, + reqwest::header::HeaderValue::from_str("Bearer dummy").unwrap(), ); - Ok(()) } async fn create_gcs_bucket(bucket_name: &str) -> anyhow::Result<()> { @@ -47,7 +46,7 @@ mod gcp_storage_test_suite { .header(reqwest::header::CONTENT_TYPE, "application/json") .build()?; - sign_gcs_request(&mut request).await?; + sign_gcs_request(&mut request); let response = client.execute(request).await?;