Throws CredentialChainExhausted unless `~/.aws/credentials` exists

[pbrisbin]

It seems something about the credential chain loading fails without the presence of ~/.aws/credentials. The file is not needed in all setups (e.g. SSO-only) and so may not exist. Other SDKs (tested with AWS CLIv2) seem to still work, but Amazonka does not.

Reproduction:

1. Have a fresh machine with no config besides SSO auth
2. Run aws --profile x sso login and log in
3. Observe something like AWS_PROFILE=x aws s3 ls works
4. Run an Amazonka-based program AWS_PROFILE=x ./something
5. Observe CredentialChainExhausted
6. Run touch ~/.aws/credentials
7. Observe AWS_PROFILE=x ./something now works

[chris-martin]

This issue was observed in Amazonka 2.0.

[pbrisbin]

I've done some investigation, but as far as I can tell the code really shouldn't be able to exhibit this bug, which is fun.

discover is the only place that throws CredentialChainExhausted. It ostensibly throws it if it gets to the end of the chain without anything working, but it also throws it if it gets to the last link in the chain, which is EC2 metadata, and is not on EC2. I find that surprising, but shouldn't be harmful.

The second element in the chain is fromFileEnv, this is the happy path we expect since it eventually gets to the SSO code. This function just reads some ENV and calls fromFilePath, which has this:

  -- If we fail to read the credentials file, assume it's empty and
  -- move on. It is valid to configure only a config file if you plan
  -- to assume a role using any of the assume role methods.
  credentialsIni <-
    Exception.catchJust
      (\(_ :: AuthError) -> Just mempty)
      (loadIniFile credentialsFile)
      pure

Which (based on loadIniFile) should absolutely, as far as I can tell, treat a missing credentials file identically to an empty one (catchJust -> mempty). So I can't see how a touch of this file could change any behavior in this function. And besides merging credentialsIni into configIni before proceed, nothing else happens with this value :thinking:

I will attempt to write a smaller reproduction so I can test against latest main tomorrow.

[endgame]

which is EC2 metadata, and is not on EC2. I find that surprising, but shouldn't be harmful.

I think this is because otherwise we'd be making network requests to the IMDS IP (or name? I can't remember) while on a non-AWS network, and have no idea what's at the other end.

When simplifying your error reproducer, try using just that fromFileEnv function instead of the whole discover chain. My theory is that it's throwing some exception other than AuthError if the load fails, and that means it's passing that nice comment and handler.

[pbrisbin]

otherwise we'd be making network requests to the IMDS IP (or name? I can't remember) while on a non-AWS network, and have no idea what's at the other end.

Yeah, it's not the check-throw that surprised me, it was that it threw CredentialChainExhausted instead of AuthError. It doesn't matter since it's the last link in the chain, but conceptually-speaking I would expect it to throw an AuthError to signal moving on, rather than a CredentialChainExhausted to just end it there.

My theory is that it's throwing some exception other than AuthError if the load fails

Same here! But it truly looks like it's doing the right thing to me,

    loadIniFile :: FilePath -> IO (HashMap Text [(Text, Text)])
    loadIniFile path = do
      exists <- Directory.doesFileExist path
      unless exists . Exception.throwIO $ MissingFileError path

:shrug: will report back tomorrow.

[endgame]

conceptually-speaking I would expect it to throw an AuthError to signal moving on, rather than a CredentialChainExhausted to just end it there.

This seems sensible. Do you have capacity to PR that?

[pbrisbin]

Do you have capacity to PR that?

I think so. Is it OK to add a new constructor to AuthError for it, or does that come with version-compatibility baggage we'd not want to take on?

[pbrisbin]

Ah geeze, this bug was already fixed: https://github.com/brendanhay/amazonka/pull/951.

That explains why main looks good. This is not yet released, so we're still hitting it with 2.0. Sorry for the noise, we can close this Issue as duplicate.

[endgame]

No problem. Maybe we still want an additional AuthError constructor (or two), if you're still interested in that PR. One to say "we don't think we're on a network with an IMDS" and one for "IMDS fetch failed"?