Skip to content

Commit

Permalink
router: support for request/response header manipulation in weighted …
Browse files Browse the repository at this point in the history
…clusters (#2765)

Implements the request_headers_to_add, response_headers_to_add, and response_headers_to_remove fields added to weighted clusters by envoyproxy/data-plane-api#441.

Risk Level: Low - no change in behavior without configuration changes

Testing: unit and integration tests

Docs Changes: envoyproxy/data-plane-api#531

Release Notes: updated

Fixes: #2455

Signed-off-by: Stephan Zuercher [email protected]
zuercher authored Mar 13, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 725a6a4 commit 96c645c
Showing 5 changed files with 271 additions and 41 deletions.
1 change: 1 addition & 0 deletions RAW_RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -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.
44 changes: 24 additions & 20 deletions source/common/router/config_impl.cc
Original file line number Diff line number Diff line change
@@ -269,26 +269,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();
}
@@ -537,6 +519,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)
22 changes: 16 additions & 6 deletions source/common/router/config_impl.h
Original file line number Diff line number Diff line change
@@ -443,12 +443,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_);
@@ -461,11 +458,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;
68 changes: 68 additions & 0 deletions test/common/router/config_impl_test.cc
Original file line number Diff line number Diff line change
@@ -2865,6 +2865,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);
Loading

0 comments on commit 96c645c

Please sign in to comment.