Skip to content

Conversation

@byronwolfman
Copy link
Contributor

Funnily enough, this (almost) same issue in lyft/metadataproxy is what led us to take a look at go-metadataproxy. :)

Once IAM credentials are 15 minutes away from expiring, the java sdk requests new ones:

https://github.com/aws/aws-sdk-java/blob/1.11.546/aws-java-sdk-core/src/main/java/com/amazonaws/auth/EC2CredentialsFetcher.java#L49-L53

    /**
     * The threshold before credentials expire (in milliseconds) at which
     * this class will attempt to load new credentials.
     */
    private static final int EXPIRATION_THRESHOLD = 1000 * 60 * 15;

The lyft/metadataproxy would cache credentials until 5 minutes before they expired, which meant that once credentials reached 45 minutes of age, the java sdk would storm it with requests for 10 minutes until they were 55 minutes old, and lyft/metadataproxy would refresh them. The java sdk would then stop, satisfied.

The go-metadataproxy also caches credentials, but, typically for 59 minutes:

	ttl := assumedRole.Credentials.Expiration.Sub(time.Now()) - 1*time.Minute
	permissionCache.Set(arn, assumedRole, ttl)

This means we'll see 14 minutes of pain instead of 10! Other sdks seem to set a lower limit (ruby sets its threshold at 5 minutes for example, making it a model citizen) so this problem may not show up in all environments.

With this PR I'm proposing setting a threshold of 15 minutes to expiry. If it's preferred though, I can rework this to make the expiration threshold a configurable environment variable, rather than a hardcoded value.

@byronwolfman
Copy link
Contributor Author

Since we seem to be in different timezones, I should probably ask in advance: if you would prefer this as an environment variable, then

  1. Should the default value be 1 minute to stay consistent with previous versions?
  2. If the environment variable is present but a non-integer, should go-metadataproxy fall back to default and emit a warning, or panic and refuse to start?

Thanks!

@jippi jippi closed this in 8b91f56 Mar 4, 2020
@byronwolfman byronwolfman deleted the 15-minute-expiry branch March 4, 2020 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant