-
Notifications
You must be signed in to change notification settings - Fork 9.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
provider/aws: Add support for AssumeRole prior to operations #8638
Conversation
This commit enables terraform to utilise the assume role functionality of sts to execute commands with different privileges than the API keys specified. Signed-off-by: Ian Duffy <[email protected]>
Remove unnecessary &schema.Schema from the AWS provider schema definition.
@@ -75,7 +78,7 @@ func GetAccountId(iamconn *iam.IAM, stsconn *sts.STS, authProviderName string) ( | |||
} | |||
|
|||
if len(outRoles.Roles) < 1 { | |||
return "", fmt.Errorf("Failed getting account ID via 'iam:ListRoles': No roles available") | |||
return "", errors.New("Failed getting account ID via 'iam:ListRoles': No roles available") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why errors.New and not errWrap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are generating a new error rather than wrapping one produced by an upstream API?
@jen20 so pleased to see this included. I left a few nit picks about the ordering of error checking and about what way we throw errors. These are not show stoppers at all :) More nit picks |
d0418cc
to
305f114
Compare
This replaces the previous `role_arn` with a block which looks like this: ``` provider "aws" { // secret key, access key etc assume_role { role_arn = "<Role ARN>" session_name = "<Session Name>" external_id = "<External ID>" } } ``` We also modify the configuration structure and read the values from the block if present into those values and adjust the call to AssumeRole to include the SessionName and ExternalID based on the values set in the configuration block. Finally we clean up the tests and add in missing error checks, and clean up the error handling logic in the Auth helper functions.
305f114
to
e3ccb51
Compare
LGTM! Nice work :) |
I totally don't understand how to use this, could somebody please explain? I've read through #1275 as well, without any light bulbs going off. Until now, we've been using a script that does STS::AssumeRole with MFA, obtains the temporary credentials, exports them as environment variables. After that, we happily continue with terraform plan and apply. Is there now a smarter way of doing this? |
Hi @benttrep, Authenticate to your account that has the assume role privilege using any of the standard authentication methods(env vars, profile, ec2 IAM role etc). Then just follow the docs at https://www.terraform.io/docs/providers/aws/ |
Aaah 😲 This is awesome ❗ And the docs were updated as part of the release 👍 A suggestion: could we have a link to the updated docs included in the changelog? As the documentation grows, it becomes harder and harder to spot the changes in the docs. E.g. https://www.terraform.io/docs/providers/aws/index.html#Assume_role - and yes, anchors in the docs would be great. |
i'm probably missing something but the docs aren't really followable...i have the arn_role block in my assume role...but it's not working. always get
how would i know what value to put in session_id and external_id? i expected terraform to prompt me for my MFA value. |
Hi @drdamour! There are a few different issues in your example: Firstly the role ARN is indeed not valid - they have an account ID in them. The IAM console will show you the correct ARN for a given role. This covers the third error. Secondly, Assuming a role has two aspects:
In order to fix your example you'll need to do the following:
|
thanks, i was obfuscating my role id...i'm using a valid one. the whole reason we had the script was to get the session token. i suspect that's what @bentterp 's script was doing as well...so alas you still need a wrapper script to do stuff with MFA..that sucks. thanks for confirming. |
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. |
This PR builds on the work of @imduffy15 in #8506, adding in support for Session Name and External ID, and changing around the configuration block a bit.
There is still a case which needs fixing regarding the error message when no valid source credentials are passed in, and also if the source credentials are valid but the role cannot be assumed.
Automated testing is left as a follow-up to this - we have a strategy in place but it requires some (more broadly useful) changes to the acceptance testing harness to be feasible.
Fixes #1275.