router: support for request/response header manipulation in weighted clusters

RAW_RELEASE_NOTES.md

@@ -76,4 +76,5 @@ final version.
 * Added support for stripping query string for redirects.
 * Added support for specifying a metadata matcher for upstream clusters in the tcp filter
 * Added support for listening on UNIX domain sockets.
+* Added support for downstream request/upstream response header manipulation in weighted cluster.
 * Added support for range based header matching for request routing.

source/common/router/config_impl.cc

@@ -268,26 +268,8 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost,
     const std::string& runtime_key_prefix = route.route().weighted_clusters().runtime_key_prefix();
 
     for (const auto& cluster : route.route().weighted_clusters().clusters()) {
-      const std::string& cluster_name = cluster.name();
-
-      MetadataMatchCriteriaImplConstPtr cluster_metadata_match_criteria;
-      if (cluster.has_metadata_match()) {
-        const auto filter_it = cluster.metadata_match().filter_metadata().find(
-            Envoy::Config::MetadataFilters::get().ENVOY_LB);
-        if (filter_it != cluster.metadata_match().filter_metadata().end()) {
-          if (metadata_match_criteria_) {
-            cluster_metadata_match_criteria =
-                metadata_match_criteria_->mergeMatchCriteria(filter_it->second);
-          } else {
-            cluster_metadata_match_criteria.reset(new MetadataMatchCriteriaImpl(filter_it->second));
-          }
-        }
-      }
-
-      std::unique_ptr<WeightedClusterEntry> cluster_entry(
-          new WeightedClusterEntry(this, runtime_key_prefix + "." + cluster_name, loader_,
-                                   cluster_name, PROTOBUF_GET_WRAPPED_REQUIRED(cluster, weight),
-                                   std::move(cluster_metadata_match_criteria)));
+      std::unique_ptr<WeightedClusterEntry> cluster_entry(new WeightedClusterEntry(
+          this, runtime_key_prefix + "." + cluster.name(), loader_, cluster));
       weighted_clusters_.emplace_back(std::move(cluster_entry));
       total_weight += weighted_clusters_.back()->clusterWeight();
     }
@@ -536,6 +518,28 @@ void RouteEntryImplBase::validateClusters(Upstream::ClusterManager& cm) const {
   }
 }
 
+RouteEntryImplBase::WeightedClusterEntry::WeightedClusterEntry(
+    const RouteEntryImplBase* parent, const std::string runtime_key, Runtime::Loader& loader,
+    const envoy::api::v2::route::WeightedCluster_ClusterWeight& cluster)
+    : DynamicRouteEntry(parent, cluster.name()), runtime_key_(runtime_key), loader_(loader),
+      cluster_weight_(PROTOBUF_GET_WRAPPED_REQUIRED(cluster, weight)),
+      request_headers_parser_(HeaderParser::configure(cluster.request_headers_to_add())),
+      response_headers_parser_(HeaderParser::configure(cluster.response_headers_to_add(),
+                                                       cluster.response_headers_to_remove())) {
+  if (cluster.has_metadata_match()) {
+    const auto filter_it = cluster.metadata_match().filter_metadata().find(
+        Envoy::Config::MetadataFilters::get().ENVOY_LB);
+    if (filter_it != cluster.metadata_match().filter_metadata().end()) {
+      if (parent->metadata_match_criteria_) {
+        cluster_metadata_match_criteria_ =
+            parent->metadata_match_criteria_->mergeMatchCriteria(filter_it->second);
+      } else {
+        cluster_metadata_match_criteria_.reset(new MetadataMatchCriteriaImpl(filter_it->second));
+      }
+    }
+  }
+}
+
 PrefixRouteEntryImpl::PrefixRouteEntryImpl(const VirtualHostImpl& vhost,
                                            const envoy::api::v2::route::Route& route,
                                            Runtime::Loader& loader)

source/common/router/config_impl.h

