[WIP] Fix assuming role via ".aws/config"

[svenwltr]

Fixes #1184

This PR allows assuming roles via shared configuration ~/.aws/config. It still supports assuming other roles for different provider configurations afterwards.

I am somehow not fully satisfied with the code yet, so feel free to nitpick everything.

Open Questions

• Should I try to reduce code duplication due to assuming roles twice?
• I am not entirely sure, if MFA would work correctly, but AFAIS when using MFA one has to do the aws sts assume-role manually anyway, so it shouldn't be affected by this change.

[radeksimko]

Hey @svenwltr
thanks for the contribution.

It makes sense for Terraform to support assuming roles via ~/.aws/config. That said I'd rather see the INI parsing logic and all the format-specific logic, or generally as much of that code as possible in the upstream SDK, e.g. here https://github.com/aws/aws-sdk-go/tree/master/aws/credentials
The SDK is IMO much better place for it and we can make sure it's aligned with AWS' conventions/expectations and kept up to date as AWS engineers maintain the SDK.

Have you looked into the SDK and considered raising a PR there?

[svenwltr]

Hey @radeksimko,

the SDK is already able to do this, as I wrote in the actual issue:

sess, err := session.NewSessionWithOptions(session.Options{
    Config: aws.Config{
        Region: "my-region",
    },
    SharedConfigState: session.SharedConfigEnable,
    Profile:           "my-profile",
})

I did it the manual way, because Terraform already does more manually than necessary, which confused me a bit. I also strongly prefer to use the SDKs ability.

Also I am very fine, if we go with the PR #1608, which solves the same problem in another way.

[radeksimko]

Ah, I see - sorry I didn't read the whole conversation there. 🙈

Have you seen this PR? #1608 you did

It will probably need some tests & more thorough review, but it's more in-line with what we're discussing here. What do you think?

[radeksimko]

I should read more before posting comments, clearly...

Also I am very fine, if we go with the PR #1608, which solves the same problem in another way.

Cool. With that are you happy for me to close this PR?

[svenwltr]

Yeah, the issues and PRs got a bit messed up :-)

Ok, it looks like we're on the same line here. Therefore I close this in favor of #1608.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!