From 19ad23bb3d1ce47f0273a1e4e24b648995540065 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Mon, 23 Aug 2021 02:16:25 +0000 Subject: [PATCH 01/15] http: refactor core retry policy config on stream options Signed-off-by: Shikugawa --- envoy/http/async_client.h | 48 ++++++++++++++ .../filters/http/common/jwks_fetcher.cc | 66 +------------------ 2 files changed, 50 insertions(+), 64 deletions(-) diff --git a/envoy/http/async_client.h b/envoy/http/async_client.h index 2e1917aaa413..f248fc74364d 100644 --- a/envoy/http/async_client.h +++ b/envoy/http/async_client.h @@ -3,6 +3,7 @@ #include #include +#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" @@ -10,11 +11,16 @@ #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 { +static constexpr uint32_t RetryInitialDelayMilliseconds = 1000; +static 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, + 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, + std::string&& retry_on) { + StreamOptions::setRetryPolicy(p, std::move(retry_on)); + return *this; + } RequestOptions& setParentSpan(Tracing::Span& parent_span) { parent_span_ = &parent_span; return *this; diff --git a/source/extensions/filters/http/common/jwks_fetcher.cc b/source/extensions/filters/http/common/jwks_fetcher.cc index 4b2e50594fd1..fa2950ce042e 100644 --- a/source/extensions/filters/http/common/jwks_fetcher.cc +++ b/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, @@ -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 route_retry_policy_{absl::nullopt}; - void reset() { request_ = nullptr; receiver_ = nullptr; From 94fc29be42570c48aca4eb0cbb27ee6e671703e5 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Mon, 23 Aug 2021 03:04:18 +0000 Subject: [PATCH 02/15] format Signed-off-by: Shikugawa --- envoy/http/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/envoy/http/BUILD b/envoy/http/BUILD index ee9e63a01d6c..401b7fc6f9e6 100644 --- a/envoy/http/BUILD +++ b/envoy/http/BUILD @@ -32,6 +32,7 @@ envoy_cc_library( "//envoy/event:dispatcher_interface", "//envoy/tracing:http_tracer_interface", "//source/common/protobuf", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/config/route/v3:pkg_cc_proto", ], ) From 28251964e44d304ca6cbf48afd96033f8a633fc4 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Mon, 23 Aug 2021 07:34:05 +0000 Subject: [PATCH 03/15] fix Signed-off-by: Shikugawa --- envoy/http/BUILD | 1 + envoy/http/async_client.h | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/envoy/http/BUILD b/envoy/http/BUILD index 401b7fc6f9e6..7f63c6a14d7b 100644 --- a/envoy/http/BUILD +++ b/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/core/v3:pkg_cc_proto", "@envoy_api//envoy/config/route/v3:pkg_cc_proto", ], diff --git a/envoy/http/async_client.h b/envoy/http/async_client.h index f248fc74364d..74081f8aa5c4 100644 --- a/envoy/http/async_client.h +++ b/envoy/http/async_client.h @@ -226,7 +226,7 @@ class AsyncClient { } StreamOptions& setRetryPolicy(const envoy::config::core::v3::RetryPolicy& p, - std::string&& retry_on) { + const std::string& retry_on) { envoy::config::route::v3::RetryPolicy route_retry_policy; uint64_t base_interval_ms = RetryInitialDelayMilliseconds; @@ -330,8 +330,8 @@ class AsyncClient { return *this; } RequestOptions& setRetryPolicy(const envoy::config::core::v3::RetryPolicy& p, - std::string&& retry_on) { - StreamOptions::setRetryPolicy(p, std::move(retry_on)); + const std::string& retry_on) { + StreamOptions::setRetryPolicy(p, retry_on); return *this; } RequestOptions& setParentSpan(Tracing::Span& parent_span) { From 202db77a335fe901c1e619c89955e26c260e7b36 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Wed, 25 Aug 2021 10:14:13 +0000 Subject: [PATCH 04/15] nitpick Signed-off-by: Shikugawa --- envoy/http/async_client.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/envoy/http/async_client.h b/envoy/http/async_client.h index 74081f8aa5c4..8bc82a363f02 100644 --- a/envoy/http/async_client.h +++ b/envoy/http/async_client.h @@ -18,8 +18,8 @@ namespace Envoy { namespace Http { namespace { -static constexpr uint32_t RetryInitialDelayMilliseconds = 1000; -static constexpr uint32_t RetryMaxDelayMilliseconds = 10 * 1000; +constexpr uint32_t RetryInitialDelayMilliseconds = 1000; +constexpr uint32_t RetryMaxDelayMilliseconds = 10 * 1000; } // namespace /** From bc243a355dae00cc2151a9c6dee45155e0332a26 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Fri, 3 Sep 2021 21:56:05 +0900 Subject: [PATCH 05/15] fix Signed-off-by: Shikugawa --- envoy/http/async_client.h | 46 ------------------- source/common/http/utility.cc | 37 +++++++++++++++ source/common/http/utility.h | 11 +++++ .../filters/http/common/jwks_fetcher.cc | 5 +- test/common/http/utility_test.cc | 28 +++++++++++ 5 files changed, 79 insertions(+), 48 deletions(-) diff --git a/envoy/http/async_client.h b/envoy/http/async_client.h index 8bc82a363f02..8b38d80bca8f 100644 --- a/envoy/http/async_client.h +++ b/envoy/http/async_client.h @@ -17,10 +17,6 @@ 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. @@ -225,43 +221,6 @@ 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 && @@ -329,11 +288,6 @@ 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; diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 07d24106e1f1..010b681793a0 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -1033,5 +1033,42 @@ 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; + uint64_t base_interval_ms = 1000; + uint64_t max_interval_ms = 10 * base_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 diff --git a/source/common/http/utility.h b/source/common/http/utility.h index 7929e687e580..75650605a2a5 100644 --- a/source/common/http/utility.h +++ b/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 diff --git a/source/extensions/filters/http/common/jwks_fetcher.cc b/source/extensions/filters/http/common/jwks_fetcher.cc index fa2950ce042e..8bd070f47ca7 100644 --- a/source/extensions/filters/http/common/jwks_fetcher.cc +++ b/source/extensions/filters/http/common/jwks_fetcher.cc @@ -64,8 +64,9 @@ class JwksFetcherImpl : public JwksFetcher, .setChildSpanName("JWT Remote PubKey Fetch"); if (remote_jwks_.has_retry_policy()) { - options.setRetryPolicy(remote_jwks_.retry_policy(), - "5xx,gateway-error,connect-failure,reset"); + auto route_retry_policy = Http::Utility::convertCoreToRouteRetryPolicy( + remote_jwks_.retry_policy(), "5xx,gateway-error,connect-failure,reset"); + options.setRetryPolicy(route_retry_policy); options.setBufferBodyForRetry(true); } diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index 8ffe06fbdfd9..63a6ee4f1432 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -971,6 +971,34 @@ TEST(HttpUtility, CheckIsIpAddress) { } } +TEST(HttpUtility, TestConvertCoreToRouteRetryPolicy) { + std::string core_policy = R"( +num_retries: 10 +)"; + + envoy::config::core::v3::RetryPolicy core_retry_policy; + TestUtility::loadFromYaml(core_policy, core_retry_policy); + + const auto 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"); + + 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(Utility::convertCoreToRouteRetryPolicy(core_retry_policy2, "5xx"), EnvoyException); +} + // 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) { From 6bb5fe71263e75b4117be1f9e6518a7be0035063 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Sat, 4 Sep 2021 09:52:32 +0900 Subject: [PATCH 06/15] format Signed-off-by: Shikugawa --- source/common/http/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/source/common/http/BUILD b/source/common/http/BUILD index ebc5bdcf0cf6..eab00ab11a19 100644 --- a/source/common/http/BUILD +++ b/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", ], ) From bb707fd40995a9350bde3c035c29f3e861ffddf6 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Wed, 8 Sep 2021 12:36:57 +0900 Subject: [PATCH 07/15] fix Signed-off-by: Shikugawa --- envoy/http/async_client.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/envoy/http/async_client.h b/envoy/http/async_client.h index 8b38d80bca8f..2e1917aaa413 100644 --- a/envoy/http/async_client.h +++ b/envoy/http/async_client.h @@ -3,7 +3,6 @@ #include #include -#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" @@ -11,7 +10,6 @@ #include "envoy/tracing/http_tracer.h" #include "source/common/protobuf/protobuf.h" -#include "source/common/protobuf/utility.h" #include "absl/types/optional.h" From 97b3649ac8789eb752bc979ca455cc71e8ab486d Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Wed, 8 Sep 2021 14:03:54 +0900 Subject: [PATCH 08/15] fix Signed-off-by: Shikugawa --- envoy/http/BUILD | 1 - 1 file changed, 1 deletion(-) diff --git a/envoy/http/BUILD b/envoy/http/BUILD index 7f63c6a14d7b..3c741ea6b449 100644 --- a/envoy/http/BUILD +++ b/envoy/http/BUILD @@ -33,7 +33,6 @@ envoy_cc_library( "//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", ], ) From 1497ba0bb5eed6b581657ff26e3eb5f13bad6636 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Thu, 9 Sep 2021 09:07:12 +0900 Subject: [PATCH 09/15] fix Signed-off-by: Shikugawa --- source/common/http/utility.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 010b681793a0..53c32e0bb06b 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -1037,8 +1037,8 @@ 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; - uint64_t base_interval_ms = 1000; - uint64_t max_interval_ms = 10 * base_interval_ms; + constexpr uint64_t base_interval_ms = 1000; + constexpr uint64_t max_interval_ms = 10 * base_interval_ms; if (retry_policy.has_retry_back_off()) { const auto& core_back_off = retry_policy.retry_back_off(); From b519b9606c79f1b90de568833ee15a7c504a3a9b Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Fri, 10 Sep 2021 01:52:26 +0000 Subject: [PATCH 10/15] fix Signed-off-by: Shikugawa --- source/common/http/utility.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 53c32e0bb06b..72e7a7374f7c 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -1,5 +1,6 @@ #include "source/common/http/utility.h" +#include #include #include @@ -1037,8 +1038,11 @@ 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 base_interval_ms = 1000; - constexpr uint64_t max_interval_ms = 10 * base_interval_ms; + 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(); From 9ec6b4da15d9740ab47b2f0e44b7b033d6a51f18 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Wed, 15 Sep 2021 06:52:44 +0000 Subject: [PATCH 11/15] fix Signed-off-by: Shikugawa --- source/common/http/utility.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 72e7a7374f7c..0408d5e18340 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -1,6 +1,5 @@ #include "source/common/http/utility.h" -#include #include #include From 9a44698d074353f4b5f95e2ea3f69a688439a5d2 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Wed, 22 Sep 2021 15:26:31 +0000 Subject: [PATCH 12/15] fix Signed-off-by: Shikugawa --- test/common/http/utility_test.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index 63a6ee4f1432..33fa4e8da760 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -996,7 +996,9 @@ num_retries: 10 envoy::config::core::v3::RetryPolicy core_retry_policy2; TestUtility::loadFromYaml(core_policy2, core_retry_policy2); - EXPECT_THROW(Utility::convertCoreToRouteRetryPolicy(core_retry_policy2, "5xx"), EnvoyException); + 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 From 38dbb326615061cdcacd6c4a8f3666fe124a2fbf Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Thu, 23 Sep 2021 01:49:46 +0000 Subject: [PATCH 13/15] ci Signed-off-by: Shikugawa From 85a5c6ff353d94623a87f49d6a0cb540f409d2b8 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Thu, 23 Sep 2021 08:05:06 +0000 Subject: [PATCH 14/15] ci Signed-off-by: Shikugawa From 5b2721b74302b0cb4ff57e9ae6cc6e6888291221 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Fri, 24 Sep 2021 13:18:11 +0000 Subject: [PATCH 15/15] fix Signed-off-by: Shikugawa --- source/extensions/filters/http/common/jwks_fetcher.cc | 6 ++++-- test/common/http/utility_test.cc | 9 +++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/source/extensions/filters/http/common/jwks_fetcher.cc b/source/extensions/filters/http/common/jwks_fetcher.cc index 8bd070f47ca7..70cdbbc4c77b 100644 --- a/source/extensions/filters/http/common/jwks_fetcher.cc +++ b/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" @@ -64,8 +65,9 @@ class JwksFetcherImpl : public JwksFetcher, .setChildSpanName("JWT Remote PubKey Fetch"); if (remote_jwks_.has_retry_policy()) { - auto route_retry_policy = Http::Utility::convertCoreToRouteRetryPolicy( - remote_jwks_.retry_policy(), "5xx,gateway-error,connect-failure,reset"); + 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); } diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index 33fa4e8da760..cd29642ff578 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -972,22 +972,23 @@ TEST(HttpUtility, CheckIsIpAddress) { } TEST(HttpUtility, TestConvertCoreToRouteRetryPolicy) { - std::string core_policy = R"( + 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 auto route_retry_policy = Utility::convertCoreToRouteRetryPolicy( - core_retry_policy, "5xx,gateway-error,connect-failure,reset"); + 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"); - std::string core_policy2 = R"( + const std::string core_policy2 = R"( retry_back_off: base_interval: 32s max_interval: 1s