From 8337bb12f2e9eb628315420aca4fd184fab1abc4 Mon Sep 17 00:00:00 2001 From: ccaraman <ccaramanolis@lyft.com> Date: Thu, 5 Jan 2017 09:41:41 -0800 Subject: [PATCH] Expose Virtual Host in Route Entry (#314) --- include/envoy/router/router.h | 17 +- source/common/http/async_client_impl.cc | 1 + source/common/http/async_client_impl.h | 8 +- source/common/router/config_impl.cc | 39 +- source/common/router/config_impl.h | 26 +- source/common/router/router.cc | 4 +- test/common/router/config_impl_test.cc | 2 +- test/common/router/router_ratelimit_test.cc | 394 ++++++-------------- test/mocks/router/mocks.cc | 2 +- test/mocks/router/mocks.h | 11 +- 10 files changed, 181 insertions(+), 323 deletions(-) diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 352bfe3fa62e..485c8e194a9a 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -120,6 +120,19 @@ class VirtualCluster { virtual Upstream::ResourcePriority priority() const PURE; }; +/** + * Virtual host defintion. + */ +class VirtualHost { +public: + virtual ~VirtualHost() {} + + /** + * @return const std::string& the name of the virtual host. + */ + virtual const std::string& name() const PURE; +}; + class RateLimitPolicy; /** @@ -177,9 +190,9 @@ class RouteEntry { virtual const VirtualCluster* virtualCluster(const Http::HeaderMap& headers) const PURE; /** - * @return const std::string& the virtual host that owns the route. + * @return const VirtualHost& the virtual host that owns the route. */ - virtual const std::string& virtualHostName() const PURE; + virtual const VirtualHost& virtualHost() const PURE; }; /** diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index d6972f903632..36a9708d46b4 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -8,6 +8,7 @@ const std::vector<std::reference_wrapper<const Router::RateLimitPolicyEntry>> const AsyncRequestImpl::NullRateLimitPolicy AsyncRequestImpl::RouteEntryImpl::rate_limit_policy_; const AsyncRequestImpl::NullRetryPolicy AsyncRequestImpl::RouteEntryImpl::retry_policy_; const AsyncRequestImpl::NullShadowPolicy AsyncRequestImpl::RouteEntryImpl::shadow_policy_; +const AsyncRequestImpl::NullVirtualHost AsyncRequestImpl::RouteEntryImpl::virtual_host_; AsyncClientImpl::AsyncClientImpl(const Upstream::ClusterInfo& cluster, Stats::Store& stats_store, Event::Dispatcher& dispatcher, const std::string& local_zone_name, diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index 2f0aff12ef43..257e498d750b 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -84,6 +84,11 @@ class AsyncRequestImpl final : public AsyncClient::Request, const std::string& runtimeKey() const override { return EMPTY_STRING; } }; + struct NullVirtualHost : public Router::VirtualHost { + // Router::VirtualHost + const std::string& name() const override { return EMPTY_STRING; } + }; + struct RouteEntryImpl : public Router::RouteEntry { RouteEntryImpl(const std::string& cluster_name, const Optional<std::chrono::milliseconds>& timeout) @@ -108,11 +113,12 @@ class AsyncRequestImpl final : public AsyncClient::Request, const Router::VirtualCluster* virtualCluster(const Http::HeaderMap&) const override { return nullptr; } - const std::string& virtualHostName() const { return EMPTY_STRING; } + const Router::VirtualHost& virtualHost() const override { return virtual_host_; } static const NullRateLimitPolicy rate_limit_policy_; static const NullRetryPolicy retry_policy_; static const NullShadowPolicy shadow_policy_; + static const NullVirtualHost virtual_host_; const std::string& cluster_name_; Optional<std::chrono::milliseconds> timeout_; diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 6f1ec9440bed..e34738d408d1 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -69,7 +69,7 @@ bool ConfigUtility::matchHeaders(const Http::HeaderMap& headers, return matches; } -RouteEntryImplBase::RouteEntryImplBase(const VirtualHost& vhost, const Json::Object& route, +RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, const Json::Object& route, Runtime::Loader& loader) : case_sensitive_(route.getBoolean("case_sensitive", true)), prefix_rewrite_(route.getString("prefix_rewrite", "")), @@ -169,7 +169,7 @@ std::string RouteEntryImplBase::newPath(const Http::HeaderMap& headers) const { final_path); } -PrefixRouteEntryImpl::PrefixRouteEntryImpl(const VirtualHost& vhost, const Json::Object& route, +PrefixRouteEntryImpl::PrefixRouteEntryImpl(const VirtualHostImpl& vhost, const Json::Object& route, Runtime::Loader& loader) : RouteEntryImplBase(vhost, route, loader), prefix_(route.getString("prefix")) {} @@ -184,7 +184,7 @@ bool PrefixRouteEntryImpl::matches(const Http::HeaderMap& headers, uint64_t rand StringUtil::startsWith(headers.Path()->value().c_str(), prefix_, case_sensitive_); } -PathRouteEntryImpl::PathRouteEntryImpl(const VirtualHost& vhost, const Json::Object& route, +PathRouteEntryImpl::PathRouteEntryImpl(const VirtualHostImpl& vhost, const Json::Object& route, Runtime::Loader& loader) : RouteEntryImplBase(vhost, route, loader), path_(route.getString("path")) {} @@ -211,8 +211,8 @@ bool PathRouteEntryImpl::matches(const Http::HeaderMap& headers, uint64_t random return false; } -VirtualHost::VirtualHost(const Json::Object& virtual_host, Runtime::Loader& runtime, - Upstream::ClusterManager& cm) +VirtualHostImpl::VirtualHostImpl(const Json::Object& virtual_host, Runtime::Loader& runtime, + Upstream::ClusterManager& cm) : name_(virtual_host.getString("name")) { std::string require_ssl = virtual_host.getString("require_ssl", ""); @@ -257,7 +257,7 @@ VirtualHost::VirtualHost(const Json::Object& virtual_host, Runtime::Loader& runt } } -bool VirtualHost::usesRuntime() const { +bool VirtualHostImpl::usesRuntime() const { bool uses = false; for (const RouteEntryImplBasePtr& route : routes_) { // Currently a base runtime rule as well as a shadow rule can use runtime. @@ -267,7 +267,7 @@ bool VirtualHost::usesRuntime() const { return uses; } -VirtualHost::VirtualClusterEntry::VirtualClusterEntry(const Json::Object& virtual_cluster) { +VirtualHostImpl::VirtualClusterEntry::VirtualClusterEntry(const Json::Object& virtual_cluster) { if (virtual_cluster.hasObject("method")) { method_ = virtual_cluster.getString("method"); } @@ -280,7 +280,7 @@ VirtualHost::VirtualClusterEntry::VirtualClusterEntry(const Json::Object& virtua RouteMatcher::RouteMatcher(const Json::Object& config, Runtime::Loader& runtime, Upstream::ClusterManager& cm) { for (const Json::ObjectPtr& virtual_host_config : config.getObjectArray("virtual_hosts")) { - VirtualHostPtr virtual_host(new VirtualHost(*virtual_host_config, runtime, cm)); + VirtualHostPtr virtual_host(new VirtualHostImpl(*virtual_host_config, runtime, cm)); uses_runtime_ |= virtual_host->usesRuntime(); for (const std::string& domain : virtual_host_config->getStringArray("domains")) { @@ -301,8 +301,8 @@ RouteMatcher::RouteMatcher(const Json::Object& config, Runtime::Loader& runtime, } } -const RedirectEntry* VirtualHost::redirectFromEntries(const Http::HeaderMap& headers, - uint64_t random_value) const { +const RedirectEntry* VirtualHostImpl::redirectFromEntries(const Http::HeaderMap& headers, + uint64_t random_value) const { // First we check to see if we have any vhost level SSL requirements. if (ssl_requirements_ == SslRequirements::ALL && headers.ForwardedProto()->value() != "https") { return &SSL_REDIRECTOR; @@ -316,9 +316,9 @@ const RedirectEntry* VirtualHost::redirectFromEntries(const Http::HeaderMap& hea } } -const RouteEntryImplBase* VirtualHost::routeFromEntries(const Http::HeaderMap& headers, - bool redirect, - uint64_t random_value) const { +const RouteEntryImplBase* VirtualHostImpl::routeFromEntries(const Http::HeaderMap& headers, + bool redirect, + uint64_t random_value) const { for (const RouteEntryImplBasePtr& route : routes_) { if (redirect == route->isRedirect() && route->matches(headers, random_value)) { return route.get(); @@ -328,7 +328,7 @@ const RouteEntryImplBase* VirtualHost::routeFromEntries(const Http::HeaderMap& h return nullptr; } -const VirtualHost* RouteMatcher::findVirtualHost(const Http::HeaderMap& headers) const { +const VirtualHostImpl* RouteMatcher::findVirtualHost(const Http::HeaderMap& headers) const { // Fast path the case where we only have a default virtual host. if (virtual_hosts_.empty() && default_virtual_host_) { return default_virtual_host_.get(); @@ -346,7 +346,7 @@ const VirtualHost* RouteMatcher::findVirtualHost(const Http::HeaderMap& headers) const RedirectEntry* RouteMatcher::redirectRequest(const Http::HeaderMap& headers, uint64_t random_value) const { - const VirtualHost* virtual_host = findVirtualHost(headers); + const VirtualHostImpl* virtual_host = findVirtualHost(headers); if (virtual_host) { return virtual_host->redirectFromEntries(headers, random_value); } else { @@ -356,7 +356,7 @@ const RedirectEntry* RouteMatcher::redirectRequest(const Http::HeaderMap& header const RouteEntry* RouteMatcher::routeForRequest(const Http::HeaderMap& headers, uint64_t random_value) const { - const VirtualHost* virtual_host = findVirtualHost(headers); + const VirtualHostImpl* virtual_host = findVirtualHost(headers); if (virtual_host) { return virtual_host->routeFromEntries(headers, false, random_value); } else { @@ -364,10 +364,11 @@ const RouteEntry* RouteMatcher::routeForRequest(const Http::HeaderMap& headers, } } -const VirtualHost::CatchAllVirtualCluster VirtualHost::VIRTUAL_CLUSTER_CATCH_ALL; -const SslRedirector VirtualHost::SSL_REDIRECTOR; +const VirtualHostImpl::CatchAllVirtualCluster VirtualHostImpl::VIRTUAL_CLUSTER_CATCH_ALL; +const SslRedirector VirtualHostImpl::SSL_REDIRECTOR; -const VirtualCluster* VirtualHost::virtualClusterFromEntries(const Http::HeaderMap& headers) const { +const VirtualCluster* +VirtualHostImpl::virtualClusterFromEntries(const Http::HeaderMap& headers) const { for (const VirtualClusterEntry& entry : virtual_clusters_) { bool method_matches = !entry.method_.valid() || headers.Method()->value().c_str() == entry.method_.value(); diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index 5484fcedf432..baac21ff3998 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -72,12 +72,11 @@ class ConfigUtility { /** * Holds all routing configuration for an entire virtual host. */ -class VirtualHost { +class VirtualHostImpl : public VirtualHost { public: - VirtualHost(const Json::Object& virtual_host, Runtime::Loader& runtime, - Upstream::ClusterManager& cm); + VirtualHostImpl(const Json::Object& virtual_host, Runtime::Loader& runtime, + Upstream::ClusterManager& cm); - const std::string& name() const { return name_; } const RedirectEntry* redirectFromEntries(const Http::HeaderMap& headers, uint64_t random_value) const; const RouteEntryImplBase* routeFromEntries(const Http::HeaderMap& headers, bool redirect, @@ -85,6 +84,9 @@ class VirtualHost { bool usesRuntime() const; const VirtualCluster* virtualClusterFromEntries(const Http::HeaderMap& headers) const; + // Router::VirtualHost + const std::string& name() const override { return name_; } + private: enum class SslRequirements { NONE, EXTERNAL_ONLY, ALL }; @@ -120,7 +122,7 @@ class VirtualHost { SslRequirements ssl_requirements_; }; -typedef std::shared_ptr<VirtualHost> VirtualHostPtr; +typedef std::shared_ptr<VirtualHostImpl> VirtualHostPtr; /** * Implementation of RetryPolicy that reads from the JSON route config. @@ -159,7 +161,8 @@ class ShadowPolicyImpl : public ShadowPolicy { */ class RouteEntryImplBase : public RouteEntry, public Matchable, public RedirectEntry { public: - RouteEntryImplBase(const VirtualHost& vhost, const Json::Object& route, Runtime::Loader& loader); + RouteEntryImplBase(const VirtualHostImpl& vhost, const Json::Object& route, + Runtime::Loader& loader); bool isRedirect() const { return !host_redirect_.empty() || !path_redirect_.empty(); } bool usesRuntime() const { return runtime_.valid(); } @@ -174,8 +177,8 @@ class RouteEntryImplBase : public RouteEntry, public Matchable, public RedirectE const VirtualCluster* virtualCluster(const Http::HeaderMap& headers) const override { return vhost_.virtualClusterFromEntries(headers); } - const std::string& virtualHostName() const override { return vhost_.name(); } std::chrono::milliseconds timeout() const override { return timeout_; } + const VirtualHost& virtualHost() const override { return vhost_; } // Router::RedirectEntry std::string newPath(const Http::HeaderMap& headers) const override; @@ -201,7 +204,7 @@ class RouteEntryImplBase : public RouteEntry, public Matchable, public RedirectE // Default timeout is 15s if nothing is specified in the route config. static const uint64_t DEFAULT_ROUTE_TIMEOUT_MS = 15000; - const VirtualHost& vhost_; + const VirtualHostImpl& vhost_; const std::string cluster_name_; const std::chrono::milliseconds timeout_; const Optional<RuntimeData> runtime_; @@ -220,7 +223,7 @@ class RouteEntryImplBase : public RouteEntry, public Matchable, public RedirectE */ class PrefixRouteEntryImpl : public RouteEntryImplBase { public: - PrefixRouteEntryImpl(const VirtualHost& vhost, const Json::Object& route, + PrefixRouteEntryImpl(const VirtualHostImpl& vhost, const Json::Object& route, Runtime::Loader& loader); // Router::RouteEntry @@ -238,7 +241,8 @@ class PrefixRouteEntryImpl : public RouteEntryImplBase { */ class PathRouteEntryImpl : public RouteEntryImplBase { public: - PathRouteEntryImpl(const VirtualHost& vhost, const Json::Object& route, Runtime::Loader& loader); + PathRouteEntryImpl(const VirtualHostImpl& vhost, const Json::Object& route, + Runtime::Loader& loader); // Router::RouteEntry void finalizeRequestHeaders(Http::HeaderMap& headers) const override; @@ -263,7 +267,7 @@ class RouteMatcher { bool usesRuntime() const { return uses_runtime_; } private: - const VirtualHost* findVirtualHost(const Http::HeaderMap& headers) const; + const VirtualHostImpl* findVirtualHost(const Http::HeaderMap& headers) const; std::unordered_map<std::string, VirtualHostPtr> virtual_hosts_; VirtualHostPtr default_virtual_host_; diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 0718c6edbab2..5e4a5e93f3fb 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -108,7 +108,7 @@ void Filter::chargeUpstreamCode(const Http::HeaderMap& response_headers, Http::CodeUtility::ResponseStatInfo info{ config_.stats_store_, cluster_->statPrefix(), response_headers, internal_request, - route_->virtualHostName(), request_vcluster_ ? request_vcluster_->name() : "", + route_->virtualHost().name(), request_vcluster_ ? request_vcluster_->name() : "", config_.service_zone_, upstreamZone(upstream_host), is_canary}; Http::CodeUtility::chargeResponseStat(info); @@ -480,7 +480,7 @@ void Filter::onUpstreamComplete() { Http::CodeUtility::ResponseTimingInfo info{ config_.stats_store_, cluster_->statPrefix(), response_time, - upstream_request_->upstream_canary_, internal_request, route_->virtualHostName(), + upstream_request_->upstream_canary_, internal_request, route_->virtualHost().name(), request_vcluster_ ? request_vcluster_->name() : "", config_.service_zone_, upstreamZone(upstream_request_->upstream_host_)}; diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 9d465a9cab47..aca68929f94f 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -182,7 +182,7 @@ TEST(RouteMatcherTest, TestRoutes) { Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/new_endpoint/foo", "GET"); const RouteEntry* route = config.routeForRequest(headers, 0); EXPECT_EQ("www2", route->clusterName()); - EXPECT_EQ("www2", route->virtualHostName()); + EXPECT_EQ("www2", route->virtualHost().name()); route->finalizeRequestHeaders(headers); EXPECT_EQ("/api/new_endpoint/foo", headers.get_(Http::Headers::get().Path)); } diff --git a/test/common/router/router_ratelimit_test.cc b/test/common/router/router_ratelimit_test.cc index 3d6e48d34bb2..b64ac5658a60 100644 --- a/test/common/router/router_ratelimit_test.cc +++ b/test/common/router/router_ratelimit_test.cc @@ -18,152 +18,66 @@ namespace Router { TEST(BadRateLimitConfiguration, MissingActions) { std::string json = R"EOF( { - "virtual_hosts": [ - { - "name": "www2", - "domains": ["www.lyft.com"], - "routes": [ - { - "prefix": "/", - "cluster": "www2", - "rate_limits": [{}] - } - ] - } - ] + "rate_limits": [{}] } )EOF"; - Json::ObjectPtr loader = Json::Factory::LoadFromString(json); - NiceMock<Runtime::MockLoader> runtime; - NiceMock<Upstream::MockClusterManager> cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(RateLimitPolicyImpl(*Json::Factory::LoadFromString(json)), EnvoyException); } TEST(BadRateLimitConfiguration, BadType) { std::string json = R"EOF( { - "virtual_hosts": [ + "actions":[ { - "name": "www2", - "domains": ["www.lyft.com"], - "routes": [ - { - "prefix": "/", - "cluster": "www2", - "rate_limits": [ - { - "actions":[ - { - "type": "bad_type" - } - ] - } - ] - } - ] + "type": "bad_type" } ] } )EOF"; - Json::ObjectPtr loader = Json::Factory::LoadFromString(json); - NiceMock<Runtime::MockLoader> runtime; - NiceMock<Upstream::MockClusterManager> cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(RateLimitPolicyEntryImpl(*Json::Factory::LoadFromString(json)), EnvoyException); } TEST(BadRateLimitConfiguration, ActionsMissingRequiredFields) { std::string json_one = R"EOF( { - "virtual_hosts": [ + "actions":[ { - "name": "www2", - "domains": ["www.lyft.com"], - "routes": [ - { - "prefix": "/", - "cluster": "www2", - "rate_limits": [ - { - "actions":[ - { - "type": "request_headers" - } - ] - } - ] - } - ] + "type": "request_headers" } ] } )EOF"; - Json::ObjectPtr loader = Json::Factory::LoadFromString(json_one); - NiceMock<Runtime::MockLoader> runtime; - NiceMock<Upstream::MockClusterManager> cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(RateLimitPolicyEntryImpl(*Json::Factory::LoadFromString(json_one)), EnvoyException); std::string json_two = R"EOF( { - "virtual_hosts": [ + "actions":[ { - "name": "www2", - "domains": ["www.lyft.com"], - "routes": [ - { - "prefix": "/", - "cluster": "www2", - "rate_limits": [ - { - "actions":[ - { - "type": "request_headers", - "header_name" : "test" - } - ] - } - ] - } - ] + "type": "request_headers", + "header_name" : "test" } ] } )EOF"; - loader = Json::Factory::LoadFromString(json_two); - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(RateLimitPolicyEntryImpl(*Json::Factory::LoadFromString(json_two)), EnvoyException); std::string json_three = R"EOF( { - "virtual_hosts": [ + "actions":[ { - "name": "www2", - "domains": ["www.lyft.com"], - "routes": [ - { - "prefix": "/", - "cluster": "www2", - "rate_limits": [ - { - "actions":[ - { - "type": "request_headers", - "descriptor_key" : "test" - } - ] - } - ] - } - ] + "type": "request_headers", + "descriptor_key" : "test" } ] } )EOF"; - loader = Json::Factory::LoadFromString(json_three); - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(RateLimitPolicyEntryImpl(*Json::Factory::LoadFromString(json_three)), + EnvoyException); } static Http::TestHeaderMapImpl genHeaders(const std::string& host, const std::string& path, @@ -224,7 +138,7 @@ TEST_F(RateLimitConfiguration, NoRateLimit) { .size()); } -TEST_F(RateLimitConfiguration, RemoteAddress) { +TEST_F(RateLimitConfiguration, TestGetApplicationRateLimit) { std::string json = R"EOF( { "virtual_hosts": [ @@ -267,29 +181,47 @@ TEST_F(RateLimitConfiguration, RemoteAddress) { testing::ContainerEq(descriptors)); } -TEST_F(RateLimitConfiguration, RemoteAddressRouteKey) { +class RateLimitPolicyEntryTest : public testing::Test { +public: + void SetUpTest(const std::string json) { + Json::ObjectPtr loader = Json::Factory::LoadFromString(json); + rate_limit_entry_.reset(new RateLimitPolicyEntryImpl(*loader)); + descriptors_.clear(); + } + + std::unique_ptr<RateLimitPolicyEntryImpl> rate_limit_entry_; + Http::TestHeaderMapImpl header_; + NiceMock<MockRouteEntry> route_; + std::vector<::RateLimit::Descriptor> descriptors_; +}; + +TEST_F(RateLimitPolicyEntryTest, RateLimitPolicyEntryMembers) { std::string json = R"EOF( { - "virtual_hosts": [ + "stage": 2, + "kill_switch_key": "no_ratelimit", + "route_key": "my_route", + "actions":[ { - "name": "www2", - "domains": ["www.lyft.com"], - "routes": [ - { - "prefix": "/", - "cluster": "www2", - "rate_limits": [ - { - "route_key": "my_route", - "actions":[ - { - "type": "remote_address" - } - ] - } - ] - } - ] + "type": "remote_address" + } + ] +} +)EOF"; + + SetUpTest(json); + + EXPECT_EQ(2, rate_limit_entry_->stage()); + EXPECT_EQ("no_ratelimit", rate_limit_entry_->killSwitchKey()); + EXPECT_EQ("my_route", rate_limit_entry_->routeKey()); +} + +TEST_F(RateLimitPolicyEntryTest, RemoteAddress) { + std::string json = R"EOF( +{ + "actions":[ + { + "type": "remote_address" } ] } @@ -298,43 +230,39 @@ TEST_F(RateLimitConfiguration, RemoteAddressRouteKey) { SetUpTest(json); std::string address = "10.0.0.1"; - route_ = config_->routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0); - std::vector<std::reference_wrapper<const RateLimitPolicyEntry>> rate_limits = - route_->rateLimitPolicy().getApplicableRateLimit(0); - EXPECT_EQ(1U, rate_limits.size()); + rate_limit_entry_->populateDescriptors(route_, descriptors_, "", header_, address); + EXPECT_THAT(std::vector<::RateLimit::Descriptor>({{{{"remote_address", address}}}}), + testing::ContainerEq(descriptors_)); +} - std::vector<::RateLimit::Descriptor> descriptors; - for (const RateLimitPolicyEntry& rate_limit : rate_limits) { - rate_limit.populateDescriptors(*route_, descriptors, "", header_, address); - } +TEST_F(RateLimitPolicyEntryTest, RemoteAddressRouteKey) { + std::string json = R"EOF( +{ + "route_key": "my_route", + "actions":[ + { + "type": "remote_address" + } + ] +} +)EOF"; + + SetUpTest(json); + std::string address = "10.0.1"; + + rate_limit_entry_->populateDescriptors(route_, descriptors_, "", header_, address); EXPECT_THAT(std::vector<::RateLimit::Descriptor>( {{{{"remote_address", address}}}, {{{"route_key", "my_route"}, {"remote_address", address}}}}), - testing::ContainerEq(descriptors)); + testing::ContainerEq(descriptors_)); } -TEST_F(RateLimitConfiguration, NoAddress) { +TEST_F(RateLimitPolicyEntryTest, NoAddress) { std::string json = R"EOF( { - "virtual_hosts": [ + "actions":[ { - "name": "www2", - "domains": ["www.lyft.com"], - "routes": [ - { - "prefix": "/", - "cluster": "www2", - "rate_limits": [ - { - "actions":[ - { - "type": "remote_address" - } - ] - } - ] - } - ] + "type": "remote_address" } ] } @@ -342,40 +270,16 @@ TEST_F(RateLimitConfiguration, NoAddress) { SetUpTest(json); - route_ = config_->routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0); - std::vector<std::reference_wrapper<const RateLimitPolicyEntry>> rate_limits = - route_->rateLimitPolicy().getApplicableRateLimit(0); - EXPECT_EQ(1U, rate_limits.size()); - - std::vector<::RateLimit::Descriptor> descriptors; - for (const RateLimitPolicyEntry& rate_limit : rate_limits) { - rate_limit.populateDescriptors(*route_, descriptors, "", header_, ""); - } - EXPECT_TRUE(descriptors.empty()); + rate_limit_entry_->populateDescriptors(route_, descriptors_, "", header_, ""); + EXPECT_TRUE(descriptors_.empty()); } -TEST_F(RateLimitConfiguration, ServiceToService) { +TEST_F(RateLimitPolicyEntryTest, ServiceToService) { std::string json = R"EOF( { - "virtual_hosts": [ + "actions":[ { - "name": "www2", - "domains": ["www.lyft.com"], - "routes": [ - { - "prefix": "/foo", - "cluster": "fake_cluster", - "rate_limits": [ - { - "actions":[ - { - "type": "service_to_service" - } - ] - } - ] - } - ] + "type": "service_to_service" } ] } @@ -383,45 +287,21 @@ TEST_F(RateLimitConfiguration, ServiceToService) { SetUpTest(json); - route_ = config_->routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0); - std::vector<std::reference_wrapper<const RateLimitPolicyEntry>> rate_limits = - route_->rateLimitPolicy().getApplicableRateLimit(0); - EXPECT_EQ(1U, rate_limits.size()); - - std::vector<::RateLimit::Descriptor> descriptors; - for (const RateLimitPolicyEntry& rate_limit : rate_limits) { - rate_limit.populateDescriptors(*route_, descriptors, "service_cluster", header_, ""); - } + rate_limit_entry_->populateDescriptors(route_, descriptors_, "service_cluster", header_, ""); EXPECT_THAT(std::vector<::RateLimit::Descriptor>( {{{{"to_cluster", "fake_cluster"}}}, {{{"to_cluster", "fake_cluster"}, {"from_cluster", "service_cluster"}}}}), - testing::ContainerEq(descriptors)); + testing::ContainerEq(descriptors_)); } -TEST_F(RateLimitConfiguration, RequestHeaders) { +TEST_F(RateLimitPolicyEntryTest, RequestHeaders) { std::string json = R"EOF( { - "virtual_hosts": [ + "actions": [ { - "name": "www2", - "domains": ["www.lyft.com"], - "routes": [ - { - "prefix": "/", - "cluster": "www2", - "rate_limits": [ - { - "actions": [ - { - "type": "request_headers", - "header_name": "x-header-name", - "descriptor_key" : "my_header_name" - } - ] - } - ] - } - ] + "type": "request_headers", + "header_name": "x-header-name", + "descriptor_key" : "my_header_name" } ] } @@ -430,44 +310,19 @@ TEST_F(RateLimitConfiguration, RequestHeaders) { SetUpTest(json); Http::TestHeaderMapImpl header{{"x-header-name", "test_value"}}; - route_ = config_->routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0); - std::vector<std::reference_wrapper<const RateLimitPolicyEntry>> rate_limits = - route_->rateLimitPolicy().getApplicableRateLimit(0); - EXPECT_EQ(1U, rate_limits.size()); - - std::vector<::RateLimit::Descriptor> descriptors; - for (const RateLimitPolicyEntry& rate_limit : rate_limits) { - rate_limit.populateDescriptors(*route_, descriptors, "service_cluster", header, ""); - } + rate_limit_entry_->populateDescriptors(route_, descriptors_, "service_cluster", header, ""); EXPECT_THAT(std::vector<::RateLimit::Descriptor>({{{{"my_header_name", "test_value"}}}}), - testing::ContainerEq(descriptors)); + testing::ContainerEq(descriptors_)); } - -TEST_F(RateLimitConfiguration, RequestHeadersRouteKey) { +TEST_F(RateLimitPolicyEntryTest, RequestHeadersRouteKey) { std::string json = R"EOF( { - "virtual_hosts": [ + "route_key": "my_route", + "actions": [ { - "name": "www2", - "domains": ["www.lyft.com"], - "routes": [ - { - "prefix": "/", - "cluster": "www2", - "rate_limits": [ - { - "route_key": "my_route", - "actions": [ - { - "type": "request_headers", - "header_name": "x-header-name", - "descriptor_key" : "my_header_name" - } - ] - } - ] - } - ] + "type": "request_headers", + "header_name": "x-header-name", + "descriptor_key" : "my_header_name" } ] } @@ -476,45 +331,21 @@ TEST_F(RateLimitConfiguration, RequestHeadersRouteKey) { SetUpTest(json); Http::TestHeaderMapImpl header{{"x-header-name", "test_value"}}; - route_ = config_->routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0); - std::vector<std::reference_wrapper<const RateLimitPolicyEntry>> rate_limits = - route_->rateLimitPolicy().getApplicableRateLimit(0); - EXPECT_EQ(1U, rate_limits.size()); - - std::vector<::RateLimit::Descriptor> descriptors; - for (const RateLimitPolicyEntry& rate_limit : rate_limits) { - rate_limit.populateDescriptors(*route_, descriptors, "service_cluster", header, ""); - } + rate_limit_entry_->populateDescriptors(route_, descriptors_, "service_cluster", header, ""); EXPECT_THAT(std::vector<::RateLimit::Descriptor>( {{{{"my_header_name", "test_value"}}}, {{{"route_key", "my_route"}, {"my_header_name", "test_value"}}}}), - testing::ContainerEq(descriptors)); + testing::ContainerEq(descriptors_)); } -TEST_F(RateLimitConfiguration, RequestHeadersNoMatch) { +TEST_F(RateLimitPolicyEntryTest, RequestHeadersNoMatch) { std::string json = R"EOF( { - "virtual_hosts": [ + "actions": [ { - "name": "www2", - "domains": ["www.lyft.com"], - "routes": [ - { - "prefix": "/", - "cluster": "www2", - "rate_limits": [ - { - "actions": [ - { - "type": "request_headers", - "header_name": "x-header", - "descriptor_key" : "my_header_name" - } - ] - } - ] - } - ] + "type": "request_headers", + "header_name": "x-header", + "descriptor_key" : "my_header_name" } ] } @@ -522,16 +353,9 @@ TEST_F(RateLimitConfiguration, RequestHeadersNoMatch) { SetUpTest(json); Http::TestHeaderMapImpl header{{"x-header-name", "test_value"}}; - route_ = config_->routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0); - std::vector<std::reference_wrapper<const RateLimitPolicyEntry>> rate_limits = - route_->rateLimitPolicy().getApplicableRateLimit(0); - EXPECT_EQ(1U, rate_limits.size()); - std::vector<::RateLimit::Descriptor> descriptors; - for (const RateLimitPolicyEntry& rate_limit : rate_limits) { - rate_limit.populateDescriptors(*route_, descriptors, "service_cluster", header, ""); - } - EXPECT_TRUE(descriptors.empty()); + rate_limit_entry_->populateDescriptors(route_, descriptors_, "service_cluster", header, ""); + EXPECT_TRUE(descriptors_.empty()); } } // Router \ No newline at end of file diff --git a/test/mocks/router/mocks.cc b/test/mocks/router/mocks.cc index 4b2935e0e80f..df0d3a4cfeed 100644 --- a/test/mocks/router/mocks.cc +++ b/test/mocks/router/mocks.cc @@ -40,7 +40,7 @@ MockRouteEntry::MockRouteEntry() { ON_CALL(*this, shadowPolicy()).WillByDefault(ReturnRef(shadow_policy_)); ON_CALL(*this, timeout()).WillByDefault(Return(std::chrono::milliseconds(10))); ON_CALL(*this, virtualCluster(_)).WillByDefault(Return(&virtual_cluster_)); - ON_CALL(*this, virtualHostName()).WillByDefault(ReturnRef(vhost_name_)); + ON_CALL(*this, virtualHost()).WillByDefault(ReturnRef(virtual_host_)); } MockRouteEntry::~MockRouteEntry() {} diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index e62de44b21e3..468d77247e55 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -111,6 +111,14 @@ class TestVirtualCluster : public VirtualCluster { Upstream::ResourcePriority priority_{Upstream::ResourcePriority::Default}; }; +class TestVirtualHost : public VirtualHost { +public: + // Router::VirtualHost + const std::string& name() const override { return name_; } + + std::string name_{"fake_vhost"}; +}; + class MockRouteEntry : public RouteEntry { public: MockRouteEntry(); @@ -126,13 +134,14 @@ class MockRouteEntry : public RouteEntry { MOCK_CONST_METHOD0(timeout, std::chrono::milliseconds()); MOCK_CONST_METHOD1(virtualCluster, const VirtualCluster*(const Http::HeaderMap& headers)); MOCK_CONST_METHOD0(virtualHostName, const std::string&()); + MOCK_CONST_METHOD0(virtualHost, const VirtualHost&()); std::string cluster_name_{"fake_cluster"}; - std::string vhost_name_{"fake_vhost"}; TestVirtualCluster virtual_cluster_; TestRetryPolicy retry_policy_; testing::NiceMock<MockRateLimitPolicy> rate_limit_policy_; TestShadowPolicy shadow_policy_; + TestVirtualHost virtual_host_; }; class MockConfig : public Config {