mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(media): support IRSA/credential-chain S3 auth and configurable signing region (#1406)
Signed-off-by: Tyler Longwell <tlongwell@block.xyz> Co-authored-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Tyler Longwell <tlongwell@block.xyz>
This commit is contained in:
co-authored by
npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d
Tyler Longwell
parent
f061ae9c87
commit
06ef533ec7
@@ -8,6 +8,10 @@ fn default_max_file_bytes() -> u64 {
|
||||
104_857_600 // 100 MB
|
||||
}
|
||||
|
||||
fn default_s3_region() -> String {
|
||||
"us-east-1".to_string()
|
||||
}
|
||||
|
||||
/// Configuration for media storage (S3/MinIO).
|
||||
#[derive(Debug, Clone, serde::Deserialize)]
|
||||
pub struct MediaConfig {
|
||||
@@ -19,6 +23,14 @@ pub struct MediaConfig {
|
||||
pub s3_secret_key: String,
|
||||
/// S3 bucket name.
|
||||
pub s3_bucket: String,
|
||||
/// AWS region for SigV4 request signing (e.g. "us-west-2").
|
||||
///
|
||||
/// Must match the region of `s3_endpoint` for real AWS S3, otherwise
|
||||
/// requests are signed with the wrong credential scope and AWS rejects
|
||||
/// them. Defaults to "us-east-1" to preserve MinIO/local behavior, where
|
||||
/// the value is not meaningfully checked.
|
||||
#[serde(default = "default_s3_region")]
|
||||
pub s3_region: String,
|
||||
/// Maximum upload size for images (bytes). Default: 50 MB.
|
||||
pub max_image_bytes: u64,
|
||||
/// Maximum upload size for animated GIFs (bytes). Default: 10 MB.
|
||||
|
||||
@@ -22,18 +22,43 @@ pub struct MediaStorage {
|
||||
|
||||
impl MediaStorage {
|
||||
/// Create a new storage client from config.
|
||||
///
|
||||
/// Credential selection:
|
||||
/// - If both `s3_access_key` and `s3_secret_key` are non-empty, use them as
|
||||
/// static credentials (MinIO/local/dev, or any static-key deployment).
|
||||
/// - Otherwise, fall back to the AWS default credential chain via
|
||||
/// [`Credentials::default`]: environment, shared profile, web-identity
|
||||
/// token (IRSA on EKS — `AssumeRoleWithWebIdentity`), container, and
|
||||
/// instance-metadata providers, in that order. This lets the relay use
|
||||
/// the pod's IAM role without long-lived static keys.
|
||||
pub fn new(config: &MediaConfig) -> Result<Self, MediaError> {
|
||||
let region = Region::Custom {
|
||||
region: "us-east-1".into(),
|
||||
region: config.s3_region.clone(),
|
||||
endpoint: config.s3_endpoint.clone(),
|
||||
};
|
||||
let creds = Credentials::new(
|
||||
Some(&config.s3_access_key),
|
||||
Some(&config.s3_secret_key),
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
)
|
||||
let creds = match (
|
||||
config.s3_access_key.is_empty(),
|
||||
config.s3_secret_key.is_empty(),
|
||||
) {
|
||||
(false, false) => Credentials::new(
|
||||
Some(&config.s3_access_key),
|
||||
Some(&config.s3_secret_key),
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
),
|
||||
(true, true) => {
|
||||
// No static keys configured: resolve from the AWS credential chain
|
||||
// (IRSA web-identity, env, profile, instance metadata).
|
||||
Credentials::default()
|
||||
}
|
||||
_ => {
|
||||
return Err(MediaError::StorageError(
|
||||
"s3_access_key and s3_secret_key must be configured together, or both empty to use the AWS credential chain"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
.map_err(|e| MediaError::StorageError(e.to_string()))?;
|
||||
let bucket = Bucket::new(&config.s3_bucket, region, creds)
|
||||
.map_err(|e| MediaError::StorageError(e.to_string()))?
|
||||
@@ -219,6 +244,55 @@ mod tests {
|
||||
)
|
||||
}
|
||||
|
||||
fn storage_config(access: &str, secret: &str) -> crate::config::MediaConfig {
|
||||
crate::config::MediaConfig {
|
||||
s3_endpoint: "http://localhost:9000".to_string(),
|
||||
s3_access_key: access.to_string(),
|
||||
s3_secret_key: secret.to_string(),
|
||||
s3_bucket: "buzz-media".to_string(),
|
||||
s3_region: "us-west-2".to_string(),
|
||||
max_image_bytes: 50 * 1024 * 1024,
|
||||
max_gif_bytes: 10 * 1024 * 1024,
|
||||
max_video_bytes: 524_288_000,
|
||||
max_file_bytes: 104_857_600,
|
||||
public_base_url: "http://localhost:3000/media".to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Static keys present: builds a client without touching the AWS
|
||||
/// credential chain (no env/metadata access), and the signing region
|
||||
/// comes from config rather than a hardcoded "us-east-1".
|
||||
#[test]
|
||||
fn static_keys_build_client_with_configured_region() {
|
||||
let storage = MediaStorage::new(&storage_config("buzz_dev", "buzz_dev_secret"))
|
||||
.expect("static creds should build a client");
|
||||
match storage.bucket.region {
|
||||
Region::Custom { ref region, .. } => assert_eq!(region, "us-west-2"),
|
||||
other => panic!("expected Custom region, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn partial_static_keys_are_rejected() {
|
||||
let err = match MediaStorage::new(&storage_config("buzz_dev", "")) {
|
||||
Ok(_) => panic!("partial static creds must not silently use credential chain"),
|
||||
Err(err) => err,
|
||||
};
|
||||
assert!(
|
||||
err.to_string().contains("must be configured together"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
|
||||
let err = match MediaStorage::new(&storage_config("", "buzz_dev_secret")) {
|
||||
Ok(_) => panic!("partial static creds must not silently use credential chain"),
|
||||
Err(err) => err,
|
||||
};
|
||||
assert!(
|
||||
err.to_string().contains("must be configured together"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sidecar_keys_are_community_scoped() {
|
||||
let a = tenant(1);
|
||||
|
||||
@@ -461,6 +461,7 @@ mod tests {
|
||||
s3_access_key: String::new(),
|
||||
s3_secret_key: String::new(),
|
||||
s3_bucket: String::new(),
|
||||
s3_region: "us-east-1".to_string(),
|
||||
max_image_bytes: 50 * 1024 * 1024,
|
||||
max_gif_bytes: 10 * 1024 * 1024,
|
||||
max_video_bytes: 524_288_000,
|
||||
|
||||
@@ -416,6 +416,7 @@ mod tests {
|
||||
s3_access_key: String::new(),
|
||||
s3_secret_key: String::new(),
|
||||
s3_bucket: String::new(),
|
||||
s3_region: "us-east-1".to_string(),
|
||||
max_image_bytes: 50 * 1024 * 1024,
|
||||
max_gif_bytes: 10 * 1024 * 1024,
|
||||
max_video_bytes: 524_288_000,
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
//! Live round-trip test for the **static-credentials** S3 path against a local
|
||||
//! MinIO, guarded by `#[ignore]`.
|
||||
//!
|
||||
//! This is the path local/dev and any static-key deployment uses
|
||||
//! (`s3_access_key`/`s3_secret_key` both non-empty -> `Credentials::new`). It
|
||||
//! exists to prove that adding the IRSA/credential-chain fallback did **not**
|
||||
//! regress hardcoded credentials.
|
||||
//!
|
||||
//! Run it against the docker-compose MinIO (creds `buzz_dev`/`buzz_dev_secret`,
|
||||
//! bucket `buzz-media`, endpoint `http://localhost:9000`):
|
||||
//!
|
||||
//! ```bash
|
||||
//! docker compose up -d minio minio-init
|
||||
//! cargo test -p buzz-media --test static_creds_minio -- --ignored
|
||||
//! ```
|
||||
//!
|
||||
//! Overridable via `BUZZ_S3_ENDPOINT` / `BUZZ_S3_ACCESS_KEY` /
|
||||
//! `BUZZ_S3_SECRET_KEY` / `BUZZ_S3_BUCKET`.
|
||||
|
||||
use buzz_media::config::MediaConfig;
|
||||
use buzz_media::storage::MediaStorage;
|
||||
|
||||
fn minio_config() -> MediaConfig {
|
||||
MediaConfig {
|
||||
s3_endpoint: std::env::var("BUZZ_S3_ENDPOINT")
|
||||
.unwrap_or_else(|_| "http://localhost:9000".to_string()),
|
||||
s3_access_key: std::env::var("BUZZ_S3_ACCESS_KEY")
|
||||
.unwrap_or_else(|_| "buzz_dev".to_string()),
|
||||
s3_secret_key: std::env::var("BUZZ_S3_SECRET_KEY")
|
||||
.unwrap_or_else(|_| "buzz_dev_secret".to_string()),
|
||||
s3_bucket: std::env::var("BUZZ_S3_BUCKET").unwrap_or_else(|_| "buzz-media".to_string()),
|
||||
s3_region: "us-east-1".to_string(),
|
||||
max_image_bytes: 50 * 1024 * 1024,
|
||||
max_gif_bytes: 10 * 1024 * 1024,
|
||||
max_video_bytes: 524_288_000,
|
||||
max_file_bytes: 104_857_600,
|
||||
public_base_url: "http://localhost:3000/media".to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[ignore = "requires a live MinIO (docker compose up -d minio minio-init)"]
|
||||
async fn static_creds_round_trip_against_minio() {
|
||||
let storage =
|
||||
MediaStorage::new(&minio_config()).expect("static creds should build a storage client");
|
||||
|
||||
let key = format!("_test/static-creds-{}.bin", std::process::id());
|
||||
let body = b"hardcoded-creds-still-work";
|
||||
|
||||
// PUT
|
||||
storage
|
||||
.put(&key, body, "application/octet-stream")
|
||||
.await
|
||||
.expect("put with static creds should succeed");
|
||||
|
||||
// HEAD -> exists with correct size
|
||||
assert!(storage.head(&key).await.expect("head should succeed"));
|
||||
let meta = storage
|
||||
.head_with_metadata(&key)
|
||||
.await
|
||||
.expect("head_with_metadata should succeed")
|
||||
.expect("object should exist");
|
||||
assert_eq!(meta.size, body.len() as u64);
|
||||
|
||||
// GET round-trips the bytes
|
||||
let got = storage.get(&key).await.expect("get should succeed");
|
||||
assert_eq!(got, body);
|
||||
|
||||
// DELETE, then HEAD reports absence
|
||||
storage.delete(&key).await.expect("delete should succeed");
|
||||
assert!(!storage.head(&key).await.expect("head after delete"));
|
||||
}
|
||||
@@ -459,6 +459,7 @@ mod tests {
|
||||
"buzz_dev",
|
||||
"buzz_dev_secret",
|
||||
"buzz-git",
|
||||
"us-east-1",
|
||||
)
|
||||
.expect("connect minio")
|
||||
}
|
||||
|
||||
@@ -91,6 +91,9 @@ pub enum StoreError {
|
||||
/// Any other backend / transport error.
|
||||
#[error("s3 backend error: {0}")]
|
||||
Backend(#[from] S3Error),
|
||||
/// Invalid storage configuration detected at client construction.
|
||||
#[error("git store config error: {0}")]
|
||||
Config(String),
|
||||
/// Conformance probe failed — backend does not satisfy A1/A2/A3.
|
||||
#[error(transparent)]
|
||||
Probe(ProbeFailure),
|
||||
@@ -172,18 +175,40 @@ impl GitStore {
|
||||
/// Build a client against an S3-compatible endpoint (e.g. MinIO).
|
||||
///
|
||||
/// Uses path-style addressing for MinIO compatibility; AWS S3 accepts both.
|
||||
///
|
||||
/// Credential selection mirrors [`buzz_media::MediaStorage::new`]:
|
||||
/// - both `access_key` and `secret_key` non-empty → static credentials
|
||||
/// (MinIO/local/dev, or any static-key deployment);
|
||||
/// - both empty → the AWS default credential chain via
|
||||
/// [`Credentials::default`] (env, profile, web-identity/IRSA, container,
|
||||
/// instance metadata), so the relay can use its pod IAM role;
|
||||
/// - exactly one empty → a config error, to surface a half-configured
|
||||
/// static deployment instead of silently falling back to the chain.
|
||||
pub fn new(
|
||||
endpoint: &str,
|
||||
access_key: &str,
|
||||
secret_key: &str,
|
||||
bucket_name: &str,
|
||||
region: &str,
|
||||
) -> Result<Self, StoreError> {
|
||||
let region = Region::Custom {
|
||||
region: "us-east-1".into(),
|
||||
region: region.into(),
|
||||
endpoint: endpoint.into(),
|
||||
};
|
||||
let creds = Credentials::new(Some(access_key), Some(secret_key), None, None, None)
|
||||
.map_err(|e| StoreError::Backend(S3Error::Credentials(e)))?;
|
||||
let creds = match (access_key.is_empty(), secret_key.is_empty()) {
|
||||
(false, false) => Credentials::new(Some(access_key), Some(secret_key), None, None, None),
|
||||
(true, true) => {
|
||||
// No static keys configured: resolve from the AWS credential
|
||||
// chain (IRSA web-identity, env, profile, instance metadata).
|
||||
Credentials::default()
|
||||
}
|
||||
_ => {
|
||||
return Err(StoreError::Config(
|
||||
"s3 access_key and secret_key must be configured together, or both empty to use the AWS credential chain".to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
.map_err(|e| StoreError::Backend(S3Error::Credentials(e)))?;
|
||||
let bucket = Bucket::new(bucket_name, region, creds)
|
||||
.map_err(StoreError::Backend)?
|
||||
.with_path_style();
|
||||
@@ -896,6 +921,44 @@ mod tests {
|
||||
Err(StoreError::Backend(S3Error::HttpFailWithBody(403, _)))
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn static_keys_build_store_with_configured_region() {
|
||||
let store = GitStore::new(
|
||||
"http://localhost:9000",
|
||||
"buzz_dev",
|
||||
"buzz_dev_secret",
|
||||
"buzz-git",
|
||||
"us-west-2",
|
||||
)
|
||||
.expect("static creds should build a git store");
|
||||
match store.bucket.region {
|
||||
Region::Custom { ref region, .. } => assert_eq!(region, "us-west-2"),
|
||||
ref other => panic!("expected Custom region, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn partial_static_keys_are_rejected() {
|
||||
for (access, secret) in [("buzz_dev", ""), ("", "buzz_dev_secret")] {
|
||||
let err = match GitStore::new(
|
||||
"http://localhost:9000",
|
||||
access,
|
||||
secret,
|
||||
"buzz-git",
|
||||
"us-east-1",
|
||||
) {
|
||||
Ok(_) => {
|
||||
panic!("partial static creds must not silently use the credential chain")
|
||||
}
|
||||
Err(err) => err,
|
||||
};
|
||||
assert!(
|
||||
matches!(err, StoreError::Config(_)),
|
||||
"expected Config error, got {err:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -920,6 +983,7 @@ mod probe {
|
||||
"buzz_dev",
|
||||
"buzz_dev_secret",
|
||||
"buzz-git",
|
||||
"us-east-1",
|
||||
)
|
||||
.expect("connect minio")
|
||||
}
|
||||
|
||||
@@ -296,6 +296,9 @@ impl Config {
|
||||
s3_secret_key: std::env::var("BUZZ_S3_SECRET_KEY")
|
||||
.unwrap_or_else(|_| "buzz_dev_secret".to_string()),
|
||||
s3_bucket: std::env::var("BUZZ_S3_BUCKET").unwrap_or_else(|_| "buzz-media".to_string()),
|
||||
s3_region: std::env::var("BUZZ_S3_REGION")
|
||||
.or_else(|_| std::env::var("AWS_REGION"))
|
||||
.unwrap_or_else(|_| "us-east-1".to_string()),
|
||||
max_image_bytes: std::env::var("BUZZ_MAX_IMAGE_BYTES")
|
||||
.ok()
|
||||
.and_then(|v| v.parse().ok())
|
||||
|
||||
@@ -370,6 +370,7 @@ impl AppState {
|
||||
&config.media.s3_access_key,
|
||||
&config.media.s3_secret_key,
|
||||
&config.media.s3_bucket,
|
||||
&config.media.s3_region,
|
||||
)
|
||||
.expect("media storage was already constructed with this S3 config");
|
||||
let nip98_replay: Arc<dyn Nip98ReplayGuard> =
|
||||
|
||||
Reference in New Issue
Block a user