Add support for AssumeRoleWithWebIdentity #178

mwieczorek · 2022-03-11T12:19:18Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #149

This PR adds an option to pass webIdentityToken to Config and use it for getting credentials. No other 'source credentials' are required.

hashicorp-cla · 2022-03-11T12:19:22Z

All committers have signed the CLA.

mwieczorek · 2022-03-11T12:23:44Z

credentials.go

@@ -153,6 +158,25 @@ Error: %w`, err)
 	return provider, creds.Source, err
 }

+func webIdentityCredentialsProvider(ctx context.Context, awsConfig aws.Config, c *Config) (aws.CredentialsProvider, error) {
+	ar := c.AssumeRole


I reused config.AssumeRole 'block' here, although not every option is valid for 'assumeRoleWithWebIdentity'.
@gdavison - let me know if it's ok, or if you prefer to have something like config.WebIdentityAssumeRole

In hashicorp/terraform-provider-aws#22907 (comment) you accepted combining both at the provider level, but I don't know how it should be done here.

I'm reconsidering that statement 🙂 Since the two operations support different options, it would probably be better to explicitly choose one or the other, both here and in the provider.

ok, I changed the Config and created a separate struct for AssumeRoleWithWebIdentity

mwieczorek · 2022-03-11T12:26:27Z

internal/config/config.go

@@ -79,3 +80,7 @@ func (c Config) ResolveSharedCredentialsFiles() ([]string, error) {
 	}
 	return v, nil
 }
+
+func (c Config) GetIdentityToken() ([]byte, error) {


I need it for https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/credentials@v1.10.0/stscreds#NewWebIdentityRoleProvider / https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/credentials@v1.10.0/stscreds#IdentityTokenRetriever

Let me know if you prefer to have 'implementation' of that interface separately from Config type

Since it only references fields from c.AssumeRoleWithWebIdentity, it would probably be good to move it there

mwieczorek · 2022-03-11T12:27:52Z

credentials.go

@@ -133,6 +133,11 @@ func getCredentialsProvider(ctx context.Context, c *Config) (aws.CredentialsProv
 		return nil, "", fmt.Errorf("loading configuration: %w", err)
 	}

+	if c.WebIdentityToken != "" && c.AssumeRole != nil {


Let me know if you'd like to additionally validate/error when AssumeRole is nil when WebIdentityToken is passed.

mwieczorek · 2022-03-11T12:31:20Z

I also tested it against a real AWS account/provider/role setup with simple code like

        ctx := context.Background()
	c := awsbase.Config{
		Region:           "us-east-1",
		WebIdentityToken: "<my id token>",
		AssumeRole: &config.AssumeRole{
			RoleARN: "arn:aws:iam::123456789012:role/role-name",
		},
	}
	cfg, err := awsbase.GetAwsConfig(ctx, &c)

djcrabhat · 2022-03-28T01:16:34Z

Oh I hit up against this exact problem today. Was gonna make in gitlab CI project to deploy something across a dozen accounts in my org via aliased providers, found I can't share one identity token file via CLI and get each provider to assume a unique role. Guess I'm in the cutting edge of terraform a CD! Thanks so much for implementing this.

gdavison

Thanks for creating this, @mwieczorek. It's looking good. I have a few comments so far.

gdavison · 2022-04-07T01:02:34Z

credentials.go

@@ -153,6 +158,25 @@ Error: %w`, err)
 	return provider, creds.Source, err
 }

