Add retriable headers retry policy. (envoyproxy#8187)

Configured via 'retriable-headers' retry policy and 'retriable_headers'
list of headers (both can be set via config or request headers) . If the
upstream response has any of the retriable headers set, retry will be
triggered.

Signed-off-by: Oleg Shaldibin <[email protected]>

api/envoy/api/v2/route/route.proto

@@ -848,7 +848,7 @@ message RouteAction {
 }
 
 // HTTP retry :ref:`architecture overview <arch_overview_http_routing_retry>`.
-// [#comment:next free field: 9]
+// [#comment:next free field: 10]
 message RetryPolicy {
   // Specifies the conditions under which retry takes place. These are the same
   // conditions documented for :ref:`config_http_filters_router_x-envoy-retry-on` and
@@ -933,6 +933,11 @@ message RetryPolicy {
   // the base interval. The documentation for :ref:`config_http_filters_router_x-envoy-max-retries`
   // describes Envoy's back-off algorithm.
   RetryBackOff retry_back_off = 8;
+
+  // HTTP headers that trigger a retry if present in the response. A retry will be
+  // triggered if any of the header matches match the upstream response headers.
+  // The field is only consulted if 'retriable-headers' retry policy is active.
+  repeated HeaderMatcher retriable_headers = 9;
 }
 
 // HTTP request hedging :ref:`architecture overview <arch_overview_http_routing_hedging>`.

docs/root/configuration/http/http_filters/router_filter.rst

@@ -110,6 +110,11 @@ retriable-status-codes
   in either :ref:`the retry policy <envoy_api_field_route.RetryPolicy.retriable_status_codes>`
   or in the :ref:`config_http_filters_router_x-envoy-retriable-status-codes` header.
 
+retriable-headers
+  Envoy will attempt a retry if the upstream server response includes any headers matching in either
+  :ref:`the retry policy <envoy_api_field_route.RetryPolicy.retriable_headers>` or in the
+  :ref:`config_http_filters_router_x-envoy-retriable-header-names` header.
+
 The number of retries can be controlled via the
 :ref:`config_http_filters_router_x-envoy-max-retries` header or via the :ref:`route
 configuration <envoy_api_field_route.RouteAction.retry_policy>` or via the
@@ -158,6 +163,25 @@ Note that retry policies can also be applied at the :ref:`route level
 
 By default, Envoy will *not* perform retries unless you've configured them per above.
 
+.. _config_http_filters_router_x-envoy-retriable-header-names:
+
+x-envoy-retriable-header-names
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Setting this header informs Envoy about what response headers should be considered retriable. It is used
+in conjunction with the :ref:`retriable-headers <config_http_filters_router_x-envoy-retry-on>` retry policy.
+When the corresponding retry policy is set, the response headers provided by this list header value will be
+considered retriable in addition to the response headers enabled for retry through other retry policies.
+
+The list is a comma-separated list of header names: "X-Upstream-Retry,X-Try-Again" would cause any upstream
+responses containing either one of the specified headers to be retriable if 'retriable-headers' retry policy
+is enabled. Header names are case-insensitive.
+
+Only the names of retriable response headers can be specified via the request header. A more sophisticated
+retry policy based on the response headers can be specified by using arbitrary header matching rules
+via :ref:`retry policy configuration <envoy_api_field_route.RetryPolicy.retriable_headers>`.
+
+This header will only be honored for requests from internal clients.
+
 .. _config_http_filters_router_x-envoy-retriable-status-codes:
 
 x-envoy-retriable-status-codes

docs/root/intro/version_history.rst

@@ -49,6 +49,7 @@ Version history
 * rbac: added conditions to the policy, see :ref:`condition <envoy_api_field_config.rbac.v2.Policy.condition>`.
 * router: added :ref:`rq_retry_skipped_request_not_complete <config_http_filters_router_stats>` counter stat to router stats.
 * router: :ref:`Scoped routing <arch_overview_http_routing_route_scope>` is supported.
+* router: added new :ref:`retriable-headers <config_http_filters_router_x-envoy-retry-on>` retry policy. Retries can now be configured to trigger by arbitrary response header matching.
 * router check tool: add coverage reporting & enforcement.
 * router check tool: add comprehensive coverage reporting.
 * router check tool: add deprecated field check.

include/envoy/http/header_map.h

@@ -300,6 +300,7 @@ class HeaderEntry {
   HEADER_FUNC(EnvoyRetryOn)                                                                        \
   HEADER_FUNC(EnvoyRetryGrpcOn)                                                                    \
   HEADER_FUNC(EnvoyRetriableStatusCodes)                                                           \
+  HEADER_FUNC(EnvoyRetriableHeaderNames)                                                           \
   HEADER_FUNC(EnvoyUpstreamAltStatName)                                                            \
   HEADER_FUNC(EnvoyUpstreamCanary)                                                                 \
   HEADER_FUNC(EnvoyUpstreamHealthCheckedCluster)                                                   \
@@ -555,5 +556,20 @@ using HeaderMapPtr = std::unique_ptr<HeaderMap>;
  */
 using HeaderVector = std::vector<std::pair<LowerCaseString, std::string>>;
 
+/**
+ * An interface to be implemented by header matchers.
+ */
+class HeaderMatcher {
+public:
+  virtual ~HeaderMatcher() = default;
+
+  /*
+   * Check whether header matcher matches any headers in a given HeaderMap.
+   */
+  virtual bool matchesHeaders(const HeaderMap& headers) const PURE;
+};
+
+using HeaderMatcherSharedPtr = std::shared_ptr<HeaderMatcher>;
+
 } // namespace Http
 } // namespace Envoy

include/envoy/router/router.h

@@ -160,6 +160,7 @@ class RetryPolicy {
   static const uint32_t RETRY_ON_GRPC_INTERNAL           = 0x200;
   static const uint32_t RETRY_ON_RETRIABLE_STATUS_CODES  = 0x400;
   static const uint32_t RETRY_ON_RESET                   = 0x800;
+  static const uint32_t RETRY_ON_RETRIABLE_HEADERS       = 0x1000;
   // clang-format on
 
   virtual ~RetryPolicy() = default;
@@ -204,6 +205,12 @@ class RetryPolicy {
    */
   virtual const std::vector<uint32_t>& retriableStatusCodes() const PURE;
 
+  /**
+   * @return std::vector<Http::HeaderMatcherSharedPtr>& list of response header matchers that
+   * will be checked when the 'retriable-headers' retry policy is enabled.
+   */
+  virtual const std::vector<Http::HeaderMatcherSharedPtr>& retriableHeaders() const PURE;
+
   /**
    * @return absl::optional<std::chrono::milliseconds> base retry interval
    */

source/common/http/async_client_impl.h

@@ -132,12 +132,16 @@ class AsyncStreamImpl : public AsyncClient::Stream,
     const std::vector<uint32_t>& retriableStatusCodes() const override {
       return retriable_status_codes_;
     }
+    const std::vector<Http::HeaderMatcherSharedPtr>& retriableHeaders() const override {
+      return retriable_headers_;
+    }
     absl::optional<std::chrono::milliseconds> baseInterval() const override {
       return absl::nullopt;
     }
     absl::optional<std::chrono::milliseconds> maxInterval() const override { return absl::nullopt; }
 
     const std::vector<uint32_t> retriable_status_codes_{};
+    const std::vector<Http::HeaderMatcherSharedPtr> retriable_headers_{};
   };
 
   struct NullShadowPolicy : public Router::ShadowPolicy {

source/common/http/conn_manager_utility.cc

@@ -176,6 +176,7 @@ Network::Address::InstanceConstSharedPtr ConnectionManagerUtility::mutateRequest
     }
 
     request_headers.removeEnvoyRetriableStatusCodes();
+    request_headers.removeEnvoyRetriableHeaderNames();
     request_headers.removeEnvoyRetryOn();
     request_headers.removeEnvoyRetryGrpcOn();
     request_headers.removeEnvoyMaxRetries();

source/common/http/header_utility.h

@@ -35,7 +35,7 @@ class HeaderUtility {
   // A HeaderData specifies one of exact value or regex or range element
   // to match in a request's header, specified in the header_match_type_ member.
   // It is the runtime equivalent of the HeaderMatchSpecifier proto in RDS API.
-  struct HeaderData {
+  struct HeaderData : public HeaderMatcher {
     HeaderData(const envoy::api::v2::route::HeaderMatcher& config);
     HeaderData(const Json::Object& config);
 
@@ -45,18 +45,35 @@ class HeaderUtility {
     Regex::CompiledMatcherPtr regex_;
     envoy::type::Int64Range range_;
     const bool invert_match_;
+
+    // HeaderMatcher
+    bool matchesHeaders(const HeaderMap& headers) const override {
+      return HeaderUtility::matchHeaders(headers, *this);
+    };
   };
 
   using HeaderDataPtr = std::unique_ptr<HeaderData>;
 
   /**
-   * Build a vector of HeaderData given input config.
+   * Build a vector of HeaderDataPtr given input config.
    */
   static std::vector<HeaderUtility::HeaderDataPtr> buildHeaderDataVector(
       const Protobuf::RepeatedPtrField<envoy::api::v2::route::HeaderMatcher>& header_matchers) {
     std::vector<HeaderUtility::HeaderDataPtr> ret;
-    for (const auto& header_match : header_matchers) {
-      ret.emplace_back(std::make_unique<HeaderUtility::HeaderData>(header_match));
+    for (const auto& header_matcher : header_matchers) {
+      ret.emplace_back(std::make_unique<HeaderUtility::HeaderData>(header_matcher));
+    }
+    return ret;
+  }
+
+  /**
+   * Build a vector of HeaderMatcherSharedPtr given input config.
+   */
+  static std::vector<Http::HeaderMatcherSharedPtr> buildHeaderMatcherVector(
+      const Protobuf::RepeatedPtrField<envoy::api::v2::route::HeaderMatcher>& header_matchers) {
+    std::vector<Http::HeaderMatcherSharedPtr> ret;
+    for (const auto& header_matcher : header_matchers) {
+      ret.emplace_back(std::make_shared<HeaderUtility::HeaderData>(header_matcher));
     }
     return ret;
   }

source/common/http/headers.h

@@ -97,6 +97,8 @@ class HeaderValues {
   const LowerCaseString EnvoyRetryGrpcOn{absl::StrCat(prefix(), "-retry-grpc-on")};
   const LowerCaseString EnvoyRetriableStatusCodes{
       absl::StrCat(prefix(), "-retriable-status-codes")};
+  const LowerCaseString EnvoyRetriableHeaderNames{
+      absl::StrCat(prefix(), "-retriable-header-names")};
   const LowerCaseString EnvoyUpstreamAltStatName{absl::StrCat(prefix(), "-upstream-alt-stat-name")};
   const LowerCaseString EnvoyUpstreamCanary{absl::StrCat(prefix(), "-upstream-canary")};
   const LowerCaseString EnvoyUpstreamHostAddress{absl::StrCat(prefix(), "-upstream-host-address")};
@@ -208,6 +210,7 @@ class HeaderValues {
     const std::string RefusedStream{"refused-stream"};
     const std::string Retriable4xx{"retriable-4xx"};
     const std::string RetriableStatusCodes{"retriable-status-codes"};
+    const std::string RetriableHeaders{"retriable-headers"};
     const std::string Reset{"reset"};
   } EnvoyRetryOnValues;
 

source/common/router/BUILD

@@ -203,6 +203,7 @@ envoy_cc_library(
         "//source/common/common:utility_lib",
         "//source/common/grpc:common_lib",
         "//source/common/http:codes_lib",
+        "//source/common/http:header_utility_lib",
         "//source/common/http:headers_lib",
         "//source/common/http:utility_lib",
     ],

source/common/router/config_impl.cc

@@ -67,7 +67,9 @@ HedgePolicyImpl::HedgePolicyImpl()
 
 RetryPolicyImpl::RetryPolicyImpl(const envoy::api::v2::route::RetryPolicy& retry_policy,
                                  ProtobufMessage::ValidationVisitor& validation_visitor)
-    : validation_visitor_(&validation_visitor) {
+    : retriable_headers_(
+          Http::HeaderUtility::buildHeaderMatcherVector(retry_policy.retriable_headers())),
+      validation_visitor_(&validation_visitor) {
   per_try_timeout_ =
       std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(retry_policy, per_try_timeout, 0));
   num_retries_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(retry_policy, num_retries, 1);

source/common/router/config_impl.h

@@ -239,6 +239,9 @@ class RetryPolicyImpl : public RetryPolicy {
   const std::vector<uint32_t>& retriableStatusCodes() const override {
     return retriable_status_codes_;
   }
+  const std::vector<Http::HeaderMatcherSharedPtr>& retriableHeaders() const override {
+    return retriable_headers_;
+  }
   absl::optional<std::chrono::milliseconds> baseInterval() const override { return base_interval_; }
   absl::optional<std::chrono::milliseconds> maxInterval() const override { return max_interval_; }
 
@@ -255,6 +258,7 @@ class RetryPolicyImpl : public RetryPolicy {
   std::pair<std::string, ProtobufTypes::MessagePtr> retry_priority_config_;
   uint32_t host_selection_attempts_{1};
   std::vector<uint32_t> retriable_status_codes_;
+  std::vector<Http::HeaderMatcherSharedPtr> retriable_headers_;
   absl::optional<std::chrono::milliseconds> base_interval_;
   absl::optional<std::chrono::milliseconds> max_interval_;
   ProtobufMessage::ValidationVisitor* validation_visitor_{};

source/common/router/retry_state_impl.cc

@@ -21,6 +21,7 @@ const uint32_t RetryPolicy::RETRY_ON_5XX;
 const uint32_t RetryPolicy::RETRY_ON_GATEWAY_ERROR;
 const uint32_t RetryPolicy::RETRY_ON_CONNECT_FAILURE;
 const uint32_t RetryPolicy::RETRY_ON_RETRIABLE_4XX;
+const uint32_t RetryPolicy::RETRY_ON_RETRIABLE_HEADERS;
 const uint32_t RetryPolicy::RETRY_ON_RETRIABLE_STATUS_CODES;
 const uint32_t RetryPolicy::RETRY_ON_RESET;
 const uint32_t RetryPolicy::RETRY_ON_GRPC_CANCELLED;
@@ -56,7 +57,8 @@ RetryStateImpl::RetryStateImpl(const RetryPolicy& route_policy, Http::HeaderMap&
     : cluster_(cluster), runtime_(runtime), random_(random), dispatcher_(dispatcher),
       priority_(priority), retry_host_predicates_(route_policy.retryHostPredicates()),
       retry_priority_(route_policy.retryPriority()),
-      retriable_status_codes_(route_policy.retriableStatusCodes()) {
+      retriable_status_codes_(route_policy.retriableStatusCodes()),
+      retriable_headers_(route_policy.retriableHeaders()) {
 
   retry_on_ = route_policy.retryOn();
   retries_remaining_ = std::max(retries_remaining_, route_policy.numRetries());
@@ -93,6 +95,7 @@ RetryStateImpl::RetryStateImpl(const RetryPolicy& route_policy, Http::HeaderMap&
       retries_remaining_ = temp;
     }
   }
+
   if (request_headers.EnvoyRetriableStatusCodes()) {
     for (const auto code : StringUtil::splitToken(
              request_headers.EnvoyRetriableStatusCodes()->value().getStringView(), ",")) {
@@ -102,6 +105,21 @@ RetryStateImpl::RetryStateImpl(const RetryPolicy& route_policy, Http::HeaderMap&
       }
     }
   }
+
+  if (request_headers.EnvoyRetriableHeaderNames()) {
+    // Retriable headers in the configuration are specified via HeaderMatcher.
+    // Giving the same flexibility via request header would require the user
+    // to provide HeaderMatcher serialized into a string. To avoid this extra
+    // complexity we only support name-only header matchers via request
+    // header. Anything more sophisticated needs to be provided via config.
+    for (const auto header_name : StringUtil::splitToken(
+             request_headers.EnvoyRetriableHeaderNames()->value().getStringView(), ",")) {
+      envoy::api::v2::route::HeaderMatcher header_matcher;
+      header_matcher.set_name(std::string(absl::StripAsciiWhitespace(header_name)));
+      retriable_headers_.emplace_back(
+          std::make_shared<Http::HeaderUtility::HeaderData>(header_matcher));
+    }
+  }
 }
 
 RetryStateImpl::~RetryStateImpl() { resetRetry(); }
@@ -131,6 +149,8 @@ std::pair<uint32_t, bool> RetryStateImpl::parseRetryOn(absl::string_view config)
       ret |= RetryPolicy::RETRY_ON_REFUSED_STREAM;
     } else if (retry_on == Http::Headers::get().EnvoyRetryOnValues.RetriableStatusCodes) {
       ret |= RetryPolicy::RETRY_ON_RETRIABLE_STATUS_CODES;
+    } else if (retry_on == Http::Headers::get().EnvoyRetryOnValues.RetriableHeaders) {
+      ret |= RetryPolicy::RETRY_ON_RETRIABLE_HEADERS;
     } else if (retry_on == Http::Headers::get().EnvoyRetryOnValues.Reset) {
       ret |= RetryPolicy::RETRY_ON_RESET;
     } else {
@@ -265,6 +285,14 @@ bool RetryStateImpl::wouldRetryFromHeaders(const Http::HeaderMap& response_heade
     }
   }
 
+  if (retry_on_ & RetryPolicy::RETRY_ON_RETRIABLE_HEADERS) {
+    for (const auto& retriable_header : retriable_headers_) {
+      if (retriable_header->matchesHeaders(response_headers)) {
+        return true;
+      }
+    }
+  }
+
   if (retry_on_ &
       (RetryPolicy::RETRY_ON_GRPC_CANCELLED | RetryPolicy::RETRY_ON_GRPC_DEADLINE_EXCEEDED |
        RetryPolicy::RETRY_ON_GRPC_RESOURCE_EXHAUSTED | RetryPolicy::RETRY_ON_GRPC_UNAVAILABLE |

source/common/router/retry_state_impl.h

@@ -11,6 +11,7 @@
 #include "envoy/upstream/upstream.h"
 
 #include "common/common/backoff_strategy.h"
+#include "common/http/header_utility.h"
 
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
@@ -108,6 +109,7 @@ class RetryStateImpl : public RetryState {
   Upstream::RetryPrioritySharedPtr retry_priority_;
   uint32_t host_selection_max_attempts_;
   std::vector<uint32_t> retriable_status_codes_;
+  std::vector<Http::HeaderMatcherSharedPtr> retriable_headers_;
 };
 
 } // namespace Router

test/common/router/config_impl_test.cc

@@ -5762,6 +5762,43 @@ name: RetriableStatusCodes
   EXPECT_EQ(expected_codes, retry_policy.retriableStatusCodes());
 }
 
+TEST_F(RouteConfigurationV2, RetriableHeaders) {
+  const std::string RetriableHeaders = R"EOF(
+name: RetriableHeaders
+virtual_hosts:
+  - name: regex
+    domains: [idle.lyft.com]
+    routes:
+      - match:
+          safe_regex:
+            google_re2: {}
+            regex: "/regex"
+        route:
+          cluster: some-cluster
+          retry_policy:
+            retriable_headers:
+            - name: ":status"
+              exact_match: "500"
+            - name: X-Upstream-Pushback
+  )EOF";
+
+  TestConfigImpl config(parseRouteConfigurationFromV2Yaml(RetriableHeaders), factory_context_,
+                        true);
+  Http::TestHeaderMapImpl headers = genRedirectHeaders("idle.lyft.com", "/regex", true, false);
+  const auto& retry_policy = config.route(headers, 0)->routeEntry()->retryPolicy();
+  ASSERT_EQ(2, retry_policy.retriableHeaders().size());
+
+  Http::TestHeaderMapImpl expected_0{{":status", "500"}};
+  Http::TestHeaderMapImpl unexpected_0{{":status", "200"}};
+  Http::TestHeaderMapImpl expected_1{{"x-upstream-pushback", "bar"}};
+  Http::TestHeaderMapImpl unexpected_1{{"x-test", "foo"}};
+
+  EXPECT_TRUE(retry_policy.retriableHeaders()[0]->matchesHeaders(expected_0));
+  EXPECT_FALSE(retry_policy.retriableHeaders()[0]->matchesHeaders(unexpected_0));
+  EXPECT_TRUE(retry_policy.retriableHeaders()[1]->matchesHeaders(expected_1));
+  EXPECT_FALSE(retry_policy.retriableHeaders()[1]->matchesHeaders(unexpected_1));
+}
+
 TEST_F(RouteConfigurationV2, UpgradeConfigs) {
   const std::string UpgradeYaml = R"EOF(
 name: RetriableStatusCodes