diff --git a/core/src/api.rs b/core/src/api.rs index eb4b5d03..7533b7c2 100644 --- a/core/src/api.rs +++ b/core/src/api.rs @@ -375,6 +375,10 @@ where /// credential providers together. The chain will try each provider in order until one /// returns credentials or all providers have been exhausted. /// +/// A provider error does not stop the chain. If no provider returns credentials and +/// at least one provider failed with a retryable error, the first such error is +/// returned so that the caller can retry it. Otherwise the chain returns `Ok(None)`. +/// /// # Example /// /// ```no_run @@ -479,6 +483,8 @@ where type Credential = C; async fn provide_credential(&self, ctx: &Context) -> Result> { + let mut retryable_error = None; + for provider in &self.providers { log::debug!("Trying credential provider: {provider:?}"); @@ -493,19 +499,26 @@ where } Err(e) => { log::warn!("Error loading credential from provider {provider:?}: {e:?}"); + if e.is_retryable() && retryable_error.is_none() { + retryable_error = Some(e); + } // Continue to next provider on error continue; } } } - Ok(None) + match retryable_error { + Some(err) => Err(err), + None => Ok(None), + } } } #[cfg(test)] mod tests { use super::*; + use crate::Error; #[derive(Clone, Debug)] struct ExactCredential { @@ -533,4 +546,56 @@ mod tests { assert!(!credential.is_valid_at(timestamp + Duration::from_secs(1))); assert!(!None::.is_valid_at(timestamp)); } + + #[derive(Debug)] + struct FixedProvider(fn() -> Result>); + + impl ProvideCredential for FixedProvider { + type Credential = &'static str; + + async fn provide_credential(&self, _ctx: &Context) -> Result> { + (self.0)() + } + } + + fn chain_result(chain: ProvideCredentialChain<&'static str>) -> Result> { + futures::executor::block_on(chain.provide_credential(&Context::new())) + } + + #[test] + fn chain_returns_later_credential_after_retryable_error() { + let chain = ProvideCredentialChain::new() + .push(FixedProvider(|| { + Err(Error::unexpected("transient").set_retryable(true)) + })) + .push(FixedProvider(|| Ok(Some("credential")))); + + assert_eq!(chain_result(chain).unwrap(), Some("credential")); + } + + #[test] + fn chain_returns_first_retryable_error_when_no_credential_found() { + let chain = ProvideCredentialChain::new() + .push(FixedProvider(|| Err(Error::config_invalid("persistent")))) + .push(FixedProvider(|| { + Err(Error::unexpected("first transient").set_retryable(true)) + })) + .push(FixedProvider(|| Ok(None))) + .push(FixedProvider(|| { + Err(Error::unexpected("second transient").set_retryable(true)) + })); + + let err = chain_result(chain).expect_err("retryable error must be returned"); + assert_eq!(err.to_string(), "first transient"); + assert!(err.is_retryable()); + } + + #[test] + fn chain_skips_persistent_errors_when_no_credential_found() { + let chain = ProvideCredentialChain::new() + .push(FixedProvider(|| Err(Error::config_invalid("persistent")))) + .push(FixedProvider(|| Ok(None))); + + assert_eq!(chain_result(chain).unwrap(), None); + } } diff --git a/core/src/signer.rs b/core/src/signer.rs index 168bf9f9..9ec86e05 100644 --- a/core/src/signer.rs +++ b/core/src/signer.rs @@ -180,7 +180,7 @@ impl Signer { mod tests { use super::*; use crate::time::Timestamp; - use crate::{ErrorKind, ProvideCredential, SignRequest}; + use crate::{ErrorKind, ProvideCredential, ProvideCredentialChain, SignRequest}; use asyncband::oneshot; use futures::future::{join_all, pending}; use futures::poll; @@ -1030,6 +1030,60 @@ mod tests { ); } + #[test] + fn chained_retryable_provider_error_reaches_caller() { + let base = Timestamp::from_second(3_500).expect("timestamp must be valid"); + let refreshed = ExpiringCredential { + generation: 2, + fresh: true, + expires_at: base + Duration::from_secs(30), + required_until: base + Duration::from_secs(10), + }; + let (provider, calls) = SequenceProvider::new([ + Err(Error::unexpected("injected transient failure").set_retryable(true)), + Ok(Some(refreshed)), + ]); + let signer = Signer::new( + Context::new(), + ProvideCredentialChain::new().push(provider), + OperationSigner, + ); + + let mut parts = request_parts(); + let err = futures::executor::block_on(signer.sign(&mut parts, None)) + .expect_err("retryable provider error must be returned"); + assert_eq!(err.kind(), ErrorKind::Unexpected); + assert_eq!(err.to_string(), "injected transient failure"); + assert!(err.is_retryable()); + assert_eq!(calls.load(Ordering::SeqCst), 1); + + futures::executor::block_on(signer.sign(&mut parts, None)) + .expect("caller retry must load the credential"); + assert_eq!(calls.load(Ordering::SeqCst), 2); + assert_eq!( + parts.headers.get("x-credential-generation"), + Some(&HeaderValue::from_static("2")) + ); + } + + #[test] + fn chained_persistent_provider_error_stays_persistent() { + let (provider, calls) = + SequenceProvider::new([Err(Error::unexpected("injected persistent failure"))]); + let signer = Signer::new( + Context::new(), + ProvideCredentialChain::new().push(provider), + OperationSigner, + ); + + let mut parts = request_parts(); + let err = futures::executor::block_on(signer.sign(&mut parts, None)) + .expect_err("missing credential must be returned"); + assert_eq!(err.kind(), ErrorKind::CredentialInvalid); + assert!(!err.is_retryable()); + assert_eq!(calls.load(Ordering::SeqCst), 1); + } + #[test] fn missing_refresh_does_not_fall_back_and_caller_can_retry() { let base = Timestamp::from_second(4_000).expect("timestamp must be valid");