From 9ec09447bd6b2227d236fb330577850b606cc7bc Mon Sep 17 00:00:00 2001 From: Zhangdong Ma Date: Sat, 9 Oct 2021 15:52:18 +0800 Subject: [PATCH 01/12] thrift router: Support request subset lb Signed-off-by: Zhangdong Ma --- .../network/thrift_proxy/router/router_impl.h | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/network/thrift_proxy/router/router_impl.h b/source/extensions/filters/network/thrift_proxy/router/router_impl.h index 0b60226f57dce..50e043df9d2b1 100644 --- a/source/extensions/filters/network/thrift_proxy/router/router_impl.h +++ b/source/extensions/filters/network/thrift_proxy/router/router_impl.h @@ -12,6 +12,7 @@ #include "envoy/upstream/load_balancer.h" #include "source/common/http/header_utility.h" +#include "source/common/router/metadatamatchcriteria_impl.h" #include "source/common/upstream/load_balancer_impl.h" #include "source/extensions/filters/network/thrift_proxy/conn_manager.h" #include "source/extensions/filters/network/thrift_proxy/filters/filter.h" @@ -271,7 +272,23 @@ class Router : public Tcp::ConnectionPool::UpstreamCallbacks, // Upstream::LoadBalancerContext const Network::Connection* downstreamConnection() const override; const Envoy::Router::MetadataMatchCriteria* metadataMatchCriteria() override { - return route_entry_ ? route_entry_->metadataMatchCriteria() : nullptr; + const Envoy::Router::MetadataMatchCriteria* route_criteria = + (route_entry_ != nullptr) ? route_entry_->metadataMatchCriteria() : nullptr; + + // Support request subset lb in thrift + const auto& request_metadata = callbacks_->streamInfo().dynamicMetadata().filter_metadata(); + const auto filter_it = request_metadata.find(Envoy::Config::MetadataFilters::get().ENVOY_LB); + + if (filter_it != request_metadata.end() && route_criteria != nullptr) { + metadata_match_criteria_ = route_criteria->mergeMatchCriteria(filter_it->second); + } else if (filter_it != request_metadata.end()) { + metadata_match_criteria_ = + std::make_unique(filter_it->second); + } else { + return route_criteria; + } + + return metadata_match_criteria_.get(); } // Tcp::ConnectionPool::UpstreamCallbacks @@ -287,6 +304,7 @@ class Router : public Tcp::ConnectionPool::UpstreamCallbacks, std::unique_ptr upstream_response_callbacks_{}; RouteConstSharedPtr route_{}; const RouteEntry* route_entry_{}; + Envoy::Router::MetadataMatchCriteriaConstPtr metadata_match_criteria_; std::unique_ptr upstream_request_; Buffer::OwnedImpl upstream_request_buffer_; From abb822d6c864e44e2eaf4d88c3cc6be827a422dd Mon Sep 17 00:00:00 2001 From: Zhangdong Ma Date: Mon, 11 Oct 2021 15:46:17 +0800 Subject: [PATCH 02/12] unit test Signed-off-by: Zhangdong Ma --- .../network/thrift_proxy/router_test.cc | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/test/extensions/filters/network/thrift_proxy/router_test.cc b/test/extensions/filters/network/thrift_proxy/router_test.cc index 22bcf7ca0910e..bdf8a94826ce3 100644 --- a/test/extensions/filters/network/thrift_proxy/router_test.cc +++ b/test/extensions/filters/network/thrift_proxy/router_test.cc @@ -136,6 +136,35 @@ class ThriftRouterTestBase { metadata_->setSequenceId(sequence_id); } + void verifyMetadataMatchCriteriaFromRequest(bool route_entry_has_match) { + ProtobufWkt::Struct request_struct, route_struct; + ProtobufWkt::Value val; + + // Populate metadata like StreamInfo.setDynamicMetadata() would. + auto& fields_map = *request_struct.mutable_fields(); + val.set_string_value("v3.1"); + fields_map["version"] = val; + val.set_string_value("devel"); + fields_map["stage"] = val; + (*callbacks_.stream_info_.metadata_ + .mutable_filter_metadata())[Envoy::Config::MetadataFilters::get().ENVOY_LB] = + request_struct; + + // Populate route entry's metadata which will be overridden. + val.set_string_value("v3.0"); + fields_map = *request_struct.mutable_fields(); + fields_map["version"] = val; + Envoy::Router::MetadataMatchCriteriaImpl route_entry_matches(route_struct); + + if (route_entry_has_match) { + ON_CALL(callbacks_.route_->route_entry_, metadataMatchCriteria()) + .WillByDefault(Return(&route_entry_matches)); + } else { + ON_CALL(callbacks_.route_->route_entry_, metadataMatchCriteria()) + .WillByDefault(Return(nullptr)); + } + } + void startRequest(MessageType msg_type, std::string method = "method", const bool strip_service_name = false, const TransportType transport_type = TransportType::Framed, @@ -697,6 +726,20 @@ TEST_F(ThriftRouterTest, NoCluster) { EXPECT_EQ(1U, context_.scope().counterFromString("test.unknown_cluster").value()); } +TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRequest) { + initializeRouter(); + initializeMetadata(MessageType::Call); + + verifyMetadataMatchCriteriaFromRequest(true); +} + +TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRequestNoRouteEntryMatch) { + initializeRouter(); + initializeMetadata(MessageType::Call); + + verifyMetadataMatchCriteriaFromRequest(false); +} + TEST_F(ThriftRouterTest, ClusterMaintenanceMode) { initializeRouter(); initializeMetadata(MessageType::Call); From 58df464b966992f2e7ec1e65db8742049e7c1139 Mon Sep 17 00:00:00 2001 From: Zhangdong Ma Date: Mon, 11 Oct 2021 15:50:30 +0800 Subject: [PATCH 03/12] format fix Signed-off-by: Zhangdong Ma --- test/extensions/filters/network/thrift_proxy/router_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/extensions/filters/network/thrift_proxy/router_test.cc b/test/extensions/filters/network/thrift_proxy/router_test.cc index bdf8a94826ce3..66a945dc3c8c2 100644 --- a/test/extensions/filters/network/thrift_proxy/router_test.cc +++ b/test/extensions/filters/network/thrift_proxy/router_test.cc @@ -22,7 +22,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -using testing::_; +using ::testing::TestParamInfo; using testing::AtLeast; using testing::Combine; using testing::ContainsRegex; @@ -32,8 +32,8 @@ using testing::NiceMock; using testing::Ref; using testing::Return; using testing::ReturnRef; -using ::testing::TestParamInfo; using testing::Values; +using testing::_; namespace Envoy { namespace Extensions { @@ -1593,7 +1593,7 @@ TEST_F(ThriftRouterTest, ShadowRequests) { shadow_clusters.try_emplace("shadow_cluster_1", std::make_shared()); shadow_clusters.try_emplace("shadow_cluster_2", std::make_shared()); - for (auto& [name, shadow_cluster_info] : shadow_clusters) { + for (auto & [ name, shadow_cluster_info ] : shadow_clusters) { auto& shadow_cluster = shadow_cluster_info->cluster; auto& upstream_connection = shadow_cluster_info->connection; auto& conn_state = shadow_cluster_info->conn_state; From f68bb91e375adb024932cf9fed1b69ce0a6b1c4d Mon Sep 17 00:00:00 2001 From: Zhangdong Ma Date: Mon, 11 Oct 2021 16:28:16 +0800 Subject: [PATCH 04/12] format fix Signed-off-by: Zhangdong Ma --- test/extensions/filters/network/thrift_proxy/router_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/extensions/filters/network/thrift_proxy/router_test.cc b/test/extensions/filters/network/thrift_proxy/router_test.cc index 66a945dc3c8c2..bdf8a94826ce3 100644 --- a/test/extensions/filters/network/thrift_proxy/router_test.cc +++ b/test/extensions/filters/network/thrift_proxy/router_test.cc @@ -22,7 +22,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -using ::testing::TestParamInfo; +using testing::_; using testing::AtLeast; using testing::Combine; using testing::ContainsRegex; @@ -32,8 +32,8 @@ using testing::NiceMock; using testing::Ref; using testing::Return; using testing::ReturnRef; +using ::testing::TestParamInfo; using testing::Values; -using testing::_; namespace Envoy { namespace Extensions { @@ -1593,7 +1593,7 @@ TEST_F(ThriftRouterTest, ShadowRequests) { shadow_clusters.try_emplace("shadow_cluster_1", std::make_shared()); shadow_clusters.try_emplace("shadow_cluster_2", std::make_shared()); - for (auto & [ name, shadow_cluster_info ] : shadow_clusters) { + for (auto& [name, shadow_cluster_info] : shadow_clusters) { auto& shadow_cluster = shadow_cluster_info->cluster; auto& upstream_connection = shadow_cluster_info->connection; auto& conn_state = shadow_cluster_info->conn_state; From 2aa1ed42bda520cd2f67f1f2863c8adf1f75564c Mon Sep 17 00:00:00 2001 From: Zhangdong Ma Date: Tue, 12 Oct 2021 14:25:32 +0800 Subject: [PATCH 05/12] unit test Signed-off-by: Zhangdong Ma --- .../filters/network/thrift_proxy/router_test.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/extensions/filters/network/thrift_proxy/router_test.cc b/test/extensions/filters/network/thrift_proxy/router_test.cc index bdf8a94826ce3..631047fbed4ef 100644 --- a/test/extensions/filters/network/thrift_proxy/router_test.cc +++ b/test/extensions/filters/network/thrift_proxy/router_test.cc @@ -163,6 +163,22 @@ class ThriftRouterTestBase { ON_CALL(callbacks_.route_->route_entry_, metadataMatchCriteria()) .WillByDefault(Return(nullptr)); } + + auto match = router_->metadataMatchCriteria()->metadataMatchCriteria(); + EXPECT_EQ(match.size(), 2); + auto it = match.begin(); + + // Note: metadataMatchCriteria() keeps its entries sorted, so the order for checks + // below matters. + + // `stage` was only set by the request, not by the route entry. + EXPECT_EQ((*it)->name(), "stage"); + EXPECT_EQ((*it)->value().value().string_value(), "devel"); + it++; + + // `version` should be what came from the request, overriding the route entry. + EXPECT_EQ((*it)->name(), "version"); + EXPECT_EQ((*it)->value().value().string_value(), "v3.1"); } void startRequest(MessageType msg_type, std::string method = "method", From 3ea7cccf64b9358dc822bab28ca0fa89ccf85230 Mon Sep 17 00:00:00 2001 From: Zhangdong Ma Date: Tue, 12 Oct 2021 16:22:49 +0800 Subject: [PATCH 06/12] unit test Signed-off-by: Zhangdong Ma --- .../network/thrift_proxy/router/router_impl.h | 12 ++-- .../network/thrift_proxy/router_test.cc | 72 +++++++++++++++++-- 2 files changed, 75 insertions(+), 9 deletions(-) diff --git a/source/extensions/filters/network/thrift_proxy/router/router_impl.h b/source/extensions/filters/network/thrift_proxy/router/router_impl.h index 50e043df9d2b1..857c92b4ef44c 100644 --- a/source/extensions/filters/network/thrift_proxy/router/router_impl.h +++ b/source/extensions/filters/network/thrift_proxy/router/router_impl.h @@ -279,11 +279,13 @@ class Router : public Tcp::ConnectionPool::UpstreamCallbacks, const auto& request_metadata = callbacks_->streamInfo().dynamicMetadata().filter_metadata(); const auto filter_it = request_metadata.find(Envoy::Config::MetadataFilters::get().ENVOY_LB); - if (filter_it != request_metadata.end() && route_criteria != nullptr) { - metadata_match_criteria_ = route_criteria->mergeMatchCriteria(filter_it->second); - } else if (filter_it != request_metadata.end()) { - metadata_match_criteria_ = - std::make_unique(filter_it->second); + if (filter_it != request_metadata.end()) { + if (route_criteria != nullptr) { + metadata_match_criteria_ = route_criteria->mergeMatchCriteria(filter_it->second); + } else { + metadata_match_criteria_ = + std::make_unique(filter_it->second); + } } else { return route_criteria; } diff --git a/test/extensions/filters/network/thrift_proxy/router_test.cc b/test/extensions/filters/network/thrift_proxy/router_test.cc index 631047fbed4ef..47a215b14f5ea 100644 --- a/test/extensions/filters/network/thrift_proxy/router_test.cc +++ b/test/extensions/filters/network/thrift_proxy/router_test.cc @@ -137,7 +137,7 @@ class ThriftRouterTestBase { } void verifyMetadataMatchCriteriaFromRequest(bool route_entry_has_match) { - ProtobufWkt::Struct request_struct, route_struct; + ProtobufWkt::Struct request_struct; ProtobufWkt::Value val; // Populate metadata like StreamInfo.setDynamicMetadata() would. @@ -154,13 +154,13 @@ class ThriftRouterTestBase { val.set_string_value("v3.0"); fields_map = *request_struct.mutable_fields(); fields_map["version"] = val; - Envoy::Router::MetadataMatchCriteriaImpl route_entry_matches(route_struct); + Envoy::Router::MetadataMatchCriteriaImpl route_entry_matches(request_struct); if (route_entry_has_match) { - ON_CALL(callbacks_.route_->route_entry_, metadataMatchCriteria()) + ON_CALL(route_entry_, metadataMatchCriteria()) .WillByDefault(Return(&route_entry_matches)); } else { - ON_CALL(callbacks_.route_->route_entry_, metadataMatchCriteria()) + ON_CALL(route_entry_, metadataMatchCriteria()) .WillByDefault(Return(nullptr)); } @@ -181,6 +181,48 @@ class ThriftRouterTestBase { EXPECT_EQ((*it)->value().value().string_value(), "v3.1"); } + void verifyMetadataMatchCriteriaFromRoute(bool route_entry_has_match) { + ProtobufWkt::Struct route_struct; + ProtobufWkt::Value val; + + // Populate metadata like StreamInfo.setDynamicMetadata() would. + auto& fields_map = *route_struct.mutable_fields(); + val.set_string_value("v3.1"); + fields_map["version"] = val; + val.set_string_value("devel"); + fields_map["stage"] = val; + + Envoy::Router::MetadataMatchCriteriaImpl route_entry_matches(route_struct); + + if (route_entry_has_match) { + ON_CALL(route_entry_, metadataMatchCriteria()) + .WillByDefault(Return(&route_entry_matches)); + + EXPECT_NE(nullptr, router_->metadataMatchCriteria()); + + auto match = router_->metadataMatchCriteria()->metadataMatchCriteria(); + EXPECT_EQ(match.size(), 2); + auto it = match.begin(); + + // Note: metadataMatchCriteria() keeps its entries sorted, so the order for checks + // below matters. + + // `stage` was set by the route entry. + EXPECT_EQ((*it)->name(), "stage"); + EXPECT_EQ((*it)->value().value().string_value(), "devel"); + it++; + + // `version` was set by the route entry. + EXPECT_EQ((*it)->name(), "version"); + EXPECT_EQ((*it)->value().value().string_value(), "v3.1"); + } else { + ON_CALL(callbacks_.route_->route_entry_, metadataMatchCriteria()) + .WillByDefault(Return(nullptr)); + + EXPECT_EQ(nullptr, router_->metadataMatchCriteria()); + } + } + void startRequest(MessageType msg_type, std::string method = "method", const bool strip_service_name = false, const TransportType transport_type = TransportType::Framed, @@ -742,20 +784,42 @@ TEST_F(ThriftRouterTest, NoCluster) { EXPECT_EQ(1U, context_.scope().counterFromString("test.unknown_cluster").value()); } +// Test for request metadata is not empty TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRequest) { initializeRouter(); initializeMetadata(MessageType::Call); + // Test case for router filter metadata is not empty verifyMetadataMatchCriteriaFromRequest(true); } +// Test for request metadata is not empty TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRequestNoRouteEntryMatch) { initializeRouter(); initializeMetadata(MessageType::Call); + // Test case for router filter metadata is empty verifyMetadataMatchCriteriaFromRequest(false); } +// Test for request metadata is empty +TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRoute) { + initializeRouter(); + startRequest(MessageType::Call); + + // Test case for router filter metadata is not empty + verifyMetadataMatchCriteriaFromRoute(true); +} + +// Test for request metadata is empty +TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRouteNoRouteEntryMatch) { + initializeRouter(); + startRequest(MessageType::Call); + + // Test case for router filter metadata is empty + verifyMetadataMatchCriteriaFromRoute(false); +} + TEST_F(ThriftRouterTest, ClusterMaintenanceMode) { initializeRouter(); initializeMetadata(MessageType::Call); From 4342443b62048b157005dcd4762aa9a9892379ba Mon Sep 17 00:00:00 2001 From: Zhangdong Ma Date: Tue, 12 Oct 2021 16:25:45 +0800 Subject: [PATCH 07/12] format fix Signed-off-by: Zhangdong Ma --- .../filters/network/thrift_proxy/router_test.cc | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/test/extensions/filters/network/thrift_proxy/router_test.cc b/test/extensions/filters/network/thrift_proxy/router_test.cc index 47a215b14f5ea..5a13292d3d805 100644 --- a/test/extensions/filters/network/thrift_proxy/router_test.cc +++ b/test/extensions/filters/network/thrift_proxy/router_test.cc @@ -157,11 +157,9 @@ class ThriftRouterTestBase { Envoy::Router::MetadataMatchCriteriaImpl route_entry_matches(request_struct); if (route_entry_has_match) { - ON_CALL(route_entry_, metadataMatchCriteria()) - .WillByDefault(Return(&route_entry_matches)); + ON_CALL(route_entry_, metadataMatchCriteria()).WillByDefault(Return(&route_entry_matches)); } else { - ON_CALL(route_entry_, metadataMatchCriteria()) - .WillByDefault(Return(nullptr)); + ON_CALL(route_entry_, metadataMatchCriteria()).WillByDefault(Return(nullptr)); } auto match = router_->metadataMatchCriteria()->metadataMatchCriteria(); @@ -195,11 +193,9 @@ class ThriftRouterTestBase { Envoy::Router::MetadataMatchCriteriaImpl route_entry_matches(route_struct); if (route_entry_has_match) { - ON_CALL(route_entry_, metadataMatchCriteria()) - .WillByDefault(Return(&route_entry_matches)); + ON_CALL(route_entry_, metadataMatchCriteria()).WillByDefault(Return(&route_entry_matches)); EXPECT_NE(nullptr, router_->metadataMatchCriteria()); - auto match = router_->metadataMatchCriteria()->metadataMatchCriteria(); EXPECT_EQ(match.size(), 2); auto it = match.begin(); From e4bf7a3c780e6d5da933d77b866de9e75977592c Mon Sep 17 00:00:00 2001 From: Zhangdong Ma Date: Wed, 13 Oct 2021 14:44:02 +0800 Subject: [PATCH 08/12] update some comments Signed-off-by: Zhangdong Ma --- .../network/thrift_proxy/router/router_impl.h | 18 ++++++------- .../network/thrift_proxy/router_test.cc | 25 +++++++++++-------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/source/extensions/filters/network/thrift_proxy/router/router_impl.h b/source/extensions/filters/network/thrift_proxy/router/router_impl.h index 9af3095860afa..1b518195319bd 100644 --- a/source/extensions/filters/network/thrift_proxy/router/router_impl.h +++ b/source/extensions/filters/network/thrift_proxy/router/router_impl.h @@ -278,21 +278,21 @@ class Router : public Tcp::ConnectionPool::UpstreamCallbacks, const Envoy::Router::MetadataMatchCriteria* route_criteria = (route_entry_ != nullptr) ? route_entry_->metadataMatchCriteria() : nullptr; - // Support request subset lb in thrift + // Support getting metadata match criteria from thrift request. const auto& request_metadata = callbacks_->streamInfo().dynamicMetadata().filter_metadata(); const auto filter_it = request_metadata.find(Envoy::Config::MetadataFilters::get().ENVOY_LB); - if (filter_it != request_metadata.end()) { - if (route_criteria != nullptr) { - metadata_match_criteria_ = route_criteria->mergeMatchCriteria(filter_it->second); - } else { - metadata_match_criteria_ = - std::make_unique(filter_it->second); - } - } else { + if (filter_it == request_metadata.end()) { return route_criteria; } + if (route_criteria != nullptr) { + metadata_match_criteria_ = route_criteria->mergeMatchCriteria(filter_it->second); + } else { + metadata_match_criteria_ = + std::make_unique(filter_it->second); + } + return metadata_match_criteria_.get(); } diff --git a/test/extensions/filters/network/thrift_proxy/router_test.cc b/test/extensions/filters/network/thrift_proxy/router_test.cc index d940df7b8baab..998903ee33884 100644 --- a/test/extensions/filters/network/thrift_proxy/router_test.cc +++ b/test/extensions/filters/network/thrift_proxy/router_test.cc @@ -149,6 +149,8 @@ class ThriftRouterTestBase { fields_map["version"] = val; val.set_string_value("devel"); fields_map["stage"] = val; + val.set_string_value("1"); + fields_map["xkey_in_request"] = val; (*callbacks_.stream_info_.metadata_ .mutable_filter_metadata())[Envoy::Config::MetadataFilters::get().ENVOY_LB] = request_struct; @@ -157,6 +159,7 @@ class ThriftRouterTestBase { val.set_string_value("v3.0"); fields_map = *request_struct.mutable_fields(); fields_map["version"] = val; + fields_map.erase("xkey_in_request"); Envoy::Router::MetadataMatchCriteriaImpl route_entry_matches(request_struct); if (route_entry_has_match) { @@ -166,7 +169,7 @@ class ThriftRouterTestBase { } auto match = router_->metadataMatchCriteria()->metadataMatchCriteria(); - EXPECT_EQ(match.size(), 2); + EXPECT_EQ(match.size(), 3); auto it = match.begin(); // Note: metadataMatchCriteria() keeps its entries sorted, so the order for checks @@ -177,16 +180,20 @@ class ThriftRouterTestBase { EXPECT_EQ((*it)->value().value().string_value(), "devel"); it++; - // `version` should be what came from the request, overriding the route entry. + // `version` should be what came from the request and override the route entry's. EXPECT_EQ((*it)->name(), "version"); EXPECT_EQ((*it)->value().value().string_value(), "v3.1"); + it++; + + // `xkey_in_request` was only set by the request + EXPECT_EQ((*it)->name(), "xkey_in_request"); + EXPECT_EQ((*it)->value().value().string_value(), "1"); } void verifyMetadataMatchCriteriaFromRoute(bool route_entry_has_match) { ProtobufWkt::Struct route_struct; ProtobufWkt::Value val; - // Populate metadata like StreamInfo.setDynamicMetadata() would. auto& fields_map = *route_struct.mutable_fields(); val.set_string_value("v3.1"); fields_map["version"] = val; @@ -789,39 +796,35 @@ TEST_F(ThriftRouterTest, NoCluster) { EXPECT_EQ(1U, context_.scope().counterFromString("test.unknown_cluster").value()); } -// Test for request metadata is not empty +// Test the case where both dynamic metadata match criteria and route metadata match criteria is not empty. TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRequest) { initializeRouter(); initializeMetadata(MessageType::Call); - // Test case for router filter metadata is not empty verifyMetadataMatchCriteriaFromRequest(true); } -// Test for request metadata is not empty +// Test the case where route metadata match criteria is empty but with non-empty dynamic metadata match criteria. TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRequestNoRouteEntryMatch) { initializeRouter(); initializeMetadata(MessageType::Call); - // Test case for router filter metadata is empty verifyMetadataMatchCriteriaFromRequest(false); } -// Test for request metadata is empty +// Test the case where dynamic metadata match criteria is empty but with non-empty route metadata match criteria. TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRoute) { initializeRouter(); startRequest(MessageType::Call); - // Test case for router filter metadata is not empty verifyMetadataMatchCriteriaFromRoute(true); } -// Test for request metadata is empty +// Test the case where both dynamic metadata match criteria and route metadata match criteria is empty. TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRouteNoRouteEntryMatch) { initializeRouter(); startRequest(MessageType::Call); - // Test case for router filter metadata is empty verifyMetadataMatchCriteriaFromRoute(false); } From 98f5eaba2854595cf7ead6c97a995e6d1341d624 Mon Sep 17 00:00:00 2001 From: Zhangdong Ma Date: Wed, 13 Oct 2021 17:24:35 +0800 Subject: [PATCH 09/12] format fix Signed-off-by: Zhangdong Ma --- .../filters/network/thrift_proxy/router_test.cc | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/extensions/filters/network/thrift_proxy/router_test.cc b/test/extensions/filters/network/thrift_proxy/router_test.cc index 998903ee33884..097e3985e03a6 100644 --- a/test/extensions/filters/network/thrift_proxy/router_test.cc +++ b/test/extensions/filters/network/thrift_proxy/router_test.cc @@ -796,7 +796,8 @@ TEST_F(ThriftRouterTest, NoCluster) { EXPECT_EQ(1U, context_.scope().counterFromString("test.unknown_cluster").value()); } -// Test the case where both dynamic metadata match criteria and route metadata match criteria is not empty. +// Test the case where both dynamic metadata match criteria +// and route metadata match criteria is not empty. TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRequest) { initializeRouter(); initializeMetadata(MessageType::Call); @@ -804,7 +805,8 @@ TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRequest) { verifyMetadataMatchCriteriaFromRequest(true); } -// Test the case where route metadata match criteria is empty but with non-empty dynamic metadata match criteria. +// Test the case where route metadata match criteria is empty +// but with non-empty dynamic metadata match criteria. TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRequestNoRouteEntryMatch) { initializeRouter(); initializeMetadata(MessageType::Call); @@ -812,7 +814,8 @@ TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRequestNoRouteEntryMatch) { verifyMetadataMatchCriteriaFromRequest(false); } -// Test the case where dynamic metadata match criteria is empty but with non-empty route metadata match criteria. +// Test the case where dynamic metadata match criteria is empty +// but with non-empty route metadata match criteria. TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRoute) { initializeRouter(); startRequest(MessageType::Call); @@ -820,7 +823,8 @@ TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRoute) { verifyMetadataMatchCriteriaFromRoute(true); } -// Test the case where both dynamic metadata match criteria and route metadata match criteria is empty. +// Test the case where both dynamic metadata match criteria +// and route metadata match criteria is empty. TEST_F(ThriftRouterTest, MetadataMatchCriteriaFromRouteNoRouteEntryMatch) { initializeRouter(); startRequest(MessageType::Call); From 99a1ce91968d8145b3cc67ca196b56b262c0f199 Mon Sep 17 00:00:00 2001 From: Zhangdong Ma Date: Thu, 14 Oct 2021 11:07:56 +0800 Subject: [PATCH 10/12] update docs Signed-off-by: Zhangdong Ma --- docs/root/version_history/current.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index ea8d80cd4b605..a2708d83fcf0c 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -36,6 +36,7 @@ New Features * ext_authz: added :ref:`query_parameters_to_set ` and :ref:`query_parameters_to_remove ` for adding and removing query string parameters when using a gRPC authorization server. * http: added support for :ref:`retriable health check status codes `. * thrift_proxy: add upstream response zone metrics in the form ``cluster.cluster_name.zone.local_zone.upstream_zone.thrift.upstream_resp_success``. +* thrift_proxy: support subset lb for request metadata filter Deprecated ---------- From 3ec0de935667f636a15936f037036910110060b7 Mon Sep 17 00:00:00 2001 From: Zhangdong Ma Date: Thu, 14 Oct 2021 15:12:22 +0800 Subject: [PATCH 11/12] update docs Signed-off-by: Zhangdong Ma --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 23427bc85a60f..e15b0c36ac141 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -40,7 +40,7 @@ New Features * ext_authz: added :ref:`query_parameters_to_set ` and :ref:`query_parameters_to_remove ` for adding and removing query string parameters when using a gRPC authorization server. * http: added support for :ref:`retriable health check status codes `. * thrift_proxy: add upstream response zone metrics in the form ``cluster.cluster_name.zone.local_zone.upstream_zone.thrift.upstream_resp_success``. -* thrift_proxy: support subset lb for request metadata filter +* thrift_proxy: support subset lb when using request or route metadata. * vcl_socket_interface: added VCL socket interface extension for fd.io VPP integration to :ref:`contrib images `. This can be enabled via :ref:`VCL ` configuration. Deprecated From 8b0af2da0ae0e250a2da0ed842815d7911d01548 Mon Sep 17 00:00:00 2001 From: Zhangdong Ma Date: Thu, 14 Oct 2021 15:18:20 +0800 Subject: [PATCH 12/12] update docs Signed-off-by: Zhangdong Ma --- docs/root/version_history/current.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 50d4b4045f598..e15b0c36ac141 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -42,7 +42,6 @@ New Features * thrift_proxy: add upstream response zone metrics in the form ``cluster.cluster_name.zone.local_zone.upstream_zone.thrift.upstream_resp_success``. * thrift_proxy: support subset lb when using request or route metadata. * vcl_socket_interface: added VCL socket interface extension for fd.io VPP integration to :ref:`contrib images `. This can be enabled via :ref:`VCL ` configuration. -* thrift_proxy: support subset lb for request metadata filter. Deprecated ----------