Fixed an issue where request-level overrides (e.g. credentials) were not applied to endpoint discovery calls. #2657
Conversation
…not applied to endpoint discovery calls.
| AwsRequestOverrideConfiguration requestConfig = AwsRequestOverrideConfiguration.from(endpointDiscoveryRequest | ||
| .overrideConfiguration().orElse(null)); |
There was a problem hiding this comment.
One issue might be that request-level overrides like headers/query params could have unintended effects on the DescribeEndpoints request. What do you think about erring on the conservative side and just copying the credentials provider if it's present?
There was a problem hiding this comment.
I'm worried about only copying some things, because different people might have different expectations about what is used and not used. Why shouldn't the headers and query parameters be used for the discovery call? What about the API names (for user agent)? Signer? Metrics publishers? Some of these things we almost certainly want to include on the discovery call, so I'd favor just including them all so that there's no confusion.
Also, if we try to pick-and-choose now, when we add new stuff in the future, we'll definitely forget to ask "does this make sense to copy over for endpoint discovery?".
There was a problem hiding this comment.
Yep good points; I can't think of a use case where the other things on the override config would cause issues with the request, but I thought it was a valid concern. I'm in favor of just copying the override config as well.
There was a problem hiding this comment.
I don't mean to disregard your concern. I totally agree it's a valid concern. I'm just less fond of the other alternatives.
|
SonarCloud Quality Gate failed. |
…f37d95f51 Pull request: release <- staging/ac1a935f-4c29-467e-8331-51bf37d95f51
No description provided.