diff --git a/api/envoy/api/v2/route/route_components.proto b/api/envoy/api/v2/route/route_components.proto index d73fbb8674c90..feec8a370b1da 100644 --- a/api/envoy/api/v2/route/route_components.proto +++ b/api/envoy/api/v2/route/route_components.proto @@ -675,8 +675,8 @@ message RouteAction { message FilterState { // The name of the Object in the per-request filterState, which is an - // Envoy::Http::Hashable object. If there is no data associated with the key, - // or the stored object is not Envoy::Http::Hashable, no hash will be produced. + // Envoy::Hashable object. If there is no data associated with the key, + // or the stored object is not Envoy::Hashable, no hash will be produced. string key = 1 [(validate.rules).string = {min_bytes: 1}]; } diff --git a/api/envoy/config/route/v3/route_components.proto b/api/envoy/config/route/v3/route_components.proto index 5a915eee87ca1..90479bac5ee10 100644 --- a/api/envoy/config/route/v3/route_components.proto +++ b/api/envoy/config/route/v3/route_components.proto @@ -752,8 +752,8 @@ message RouteAction { "envoy.api.v2.route.RouteAction.HashPolicy.FilterState"; // The name of the Object in the per-request filterState, which is an - // Envoy::Http::Hashable object. If there is no data associated with the key, - // or the stored object is not Envoy::Http::Hashable, no hash will be produced. + // Envoy::Hashable object. If there is no data associated with the key, + // or the stored object is not Envoy::Hashable, no hash will be produced. string key = 1 [(validate.rules).string = {min_len: 1}]; } diff --git a/api/envoy/type/v3/hash_policy.proto b/api/envoy/type/v3/hash_policy.proto index 96c39299698fc..41c4fb2d2b7f1 100644 --- a/api/envoy/type/v3/hash_policy.proto +++ b/api/envoy/type/v3/hash_policy.proto @@ -23,9 +23,20 @@ message HashPolicy { option (udpa.annotations.versioning).previous_message_type = "envoy.type.HashPolicy.SourceIp"; } + // An Object in the :ref:`filterState ` will be used + // to compute the hash used by hash-based load balancing algorithms. + message FilterState { + // The name of the Object in the filterState, which is an Envoy::Hashable object. If there is no + // data associated with the key, or the stored object is not Envoy::Hashable, no hash will be + // produced. + string key = 1 [(validate.rules).string = {min_len: 1}]; + } + oneof policy_specifier { option (validate.required) = true; SourceIp source_ip = 1; + + FilterState filter_state = 2; } } diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 3c9439b5918ac..afa24510f4f76 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -57,6 +57,7 @@ New Features * http: added support for :ref:`retriable health check status codes `. * listener: added API for extensions to access :ref:`typed_filter_metadata ` configured in the listener's :ref:`metadata ` field. * oauth filter: added :ref:`cookie_names ` to allow overriding (default) cookie names (``BearerToken``, ``OauthHMAC``, and ``OauthExpires``) set by the filter. +* tcp: added a :ref:`FilterState ` :ref:`hash policy `, used by :ref:`TCP proxy ` to allow hashing load balancer algorithms to hash on objects in filter state. * thrift_proxy: add upstream response zone metrics in the form ``cluster.cluster_name.zone.local_zone.upstream_zone.thrift.upstream_resp_success``. * thrift_proxy: add upstream metrics to show decoding errors and whether exception is from local or remote, e.g. ``cluster.cluster_name.thrift.upstream_resp_exception_remote``. * thrift_proxy: add host level success/error metrics where success is a reply of type success and error is any other response to a call. diff --git a/envoy/common/BUILD b/envoy/common/BUILD index dd12783097de0..58aa7f91d6df9 100644 --- a/envoy/common/BUILD +++ b/envoy/common/BUILD @@ -99,3 +99,8 @@ envoy_cc_library( name = "scope_tracker_interface", hdrs = ["scope_tracker.h"], ) + +envoy_cc_library( + name = "hashable_interface", + hdrs = ["hashable.h"], +) diff --git a/envoy/common/hashable.h b/envoy/common/hashable.h new file mode 100644 index 0000000000000..af357911748eb --- /dev/null +++ b/envoy/common/hashable.h @@ -0,0 +1,25 @@ +#pragma once + +#include "envoy/common/pure.h" + +#include "absl/types/optional.h" + +namespace Envoy { + +/** + * Interface for hashable types used in heterogeneous contexts (see, for example, usage in + * FilterStateHashMethod). + */ +class Hashable { +public: + virtual ~Hashable() = default; + + /** + * Request the 64-bit hash for this object. + * @return absl::optional the hash value, or absl::nullopt if a hash could not be + * produced for this instance. + */ + virtual absl::optional hash() const PURE; +}; + +} // namespace Envoy diff --git a/envoy/http/hash_policy.h b/envoy/http/hash_policy.h index 2a031247d93d0..de04a8eecfa20 100644 --- a/envoy/http/hash_policy.h +++ b/envoy/http/hash_policy.h @@ -9,12 +9,6 @@ namespace Envoy { namespace Http { -class Hashable { -public: - virtual absl::optional hash() const PURE; - virtual ~Hashable() = default; -}; - /** * Request hash policy. I.e., if using a hashing load balancer, how a request should be hashed onto * an upstream host. diff --git a/envoy/network/BUILD b/envoy/network/BUILD index 3a292bdd6cd7b..d454518fbdbf4 100644 --- a/envoy/network/BUILD +++ b/envoy/network/BUILD @@ -102,7 +102,7 @@ envoy_cc_library( hdrs = ["hash_policy.h"], external_deps = ["abseil_optional"], deps = [ - ":address_interface", + ":connection_interface", ], ) diff --git a/envoy/network/hash_policy.h b/envoy/network/hash_policy.h index 49a38da8c14d4..c7d62591c1137 100644 --- a/envoy/network/hash_policy.h +++ b/envoy/network/hash_policy.h @@ -1,6 +1,6 @@ #pragma once -#include "envoy/network/address.h" +#include "envoy/network/connection.h" #include "absl/types/optional.h" @@ -14,14 +14,14 @@ class HashPolicy { virtual ~HashPolicy() = default; /** - * @param downstream_address is the address of the connected client. - * @param upstream_address is the address of the connected server. + * @param connection is the raw downstream connection. Different implementations of HashPolicy can + * compute hashes based on different data accessible from the connection (e.g. IP address, + * filter state, etc.). * @return absl::optional an optional hash value to route on. A hash value might not be - * returned if for example the downstream address is nullptr. + * returned if the hash policy implementation doesn't find the expected data in the connection + * (e.g. IP address is null, filter state is not populated, etc.). */ - virtual absl::optional - generateHash(const Network::Address::Instance* downstream_address, - const Network::Address::Instance* upstream_address) const PURE; + virtual absl::optional generateHash(const Network::Connection& connection) const PURE; }; } // namespace Network } // namespace Envoy diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 6298fe50b3b29..46155fea98790 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -346,6 +346,7 @@ envoy_cc_library( hdrs = ["hash_policy.h"], deps = [ ":utility_lib", + "//envoy/common:hashable_interface", "//envoy/http:hash_policy_interface", "@envoy_api//envoy/config/route/v3:pkg_cc_proto", ], diff --git a/source/common/http/hash_policy.cc b/source/common/http/hash_policy.cc index a2eda3f01579d..1364404fda231 100644 --- a/source/common/http/hash_policy.cc +++ b/source/common/http/hash_policy.cc @@ -2,6 +2,7 @@ #include +#include "envoy/common/hashable.h" #include "envoy/config/route/v3/route_components.pb.h" #include "source/common/common/matchers.h" diff --git a/source/common/http/hash_policy.h b/source/common/http/hash_policy.h index 31368e552408c..20425a0f4d469 100644 --- a/source/common/http/hash_policy.h +++ b/source/common/http/hash_policy.h @@ -8,8 +8,7 @@ namespace Envoy { namespace Http { /** - * Implementation of HashPolicy that reads from the proto route config and only currently supports - * hashing on an HTTP header. + * Implementation of HashPolicy that reads from the proto route config. */ class HashPolicyImpl : public HashPolicy { public: diff --git a/source/common/network/BUILD b/source/common/network/BUILD index d75313240dd63..b75b299dc7c8f 100644 --- a/source/common/network/BUILD +++ b/source/common/network/BUILD @@ -133,6 +133,8 @@ envoy_cc_library( srcs = ["hash_policy.cc"], hdrs = ["hash_policy.h"], deps = [ + "//envoy/common:hashable_interface", + "//envoy/network:connection_interface", "//envoy/network:hash_policy_interface", "//source/common/common:assert_lib", "//source/common/common:hash_lib", diff --git a/source/common/network/hash_policy.cc b/source/common/network/hash_policy.cc index 2baf9fe8d0e51..7957c19c5ff93 100644 --- a/source/common/network/hash_policy.cc +++ b/source/common/network/hash_policy.cc @@ -1,6 +1,7 @@ #include "source/common/network/hash_policy.h" #include "envoy/common/exception.h" +#include "envoy/common/hashable.h" #include "envoy/type/v3/hash_policy.pb.h" #include "source/common/common/assert.h" @@ -10,8 +11,8 @@ namespace Network { class SourceIpHashMethod : public HashPolicyImpl::HashMethod { public: - absl::optional evaluate(const Network::Address::Instance* downstream_addr, - const Network::Address::Instance*) const override { + absl::optional evaluate(const Network::Connection& connection) const override { + const auto* downstream_addr = connection.connectionInfoProvider().remoteAddress().get(); if (downstream_addr && downstream_addr->ip()) { ASSERT(!downstream_addr->ip()->addressAsString().empty()); return HashUtil::xxHash64(downstream_addr->ip()->addressAsString()); @@ -21,6 +22,22 @@ class SourceIpHashMethod : public HashPolicyImpl::HashMethod { } }; +class FilterStateHashMethod : public HashPolicyImpl::HashMethod { +public: + FilterStateHashMethod(absl::string_view key) : key_(key) {} + + absl::optional evaluate(const Network::Connection& connection) const override { + const auto& filter_state = connection.streamInfo().filterState(); + if (filter_state.hasData(key_)) { + return filter_state.getDataReadOnly(key_).hash(); + } + return absl::nullopt; + } + +private: + const std::string key_; +}; + HashPolicyImpl::HashPolicyImpl( const absl::Span& hash_policies) { ASSERT(hash_policies.size() == 1); @@ -28,15 +45,16 @@ HashPolicyImpl::HashPolicyImpl( case envoy::type::v3::HashPolicy::PolicySpecifierCase::kSourceIp: hash_impl_ = std::make_unique(); break; + case envoy::type::v3::HashPolicy::PolicySpecifierCase::kFilterState: + hash_impl_ = std::make_unique(hash_policies[0]->filter_state().key()); + break; default: NOT_REACHED_GCOVR_EXCL_LINE; } } -absl::optional -HashPolicyImpl::generateHash(const Network::Address::Instance* downstream_addr, - const Network::Address::Instance* upstream_addr) const { - return hash_impl_->evaluate(downstream_addr, upstream_addr); +absl::optional HashPolicyImpl::generateHash(const Network::Connection& connection) const { + return hash_impl_->evaluate(connection); } } // namespace Network diff --git a/source/common/network/hash_policy.h b/source/common/network/hash_policy.h index 1b5d09489fcf0..d2b0c10744951 100644 --- a/source/common/network/hash_policy.h +++ b/source/common/network/hash_policy.h @@ -1,5 +1,6 @@ #pragma once +#include "envoy/network/connection.h" #include "envoy/network/hash_policy.h" #include "envoy/type/v3/hash_policy.pb.h" @@ -15,16 +16,12 @@ class HashPolicyImpl : public Network::HashPolicy { explicit HashPolicyImpl(const absl::Span& hash_policy); // Network::HashPolicy - absl::optional - generateHash(const Network::Address::Instance* downstream_addr, - const Network::Address::Instance* upstream_addr) const override; + absl::optional generateHash(const Network::Connection& connection) const override; class HashMethod { public: virtual ~HashMethod() = default; - virtual absl::optional - evaluate(const Network::Address::Instance* downstream_addr, - const Network::Address::Instance* upstream_addr) const PURE; + virtual absl::optional evaluate(const Network::Connection& connection) const PURE; }; using HashMethodPtr = std::unique_ptr; diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h index 3b64e58f5add5..ccd01ac8c098e 100644 --- a/source/common/tcp_proxy/tcp_proxy.h +++ b/source/common/tcp_proxy/tcp_proxy.h @@ -259,9 +259,7 @@ class Filter : public Network::ReadFilter, absl::optional computeHashKey() override { auto hash_policy = config_->hashPolicy(); if (hash_policy) { - return hash_policy->generateHash( - downstreamConnection()->connectionInfoProvider().remoteAddress().get(), - downstreamConnection()->connectionInfoProvider().localAddress().get()); + return hash_policy->generateHash(*downstreamConnection()); } return {}; diff --git a/test/common/router/BUILD b/test/common/router/BUILD index 596a8a86e7a3f..93864dfe0e67a 100644 --- a/test/common/router/BUILD +++ b/test/common/router/BUILD @@ -25,6 +25,7 @@ envoy_cc_test_library( srcs = ["config_impl_test.cc"], deps = [ ":route_fuzz_proto_cc_proto", + "//envoy/common:hashable_interface", "//source/common/config:metadata_lib", "//source/common/http:header_map_lib", "//source/common/http:headers_lib", diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 30e4ec1d4fe4e..8ea8287a1a0d9 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -6,6 +6,7 @@ #include #include +#include "envoy/common/hashable.h" #include "envoy/config/route/v3/route.pb.h" #include "envoy/config/route/v3/route.pb.validate.h" #include "envoy/config/route/v3/route_components.pb.h" @@ -2803,7 +2804,7 @@ class RouterMatcherFilterStateHashPolicyTest : public RouterMatcherHashPolicyTes StreamInfo::FilterState::LifeSpan::FilterChain); } class NonHashable : public StreamInfo::FilterState::Object {}; - class HashableObj : public StreamInfo::FilterState::Object, public Http::Hashable { + class HashableObj : public StreamInfo::FilterState::Object, public Hashable { absl::optional hash() const override { return 12345; }; }; diff --git a/test/common/tcp_proxy/BUILD b/test/common/tcp_proxy/BUILD index 4105647647b6d..23b6a081fa267 100644 --- a/test/common/tcp_proxy/BUILD +++ b/test/common/tcp_proxy/BUILD @@ -54,6 +54,7 @@ envoy_cc_test( ], deps = [ ":tcp_proxy_test_base", + "//envoy/common:hashable_interface", ], ) diff --git a/test/common/tcp_proxy/config_test.cc b/test/common/tcp_proxy/config_test.cc index 891043958be7f..abbe6cf88cd3c 100644 --- a/test/common/tcp_proxy/config_test.cc +++ b/test/common/tcp_proxy/config_test.cc @@ -1,3 +1,5 @@ +#include "envoy/common/hashable.h" + #include "test/common/tcp_proxy/tcp_proxy_test_base.h" namespace Envoy { @@ -454,7 +456,22 @@ TEST(ConfigTest, HashWithSourceIpConfig) { EXPECT_NE(nullptr, config_obj.hashPolicy()); } -TEST(ConfigTest, HashWithSourceIpDefaultConfig) { +TEST(ConfigTest, HashWithFilterStateConfig) { + const std::string yaml = R"EOF( + stat_prefix: name + cluster: foo + hash_policy: + - filter_state: { + key: foo + } +)EOF"; + + NiceMock factory_context; + Config config_obj(constructConfigFromYaml(yaml, factory_context)); + EXPECT_NE(nullptr, config_obj.hashPolicy()); +} + +TEST(ConfigTest, HashWithDefaultConfig) { const std::string yaml = R"EOF( stat_prefix: name cluster: foo @@ -545,14 +562,7 @@ TEST_F(TcpProxyNonDeprecatedConfigRoutingTest, ClusterNameSet) { class TcpProxyHashingTest : public testing::Test { public: - void setup() { - const std::string yaml = R"EOF( - stat_prefix: name - cluster: fake_cluster - hash_policy: - - source_ip: {} - )EOF"; - + void setup(const std::string& yaml) { factory_context_.cluster_manager_.initializeThreadLocalClusters({"fake_cluster"}); config_ = std::make_shared(constructConfigFromYaml(yaml, factory_context_)); } @@ -571,23 +581,74 @@ class TcpProxyHashingTest : public testing::Test { NiceMock connection_; NiceMock filter_callbacks_; std::unique_ptr filter_; + + class HashableObj : public StreamInfo::FilterState::Object, public Hashable { + public: + absl::optional hash() const override { return 31337; } + }; }; -// Test TCP proxy use source IP to hash. +// Test TCP proxy using source IP to hash. TEST_F(TcpProxyHashingTest, HashWithSourceIp) { - setup(); + const std::string yaml = R"EOF( + stat_prefix: name + cluster: fake_cluster + hash_policy: + - source_ip: {} + )EOF"; + setup(yaml); initializeFilter(); + + // Ensure there is no remote address (MockStreamInfo sets one by default), and expect no hash. + connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress(nullptr); EXPECT_CALL(factory_context_.cluster_manager_.thread_local_cluster_, tcpConnPool(_, _)) .WillOnce(Invoke([](Upstream::ResourcePriority, Upstream::LoadBalancerContext* context) { - EXPECT_TRUE(context->computeHashKey().has_value()); + EXPECT_FALSE(context->computeHashKey().has_value()); return absl::nullopt; })); + filter_->onNewConnection(); + // Set remote address, and expect a hash. connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( std::make_shared("1.2.3.4", 1111)); - connection_.stream_info_.downstream_connection_info_provider_->setLocalAddress( - std::make_shared("2.3.4.5", 2222)); + EXPECT_CALL(factory_context_.cluster_manager_.thread_local_cluster_, tcpConnPool(_, _)) + .WillOnce(Invoke([](Upstream::ResourcePriority, Upstream::LoadBalancerContext* context) { + EXPECT_TRUE(context->computeHashKey().has_value()); + return absl::nullopt; + })); + filter_->onNewConnection(); +} + +// Test TCP proxy using filter state to hash. +TEST_F(TcpProxyHashingTest, HashWithFilterState) { + const std::string yaml = R"EOF( + stat_prefix: name + cluster: fake_cluster + hash_policy: + - filter_state: { + key: foo + } + )EOF"; + setup(yaml); + initializeFilter(); + + // Expect no hash when filter state is unset. + EXPECT_CALL(factory_context_.cluster_manager_.thread_local_cluster_, tcpConnPool(_, _)) + .WillOnce(Invoke([](Upstream::ResourcePriority, Upstream::LoadBalancerContext* context) { + EXPECT_FALSE(context->computeHashKey().has_value()); + return absl::nullopt; + })); + filter_->onNewConnection(); + // Set filter state, and expect HashableObj's hash is now used. + connection_.stream_info_.filter_state_->setData("foo", std::make_unique(), + StreamInfo::FilterState::StateType::ReadOnly, + StreamInfo::FilterState::LifeSpan::FilterChain); + EXPECT_CALL(factory_context_.cluster_manager_.thread_local_cluster_, tcpConnPool(_, _)) + .WillOnce(Invoke([](Upstream::ResourcePriority, Upstream::LoadBalancerContext* context) { + EXPECT_EQ(31337, context->computeHashKey().value()); + return absl::nullopt; + })); filter_->onNewConnection(); } diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 88f04acd7cf8f..059bf4509ac95 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -148,7 +148,6 @@ GSS GTEST GURL Grabbit -Hashable HC HCM HDS @@ -704,6 +703,7 @@ handshaker hardcoded hardcodes hardcoding +hashable hasher hashtagging hd