Skip to content

feat: support default role assumers #2179

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

Closed

Conversation

AllanZhengYP
Copy link
Contributor

@AllanZhengYP AllanZhengYP commented Mar 25, 2021

Issue

Resolves #2087
Resolves #1998
Resolves #2011
Resolves #1193

Description

This change provides default role assumer for the default credentials provider that calls sts:assumeRole or sts:assumeRoleWithWebIdentity under the hood. As a result, users don't need to import STS client and supply their own role assumer(like mentioned in #1193).

These assumer is exported from STS client not the packages/ folder because it can avoid circular dependency. The credential-provider-* packages having any dependency over STS client will cause the same issue. As a result this change makes the source code comply the contract that clients/ depends on packages/; lib/ depends on packages/ and clients/.

Testing

✅ It has been manually tested with credential files containing assume role chaining
✅ It has been validate with bundler(Webpack) that tree shaking work well

Additional context

The example Lambda function size after tree shaking using default configure shows an increase from 115KB to 190KB because the base client now has dependencies over STS client

This change will add some overhead to lambda cold start performance whereas warm start performance is not changed. Here's a typical load test result(10K request, 500 concurrency). The lattency increases about 1~5%

Lambda invoke with 1 DyanmoDB call. Webpack; v3.9.0

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0   14  61.6      0     351
Processing:    36  177 244.3    103    1452
Waiting:       35  177 244.3    102
    1452
Total:         36  190 293.9    103    1788

Percentage of the requests served within a certain time (ms)
  50%    103
  66%    116
  75%    128
  80%    140
  90%    212
  95%   1126
  98%   1322
  99%   1372
 100%   1788 (longest request)

Lambda invoke with 1 DyanmoDB call. Webpack; local-released

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0   14  62.7      0     357
Processing:    33  204 289.8    104    1360
Waiting:       33  204 289.8    104    
1360
Total:         33  218 334.3    104    1700

Percentage of the requests served within a certain time (ms)
  50%    104
  66%    122
  75%    138
  80%    152
  90%    382
  95%   1197
  98%   1395
  99%   1433
 100%   1700 (longest request)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@trivikr trivikr self-requested a review March 25, 2021 22:13
@AllanZhengYP AllanZhengYP force-pushed the assume-role-automation-alternative-2 branch from 5fc28df to 0dc6a94 Compare March 25, 2021 23:41
@AllanZhengYP AllanZhengYP marked this pull request as ready for review March 25, 2021 23:41
@AllanZhengYP AllanZhengYP force-pushed the assume-role-automation-alternative-2 branch from 0dc6a94 to e9b7c02 Compare March 26, 2021 01:21
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@b7729ab). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 0dc6a94 differs from pull request most recent head e9b7c02. Consider uploading reports for the commit e9b7c02 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2179   +/-   ##
=======================================
  Coverage        ?   78.92%           
=======================================
  Files           ?      382           
  Lines           ?    16206           
  Branches        ?     3515           
=======================================
  Hits            ?    12791           
  Misses          ?     3415           
  Partials        ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7729ab...e9b7c02. Read the comment docs.

@aws-sdk-js-automation
Copy link

AWS CodeBuild CI Report

  • CodeBuild project: sdk-staging-test
  • Commit ID: 3c5b147
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@AllanZhengYP
Copy link
Contributor Author

This PR is replaced by #2221. Closing.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.