diff --git a/envoy/http/BUILD b/envoy/http/BUILD index ee9e63a01d6c2..3c741ea6b4490 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/route/v3:pkg_cc_proto", ], ) diff --git a/source/common/http/BUILD b/source/common/http/BUILD index ebc5bdcf0cf67..eab00ab11a19b 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", ], ) diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 07d24106e1f15..0408d5e183406 100644 --- a/source/common/http/utility.cc +++ b/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 diff --git a/source/common/http/utility.h b/source/common/http/utility.h index 7929e687e5802..75650605a2a58 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 4b2e50594fd13..70cdbbc4c77bf 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" @@ -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, @@ -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 route_retry_policy_{absl::nullopt}; - void reset() { request_ = nullptr; receiver_ = nullptr; diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index 8ffe06fbdfd93..cd29642ff5780 100644 --- a/test/common/http/utility_test.cc +++ b/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) {