Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions config/default.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Comment on lines +153 to +156

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[documentation · low]
The new option also accepts "path", but the default configuration only documents the virtual override. An explicit path override is important when auto-detection selects virtual style (for example, an Alibaba/custom bucket-in-host endpoint that nevertheless expects path style). Please list both accepted values and describe "path" as the force-path override; also consider avoiding “require” for the provider examples unless that requirement is guaranteed, since some S3-compatible providers support both styles.

Suggestion:

Suggested change
# # 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"
# # path style). Set "virtual" to force virtual-host addressing, or "path"
# # to force path-style addressing. For example, virtual-host style can be used
# # with AWS S3, Cloudflare R2, or Tigris
# # (endpoint = "https://t3.storage.dev", region = "auto").
# addressing_style = "virtual"

# prefix = "validation/"
# credential_process = ""
# cache_max_size_gb = 10
Expand Down
13 changes: 13 additions & 0 deletions docs/src/configuration/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -375,13 +375,26 @@ 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:

- `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.
- 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:

```toml
[backend.oss]
endpoint = "https://t3.storage.dev"
bucket = "agentenv-snapshots"
region = "auto"
addressing_style = "virtual"
```

Other path override:

Expand Down
5 changes: 5 additions & 0 deletions docs/src/getting-started/on-demand-loading.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
25 changes: 25 additions & 0 deletions src/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,29 @@ 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,
}

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,
Expand All @@ -289,6 +312,8 @@ pub struct OssBackendConfig {
pub access_key_secret: Option<String>,
pub security_token: Option<String>,
pub region: Option<String>,
#[serde(alias = "addressingStyle", alias = "addressing-style")]
pub addressing_style: Option<OssAddressingStyle>,
Comment thread
davidmyriel marked this conversation as resolved.
Comment thread
davidmyriel marked this conversation as resolved.
pub cache_max_size_gb: Option<u64>,
}

Expand Down
22 changes: 21 additions & 1 deletion src/setup/deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -663,6 +664,12 @@ fn overlaybd_runtime_oss_config(oss: &OssBackendConfig) -> Result<serde_json::Va
"enable": true,
"defaultRegion": region,
"defaultEndpoint": endpoint,
// Empty string means the overlaybd runtime auto-detects the style per
// endpoint, matching the snapshot repository client's behavior.
"defaultAddressingStyle": oss
.addressing_style
.map(OssAddressingStyle::as_str)
.unwrap_or(""),
});

match credential_source {
Expand Down Expand Up @@ -724,6 +731,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),
}
}
Expand Down Expand Up @@ -780,6 +788,18 @@ mod tests {
config["defaultEndpoint"],
"https://oss-cn-hangzhou.aliyuncs.com"
);
assert_eq!(config["defaultAddressingStyle"], "");
}

#[test]
fn overlaybd_runtime_oss_config_propagates_addressing_style() {
use crate::cfg::OssAddressingStyle;

let mut oss = sample_oss_config();
oss.addressing_style = Some(OssAddressingStyle::Virtual);

let config = overlaybd_runtime_oss_config(&oss).expect("derive overlaybd oss config");
assert_eq!(config["defaultAddressingStyle"], "virtual");
}

#[test]
Expand Down
66 changes: 65 additions & 1 deletion src/snapshot/repository/backends/oss/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,15 @@ impl OssClient {
region: String,
prefix: String,
credential_source: CredentialSource,
addressing_override: Option<AddressingStyle>,
) -> Result<Self> {
// 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: detect_addressing_style(&endpoint, &bucket)?,
addressing_style,
bucket,
endpoint,
region,
Expand Down Expand Up @@ -395,3 +400,62 @@ 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
);
}

#[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");
}
}
40 changes: 37 additions & 3 deletions src/snapshot/repository/backends/oss/config.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -14,6 +14,7 @@ pub(super) struct NormalizedOssConfig {
prefix: String,
credential_source: CredentialSource,
snapshot_image_storage: SnapshotImageStoragePolicy,
addressing_style: Option<AddressingStyle>,
}

impl NormalizedOssConfig {
Expand Down Expand Up @@ -54,13 +55,18 @@ 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,
region,
prefix,
credential_source,
snapshot_image_storage,
addressing_style,
})
}

Expand All @@ -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<AddressingStyle> {
self.addressing_style.clone()
}

pub(crate) fn credential_source(&self) -> CredentialSource {
self.credential_source.clone()
}
Expand Down Expand Up @@ -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),
}
}
Expand Down Expand Up @@ -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)
);
}
}
1 change: 1 addition & 0 deletions src/snapshot/repository/backends/oss/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ impl OssBackend {
config.region().to_string(),
config.prefix().to_string(),
config.credential_source(),
config.addressing_style(),
Comment thread
davidmyriel marked this conversation as resolved.
)?);

let repository: Arc<dyn SnapshotRepository> = Arc::new(OssSnapshotRepository::new(
Expand Down
1 change: 1 addition & 0 deletions src/snapshot/repository/backends/oss/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading