search

envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
25.05k stars 4.82k forks source link

Retry failed ext_authz requests #17918

Open jvanhorenbeke opened 3 years ago

jvanhorenbeke commented 3 years ago

Title: Retry failed ext_authz requests

Description:

Is there a way to retry ext_authz requests after a network failure not an authorization service failure?

wbpcode commented 3 years ago

Nope. Although we can make a retry-able async http reqeust by setting retry policy in AsyncClient::RequestOptions, ext_authz does not expose it as an config for now.

jvanhorenbeke commented 3 years ago

Thank you. Is this by design? I would be open to build support for retries and push it upstream.

wbpcode commented 3 years ago

Thank you. Is this by design? I would be open to build support for retries and push it upstream.

I think this is just ignored in implementation. It's not because of a special design.

brianpgerson commented 3 years ago

A "major feature" is defined as any change that is > 100 LOC altered (not including tests), or changes any user-facing behavior. We will use the GitHub issue to discuss the feature and come to agreement. This is to prevent your time being wasted, as well as ours. The GitHub review process for major features is also important so that organizations with commit access can come to agreement on design. If it is appropriate to write a design document, the document must be hosted either in the GitHub tracking issue, or linked to from the issue and hosted in a world-readable location.

Reading the contributing guide's requests for any "major feature" - hard to say for sure whether this would be considered a major change from a LOC perspective, but would you recommend any further discussion before we might start working on this? (@jvanhorenbeke and i would likely be working together on this change)

esmet commented 3 years ago

@brianpgerson I think this is a minor feature. I agree with @wbpcode that we could use a retry policy in the AsyncClient's request options, and the fact that we don't do that right now is probably an oversight. If you don't start working on a PR for this yourself, I might give it a try soon-ish - maybe in the next few days.

brianpgerson commented 3 years ago

Thanks for weighing in - I've been looking for a good opportunity to contribute to Envoy (esp around ext_authz) so if you don't mind hanging back, I'd like to give it a try first (but will likely not be able to put up a PR in the next couple days).

esmet commented 3 years ago

Sure thing!

github-actions[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

ggreenway commented 3 months ago

This was added for gRPC ext_authz requests in #32241.

But there is still not a way to retry http ext_authz requests.