From a2a9f64e37535e3b2b894a505b43ef6dca2fe73f Mon Sep 17 00:00:00 2001 From: haphungw Date: Wed, 28 Jan 2026 01:30:08 +0000 Subject: [PATCH 1/3] fix(secret_manager): garbage collect locational secrets in integration tests Fixes #4349 The stale secrets were not correctly removed since wrong arguments were passed into `delete_secret_by_project_and_location_and_secret`. This fix parses the secret name to properly delete stale secrets. --- .../src/secret_manager/openapi_locational.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/integration/src/secret_manager/openapi_locational.rs b/tests/integration/src/secret_manager/openapi_locational.rs index 24ac86b526..3131a6b648 100644 --- a/tests/integration/src/secret_manager/openapi_locational.rs +++ b/tests/integration/src/secret_manager/openapi_locational.rs @@ -399,12 +399,18 @@ async fn cleanup_stale_secrets( let pending = stale_secrets .iter() - .map(|secret_id| { + .map(|name| { + // format: projects/{project}/locations/{location}/secrets/{secret} + let parts: Vec<&str> = name.split('/').collect(); + let project = parts[1]; + let location = parts[3]; + let secret = parts[5]; + client .delete_secret_by_project_and_location_and_secret() - .set_project(project_id) - .set_location(location_id) - .set_secret(secret_id) + .set_project(project) + .set_location(location) + .set_secret(secret) .with_idempotency(true) .send() }) From 281872149ccf2d8d88b6c12658d6077f6a9c109f Mon Sep 17 00:00:00 2001 From: haphungw Date: Wed, 28 Jan 2026 05:04:35 +0000 Subject: [PATCH 2/3] fix(secret_manager): validate secret name format safely Prevent panics from invalid input by returning a descriptive error. --- .../src/secret_manager/openapi_locational.rs | 49 ++++++++++++++++--- 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/tests/integration/src/secret_manager/openapi_locational.rs b/tests/integration/src/secret_manager/openapi_locational.rs index 3131a6b648..774b84aadc 100644 --- a/tests/integration/src/secret_manager/openapi_locational.rs +++ b/tests/integration/src/secret_manager/openapi_locational.rs @@ -13,6 +13,7 @@ // limitations under the License. use crate::Result; +use anyhow::anyhow; use gax::options::RequestOptionsBuilder; use google_cloud_test_utils::runtime_config::{project_id, test_service_account}; use rand::{Rng, distr::Alphanumeric}; @@ -400,12 +401,7 @@ async fn cleanup_stale_secrets( let pending = stale_secrets .iter() .map(|name| { - // format: projects/{project}/locations/{location}/secrets/{secret} - let parts: Vec<&str> = name.split('/').collect(); - let project = parts[1]; - let location = parts[3]; - let secret = parts[5]; - + let (project, location, secret) = extract(name).unwrap(); client .delete_secret_by_project_and_location_and_secret() .set_project(project) @@ -425,3 +421,44 @@ async fn cleanup_stale_secrets( Ok(()) } + +fn extract(name: &str) -> Result<(&str, &str, &str)> { + // format: projects/{project}/locations/{location}/secrets/{secret} + let parts: Vec<&str> = name.split('/').collect(); + match parts[..] { + [ + "projects", + p, + "locations", + l, + "secrets", + s, + ] => Ok((p, l, s)), + _ => Err(anyhow!("invalid secret name: {}", name)), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_extract_valid_name() { + let name = "projects/my-project/locations/us-central1/secrets/my-secret"; + let (project, location, secret) = extract(name).unwrap(); + assert_eq!(project, "my-project"); + assert_eq!(location, "us-central1"); + assert_eq!(secret, "my-secret"); + } + + #[test] + fn test_extract_invalid_name() { + let name = "invalid/format"; + assert!(extract(name).is_err()); + } + + #[test] + fn test_extract_missing_parts() { + assert!(extract("projects/only-one").is_err()); + } +} From 6f699c849093826ed21aaae64ffd84b595e2ca55 Mon Sep 17 00:00:00 2001 From: haphungw Date: Wed, 28 Jan 2026 15:48:09 +0000 Subject: [PATCH 3/3] fix(secret_manager): prevent panic on invalid secret names --- .../src/secret_manager/openapi_locational.rs | 56 ++++++++----------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/tests/integration/src/secret_manager/openapi_locational.rs b/tests/integration/src/secret_manager/openapi_locational.rs index 774b84aadc..8a08b1eb87 100644 --- a/tests/integration/src/secret_manager/openapi_locational.rs +++ b/tests/integration/src/secret_manager/openapi_locational.rs @@ -398,19 +398,18 @@ async fn cleanup_stale_secrets( page_token = response.next_page_token; } - let pending = stale_secrets - .iter() - .map(|name| { - let (project, location, secret) = extract(name).unwrap(); - client - .delete_secret_by_project_and_location_and_secret() - .set_project(project) - .set_location(location) - .set_secret(secret) - .with_idempotency(true) - .send() - }) - .collect::>(); + let pending = stale_secrets.iter().map(|name| async move { + let (project, location, secret) = parse_secret_name(name)?; + client + .delete_secret_by_project_and_location_and_secret() + .set_project(project) + .set_location(location) + .set_secret(secret) + .with_idempotency(true) + .send() + .await?; + Ok::<(), anyhow::Error>(()) + }); // Print the errors, but otherwise ignore them. futures::future::join_all(pending) @@ -422,18 +421,11 @@ async fn cleanup_stale_secrets( Ok(()) } -fn extract(name: &str) -> Result<(&str, &str, &str)> { +fn parse_secret_name(name: &str) -> Result<(&str, &str, &str)> { // format: projects/{project}/locations/{location}/secrets/{secret} let parts: Vec<&str> = name.split('/').collect(); match parts[..] { - [ - "projects", - p, - "locations", - l, - "secrets", - s, - ] => Ok((p, l, s)), + ["projects", p, "locations", l, "secrets", s] => Ok((p, l, s)), _ => Err(anyhow!("invalid secret name: {}", name)), } } @@ -441,24 +433,24 @@ fn extract(name: &str) -> Result<(&str, &str, &str)> { #[cfg(test)] mod tests { use super::*; + use test_case::test_case; #[test] fn test_extract_valid_name() { let name = "projects/my-project/locations/us-central1/secrets/my-secret"; - let (project, location, secret) = extract(name).unwrap(); + let (project, location, secret) = parse_secret_name(name).unwrap(); assert_eq!(project, "my-project"); assert_eq!(location, "us-central1"); assert_eq!(secret, "my-secret"); } - #[test] - fn test_extract_invalid_name() { - let name = "invalid/format"; - assert!(extract(name).is_err()); - } - - #[test] - fn test_extract_missing_parts() { - assert!(extract("projects/only-one").is_err()); + #[test_case("invalid/format")] + #[test_case("projects/only-one")] + #[test_case("projects/p/locations/l/bad/s")] + #[test_case("projects/p/bad/l/secrets/s")] + #[test_case("bad/p/locations/l/secrets/s")] + fn test_extract_invalid_name(input: &str) { + let result = parse_secret_name(input); + assert!(result.is_err(), "{result:?}"); } }