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

EC2CredentialsFetcher refreshes credentials too eagerly #1893

Closed
jutley opened this issue Jan 23, 2019 · 4 comments
Closed

EC2CredentialsFetcher refreshes credentials too eagerly #1893

jutley opened this issue Jan 23, 2019 · 4 comments
Labels
feature-request A feature should be added or improved. needs-discussion This issue/PR requires more discussion with community.

Comments

@jutley
Copy link

jutley commented Jan 23, 2019

In this SDK, EC2 Instance Profile credentials are refreshed if they are within 15 minutes of expiration, as seen here:
https://github.com/aws/aws-sdk-java/blob/master/aws-java-sdk-core/src/main/java/com/amazonaws/auth/EC2CredentialsFetcher.java#L43-L53

I believe this is an issue for a couple reasons:

  1. Most AWS SDKs refresh tokens if they are within 5 minutes of expiration. I verified this in the Ruby and Go SDKs. Javascript does not seem to refresh tokens early. Java is far longer than any other SDK I've seen so far.
  2. 15 minutes is the minimum length of time a token can be created for, which matches this SDK's early expiration window. This is problematic as there are 3rd party tools that help to manage Instance Profile credentials (such as KIAM, in our case) that default to using the most conservative configurations possible (reasonably so). In this scenario, this SDK refreshes tokens on EVERY request.
  3. This is non-configurable.

I think this 15 minute window should be dropped to 5 minutes to match the other libraries and enable conservative token policies.

@millems
Copy link
Contributor

millems commented Jan 24, 2019

Sounds like a reasonable proposal, and easy to fix. +1 to making it 5 minutes (to match the other SDKs) and making it configurable.

@millems millems added the needs-discussion This issue/PR requires more discussion with community. label Jan 24, 2019
@idimaster
Copy link

Under load synchronization on fetchCredentials method caused significant performance degradation

@tmc24win
Copy link

I am also observing a significant performance degradation as a result of this synchronized fetchCredentials() function.

Can you remove the synchronized in addition to the 5 minute reduction?

@millems
Copy link
Contributor

millems commented Mar 31, 2020

Unfortunately we're concentrating our development effort on 2.x of the SDK (based on customer feedback), and aren't planning to make many changes in 1.11.x at this time. If this is still an issue in 2.x of the SDK, please open an issue there and we'll investigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. needs-discussion This issue/PR requires more discussion with community.
Projects
None yet
Development

No branches or pull requests

5 participants