Skip to content

Commit

Permalink
rds: expose route metadata to requestinfo (#2392)
Browse files Browse the repository at this point in the history
Signed-off-by: Matt Rice <mattrice@google.com>
  • Loading branch information
mrice32 authored and mattklein123 committed Jan 20, 2018
1 parent 5c15e35 commit 29989a3
Show file tree
Hide file tree
Showing 17 changed files with 102 additions and 8 deletions.
11 changes: 11 additions & 0 deletions include/envoy/request_info/request_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
#include "envoy/upstream/upstream.h"

namespace Envoy {

namespace Router {
class RouteEntry;
} // namespace Router

namespace RequestInfo {

enum ResponseFlag {
Expand Down Expand Up @@ -158,6 +163,12 @@ class RequestInfo {
* inferred from proxy proto, x-forwarded-for, etc.
*/
virtual const Network::Address::InstanceConstSharedPtr& downstreamRemoteAddress() const PURE;

/**
* @return const Router::RouteEntry* Get the route entry selected for this request. Note: this
* will be nullptr if no route was selected.
*/
virtual const Router::RouteEntry* routeEntry() const PURE;
};

} // namespace RequestInfo
Expand Down
8 changes: 8 additions & 0 deletions include/envoy/router/router.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include "common/protobuf/protobuf.h"
#include "common/protobuf/utility.h"

#include "api/base.pb.h"

namespace Envoy {
namespace Router {

Expand Down Expand Up @@ -393,6 +395,12 @@ class RouteEntry {
* @return bool true if the virtual host rate limits should be included.
*/
virtual bool includeVirtualHostRateLimits() const PURE;

/**
* @return const envoy::api::v2::Metadata& return the metadata provided in the config for this
* route.
*/
virtual const envoy::api::v2::Metadata& metadata() const PURE;
};

/**
Expand Down
1 change: 1 addition & 0 deletions source/common/http/async_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const AsyncStreamImpl::NullShadowPolicy AsyncStreamImpl::RouteEntryImpl::shadow_
const AsyncStreamImpl::NullVirtualHost AsyncStreamImpl::RouteEntryImpl::virtual_host_;
const AsyncStreamImpl::NullRateLimitPolicy AsyncStreamImpl::NullVirtualHost::rate_limit_policy_;
const std::multimap<std::string, std::string> AsyncStreamImpl::RouteEntryImpl::opaque_config_;
const envoy::api::v2::Metadata AsyncStreamImpl::RouteEntryImpl::metadata_;

AsyncClientImpl::AsyncClientImpl(const Upstream::ClusterInfo& cluster, Stats::Store& stats_store,
Event::Dispatcher& dispatcher,
Expand Down
2 changes: 2 additions & 0 deletions source/common/http/async_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,14 @@ class AsyncStreamImpl : public AsyncClient::Stream,
bool autoHostRewrite() const override { return false; }
bool useWebSocket() const override { return false; }
bool includeVirtualHostRateLimits() const override { return true; }
const envoy::api::v2::Metadata& metadata() const override { return metadata_; }

static const NullRateLimitPolicy rate_limit_policy_;
static const NullRetryPolicy retry_policy_;
static const NullShadowPolicy shadow_policy_;
static const NullVirtualHost virtual_host_;
static const std::multimap<std::string, std::string> opaque_config_;
static const envoy::api::v2::Metadata metadata_;

const std::string& cluster_name_;
Optional<std::chrono::milliseconds> timeout_;
Expand Down
13 changes: 9 additions & 4 deletions source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -510,12 +510,12 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers,
ASSERT(request_info_.downstream_remote_address_ != nullptr);

ASSERT(!cached_route_.valid());
cached_route_.value(snapped_route_config_->route(*request_headers_, stream_id_));
refreshCachedRoute();

// Check for WebSocket upgrade request if the route exists, and supports WebSockets.
// TODO if there are no filters when starting a filter iteration, the connection manager
// should return 404. The current returns no response if there is no router filter.
if ((protocol == Protocol::Http11) && cached_route_.value()) {
if (protocol == Protocol::Http11 && cached_route_.value()) {
const Router::RouteEntry* route_entry = cached_route_.value()->routeEntry();
const bool websocket_allowed = (route_entry != nullptr) && route_entry->useWebSocket();
const bool websocket_requested = Utility::isWebSocketUpgradeRequest(*request_headers_);
Expand Down Expand Up @@ -765,6 +765,12 @@ void ConnectionManagerImpl::startDrainSequence() {
drain_timer_->enableTimer(config_.drainTimeout());
}

void ConnectionManagerImpl::ActiveStream::refreshCachedRoute() {
Router::RouteConstSharedPtr route = snapped_route_config_->route(*request_headers_, stream_id_);
request_info_.route_entry_ = route ? route->routeEntry() : nullptr;
cached_route_.value(std::move(route));
}

void ConnectionManagerImpl::ActiveStream::encodeHeaders(ActiveStreamEncoderFilter* filter,
HeaderMap& headers, bool end_stream) {
std::list<ActiveStreamEncoderFilterPtr>::iterator entry = commonEncodePrefix(filter, end_stream);
Expand Down Expand Up @@ -1149,8 +1155,7 @@ Tracing::Config& ConnectionManagerImpl::ActiveStreamFilterBase::tracingConfig()

Router::RouteConstSharedPtr ConnectionManagerImpl::ActiveStreamFilterBase::route() {
if (!parent_.cached_route_.valid()) {
parent_.cached_route_.value(
parent_.snapped_route_config_->route(*parent_.request_headers_, parent_.stream_id_));
parent_.refreshCachedRoute();
}

return parent_.cached_route_.value();
Expand Down
2 changes: 2 additions & 0 deletions source/common/http/conn_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,8 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,

void traceRequest();

void refreshCachedRoute();

// Pass on watermark callbacks to watermark subscribers. This boils down to passing watermark
// events for this stream and the downstream connection to the router filter.
void callHighWatermarkCallbacks();
Expand Down
3 changes: 3 additions & 0 deletions source/common/request_info/request_info_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ struct RequestInfoImpl : public RequestInfo {
return downstream_remote_address_;
}

const Router::RouteEntry* routeEntry() const override { return route_entry_; }

Optional<Http::Protocol> protocol_;
const SystemTime start_time_;
const MonotonicTime start_time_monotonic_;
Expand All @@ -88,6 +90,7 @@ struct RequestInfoImpl : public RequestInfo {
bool hc_request_{};
Network::Address::InstanceConstSharedPtr downstream_local_address_;
Network::Address::InstanceConstSharedPtr downstream_remote_address_;
const Router::RouteEntry* route_entry_{};
};

} // namespace RequestInfo
Expand Down
4 changes: 4 additions & 0 deletions source/common/router/config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,10 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost,
}
}

if (route.has_metadata()) {
metadata_ = route.metadata();
}

// If this is a weighted_cluster, we create N internal route entries
// (called WeightedClusterEntry), such that each object is a simple
// single cluster, pointing back to the parent. Metadata criteria
Expand Down
3 changes: 3 additions & 0 deletions source/common/router/config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ class RouteEntryImplBase : public RouteEntry,
return opaque_config_;
}
bool includeVirtualHostRateLimits() const override { return include_vh_rate_limits_; }
const envoy::api::v2::Metadata& metadata() const override { return metadata_; }

// Router::DirectResponseEntry
std::string newPath(const Http::HeaderMap& headers) const override;
Expand Down Expand Up @@ -401,6 +402,7 @@ class RouteEntryImplBase : public RouteEntry,
bool includeVirtualHostRateLimits() const override {
return parent_->includeVirtualHostRateLimits();
}
const envoy::api::v2::Metadata& metadata() const override { return parent_->metadata(); }

// Router::Route
const DirectResponseEntry* directResponseEntry() const override { return nullptr; }
Expand Down Expand Up @@ -483,6 +485,7 @@ class RouteEntryImplBase : public RouteEntry,
MetadataMatchCriteriaImplConstPtr metadata_match_criteria_;
HeaderParserPtr request_headers_parser_;
HeaderParserPtr response_headers_parser_;
envoy::api::v2::Metadata metadata_;

// TODO(danielhochman): refactor multimap into unordered_map since JSON is unordered map.
const std::multimap<std::string, std::string> opaque_config_;
Expand Down
3 changes: 3 additions & 0 deletions test/common/access_log/access_log_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ class TestRequestInfo : public RequestInfo::RequestInfo {
return downstream_remote_address_;
}

const Router::RouteEntry* routeEntry() const override { return route_entry_; }

SystemTime start_time_;
Optional<std::chrono::microseconds> request_received_duration_{std::chrono::microseconds(1000)};
Optional<std::chrono::microseconds> response_received_duration_{std::chrono::microseconds(2000)};
Expand All @@ -102,6 +104,7 @@ class TestRequestInfo : public RequestInfo::RequestInfo {
Network::Address::InstanceConstSharedPtr upstream_local_address_;
Network::Address::InstanceConstSharedPtr downstream_local_address_;
Network::Address::InstanceConstSharedPtr downstream_remote_address_;
const Router::RouteEntry* route_entry_{};
};

class AccessLogImplTest : public testing::Test {
Expand Down
26 changes: 22 additions & 4 deletions test/common/http/conn_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,7 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLog) {
EXPECT_EQ(request_info.responseCode().value(), uint32_t(200));
EXPECT_NE(nullptr, request_info.downstreamLocalAddress());
EXPECT_NE(nullptr, request_info.downstreamRemoteAddress());
EXPECT_NE(nullptr, request_info.routeEntry());
}));

StreamDecoder* decoder = nullptr;
Expand Down Expand Up @@ -1637,19 +1638,36 @@ TEST_F(HttpConnectionManagerImplTest, FilterClearRouteCache) {
decoder->decodeHeaders(std::move(headers), true);
}));

setupFilterChain(2, 2);
setupFilterChain(3, 2);

Router::RouteConstSharedPtr route1 = std::make_shared<NiceMock<Router::MockRoute>>();
Router::RouteConstSharedPtr route2 = std::make_shared<NiceMock<Router::MockRoute>>();

EXPECT_CALL(*route_config_provider_.route_config_, route(_, _)).Times(2);
EXPECT_CALL(*route_config_provider_.route_config_, route(_, _))
.WillOnce(Return(route1))
.WillOnce(Return(route2))
.WillOnce(Return(nullptr));

EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, true))
.WillOnce(InvokeWithoutArgs([&]() -> FilterHeadersStatus {
decoder_filters_[0]->callbacks_->route();
EXPECT_EQ(route1, decoder_filters_[0]->callbacks_->route());
EXPECT_EQ(route1->routeEntry(),
decoder_filters_[0]->callbacks_->requestInfo().routeEntry());
decoder_filters_[0]->callbacks_->clearRouteCache();
return FilterHeadersStatus::Continue;
}));
EXPECT_CALL(*decoder_filters_[1], decodeHeaders(_, true))
.WillOnce(InvokeWithoutArgs([&]() -> FilterHeadersStatus {
decoder_filters_[1]->callbacks_->route();
EXPECT_EQ(route2, decoder_filters_[1]->callbacks_->route());
EXPECT_EQ(route2->routeEntry(),
decoder_filters_[1]->callbacks_->requestInfo().routeEntry());
decoder_filters_[1]->callbacks_->clearRouteCache();
return FilterHeadersStatus::Continue;
}));
EXPECT_CALL(*decoder_filters_[2], decodeHeaders(_, true))
.WillOnce(InvokeWithoutArgs([&]() -> FilterHeadersStatus {
EXPECT_EQ(nullptr, decoder_filters_[2]->callbacks_->route());
EXPECT_EQ(nullptr, decoder_filters_[2]->callbacks_->requestInfo().routeEntry());
return FilterHeadersStatus::StopIteration;
}));

Expand Down
1 change: 1 addition & 0 deletions test/common/request_info/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ envoy_cc_test(
"//include/envoy/http:protocol_interface",
"//include/envoy/upstream:host_description_interface",
"//source/common/request_info:request_info_lib",
"//test/mocks/router:router_mocks",
"//test/mocks/upstream:upstream_mocks",
],
)
Expand Down
6 changes: 6 additions & 0 deletions test/common/request_info/request_info_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "common/request_info/request_info_impl.h"

#include "test/mocks/router/mocks.h"
#include "test/mocks/upstream/mocks.h"

#include "fmt/format.h"
Expand Down Expand Up @@ -125,6 +126,11 @@ TEST(RequestInfoImplTest, MiscSettersAndGetters) {
EXPECT_FALSE(request_info.healthCheck());
request_info.healthCheck(true);
EXPECT_TRUE(request_info.healthCheck());

EXPECT_EQ(nullptr, request_info.routeEntry());
NiceMock<Router::MockRouteEntry> route_entry;
request_info.route_entry_ = &route_entry;
EXPECT_EQ(&route_entry, request_info.routeEntry());
}

{
Expand Down
1 change: 1 addition & 0 deletions test/common/router/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ envoy_cc_test(
name = "config_impl_test",
srcs = ["config_impl_test.cc"],
deps = [
"//source/common/config:metadata_lib",
"//source/common/config:rds_json_lib",
"//source/common/http:header_map_lib",
"//source/common/http:headers_lib",
Expand Down
24 changes: 24 additions & 0 deletions test/common/router/config_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3402,6 +3402,30 @@ name: foo
}
}

TEST(RouteConfigurationV2, Metadata) {
std::string yaml = R"EOF(
name: foo
virtual_hosts:
- name: bar
domains: ["*"]
routes:
- match: { prefix: "/"}
route: { cluster: www2 }
metadata: { filter_metadata: { com.bar.foo: { baz: test_value } } }
)EOF";

NiceMock<Runtime::MockLoader> runtime;
NiceMock<Upstream::MockClusterManager> cm;
ConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), runtime, cm, true);

auto* route_entry = config.route(genHeaders("www.foo.com", "/", "GET"), 0)->routeEntry();

const auto& metadata = route_entry->metadata();

EXPECT_EQ("test_value",
Envoy::Config::Metadata::metadataValue(metadata, "com.bar.foo", "baz").string_value());
}

} // namespace
} // namespace Router
} // namespace Envoy
1 change: 1 addition & 0 deletions test/mocks/request_info/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class MockRequestInfo : public RequestInfo {
MOCK_METHOD1(healthCheck, void(bool is_hc));
MOCK_CONST_METHOD0(downstreamLocalAddress, const Network::Address::InstanceConstSharedPtr&());
MOCK_CONST_METHOD0(downstreamRemoteAddress, const Network::Address::InstanceConstSharedPtr&());
MOCK_CONST_METHOD0(routeEntry, const Router::RouteEntry*());

std::shared_ptr<testing::NiceMock<Upstream::MockHostDescription>> host_{
new testing::NiceMock<Upstream::MockHostDescription>()};
Expand Down
1 change: 1 addition & 0 deletions test/mocks/router/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ class MockRouteEntry : public RouteEntry {
MOCK_CONST_METHOD0(opaqueConfig, const std::multimap<std::string, std::string>&());
MOCK_CONST_METHOD0(includeVirtualHostRateLimits, bool());
MOCK_CONST_METHOD0(corsPolicy, const CorsPolicy*());
MOCK_CONST_METHOD0(metadata, const envoy::api::v2::Metadata&());

std::string cluster_name_{"fake_cluster"};
std::multimap<std::string, std::string> opaque_config_;
Expand Down

0 comments on commit 29989a3

Please sign in to comment.