diff --git a/api/envoy/api/v2/route/route.proto b/api/envoy/api/v2/route/route.proto index e260cff69a963..a1f58891c8788 100644 --- a/api/envoy/api/v2/route/route.proto +++ b/api/envoy/api/v2/route/route.proto @@ -585,6 +585,27 @@ message RouteAction { // Connection properties hash policy. ConnectionProperties connection_properties = 3; } + + // The flag that shortcircuits the hash computing. This field provides a + // 'fallback' style of configuration: "if a terminal policy doesn't work, + // fallback to rest of the policy list", it saves time when the terminal + // policy works. + // + // 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 + // Header B false + // Header C false + // ========= ======== + // + // The generateHash process ends if policy "header A" generates a hash, 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 +617,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 diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index d5301073fd1ac..110000f35096f 100644 --- a/source/common/router/config_impl.cc +++ b/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 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& ttl) - : key_(key), path_(path), ttl_(ttl) {} + const absl::optional& ttl, bool terminal) + : HashMethodImplBase(terminal), key_(key), path_(path), ttl_(ttl) {} absl::optional evaluate(const Network::Address::Instance*, const Http::HeaderMap& headers, const HashPolicy::AddCookieCallback add_cookie) const override { absl::optional 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 ttl_; }; -class IpHashMethod : public HashPolicyImpl::HashMethod { +class IpHashMethod : public HashMethodImplBase { public: + IpHashMethod(bool terminal) : HashMethodImplBase(terminal) {} + absl::optional 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 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; } diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index 238f54b99d51b..3aa34f0a2b925 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -234,6 +234,9 @@ class HashPolicyImpl : public HashPolicy { virtual absl::optional 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 HashMethodPtr; diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 9951064466b8a..5c65467dda95f 100644 --- a/test/common/router/config_impl_test.cc +++ b/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 factory_context; {