Merge pull request grpc#23538 from karthikravis/fix-xds-breakage

Revert "Adding Fake headers for header matching."
librato · Jul 18, 2020 · 6d12fa7 · 6d12fa7
2 parents 97fdeb8 + a9aa148
commit 6d12fa7
Show file tree

Hide file tree

Showing 20 changed files with 43 additions and 414 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -206,8 +206,6 @@
     <file baseinstalldir="/" name="src/core/ext/filters/deadline/deadline_filter.h" role="src" />
     <file baseinstalldir="/" name="src/core/ext/filters/http/client/http_client_filter.cc" role="src" />
     <file baseinstalldir="/" name="src/core/ext/filters/http/client/http_client_filter.h" role="src" />
-    <file baseinstalldir="/" name="src/core/ext/filters/http/client/util.cc" role="src" />
-    <file baseinstalldir="/" name="src/core/ext/filters/http/client/util.h" role="src" />
     <file baseinstalldir="/" name="src/core/ext/filters/http/client_authority_filter.cc" role="src" />
     <file baseinstalldir="/" name="src/core/ext/filters/http/client_authority_filter.h" role="src" />
     <file baseinstalldir="/" name="src/core/ext/filters/http/http_filters_plugin.cc" role="src" />

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
@@ -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<Route>;
 
-    RoutePicker(RouteTable route_table, std::string user_agent,
-                RefCountedPtr<XdsRoutingLbConfig> config)
-        : route_table_(std::move(route_table)),
-          user_agent_(std::move(user_agent)),
-          config_(std::move(config)) {}
+    explicit RoutePicker(RouteTable route_table,
+                         RefCountedPtr<XdsRoutingLbConfig> 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<XdsRoutingLbConfig> config_;
@@ -220,9 +215,6 @@ class XdsRoutingLb : public LoadBalancingPolicy {
 
   // Children.
   std::map<std::string, OrphanablePtr<XdsRoutingChild>> actions_;
-
-  // Storing user_agent generated from args from http layer.
-  std::string user_agent_;
 };
 
 //
@@ -270,24 +262,10 @@ absl::optional<absl::string_view> GetMetadataValue(
 
 bool HeaderMatchHelper(
     const XdsApi::RdsUpdate::RdsRoute::Matchers::HeaderMatcher& header_matcher,
-    LoadBalancingPolicy::MetadataInterface* initial_metadata,
-    const std::string& user_agent, absl::string_view deadline) {
+    LoadBalancingPolicy::MetadataInterface* initial_metadata) {
   std::string concatenated_value;
-  absl::optional<absl::string_view> 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);
-  }
+  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) {
@@ -325,13 +303,11 @@ bool HeaderMatchHelper(
 }
 
 bool HeadersMatch(
+    LoadBalancingPolicy::PickArgs args,
     const std::vector<XdsApi::RdsUpdate::RdsRoute::Matchers::HeaderMatcher>&
-        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 +325,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 +346,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 +471,7 @@ void XdsRoutingLb::UpdateStateLocked() {
         }
         route_table.push_back(std::move(route));
       }
-      picker = absl::make_unique<RoutePicker>(std::move(route_table),
-                                              user_agent_, config_);
+      picker = absl::make_unique<RoutePicker>(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
@@ -31,7 +31,6 @@
 #include <grpc/support/log.h>
 
 #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<std::string> 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 */