@@ -442,12 +442,9 @@ class RouteEntryImplBase : public RouteEntry,
    */
   class WeightedClusterEntry : public DynamicRouteEntry {
   public:
-    WeightedClusterEntry(const RouteEntryImplBase* parent, const std::string runtime_key,
-                         Runtime::Loader& loader, const std::string& name, uint64_t weight,
-                         MetadataMatchCriteriaImplConstPtr cluster_metadata_match_criteria)
-        : DynamicRouteEntry(parent, name), runtime_key_(runtime_key), loader_(loader),
-          cluster_weight_(weight),
-          cluster_metadata_match_criteria_(std::move(cluster_metadata_match_criteria)) {}
+    WeightedClusterEntry(const RouteEntryImplBase* parent, const std::string rutime_key,
+                         Runtime::Loader& loader,
+                         const envoy::api::v2::route::WeightedCluster_ClusterWeight& cluster);
 
     uint64_t clusterWeight() const {
       return loader_.snapshot().getInteger(runtime_key_, cluster_weight_);
@@ -460,11 +457,24 @@ class RouteEntryImplBase : public RouteEntry,
       return DynamicRouteEntry::metadataMatchCriteria();
     }
 
+    void finalizeRequestHeaders(Http::HeaderMap& headers,
+                                const RequestInfo::RequestInfo& request_info) const override {
+      request_headers_parser_->evaluateHeaders(headers, request_info);
+      DynamicRouteEntry::finalizeRequestHeaders(headers, request_info);
+    }
+    void finalizeResponseHeaders(Http::HeaderMap& headers,
+                                 const RequestInfo::RequestInfo& request_info) const override {
+      response_headers_parser_->evaluateHeaders(headers, request_info);
+      DynamicRouteEntry::finalizeResponseHeaders(headers, request_info);
+    }
+
   private:
     const std::string runtime_key_;
     Runtime::Loader& loader_;
     const uint64_t cluster_weight_;
     MetadataMatchCriteriaImplConstPtr cluster_metadata_match_criteria_;
+    HeaderParserPtr request_headers_parser_;
+    HeaderParserPtr response_headers_parser_;
   };
 
   typedef std::shared_ptr<WeightedClusterEntry> WeightedClusterEntrySharedPtr;

test/common/router/config_impl_test.cc

@@ -2874,6 +2874,74 @@ TEST(RouteMatcherTest, TestWeightedClusterInvalidClusterName) {
                EnvoyException);
 }
 
+TEST(RouteMatcherTest, TestWeightedClusterHeaderManipulation) {
+  std::string yaml = R"EOF(
+virtual_hosts:
+  - name: www2
+    domains: ["www.lyft.com"]
+    routes:
+      - match: { prefix: "/" }
+        route:
+          weighted_clusters:
+            clusters:
+              - name: cluster1
+                weight: 50
+                request_headers_to_add:
+                  - header:
+                      key: x-req-cluster
+                      value: cluster1
+                response_headers_to_add:
+                  - header:
+                      key: x-resp-cluster
+                      value: cluster1
+                response_headers_to_remove: [ "x-remove-cluster1" ]
+              - name: cluster2
+                weight: 50
+                request_headers_to_add:
+                  - header:
+                      key: x-req-cluster
+                      value: cluster2
+                response_headers_to_add:
+                  - header:
+                      key: x-resp-cluster
+                      value: cluster2
+                response_headers_to_remove: [ "x-remove-cluster2" ]
+  )EOF";
+
+  NiceMock<Runtime::MockLoader> runtime;
+  NiceMock<Upstream::MockClusterManager> cm;
+  ConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), runtime, cm, true);
+  NiceMock<Envoy::RequestInfo::MockRequestInfo> request_info;
+
+  {
+    Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/foo", "GET");
+    Http::TestHeaderMapImpl resp_headers({{"x-remove-cluster1", "value"}});
+    const RouteEntry* route = config.route(headers, 0)->routeEntry();
+    EXPECT_EQ("cluster1", route->clusterName());
+
+    route->finalizeRequestHeaders(headers, request_info);
+    EXPECT_EQ("cluster1", headers.get_("x-req-cluster"));
+
+    route->finalizeResponseHeaders(resp_headers, request_info);
+    EXPECT_EQ("cluster1", resp_headers.get_("x-resp-cluster"));
+    EXPECT_FALSE(resp_headers.has("x-remove-cluster1"));
+  }
+
+  {
+    Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/foo", "GET");
+    Http::TestHeaderMapImpl resp_headers({{"x-remove-cluster2", "value"}});
+    const RouteEntry* route = config.route(headers, 55)->routeEntry();
+    EXPECT_EQ("cluster2", route->clusterName());
+
+    route->finalizeRequestHeaders(headers, request_info);
+    EXPECT_EQ("cluster2", headers.get_("x-req-cluster"));
+
+    route->finalizeResponseHeaders(resp_headers, request_info);
+    EXPECT_EQ("cluster2", resp_headers.get_("x-resp-cluster"));
+    EXPECT_FALSE(resp_headers.has("x-remove-cluster2"));
+  }
+}
+
 TEST(NullConfigImplTest, All) {
   NullConfigImpl config;
   Http::TestHeaderMapImpl headers = genRedirectHeaders("redirect.lyft.com", "/baz", true, false);