mirror of
https://github.com/block/buzz.git
synced 2026-10-27 12:40:30 +00:00
fix(deletion): allow IRSA S3 credentials (#6495)
## 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 <d97ebdbb198c7237c94f84ea8bb8a73583ea067407eebd0062abbb3962527fb1@buzz.block.builderlab.xyz> Co-authored-by: coder 0 <d97ebdbb198c7237c94f84ea8bb8a73583ea067407eebd0062abbb3962527fb1@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -547,23 +547,17 @@ fn nonempty_s3_region(region: String) -> Option<String> {
|
||||
(!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<Services> {
|
||||
let store = connect_store().await?;
|
||||
connect_services_with_store(store).await
|
||||
}
|
||||
|
||||
async fn connect_services_with_store(store: DeletionStore) -> Result<Services> {
|
||||
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<Services> {
|
||||
})
|
||||
}
|
||||
|
||||
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, 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<String> {
|
||||
std::env::var(name)
|
||||
.ok()
|
||||
@@ -604,6 +616,12 @@ fn required_env(name: &str) -> Result<String> {
|
||||
.ok_or_else(|| anyhow::anyhow!("{name} is required for community deletion"))
|
||||
}
|
||||
|
||||
fn optional_env_from(get_env: impl Fn(&str) -> Option<String>, name: &str) -> String {
|
||||
get_env(name)
|
||||
.map(|value| value.trim().to_owned())
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
fn env_parse<T>(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<String> + 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<MediaStorage> {
|
||||
let endpoint = std::env::var("BUZZ_TEST_S3_ENDPOINT")
|
||||
.or_else(|_| std::env::var("BUZZ_S3_ENDPOINT"))
|
||||
|
||||
Reference in New Issue
Block a user