From d7565c37a891a22d8f6631c187e053d552d05730 Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Fri, 17 Jul 2020 15:46:48 -0700 Subject: [PATCH 1/3] Revert "Adding Fake headers for header matching." This reverts commit 8c013bfcdc725565bed0e8bdd7b3c37ae0731dc5. --- BUILD | 16 -- BUILD.gn | 2 - CMakeLists.txt | 2 - Makefile | 2 - build_autogenerated.yaml | 4 - config.m4 | 1 - config.w32 | 1 - gRPC-C++.podspec | 2 - gRPC-Core.podspec | 3 - grpc.gemspec | 2 - grpc.gyp | 2 - package.xml | 2 - .../lb_policy/xds/xds_routing.cc | 58 +---- .../filters/http/client/http_client_filter.cc | 33 ++- src/core/ext/filters/http/client/util.cc | 53 ---- src/core/ext/filters/http/client/util.h | 30 --- src/python/grpcio/grpc_core_dependencies.py | 1 - test/cpp/end2end/xds_end2end_test.cc | 239 ------------------ tools/doxygen/Doxyfile.c++.internal | 2 - tools/doxygen/Doxyfile.core.internal | 2 - 20 files changed, 42 insertions(+), 415 deletions(-) delete mode 100644 src/core/ext/filters/http/client/util.cc delete mode 100644 src/core/ext/filters/http/client/util.h diff --git a/BUILD b/BUILD index f1f0806a186..c6438a08032 100644 --- a/BUILD +++ b/BUILD @@ -1163,20 +1163,6 @@ grpc_cc_library( ], ) -grpc_cc_library( - name = "grpc_http_util", - srcs = [ - "src/core/ext/filters/http/client/util.cc", - ], - hdrs = [ - "src/core/ext/filters/http/client/util.h", - ], - language = "c++", - deps = [ - "grpc_base", - ], -) - grpc_cc_library( name = "grpc_http_filters", srcs = [ @@ -1195,7 +1181,6 @@ grpc_cc_library( language = "c++", deps = [ "grpc_base", - "grpc_http_util", "grpc_message_size_filter", ], ) @@ -1471,7 +1456,6 @@ grpc_cc_library( deps = [ "grpc_base", "grpc_client_channel", - "grpc_http_util", "grpc_resolver_xds_header", "grpc_xds_api_header", ], diff --git a/BUILD.gn b/BUILD.gn index a8f36b73b25..8defd9122d4 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -320,8 +320,6 @@ config("grpc_config") { "src/core/ext/filters/deadline/deadline_filter.h", "src/core/ext/filters/http/client/http_client_filter.cc", "src/core/ext/filters/http/client/http_client_filter.h", - "src/core/ext/filters/http/client/util.cc", - "src/core/ext/filters/http/client/util.h", "src/core/ext/filters/http/client_authority_filter.cc", "src/core/ext/filters/http/client_authority_filter.h", "src/core/ext/filters/http/http_filters_plugin.cc", diff --git a/CMakeLists.txt b/CMakeLists.txt index b99c4897474..d5f54cb442d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1396,7 +1396,6 @@ add_library(grpc src/core/ext/filters/client_idle/client_idle_filter.cc src/core/ext/filters/deadline/deadline_filter.cc src/core/ext/filters/http/client/http_client_filter.cc - src/core/ext/filters/http/client/util.cc src/core/ext/filters/http/client_authority_filter.cc src/core/ext/filters/http/http_filters_plugin.cc src/core/ext/filters/http/message_compress/message_compress_filter.cc @@ -2075,7 +2074,6 @@ add_library(grpc_unsecure src/core/ext/filters/client_idle/client_idle_filter.cc src/core/ext/filters/deadline/deadline_filter.cc src/core/ext/filters/http/client/http_client_filter.cc - src/core/ext/filters/http/client/util.cc src/core/ext/filters/http/client_authority_filter.cc src/core/ext/filters/http/http_filters_plugin.cc src/core/ext/filters/http/message_compress/message_compress_filter.cc diff --git a/Makefile b/Makefile index dba3fc8637c..f2da71bf980 100644 --- a/Makefile +++ b/Makefile @@ -3462,7 +3462,6 @@ LIBGRPC_SRC = \ src/core/ext/filters/client_idle/client_idle_filter.cc \ src/core/ext/filters/deadline/deadline_filter.cc \ src/core/ext/filters/http/client/http_client_filter.cc \ - src/core/ext/filters/http/client/util.cc \ src/core/ext/filters/http/client_authority_filter.cc \ src/core/ext/filters/http/http_filters_plugin.cc \ src/core/ext/filters/http/message_compress/message_compress_filter.cc \ @@ -4110,7 +4109,6 @@ LIBGRPC_UNSECURE_SRC = \ src/core/ext/filters/client_idle/client_idle_filter.cc \ src/core/ext/filters/deadline/deadline_filter.cc \ src/core/ext/filters/http/client/http_client_filter.cc \ - src/core/ext/filters/http/client/util.cc \ src/core/ext/filters/http/client_authority_filter.cc \ src/core/ext/filters/http/http_filters_plugin.cc \ src/core/ext/filters/http/message_compress/message_compress_filter.cc \ diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index e808c7f9c51..7d96fb61766 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -426,7 +426,6 @@ libs: - src/core/ext/filters/client_channel/xds/xds_client_stats.h - src/core/ext/filters/deadline/deadline_filter.h - src/core/ext/filters/http/client/http_client_filter.h - - src/core/ext/filters/http/client/util.h - src/core/ext/filters/http/client_authority_filter.h - src/core/ext/filters/http/message_compress/message_compress_filter.h - src/core/ext/filters/http/message_compress/message_decompress_filter.h @@ -809,7 +808,6 @@ libs: - src/core/ext/filters/client_idle/client_idle_filter.cc - src/core/ext/filters/deadline/deadline_filter.cc - src/core/ext/filters/http/client/http_client_filter.cc - - src/core/ext/filters/http/client/util.cc - src/core/ext/filters/http/client_authority_filter.cc - src/core/ext/filters/http/http_filters_plugin.cc - src/core/ext/filters/http/message_compress/message_compress_filter.cc @@ -1357,7 +1355,6 @@ libs: - src/core/ext/filters/client_channel/xds/xds_client_stats.h - src/core/ext/filters/deadline/deadline_filter.h - src/core/ext/filters/http/client/http_client_filter.h - - src/core/ext/filters/http/client/util.h - src/core/ext/filters/http/client_authority_filter.h - src/core/ext/filters/http/message_compress/message_compress_filter.h - src/core/ext/filters/http/message_compress/message_decompress_filter.h @@ -1676,7 +1673,6 @@ libs: - src/core/ext/filters/client_idle/client_idle_filter.cc - src/core/ext/filters/deadline/deadline_filter.cc - src/core/ext/filters/http/client/http_client_filter.cc - - src/core/ext/filters/http/client/util.cc - src/core/ext/filters/http/client_authority_filter.cc - src/core/ext/filters/http/http_filters_plugin.cc - src/core/ext/filters/http/message_compress/message_compress_filter.cc diff --git a/config.m4 b/config.m4 index 4c7c2079ce5..8cf43fc8a52 100644 --- a/config.m4 +++ b/config.m4 @@ -106,7 +106,6 @@ if test "$PHP_GRPC" != "no"; then src/core/ext/filters/client_idle/client_idle_filter.cc \ src/core/ext/filters/deadline/deadline_filter.cc \ src/core/ext/filters/http/client/http_client_filter.cc \ - src/core/ext/filters/http/client/util.cc \ src/core/ext/filters/http/client_authority_filter.cc \ src/core/ext/filters/http/http_filters_plugin.cc \ src/core/ext/filters/http/message_compress/message_compress_filter.cc \ diff --git a/config.w32 b/config.w32 index c75527efaec..8f027adf682 100644 --- a/config.w32 +++ b/config.w32 @@ -74,7 +74,6 @@ if (PHP_GRPC != "no") { "src\\core\\ext\\filters\\client_idle\\client_idle_filter.cc " + "src\\core\\ext\\filters\\deadline\\deadline_filter.cc " + "src\\core\\ext\\filters\\http\\client\\http_client_filter.cc " + - "src\\core\\ext\\filters\\http\\client\\util.cc " + "src\\core\\ext\\filters\\http\\client_authority_filter.cc " + "src\\core\\ext\\filters\\http\\http_filters_plugin.cc " + "src\\core\\ext\\filters\\http\\message_compress\\message_compress_filter.cc " + diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index 65078e06dd2..2f06645b1eb 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -269,7 +269,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/xds/xds_client_stats.h', 'src/core/ext/filters/deadline/deadline_filter.h', 'src/core/ext/filters/http/client/http_client_filter.h', - 'src/core/ext/filters/http/client/util.h', 'src/core/ext/filters/http/client_authority_filter.h', 'src/core/ext/filters/http/message_compress/message_compress_filter.h', 'src/core/ext/filters/http/message_compress/message_decompress_filter.h', @@ -758,7 +757,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/xds/xds_client_stats.h', 'src/core/ext/filters/deadline/deadline_filter.h', 'src/core/ext/filters/http/client/http_client_filter.h', - 'src/core/ext/filters/http/client/util.h', 'src/core/ext/filters/http/client_authority_filter.h', 'src/core/ext/filters/http/message_compress/message_compress_filter.h', 'src/core/ext/filters/http/message_compress/message_decompress_filter.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 01f95dfca2b..515fa312684 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -304,8 +304,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/deadline/deadline_filter.h', 'src/core/ext/filters/http/client/http_client_filter.cc', 'src/core/ext/filters/http/client/http_client_filter.h', - 'src/core/ext/filters/http/client/util.cc', - 'src/core/ext/filters/http/client/util.h', 'src/core/ext/filters/http/client_authority_filter.cc', 'src/core/ext/filters/http/client_authority_filter.h', 'src/core/ext/filters/http/http_filters_plugin.cc', @@ -1155,7 +1153,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/xds/xds_client_stats.h', 'src/core/ext/filters/deadline/deadline_filter.h', 'src/core/ext/filters/http/client/http_client_filter.h', - 'src/core/ext/filters/http/client/util.h', 'src/core/ext/filters/http/client_authority_filter.h', 'src/core/ext/filters/http/message_compress/message_compress_filter.h', 'src/core/ext/filters/http/message_compress/message_decompress_filter.h', diff --git a/grpc.gemspec b/grpc.gemspec index 3c75f5e1e7b..81ea3643abd 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -226,8 +226,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/ext/filters/deadline/deadline_filter.h ) s.files += %w( src/core/ext/filters/http/client/http_client_filter.cc ) s.files += %w( src/core/ext/filters/http/client/http_client_filter.h ) - s.files += %w( src/core/ext/filters/http/client/util.cc ) - s.files += %w( src/core/ext/filters/http/client/util.h ) s.files += %w( src/core/ext/filters/http/client_authority_filter.cc ) s.files += %w( src/core/ext/filters/http/client_authority_filter.h ) s.files += %w( src/core/ext/filters/http/http_filters_plugin.cc ) diff --git a/grpc.gyp b/grpc.gyp index f43b07da560..a7cae5e3f8b 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -503,7 +503,6 @@ 'src/core/ext/filters/client_idle/client_idle_filter.cc', 'src/core/ext/filters/deadline/deadline_filter.cc', 'src/core/ext/filters/http/client/http_client_filter.cc', - 'src/core/ext/filters/http/client/util.cc', 'src/core/ext/filters/http/client_authority_filter.cc', 'src/core/ext/filters/http/http_filters_plugin.cc', 'src/core/ext/filters/http/message_compress/message_compress_filter.cc', @@ -1013,7 +1012,6 @@ 'src/core/ext/filters/client_idle/client_idle_filter.cc', 'src/core/ext/filters/deadline/deadline_filter.cc', 'src/core/ext/filters/http/client/http_client_filter.cc', - 'src/core/ext/filters/http/client/util.cc', 'src/core/ext/filters/http/client_authority_filter.cc', 'src/core/ext/filters/http/http_filters_plugin.cc', 'src/core/ext/filters/http/message_compress/message_compress_filter.cc', diff --git a/package.xml b/package.xml index 331fd983ae9..bd886a4f58e 100644 --- a/package.xml +++ b/package.xml @@ -206,8 +206,6 @@ - - diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc index 19cfd814df9..0a66c35edf8 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc @@ -37,7 +37,6 @@ #include "src/core/ext/filters/client_channel/lb_policy_registry.h" #include "src/core/ext/filters/client_channel/resolver/xds/xds_resolver.h" #include "src/core/ext/filters/client_channel/xds/xds_api.h" -#include "src/core/ext/filters/http/client/util.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/orphanable.h" @@ -120,18 +119,14 @@ class XdsRoutingLb : public LoadBalancingPolicy { // Maintains an ordered xds route table as provided by RDS response. using RouteTable = std::vector; - RoutePicker(RouteTable route_table, std::string user_agent, - RefCountedPtr config) - : route_table_(std::move(route_table)), - user_agent_(std::move(user_agent)), - config_(std::move(config)) {} + explicit RoutePicker(RouteTable route_table, + RefCountedPtr config) + : route_table_(std::move(route_table)), config_(std::move(config)) {} PickResult Pick(PickArgs args) override; private: RouteTable route_table_; - // Storing user_agent generated from args from http layer. - std::string user_agent_; // Take a reference to config so that we can use // XdsApi::RdsUpdate::RdsRoute::Matchers from it. RefCountedPtr config_; @@ -220,9 +215,6 @@ class XdsRoutingLb : public LoadBalancingPolicy { // Children. std::map> actions_; - - // Storing user_agent generated from args from http layer. - std::string user_agent_; }; // @@ -270,24 +262,8 @@ absl::optional GetMetadataValue( bool HeaderMatchHelper( const XdsApi::RdsUpdate::RdsRoute::Matchers::HeaderMatcher& header_matcher, - LoadBalancingPolicy::MetadataInterface* initial_metadata, - const std::string& user_agent, absl::string_view deadline) { - std::string concatenated_value; - absl::optional value; - if (header_matcher.name == "grpc-tags-bin" || - header_matcher.name == "grpc-trace-bin" || - header_matcher.name == "grpc-previous-rpc-attempts") { - value = absl::nullopt; - } else if (header_matcher.name == "content-type") { - value = "application/grpc"; - } else if (header_matcher.name == "user-agent") { - value = user_agent; - } else if (header_matcher.name == "grpc-timeout") { - value = deadline; - } else { - value = GetMetadataValue(header_matcher.name, initial_metadata, - &concatenated_value); - } + LoadBalancingPolicy::MetadataInterface* initial_metadata) { + auto value = GetMetadataValue(header_matcher.name, initial_metadata); if (!value.has_value()) { if (header_matcher.type == XdsApi::RdsUpdate::RdsRoute::Matchers:: HeaderMatcher::HeaderMatcherType::PRESENT) { @@ -325,13 +301,11 @@ bool HeaderMatchHelper( } bool HeadersMatch( + LoadBalancingPolicy::PickArgs args, const std::vector& - header_matchers, - LoadBalancingPolicy::MetadataInterface* initial_metadata, - const std::string& user_agent, absl::string_view deadline) { + header_matchers) { for (const auto& header_matcher : header_matchers) { - bool match = HeaderMatchHelper(header_matcher, initial_metadata, user_agent, - deadline); + bool match = HeaderMatchHelper(header_matcher, args.initial_metadata); if (header_matcher.invert_match) match = !match; if (!match) return false; } @@ -349,16 +323,11 @@ XdsRoutingLb::PickResult XdsRoutingLb::RoutePicker::Pick(PickArgs args) { // Path matching. if (!PathMatch(args.path, route.matchers->path_matcher)) continue; // Header Matching. - if (!HeadersMatch(route.matchers->header_matchers, args.initial_metadata, - user_agent_, - args.call_state->ExperimentalGetCallAttribute( - kCallAttributeDeadline))) - continue; + if (!HeadersMatch(args, route.matchers->header_matchers)) continue; // Match fraction check if (route.matchers->fraction_per_million.has_value() && - !UnderFraction(route.matchers->fraction_per_million.value())) { + !UnderFraction(route.matchers->fraction_per_million.value())) continue; - } // Found a match return route.picker->Pick(args); } @@ -375,9 +344,7 @@ XdsRoutingLb::PickResult XdsRoutingLb::RoutePicker::Pick(PickArgs args) { // XdsRoutingLb // -XdsRoutingLb::XdsRoutingLb(Args args) - : LoadBalancingPolicy(std::move(args)), - user_agent_(GenerateUserAgentFromArgs(args.args, "chttp2")) {} +XdsRoutingLb::XdsRoutingLb(Args args) : LoadBalancingPolicy(std::move(args)) {} XdsRoutingLb::~XdsRoutingLb() { if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_routing_lb_trace)) { @@ -502,8 +469,7 @@ void XdsRoutingLb::UpdateStateLocked() { } route_table.push_back(std::move(route)); } - picker = absl::make_unique(std::move(route_table), - user_agent_, config_); + picker = absl::make_unique(std::move(route_table), config_); break; } case GRPC_CHANNEL_CONNECTING: diff --git a/src/core/ext/filters/http/client/http_client_filter.cc b/src/core/ext/filters/http/client/http_client_filter.cc index 44fefec3002..f09c60ce8f6 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -31,7 +31,6 @@ #include #include "src/core/ext/filters/http/client/http_client_filter.h" -#include "src/core/ext/filters/http/client/util.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/manual_constructor.h" #include "src/core/lib/profiling/timers.h" @@ -529,8 +528,36 @@ static size_t max_payload_size_from_args(const grpc_channel_args* args) { static grpc_core::ManagedMemorySlice user_agent_from_args( const grpc_channel_args* args, const char* transport_name) { - return grpc_core::ManagedMemorySlice( - grpc_core::GenerateUserAgentFromArgs(args, transport_name).c_str()); + std::vector user_agent_fields; + + for (size_t i = 0; args && i < args->num_args; i++) { + if (0 == strcmp(args->args[i].key, GRPC_ARG_PRIMARY_USER_AGENT_STRING)) { + if (args->args[i].type != GRPC_ARG_STRING) { + gpr_log(GPR_ERROR, "Channel argument '%s' should be a string", + GRPC_ARG_PRIMARY_USER_AGENT_STRING); + } else { + user_agent_fields.push_back(args->args[i].value.string); + } + } + } + + user_agent_fields.push_back( + absl::StrFormat("grpc-c/%s (%s; %s)", grpc_version_string(), + GPR_PLATFORM_STRING, transport_name)); + + for (size_t i = 0; args && i < args->num_args; i++) { + if (0 == strcmp(args->args[i].key, GRPC_ARG_SECONDARY_USER_AGENT_STRING)) { + if (args->args[i].type != GRPC_ARG_STRING) { + gpr_log(GPR_ERROR, "Channel argument '%s' should be a string", + GRPC_ARG_SECONDARY_USER_AGENT_STRING); + } else { + user_agent_fields.push_back(args->args[i].value.string); + } + } + } + + std::string user_agent_string = absl::StrJoin(user_agent_fields, " "); + return grpc_core::ManagedMemorySlice(user_agent_string.c_str()); } /* Constructor for channel_data */ diff --git a/src/core/ext/filters/http/client/util.cc b/src/core/ext/filters/http/client/util.cc deleted file mode 100644 index d79fb49e763..00000000000 --- a/src/core/ext/filters/http/client/util.cc +++ /dev/null @@ -1,53 +0,0 @@ -// -// Copyright 2015 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -#include - -#include "absl/strings/str_format.h" -#include "absl/strings/str_join.h" - -#include "src/core/ext/filters/http/client/util.h" - -namespace grpc_core { -std::string GenerateUserAgentFromArgs(const grpc_channel_args* args, - const char* transport_name) { - std::vector user_agent_fields; - for (size_t i = 0; args && i < args->num_args; i++) { - if (0 == strcmp(args->args[i].key, GRPC_ARG_PRIMARY_USER_AGENT_STRING)) { - if (args->args[i].type != GRPC_ARG_STRING) { - gpr_log(GPR_ERROR, "Channel argument '%s' should be a string", - GRPC_ARG_PRIMARY_USER_AGENT_STRING); - } else { - user_agent_fields.push_back(args->args[i].value.string); - } - } - } - user_agent_fields.push_back( - absl::StrFormat("grpc-c/%s (%s; %s)", grpc_version_string(), - GPR_PLATFORM_STRING, transport_name)); - for (size_t i = 0; args && i < args->num_args; i++) { - if (0 == strcmp(args->args[i].key, GRPC_ARG_SECONDARY_USER_AGENT_STRING)) { - if (args->args[i].type != GRPC_ARG_STRING) { - gpr_log(GPR_ERROR, "Channel argument '%s' should be a string", - GRPC_ARG_SECONDARY_USER_AGENT_STRING); - } else { - user_agent_fields.push_back(args->args[i].value.string); - } - } - } - return absl::StrJoin(user_agent_fields, " "); -} -} // namespace grpc_core diff --git a/src/core/ext/filters/http/client/util.h b/src/core/ext/filters/http/client/util.h deleted file mode 100644 index 725c3dba4b4..00000000000 --- a/src/core/ext/filters/http/client/util.h +++ /dev/null @@ -1,30 +0,0 @@ -// -// Copyright 2015 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -#ifndef GRPC_CORE_EXT_FILTERS_HTTP_CLIENT_UTIL_H -#define GRPC_CORE_EXT_FILTERS_HTTP_CLIENT_UTIL_H - -#include - -#include "absl/strings/str_join.h" - -#include "src/core/lib/channel/channel_stack.h" - -namespace grpc_core { -std::string GenerateUserAgentFromArgs(const grpc_channel_args* args, - const char* transport_name); -} // namespace grpc_core -#endif /* GRPC_CORE_EXT_FILTERS_HTTP_CLIENT_UTIL_H */ diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 7cd03d4a6cc..5b9dc43d30d 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -83,7 +83,6 @@ CORE_SOURCE_FILES = [ 'src/core/ext/filters/client_idle/client_idle_filter.cc', 'src/core/ext/filters/deadline/deadline_filter.cc', 'src/core/ext/filters/http/client/http_client_filter.cc', - 'src/core/ext/filters/http/client/util.cc', 'src/core/ext/filters/http/client_authority_filter.cc', 'src/core/ext/filters/http/http_filters_plugin.cc', 'src/core/ext/filters/http/message_compress/message_compress_filter.cc', diff --git a/test/cpp/end2end/xds_end2end_test.cc b/test/cpp/end2end/xds_end2end_test.cc index 0d849f12fa5..10f1a1744ea 100644 --- a/test/cpp/end2end/xds_end2end_test.cc +++ b/test/cpp/end2end/xds_end2end_test.cc @@ -3608,245 +3608,6 @@ TEST_P(LdsRdsTest, XdsRoutingHeadersMatching) { gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_ROUTING"); } -TEST_P(LdsRdsTest, XdsRoutingHeadersMatchingSpecialHeaderContentType) { - gpr_setenv("GRPC_XDS_EXPERIMENTAL_ROUTING", "true"); - const char* kNewCluster1Name = "new_cluster_1"; - const size_t kNumEchoRpcs = 100; - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - // Populate new EDS resources. - AdsServiceImpl::EdsResourceArgs args({ - {"locality0", GetBackendPorts(0, 1)}, - }); - AdsServiceImpl::EdsResourceArgs args1({ - {"locality0", GetBackendPorts(1, 2)}, - }); - balancers_[0]->ads_service()->SetEdsResource( - AdsServiceImpl::BuildEdsResource(args)); - balancers_[0]->ads_service()->SetEdsResource( - AdsServiceImpl::BuildEdsResource(args1, kNewCluster1Name)); - // Populate new CDS resources. - Cluster new_cluster1 = balancers_[0]->ads_service()->default_cluster(); - new_cluster1.set_name(kNewCluster1Name); - balancers_[0]->ads_service()->SetCdsResource(new_cluster1); - // Populating Route Configurations for LDS. - RouteConfiguration route_config = - balancers_[0]->ads_service()->default_route_config(); - auto* route1 = route_config.mutable_virtual_hosts(0)->mutable_routes(0); - route1->mutable_match()->set_prefix(""); - auto* header_matcher1 = route1->mutable_match()->add_headers(); - header_matcher1->set_name("content-type"); - header_matcher1->set_exact_match("notapplication/grpc"); - route1->mutable_route()->set_cluster(kNewCluster1Name); - auto* default_route = route_config.mutable_virtual_hosts(0)->add_routes(); - default_route->mutable_match()->set_prefix(""); - auto* header_matcher2 = default_route->mutable_match()->add_headers(); - header_matcher2->set_name("content-type"); - header_matcher2->set_exact_match("application/grpc"); - default_route->mutable_route()->set_cluster(kDefaultResourceName); - SetRouteConfiguration(0, route_config); - // Make sure the backend is up. - WaitForAllBackends(0, 1); - // Send RPCs. - CheckRpcSendOk(kNumEchoRpcs); - EXPECT_EQ(kNumEchoRpcs, backends_[0]->backend_service()->request_count()); - EXPECT_EQ(0, backends_[1]->backend_service()->request_count()); - const auto& response_state = RouteConfigurationResponseState(0); - EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::ACKED); - gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_ROUTING"); -} - -TEST_P(LdsRdsTest, XdsRoutingHeadersMatchingSpecialHeaderUserAgent) { - gpr_setenv("GRPC_XDS_EXPERIMENTAL_ROUTING", "true"); - const char* kNewCluster1Name = "new_cluster_1"; - const size_t kNumEchoRpcs = 100; - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - // Populate new EDS resources. - AdsServiceImpl::EdsResourceArgs args({ - {"locality0", GetBackendPorts(0, 1)}, - }); - AdsServiceImpl::EdsResourceArgs args1({ - {"locality0", GetBackendPorts(1, 2)}, - }); - balancers_[0]->ads_service()->SetEdsResource( - AdsServiceImpl::BuildEdsResource(args)); - balancers_[0]->ads_service()->SetEdsResource( - AdsServiceImpl::BuildEdsResource(args1, kNewCluster1Name)); - // Populate new CDS resources. - Cluster new_cluster1 = balancers_[0]->ads_service()->default_cluster(); - new_cluster1.set_name(kNewCluster1Name); - balancers_[0]->ads_service()->SetCdsResource(new_cluster1); - // Populating Route Configurations for LDS. - RouteConfiguration route_config = - balancers_[0]->ads_service()->default_route_config(); - auto* route1 = route_config.mutable_virtual_hosts(0)->mutable_routes(0); - route1->mutable_match()->set_prefix(""); - auto* header_matcher1 = route1->mutable_match()->add_headers(); - header_matcher1->set_name("user-agent"); - header_matcher1->mutable_safe_regex_match()->set_regex( - "(does-not-match-grpc-c).*"); - route1->mutable_route()->set_cluster(kNewCluster1Name); - auto* default_route = route_config.mutable_virtual_hosts(0)->add_routes(); - default_route->mutable_match()->set_prefix(""); - auto* header_matcher2 = default_route->mutable_match()->add_headers(); - header_matcher2->set_name("user-agent"); - // user-agent string is a 2-part string like "grpc-c++/1.31.0-dev - // grpc-c/11.0.0". - header_matcher2->mutable_safe_regex_match()->set_regex( - "((grpc-c).*[0-9]+.[0-9]+.[0-9]+.*){2}"); - default_route->mutable_route()->set_cluster(kDefaultResourceName); - SetRouteConfiguration(0, route_config); - // Make sure backend is up. - WaitForAllBackends(0, 1); - // Send RPCs. - CheckRpcSendOk(kNumEchoRpcs); - EXPECT_EQ(kNumEchoRpcs, backends_[0]->backend_service()->request_count()); - EXPECT_EQ(0, backends_[1]->backend_service()->request_count()); - const auto& response_state = RouteConfigurationResponseState(0); - EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::ACKED); - gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_ROUTING"); -} - -TEST_P(LdsRdsTest, XdsRoutingHeadersMatchingSpecialHeaderGrpcTimeout) { - gpr_setenv("GRPC_XDS_EXPERIMENTAL_ROUTING", "true"); - const char* kNewCluster1Name = "new_cluster_1"; - const size_t kNumEchoRpcs = 100; - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - // Populate new EDS resources. - AdsServiceImpl::EdsResourceArgs args({ - {"locality0", GetBackendPorts(0, 1)}, - }); - AdsServiceImpl::EdsResourceArgs args1({ - {"locality0", GetBackendPorts(1, 2)}, - }); - balancers_[0]->ads_service()->SetEdsResource( - AdsServiceImpl::BuildEdsResource(args)); - balancers_[0]->ads_service()->SetEdsResource( - AdsServiceImpl::BuildEdsResource(args1, kNewCluster1Name)); - // Populate new CDS resources. - Cluster new_cluster1 = balancers_[0]->ads_service()->default_cluster(); - new_cluster1.set_name(kNewCluster1Name); - balancers_[0]->ads_service()->SetCdsResource(new_cluster1); - // Populating Route Configurations for LDS. - RouteConfiguration route_config = - balancers_[0]->ads_service()->default_route_config(); - auto* route1 = route_config.mutable_virtual_hosts(0)->mutable_routes(0); - route1->mutable_match()->set_prefix(""); - auto* header_matcher1 = route1->mutable_match()->add_headers(); - header_matcher1->set_name("grpc-timeout"); - header_matcher1->mutable_safe_regex_match()->set_regex("[0-9]+(s|h)"); - route1->mutable_route()->set_cluster(kNewCluster1Name); - auto* default_route = route_config.mutable_virtual_hosts(0)->add_routes(); - default_route->mutable_match()->set_prefix(""); - auto* header_matcher2 = default_route->mutable_match()->add_headers(); - header_matcher2->set_name("grpc-timeout"); - header_matcher2->mutable_safe_regex_match()->set_regex("[0-9]+(m|S|M|H)"); - default_route->mutable_route()->set_cluster(kDefaultResourceName); - SetRouteConfiguration(0, route_config); - // Make sure backend is up. - WaitForAllBackends(0, 1); - // Send RPCs. - CheckRpcSendOk(kNumEchoRpcs); - EXPECT_EQ(kNumEchoRpcs, backends_[0]->backend_service()->request_count()); - EXPECT_EQ(0, backends_[1]->backend_service()->request_count()); - const auto& response_state = RouteConfigurationResponseState(0); - EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::ACKED); - gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_ROUTING"); -} - -TEST_P(LdsRdsTest, XdsRoutingHeadersMatchingSpecialCasesToIgnore) { - gpr_setenv("GRPC_XDS_EXPERIMENTAL_ROUTING", "true"); - const char* kNewCluster1Name = "new_cluster_1"; - const char* kNewCluster2Name = "new_cluster_2"; - const char* kNewCluster3Name = "new_cluster_3"; - const size_t kNumEcho1Rpcs = 100; - const size_t kNumEchoRpcs = 5; - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - // Populate new EDS resources. - AdsServiceImpl::EdsResourceArgs args({ - {"locality0", GetBackendPorts(0, 1)}, - }); - AdsServiceImpl::EdsResourceArgs args1({ - {"locality0", GetBackendPorts(1, 2)}, - }); - AdsServiceImpl::EdsResourceArgs args2({ - {"locality0", GetBackendPorts(2, 3)}, - }); - AdsServiceImpl::EdsResourceArgs args3({ - {"locality0", GetBackendPorts(3, 4)}, - }); - balancers_[0]->ads_service()->SetEdsResource( - AdsServiceImpl::BuildEdsResource(args)); - balancers_[0]->ads_service()->SetEdsResource( - AdsServiceImpl::BuildEdsResource(args1, kNewCluster1Name)); - balancers_[0]->ads_service()->SetEdsResource( - AdsServiceImpl::BuildEdsResource(args2, kNewCluster2Name)); - balancers_[0]->ads_service()->SetEdsResource( - AdsServiceImpl::BuildEdsResource(args3, kNewCluster3Name)); - // Populate new CDS resources. - Cluster new_cluster1 = balancers_[0]->ads_service()->default_cluster(); - new_cluster1.set_name(kNewCluster1Name); - balancers_[0]->ads_service()->SetCdsResource(new_cluster1); - Cluster new_cluster2 = balancers_[0]->ads_service()->default_cluster(); - new_cluster2.set_name(kNewCluster2Name); - balancers_[0]->ads_service()->SetCdsResource(new_cluster2); - Cluster new_cluster3 = balancers_[0]->ads_service()->default_cluster(); - new_cluster1.set_name(kNewCluster3Name); - balancers_[0]->ads_service()->SetCdsResource(new_cluster3); - // Populating Route Configurations for LDS. - RouteConfiguration route_config = - balancers_[0]->ads_service()->default_route_config(); - auto* route1 = route_config.mutable_virtual_hosts(0)->mutable_routes(0); - route1->mutable_match()->set_prefix("/grpc.testing.EchoTest1Service/"); - auto* header_matcher1 = route1->mutable_match()->add_headers(); - header_matcher1->set_name("grpc-tags-bin"); - header_matcher1->set_exact_match("grpc-tags-bin"); - route1->mutable_route()->set_cluster(kNewCluster1Name); - auto route2 = route_config.mutable_virtual_hosts(0)->add_routes(); - route2->mutable_match()->set_prefix("/grpc.testing.EchoTest1Service/"); - auto* header_matcher2 = route2->mutable_match()->add_headers(); - header_matcher2->set_name("grpc-trace-bin"); - header_matcher2->set_exact_match("grpc-trace-bin"); - route2->mutable_route()->set_cluster(kNewCluster2Name); - auto route3 = route_config.mutable_virtual_hosts(0)->add_routes(); - route3->mutable_match()->set_prefix("/grpc.testing.EchoTest1Service/"); - auto* header_matcher3 = route3->mutable_match()->add_headers(); - header_matcher3->set_name("grpc-previous-rpc-attempts"); - header_matcher3->set_exact_match("grpc-previous-rpc-attempts"); - route3->mutable_route()->set_cluster(kNewCluster3Name); - auto* default_route = route_config.mutable_virtual_hosts(0)->add_routes(); - default_route->mutable_match()->set_prefix(""); - default_route->mutable_route()->set_cluster(kDefaultResourceName); - SetRouteConfiguration(0, route_config); - // Send headers which will mismatch each route - std::vector> metadata = { - {"grpc-tags-bin", "grpc-tags-bin"}, - {"grpc-trace-bin", "grpc-trace-bin"}, - {"grpc-previous-rpc-attempts", "grpc-previous-rpc-attempts"}, - }; - WaitForAllBackends(0, 1); - CheckRpcSendOk(kNumEchoRpcs, RpcOptions().set_metadata(metadata)); - CheckRpcSendOk(kNumEcho1Rpcs, RpcOptions() - .set_rpc_service(SERVICE_ECHO1) - .set_rpc_method(METHOD_ECHO1) - .set_metadata(metadata)); - // Verify that only the default backend got RPCs since all previous routes - // were mismatched. - for (size_t i = 1; i < 4; ++i) { - EXPECT_EQ(0, backends_[i]->backend_service()->request_count()); - EXPECT_EQ(0, backends_[i]->backend_service1()->request_count()); - EXPECT_EQ(0, backends_[i]->backend_service2()->request_count()); - } - EXPECT_EQ(kNumEchoRpcs, backends_[0]->backend_service()->request_count()); - EXPECT_EQ(kNumEcho1Rpcs, backends_[0]->backend_service1()->request_count()); - EXPECT_EQ(0, backends_[0]->backend_service2()->request_count()); - const auto& response_state = RouteConfigurationResponseState(0); - EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::ACKED); - gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_ROUTING"); -} TEST_P(LdsRdsTest, XdsRoutingRuntimeFractionMatching) { gpr_setenv("GRPC_XDS_EXPERIMENTAL_ROUTING", "true"); const char* kNewCluster1Name = "new_cluster_1"; diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index b4718a3de08..b7997f9c8ea 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1183,8 +1183,6 @@ src/core/ext/filters/deadline/deadline_filter.cc \ src/core/ext/filters/deadline/deadline_filter.h \ src/core/ext/filters/http/client/http_client_filter.cc \ src/core/ext/filters/http/client/http_client_filter.h \ -src/core/ext/filters/http/client/util.cc \ -src/core/ext/filters/http/client/util.h \ src/core/ext/filters/http/client_authority_filter.cc \ src/core/ext/filters/http/client_authority_filter.h \ src/core/ext/filters/http/http_filters_plugin.cc \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 583d0fdc99e..1edb9e7b663 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -990,8 +990,6 @@ src/core/ext/filters/deadline/deadline_filter.cc \ src/core/ext/filters/deadline/deadline_filter.h \ src/core/ext/filters/http/client/http_client_filter.cc \ src/core/ext/filters/http/client/http_client_filter.h \ -src/core/ext/filters/http/client/util.cc \ -src/core/ext/filters/http/client/util.h \ src/core/ext/filters/http/client_authority_filter.cc \ src/core/ext/filters/http/client_authority_filter.h \ src/core/ext/filters/http/http_filters_plugin.cc \ From 36e735bb72ac7c84c80159753b2c207e97173fe3 Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Fri, 17 Jul 2020 16:56:09 -0700 Subject: [PATCH 2/3] Fix concatenated_value --- .../ext/filters/client_channel/lb_policy/xds/xds_routing.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc index 0a66c35edf8..9d63ed6f069 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc @@ -263,7 +263,8 @@ absl::optional GetMetadataValue( bool HeaderMatchHelper( const XdsApi::RdsUpdate::RdsRoute::Matchers::HeaderMatcher& header_matcher, LoadBalancingPolicy::MetadataInterface* initial_metadata) { - auto value = GetMetadataValue(header_matcher.name, initial_metadata); + std::string concatenated_value; + auto value = GetMetadataValue(header_matcher.name, initial_metadata, &concatenated_value); if (!value.has_value()) { if (header_matcher.type == XdsApi::RdsUpdate::RdsRoute::Matchers:: HeaderMatcher::HeaderMatcherType::PRESENT) { From a9aa148603a8b9d59ddfdc09ba052d120daba865 Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Fri, 17 Jul 2020 18:07:33 -0700 Subject: [PATCH 3/3] Fix formatting issue --- .../ext/filters/client_channel/lb_policy/xds/xds_routing.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc index 9d63ed6f069..45770788868 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc @@ -264,7 +264,8 @@ bool HeaderMatchHelper( const XdsApi::RdsUpdate::RdsRoute::Matchers::HeaderMatcher& header_matcher, LoadBalancingPolicy::MetadataInterface* initial_metadata) { std::string concatenated_value; - auto value = GetMetadataValue(header_matcher.name, initial_metadata, &concatenated_value); + auto value = GetMetadataValue(header_matcher.name, initial_metadata, + &concatenated_value); if (!value.has_value()) { if (header_matcher.type == XdsApi::RdsUpdate::RdsRoute::Matchers:: HeaderMatcher::HeaderMatcherType::PRESENT) {