From 5ec91d97111b73885942fa6caad3d97d77a74f58 Mon Sep 17 00:00:00 2001 From: Stephan Zuercher Date: Mon, 2 Oct 2017 14:41:27 -0700 Subject: [PATCH 1/5] router: thread metadata_matches through to RouteEntryImplBase Signed-off-by: Stephan Zuercher --- include/envoy/router/BUILD | 4 ++ include/envoy/router/router.h | 38 +++++++++++++++++ source/common/http/async_client_impl.h | 1 + source/common/protobuf/utility.cc | 57 ++++++++++++++++++++++++++ source/common/protobuf/utility.h | 44 ++++++++++++++++++++ source/common/router/config_impl.h | 2 + test/mocks/router/mocks.h | 1 + 7 files changed, 147 insertions(+) diff --git a/include/envoy/router/BUILD b/include/envoy/router/BUILD index 3e6ffe26a56c4..c05c964fe3f81 100644 --- a/include/envoy/router/BUILD +++ b/include/envoy/router/BUILD @@ -36,9 +36,13 @@ envoy_cc_library( hdrs = ["router.h"], deps = [ "//include/envoy/common:optional", + "//include/envoy/http:access_log_interface", "//include/envoy/http:codec_interface", "//include/envoy/http:header_map_interface", + "//include/envoy/tracing:http_tracer_interface", "//include/envoy/upstream:resource_manager_interface", + "//source/common/protobuf", + "//source/common/protobuf:utility_lib", ], ) diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 5abf6bfc5e553..4895aea51aad8 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -15,6 +15,9 @@ #include "envoy/tracing/http_tracer.h" #include "envoy/upstream/resource_manager.h" +#include "common/protobuf/protobuf.h" +#include "common/protobuf/utility.h" + namespace Envoy { namespace Router { @@ -226,6 +229,35 @@ class HashPolicy { const Http::HeaderMap& headers) const PURE; }; +class MetadataMatch { +public: + virtual ~MetadataMatch() {} + + /* + * @return const std::string& the name of the metadata key + */ + virtual const std::string& name() const PURE; + + /* + * @return const Envoy::HashedValue& the value for the metadata key + */ + virtual const HashedValue& value() const PURE; +}; + +typedef std::shared_ptr MetadataMatchConstSharedPtr; + +class MetadataMatches { +public: + virtual ~MetadataMatches() {} + + /* + * @return std::vector& a vector of + * metadata to be matched against upstream endpoints when load + * balancing, sorted lexically by name. + */ + virtual const std::vector& metadataMatches() const PURE; +}; + /** * An individual resolved route entry. */ @@ -307,6 +339,12 @@ class RouteEntry { */ virtual bool useWebSocket() const PURE; + /** + * @return MetadataMatches* the metadata that a subset load balancer should match when selecting + * an upstream host + */ + virtual const MetadataMatches* metadataMatches() const PURE; + /** * @return const std::multimap the opaque configuration associated * with the route diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index 2371b097ab2fc..a7580a518beba 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -146,6 +146,7 @@ class AsyncStreamImpl : public AsyncClient::Stream, void finalizeRequestHeaders(Http::HeaderMap&, const Http::AccessLog::RequestInfo&) const override {} const Router::HashPolicy* hashPolicy() const override { return nullptr; } + const Router::MetadataMatches* metadataMatches() const override { return nullptr; } Upstream::ResourcePriority priority() const override { return Upstream::ResourcePriority::Default; } diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc index 19cc51a38742b..e0e6960108ca4 100644 --- a/source/common/protobuf/utility.cc +++ b/source/common/protobuf/utility.cc @@ -75,4 +75,61 @@ void MessageUtil::jsonConvert(const Protobuf::Message& source, Protobuf::Message MessageUtil::loadFromJson(json, dest); } +bool ValueUtil::equal(const ProtobufWkt::Value& v1, const ProtobufWkt::Value& v2) { + ProtobufWkt::Value::KindCase kind = v1.kind_case(); + if (kind != v2.kind_case()) { + return false; + } + + switch (kind) { + case ProtobufWkt::Value::kNullValue: + return true; + + case ProtobufWkt::Value::kNumberValue: + return v1.number_value() == v2.number_value(); + + case ProtobufWkt::Value::kStringValue: + return v1.string_value() == v2.string_value(); + + case ProtobufWkt::Value::kBoolValue: + return v1.bool_value() == v2.bool_value(); + + case ProtobufWkt::Value::kStructValue: { + const ProtobufWkt::Struct& s1 = v1.struct_value(); + const ProtobufWkt::Struct& s2 = v2.struct_value(); + if (s1.fields_size() != s2.fields_size()) { + return false; + } + for (const auto& it1 : s1.fields()) { + const auto& it2 = s2.fields().find(it1.first); + if (it2 == s2.fields().end()) { + return false; + } + + if (!equal(it1.second, it2->second)) { + return false; + } + } + return true; + } + + case ProtobufWkt::Value::kListValue: { + const ProtobufWkt::ListValue& l1 = v1.list_value(); + const ProtobufWkt::ListValue& l2 = v2.list_value(); + if (l1.values_size() != l2.values_size()) { + return false; + } + for (int i = 0; i < l1.values_size(); i++) { + if (!equal(l1.values(i), l2.values(i))) { + return false; + } + } + return true; + } + + default: + RELEASE_ASSERT(false); + } +} + } // namespace Envoy diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index f49305a44c5d7..8d794224ee184 100644 --- a/source/common/protobuf/utility.h +++ b/source/common/protobuf/utility.h @@ -110,4 +110,48 @@ class MessageUtil { } }; +class ValueUtil { +public: + static std::size_t hash(const ProtobufWkt::Value& value) { return MessageUtil::hash(value); } + + /** + * Compare two ProtobufWkt::Values for equality. + * @param v1 message of type type.googleapis.com/google.protobuf.Value + * @param v2 message of type type.googleapis.com/google.protobuf.Value + * @return true if v1 and v2 are identical + */ + static bool equal(const ProtobufWkt::Value& v1, const ProtobufWkt::Value& v2); +}; + +/** + * HashedValue is a wrapper around ProtobufWkt::Value that computes + * and stores a hash code for the Value at construction. + */ +class HashedValue { +public: + HashedValue(const ProtobufWkt::Value& value) : value_(value), hash_(ValueUtil::hash(value)){}; + HashedValue(const HashedValue& v) : value_(v.value_), hash_(v.hash_){}; + + const ProtobufWkt::Value& value() const { return value_; } + std::size_t hash() const { return hash_; } + + bool operator==(const HashedValue& rhs) const { + return hash_ == rhs.hash_ && ValueUtil::equal(value_, rhs.value_); + } + + bool operator!=(const HashedValue& rhs) const { return !(*this == rhs); } + +private: + const ProtobufWkt::Value value_; + const std::size_t hash_; +}; + } // namespace Envoy + +namespace std { +// Inject an implementation of std::hash for Envoy::HashedValue into the std namespace. +template <> struct hash { + std::size_t operator()(Envoy::HashedValue const& v) const { return v.hash(); } +}; + +} // namespace std diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index 12f7e104c8c7f..45433c6c8f2a1 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -255,6 +255,7 @@ class RouteEntryImplBase : public RouteEntry, void finalizeRequestHeaders(Http::HeaderMap& headers, const Http::AccessLog::RequestInfo& request_info) const override; const HashPolicy* hashPolicy() const override { return hash_policy_.get(); } + const MetadataMatches* metadataMatches() const override { return nullptr; } Upstream::ResourcePriority priority() const override { return priority_; } const RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; } const RetryPolicy& retryPolicy() const override { return retry_policy_; } @@ -315,6 +316,7 @@ class RouteEntryImplBase : public RouteEntry, const RetryPolicy& retryPolicy() const override { return parent_->retryPolicy(); } const ShadowPolicy& shadowPolicy() const override { return parent_->shadowPolicy(); } std::chrono::milliseconds timeout() const override { return parent_->timeout(); } + const MetadataMatches* metadataMatches() const override { return nullptr; } const VirtualCluster* virtualCluster(const Http::HeaderMap& headers) const override { return parent_->virtualCluster(headers); diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 0b1e99553d2f5..5734d41fde752 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -184,6 +184,7 @@ class MockRouteEntry : public RouteEntry { void(Http::HeaderMap& headers, const Http::AccessLog::RequestInfo& request_info)); MOCK_CONST_METHOD0(hashPolicy, const HashPolicy*()); + MOCK_CONST_METHOD0(metadataMatches, const Router::MetadataMatches*()); MOCK_CONST_METHOD0(priority, Upstream::ResourcePriority()); MOCK_CONST_METHOD0(rateLimitPolicy, const RateLimitPolicy&()); MOCK_CONST_METHOD0(retryPolicy, const RetryPolicy&()); From 80c4d17d97ff4764a69c69e55ef59f5293f3d260 Mon Sep 17 00:00:00 2001 From: Stephan Zuercher Date: Mon, 2 Oct 2017 15:20:38 -0700 Subject: [PATCH 2/5] protobuf: test new protobuf utilities Signed-off-by: Stephan Zuercher --- test/common/protobuf/utility_test.cc | 126 +++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc index 8a5b8b1b0f297..6a1365e041990 100644 --- a/test/common/protobuf/utility_test.cc +++ b/test/common/protobuf/utility_test.cc @@ -1,3 +1,5 @@ +#include + #include "common/protobuf/protobuf.h" #include "common/protobuf/utility.h" @@ -51,4 +53,128 @@ TEST(UtilityTest, LoadTextProtoFromFile_Failure) { "\" as a text protobuf (type envoy.api.v2.Bootstrap)"); } +TEST(UtilityTest, ValueUtilEqual_NullValues) { + ProtobufWkt::Value v1, v2; + v1.set_null_value(ProtobufWkt::NULL_VALUE); + v2.set_null_value(ProtobufWkt::NULL_VALUE); + + ProtobufWkt::Value other; + other.set_string_value("s"); + + EXPECT_TRUE(ValueUtil::equal(v1, v2)); + EXPECT_FALSE(ValueUtil::equal(v1, other)); +} + +TEST(UtilityTest, ValueUtilEqual_StringValues) { + ProtobufWkt::Value v1, v2, v3; + v1.set_string_value("s"); + v2.set_string_value("s"); + v3.set_string_value("not_s"); + + EXPECT_TRUE(ValueUtil::equal(v1, v2)); + EXPECT_FALSE(ValueUtil::equal(v1, v3)); +} + +TEST(UtilityTest, ValueUtilEqual_NumberValues) { + ProtobufWkt::Value v1, v2, v3; + v1.set_number_value(1.0); + v2.set_number_value(1.0); + v3.set_number_value(100.0); + + EXPECT_TRUE(ValueUtil::equal(v1, v2)); + EXPECT_FALSE(ValueUtil::equal(v1, v3)); +} + +TEST(UtilityTest, ValueUtilEqual_BoolValues) { + ProtobufWkt::Value v1, v2, v3; + v1.set_bool_value(true); + v2.set_bool_value(true); + v3.set_bool_value(false); + + EXPECT_TRUE(ValueUtil::equal(v1, v2)); + EXPECT_FALSE(ValueUtil::equal(v1, v3)); +} + +TEST(UtilityTest, ValueUtilEqual_StructValues) { + ProtobufWkt::Value string_val1, string_val2, bool_val; + + string_val1.set_string_value("s1"); + string_val2.set_string_value("s2"); + bool_val.set_bool_value(true); + + ProtobufWkt::Value v1, v2, v3, v4; + v1.mutable_struct_value()->mutable_fields()->insert({"f1", string_val1}); + v1.mutable_struct_value()->mutable_fields()->insert({"f2", bool_val}); + + v2.mutable_struct_value()->mutable_fields()->insert({"f1", string_val1}); + v2.mutable_struct_value()->mutable_fields()->insert({"f2", bool_val}); + + v3.mutable_struct_value()->mutable_fields()->insert({"f1", string_val2}); + v3.mutable_struct_value()->mutable_fields()->insert({"f2", bool_val}); + + v4.mutable_struct_value()->mutable_fields()->insert({"f1", string_val1}); + + EXPECT_TRUE(ValueUtil::equal(v1, v2)); + EXPECT_FALSE(ValueUtil::equal(v1, v3)); + EXPECT_FALSE(ValueUtil::equal(v1, v4)); +} + +TEST(UtilityTest, ValueUtilEqual_ListValues) { + ProtobufWkt::Value v1, v2, v3, v4; + v1.mutable_list_value()->add_values()->set_string_value("s"); + v1.mutable_list_value()->add_values()->set_bool_value(true); + + v2.mutable_list_value()->add_values()->set_string_value("s"); + v2.mutable_list_value()->add_values()->set_bool_value(true); + + v3.mutable_list_value()->add_values()->set_bool_value(true); + v3.mutable_list_value()->add_values()->set_string_value("s"); + + v4.mutable_list_value()->add_values()->set_string_value("s"); + + EXPECT_TRUE(ValueUtil::equal(v1, v2)); + EXPECT_FALSE(ValueUtil::equal(v1, v3)); + EXPECT_FALSE(ValueUtil::equal(v1, v4)); +} + +TEST(UtilityTest, ValueUtilHash) { + ProtobufWkt::Value v; + v.set_string_value("s1"); + + EXPECT_NE(ValueUtil::hash(v), 0); +} + +TEST(UtilityTest, HashedValue) { + ProtobufWkt::Value v1, v2, v3; + v1.set_string_value("s"); + v2.set_string_value("s"); + v3.set_string_value("not_s"); + + HashedValue hv1(v1), hv2(v2), hv3(v3); + + EXPECT_EQ(hv1, hv2); + EXPECT_NE(hv1, hv3); + + HashedValue copy(hv1); + EXPECT_EQ(hv1, copy); +} + +TEST(UtilityTest, HashedValueStdHash) { + ProtobufWkt::Value v1, v2, v3; + v1.set_string_value("s"); + v2.set_string_value("s"); + v3.set_string_value("not_s"); + + HashedValue hv1(v1), hv2(v2), hv3(v3); + + std::unordered_set set; + set.emplace(hv1); + set.emplace(hv2); + set.emplace(hv3); + + EXPECT_EQ(set.size(), 2); // hv1 == hv2 + EXPECT_NE(set.find(hv1), set.end()); + EXPECT_NE(set.find(hv3), set.end()); +} + } // namespace Envoy From 3e9a6a0e840bd7d5a713ca31d3485bdbd06cae51 Mon Sep 17 00:00:00 2001 From: Stephan Zuercher Date: Wed, 11 Oct 2017 10:39:20 -0700 Subject: [PATCH 3/5] rename MetadataMatches to MetadataCriteria Signed-off-by: Stephan Zuercher --- include/envoy/router/router.h | 20 ++++++++++---------- source/common/http/async_client_impl.h | 2 +- source/common/router/config_impl.h | 4 ++-- test/mocks/router/mocks.h | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 4895aea51aad8..796239a6af5c5 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -229,9 +229,9 @@ class HashPolicy { const Http::HeaderMap& headers) const PURE; }; -class MetadataMatch { +class MetadataMatchCriterion { public: - virtual ~MetadataMatch() {} + virtual ~MetadataMatchCriterion() {} /* * @return const std::string& the name of the metadata key @@ -244,18 +244,18 @@ class MetadataMatch { virtual const HashedValue& value() const PURE; }; -typedef std::shared_ptr MetadataMatchConstSharedPtr; +typedef std::shared_ptr MetadataMatchCriterionConstSharedPtr; -class MetadataMatches { +class MetadataMatchCriteria { public: - virtual ~MetadataMatches() {} + virtual ~MetadataMatchCriteria() {} /* - * @return std::vector& a vector of + * @return std::vector& a vector of * metadata to be matched against upstream endpoints when load * balancing, sorted lexically by name. */ - virtual const std::vector& metadataMatches() const PURE; + virtual const std::vector& metadataMatchCriteria() const PURE; }; /** @@ -340,10 +340,10 @@ class RouteEntry { virtual bool useWebSocket() const PURE; /** - * @return MetadataMatches* the metadata that a subset load balancer should match when selecting - * an upstream host + * @return MetadataMatchCriteria* the metadata that a subset load balancer should match when + * selecting an upstream host */ - virtual const MetadataMatches* metadataMatches() const PURE; + virtual const MetadataMatchCriteria* metadataMatchCriteria() const PURE; /** * @return const std::multimap the opaque configuration associated diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index a7580a518beba..616089fceff57 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -146,7 +146,7 @@ class AsyncStreamImpl : public AsyncClient::Stream, void finalizeRequestHeaders(Http::HeaderMap&, const Http::AccessLog::RequestInfo&) const override {} const Router::HashPolicy* hashPolicy() const override { return nullptr; } - const Router::MetadataMatches* metadataMatches() const override { return nullptr; } + const Router::MetadataMatchCriteria* metadataMatchCriteria() const override { return nullptr; } Upstream::ResourcePriority priority() const override { return Upstream::ResourcePriority::Default; } diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index 45433c6c8f2a1..287504f1acd04 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -255,7 +255,7 @@ class RouteEntryImplBase : public RouteEntry, void finalizeRequestHeaders(Http::HeaderMap& headers, const Http::AccessLog::RequestInfo& request_info) const override; const HashPolicy* hashPolicy() const override { return hash_policy_.get(); } - const MetadataMatches* metadataMatches() const override { return nullptr; } + const MetadataMatchCriteria* metadataMatchCriteria() const override { return nullptr; } Upstream::ResourcePriority priority() const override { return priority_; } const RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; } const RetryPolicy& retryPolicy() const override { return retry_policy_; } @@ -316,7 +316,7 @@ class RouteEntryImplBase : public RouteEntry, const RetryPolicy& retryPolicy() const override { return parent_->retryPolicy(); } const ShadowPolicy& shadowPolicy() const override { return parent_->shadowPolicy(); } std::chrono::milliseconds timeout() const override { return parent_->timeout(); } - const MetadataMatches* metadataMatches() const override { return nullptr; } + const MetadataMatchCriteria* metadataMatchCriteria() const override { return nullptr; } const VirtualCluster* virtualCluster(const Http::HeaderMap& headers) const override { return parent_->virtualCluster(headers); diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 5734d41fde752..43c9a95b98f86 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -184,7 +184,7 @@ class MockRouteEntry : public RouteEntry { void(Http::HeaderMap& headers, const Http::AccessLog::RequestInfo& request_info)); MOCK_CONST_METHOD0(hashPolicy, const HashPolicy*()); - MOCK_CONST_METHOD0(metadataMatches, const Router::MetadataMatches*()); + MOCK_CONST_METHOD0(metadataMatchCriteria, const Router::MetadataMatchCriteria*()); MOCK_CONST_METHOD0(priority, Upstream::ResourcePriority()); MOCK_CONST_METHOD0(rateLimitPolicy, const RateLimitPolicy&()); MOCK_CONST_METHOD0(retryPolicy, const RetryPolicy&()); From efd8db289b96da9a9d5cfbce04c2d2cb790761ce Mon Sep 17 00:00:00 2001 From: Stephan Zuercher Date: Wed, 11 Oct 2017 11:25:17 -0700 Subject: [PATCH 4/5] format Signed-off-by: Stephan Zuercher --- include/envoy/router/router.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 796239a6af5c5..8182735e1bb5a 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -255,7 +255,8 @@ class MetadataMatchCriteria { * metadata to be matched against upstream endpoints when load * balancing, sorted lexically by name. */ - virtual const std::vector& metadataMatchCriteria() const PURE; + virtual const std::vector& + metadataMatchCriteria() const PURE; }; /** From 8b86599ccd2147f106c9e9821484febb6f0df69a Mon Sep 17 00:00:00 2001 From: Stephan Zuercher Date: Wed, 11 Oct 2017 12:08:15 -0700 Subject: [PATCH 5/5] use NOT_REACHED Signed-off-by: Stephan Zuercher --- source/common/protobuf/utility.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc index e0e6960108ca4..fbdefb20a5202 100644 --- a/source/common/protobuf/utility.cc +++ b/source/common/protobuf/utility.cc @@ -128,7 +128,7 @@ bool ValueUtil::equal(const ProtobufWkt::Value& v1, const ProtobufWkt::Value& v2 } default: - RELEASE_ASSERT(false); + NOT_REACHED; } }