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

envoy/http/BUILD

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

source/common/http/BUILD

@@ -454,6 +454,7 @@ envoy_cc_library(
         "//source/common/protobuf:utility_lib",
         "//source/common/runtime:runtime_features_lib",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
+        "@envoy_api//envoy/config/route/v3:pkg_cc_proto",
     ],
 )
 

source/common/http/utility.cc

@@ -1033,5 +1033,45 @@ Utility::AuthorityAttributes Utility::parseAuthority(absl::string_view host) {
   return {is_ip_address, host_to_resolve, port};
 }
 
+envoy::config::route::v3::RetryPolicy
+Utility::convertCoreToRouteRetryPolicy(const envoy::config::core::v3::RetryPolicy& retry_policy,
+                                       const std::string& retry_on) {
+  envoy::config::route::v3::RetryPolicy route_retry_policy;
+  constexpr uint64_t default_base_interval_ms = 1000;
+  constexpr uint64_t default_max_interval_ms = 10 * default_base_interval_ms;
+
+  uint64_t base_interval_ms = default_base_interval_ms;
+  uint64_t max_interval_ms = default_max_interval_ms;
+
+  if (retry_policy.has_retry_back_off()) {
+    const auto& core_back_off = 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(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(retry_on);
+  route_retry_policy.mutable_per_try_timeout()->CopyFrom(
+      route_retry_policy.retry_back_off().max_interval());
+
+  return route_retry_policy;
+}
+
 } // namespace Http
 } // namespace Envoy

source/common/http/utility.h

@@ -6,6 +6,7 @@
 
 #include "envoy/config/core/v3/http_uri.pb.h"
 #include "envoy/config/core/v3/protocol.pb.h"
+#include "envoy/config/route/v3/route_components.pb.h"
 #include "envoy/grpc/status.h"
 #include "envoy/http/codes.h"
 #include "envoy/http/filter.h"
@@ -571,6 +572,16 @@ struct AuthorityAttributes {
  * @return hostname parse result. that includes whether host is IP Address, hostname and port-name
  */
 AuthorityAttributes parseAuthority(absl::string_view host);
+
+/**
+ * It returns RetryPolicy defined in core api to route api.
+ * @param retry_policy core retry policy
+ * @param retry_on this specifies when retry should be invoked.
+ * @return route retry policy
+ */
+envoy::config::route::v3::RetryPolicy
+convertCoreToRouteRetryPolicy(const envoy::config::core::v3::RetryPolicy& retry_policy,
+                              const std::string& retry_on);
 } // namespace Utility
 } // namespace Http
 } // namespace Envoy

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

@@ -1,5 +1,6 @@
 #include "source/extensions/filters/http/common/jwks_fetcher.h"
 
+#include "envoy/config/core/v3/base.pb.h"
 #include "envoy/config/core/v3/http_uri.pb.h"
 
 #include "source/common/common/enum_to_int.h"
@@ -16,60 +17,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 +25,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 +65,10 @@ class JwksFetcherImpl : public JwksFetcher,
                        .setChildSpanName("JWT Remote PubKey Fetch");
 
     if (remote_jwks_.has_retry_policy()) {
-      options.setRetryPolicy(route_retry_policy_.value());
+      envoy::config::route::v3::RetryPolicy route_retry_policy =
+          Http::Utility::convertCoreToRouteRetryPolicy(remote_jwks_.retry_policy(),
+                                                       "5xx,gateway-error,connect-failure,reset");
+      options.setRetryPolicy(route_retry_policy);
       options.setBufferBodyForRetry(true);
     }
 
@@ -178,11 +124,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;

test/common/http/utility_test.cc

@@ -971,6 +971,37 @@ TEST(HttpUtility, CheckIsIpAddress) {
   }
 }
 
+TEST(HttpUtility, TestConvertCoreToRouteRetryPolicy) {
+  const std::string core_policy = R"(
+num_retries: 10
+)";
+
+  envoy::config::core::v3::RetryPolicy core_retry_policy;
+  TestUtility::loadFromYaml(core_policy, core_retry_policy);
+
+  const envoy::config::route::v3::RetryPolicy route_retry_policy =
+      Utility::convertCoreToRouteRetryPolicy(core_retry_policy,
+                                             "5xx,gateway-error,connect-failure,reset");
+  EXPECT_EQ(route_retry_policy.num_retries().value(), 10);
+  EXPECT_EQ(route_retry_policy.per_try_timeout().seconds(), 10);
+  EXPECT_EQ(route_retry_policy.retry_back_off().base_interval().seconds(), 1);
+  EXPECT_EQ(route_retry_policy.retry_back_off().max_interval().seconds(), 10);
+  EXPECT_EQ(route_retry_policy.retry_on(), "5xx,gateway-error,connect-failure,reset");
+
+  const std::string core_policy2 = R"(
+retry_back_off:
+  base_interval: 32s
+  max_interval: 1s
+num_retries: 10
+)";
+
+  envoy::config::core::v3::RetryPolicy core_retry_policy2;
+  TestUtility::loadFromYaml(core_policy2, core_retry_policy2);
+  EXPECT_THROW_WITH_MESSAGE(Utility::convertCoreToRouteRetryPolicy(core_retry_policy2, "5xx"),
+                            EnvoyException,
+                            "max_interval must be greater than or equal to the base_interval");
+}
+
 // Validates TE header is stripped if it contains an unsupported value
 // Also validate the behavior if a nominated header does not exist
 TEST(HttpUtility, TestTeHeaderGzipTrailersSanitized) {