+func webIdentityCredentialsProvider(ctx context.Context, awsConfig aws.Config, c *Config) (aws.CredentialsProvider, error) {
+	ar := c.AssumeRole


I'm reconsidering that statement 🙂 Since the two operations support different options, it would probably be better to explicitly choose one or the other, both here and in the provider.

internal/config/config.go

gdavison · 2022-04-07T01:08:20Z

credentials_test.go

@@ -189,6 +189,39 @@ func TestAWSGetCredentials_sharedCredentialsFile(t *testing.T) {
 	validateCredentialsProvider(credsParam, "accesskey2", "secretkey2", "", sharedConfigCredentialsSource(fileParamName), t)
 }

+func TestAWSGetCredentials_webIdentityToken(t *testing.T) {


I've created #227 that adds tests for configuring this using environment variables or the shared config files. Those tests can be updated with support for the configuration block in addition to this test

The PR #227 has been merged, so if you rebase this branch or merge main into this branch, you can pick up those tests

gdavison · 2022-04-07T01:11:23Z

credentials.go

+	client := stsClient(awsConfig, c)
+
+	appCreds := stscreds.NewWebIdentityRoleProvider(client, ar.RoleARN, c, func(opts *stscreds.WebIdentityRoleOptions) {
+		opts.RoleSessionName = ar.SessionName


FYI stscreds.WebIdentityRoleProvider doesn't currently support the Policy field from sts.AssumeRoleWithWebIdentityInput. I've created aws/aws-sdk-go-v2#1662 and may create a PR in the SDK

mwieczorek · 2022-04-07T08:54:49Z

Hi @gdavison , thank you for the review. I updated the PR, please take a look.

gdavison

It's looking good

gdavison · 2022-04-12T23:06:05Z

internal/config/config.go

@@ -79,3 +80,7 @@ func (c Config) ResolveSharedCredentialsFiles() ([]string, error) {
 	}
 	return v, nil
 }
+
+func (c Config) GetIdentityToken() ([]byte, error) {


Since it only references fields from c.AssumeRoleWithWebIdentity, it would probably be good to move it there

gdavison · 2022-04-12T23:09:11Z

credentials_test.go

+	if a, e := source, ""; a != e {
+		t.Errorf("Expected initial source to be %q, %q given", e, a)
+	}


For the other credentials providers, we return the name of the credentials source. In this case, it would be stscreds.WebIdentityProviderName (i.e. WebIdentityCredentials)

gdavison · 2022-04-12T23:14:46Z

credentials_test.go

@@ -189,6 +189,39 @@ func TestAWSGetCredentials_sharedCredentialsFile(t *testing.T) {
 	validateCredentialsProvider(credsParam, "accesskey2", "secretkey2", "", sharedConfigCredentialsSource(fileParamName), t)
 }

+func TestAWSGetCredentials_webIdentityToken(t *testing.T) {


The PR #227 has been merged, so if you rebase this branch or merge main into this branch, you can pick up those tests

mwieczorek · 2022-04-13T14:46:04Z

hi @gdavison
I moved GetIdentityToken as you suggested, and changed credentialsProvider name returned from getCredentialsProvider

I also rebased this PR, and updated test cases in TestAssumeRoleWithWebIdentity. I changed the input struct (removed SetConfig, added SetConfigTokenFile - this allows me to have separate test cases when we pass to config token directly or when we pass path to file with token). Let me know what you think.

…` config

gdavison

This is great, @mwieczorek. Since AWS has added Duration and Policy to stscreds.WebIdentityRoleOptions (aws/aws-sdk-go-v2#1670), I've added them to AssumeRoleWithWebIdentity. I've also added the ability to assume another role after using the web identity

mwieczorek · 2022-04-27T06:53:33Z

thanks @gdavison for all your help :)

mwieczorek requested a review from a team as a code owner March 11, 2022 12:19

mwieczorek commented Mar 11, 2022

View reviewed changes

gdavison self-assigned this Mar 28, 2022

gdavison reviewed Apr 7, 2022

View reviewed changes

gdavison reviewed Apr 12, 2022

View reviewed changes

Michal Wieczorek added 5 commits April 13, 2022 16:01

Add support for AssumeRoleWithWebIdentity

774e4c0

Add support for AssumeRoleWithWebIdentity

0fca859

Move GetIdentityToken implementation

72f56c1

Set proper credentialsProvider name

a65caf5

Add testcases for TestAssumeRoleWithWebIdentity

b622389

gdavison added 4 commits April 26, 2022 14:52

Tweaks to web identity tests

589e453

Updates AWS SDK for Go v2 dependencies

4e0a1a4

Adds Duration and Policy parameters to `AssumeRoleWithWebIdentity…

94d64a1

…` config

Enable assuming role after assuming role with web identity

8c89298

gdavison approved these changes Apr 26, 2022

View reviewed changes

gdavison merged commit a7b41e5 into hashicorp:main Apr 26, 2022

gdavison added a commit that referenced this pull request Apr 27, 2022

Updates CHANGELOG for #178

e9e5371

mwieczorek deleted the web-identity-token branch April 27, 2022 06:49

ewbankkit mentioned this pull request Apr 28, 2022

Add support for Assume Role with Web Identity and honour skip_metadata_api_check = false hashicorp/terraform-provider-aws#24441

Merged

ewbankkit mentioned this pull request Jul 5, 2022

Fix web id assume role hashicorp/terraform-provider-aws#25681

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for AssumeRoleWithWebIdentity #178

Add support for AssumeRoleWithWebIdentity #178

mwieczorek commented Mar 11, 2022 •

edited by gdavison

Loading

hashicorp-cla commented Mar 11, 2022 •

edited

Loading

mwieczorek Mar 11, 2022

gdavison Apr 7, 2022

mwieczorek Apr 7, 2022

mwieczorek Mar 11, 2022

gdavison Apr 12, 2022

mwieczorek Mar 11, 2022

mwieczorek commented Mar 11, 2022

djcrabhat commented Mar 28, 2022 •

edited

Loading

gdavison left a comment

gdavison Apr 7, 2022

gdavison Apr 7, 2022

gdavison Apr 12, 2022

gdavison Apr 7, 2022

mwieczorek commented Apr 7, 2022

gdavison left a comment

gdavison Apr 12, 2022

gdavison Apr 12, 2022

gdavison Apr 12, 2022

mwieczorek commented Apr 13, 2022

gdavison left a comment

mwieczorek commented Apr 27, 2022

Add support for AssumeRoleWithWebIdentity #178

Add support for AssumeRoleWithWebIdentity #178

Conversation

mwieczorek commented Mar 11, 2022 • edited by gdavison Loading

Community Note

hashicorp-cla commented Mar 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwieczorek commented Mar 11, 2022

djcrabhat commented Mar 28, 2022 • edited Loading

gdavison left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwieczorek commented Apr 7, 2022

gdavison left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwieczorek commented Apr 13, 2022

gdavison left a comment

Choose a reason for hiding this comment

mwieczorek commented Apr 27, 2022

mwieczorek commented Mar 11, 2022 •

edited by gdavison

Loading

hashicorp-cla commented Mar 11, 2022 •

edited

Loading

djcrabhat commented Mar 28, 2022 •

edited

Loading