mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
feat: configure S3 URL addressing style (#3400)
## Summary - add one strict `BUZZ_S3_ADDRESSING_STYLE=path|virtual` setting shared by media and Git/CAS storage - preserve path-style defaults for bundled Compose/Helm MinIO while supporting Railway's virtual-hosted bucket contract - fail startup on invalid or non-Unicode values before dependency connection, and validate the Helm value with the same two choices - document operator mappings and why endpoint and bucket remain separate for routing and SigV4 signing ## Best-practice rationale AWS documents both URL forms and favors virtual-hosted addressing for S3, while compatibility endpoints such as the bundled MinIO deployment can require path style. `rust-s3` defaults to virtual/subdomain addressing and provides `with_path_style()` for the explicit compatibility case. Some providers buckets only support as virtual-hosted bucket styles. This PR therefore uses one explicit, provider-neutral switch rather than endpoint heuristics or fallback behavior, while retaining `path` as Buzz's backward-compatible default. Sources: - https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html - https://docs.rs/rust-s3/0.37.0/s3/bucket/struct.Bucket.html - https://docs.railway.com/storage-buckets#url-style - https://github.com/minio/minio/blob/master/docs/config/README.md#domain ## Validation - `cargo fmt --all` - `cargo check --workspace --all-targets` - targeted `buzz-media` and `buzz-relay` parsing/client-construction tests for defaults, strict errors, and both URL styles - Helm unittest: 45/45 passed - Compose config/render validation passed - local MinIO path-mode relay startup passed the Git A3 conformance probe and became ready - unreachable object storage failed startup and readiness never opened - push hooks completed the broader Rust and desktop suites successfully --------- Signed-off-by: npub122y0pqkertljmedu303rl0aqrj3w8pvu43t6jxm6875lzg6f2pwqegc3xc <5288f082d91aff2de5bc8be23fbfa01ca2e3859cac57a91b7a3fa9f12349505c@buzz.block.builderlab.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz> Co-authored-by: npub122y0pqkertljmedu303rl0aqrj3w8pvu43t6jxm6875lzg6f2pwqegc3xc <5288f082d91aff2de5bc8be23fbfa01ca2e3859cac57a91b7a3fa9f12349505c@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -1583,22 +1583,15 @@ mod tests {
|
||||
}
|
||||
|
||||
fn live_store() -> GitStore {
|
||||
let endpoint = std::env::var("BUZZ_GIT_S3_ENDPOINT")
|
||||
.or_else(|_| std::env::var("BUZZ_S3_ENDPOINT"))
|
||||
.unwrap_or_else(|_| "http://localhost:9000".into());
|
||||
let access_key = std::env::var("BUZZ_GIT_S3_ACCESS_KEY")
|
||||
.or_else(|_| std::env::var("BUZZ_S3_ACCESS_KEY"))
|
||||
.unwrap_or_else(|_| "buzz_dev".into());
|
||||
let secret_key = std::env::var("BUZZ_GIT_S3_SECRET_KEY")
|
||||
.or_else(|_| std::env::var("BUZZ_S3_SECRET_KEY"))
|
||||
.unwrap_or_else(|_| "buzz_dev_secret".into());
|
||||
let bucket = std::env::var("BUZZ_GIT_S3_BUCKET")
|
||||
.or_else(|_| std::env::var("BUZZ_S3_BUCKET"))
|
||||
.unwrap_or_else(|_| "buzz-media".into());
|
||||
let region = std::env::var("BUZZ_GIT_S3_REGION")
|
||||
.or_else(|_| std::env::var("BUZZ_S3_REGION"))
|
||||
.unwrap_or_else(|_| "us-east-1".into());
|
||||
GitStore::new(&endpoint, &access_key, &secret_key, &bucket, ®ion).expect("connect minio")
|
||||
GitStore::new(
|
||||
"http://localhost:9000",
|
||||
"buzz_dev",
|
||||
"buzz_dev_secret",
|
||||
"buzz-media",
|
||||
"us-east-1",
|
||||
buzz_media::config::S3AddressingStyle::Path,
|
||||
)
|
||||
.expect("connect local MinIO")
|
||||
}
|
||||
|
||||
fn tenant() -> TenantContext {
|
||||
|
||||
@@ -543,8 +543,15 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn materialized_repo_is_created_under_configured_scratch_dir() {
|
||||
let scratch = TempDir::new().unwrap();
|
||||
let store = GitStore::new("http://localhost:9000", "x", "x", "x", "us-east-1")
|
||||
.expect("construct store");
|
||||
let store = GitStore::new(
|
||||
"http://localhost:9000",
|
||||
"x",
|
||||
"x",
|
||||
"x",
|
||||
"us-east-1",
|
||||
buzz_media::config::S3AddressingStyle::Path,
|
||||
)
|
||||
.expect("construct store");
|
||||
let manifest = Manifest {
|
||||
version: 1,
|
||||
head: "refs/heads/main".into(),
|
||||
@@ -587,8 +594,9 @@ mod tests {
|
||||
"buzz_dev_secret",
|
||||
"buzz-git",
|
||||
"us-east-1",
|
||||
buzz_media::config::S3AddressingStyle::Path,
|
||||
)
|
||||
.expect("connect minio")
|
||||
.expect("connect local MinIO")
|
||||
}
|
||||
|
||||
/// Build a tiny on-disk repo, return (pack bytes, head_oid).
|
||||
|
||||
@@ -174,7 +174,9 @@ pub struct GitStore {
|
||||
impl GitStore {
|
||||
/// Build a client against an S3-compatible endpoint (e.g. MinIO).
|
||||
///
|
||||
/// Uses path-style addressing for MinIO compatibility; AWS S3 accepts both.
|
||||
/// `addressing_style` is shared with media storage so both paths sign and
|
||||
/// route requests consistently. Path style supports the bundled MinIO DNS;
|
||||
/// virtual-hosted style supports standard S3 and providers such as Railway.
|
||||
///
|
||||
/// Credential selection mirrors [`buzz_media::MediaStorage::new`]:
|
||||
/// - both `access_key` and `secret_key` non-empty → static credentials
|
||||
@@ -190,6 +192,7 @@ impl GitStore {
|
||||
secret_key: &str,
|
||||
bucket_name: &str,
|
||||
region: &str,
|
||||
addressing_style: buzz_media::config::S3AddressingStyle,
|
||||
) -> Result<Self, StoreError> {
|
||||
let region = Region::Custom {
|
||||
region: region.into(),
|
||||
@@ -209,9 +212,11 @@ impl GitStore {
|
||||
}
|
||||
}
|
||||
.map_err(|e| StoreError::Backend(S3Error::Credentials(e)))?;
|
||||
let bucket = Bucket::new(bucket_name, region, creds)
|
||||
.map_err(StoreError::Backend)?
|
||||
.with_path_style();
|
||||
let bucket = Bucket::new(bucket_name, region, creds).map_err(StoreError::Backend)?;
|
||||
let bucket = match addressing_style {
|
||||
buzz_media::config::S3AddressingStyle::Path => bucket.with_path_style(),
|
||||
buzz_media::config::S3AddressingStyle::Virtual => bucket,
|
||||
};
|
||||
Ok(Self {
|
||||
bucket: Arc::from(bucket),
|
||||
})
|
||||
@@ -950,6 +955,7 @@ mod tests {
|
||||
"buzz_dev_secret",
|
||||
"buzz-git",
|
||||
"us-west-2",
|
||||
buzz_media::config::S3AddressingStyle::Path,
|
||||
)
|
||||
.expect("static creds should build a git store");
|
||||
match store.bucket.region {
|
||||
@@ -958,6 +964,34 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn constructor_applies_both_addressing_styles() {
|
||||
for (style, expected_url, path_style) in [
|
||||
(
|
||||
buzz_media::config::S3AddressingStyle::Path,
|
||||
"https://storage.example/buzz-git",
|
||||
true,
|
||||
),
|
||||
(
|
||||
buzz_media::config::S3AddressingStyle::Virtual,
|
||||
"https://buzz-git.storage.example",
|
||||
false,
|
||||
),
|
||||
] {
|
||||
let store = GitStore::new(
|
||||
"https://storage.example",
|
||||
"buzz_dev",
|
||||
"buzz_dev_secret",
|
||||
"buzz-git",
|
||||
"us-east-1",
|
||||
style,
|
||||
)
|
||||
.expect("construct git store");
|
||||
assert_eq!(store.bucket.url(), expected_url);
|
||||
assert_eq!(store.bucket.is_path_style(), path_style);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn partial_static_keys_are_rejected() {
|
||||
for (access, secret) in [("buzz_dev", ""), ("", "buzz_dev_secret")] {
|
||||
@@ -967,6 +1001,7 @@ mod tests {
|
||||
secret,
|
||||
"buzz-git",
|
||||
"us-east-1",
|
||||
buzz_media::config::S3AddressingStyle::Path,
|
||||
) {
|
||||
Ok(_) => {
|
||||
panic!("partial static creds must not silently use the credential chain")
|
||||
@@ -998,14 +1033,29 @@ mod probe {
|
||||
}
|
||||
|
||||
fn store() -> GitStore {
|
||||
// This is the dedicated backend conformance path, so all connection and
|
||||
// signing inputs are overridable for a real provider such as Railway.
|
||||
// The hydrate/CAS live tests use explicit local MinIO fixtures instead.
|
||||
let endpoint =
|
||||
std::env::var("BUZZ_S3_ENDPOINT").unwrap_or_else(|_| "http://localhost:9000".into());
|
||||
let access_key = std::env::var("BUZZ_S3_ACCESS_KEY").unwrap_or_else(|_| "buzz_dev".into());
|
||||
let secret_key =
|
||||
std::env::var("BUZZ_S3_SECRET_KEY").unwrap_or_else(|_| "buzz_dev_secret".into());
|
||||
let bucket = std::env::var("BUZZ_S3_BUCKET").unwrap_or_else(|_| "buzz-git".into());
|
||||
let region = std::env::var("BUZZ_S3_REGION").unwrap_or_else(|_| "us-east-1".into());
|
||||
let addressing_style = std::env::var("BUZZ_S3_ADDRESSING_STYLE")
|
||||
.unwrap_or_else(|_| "path".into())
|
||||
.parse()
|
||||
.expect("BUZZ_S3_ADDRESSING_STYLE must be path or virtual");
|
||||
GitStore::new(
|
||||
"http://localhost:9000",
|
||||
"buzz_dev",
|
||||
"buzz_dev_secret",
|
||||
"buzz-git",
|
||||
"us-east-1",
|
||||
&endpoint,
|
||||
&access_key,
|
||||
&secret_key,
|
||||
&bucket,
|
||||
®ion,
|
||||
addressing_style,
|
||||
)
|
||||
.expect("connect minio")
|
||||
.expect("connect S3-compatible storage")
|
||||
}
|
||||
|
||||
fn sha256_hex(b: &[u8]) -> String {
|
||||
|
||||
@@ -630,6 +630,16 @@ impl Config {
|
||||
.and_then(|v| v.parse().ok())
|
||||
.unwrap_or(9102);
|
||||
|
||||
let s3_addressing_style = match std::env::var("BUZZ_S3_ADDRESSING_STYLE") {
|
||||
Ok(value) => value.parse().map_err(ConfigError::InvalidValue)?,
|
||||
Err(std::env::VarError::NotPresent) => buzz_media::config::S3AddressingStyle::default(),
|
||||
Err(std::env::VarError::NotUnicode(_)) => {
|
||||
return Err(ConfigError::InvalidValue(
|
||||
"BUZZ_S3_ADDRESSING_STYLE must be valid Unicode and one of 'path' or 'virtual'"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
};
|
||||
let media = buzz_media::MediaConfig {
|
||||
s3_endpoint: std::env::var("BUZZ_S3_ENDPOINT")
|
||||
.unwrap_or_else(|_| "http://localhost:9000".to_string()),
|
||||
@@ -641,6 +651,7 @@ impl Config {
|
||||
s3_region: std::env::var("BUZZ_S3_REGION")
|
||||
.or_else(|_| std::env::var("AWS_REGION"))
|
||||
.unwrap_or_else(|_| "us-east-1".to_string()),
|
||||
s3_addressing_style,
|
||||
max_image_bytes: std::env::var("BUZZ_MAX_IMAGE_BYTES")
|
||||
.ok()
|
||||
.and_then(|v| v.parse().ok())
|
||||
@@ -990,6 +1001,11 @@ mod tests {
|
||||
!config.require_media_get_auth,
|
||||
"require_media_get_auth should default to false for staged client rollout"
|
||||
);
|
||||
assert_eq!(
|
||||
config.media.s3_addressing_style,
|
||||
buzz_media::config::S3AddressingStyle::Path,
|
||||
"S3 addressing must default to path style for bundled MinIO compatibility"
|
||||
);
|
||||
assert!(
|
||||
config.join_policy.is_none(),
|
||||
"join_policy should default to None so policy prompts and acceptance receipts are opt-in"
|
||||
@@ -1000,6 +1016,61 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn s3_addressing_style_env_accepts_virtual_and_rejects_invalid_values() {
|
||||
let _guard = ENV_MUTEX.lock().unwrap();
|
||||
let previous = std::env::var_os("BUZZ_S3_ADDRESSING_STYLE");
|
||||
|
||||
std::env::set_var("BUZZ_S3_ADDRESSING_STYLE", "virtual");
|
||||
let configured = Config::from_env()
|
||||
.expect("virtual style config")
|
||||
.media
|
||||
.s3_addressing_style;
|
||||
|
||||
std::env::set_var("BUZZ_S3_ADDRESSING_STYLE", "auto");
|
||||
let invalid = Config::from_env();
|
||||
|
||||
if let Some(value) = previous {
|
||||
std::env::set_var("BUZZ_S3_ADDRESSING_STYLE", value);
|
||||
} else {
|
||||
std::env::remove_var("BUZZ_S3_ADDRESSING_STYLE");
|
||||
}
|
||||
|
||||
assert_eq!(configured, buzz_media::config::S3AddressingStyle::Virtual);
|
||||
assert!(matches!(
|
||||
invalid,
|
||||
Err(ConfigError::InvalidValue(ref message))
|
||||
if message.contains("BUZZ_S3_ADDRESSING_STYLE must be 'path' or 'virtual'")
|
||||
));
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn s3_addressing_style_env_rejects_non_unicode_values() {
|
||||
use std::os::unix::ffi::OsStringExt;
|
||||
|
||||
let _guard = ENV_MUTEX.lock().unwrap();
|
||||
let previous = std::env::var_os("BUZZ_S3_ADDRESSING_STYLE");
|
||||
std::env::set_var(
|
||||
"BUZZ_S3_ADDRESSING_STYLE",
|
||||
std::ffi::OsString::from_vec(vec![0xff]),
|
||||
);
|
||||
|
||||
let invalid = Config::from_env();
|
||||
|
||||
if let Some(value) = previous {
|
||||
std::env::set_var("BUZZ_S3_ADDRESSING_STYLE", value);
|
||||
} else {
|
||||
std::env::remove_var("BUZZ_S3_ADDRESSING_STYLE");
|
||||
}
|
||||
|
||||
assert!(matches!(
|
||||
invalid,
|
||||
Err(ConfigError::InvalidValue(ref message))
|
||||
if message.contains("must be valid Unicode")
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redis_pool_size_env_override_and_invalid_fallback() {
|
||||
let _guard = ENV_MUTEX.lock().unwrap();
|
||||
|
||||
@@ -697,6 +697,7 @@ impl AppState {
|
||||
&config.media.s3_secret_key,
|
||||
&config.media.s3_bucket,
|
||||
&config.media.s3_region,
|
||||
config.media.s3_addressing_style,
|
||||
)
|
||||
.expect("media storage was already constructed with this S3 config");
|
||||
let git_pack_cache = Arc::new(
|
||||
|
||||
Reference in New Issue
Block a user