Add terminal attribute to request hash.

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

@@ -585,6 +585,16 @@ message RouteAction {
       // Connection properties hash policy.
       ConnectionProperties connection_properties = 3;
     }
+
+    // If true, and there is already a hash computed, ignore rest of the
+    // list of hash polices.
+    // For example, if the following hash methods are configured:
+    //   [(specifier, terminal),...]
+    //   [(Header A,    true),
+    //    (Cookie B,    false),
+    //    (Cookie C,    false)]
+    // The generateHash process ends after computing "header A", as it's a terminal policy.
+    bool terminal = 4;
   }
 
   // Specifies a list of hash policies to use for ring hash load balancing. Each
@@ -596,7 +606,9 @@ message RouteAction {
   // hash policies fail to generate a hash, no hash will be produced for
   // the route. In this case, the behavior is the same as if no hash policies
   // were specified (i.e. the ring hash load balancer will choose a random
-  // backend).
+  // backend). If a hash policy has the "terminal" attribute set to true, and
+  // there is already a hash generated, the hash is returned immediately,
+  // ignoring the rest of the hash policy list.
   repeated HashPolicy hash_policy = 15;
 
   // Indicates that a HTTP/1.1 client connection to this particular route is allowed to

source/common/router/config_impl.cc

@@ -76,9 +76,20 @@ ShadowPolicyImpl::ShadowPolicyImpl(const envoy::api::v2::route::RouteAction& con
   runtime_key_ = config.request_mirror_policy().runtime_key();
 }
 
-class HeaderHashMethod : public HashPolicyImpl::HashMethod {
+class HashMethodImplBase : public HashPolicyImpl::HashMethod {
 public:
-  HeaderHashMethod(const std::string& header_name) : header_name_(header_name) {}
+  HashMethodImplBase(bool terminal) : terminal_(terminal) {}
+
+  bool terminal() const override { return terminal_; }
+
+private:
+  const bool terminal_;
+};
+
+class HeaderHashMethod : public HashMethodImplBase {
+public:
+  HeaderHashMethod(const std::string& header_name, bool terminal)
+      : HashMethodImplBase(terminal), header_name_(header_name) {}
 
   absl::optional<uint64_t> evaluate(const Network::Address::Instance*,
                                     const Http::HeaderMap& headers,
@@ -96,18 +107,17 @@ class HeaderHashMethod : public HashPolicyImpl::HashMethod {
   const Http::LowerCaseString header_name_;
 };
 
-class CookieHashMethod : public HashPolicyImpl::HashMethod {
+class CookieHashMethod : public HashMethodImplBase {
 public:
   CookieHashMethod(const std::string& key, const std::string& path,
-                   const absl::optional<std::chrono::seconds>& ttl)
-      : key_(key), path_(path), ttl_(ttl) {}
+                   const absl::optional<std::chrono::seconds>& ttl, bool terminal)
+      : HashMethodImplBase(terminal), key_(key), path_(path), ttl_(ttl) {}
 
   absl::optional<uint64_t> evaluate(const Network::Address::Instance*,
                                     const Http::HeaderMap& headers,
                                     const HashPolicy::AddCookieCallback add_cookie) const override {
     absl::optional<uint64_t> hash;
     std::string value = Http::Utility::parseCookieValue(headers, key_);
-
     if (value.empty() && ttl_.has_value()) {
       value = add_cookie(key_, path_, ttl_.value());
       hash = HashUtil::xxHash64(value);
@@ -124,8 +134,10 @@ class CookieHashMethod : public HashPolicyImpl::HashMethod {
   const absl::optional<std::chrono::seconds> ttl_;
 };
 
-class IpHashMethod : public HashPolicyImpl::HashMethod {
+class IpHashMethod : public HashMethodImplBase {
 public:
+  IpHashMethod(bool terminal) : HashMethodImplBase(terminal) {}
+
   absl::optional<uint64_t> evaluate(const Network::Address::Instance* downstream_addr,
                                     const Http::HeaderMap&,
                                     const HashPolicy::AddCookieCallback) const override {
@@ -153,20 +165,21 @@ HashPolicyImpl::HashPolicyImpl(
   for (auto& hash_policy : hash_policies) {
     switch (hash_policy.policy_specifier_case()) {
     case envoy::api::v2::route::RouteAction::HashPolicy::kHeader:
-      hash_impls_.emplace_back(new HeaderHashMethod(hash_policy.header().header_name()));
+      hash_impls_.emplace_back(
+          new HeaderHashMethod(hash_policy.header().header_name(), hash_policy.terminal()));
       break;
     case envoy::api::v2::route::RouteAction::HashPolicy::kCookie: {
       absl::optional<std::chrono::seconds> ttl;
       if (hash_policy.cookie().has_ttl()) {
         ttl = std::chrono::seconds(hash_policy.cookie().ttl().seconds());
       }
-      hash_impls_.emplace_back(
-          new CookieHashMethod(hash_policy.cookie().name(), hash_policy.cookie().path(), ttl));
+      hash_impls_.emplace_back(new CookieHashMethod(
+          hash_policy.cookie().name(), hash_policy.cookie().path(), ttl, hash_policy.terminal()));
       break;
     }
     case envoy::api::v2::route::RouteAction::HashPolicy::kConnectionProperties:
       if (hash_policy.connection_properties().source_ip()) {
-        hash_impls_.emplace_back(new IpHashMethod());
+        hash_impls_.emplace_back(new IpHashMethod(hash_policy.terminal()));
       }
       break;
     default:
@@ -190,6 +203,11 @@ HashPolicyImpl::generateHash(const Network::Address::Instance* downstream_addr,
       const uint64_t old_value = hash ? ((hash.value() << 1) | (hash.value() >> 63)) : 0;
       hash = old_value ^ new_hash.value();
     }
+    // If the policy is a terminal policy and a hash has been generated, ignore
+    // the rest of the hash policies.
+    if (hash_impl->terminal() && hash) {
+      break;
+    }
   }
   return hash;
 }

source/common/router/config_impl.h

@@ -234,6 +234,9 @@ class HashPolicyImpl : public HashPolicy {
     virtual absl::optional<uint64_t> evaluate(const Network::Address::Instance* downstream_addr,
                                               const Http::HeaderMap& headers,
                                               const AddCookieCallback add_cookie) const PURE;
+
+    // If the method is a terminal method, ignore rest of the hash policy chain.
+    virtual bool terminal() const PURE;
   };
 
   typedef std::unique_ptr<HashMethod> HashMethodPtr;

test/common/router/config_impl_test.cc

@@ -1699,6 +1699,65 @@ TEST_F(RouterMatcherHashPolicyTest, HashMultiple) {
   EXPECT_NE(hash_h, hash_both);
 }
 
+TEST_F(RouterMatcherHashPolicyTest, HashTerminal) {
+  // Hash policy list: cookie, header [terminal=true], user_ip.
+  auto route = route_config_.mutable_virtual_hosts(0)->mutable_routes(0)->mutable_route();
+  route->add_hash_policy()->mutable_cookie()->set_name("cookie_hash");
+  auto* header_hash = route->add_hash_policy();
+  header_hash->mutable_header()->set_header_name("foo_header");
+  header_hash->set_terminal(true);
+  route->add_hash_policy()->mutable_connection_properties()->set_source_ip(true);
+  Network::Address::Ipv4Instance address1("4.3.2.1");
+  Network::Address::Ipv4Instance address2("1.2.3.4");
+
+  uint64_t hash_1, hash_2;
+  // Test terminal works when there is hash computed, the rest of the policy
+  // list is ignored.
+  {
+    Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/foo", "GET");
+    headers.addCopy("Cookie", "cookie_hash=foo;");
+    headers.addCopy("foo_header", "bar");
+    Router::RouteConstSharedPtr route = config().route(headers, 0);
+    hash_1 = route->routeEntry()
+                 ->hashPolicy()
+                 ->generateHash(&address1, headers, add_cookie_nop_)
+                 .value();
+  }
+  {
+    Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/foo", "GET");
+    headers.addCopy("Cookie", "cookie_hash=foo;");
+    headers.addCopy("foo_header", "bar");
+    Router::RouteConstSharedPtr route = config().route(headers, 0);
+    hash_2 = route->routeEntry()
+                 ->hashPolicy()
+                 ->generateHash(&address2, headers, add_cookie_nop_)
+                 .value();
+  }
+  EXPECT_EQ(hash_1, hash_2);
+
+  // If no hash computed after evaluating a hash policy, the rest of the policy
+  // list is evaluated.
+  {
+    // Input: {}, {}, address1. Hash on address1.
+    Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/foo", "GET");
+    Router::RouteConstSharedPtr route = config().route(headers, 0);
+    hash_1 = route->routeEntry()
+                 ->hashPolicy()
+                 ->generateHash(&address1, headers, add_cookie_nop_)
+                 .value();
+  }
+  {
+    // Input: {}, {}, address2. Hash on address2.
+    Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/foo", "GET");
+    Router::RouteConstSharedPtr route = config().route(headers, 0);
+    hash_2 = route->routeEntry()
+                 ->hashPolicy()
+                 ->generateHash(&address2, headers, add_cookie_nop_)
+                 .value();
+  }
+  EXPECT_NE(hash_1, hash_2);
+}
+
 TEST_F(RouterMatcherHashPolicyTest, InvalidHashPolicies) {
   NiceMock<Server::Configuration::MockFactoryContext> factory_context;
   {