fix: don't transform a ProfileDidNotContainCredentials error if base_…

…provider is running for first provider in chain (#873)

* fix: don't transform a ProfileDidNotContainCredentials if base_provider is running for first provider in chain
add: more comments to aws_config::profile::credentials::repr functions

* add: Test ensuring provider chain hits IMDS even if some config data exists

* update: test to expect success response

* add: clarifying comment about chain vs base providers

aws/rust-runtime/aws-config/src/default_provider.rs

@@ -524,6 +524,7 @@ pub mod credentials {
         make_test!(imds_default_chain_error);
         make_test!(imds_default_chain_success);
         make_test!(imds_assume_role);
+        make_test!(imds_config_with_no_creds);
         make_test!(imds_disabled);
         make_test!(imds_default_chain_retries);
 

aws/rust-runtime/aws-config/src/profile/credentials/repr.rs

@@ -113,6 +113,7 @@ pub fn resolve_chain<'a>(
     let mut visited_profiles = vec![];
     let mut chain = vec![];
     let base = loop {
+        // Get the next profile in the chain
         let profile = profile_set.get_profile(source_profile_name).ok_or(
             ProfileFileError::MissingProfile {
                 profile: source_profile_name.into(),
@@ -124,6 +125,8 @@ pub fn resolve_chain<'a>(
                 .into(),
             },
         )?;
+        // If the profile we just got is one we've already seen, we're in a loop and
+        // need to break out with a CredentialLoop error
         if visited_profiles.contains(&source_profile_name) {
             return Err(ProfileFileError::CredentialLoop {
                 profiles: visited_profiles
@@ -133,6 +136,7 @@ pub fn resolve_chain<'a>(
                 next: source_profile_name.to_string(),
             });
         }
+        // otherwise, store the name of the profile in case we see it again later
         visited_profiles.push(source_profile_name);
         // After the first item in the chain, we will prioritize static credentials if they exist
         if visited_profiles.len() > 1 {
@@ -141,22 +145,32 @@ pub fn resolve_chain<'a>(
                 break BaseProvider::AccessKey(static_credentials);
             }
         }
-        let next_profile = match chain_provider(profile) {
-            // this provider wasn't a chain provider, reload it as a base provider
-            None => {
+
+        let next_profile = {
+            // The existence of a `role_arn` is the only signal that multiple profiles will be chained.
+            // We check for one here and then process the profile accordingly as either a "chain provider"
+            // or a "base provider"
+            if let Some(role_provider) = role_arn_from_profile(profile) {
+                let next = chain_provider(profile)?;
+                chain.push(role_provider);
+                next
+            } else {
                 break base_provider(profile).map_err(|err| {
-                    ProfileFileError::InvalidCredentialSource {
-                        profile: profile.name().into(),
-                        message: format!("could not load source profile: {}", err).into(),
+                    // It's possible for base_provider to return a `ProfileFileError::ProfileDidNotContainCredentials`
+                    // if we're still looking at the first provider we want to surface it. However,
+                    // if we're looking at any provider after the first we want to instead return a `ProfileFileError::InvalidCredentialSource`
+                    if visited_profiles.len() == 1 {
+                        err
+                    } else {
+                        ProfileFileError::InvalidCredentialSource {
+                            profile: profile.name().into(),
+                            message: format!("could not load source profile: {}", err).into(),
+                        }
                     }
                 })?;
             }
-            Some(result) => {
-                let (chain_profile, next) = result?;
-                chain.push(chain_profile);
-                next
-            }
         };
+
         match next_profile {
             NextProfile::SelfReference => {
                 // self referential profile, don't go through the loop because it will error
@@ -205,13 +219,12 @@ enum NextProfile<'a> {
     Named(&'a str),
 }
 
-fn chain_provider(profile: &Profile) -> Option<Result<(RoleArn, NextProfile), ProfileFileError>> {
-    let role_provider = role_arn_from_profile(profile)?;
+fn chain_provider(profile: &Profile) -> Result<NextProfile, ProfileFileError> {
     let (source_profile, credential_source) = (
         profile.get(role::SOURCE_PROFILE),
         profile.get(role::CREDENTIAL_SOURCE),
     );
-    let profile = match (source_profile, credential_source) {
+    match (source_profile, credential_source) {
         (Some(_), Some(_)) => Err(ProfileFileError::InvalidCredentialSource {
             profile: profile.name().to_string(),
             message: "profile contained both source_profile and credential_source. \
@@ -225,14 +238,12 @@ fn chain_provider(profile: &Profile) -> Option<Result<(RoleArn, NextProfile), Pr
                     .into(),
         }),
         (Some(source_profile), None) if source_profile == profile.name() => {
-            Ok((role_provider, NextProfile::SelfReference))
+            Ok(NextProfile::SelfReference)
         }
-
-        (Some(source_profile), None) => Ok((role_provider, NextProfile::Named(source_profile))),
+        (Some(source_profile), None) => Ok(NextProfile::Named(source_profile)),
         // we want to loop back into this profile and pick up the credential source
-        (None, Some(_credential_source)) => Ok((role_provider, NextProfile::SelfReference)),
-    };
-    Some(profile)
+        (None, Some(_credential_source)) => Ok(NextProfile::SelfReference),
+    }
 }
 
 fn role_arn_from_profile(profile: &Profile) -> Option<RoleArn> {
@@ -285,11 +296,13 @@ fn static_creds_from_profile(profile: &Profile) -> Result<Credentials, ProfileFi
     let access_key = profile.get(AWS_ACCESS_KEY_ID);
     let secret_key = profile.get(AWS_SECRET_ACCESS_KEY);
     let session_token = profile.get(AWS_SESSION_TOKEN);
+    // If all three fields are missing return a `ProfileFileError::ProfileDidNotContainCredentials`
     if let (None, None, None) = (access_key, secret_key, session_token) {
         return Err(ProfileFileError::ProfileDidNotContainCredentials {
             profile: profile.name().to_string(),
         });
     }
+    // Otherwise, check to make sure the access and secret keys are defined
     let access_key = access_key.ok_or_else(|| ProfileFileError::InvalidCredentialSource {
         profile: profile.name().to_string(),
         message: "profile missing aws_access_key_id".into(),
@@ -298,6 +311,7 @@ fn static_creds_from_profile(profile: &Profile) -> Result<Credentials, ProfileFi
         profile: profile.name().to_string(),
         message: "profile missing aws_secret_access_key".into(),
     })?;
+    // There might not be an active session token so we don't error out if it's missing
     Ok(Credentials::new(
         access_key,
         secret_key,
@@ -335,7 +349,7 @@ mod tests {
         match (expected, actual) {
             (TestOutput::Error(s), Err(e)) => assert!(
                 format!("{}", e).contains(&s),
-                "expected {} to contain `{}`",
+                "expected\n{}\nto contain\n{}\n",
                 e,
                 s
             ),

aws/rust-runtime/aws-config/test-data/default-provider-chain/imds_config_with_no_creds/env.json

@@ -0,0 +1,4 @@
+{
+  "HOME": "/home",
+  "AWS_REGION": "us-east-1"
+}

aws/rust-runtime/aws-config/test-data/default-provider-chain/imds_config_with_no_creds/fs/home/.aws/config

@@ -0,0 +1,2 @@
+[default]
+max_attempts = 1