http: add core retry policy to route retry policy conversion utility

envoy/http/BUILD

@@ -32,6 +32,8 @@ envoy_cc_library(
         "//envoy/event:dispatcher_interface",
         "//envoy/tracing:http_tracer_interface",
         "//source/common/protobuf",
+        "//source/common/protobuf:utility_lib",
+        "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/route/v3:pkg_cc_proto",
     ],
 )

envoy/http/async_client.h

@@ -3,18 +3,24 @@
 #include <chrono>
 #include <memory>
 
+#include "envoy/config/core/v3/base.pb.h"
 #include "envoy/config/route/v3/route_components.pb.h"
 #include "envoy/event/dispatcher.h"
 #include "envoy/http/message.h"
 #include "envoy/stream_info/stream_info.h"
 #include "envoy/tracing/http_tracer.h"
 
 #include "source/common/protobuf/protobuf.h"
+#include "source/common/protobuf/utility.h"
 
 #include "absl/types/optional.h"
 
 namespace Envoy {
 namespace Http {
+namespace {
+constexpr uint32_t RetryInitialDelayMilliseconds = 1000;
+constexpr uint32_t RetryMaxDelayMilliseconds = 10 * 1000;
+} // namespace
 
 /**
  * Supports sending an HTTP request message and receiving a response asynchronously.
@@ -219,6 +225,43 @@ class AsyncClient {
       return *this;
     }
 
+    StreamOptions& setRetryPolicy(const envoy::config::core::v3::RetryPolicy& p,
+                                  const std::string& retry_on) {
+      envoy::config::route::v3::RetryPolicy route_retry_policy;
+
+      uint64_t base_interval_ms = RetryInitialDelayMilliseconds;
+      uint64_t max_interval_ms = RetryMaxDelayMilliseconds;
+
+      if (p.has_retry_back_off()) {
+        const auto& core_back_off = p.retry_back_off();
+
+        base_interval_ms = PROTOBUF_GET_MS_REQUIRED(core_back_off, base_interval);
+        max_interval_ms =
+            PROTOBUF_GET_MS_OR_DEFAULT(core_back_off, max_interval, base_interval_ms * 10);
+
+        if (max_interval_ms < base_interval_ms) {
+          max_interval_ms = base_interval_ms * 10;
+        }
+      }
+
+      route_retry_policy.mutable_num_retries()->set_value(
+          PROTOBUF_GET_WRAPPED_OR_DEFAULT(p, num_retries, 1));
+
+      auto* route_mutable_back_off = route_retry_policy.mutable_retry_back_off();
+
+      route_mutable_back_off->mutable_base_interval()->CopyFrom(
+          Protobuf::util::TimeUtil::MillisecondsToDuration(base_interval_ms));
+      route_mutable_back_off->mutable_max_interval()->CopyFrom(
+          Protobuf::util::TimeUtil::MillisecondsToDuration(max_interval_ms));
+
+      // set all the other fields with appropriate values.
+      route_retry_policy.set_retry_on(retry_on);
+      route_retry_policy.mutable_per_try_timeout()->CopyFrom(
+          route_retry_policy.retry_back_off().max_interval());
+
+      return setRetryPolicy(route_retry_policy);
+    }
+
     // For gmock test
     bool operator==(const StreamOptions& src) const {
       return timeout == src.timeout && buffer_body_for_retry == src.buffer_body_for_retry &&
@@ -286,6 +329,11 @@ class AsyncClient {
       StreamOptions::setRetryPolicy(p);
       return *this;
     }
+    RequestOptions& setRetryPolicy(const envoy::config::core::v3::RetryPolicy& p,
+                                   const std::string& retry_on) {
+      StreamOptions::setRetryPolicy(p, retry_on);
+      return *this;
+    }
     RequestOptions& setParentSpan(Tracing::Span& parent_span) {
       parent_span_ = &parent_span;
       return *this;

source/extensions/filters/http/common/jwks_fetcher.cc

@@ -16,60 +16,6 @@ namespace Extensions {
 namespace HttpFilters {
 namespace Common {
 namespace {
-// Parameters of the jittered backoff strategy.
-static constexpr uint32_t RetryInitialDelayMilliseconds = 1000;
-static constexpr uint32_t RetryMaxDelayMilliseconds = 10 * 1000;
-
-/**
- * @details construct an envoy.config.route.v3.RetryPolicy protobuf message
- *          from a less feature rich envoy.config.core.v3.RetryPolicy one.
- *
- *          this is about limiting the user's possibilities.
- *          just doing truncated exponential backoff
- *
- *          the upstream.use_retry feature flag will need to be turned on (default)
- *          for this to work.
- *
- * @param retry policy from the RemoteJwks proto
- * @return a retry policy usable by the http async client.
- */
-envoy::config::route::v3::RetryPolicy
-adaptRetryPolicy(const envoy::config::core::v3::RetryPolicy& core_retry_policy) {
-  envoy::config::route::v3::RetryPolicy route_retry_policy;
-
-  uint64_t base_interval_ms = RetryInitialDelayMilliseconds;
-  uint64_t max_interval_ms = RetryMaxDelayMilliseconds;
-
-  if (core_retry_policy.has_retry_back_off()) {
-    const auto& core_back_off = core_retry_policy.retry_back_off();
-
-    base_interval_ms = PROTOBUF_GET_MS_REQUIRED(core_back_off, base_interval);
-
-    max_interval_ms =
-        PROTOBUF_GET_MS_OR_DEFAULT(core_back_off, max_interval, base_interval_ms * 10);
-
-    if (max_interval_ms < base_interval_ms) {
-      throw EnvoyException("max_interval must be greater than or equal to the base_interval");
-    }
-  }
-
-  route_retry_policy.mutable_num_retries()->set_value(
-      PROTOBUF_GET_WRAPPED_OR_DEFAULT(core_retry_policy, num_retries, 1));
-
-  auto* route_mutable_back_off = route_retry_policy.mutable_retry_back_off();
-
-  route_mutable_back_off->mutable_base_interval()->CopyFrom(
-      Protobuf::util::TimeUtil::MillisecondsToDuration(base_interval_ms));
-  route_mutable_back_off->mutable_max_interval()->CopyFrom(
-      Protobuf::util::TimeUtil::MillisecondsToDuration(max_interval_ms));
-
-  // set all the other fields with appropriate values.
-  route_retry_policy.set_retry_on("5xx,gateway-error,connect-failure,reset");
-  route_retry_policy.mutable_per_try_timeout()->CopyFrom(
-      route_retry_policy.retry_back_off().max_interval());
-
-  return route_retry_policy;
-}
 
 class JwksFetcherImpl : public JwksFetcher,
                         public Logger::Loggable<Logger::Id::filter>,
@@ -78,10 +24,6 @@ class JwksFetcherImpl : public JwksFetcher,
   JwksFetcherImpl(Upstream::ClusterManager& cm, const RemoteJwks& remote_jwks)
       : cm_(cm), remote_jwks_(remote_jwks) {
     ENVOY_LOG(trace, "{}", __func__);
-
-    if (remote_jwks_.has_retry_policy()) {
-      route_retry_policy_ = adaptRetryPolicy(remote_jwks_.retry_policy());
-    }
   }
 
   ~JwksFetcherImpl() override { cancel(); }
@@ -122,7 +64,8 @@ class JwksFetcherImpl : public JwksFetcher,
                        .setChildSpanName("JWT Remote PubKey Fetch");
 
     if (remote_jwks_.has_retry_policy()) {
-      options.setRetryPolicy(route_retry_policy_.value());
+      options.setRetryPolicy(remote_jwks_.retry_policy(),
+                             "5xx,gateway-error,connect-failure,reset");
       options.setBufferBodyForRetry(true);
     }
 
@@ -178,11 +121,6 @@ class JwksFetcherImpl : public JwksFetcher,
   const RemoteJwks& remote_jwks_;
   Http::AsyncClient::Request* request_{};
 
-  // http async client uses richer semantics than the ones allowed in RemoteJwks
-  // envoy.config.route.v3.RetryPolicy vs envoy.config.core.v3.RetryPolicy
-  // mapping is done in constructor and reused.
-  absl::optional<envoy::config::route::v3::RetryPolicy> route_retry_policy_{absl::nullopt};
-
   void reset() {
     request_ = nullptr;
     receiver_ = nullptr;