From 47526784d5c1967de6b2c5d1ee642bccfefbfab9 Mon Sep 17 00:00:00 2001 From: Brad Seiler Date: Fri, 21 Aug 2026 19:14:00 -0400 Subject: [PATCH] fix(deletion): allow IRSA S3 credentials (#6495) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why `buzz-admin deletions` runs inside bb-public relay pods, where S3 credentials are provided by the pod credential chain rather than static `BUZZ_S3_ACCESS_KEY` / `BUZZ_S3_SECRET_KEY` values. The deletion CLI was requiring those two env vars to be non-empty before constructing the shared media storage client, so it could not reach the existing default AWS credential chain. ## What - Treat missing/blank deletion S3 access key and secret key as an empty credential pair so `MediaStorage::new` can use `Credentials::default()`. - Keep the existing static-credential path unchanged when both values are non-empty. - Keep deletion unit tests deterministic by covering only deletion env normalization for missing/blank pair, trimmed static pair, and partial/whitespace-partial outputs; shared media tests continue to own credential-pair enforcement. ## Risk Assessment Low and scoped to the operator-only community deletion CLI startup path. The shared media storage credential validation still owns static-vs-default credential selection and still rejects mixed partial credentials. ## Testing At committed head `0a86c2914b1f97caf4788a771048aa8d9d9d88ac` with a clean worktree before and after (`git rev-parse HEAD` before/after matched): - `just fmt-check` — passed. - `cargo test -p buzz-deletion` — passed: 12 passed, 9 ignored. - `cargo test -p buzz-media` — passed: 120 passed; `static_creds_round_trip_against_minio` remained ignored because it requires live MinIO. - `cargo test -p buzz-admin` — passed: 1 passed. - `cargo clippy -p buzz-deletion --all-targets -- -D warnings` — passed. - Startup smoke at the same head: built `buzz-admin`, then ran `target/debug/buzz-admin deletions drain` with `BUZZ_S3_ACCESS_KEY=` and `BUZZ_S3_SECRET_KEY=' '` plus `AWS_ACCESS_KEY_ID` / `AWS_SECRET_ACCESS_KEY` fallback credentials; command exited `0`, proving startup transitions past deletion S3 key validation and exercises the shared default credential-chain branch using AWS env fallback credentials. - `git push origin HEAD:seiler/deletion-irsa-credentials` — passed; pre-push hooks passed. Not run: the full `TESTING.md` live-local relay workflow. Docker Desktop currently refuses CLI access on this machine with `Sign in to continue using Docker Desktop. Membership in the [squareup] organization is required.` ## References - Buzz channel: `buzz://message?channel=9e4aabc6-414c-4978-aba7-b9f5228776de&id=177c5b8ad9e78a647f438ec7040d25f0c20a7c2b8678dd0e34768effe053f7f4` Generated with Codex --------- Signed-off-by: coder 0 Co-authored-by: coder 0 --- crates/buzz-deletion/src/lib.rs | 98 ++++++++++++++++++++++++++++++--- 1 file changed, 89 insertions(+), 9 deletions(-) diff --git a/crates/buzz-deletion/src/lib.rs b/crates/buzz-deletion/src/lib.rs index 4e27b85fe9..ae3dbe4f39 100644 --- a/crates/buzz-deletion/src/lib.rs +++ b/crates/buzz-deletion/src/lib.rs @@ -547,23 +547,17 @@ fn nonempty_s3_region(region: String) -> Option { (!region.is_empty()).then(|| region.to_string()) } -fn s3_region_from_env() -> String { - resolve_s3_region( - std::env::var("BUZZ_S3_REGION").ok(), - std::env::var("AWS_REGION").ok(), - ) -} - async fn connect_services() -> Result { let store = connect_store().await?; connect_services_with_store(store).await } async fn connect_services_with_store(store: DeletionStore) -> Result { + let (s3_access_key, s3_secret_key) = s3_key_pair_from_env(); let media_config = buzz_media::MediaConfig { s3_endpoint: required_env("BUZZ_S3_ENDPOINT")?, - s3_access_key: required_env("BUZZ_S3_ACCESS_KEY")?, - s3_secret_key: required_env("BUZZ_S3_SECRET_KEY")?, + s3_access_key, + s3_secret_key, s3_bucket: required_env("BUZZ_S3_BUCKET")?, s3_region: s3_region_from_env(), s3_addressing_style: std::env::var("BUZZ_S3_ADDRESSING_STYLE") @@ -596,6 +590,24 @@ async fn connect_services_with_store(store: DeletionStore) -> Result { }) } +fn s3_region_from_env() -> String { + resolve_s3_region( + std::env::var("BUZZ_S3_REGION").ok(), + std::env::var("AWS_REGION").ok(), + ) +} + +fn s3_key_pair_from_env() -> (String, String) { + s3_key_pair_from(|name| std::env::var(name).ok()) +} + +fn s3_key_pair_from(get_env: impl Fn(&str) -> Option) -> (String, String) { + ( + optional_env_from(&get_env, "BUZZ_S3_ACCESS_KEY"), + optional_env_from(&get_env, "BUZZ_S3_SECRET_KEY"), + ) +} + fn required_env(name: &str) -> Result { std::env::var(name) .ok() @@ -604,6 +616,12 @@ fn required_env(name: &str) -> Result { .ok_or_else(|| anyhow::anyhow!("{name} is required for community deletion")) } +fn optional_env_from(get_env: impl Fn(&str) -> Option, name: &str) -> String { + get_env(name) + .map(|value| value.trim().to_owned()) + .unwrap_or_default() +} + fn env_parse(name: &str, default: T) -> T where T: std::str::FromStr, @@ -1543,6 +1561,68 @@ mod tests { (db, services, claim) } + fn env_of<'a>(set: &'a [(&'a str, &'a str)]) -> impl Fn(&str) -> Option + use<'a> { + move |name| { + set.iter() + .find(|(key, _)| *key == name) + .map(|(_, value)| (*value).to_string()) + } + } + + #[test] + fn deletion_s3_key_pair_normalizes_missing_and_blank_pairs_for_default_credentials() { + assert_eq!( + s3_key_pair_from(env_of(&[])), + (String::new(), String::new()) + ); + + assert_eq!( + s3_key_pair_from(env_of(&[ + ("BUZZ_S3_ACCESS_KEY", ""), + ("BUZZ_S3_SECRET_KEY", " "), + ])), + (String::new(), String::new()) + ); + } + + #[test] + fn deletion_s3_key_pair_trims_static_and_preserves_partial_pairs() { + assert_eq!( + s3_key_pair_from(env_of(&[ + ("BUZZ_S3_ACCESS_KEY", " buzz_dev "), + ("BUZZ_S3_SECRET_KEY", " buzz_dev_secret "), + ])), + ("buzz_dev".to_string(), "buzz_dev_secret".to_string()) + ); + + for (env, expected) in [ + ( + &[("BUZZ_S3_ACCESS_KEY", " buzz_dev ")][..], + ("buzz_dev".to_string(), String::new()), + ), + ( + &[("BUZZ_S3_SECRET_KEY", " buzz_dev_secret ")][..], + (String::new(), "buzz_dev_secret".to_string()), + ), + ( + &[ + ("BUZZ_S3_ACCESS_KEY", " buzz_dev "), + ("BUZZ_S3_SECRET_KEY", " "), + ][..], + ("buzz_dev".to_string(), String::new()), + ), + ( + &[ + ("BUZZ_S3_ACCESS_KEY", " "), + ("BUZZ_S3_SECRET_KEY", " buzz_dev_secret "), + ][..], + (String::new(), "buzz_dev_secret".to_string()), + ), + ] { + assert_eq!(s3_key_pair_from(env_of(env)), expected); + } + } + fn deletion_test_media_storage() -> Arc { let endpoint = std::env::var("BUZZ_TEST_S3_ENDPOINT") .or_else(|_| std::env::var("BUZZ_S3_ENDPOINT"))