From a36d94573eb54aba1ab3da30f644f62e574691cb Mon Sep 17 00:00:00 2001 From: David Myriel Date: Wed, 29 Jul 2026 11:23:23 -0400 Subject: [PATCH 1/6] feat(oss): support configurable S3 addressing style --- config/default.toml | 6 ++ .../examples/tigris_smoke.rs | 77 +++++++++++++++++++ docs/src/configuration/reference.md | 12 +++ docs/src/getting-started/on-demand-loading.md | 5 ++ src/cfg.rs | 16 ++++ src/setup/deps.rs | 1 + .../repository/backends/oss/client.rs | 55 ++++++++++++- .../repository/backends/oss/config.rs | 40 +++++++++- src/snapshot/repository/backends/oss/mod.rs | 1 + .../repository/backends/oss/repository.rs | 1 + 10 files changed, 210 insertions(+), 4 deletions(-) create mode 100644 crates/object-store-operator/examples/tigris_smoke.rs diff --git a/config/default.toml b/config/default.toml index 3dab300b..4ee28ede 100644 --- a/config/default.toml +++ b/config/default.toml @@ -148,6 +148,12 @@ p2p_enabled = true # endpoint = "https://oss-cn-beijing-internal.aliyuncs.com" # bucket = "agentenv-oss-validation" # region = "cn-beijing" +# # Bucket addressing style. Leave unset to auto-detect (Alibaba OSS and +# # bucket-in-endpoint hosts use virtual-host style; other endpoints default to +# # path style). Set "virtual" for providers that require virtual-host +# # addressing, e.g. AWS S3 new buckets, Cloudflare R2, or Tigris +# # (endpoint = "https://t3.storage.dev", region = "auto"). +# addressing_style = "virtual" # prefix = "validation/" # credential_process = "" # cache_max_size_gb = 10 diff --git a/crates/object-store-operator/examples/tigris_smoke.rs b/crates/object-store-operator/examples/tigris_smoke.rs new file mode 100644 index 00000000..b0ba313a --- /dev/null +++ b/crates/object-store-operator/examples/tigris_smoke.rs @@ -0,0 +1,77 @@ +//! End-to-end smoke test for the S3-compatible snapshot backend against a real +//! object store (e.g. Tigris). It exercises the SAME `build_object_store_operator` +//! path the OSS snapshot backend uses, so a successful round-trip proves that the +//! configured `addressing_style` actually reaches the wire. +//! +//! Usage: +//! SMOKE_BUCKET=my-bucket \ +//! SMOKE_ACCESS_KEY_ID=tid_xxx SMOKE_SECRET_ACCESS_KEY=tsec_xxx \ +//! cargo run -p object-store-operator --example tigris_smoke +//! +//! Env (defaults in brackets): +//! SMOKE_ENDPOINT [https://t3.storage.dev] +//! SMOKE_REGION [auto] +//! SMOKE_STYLE [virtual] -- "virtual" or "path" +//! SMOKE_BUCKET (required) +//! SMOKE_ACCESS_KEY_ID / SMOKE_SECRET_ACCESS_KEY (required) +//! +//! Tip: run once with SMOKE_STYLE=path against a modern Tigris bucket to see the +//! failure this fix addresses, then again with SMOKE_STYLE=virtual to see it pass. + +use object_store_operator::{ + build_object_store_operator, AddressingStyle, ObjectStoreOperatorConfig, ResolvedCredential, +}; + +fn env_or(key: &str, default: &str) -> String { + std::env::var(key).unwrap_or_else(|_| default.to_string()) +} + +fn env_req(key: &str) -> String { + std::env::var(key).unwrap_or_else(|_| panic!("missing required env var {key}")) +} + +#[tokio::main] +async fn main() -> anyhow::Result<()> { + let endpoint = env_or("SMOKE_ENDPOINT", "https://t3.storage.dev"); + let region = env_or("SMOKE_REGION", "auto"); + let style = match env_or("SMOKE_STYLE", "virtual").as_str() { + "virtual" => AddressingStyle::Virtual, + "path" => AddressingStyle::Path, + other => anyhow::bail!("SMOKE_STYLE must be 'virtual' or 'path', got '{other}'"), + }; + let bucket = env_req("SMOKE_BUCKET"); + let cred = ResolvedCredential::new( + env_req("SMOKE_ACCESS_KEY_ID"), + env_req("SMOKE_SECRET_ACCESS_KEY"), + None, + None, + )?; + + let config = ObjectStoreOperatorConfig { + bucket: bucket.clone(), + endpoint: endpoint.clone(), + region, + addressing_style: style.clone(), + timeout: None, + max_retries: None, + }; + + println!("endpoint={endpoint} bucket={bucket} addressing_style={style:?}"); + let op = build_object_store_operator(&config, Some(&cred))?; + + let key = "agentenv-smoke/roundtrip.txt"; + let payload = b"agentenv addressing-style smoke test".to_vec(); + + println!("-> write {key}"); + op.write(key, payload.clone()).await?; + + println!("-> read {key}"); + let got = op.read(key).await?.to_vec(); + anyhow::ensure!(got == payload, "read-back mismatch: object content differs"); + + println!("-> delete {key}"); + op.delete(key).await?; + + println!("OK: {style:?}-host round-trip succeeded against {endpoint}"); + Ok(()) +} diff --git a/docs/src/configuration/reference.md b/docs/src/configuration/reference.md index 0b7f730c..08a93ba2 100644 --- a/docs/src/configuration/reference.md +++ b/docs/src/configuration/reference.md @@ -375,6 +375,7 @@ OSS-backed snapshot repository configuration. This section is required when `sna | `access_key_secret` | string | unset | Static OSS access key secret. Required when `credential_process` is not set | | `security_token` | string | unset | Optional session token paired with static access key credentials | | `region` | string | none | Region passed to the S3-compatible object-store client; required for current OSS backend | +| `addressing_style` | string | auto-detect | Bucket addressing style, `"virtual"` or `"path"`. When unset, the backend auto-detects: Alibaba OSS and bucket-in-endpoint hosts use virtual-host style, other endpoints default to path style. Set `"virtual"` for S3-compatible providers that require virtual-host addressing, such as AWS S3 buckets created after the path-style deprecation, Cloudflare R2, or Tigris | | `cache_max_size_gb` | integer | `10` | Maximum size of the node-local OSS artifact cache in GiB | Notes: @@ -382,6 +383,17 @@ Notes: - `credential_process` and static access key settings are mutually exclusive in practice; when `credential_process` is set, the backend ignores static credential fields. - `credential_process` should be written as a portable argv-style command line. Avoid `$VAR`, backticks, `$(...)`, pipes, and shell builtins. - Although the config section is still named `oss`, the runtime path is implemented via a shared S3-compatible client, so `region` must be configured. +- Leave `addressing_style` unset for Alibaba OSS and MinIO. Set it to `"virtual"` for S3-compatible providers that only support virtual-host bucket addressing; path-style requests to those endpoints are rejected. + +For an S3-compatible provider that requires virtual-host addressing — for example [Tigris](https://www.tigrisdata.com/docs/) — set `addressing_style` explicitly: + +```toml +[backend.oss] +endpoint = "https://t3.storage.dev" +bucket = "agentenv-snapshots" +region = "auto" +addressing_style = "virtual" +``` Other path override: diff --git a/docs/src/getting-started/on-demand-loading.md b/docs/src/getting-started/on-demand-loading.md index 28fa1845..800d75dd 100644 --- a/docs/src/getting-started/on-demand-loading.md +++ b/docs/src/getting-started/on-demand-loading.md @@ -67,6 +67,11 @@ access_key_secret = "YOUR_ACCESS_KEY_SECRET" max_size_gb = 100 ``` +If your provider only supports virtual-host bucket addressing — for example +[Tigris](https://www.tigrisdata.com/docs/) or Cloudflare R2 — also set +`addressing_style = "virtual"`; see the +[configuration reference](../configuration/reference.md#backendoss) for details. + ## 3. Apply the configuration If AgentENV is running as a systemd service, restart it: diff --git a/src/cfg.rs b/src/cfg.rs index baf32196..5da48c7c 100644 --- a/src/cfg.rs +++ b/src/cfg.rs @@ -278,6 +278,20 @@ pub struct SnapshotImagePublishConfig { pub enabled: bool, } +/// Bucket addressing style for the S3-compatible snapshot backend. +/// +/// When unset, the backend auto-detects the style: Alibaba OSS and +/// bucket-in-endpoint hosts use virtual-host addressing, and everything else +/// falls back to path style. Set this explicitly for S3-compatible providers +/// that require virtual-host addressing, such as AWS S3 buckets created after +/// the path-style deprecation, Cloudflare R2, or Tigris. +#[derive(Debug, Deserialize, Clone, Copy, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum OssAddressingStyle { + Path, + Virtual, +} + #[derive(Debug, Deserialize, Clone)] pub struct OssBackendConfig { pub endpoint: String, @@ -289,6 +303,8 @@ pub struct OssBackendConfig { pub access_key_secret: Option, pub security_token: Option, pub region: Option, + #[serde(alias = "addressingStyle", alias = "addressing-style")] + pub addressing_style: Option, pub cache_max_size_gb: Option, } diff --git a/src/setup/deps.rs b/src/setup/deps.rs index 19dc2204..5b09a7a0 100644 --- a/src/setup/deps.rs +++ b/src/setup/deps.rs @@ -724,6 +724,7 @@ mod tests { access_key_secret: Some(" sk ".to_string()), security_token: Some(" token ".to_string()), region: Some(" cn-hangzhou ".to_string()), + addressing_style: None, cache_max_size_gb: Some(4), } } diff --git a/src/snapshot/repository/backends/oss/client.rs b/src/snapshot/repository/backends/oss/client.rs index 43aaf2b5..f31bef3f 100644 --- a/src/snapshot/repository/backends/oss/client.rs +++ b/src/snapshot/repository/backends/oss/client.rs @@ -34,10 +34,17 @@ impl OssClient { region: String, prefix: String, credential_source: CredentialSource, + addressing_override: Option, ) -> Result { + // An explicit config override wins; otherwise fall back to + // endpoint-based detection (Alibaba OSS / bucket-in-endpoint hosts). + let addressing_style = match addressing_override { + Some(style) => style, + None => detect_addressing_style(&endpoint, &bucket)?, + }; Ok(Self { operator_config: ObjectStoreOperatorConfig { - addressing_style: detect_addressing_style(&endpoint, &bucket)?, + addressing_style, bucket, endpoint, region, @@ -395,3 +402,49 @@ async fn upload_file_to_operator( fn io_error_to_opendal(error: std::io::Error, message: &'static str) -> OpenDalError { OpenDalError::new(OpenDalErrorKind::Unexpected, message).set_source(error) } + +#[cfg(test)] +mod tests { + use super::{detect_addressing_style, OssClient}; + use object_store_operator::{AddressingStyle, CredentialSource}; + + #[test] + fn detect_defaults_to_path_for_generic_endpoint() { + // Virtual-host-only providers such as Tigris (t3.storage.dev) are not + // matched by the heuristic and fall back to path style, which is why an + // explicit `addressing_style` override exists. + assert_eq!( + detect_addressing_style("https://t3.storage.dev", "snapshots").unwrap(), + AddressingStyle::Path + ); + } + + #[test] + fn detect_uses_virtual_for_aliyun_and_bucket_host() { + assert_eq!( + detect_addressing_style("https://oss-cn-hangzhou.aliyuncs.com", "b").unwrap(), + AddressingStyle::Virtual + ); + assert_eq!( + detect_addressing_style("https://b.example.com", "b").unwrap(), + AddressingStyle::Virtual + ); + } + + #[test] + fn explicit_override_takes_precedence_over_detection() { + let client = OssClient::new( + "snapshots".to_string(), + "https://t3.storage.dev".to_string(), + "auto".to_string(), + String::new(), + CredentialSource::Anonymous, + Some(AddressingStyle::Virtual), + ) + .expect("build client"); + assert_eq!( + client.operator_config.addressing_style, + AddressingStyle::Virtual + ); + } +} diff --git a/src/snapshot/repository/backends/oss/config.rs b/src/snapshot/repository/backends/oss/config.rs index 6a1bd097..43b77d21 100644 --- a/src/snapshot/repository/backends/oss/config.rs +++ b/src/snapshot/repository/backends/oss/config.rs @@ -1,10 +1,10 @@ use anyhow::{Context, Result}; use object_store_operator::{ - credential_source_from_fields, normalized_credential, CredentialFields, CredentialSource, - CredentialSourceOptions, + credential_source_from_fields, normalized_credential, AddressingStyle, CredentialFields, + CredentialSource, CredentialSourceOptions, }; -use crate::cfg::{OssBackendConfig, SnapshotImageStoragePolicy}; +use crate::cfg::{OssAddressingStyle, OssBackendConfig, SnapshotImageStoragePolicy}; #[derive(Debug, Clone)] pub(super) struct NormalizedOssConfig { @@ -14,6 +14,7 @@ pub(super) struct NormalizedOssConfig { prefix: String, credential_source: CredentialSource, snapshot_image_storage: SnapshotImageStoragePolicy, + addressing_style: Option, } impl NormalizedOssConfig { @@ -54,6 +55,10 @@ impl NormalizedOssConfig { required_secret_access_key_label: "backend.oss.access_key_secret", }, )?; + let addressing_style = config.addressing_style.map(|style| match style { + OssAddressingStyle::Path => AddressingStyle::Path, + OssAddressingStyle::Virtual => AddressingStyle::Virtual, + }); Ok(Self { bucket, endpoint, @@ -61,6 +66,7 @@ impl NormalizedOssConfig { prefix, credential_source, snapshot_image_storage, + addressing_style, }) } @@ -80,6 +86,12 @@ impl NormalizedOssConfig { &self.region } + /// Explicit bucket addressing style override, if configured. `None` means + /// the client should fall back to endpoint-based auto-detection. + pub(crate) fn addressing_style(&self) -> Option { + self.addressing_style.clone() + } + pub(crate) fn credential_source(&self) -> CredentialSource { self.credential_source.clone() } @@ -114,6 +126,7 @@ mod tests { access_key_secret: Some(" sk ".to_string()), security_token: Some(" token ".to_string()), region: Some(" cn-hangzhou ".to_string()), + addressing_style: None, cache_max_size_gb: Some(8), } } @@ -173,4 +186,25 @@ mod tests { .expect_err("missing region must fail"); assert!(err.to_string().contains("backend.oss.region is required")); } + + #[test] + fn normalized_config_maps_explicit_addressing_style() { + use crate::cfg::OssAddressingStyle; + use object_store_operator::AddressingStyle; + + let default = + NormalizedOssConfig::new(&sample_config(), SnapshotImageStoragePolicy::ObjectStorage) + .expect("normalize config"); + assert_eq!(default.addressing_style(), None); + + let mut config = sample_config(); + config.addressing_style = Some(OssAddressingStyle::Virtual); + let normalized = + NormalizedOssConfig::new(&config, SnapshotImageStoragePolicy::ObjectStorage) + .expect("normalize config"); + assert_eq!( + normalized.addressing_style(), + Some(AddressingStyle::Virtual) + ); + } } diff --git a/src/snapshot/repository/backends/oss/mod.rs b/src/snapshot/repository/backends/oss/mod.rs index c3adede3..3a620f32 100644 --- a/src/snapshot/repository/backends/oss/mod.rs +++ b/src/snapshot/repository/backends/oss/mod.rs @@ -64,6 +64,7 @@ impl OssBackend { config.region().to_string(), config.prefix().to_string(), config.credential_source(), + config.addressing_style(), )?); let repository: Arc = Arc::new(OssSnapshotRepository::new( diff --git a/src/snapshot/repository/backends/oss/repository.rs b/src/snapshot/repository/backends/oss/repository.rs index 06f747ef..bc30f24f 100644 --- a/src/snapshot/repository/backends/oss/repository.rs +++ b/src/snapshot/repository/backends/oss/repository.rs @@ -1185,6 +1185,7 @@ mod tests { "region".to_string(), "prefix".to_string(), CredentialSource::Anonymous, + None, ) .expect("oss client"); OssSnapshotRepository::new(Arc::new(client), SnapshotImageStoragePolicy::ObjectStorage) From b54d04a710c7fda55410d04faf4321483ac4baa2 Mon Sep 17 00:00:00 2001 From: David Myriel Date: Wed, 29 Jul 2026 11:19:52 -0400 Subject: [PATCH 2/6] fix(oss): propagate addressing style to overlaybd remote layer reads Address review feedback: - propagate backend.oss.addressing_style into the generated overlaybd runtime config (ossConfig.defaultAddressingStyle) so remote managed snapshot layer reads use the same style as the repository client - always run detect_addressing_style for endpoint validation, then apply the explicit override on top (repository client and overlaybd URL parse) - validate defaultAddressingStyle at overlaybd config load - add addressing_style to the OssBackendConfig literal in tests/snapshot_oss_e2e_test.rs - smoke example: collision-resistant per-run object key, and always attempt cleanup once the write succeeded, surfacing the primary error - cover the above with unit tests plus an ignored MinIO remote-layer read test using an explicit addressing style Co-Authored-By: Claude Fable 5 --- .../examples/tigris_smoke.rs | 33 +++++-- docs/src/configuration/reference.md | 1 + src/cfg.rs | 9 ++ src/setup/deps.rs | 21 ++++- .../repository/backends/oss/client.rs | 23 +++-- storage/overlaybd/src/backend/oss.rs | 87 ++++++++++++++++++- storage/overlaybd/src/config.rs | 13 +++ storage/overlaybd/tests/oss_backend_minio.rs | 45 ++++++++++ tests/snapshot_oss_e2e_test.rs | 1 + 9 files changed, 218 insertions(+), 15 deletions(-) diff --git a/crates/object-store-operator/examples/tigris_smoke.rs b/crates/object-store-operator/examples/tigris_smoke.rs index b0ba313a..2c39cb61 100644 --- a/crates/object-store-operator/examples/tigris_smoke.rs +++ b/crates/object-store-operator/examples/tigris_smoke.rs @@ -59,18 +59,39 @@ async fn main() -> anyhow::Result<()> { println!("endpoint={endpoint} bucket={bucket} addressing_style={style:?}"); let op = build_object_store_operator(&config, Some(&cred))?; - let key = "agentenv-smoke/roundtrip.txt"; + // Collision-resistant per-run key so concurrent runs cannot interfere and + // the test never overwrites or deletes a pre-existing object. + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("system clock before unix epoch") + .as_nanos(); + let key = format!( + "agentenv-smoke/{nonce}-{}/roundtrip.txt", + std::process::id() + ); let payload = b"agentenv addressing-style smoke test".to_vec(); println!("-> write {key}"); - op.write(key, payload.clone()).await?; + op.write(&key, payload.clone()).await?; - println!("-> read {key}"); - let got = op.read(key).await?.to_vec(); - anyhow::ensure!(got == payload, "read-back mismatch: object content differs"); + // From here on the object exists remotely, so always attempt cleanup even + // when the read or content check fails, then surface the primary error. + let round_trip = async { + println!("-> read {key}"); + let got = op.read(&key).await?.to_vec(); + anyhow::ensure!(got == payload, "read-back mismatch: object content differs"); + Ok(()) + } + .await; println!("-> delete {key}"); - op.delete(key).await?; + let cleanup = op.delete(&key).await; + if let Err(err) = &cleanup { + eprintln!("warning: failed to delete {key}: {err}"); + } + + round_trip?; + cleanup?; println!("OK: {style:?}-host round-trip succeeded against {endpoint}"); Ok(()) diff --git a/docs/src/configuration/reference.md b/docs/src/configuration/reference.md index 08a93ba2..67b0f39c 100644 --- a/docs/src/configuration/reference.md +++ b/docs/src/configuration/reference.md @@ -384,6 +384,7 @@ Notes: - `credential_process` should be written as a portable argv-style command line. Avoid `$VAR`, backticks, `$(...)`, pipes, and shell builtins. - Although the config section is still named `oss`, the runtime path is implemented via a shared S3-compatible client, so `region` must be configured. - Leave `addressing_style` unset for Alibaba OSS and MinIO. Set it to `"virtual"` for S3-compatible providers that only support virtual-host bucket addressing; path-style requests to those endpoints are rejected. +- The setting covers both halves of the data path: the snapshot repository client (metadata and artifact upload/download) and the generated OverlayBD runtime config (`ossConfig.defaultAddressingStyle`), which the runtime uses when reading remote managed snapshot layers during sandbox restore. For an S3-compatible provider that requires virtual-host addressing — for example [Tigris](https://www.tigrisdata.com/docs/) — set `addressing_style` explicitly: diff --git a/src/cfg.rs b/src/cfg.rs index 5da48c7c..b2502605 100644 --- a/src/cfg.rs +++ b/src/cfg.rs @@ -292,6 +292,15 @@ pub enum OssAddressingStyle { Virtual, } +impl OssAddressingStyle { + pub fn as_str(self) -> &'static str { + match self { + Self::Path => "path", + Self::Virtual => "virtual", + } + } +} + #[derive(Debug, Deserialize, Clone)] pub struct OssBackendConfig { pub endpoint: String, diff --git a/src/setup/deps.rs b/src/setup/deps.rs index 5b09a7a0..0cf82df3 100644 --- a/src/setup/deps.rs +++ b/src/setup/deps.rs @@ -11,7 +11,8 @@ use serde::Deserialize; use tracing::{debug, info}; use crate::cfg::{ - AppConfig, OssBackendConfig, OverlaybdDependencyConfig, SnapshotRepositoryBackendKind, + AppConfig, OssAddressingStyle, OssBackendConfig, OverlaybdDependencyConfig, + SnapshotRepositoryBackendKind, }; use crate::digest::FileDigest; @@ -663,6 +664,12 @@ fn overlaybd_runtime_oss_config(oss: &OssBackendConfig) -> Result, ) -> Result { - // An explicit config override wins; otherwise fall back to - // endpoint-based detection (Alibaba OSS / bucket-in-endpoint hosts). - let addressing_style = match addressing_override { - Some(style) => style, - None => detect_addressing_style(&endpoint, &bucket)?, - }; + // Detection also validates the endpoint URL, so it always runs; an + // explicit config override then wins over the detected style. + let detected_style = detect_addressing_style(&endpoint, &bucket)?; + let addressing_style = addressing_override.unwrap_or(detected_style); Ok(Self { operator_config: ObjectStoreOperatorConfig { addressing_style, @@ -447,4 +445,17 @@ mod tests { AddressingStyle::Virtual ); } + + #[test] + fn explicit_override_still_validates_endpoint() { + OssClient::new( + "snapshots".to_string(), + "not a valid endpoint".to_string(), + "auto".to_string(), + String::new(), + CredentialSource::Anonymous, + Some(AddressingStyle::Virtual), + ) + .expect_err("malformed endpoint must fail even with an explicit override"); + } } diff --git a/storage/overlaybd/src/backend/oss.rs b/storage/overlaybd/src/backend/oss.rs index 6040105c..ce45911d 100644 --- a/storage/overlaybd/src/backend/oss.rs +++ b/storage/overlaybd/src/backend/oss.rs @@ -31,6 +31,7 @@ struct OssBackendInner { credentials: CachedCredentialSource, default_region: String, default_endpoint: String, + addressing_override: Option, timeout: Duration, retry_count: u32, cached_operators: RwLock>, @@ -63,6 +64,7 @@ struct OperatorCacheKey { impl OssBackend { pub fn new(config: &OssConfig) -> Result { let credential_source = credential_source_from_config(config)?; + let addressing_override = parse_addressing_style(&config.default_addressing_style)?; let timeout = if config.timeout_secs == 0 { DEFAULT_TIMEOUT } else { @@ -79,6 +81,7 @@ impl OssBackend { credentials: CachedCredentialSource::new(credential_source), default_region: config.default_region.clone(), default_endpoint: config.default_endpoint.clone(), + addressing_override, timeout, retry_count, cached_operators: RwLock::new(HashMap::new()), @@ -95,6 +98,7 @@ impl OssBackend { url.as_ref(), &self.inner.default_endpoint, &self.inner.default_region, + self.inner.addressing_override.clone(), )?; Ok(Arc::new(OssFile { @@ -125,6 +129,7 @@ impl OssBackend { url.as_ref(), &self.inner.default_endpoint, &self.inner.default_region, + self.inner.addressing_override.clone(), )?; let key = location.key.clone(); @@ -143,7 +148,12 @@ impl OssBackend { } impl ParsedOssUrl { - fn parse(raw: &str, default_endpoint: &str, default_region: &str) -> Result { + fn parse( + raw: &str, + default_endpoint: &str, + default_region: &str, + addressing_override: Option, + ) -> Result { let url = Url::parse(raw).context(format!("invalid oss url {raw}"))?; ensure!( matches!(url.scheme(), "s3" | "oss"), @@ -187,7 +197,10 @@ impl ParsedOssUrl { } }) .ok_or_else(|| anyhow::anyhow!("oss region is missing"))?; - let addressing_style = detect_addressing_style(&endpoint, &bucket)?; + // Detection also validates the endpoint URL, so it always runs; an + // explicit override from the config then wins over the detected style. + let detected_style = detect_addressing_style(&endpoint, &bucket)?; + let addressing_style = addressing_override.unwrap_or(detected_style); Ok(Self { bucket, @@ -453,6 +466,19 @@ fn detect_addressing_style(endpoint: &str, bucket: &str) -> Result Result> { + match value.trim() { + "" => Ok(None), + "virtual" => Ok(Some(AddressingStyle::Virtual)), + "path" => Ok(Some(AddressingStyle::Path)), + other => bail!( + "invalid oss defaultAddressingStyle '{other}': expected 'virtual', 'path', or empty for auto-detection" + ), + } +} + fn credential_source_from_config(config: &OssConfig) -> Result { credential_source_from_fields( CredentialFields { @@ -511,6 +537,7 @@ mod tests { credential_process: "echo '{}'".to_string(), default_region: "us-east-1".to_string(), default_endpoint: "https://s3.us-east-1.amazonaws.com".to_string(), + default_addressing_style: String::new(), timeout_secs: 30, retry_count: 3, }; @@ -518,4 +545,60 @@ mod tests { let err = credential_source_from_config(&config).expect_err("mixed source should fail"); assert!(err.to_string().contains("credential_process")); } + + #[test] + fn test_parse_addressing_style_values() { + assert_eq!(parse_addressing_style("").expect("empty"), None); + assert_eq!(parse_addressing_style(" ").expect("blank"), None); + assert_eq!( + parse_addressing_style("virtual").expect("virtual"), + Some(AddressingStyle::Virtual) + ); + assert_eq!( + parse_addressing_style("path").expect("path"), + Some(AddressingStyle::Path) + ); + parse_addressing_style("bogus").expect_err("invalid style must be rejected"); + } + + #[test] + fn test_parsed_url_applies_addressing_override() { + // A generic endpoint auto-detects to path style... + let detected = ParsedOssUrl::parse( + "s3://demo-bucket/layers/obj", + "https://t3.storage.dev", + "auto", + None, + ) + .expect("parse without override"); + assert_eq!(detected.addressing_style, AddressingStyle::Path); + + // ...and the config-level override switches remote layer reads to + // virtual-host style for providers that require it. + let overridden = ParsedOssUrl::parse( + "s3://demo-bucket/layers/obj", + "https://t3.storage.dev", + "auto", + Some(AddressingStyle::Virtual), + ) + .expect("parse with override"); + assert_eq!(overridden.addressing_style, AddressingStyle::Virtual); + assert_eq!( + overridden + .operator_config(Duration::from_secs(30), 3) + .addressing_style, + AddressingStyle::Virtual + ); + } + + #[test] + fn test_parsed_url_override_still_validates_endpoint() { + ParsedOssUrl::parse( + "s3://demo-bucket/layers/obj", + "not a valid endpoint", + "auto", + Some(AddressingStyle::Virtual), + ) + .expect_err("malformed endpoint must fail even with an explicit override"); + } } diff --git a/storage/overlaybd/src/config.rs b/storage/overlaybd/src/config.rs index e706b800..cf9d472e 100644 --- a/storage/overlaybd/src/config.rs +++ b/storage/overlaybd/src/config.rs @@ -196,6 +196,10 @@ pub struct OssConfig { pub credential_process: String, pub default_region: String, pub default_endpoint: String, + /// Bucket addressing style: `"virtual"`, `"path"`, or empty to auto-detect + /// from the endpoint (Alibaba OSS and bucket-in-endpoint hosts use + /// virtual-host style, other endpoints default to path style). + pub default_addressing_style: String, /// Per-request timeout in seconds (connect + transfer). Default 30. pub timeout_secs: u64, /// Number of retries on transient failures. Default 3. @@ -212,6 +216,7 @@ impl Default for OssConfig { credential_process: String::new(), default_region: String::new(), default_endpoint: String::new(), + default_addressing_style: String::new(), timeout_secs: 30, retry_count: 3, } @@ -579,6 +584,14 @@ pub fn validate_global_config(cfg: &GlobalConfig) -> Result<()> { && !cfg.oss_config.secret_access_key.is_empty()), "ossConfig.securityToken requires accessKeyId and secretAccessKey" ); + ensure!( + matches!( + cfg.oss_config.default_addressing_style.trim(), + "" | "virtual" | "path" + ), + "ossConfig.defaultAddressingStyle must be 'virtual', 'path', or empty for auto-detection, got '{}'", + cfg.oss_config.default_addressing_style + ); } ensure!(cfg.nr_io_rings != 0, "nr_io_rings cannot be zero"); diff --git a/storage/overlaybd/tests/oss_backend_minio.rs b/storage/overlaybd/tests/oss_backend_minio.rs index f8850278..186d62b8 100644 --- a/storage/overlaybd/tests/oss_backend_minio.rs +++ b/storage/overlaybd/tests/oss_backend_minio.rs @@ -14,12 +14,29 @@ fn backend(fixture: &MinioFixture) -> OssBackend { credential_process: String::new(), default_region: fixture.region.clone(), default_endpoint: fixture.endpoint.clone(), + default_addressing_style: String::new(), timeout_secs: 30, retry_count: 3, }; OssBackend::new(&config).expect("create oss backend") } +fn backend_with_addressing_style(fixture: &MinioFixture, style: &str) -> OssBackend { + let config = OssConfig { + enable: true, + access_key_id: MINIO_USER.to_string(), + secret_access_key: MINIO_PASS.to_string(), + security_token: String::new(), + credential_process: String::new(), + default_region: fixture.region.clone(), + default_endpoint: fixture.endpoint.clone(), + default_addressing_style: style.to_string(), + timeout_secs: 30, + retry_count: 3, + }; + OssBackend::new(&config).expect("create oss backend with explicit addressing style") +} + fn backend_with_bad_credentials(fixture: &MinioFixture) -> OssBackend { let config = OssConfig { enable: true, @@ -29,6 +46,7 @@ fn backend_with_bad_credentials(fixture: &MinioFixture) -> OssBackend { credential_process: String::new(), default_region: fixture.region.clone(), default_endpoint: fixture.endpoint.clone(), + default_addressing_style: String::new(), timeout_secs: 10, retry_count: 0, }; @@ -77,6 +95,33 @@ async fn minio_upload_and_read_back() { assert!(got.is_empty()); } +#[tokio::test] +#[ignore = "requires docker"] +async fn minio_remote_layer_read_with_explicit_addressing_style() { + // MinIO only serves path-style requests without extra DNS/domain setup, so + // an explicit "path" override exercises the full config -> backend -> read + // propagation live. The virtual-host branch is covered by unit tests, since + // it needs wildcard DNS the fixture cannot provide. + let fixture = MinioFixture::start().await.expect("start minio"); + let backend = backend_with_addressing_style(&fixture, "path"); + + let payload = b"remote layer bytes".to_vec(); + let url = fixture.object_url("layers/explicit-style-object"); + backend + .upload_bytes(&url, payload.clone()) + .await + .expect("upload with explicit addressing style"); + + let file = backend + .open_with_size_hint(&url, None) + .expect("open with explicit addressing style"); + let got = file + .read_at(0, payload.len()) + .await + .expect("read remote layer with explicit addressing style"); + assert_eq!(got.as_ref(), payload.as_slice()); +} + #[tokio::test] #[ignore = "requires docker"] async fn minio_wrong_credentials_returns_error() { diff --git a/tests/snapshot_oss_e2e_test.rs b/tests/snapshot_oss_e2e_test.rs index 7640d708..09dc4592 100644 --- a/tests/snapshot_oss_e2e_test.rs +++ b/tests/snapshot_oss_e2e_test.rs @@ -83,6 +83,7 @@ fn test_oss_config(fixture: &MinioFixture, prefix: &str) -> OssBackendConfig { access_key_secret: Some(MINIO_PASS.to_string()), security_token: None, region: Some(fixture.region.clone()), + addressing_style: None, cache_max_size_gb: Some(1), } } From 70180d035c30e70d59063d202188cc73afce26c5 Mon Sep 17 00:00:00 2001 From: David Myriel Date: Wed, 29 Jul 2026 11:33:24 -0400 Subject: [PATCH 3/6] fix(oss): harden smoke example and prove addressing override on the wire Address second-round review feedback: - smoke example: propagate missing env vars as errors instead of panicking, use a random UUID object key so concurrent runs on any host cannot collide, and attempt cleanup even after a failed write since a timeout can leave the object committed remotely - replace the MinIO explicit-style test (which exercised a combination auto-detection would have picked anyway) with a wire-level test: bucket '127' on a 127.0.0.1 endpoint auto-detects as virtual-host, so an explicit 'path' override observably flips the recorded request to path style; the test fails if the override is dropped anywhere - also assert invalid defaultAddressingStyle values are rejected at backend construction Co-Authored-By: Claude Fable 5 --- Cargo.lock | 1 + crates/object-store-operator/Cargo.toml | 3 + .../examples/tigris_smoke.rs | 31 +++---- .../overlaybd/tests/oss_addressing_style.rs | 90 +++++++++++++++++++ storage/overlaybd/tests/oss_backend_minio.rs | 43 --------- 5 files changed, 110 insertions(+), 58 deletions(-) create mode 100644 storage/overlaybd/tests/oss_addressing_style.rs diff --git a/Cargo.lock b/Cargo.lock index 0a10e20b..cb1f6f4e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5093,6 +5093,7 @@ dependencies = [ "serde_json", "thiserror 2.0.18", "tokio", + "uuid", ] [[package]] diff --git a/crates/object-store-operator/Cargo.toml b/crates/object-store-operator/Cargo.toml index 11e85ac9..b92e9ffd 100644 --- a/crates/object-store-operator/Cargo.toml +++ b/crates/object-store-operator/Cargo.toml @@ -11,3 +11,6 @@ serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" thiserror = "2.0.18" tokio = { version = "1", features = ["macros", "process", "rt-multi-thread", "sync", "time"] } + +[dev-dependencies] +uuid = { version = "1.0", features = ["v4"] } diff --git a/crates/object-store-operator/examples/tigris_smoke.rs b/crates/object-store-operator/examples/tigris_smoke.rs index 2c39cb61..ef11f558 100644 --- a/crates/object-store-operator/examples/tigris_smoke.rs +++ b/crates/object-store-operator/examples/tigris_smoke.rs @@ -26,8 +26,8 @@ fn env_or(key: &str, default: &str) -> String { std::env::var(key).unwrap_or_else(|_| default.to_string()) } -fn env_req(key: &str) -> String { - std::env::var(key).unwrap_or_else(|_| panic!("missing required env var {key}")) +fn env_req(key: &str) -> anyhow::Result { + std::env::var(key).map_err(|err| anyhow::anyhow!("required env var {key}: {err}")) } #[tokio::main] @@ -39,10 +39,10 @@ async fn main() -> anyhow::Result<()> { "path" => AddressingStyle::Path, other => anyhow::bail!("SMOKE_STYLE must be 'virtual' or 'path', got '{other}'"), }; - let bucket = env_req("SMOKE_BUCKET"); + let bucket = env_req("SMOKE_BUCKET")?; let cred = ResolvedCredential::new( - env_req("SMOKE_ACCESS_KEY_ID"), - env_req("SMOKE_SECRET_ACCESS_KEY"), + env_req("SMOKE_ACCESS_KEY_ID")?, + env_req("SMOKE_SECRET_ACCESS_KEY")?, None, None, )?; @@ -59,20 +59,21 @@ async fn main() -> anyhow::Result<()> { println!("endpoint={endpoint} bucket={bucket} addressing_style={style:?}"); let op = build_object_store_operator(&config, Some(&cred))?; - // Collision-resistant per-run key so concurrent runs cannot interfere and + // Random per-run key so concurrent runs (on any host) cannot interfere and // the test never overwrites or deletes a pre-existing object. - let nonce = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .expect("system clock before unix epoch") - .as_nanos(); - let key = format!( - "agentenv-smoke/{nonce}-{}/roundtrip.txt", - std::process::id() - ); + let key = format!("agentenv-smoke/{}/roundtrip.txt", uuid::Uuid::new_v4()); let payload = b"agentenv addressing-style smoke test".to_vec(); + // A failed write can still have committed the object remotely (timeout or + // retry ambiguity), so attempt cleanup even then; the unique key makes the + // extra delete safe. println!("-> write {key}"); - op.write(&key, payload.clone()).await?; + if let Err(write_err) = op.write(&key, payload.clone()).await { + if let Err(cleanup_err) = op.delete(&key).await { + eprintln!("warning: failed to delete {key} after write error: {cleanup_err}"); + } + return Err(write_err.into()); + } // From here on the object exists remotely, so always attempt cleanup even // when the read or content check fails, then surface the primary error. diff --git a/storage/overlaybd/tests/oss_addressing_style.rs b/storage/overlaybd/tests/oss_addressing_style.rs new file mode 100644 index 00000000..40c0dc71 --- /dev/null +++ b/storage/overlaybd/tests/oss_addressing_style.rs @@ -0,0 +1,90 @@ +//! Wire-level proof that `ossConfig.defaultAddressingStyle` propagates from the +//! backend config all the way into the HTTP requests issued for remote layer +//! reads. No docker or DNS setup required. +//! +//! The setup is chosen so that auto-detection and the explicit override +//! disagree: with endpoint `http://127.0.0.1:{port}` and bucket `127`, the +//! endpoint host starts with `"127."` and auto-detection therefore picks +//! virtual-host style. An explicit `"path"` override must flip the request to +//! path style (`GET /127/` against the plain endpoint host). If the +//! override were dropped anywhere along the way, the client would target the +//! unresolvable virtual host `127.127.0.0.1` and the recorder below would never +//! see a path-style request. + +use overlaybd::backend::oss::OssBackend; +use overlaybd::config::OssConfig; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::TcpListener; + +fn config_with_style(endpoint: String, style: &str) -> OssConfig { + OssConfig { + enable: true, + access_key_id: String::new(), + secret_access_key: String::new(), + security_token: String::new(), + credential_process: String::new(), + default_region: "auto".to_string(), + default_endpoint: endpoint, + default_addressing_style: style.to_string(), + timeout_secs: 5, + retry_count: 1, + } +} + +/// Accept one connection, capture the request head, and answer with an error +/// status; the test only cares about what the request looked like. +async fn record_one_request(listener: TcpListener) -> String { + let (mut socket, _) = listener.accept().await.expect("accept recorder connection"); + let mut buf = vec![0u8; 8192]; + let n = socket.read(&mut buf).await.expect("read request head"); + let head = String::from_utf8_lossy(&buf[..n]).into_owned(); + let _ = socket + .write_all( + b"HTTP/1.1 500 Internal Server Error\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", + ) + .await; + head +} + +#[tokio::test] +async fn explicit_path_override_reaches_the_wire() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("bind recorder listener"); + let port = listener.local_addr().expect("recorder addr").port(); + let recorder = tokio::spawn(record_one_request(listener)); + + let config = config_with_style(format!("http://127.0.0.1:{port}"), "path"); + let backend = OssBackend::new(&config).expect("create oss backend"); + let file = backend + .open_with_size_hint("s3://127/layers/explicit-style-object", None) + .expect("open remote layer file"); + + // The read fails (the recorder answers 500); only the request matters. + let _ = file.read_at(0, 4).await; + + let head = tokio::time::timeout(std::time::Duration::from_secs(30), recorder) + .await + .expect("no request reached the recorder — was the addressing override dropped?") + .expect("recorder task"); + let request_line = head.lines().next().unwrap_or_default().to_string(); + assert!( + request_line.contains("/127/layers/explicit-style-object"), + "expected a path-style request for bucket '127', got: {request_line}" + ); + assert!( + head.lines() + .any(|line| line.to_ascii_lowercase() == format!("host: 127.0.0.1:{port}")), + "expected the plain endpoint host, got request head: {head}" + ); +} + +#[test] +fn invalid_addressing_style_is_rejected() { + let config = config_with_style("http://127.0.0.1:9000".to_string(), "bogus"); + let err = OssBackend::new(&config).expect_err("invalid style must fail"); + assert!( + err.to_string().contains("defaultAddressingStyle"), + "unexpected error: {err}" + ); +} diff --git a/storage/overlaybd/tests/oss_backend_minio.rs b/storage/overlaybd/tests/oss_backend_minio.rs index 186d62b8..e854df26 100644 --- a/storage/overlaybd/tests/oss_backend_minio.rs +++ b/storage/overlaybd/tests/oss_backend_minio.rs @@ -21,22 +21,6 @@ fn backend(fixture: &MinioFixture) -> OssBackend { OssBackend::new(&config).expect("create oss backend") } -fn backend_with_addressing_style(fixture: &MinioFixture, style: &str) -> OssBackend { - let config = OssConfig { - enable: true, - access_key_id: MINIO_USER.to_string(), - secret_access_key: MINIO_PASS.to_string(), - security_token: String::new(), - credential_process: String::new(), - default_region: fixture.region.clone(), - default_endpoint: fixture.endpoint.clone(), - default_addressing_style: style.to_string(), - timeout_secs: 30, - retry_count: 3, - }; - OssBackend::new(&config).expect("create oss backend with explicit addressing style") -} - fn backend_with_bad_credentials(fixture: &MinioFixture) -> OssBackend { let config = OssConfig { enable: true, @@ -95,33 +79,6 @@ async fn minio_upload_and_read_back() { assert!(got.is_empty()); } -#[tokio::test] -#[ignore = "requires docker"] -async fn minio_remote_layer_read_with_explicit_addressing_style() { - // MinIO only serves path-style requests without extra DNS/domain setup, so - // an explicit "path" override exercises the full config -> backend -> read - // propagation live. The virtual-host branch is covered by unit tests, since - // it needs wildcard DNS the fixture cannot provide. - let fixture = MinioFixture::start().await.expect("start minio"); - let backend = backend_with_addressing_style(&fixture, "path"); - - let payload = b"remote layer bytes".to_vec(); - let url = fixture.object_url("layers/explicit-style-object"); - backend - .upload_bytes(&url, payload.clone()) - .await - .expect("upload with explicit addressing style"); - - let file = backend - .open_with_size_hint(&url, None) - .expect("open with explicit addressing style"); - let got = file - .read_at(0, payload.len()) - .await - .expect("read remote layer with explicit addressing style"); - assert_eq!(got.as_ref(), payload.as_slice()); -} - #[tokio::test] #[ignore = "requires docker"] async fn minio_wrong_credentials_returns_error() { From 8221545efb6f23dd73b3e17d2871801ffb443987 Mon Sep 17 00:00:00 2001 From: David Myriel Date: Wed, 29 Jul 2026 12:11:44 -0400 Subject: [PATCH 4/6] test(oss): drop tigris smoke example, harden addressing wire test The smoke example was a manual verification aid, not part of the feature; removing it (and its uuid dev-dependency) keeps the PR scoped to the addressing-style change. The wire-level propagation test stays and is hardened per review: the recorder now reads until the header terminator so partial TCP reads cannot flake the assertion, and a single timeout bounds the whole read + capture interaction. Co-Authored-By: Claude Fable 5 --- Cargo.lock | 1 - crates/object-store-operator/Cargo.toml | 3 - .../examples/tigris_smoke.rs | 99 ------------------- .../overlaybd/tests/oss_addressing_style.rs | 26 +++-- 4 files changed, 18 insertions(+), 111 deletions(-) delete mode 100644 crates/object-store-operator/examples/tigris_smoke.rs diff --git a/Cargo.lock b/Cargo.lock index cb1f6f4e..0a10e20b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5093,7 +5093,6 @@ dependencies = [ "serde_json", "thiserror 2.0.18", "tokio", - "uuid", ] [[package]] diff --git a/crates/object-store-operator/Cargo.toml b/crates/object-store-operator/Cargo.toml index b92e9ffd..11e85ac9 100644 --- a/crates/object-store-operator/Cargo.toml +++ b/crates/object-store-operator/Cargo.toml @@ -11,6 +11,3 @@ serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" thiserror = "2.0.18" tokio = { version = "1", features = ["macros", "process", "rt-multi-thread", "sync", "time"] } - -[dev-dependencies] -uuid = { version = "1.0", features = ["v4"] } diff --git a/crates/object-store-operator/examples/tigris_smoke.rs b/crates/object-store-operator/examples/tigris_smoke.rs deleted file mode 100644 index ef11f558..00000000 --- a/crates/object-store-operator/examples/tigris_smoke.rs +++ /dev/null @@ -1,99 +0,0 @@ -//! End-to-end smoke test for the S3-compatible snapshot backend against a real -//! object store (e.g. Tigris). It exercises the SAME `build_object_store_operator` -//! path the OSS snapshot backend uses, so a successful round-trip proves that the -//! configured `addressing_style` actually reaches the wire. -//! -//! Usage: -//! SMOKE_BUCKET=my-bucket \ -//! SMOKE_ACCESS_KEY_ID=tid_xxx SMOKE_SECRET_ACCESS_KEY=tsec_xxx \ -//! cargo run -p object-store-operator --example tigris_smoke -//! -//! Env (defaults in brackets): -//! SMOKE_ENDPOINT [https://t3.storage.dev] -//! SMOKE_REGION [auto] -//! SMOKE_STYLE [virtual] -- "virtual" or "path" -//! SMOKE_BUCKET (required) -//! SMOKE_ACCESS_KEY_ID / SMOKE_SECRET_ACCESS_KEY (required) -//! -//! Tip: run once with SMOKE_STYLE=path against a modern Tigris bucket to see the -//! failure this fix addresses, then again with SMOKE_STYLE=virtual to see it pass. - -use object_store_operator::{ - build_object_store_operator, AddressingStyle, ObjectStoreOperatorConfig, ResolvedCredential, -}; - -fn env_or(key: &str, default: &str) -> String { - std::env::var(key).unwrap_or_else(|_| default.to_string()) -} - -fn env_req(key: &str) -> anyhow::Result { - std::env::var(key).map_err(|err| anyhow::anyhow!("required env var {key}: {err}")) -} - -#[tokio::main] -async fn main() -> anyhow::Result<()> { - let endpoint = env_or("SMOKE_ENDPOINT", "https://t3.storage.dev"); - let region = env_or("SMOKE_REGION", "auto"); - let style = match env_or("SMOKE_STYLE", "virtual").as_str() { - "virtual" => AddressingStyle::Virtual, - "path" => AddressingStyle::Path, - other => anyhow::bail!("SMOKE_STYLE must be 'virtual' or 'path', got '{other}'"), - }; - let bucket = env_req("SMOKE_BUCKET")?; - let cred = ResolvedCredential::new( - env_req("SMOKE_ACCESS_KEY_ID")?, - env_req("SMOKE_SECRET_ACCESS_KEY")?, - None, - None, - )?; - - let config = ObjectStoreOperatorConfig { - bucket: bucket.clone(), - endpoint: endpoint.clone(), - region, - addressing_style: style.clone(), - timeout: None, - max_retries: None, - }; - - println!("endpoint={endpoint} bucket={bucket} addressing_style={style:?}"); - let op = build_object_store_operator(&config, Some(&cred))?; - - // Random per-run key so concurrent runs (on any host) cannot interfere and - // the test never overwrites or deletes a pre-existing object. - let key = format!("agentenv-smoke/{}/roundtrip.txt", uuid::Uuid::new_v4()); - let payload = b"agentenv addressing-style smoke test".to_vec(); - - // A failed write can still have committed the object remotely (timeout or - // retry ambiguity), so attempt cleanup even then; the unique key makes the - // extra delete safe. - println!("-> write {key}"); - if let Err(write_err) = op.write(&key, payload.clone()).await { - if let Err(cleanup_err) = op.delete(&key).await { - eprintln!("warning: failed to delete {key} after write error: {cleanup_err}"); - } - return Err(write_err.into()); - } - - // From here on the object exists remotely, so always attempt cleanup even - // when the read or content check fails, then surface the primary error. - let round_trip = async { - println!("-> read {key}"); - let got = op.read(&key).await?.to_vec(); - anyhow::ensure!(got == payload, "read-back mismatch: object content differs"); - Ok(()) - } - .await; - - println!("-> delete {key}"); - let cleanup = op.delete(&key).await; - if let Err(err) = &cleanup { - eprintln!("warning: failed to delete {key}: {err}"); - } - - round_trip?; - cleanup?; - - println!("OK: {style:?}-host round-trip succeeded against {endpoint}"); - Ok(()) -} diff --git a/storage/overlaybd/tests/oss_addressing_style.rs b/storage/overlaybd/tests/oss_addressing_style.rs index 40c0dc71..3c5bd03b 100644 --- a/storage/overlaybd/tests/oss_addressing_style.rs +++ b/storage/overlaybd/tests/oss_addressing_style.rs @@ -36,7 +36,16 @@ fn config_with_style(endpoint: String, style: &str) -> OssConfig { async fn record_one_request(listener: TcpListener) -> String { let (mut socket, _) = listener.accept().await.expect("accept recorder connection"); let mut buf = vec![0u8; 8192]; - let n = socket.read(&mut buf).await.expect("read request head"); + // A single read may return a partial head, so keep reading until the + // header terminator, EOF, or the buffer limit. + let mut n = 0; + while n < buf.len() && !buf[..n].windows(4).any(|window| window == b"\r\n\r\n") { + let read = socket.read(&mut buf[n..]).await.expect("read request head"); + if read == 0 { + break; + } + n += read; + } let head = String::from_utf8_lossy(&buf[..n]).into_owned(); let _ = socket .write_all( @@ -60,13 +69,14 @@ async fn explicit_path_override_reaches_the_wire() { .open_with_size_hint("s3://127/layers/explicit-style-object", None) .expect("open remote layer file"); - // The read fails (the recorder answers 500); only the request matters. - let _ = file.read_at(0, 4).await; - - let head = tokio::time::timeout(std::time::Duration::from_secs(30), recorder) - .await - .expect("no request reached the recorder — was the addressing override dropped?") - .expect("recorder task"); + // One bound covers the whole interaction: the read (which fails with the + // recorder's 500 response; only the request matters) and the capture. + let head = tokio::time::timeout(std::time::Duration::from_secs(30), async { + let _ = file.read_at(0, 4).await; + recorder.await.expect("recorder task") + }) + .await + .expect("no request reached the recorder — was the addressing override dropped?"); let request_line = head.lines().next().unwrap_or_default().to_string(); assert!( request_line.contains("/127/layers/explicit-style-object"), From 472f6ebee8450bf1dd33ff7554c84f4b65569eca Mon Sep 17 00:00:00 2001 From: David Myriel Date: Wed, 29 Jul 2026 12:17:52 -0400 Subject: [PATCH 5/6] Update storage/overlaybd/tests/oss_addressing_style.rs Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- storage/overlaybd/tests/oss_addressing_style.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/overlaybd/tests/oss_addressing_style.rs b/storage/overlaybd/tests/oss_addressing_style.rs index 3c5bd03b..3480069e 100644 --- a/storage/overlaybd/tests/oss_addressing_style.rs +++ b/storage/overlaybd/tests/oss_addressing_style.rs @@ -49,7 +49,7 @@ async fn record_one_request(listener: TcpListener) -> String { let head = String::from_utf8_lossy(&buf[..n]).into_owned(); let _ = socket .write_all( - b"HTTP/1.1 500 Internal Server Error\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", + b"HTTP/1.1 404 Not Found\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", ) .await; head From ec6859e6895c77f8d48ec5b1600c60632e4c62d9 Mon Sep 17 00:00:00 2001 From: David Myriel Date: Wed, 29 Jul 2026 12:31:38 -0400 Subject: [PATCH 6/6] Update storage/overlaybd/tests/oss_addressing_style.rs Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- storage/overlaybd/tests/oss_addressing_style.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/overlaybd/tests/oss_addressing_style.rs b/storage/overlaybd/tests/oss_addressing_style.rs index 3480069e..6939349f 100644 --- a/storage/overlaybd/tests/oss_addressing_style.rs +++ b/storage/overlaybd/tests/oss_addressing_style.rs @@ -70,7 +70,7 @@ async fn explicit_path_override_reaches_the_wire() { .expect("open remote layer file"); // One bound covers the whole interaction: the read (which fails with the - // recorder's 500 response; only the request matters) and the capture. + // recorder's 404 response; only the request matters) and the capture. let head = tokio::time::timeout(std::time::Duration::from_secs(30), async { let _ = file.read_at(0, 4).await; recorder.await.expect("recorder task")