Skip to content
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

aws/session: Add support for assuming role via Web Identity Tokens #2667

Merged
merged 9 commits into from
Jul 17, 2019

Conversation

jasdel
Copy link
Contributor

@jasdel jasdel commented Jun 25, 2019

Adds support for assuming an role via the Web Identity Token. Allows for OIDC token files to be used by specifying the token path through the AWS_WEB_IDENTITY_TOKEN_FILE, and AWS_ROLE_ARN environment variables.

Replaces PR #2193

@jasdel jasdel added the pr/work-in-progress This PR is a draft and needs further work. label Jun 25, 2019
"AWS_WEB_IDENTITY_TOKEN_FILE",
}
webIdentityRoleARNEnvKey = []string{
"AWS_ROLE_ARN",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jasdel 👋 Could you clarify the positioning around this new AWS_ROLE_ARN environment variable? It is noted and implemented here in the context of being just for web identity authentication, but are there plans to utilize this new AWS_ROLE_ARN environment variable as a standard around assuming a role in general?

We have use cases where assuming a role via a standardized environment variable (outside web identity authentication) would be very helpful within the Terraform AWS Provider session initialization, since it tends to be run in automation/CI scenarios. 😄

If there is the potential for general usage of this same environment variable, would it be possible to relax the new aws/session/credentials.go implementation to only return an error if AWS_WEB_IDENTITY_TOKEN_FILE is provided and AWS_ROLE_ARN is missing?

Thanks so much!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing this up. AWS_ROLE_ARN is indeed intended to allow generic Role ARN usage outside of the Web identity Token's provider.

I think the behavior your describe will be supported by this change once the integration between the providers is fixed. I'm working on this, and SDK's credential chain behavior with these new options.

The SDK's behavior around the shared config file also needs to be updated to reflect this new concept, and make web identity token play nicely with the existing credential providers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to have to walk back my statment on AWS_ROLE_ARN generic behavior. I think ideally AWS_ROLE_ARN should be used by the SDK to provide another source for an Role ARN, but this adds quite a bit of complexity to the SDK's credential chain. Especially, if the AWS_ROLE_ARN can be mixed with the shared config/credentials file to enable assume role with config/credentials. It will be important to get this behavior consistent across the SDKs.

Initially, in this change, AWS_ROLE_ARN will only be usable with the web identity token. In the future I think there is a lot of value for us to evaluate expanding this behavior to apply to other credential sources.

It might make sense to evaluate this behavior and update it so that AWS_ROLE_ARN could be applied to other environment configuration, such as environment static credentials, ECS credentials and the new web identity.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is great to hear, @jasdel! Thank you for the consideration here. Would you prefer if you/I opened a top level issue for this feature request?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bflad, creating a new feature request would be great. We can keep track of the discussion how the SDK(s) should use AWS_ROLE_ARN with their existing credential providers and chain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the feature request for supporting AWS_ROLE_ARN with the environment credentials provider: https://github.com/aws/aws-sdk-go/issues/2802

@jasdel jasdel force-pushed the feature/AssumeRoleWebIdentity branch 2 times, most recently from 2719d10 to 651883f Compare July 9, 2019 21:27
@jasdel jasdel added needs-review This issue or pull request needs review from a core team member. and removed pr/work-in-progress This PR is a draft and needs further work. labels Jul 9, 2019
@jasdel jasdel force-pushed the feature/AssumeRoleWebIdentity branch from 3d700e5 to 77d232f Compare July 10, 2019 20:06
aws/session/credentials.go Show resolved Hide resolved
aws/session/session.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
go.sum Outdated Show resolved Hide resolved
aws/session/shared_config.go Outdated Show resolved Hide resolved
aws/session/shared_config.go Outdated Show resolved Hide resolved
@jasdel jasdel force-pushed the feature/AssumeRoleWebIdentity branch from a0e50bd to da51c9d Compare July 15, 2019 23:03
@jasdel jasdel requested a review from skotambkar July 16, 2019 16:43
@jasdel jasdel self-assigned this Jul 16, 2019
dkropachev pushed a commit to dkropachev/aws-iam-authenticator that referenced this pull request May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review This issue or pull request needs review from a core team member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants