Merge pull request #23491 from markdroth/xds_header_combining

Concatenate duplicate header keys for matching.

src/core/ext/filters/client_channel/lb_policy/xds/xds_routing.cc

@@ -20,9 +20,11 @@
 #include <limits.h>
 #include <string.h>
 
+#include "absl/container/inlined_vector.h"
 #include "absl/strings/match.h"
 #include "absl/strings/numbers.h"
 #include "absl/strings/str_cat.h"
+#include "absl/strings/str_join.h"
 #include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
 #include "re2/re2.h"
@@ -227,21 +229,6 @@ class XdsRoutingLb : public LoadBalancingPolicy {
 // XdsRoutingLb::RoutePicker
 //
 
-absl::optional<absl::string_view> GetMetadataValue(
-    const std::string& key,
-    LoadBalancingPolicy::MetadataInterface* initial_metadata) {
-  // TODO(roth): Using const auto& here trigger a warning in a macos or windows
-  // build:
-  //*(args.initial_metadata) is returning values not references.
-  GPR_DEBUG_ASSERT(initial_metadata != nullptr);
-  for (const auto p : *(initial_metadata)) {
-    if (p.first == key) {
-      return p.second;
-    }
-  }
-  return absl::nullopt;
-}
-
 bool PathMatch(
     const absl::string_view& path,
     const XdsApi::RdsUpdate::RdsRoute::Matchers::PathMatcher& path_matcher) {
@@ -260,10 +247,32 @@ bool PathMatch(
   }
 }
 
+absl::optional<absl::string_view> GetMetadataValue(
+    const std::string& key,
+    LoadBalancingPolicy::MetadataInterface* initial_metadata,
+    std::string* concatenated_value) {
+  // Find all values for the specified key.
+  GPR_DEBUG_ASSERT(initial_metadata != nullptr);
+  absl::InlinedVector<absl::string_view, 1> values;
+  for (const auto p : *initial_metadata) {
+    if (p.first == key) values.push_back(p.second);
+  }
+  // If none found, no match.
+  if (values.empty()) return absl::nullopt;
+  // If exactly one found, return it as-is.
+  if (values.size() == 1) return values.front();
+  // If more than one found, concatenate the values, using
+  // *concatenated_values as a temporary holding place for the
+  // concatenated string.
+  *concatenated_value = absl::StrJoin(values, ",");
+  return *concatenated_value;
+}
+
 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<absl::string_view> value;
   if (header_matcher.name == "grpc-tags-bin" ||
       header_matcher.name == "grpc-trace-bin" ||
@@ -276,7 +285,8 @@ bool HeaderMatchHelper(
   } else if (header_matcher.name == "grpc-timeout") {
     value = deadline;
   } else {
-    value = GetMetadataValue(header_matcher.name, initial_metadata);
+    value = GetMetadataValue(header_matcher.name, initial_metadata,
+                             &concatenated_value);
   }
   if (!value.has_value()) {
     if (header_matcher.type == XdsApi::RdsUpdate::RdsRoute::Matchers::

test/cpp/end2end/xds_end2end_test.cc

@@ -3558,7 +3558,7 @@ TEST_P(LdsRdsTest, XdsRoutingHeadersMatching) {
   route1->mutable_match()->set_prefix("/grpc.testing.EchoTest1Service/");
   auto* header_matcher1 = route1->mutable_match()->add_headers();
   header_matcher1->set_name("header1");
-  header_matcher1->set_exact_match("POST");
+  header_matcher1->set_exact_match("POST,PUT,GET");
   auto* header_matcher2 = route1->mutable_match()->add_headers();
   header_matcher2->set_name("header2");
   header_matcher2->mutable_safe_regex_match()->set_regex("[a-z]*");
@@ -3582,9 +3582,10 @@ TEST_P(LdsRdsTest, XdsRoutingHeadersMatching) {
   default_route->mutable_route()->set_cluster(kDefaultResourceName);
   SetRouteConfiguration(0, route_config);
   std::vector<std::pair<std::string, std::string>> metadata = {
-      {"header1", "POST"},      {"header2", "blah"},
-      {"header3", "1"},         {"header5", "/grpc.testing.EchoTest1Service/"},
-      {"header6", "grpc.java"},
+      {"header1", "POST"}, {"header2", "blah"},
+      {"header3", "1"},    {"header5", "/grpc.testing.EchoTest1Service/"},
+      {"header1", "PUT"},  {"header6", "grpc.java"},
+      {"header1", "GET"},
   };
   const auto header_match_rpc_options = RpcOptions()
                                             .set_rpc_service(SERVICE_ECHO1)
@@ -3968,9 +3969,10 @@ TEST_P(LdsRdsTest, XdsRoutingHeadersMatchingUnmatchCases) {
   SetRouteConfiguration(0, route_config);
   // Send headers which will mismatch each route
   std::vector<std::pair<std::string, std::string>> metadata = {
-      {"header1", "POST1"},
+      {"header1", "POST"},
       {"header2", "1000"},
       {"header3", "123"},
+      {"header1", "GET"},
   };
   WaitForAllBackends(0, 1);
   CheckRpcSendOk(kNumEchoRpcs, RpcOptions().set_metadata(metadata));