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

Clear last used provider when refreshing AWSCredentialsProviderChain #1125

Closed
wants to merge 1 commit into from

Conversation

mfenniak
Copy link

Currently AWSCredentialsProviderChain's cache of the last used credential provider is maintained across refresh() invocations in the provider chain. This means that after you've accessed the credentials once, refresh() effectively only refreshes the credential provider that was previously accessed; you can never switch to one earlier in the chain via a refresh.

This PR clears the cached last provider in a refresh.

@kiiadi
Copy link
Contributor

kiiadi commented Apr 28, 2017

I don't think we'll be able to accept this as is because it's a change in behaviour - existing customers may be relying on the fact that only the 'last used' provider is preserved. Couple of possibilities that I see we could do:

  1. Introduce a new method on the chain that does this (e.g clearLastUsed or even an overload of refresh() that takes a boolean to determine whether last used is cleared).
  2. Introduce a new implementation of the AWSCredentialsProviderChain that does this.
  3. Introduce a new constructor on AWSCredentialsProviderChain that takes a clearLastUsedOnRefresh boolean flag that controls this behaviour - the default would be false.

@shorea
Copy link
Contributor

shorea commented Apr 28, 2017

What's your use case?

@mfenniak
Copy link
Author

Thanks for the feedback, @kiiadi. I've updated this pull request, following the approach of making the cache clear behavior optional with a boolean flag.

@shorea: My use-case is a tool that may be configured to use instance profile credentials to download preconfigured credentials from S3, and write them to ~/.aws/credentials. I'd like to refresh the credential provider chain between these two steps, and in that configuration, the active credential provider would switch from one provider to another.

@shorea
Copy link
Contributor

shorea commented Apr 28, 2017

That sounds like a good candidate for implementing a custom AWSCredentialsProvider instead of trying to use a chain to achieve this behavior. Chains are more geared towards supporting running the same application to multiple environments (i.e. EC2, Lambda, ECS, local dev) where the credentials may be sourced from a different location depending on the env but they never change locations once sourced.

Also do you need to persist the credentials to the ~/.aws/credentials file or can you just cache them in memory? Will other processes use those credentials?

@mfenniak
Copy link
Author

mfenniak commented May 1, 2017

@shorea: That would be an alternative implementation, sure. But it would require a lot more development work than just downloading a file and calling refresh on the default credential provider. The tool under development here is running in a docker container, so downloading to ~/.aws/credentials is effectively not persistent.

I get that the use-case is unusual. I'm not really asking this SDK to support that use case; I have a workaround in place already, albeit it is ugly.

But it seems justifiable to me that there be a mechanism to clear a cache like the one in AWSCredentialsProviderChain. It seems to fit well with the refresh method, which is documented as 'Forces this credentials provider to refresh its credentials.' I think this PR adds the functionality in a clear and compatible manner. If it's not a change you'd like to adopt, I understand. 😀

Here's a workaround, for reference:

AWSCredentialsProviderChain defaultAWSCredentialsProviderChain =
    DefaultAWSCredentialsProviderChain.getInstance();
defaultAWSCredentialsProviderChain.setReuseLastProvider(false);
defaultAWSCredentialsProviderChain.refresh();
try { defaultAWSCredentialsProviderChain.getCredentials(); } catch (Exception e) { }
defaultAWSCredentialsProviderChain.setReuseLastProvider(true);

@shorea shorea requested a review from kiiadi May 15, 2017 15:36
@millems
Copy link
Contributor

millems commented Jun 8, 2018

Sorry to say that this use-case is a bit too specific for us to include in the general SDK. We may revisit this decision if more customers need the feature.

@millems millems closed this Jun 8, 2018
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.

5 participants