Unverified Commit c3478cea authored by Russell Cohen's avatar Russell Cohen Committed by GitHub
Browse files

Make CredentialsError non-exhaustive & rework IMDS errors (#781)

* Make CredentialsError non_exhaustive

* Overhaul imds errors to simplify downstream error handling

* update changelog, fix docs/clippy errors

* CR feedback
parent 8f623f65
Loading
Loading
Loading
Loading
+3 −0
Original line number Diff line number Diff line
vNext (Month Day, Year)
=======================
**Breaking Changes**
- `CredentialsError` variants became non-exhaustive. This makes them impossible to construct directly outside of the `aws_types` crate. In order to construct credentials errors, new methods have been added for each variant. Instead of `CredentialsError::Unhandled(...)`, you should instead use `CredentialsError::unhandled`. Matching methods exist for all variants. (#781)
- The default credentials chain now returns `CredentialsError::CredentialsNotLoaded` instead of `ProviderError` when no credentials providers are configured.

**New this week**

+29 −3
Original line number Diff line number Diff line
@@ -353,8 +353,8 @@ pub mod credentials {
    mod test {
        use tracing_test::traced_test;

        use aws_types::credentials::ProvideCredentials;
        use aws_types::os_shim_internal::{Env, Fs};
        use aws_types::credentials::{CredentialsError, ProvideCredentials};
        use aws_types::os_shim_internal::{Env, Fs, TimeSource};
        use smithy_types::retry::{RetryConfig, RetryMode};

        use crate::default_provider::credentials::DefaultCredentialsChain;
@@ -362,6 +362,10 @@ pub mod credentials {
        use crate::provider_config::ProviderConfig;
        use crate::test_case::TestEnvironment;

        use smithy_async::rt::sleep::TokioSleep;
        use smithy_client::erase::boxclone::BoxCloneService;
        use smithy_client::never::NeverConnected;

        /// Test generation macro
        ///
        /// # Examples
@@ -376,7 +380,7 @@ pub mod credentials {
        /// ```
        ///
        /// **Run the test case against a real HTTPS connection:**
        /// > Note: Be careful to remove sensitive information before commiting. Always use a temporary
        /// > Note: Be careful to remove sensitive information before committing. Always use a temporary
        /// > AWS account when recording live traffic.
        /// ```rust
        /// make_test!(live: test_name)
@@ -450,6 +454,28 @@ pub mod credentials {
            assert_eq!(creds.access_key_id(), "correct_key_secondary");
        }

        #[tokio::test]
        #[traced_test]
        async fn no_providers_configured_err() {
            let conf = ProviderConfig::no_configuration()
                .with_tcp_connector(BoxCloneService::new(NeverConnected::new()))
                .with_time_source(TimeSource::real())
                .with_sleep(TokioSleep::new());
            let provider = DefaultCredentialsChain::builder()
                .configure(conf)
                .build()
                .await;
            let creds = provider
                .provide_credentials()
                .await
                .expect_err("no providers enabled");
            assert!(
                matches!(creds, CredentialsError::CredentialsNotLoaded { .. }),
                "should be NotLoaded: {:?}",
                creds
            )
        }

        #[tokio::test]
        async fn test_returns_default_retry_config_from_empty_profile() {
            let env = Env::from_slice(&[("AWS_CONFIG_FILE", "config")]);
+11 −8
Original line number Diff line number Diff line
@@ -95,17 +95,20 @@ impl EcsCredentialsProvider {
        let auth = match self.env.get(ENV_AUTHORIZATION).ok() {
            Some(auth) => Some(HeaderValue::from_str(&auth).map_err(|err| {
                tracing::warn!(token = %auth, "invalid auth token");
                CredentialsError::InvalidConfiguration(
                    EcsConfigurationErr::InvalidAuthToken { err, value: auth }.into(),
                )
                CredentialsError::invalid_configuration(EcsConfigurationErr::InvalidAuthToken {
                    err,
                    value: auth,
                })
            })?),
            None => None,
        };
        match self.provider().await {
            Provider::NotConfigured => Err(CredentialsError::CredentialsNotLoaded),
            Provider::InvalidConfiguration(err) => Err(CredentialsError::InvalidConfiguration(
                format!("{}", err).into(),
            )),
            Provider::NotConfigured => {
                Err(CredentialsError::not_loaded("ECS provider not configured"))
            }
            Provider::InvalidConfiguration(err) => {
                Err(CredentialsError::invalid_configuration(format!("{}", err)))
            }
            Provider::Configured(provider) => provider.credentials(auth).await,
        }
    }
@@ -459,7 +462,7 @@ mod test {
    fn provider(env: Env, connector: DynConnector) -> EcsCredentialsProvider {
        let provider_config = ProviderConfig::empty()
            .with_env(env)
            .with_connector(connector);
            .with_http_connector(connector);
        Builder::default().configure(&provider_config).build()
    }

+3 −5
Original line number Diff line number Diff line
@@ -74,8 +74,8 @@ impl ProvideCredentials for EnvironmentVariableCredentialsProvider {

fn to_cred_error(err: VarError) -> CredentialsError {
    match err {
        VarError::NotPresent => CredentialsError::CredentialsNotLoaded,
        e @ VarError::NotUnicode(_) => CredentialsError::Unhandled(Box::new(e)),
        VarError::NotPresent => CredentialsError::not_loaded("environment variable not set"),
        e @ VarError::NotUnicode(_) => CredentialsError::unhandled(e),
    }
}

@@ -153,9 +153,7 @@ mod test {
            .now_or_never()
            .unwrap()
            .expect_err("no credentials defined");
        if let CredentialsError::Unhandled(_) = err {
            panic!("wrong error type")
        };
        assert!(matches!(err, CredentialsError::CredentialsNotLoaded { .. }));
    }

    #[test]
+8 −9
Original line number Diff line number Diff line
@@ -49,7 +49,7 @@ impl HttpCredentialProvider {
        match credentials {
            Ok(creds) => Ok(creds),
            Err(SdkError::ServiceError { err, .. }) => Err(err),
            Err(other) => Err(CredentialsError::Unhandled(other.into())),
            Err(other) => Err(CredentialsError::unhandled(other)),
        }
    }

@@ -127,10 +127,9 @@ impl ParseStrictResponse for CredentialsResponseParser {
    type Output = credentials::Result;

    fn parse(&self, response: &Response<Bytes>) -> Self::Output {
        let str_resp = std::str::from_utf8(response.body().as_ref())
            .map_err(|err| CredentialsError::Unhandled(err.into()))?;
        let json_creds = parse_json_credentials(str_resp)
            .map_err(|err| CredentialsError::Unhandled(err.into()))?;
        let str_resp =
            std::str::from_utf8(response.body().as_ref()).map_err(CredentialsError::unhandled)?;
        let json_creds = parse_json_credentials(str_resp).map_err(CredentialsError::unhandled)?;
        match json_creds {
            JsonCredentials::RefreshableCredentials {
                access_key_id,
@@ -144,8 +143,8 @@ impl ParseStrictResponse for CredentialsResponseParser {
                Some(expiration),
                self.provider_name,
            )),
            JsonCredentials::Error { code, message } => Err(CredentialsError::ProviderError(
                format!("failed to load credentials [{}]: {}", code, message).into(),
            JsonCredentials::Error { code, message } => Err(CredentialsError::provider_error(
                format!("failed to load credentials [{}]: {}", code, message),
            )),
        }
    }
@@ -177,7 +176,7 @@ impl ClassifyResponse<SdkSuccess<Credentials>, SdkError<CredentialsError>>
            }
            // non-parseable 200s
            Err(SdkError::ServiceError {
                err: CredentialsError::Unhandled(_),
                err: CredentialsError::Unhandled { .. },
                raw,
            }) if raw.http().status().is_success() => RetryKind::Error(ErrorKind::ServerError),
            // 5xx errors
@@ -278,7 +277,7 @@ mod test {
            matches!(
                sdk_error,
                SdkError::ServiceError {
                    err: CredentialsError::ProviderError(_),
                    err: CredentialsError::ProviderError { .. },
                    ..
                }
            ),
Loading