-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Options.CredentialsProvider should support Context and returning an error #2681
Comments
This has been open for a while, is there any plan to implement this or provide further support for dynamic credentials? @monkey92t Any inputs? |
I apologize for not noticing this issue earlier. I believe we can easily resolve this problem by adding a CredentialsProviderContext, which seems to be a simpler and more compatible solution. CredentialsProvider func() (username string, password string)
CredentialsProviderContext func(ctx context.Context) (username string, password string, err error) It may seem somewhat redundant, but in other projects, it appears to be a common practice, such as with Dial() and DialContext(). |
Directly modifying the CredentialsProvider function seems more straightforward, but we must maintain compatibility with go-mod. I can accept CredentialsProvider+CredentialsProviderContext. We can mark their relationship in comments and address this issue in the future. |
Thanks @monkey92t! Any idea when it will be released? |
This was released in |
Expected Behavior
The interface for
Options.CredentialsProvider
should befunc(ctx context.Context) (username string, password string, err error)
baseClient.initConn
should return any error fromOptions.CredentialsProvider
Current Behavior
Options.CredentialsProvider
doesn't support context and doesn't pass errors down the call chain.Context (Environment)
I want to use Options.CredentialsProvider to implement dynamic password fetching from cloud providers. e.g. #2343
Detailed Description
Looking at the original commit #2097, the intent of CredentialsProvider seems to be to memory scrub plaintext passwords, but now people want to use it to dynamically fetch passwords from cloud providers. As the library generally supports Context, i'll skip the explanation for why its idiomatic and appropriate to pass context into code that is intended to make network requests. This would be a breaking change and require a version increment.
Possible Implementation
CredentialsProvider func(ctx context.Context) (username string, password string, err error)
The text was updated successfully, but these errors were encountered: