provider/aws: Added sts:GetCallerIdentity to GetAccountId for federated logins

[bigkraig]

The STS GetCallerIdentity function works for fetching the account id no matter how you're authenticated.

There appears to be a complicated GetAccountId function in auth_helpers.go that accomplishes the same thing. Since I don't know why that was created I didn't update that use, but I think that may be able to use the same method.

[radeksimko]

Thanks for discovering sts:GetCallerIdentity! 👍 It looks like AWS is listening to our requests, that's great! 😃

We could possibly use this to replace iam:ListRoles in auth_helpers.go, but the caveat of STS is that users can enable/disable STS per regions, so I'm rather thinking we'll make it one of the ways to discover account ID and probably put it in front of iam:ListRoles as hacks should always go last. That way we also don't need to introduce any breaking changes and make it work for people that have sts disabled for any reasons.

There appears to be a complicated GetAccountId function in auth_helpers.go that accomplishes the same thing.

That method is there to get around differences in environments where Terraform can run:

• EC2 instance (IAM instance profile)
• federated IAM roles
• standard IAM Users

I'm happy to bump the aws-sdk-go version but not inclined to use sts:GetCallerIdentity as the only method nor inclined to use it directly in each resource. It should be part of auth_helpers.go.

In addition to all of the above resources should not be calling GetAccountId function directly. The only reason I exported this function was #5270

We should call that function once and save the account ID somewhere (possibly to AWSClient which is passed to each resource via meta argument), so that it's cached during the terraform run and resources don't ask API again for what we know already.

[bigkraig]

@radeksimko let me know what you think. I added accountid to the AWSClient.

It looks good so far, I ran this against an existing deployment and it added tags to my aws_db_subnet_group.

[radeksimko]

Would you mind also adding the actual STS mock responses (for success + error) and two tests testing STS?

1. iam:GetUser - 403 (failure) => sts:GetCallerIdentity - 200 (success)
2. iam:GetUser - 403 (failure) => sts:GetCallerIdentity - any failure => iam:ListRoles - 200 (successful fallback to ListRoles)

You can use examples of responses from the API reference as mocks: http://docs.aws.amazon.com/STS/latest/APIReference/API_GetCallerIdentity.html

[radeksimko]

I left you some comments there, but overall this looks pretty good, I think it's quite close to to be merged. 👍 😉

Would you mind separating this into two PRs - one for adding the sts:GetCallerIdentity into GetAccountId etc. and another one for modifying all the resources that would leverage this new functionality?

[bigkraig]

@radeksimko this one is updated to be the PR for sts:GetCallerIdentity only

[radeksimko]

The second commit 7f9db15 LGTM, but I've got two reservations about the first one with dependencies:

1. There are some conflicts which need resolving
2. I think we'd prefer pinning to tagged versions rather than specific hashes if possible - i.e. v1.1.21 instead of v1.1.21-3-ge906984.

[bigkraig]

@radeksimko hows this?

[radeksimko]

@bigkraig just one last thing - can you update your godep to avoid these two lines in the diff?

-   "GodepVersion": "v63",
+   "GodepVersion": "v62",

Try go get -u github.com/tools/godep.

[bigkraig]

@radeksimko done!

[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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.