diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index b338e04858fe..5f1c7aa41917 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -201,32 +201,39 @@ class RouteEntry { }; /** - * The router configuration. + * An interface that holds a RedirectEntry or a RouteEntry for a request. */ -class Config { +class Route { public: - virtual ~Config() {} + virtual ~Route() {} /** - * Based on the incoming HTTP request headers, determine whether a redirect should take place. - * @param headers supplies the request headers. - * @param random_value supplies the random seed to use if a runtime choice is required. This - * allows stable choices between calls if desired. * @return the redirect entry or nullptr if there is no redirect needed for the request. */ - virtual const RedirectEntry* redirectRequest(const Http::HeaderMap& headers, - uint64_t random_value) const PURE; + virtual const RedirectEntry* redirectEntry() const PURE; + + /** + * @return the route entry or nullptr if there is no matching route for the request. + */ + virtual const RouteEntry* routeEntry() const PURE; +}; + +/** + * The router configuration. + */ +class Config { +public: + virtual ~Config() {} /** - * Based on the incoming HTTP request headers, choose the target route to send the remainder - * of the request to. + * Based on the incoming HTTP request headers, determine the target route (containing either a + * route entry or a redirect entry) for the request. * @param headers supplies the request headers. * @param random_value supplies the random seed to use if a runtime choice is required. This * allows stable choices between calls if desired. * @return the route or nullptr if there is no matching route for the request. */ - virtual const RouteEntry* routeForRequest(const Http::HeaderMap& headers, - uint64_t random_value) const PURE; + virtual const Route* route(const Http::HeaderMap& headers, uint64_t random_value) const PURE; /** * Return a list of headers that will be cleaned from any requests that are not from an internal @@ -266,19 +273,12 @@ class StableRouteTable { virtual ~StableRouteTable() {} /** - * Based on the incoming HTTP request headers, determine whether a redirect should take place. - * @param headers supplies the request headers. - * @return the redirect entry or nullptr if there is no redirect needed for the request. - */ - virtual const RedirectEntry* redirectRequest(const Http::HeaderMap& headers) const PURE; - - /** - * Based on the incoming HTTP request headers, choose the target route to send the remainder - * of the request to. + * Based on the incoming HTTP request headers, determine the target route (containing either a + * route entry or a redirect entry) for the request. * @param headers supplies the request headers. * @return the route or nullptr if there is no matching route for the request. */ - virtual const RouteEntry* routeForRequest(const Http::HeaderMap& headers) const PURE; + virtual const Route* route(const Http::HeaderMap& headers) const PURE; }; } // Router diff --git a/source/common/grpc/http1_bridge_filter.cc b/source/common/grpc/http1_bridge_filter.cc index ab95dddce1d5..eb9b8d5ae34b 100644 --- a/source/common/grpc/http1_bridge_filter.cc +++ b/source/common/grpc/http1_bridge_filter.cc @@ -95,18 +95,23 @@ Http::FilterTrailersStatus Http1BridgeFilter::encodeTrailers(Http::HeaderMap& tr } void Http1BridgeFilter::setupStatTracking(const Http::HeaderMap& headers) { - const Router::RouteEntry* route = decoder_callbacks_->routeTable().routeForRequest(headers); + const Router::Route* route = decoder_callbacks_->routeTable().route(headers); if (!route) { return; } + const Router::RouteEntry* route_entry = route->routeEntry(); + if (!route_entry) { + return; + } + std::vector parts = StringUtil::split(headers.Path()->value().c_str(), '/'); if (parts.size() != 2) { return; } // TODO: Cluster may not exist. - cluster_ = cm_.get(route->clusterName()); + cluster_ = cm_.get(route_entry->clusterName()); grpc_service_ = parts[0]; grpc_method_ = parts[1]; do_stat_tracking_ = true; diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index ae9b6aade4e7..6571d7a96a51 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -125,6 +125,17 @@ class AsyncRequestImpl final : public AsyncClient::Request, Optional timeout_; }; + struct RouteImpl : public Router::Route { + RouteImpl(const std::string& cluster_name, const Optional& timeout) + : route_entry_(cluster_name, timeout) {} + + // Router::Route + const Router::RedirectEntry* redirectEntry() const override { return nullptr; } + const Router::RouteEntry* routeEntry() const override { return &route_entry_; } + + RouteEntryImpl route_entry_; + }; + void cleanup(); void failDueToClientDestroy(); void onComplete(); @@ -147,12 +158,7 @@ class AsyncRequestImpl final : public AsyncClient::Request, void encodeTrailers(HeaderMapPtr&& trailers) override; // Router::StableRouteTable - const Router::RedirectEntry* redirectRequest(const Http::HeaderMap&) const override { - return nullptr; - } - const Router::RouteEntry* routeForRequest(const Http::HeaderMap&) const override { - return &route_; - } + const Router::Route* route(const Http::HeaderMap&) const override { return &route_; } MessagePtr request_; AsyncClientImpl& parent_; @@ -162,7 +168,7 @@ class AsyncRequestImpl final : public AsyncClient::Request, Router::ProdFilter router_; std::function reset_callback_; AccessLog::RequestInfoImpl request_info_; - RouteEntryImpl route_; + RouteImpl route_; bool complete_{}; friend class AsyncClientImpl; diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index f74967839a55..9f9cde6a02d3 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -251,13 +251,8 @@ class ConnectionManagerImpl : Logger::Loggable, const std::string& downstreamAddress() override; // Router::StableRouteTable - const Router::RedirectEntry* redirectRequest(const HeaderMap& headers) const { - return parent_.connection_manager_.config_.routeConfig().redirectRequest(headers, - parent_.stream_id_); - } - const Router::RouteEntry* routeForRequest(const HeaderMap& headers) const { - return parent_.connection_manager_.config_.routeConfig().routeForRequest(headers, - parent_.stream_id_); + const Router::Route* route(const HeaderMap& headers) const { + return parent_.connection_manager_.config_.routeConfig().route(headers, parent_.stream_id_); } ActiveStream& parent_; diff --git a/source/common/http/filter/ratelimit.cc b/source/common/http/filter/ratelimit.cc index a883144a6f39..bbcf02c4d7c4 100644 --- a/source/common/http/filter/ratelimit.cc +++ b/source/common/http/filter/ratelimit.cc @@ -19,19 +19,21 @@ FilterHeadersStatus Filter::decodeHeaders(HeaderMap& headers, bool) { return FilterHeadersStatus::Continue; } - const Router::RouteEntry* route = callbacks_->routeTable().routeForRequest(headers); - if (route) { + const Router::Route* route = callbacks_->routeTable().route(headers); + if (route && route->routeEntry()) { + const Router::RouteEntry* route_entry = route->routeEntry(); + // TODO: Cluster may not exist. - cluster_ = config_->cm().get(route->clusterName()); + cluster_ = config_->cm().get(route_entry->clusterName()); std::vector<::RateLimit::Descriptor> descriptors; // Get all applicable rate limit policy entries for the route. - populateRateLimitDescriptors(route->rateLimitPolicy(), descriptors, route, headers); + populateRateLimitDescriptors(route_entry->rateLimitPolicy(), descriptors, route_entry, headers); // Get all applicable rate limit policy entries for the virtual host. - populateRateLimitDescriptors(route->virtualHost().rateLimitPolicy(), descriptors, route, - headers); + populateRateLimitDescriptors(route_entry->virtualHost().rateLimitPolicy(), descriptors, + route_entry, headers); if (!descriptors.empty()) { state_ = State::Calling; diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index b729a219a650..65c7560f28da 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -175,6 +175,24 @@ std::string RouteEntryImplBase::newPath(const Http::HeaderMap& headers) const { final_path); } +const RedirectEntry* RouteEntryImplBase::redirectEntry() const { + // A route for a request can exclusively be a route entry or a redirect entry. + if (isRedirect()) { + return this; + } else { + return nullptr; + } +} + +const RouteEntry* RouteEntryImplBase::routeEntry() const { + // A route for a request can exclusively be a route entry or a redirect entry. + if (isRedirect()) { + return nullptr; + } else { + return this; + } +} + PrefixRouteEntryImpl::PrefixRouteEntryImpl(const VirtualHostImpl& vhost, const Json::Object& route, Runtime::Loader& loader) : RouteEntryImplBase(vhost, route, loader), prefix_(route.getString("prefix")) {} @@ -307,26 +325,19 @@ RouteMatcher::RouteMatcher(const Json::Object& config, Runtime::Loader& runtime, } } -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. +const Route* VirtualHostImpl::getRouteFromEntries(const Http::HeaderMap& headers, + uint64_t random_value) const { + // First check for ssl redirect. if (ssl_requirements_ == SslRequirements::ALL && headers.ForwardedProto()->value() != "https") { - return &SSL_REDIRECTOR; + return &SSL_REDIRECT_ROUTE; } else if (ssl_requirements_ == SslRequirements::EXTERNAL_ONLY && headers.ForwardedProto()->value() != "https" && !headers.EnvoyInternalRequest()) { - return &SSL_REDIRECTOR; - } else { - // See if there is a route level redirect that we need to do. We search for a route entry - // and see if it has redirect information on it. - return routeFromEntries(headers, true, random_value); + return &SSL_REDIRECT_ROUTE; } -} -const RouteEntryImplBase* VirtualHostImpl::routeFromEntries(const Http::HeaderMap& headers, - bool redirect, - uint64_t random_value) const { + // Check for a route that matches the request. for (const RouteEntryImplBasePtr& route : routes_) { - if (redirect == route->isRedirect() && route->matches(headers, random_value)) { + if (route->matches(headers, random_value)) { return route.get(); } } @@ -350,28 +361,18 @@ const VirtualHostImpl* RouteMatcher::findVirtualHost(const Http::HeaderMap& head return nullptr; } -const RedirectEntry* RouteMatcher::redirectRequest(const Http::HeaderMap& headers, - uint64_t random_value) const { - const VirtualHostImpl* virtual_host = findVirtualHost(headers); - if (virtual_host) { - return virtual_host->redirectFromEntries(headers, random_value); - } else { - return nullptr; - } -} - -const RouteEntry* RouteMatcher::routeForRequest(const Http::HeaderMap& headers, - uint64_t random_value) const { +const Route* RouteMatcher::route(const Http::HeaderMap& headers, uint64_t random_value) const { const VirtualHostImpl* virtual_host = findVirtualHost(headers); if (virtual_host) { - return virtual_host->routeFromEntries(headers, false, random_value); + return virtual_host->getRouteFromEntries(headers, random_value); } else { return nullptr; } } const VirtualHostImpl::CatchAllVirtualCluster VirtualHostImpl::VIRTUAL_CLUSTER_CATCH_ALL; -const SslRedirector VirtualHostImpl::SSL_REDIRECTOR; +const SslRedirector SslRedirectRoute::SSL_REDIRECTOR; +const SslRedirectRoute VirtualHostImpl::SSL_REDIRECT_ROUTE; const VirtualCluster* VirtualHostImpl::virtualClusterFromEntries(const Http::HeaderMap& headers) const { diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index abcd7f90721d..52bf8b66fa89 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -40,6 +40,16 @@ class SslRedirector : public RedirectEntry { std::string newPath(const Http::HeaderMap& headers) const override; }; +class SslRedirectRoute : public Route { +public: + // Router::Route + const RedirectEntry* redirectEntry() const override { return &SSL_REDIRECTOR; } + const RouteEntry* routeEntry() const override { return nullptr; } + +private: + static const SslRedirector SSL_REDIRECTOR; +}; + /** * Utility routines for loading route configuration and matching runtime request headers. */ @@ -80,10 +90,7 @@ class VirtualHostImpl : public VirtualHost { VirtualHostImpl(const Json::Object& virtual_host, Runtime::Loader& runtime, Upstream::ClusterManager& cm); - const RedirectEntry* redirectFromEntries(const Http::HeaderMap& headers, - uint64_t random_value) const; - const RouteEntryImplBase* routeFromEntries(const Http::HeaderMap& headers, bool redirect, - uint64_t random_value) const; + const Route* getRouteFromEntries(const Http::HeaderMap& headers, uint64_t random_value) const; bool usesRuntime() const; const VirtualCluster* virtualClusterFromEntries(const Http::HeaderMap& headers) const; @@ -118,7 +125,7 @@ class VirtualHostImpl : public VirtualHost { }; static const CatchAllVirtualCluster VIRTUAL_CLUSTER_CATCH_ALL; - static const SslRedirector SSL_REDIRECTOR; + static const SslRedirectRoute SSL_REDIRECT_ROUTE; const std::string name_; std::vector routes_; @@ -164,7 +171,7 @@ class ShadowPolicyImpl : public ShadowPolicy { /** * Base implementation for all route entries. */ -class RouteEntryImplBase : public RouteEntry, public Matchable, public RedirectEntry { +class RouteEntryImplBase : public RouteEntry, public Matchable, public RedirectEntry, public Route { public: RouteEntryImplBase(const VirtualHostImpl& vhost, const Json::Object& route, Runtime::Loader& loader); @@ -191,6 +198,10 @@ class RouteEntryImplBase : public RouteEntry, public Matchable, public RedirectE // Router::Matchable bool matches(const Http::HeaderMap& headers, uint64_t random_value) const override; + // Router::Route + const RedirectEntry* redirectEntry() const override; + const RouteEntry* routeEntry() const override; + protected: const bool case_sensitive_; const std::string prefix_rewrite_; @@ -267,8 +278,7 @@ class RouteMatcher { public: RouteMatcher(const Json::Object& config, Runtime::Loader& runtime, Upstream::ClusterManager& cm); - const RedirectEntry* redirectRequest(const Http::HeaderMap& headers, uint64_t random_value) const; - const RouteEntry* routeForRequest(const Http::HeaderMap& headers, uint64_t random_value) const; + const Route* route(const Http::HeaderMap& headers, uint64_t random_value) const; bool usesRuntime() const { return uses_runtime_; } private: @@ -287,14 +297,8 @@ class ConfigImpl : public Config { ConfigImpl(const Json::Object& config, Runtime::Loader& runtime, Upstream::ClusterManager& cm); // Router::Config - const RedirectEntry* redirectRequest(const Http::HeaderMap& headers, - uint64_t random_value) const override { - return route_matcher_->redirectRequest(headers, random_value); - } - - const RouteEntry* routeForRequest(const Http::HeaderMap& headers, - uint64_t random_value) const override { - return route_matcher_->routeForRequest(headers, random_value); + const Route* route(const Http::HeaderMap& headers, uint64_t random_value) const override { + return route_matcher_->route(headers, random_value); } const std::list& internalOnlyHeaders() const override { @@ -325,13 +329,7 @@ class ConfigImpl : public Config { class NullConfigImpl : public Config { public: // Router::Config - const RedirectEntry* redirectRequest(const Http::HeaderMap&, uint64_t) const override { - return nullptr; - } - - const RouteEntry* routeForRequest(const Http::HeaderMap&, uint64_t) const override { - return nullptr; - } + const Route* route(const Http::HeaderMap&, uint64_t) const override { return nullptr; } const std::list& internalOnlyHeaders() const override { return internal_only_headers_; diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 21e49092c8da..ff6379323c49 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_.global_store_, cluster_->statsScope(), EMPTY_STRING, response_headers, - internal_request, route_->virtualHost().name(), + internal_request, route_entry_->virtualHost().name(), request_vcluster_ ? request_vcluster_->name() : EMPTY_STRING, config_.local_info_.zoneName(), upstreamZone(upstream_host), is_canary}; @@ -138,17 +138,9 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e // that get handled by earlier filters. config_.stats_.rq_total_.inc(); - // First determine if we need to do a redirect before we do anything else. - const RedirectEntry* redirect = callbacks_->routeTable().redirectRequest(headers); - if (redirect) { - config_.stats_.rq_redirect_.inc(); - Http::Utility::sendRedirect(*callbacks_, redirect->newPath(headers)); - return Http::FilterHeadersStatus::StopIteration; - } - - // Determine if there is a route match. - route_ = callbacks_->routeTable().routeForRequest(headers); - if (!route_) { + // Determine if there is a route entry or a redirect for the request. + const Route* route = callbacks_->routeTable().route(headers); + if (!route) { config_.stats_.no_route_.inc(); stream_log_debug("no cluster match for URL '{}'", *callbacks_, headers.Path()->value().c_str()); @@ -159,12 +151,22 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e return Http::FilterHeadersStatus::StopIteration; } + // Determine if there is a redirect for the request. + if (route->redirectEntry()) { + config_.stats_.rq_redirect_.inc(); + Http::Utility::sendRedirect(*callbacks_, route->redirectEntry()->newPath(headers)); + return Http::FilterHeadersStatus::StopIteration; + } + + // A route entry matches for the request. + route_entry_ = route->routeEntry(); + // Set up stat prefixes, etc. - request_vcluster_ = route_->virtualCluster(headers); - stream_log_debug("cluster '{}' match for URL '{}'", *callbacks_, route_->clusterName(), + request_vcluster_ = route_entry_->virtualCluster(headers); + stream_log_debug("cluster '{}' match for URL '{}'", *callbacks_, route_entry_->clusterName(), headers.Path()->value().c_str()); - cluster_ = config_.cm_.get(route_->clusterName()); + cluster_ = config_.cm_.get(route_entry_->clusterName()); const Http::HeaderEntry* request_alt_name = headers.EnvoyUpstreamAltStatName(); if (request_alt_name) { @@ -182,19 +184,19 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e // Fetch a connection pool for the upstream cluster. Http::ConnectionPool::Instance* conn_pool = - config_.cm_.httpConnPoolForCluster(route_->clusterName(), finalPriority()); + config_.cm_.httpConnPoolForCluster(route_entry_->clusterName(), finalPriority()); if (!conn_pool) { sendNoHealthyUpstreamResponse(); return Http::FilterHeadersStatus::StopIteration; } - timeout_ = FilterUtility::finalTimeout(*route_, headers); - route_->finalizeRequestHeaders(headers); + timeout_ = FilterUtility::finalTimeout(*route_entry_, headers); + route_entry_->finalizeRequestHeaders(headers); FilterUtility::setUpstreamScheme(headers, *cluster_); - retry_state_ = createRetryState(route_->retryPolicy(), headers, *cluster_, config_.runtime_, + retry_state_ = createRetryState(route_entry_->retryPolicy(), headers, *cluster_, config_.runtime_, config_.random_, callbacks_->dispatcher(), finalPriority()); - do_shadowing_ = - FilterUtility::shouldShadow(route_->shadowPolicy(), config_.runtime_, callbacks_->streamId()); + do_shadowing_ = FilterUtility::shouldShadow(route_entry_->shadowPolicy(), config_.runtime_, + callbacks_->streamId()); #ifndef NDEBUG headers.iterate([](const Http::HeaderEntry& header, void* context) -> void { @@ -267,7 +269,7 @@ Upstream::ResourcePriority Filter::finalPriority() { if (request_vcluster_) { return request_vcluster_->priority(); } else { - return route_->priority(); + return route_entry_->priority(); } } @@ -276,7 +278,7 @@ void Filter::maybeDoShadowing() { return; } - ASSERT(!route_->shadowPolicy().cluster().empty()); + ASSERT(!route_entry_->shadowPolicy().cluster().empty()); Http::MessagePtr request(new Http::RequestMessageImpl( Http::HeaderMapPtr{new Http::HeaderMapImpl(*downstream_headers_)})); if (callbacks_->decodingBuffer()) { @@ -286,7 +288,7 @@ void Filter::maybeDoShadowing() { request->trailers(Http::HeaderMapPtr{new Http::HeaderMapImpl(*downstream_trailers_)}); } - config_.shadowWriter().shadow(route_->shadowPolicy().cluster(), std::move(request), + config_.shadowWriter().shadow(route_entry_->shadowPolicy().cluster(), std::move(request), timeout_.global_timeout_); } @@ -319,7 +321,7 @@ void Filter::onResetStream() { void Filter::onResponseTimeout() { stream_log_debug("upstream timeout", *callbacks_); - config_.cm_.get(route_->clusterName())->stats().upstream_rq_timeout_.inc(); + config_.cm_.get(route_entry_->clusterName())->stats().upstream_rq_timeout_.inc(); // It's possible to timeout during a retry backoff delay when we have no upstream request. In // this case we fake a reset since onUpstreamReset() doesn't care. @@ -477,7 +479,7 @@ void Filter::onUpstreamComplete() { Http::CodeUtility::ResponseTimingInfo info{ config_.global_store_, cluster_->statsScope(), EMPTY_STRING, response_time, - upstream_request_->upstream_canary_, internal_request, route_->virtualHost().name(), + upstream_request_->upstream_canary_, internal_request, route_entry_->virtualHost().name(), request_vcluster_ ? request_vcluster_->name() : EMPTY_STRING, config_.local_info_.zoneName(), upstreamZone(upstream_request_->upstream_host_)}; @@ -517,7 +519,7 @@ bool Filter::setupRetry(bool end_stream) { void Filter::doRetry() { Http::ConnectionPool::Instance* conn_pool = - config_.cm_.httpConnPoolForCluster(route_->clusterName(), finalPriority()); + config_.cm_.httpConnPoolForCluster(route_entry_->clusterName(), finalPriority()); if (!conn_pool) { sendNoHealthyUpstreamResponse(); cleanup(); @@ -641,7 +643,7 @@ void Filter::UpstreamRequest::setupPerTryTimeout() { void Filter::UpstreamRequest::onPerTryTimeout() { stream_log_debug("upstream per try timeout", *parent_.callbacks_); - parent_.config_.cm_.get(parent_.route_->clusterName()) + parent_.config_.cm_.get(parent_.route_entry_->clusterName()) ->stats() .upstream_rq_per_try_timeout_.inc(); upstream_host_->stats().rq_timeout_.inc(); diff --git a/source/common/router/router.h b/source/common/router/router.h index 649ece151c94..aa3f4d19d4b9 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -200,7 +200,7 @@ class Filter : Logger::Loggable, public Http::StreamDecoderF FilterConfig& config_; Http::StreamDecoderFilterCallbacks* callbacks_{}; - const RouteEntry* route_; + const RouteEntry* route_entry_; Upstream::ClusterInfoPtr cluster_; std::string alt_stat_prefix_; const VirtualCluster* request_vcluster_; diff --git a/test/common/http/filter/ratelimit_test.cc b/test/common/http/filter/ratelimit_test.cc index 2dd80833fcb3..35f6accd8be0 100644 --- a/test/common/http/filter/ratelimit_test.cc +++ b/test/common/http/filter/ratelimit_test.cc @@ -40,12 +40,13 @@ class HttpRateLimitFilterTest : public testing::Test { client_ = new ::RateLimit::MockClient(); filter_.reset(new Filter(config_, ::RateLimit::ClientPtr{client_})); filter_->setDecoderFilterCallbacks(filter_callbacks_); - filter_callbacks_.route_table_.route_entry_.rate_limit_policy_.rate_limit_policy_entry_.clear(); - filter_callbacks_.route_table_.route_entry_.rate_limit_policy_.rate_limit_policy_entry_ + filter_callbacks_.route_table_.route_.route_entry_.rate_limit_policy_.rate_limit_policy_entry_ + .clear(); + filter_callbacks_.route_table_.route_.route_entry_.rate_limit_policy_.rate_limit_policy_entry_ .emplace_back(route_rate_limit_); - filter_callbacks_.route_table_.route_entry_.virtual_host_.rate_limit_policy_ + filter_callbacks_.route_table_.route_.route_entry_.virtual_host_.rate_limit_policy_ .rate_limit_policy_entry_.clear(); - filter_callbacks_.route_table_.route_entry_.virtual_host_.rate_limit_policy_ + filter_callbacks_.route_table_.route_.route_entry_.virtual_host_.rate_limit_policy_ .rate_limit_policy_entry_.emplace_back(vh_rate_limit_); } @@ -74,7 +75,7 @@ class HttpRateLimitFilterTest : public testing::Test { TEST_F(HttpRateLimitFilterTest, NoRoute) { SetUpTest(filter_config); - EXPECT_CALL(filter_callbacks_.route_table_, routeForRequest(_)).WillOnce(Return(nullptr)); + EXPECT_CALL(filter_callbacks_.route_table_.route_, routeEntry()).WillOnce(Return(nullptr)); EXPECT_EQ(FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false)); EXPECT_EQ(FilterDataStatus::Continue, filter_->decodeData(data_, false)); @@ -84,7 +85,8 @@ TEST_F(HttpRateLimitFilterTest, NoRoute) { TEST_F(HttpRateLimitFilterTest, NoApplicableRateLimit) { SetUpTest(filter_config); - filter_callbacks_.route_table_.route_entry_.rate_limit_policy_.rate_limit_policy_entry_.clear(); + filter_callbacks_.route_table_.route_.route_entry_.rate_limit_policy_.rate_limit_policy_entry_ + .clear(); EXPECT_EQ(FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false)); EXPECT_EQ(FilterDataStatus::Continue, filter_->decodeData(data_, false)); EXPECT_EQ(FilterTrailersStatus::Continue, filter_->decodeTrailers(request_headers_)); @@ -114,13 +116,13 @@ TEST_F(HttpRateLimitFilterTest, OkResponse) { SetUpTest(filter_config); InSequence s; - EXPECT_CALL(filter_callbacks_.route_table_.route_entry_.rate_limit_policy_, + EXPECT_CALL(filter_callbacks_.route_table_.route_.route_entry_.rate_limit_policy_, getApplicableRateLimit(0)).Times(1); EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(filter_callbacks_.route_table_.route_entry_.virtual_host_.rate_limit_policy_, + EXPECT_CALL(filter_callbacks_.route_table_.route_.route_entry_.virtual_host_.rate_limit_policy_, getApplicableRateLimit(0)).Times(1); EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector<::RateLimit::Descriptor>{ diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 8d525f27498f..2ffcf1812c53 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -153,34 +153,37 @@ TEST(RouteMatcherTest, TestRoutes) { // Base routing testing. EXPECT_EQ("instant-server", - config.routeForRequest(genHeaders("api.lyft.com", "/", "GET"), 0)->clusterName()); - EXPECT_EQ("ats", config.routeForRequest(genHeaders("api.lyft.com", "/api/leads/me", "GET"), 0) + config.route(genHeaders("api.lyft.com", "/", "GET"), 0)->routeEntry()->clusterName()); + EXPECT_EQ("ats", config.route(genHeaders("api.lyft.com", "/api/leads/me", "GET"), 0) + ->routeEntry() + ->clusterName()); + EXPECT_EQ("ats", config.route(genHeaders("api.lyft.com", "/api/application_data", "GET"), 0) + ->routeEntry() ->clusterName()); - EXPECT_EQ("ats", - config.routeForRequest(genHeaders("api.lyft.com", "/api/application_data", "GET"), 0) - ->clusterName()); EXPECT_EQ("locations", - config.routeForRequest(genHeaders("api.lyft.com", "/api/locations?works=true", "GET"), - 0)->clusterName()); - EXPECT_EQ("locations", config.routeForRequest(genHeaders("api.lyft.com", "/api/locations", "GET"), - 0)->clusterName()); + config.route(genHeaders("api.lyft.com", "/api/locations?works=true", "GET"), 0) + ->routeEntry() + ->clusterName()); + EXPECT_EQ("locations", config.route(genHeaders("api.lyft.com", "/api/locations", "GET"), 0) + ->routeEntry() + ->clusterName()); EXPECT_EQ("www2", - config.routeForRequest(genHeaders("lyft.com", "/foo", "GET"), 0)->clusterName()); + config.route(genHeaders("lyft.com", "/foo", "GET"), 0)->routeEntry()->clusterName()); EXPECT_EQ("root_www2", - config.routeForRequest(genHeaders("wwww.lyft.com", "/", "GET"), 0)->clusterName()); + config.route(genHeaders("wwww.lyft.com", "/", "GET"), 0)->routeEntry()->clusterName()); // Timeout testing. EXPECT_EQ(std::chrono::milliseconds(30000), - config.routeForRequest(genHeaders("api.lyft.com", "/", "GET"), 0)->timeout()); + config.route(genHeaders("api.lyft.com", "/", "GET"), 0)->routeEntry()->timeout()); EXPECT_EQ( std::chrono::milliseconds(15000), - config.routeForRequest(genHeaders("api.lyft.com", "/api/leads/me", "GET"), 0)->timeout()); + config.route(genHeaders("api.lyft.com", "/api/leads/me", "GET"), 0)->routeEntry()->timeout()); // Prefix rewrite testing. { Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/new_endpoint/foo", "GET"); - const RouteEntry* route = config.routeForRequest(headers, 0); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); EXPECT_EQ("www2", route->clusterName()); EXPECT_EQ("www2", route->virtualHost().name()); route->finalizeRequestHeaders(headers); @@ -191,14 +194,14 @@ TEST(RouteMatcherTest, TestRoutes) { { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/api/locations?works=true", "GET"); - const RouteEntry* route = config.routeForRequest(headers, 0); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); route->finalizeRequestHeaders(headers); EXPECT_EQ("/rewrote?works=true", headers.get_(Http::Headers::get().Path)); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/foo", "GET"); - const RouteEntry* route = config.routeForRequest(headers, 0); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); route->finalizeRequestHeaders(headers); EXPECT_EQ("/bar", headers.get_(Http::Headers::get().Path)); } @@ -206,7 +209,7 @@ TEST(RouteMatcherTest, TestRoutes) { // Host rewrite testing. { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/host/rewrite/me", "GET"); - const RouteEntry* route = config.routeForRequest(headers, 0); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); route->finalizeRequestHeaders(headers); EXPECT_EQ("new_host", headers.get_(Http::Headers::get().Host)); } @@ -215,14 +218,14 @@ TEST(RouteMatcherTest, TestRoutes) { { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/API/locations?works=true", "GET"); - const RouteEntry* route = config.routeForRequest(headers, 0); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); route->finalizeRequestHeaders(headers); EXPECT_EQ("/rewrote?works=true", headers.get_(Http::Headers::get().Path)); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/fooD", "GET"); - const RouteEntry* route = config.routeForRequest(headers, 0); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); route->finalizeRequestHeaders(headers); EXPECT_EQ("/cAndy", headers.get_(Http::Headers::get().Path)); } @@ -230,14 +233,14 @@ TEST(RouteMatcherTest, TestRoutes) { // Case sensitive is set to true and will not rewrite { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/FOO", "GET"); - const RouteEntry* route = config.routeForRequest(headers, 0); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); route->finalizeRequestHeaders(headers); EXPECT_EQ("/FOO", headers.get_(Http::Headers::get().Path)); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/ApPles", "GET"); - const RouteEntry* route = config.routeForRequest(headers, 0); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); route->finalizeRequestHeaders(headers); EXPECT_EQ("/ApPles", headers.get_(Http::Headers::get().Path)); } @@ -245,7 +248,7 @@ TEST(RouteMatcherTest, TestRoutes) { // Case insensitive set to false so there is no rewrite { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/oLDhost/rewrite/me", "GET"); - const RouteEntry* route = config.routeForRequest(headers, 0); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); route->finalizeRequestHeaders(headers); EXPECT_EQ("api.lyft.com", headers.get_(Http::Headers::get().Host)); } @@ -253,7 +256,7 @@ TEST(RouteMatcherTest, TestRoutes) { // Case sensitive is set to false and will not rewrite { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/Tart", "GET"); - const RouteEntry* route = config.routeForRequest(headers, 0); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); route->finalizeRequestHeaders(headers); EXPECT_EQ("/Tart", headers.get_(Http::Headers::get().Path)); } @@ -261,7 +264,7 @@ TEST(RouteMatcherTest, TestRoutes) { // Case sensitive is set to false and will not rewrite { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/newhost/rewrite/me", "GET"); - const RouteEntry* route = config.routeForRequest(headers, 0); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); route->finalizeRequestHeaders(headers); EXPECT_EQ("new_host", headers.get_(Http::Headers::get().Host)); } @@ -279,55 +282,58 @@ TEST(RouteMatcherTest, TestRoutes) { // Virtual cluster testing. { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/rides", "GET"); - EXPECT_EQ("other", config.routeForRequest(headers, 0)->virtualCluster(headers)->name()); + EXPECT_EQ("other", config.route(headers, 0)->routeEntry()->virtualCluster(headers)->name()); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/rides/blah", "POST"); - EXPECT_EQ("other", config.routeForRequest(headers, 0)->virtualCluster(headers)->name()); + EXPECT_EQ("other", config.route(headers, 0)->routeEntry()->virtualCluster(headers)->name()); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/rides", "POST"); - EXPECT_EQ("ride_request", config.routeForRequest(headers, 0)->virtualCluster(headers)->name()); + EXPECT_EQ("ride_request", + config.route(headers, 0)->routeEntry()->virtualCluster(headers)->name()); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/rides/123", "PUT"); - EXPECT_EQ("update_ride", config.routeForRequest(headers, 0)->virtualCluster(headers)->name()); + EXPECT_EQ("update_ride", + config.route(headers, 0)->routeEntry()->virtualCluster(headers)->name()); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/users/123/chargeaccounts", "POST"); - EXPECT_EQ("cc_add", config.routeForRequest(headers, 0)->virtualCluster(headers)->name()); + EXPECT_EQ("cc_add", config.route(headers, 0)->routeEntry()->virtualCluster(headers)->name()); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/users/123/chargeaccounts/hello123", "PUT"); - EXPECT_EQ("cc_add", config.routeForRequest(headers, 0)->virtualCluster(headers)->name()); + EXPECT_EQ("cc_add", config.route(headers, 0)->routeEntry()->virtualCluster(headers)->name()); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/users/123/chargeaccounts/validate", "PUT"); - EXPECT_EQ("other", config.routeForRequest(headers, 0)->virtualCluster(headers)->name()); + EXPECT_EQ("other", config.route(headers, 0)->routeEntry()->virtualCluster(headers)->name()); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/foo/bar", "PUT"); - EXPECT_EQ("other", config.routeForRequest(headers, 0)->virtualCluster(headers)->name()); + EXPECT_EQ("other", config.route(headers, 0)->routeEntry()->virtualCluster(headers)->name()); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/users", "POST"); EXPECT_EQ("create_user_login", - config.routeForRequest(headers, 0)->virtualCluster(headers)->name()); + config.route(headers, 0)->routeEntry()->virtualCluster(headers)->name()); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/users/123", "PUT"); - EXPECT_EQ("update_user", config.routeForRequest(headers, 0)->virtualCluster(headers)->name()); + EXPECT_EQ("update_user", + config.route(headers, 0)->routeEntry()->virtualCluster(headers)->name()); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/users/123/location", "POST"); - EXPECT_EQ("ulu", config.routeForRequest(headers, 0)->virtualCluster(headers)->name()); + EXPECT_EQ("ulu", config.route(headers, 0)->routeEntry()->virtualCluster(headers)->name()); } { Http::TestHeaderMapImpl headers = genHeaders("api.lyft.com", "/something/else", "GET"); - EXPECT_EQ("other", config.routeForRequest(headers, 0)->virtualCluster(headers)->name()); + EXPECT_EQ("other", config.route(headers, 0)->routeEntry()->virtualCluster(headers)->name()); } } @@ -389,20 +395,20 @@ TEST(RouteMatcherTest, Priority) { EXPECT_FALSE(config.usesRuntime()); EXPECT_EQ(Upstream::ResourcePriority::High, - config.routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0)->priority()); + config.route(genHeaders("www.lyft.com", "/foo", "GET"), 0)->routeEntry()->priority()); EXPECT_EQ(Upstream::ResourcePriority::Default, - config.routeForRequest(genHeaders("www.lyft.com", "/bar", "GET"), 0)->priority()); + config.route(genHeaders("www.lyft.com", "/bar", "GET"), 0)->routeEntry()->priority()); { Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/bar", "POST"); EXPECT_EQ(Upstream::ResourcePriority::High, - config.routeForRequest(headers, 0)->virtualCluster(headers)->priority()); + config.route(headers, 0)->routeEntry()->virtualCluster(headers)->priority()); } { Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/bar", "GET"); EXPECT_EQ(Upstream::ResourcePriority::Default, - config.routeForRequest(headers, 0)->virtualCluster(headers)->priority()); + config.route(headers, 0)->routeEntry()->virtualCluster(headers)->priority()); } } @@ -469,13 +475,13 @@ TEST(RouteMatcherTest, HeaderMatchedRouting) { { EXPECT_EQ("local_service_without_headers", - config.routeForRequest(genHeaders("www.lyft.com", "/", "GET"), 0)->clusterName()); + config.route(genHeaders("www.lyft.com", "/", "GET"), 0)->routeEntry()->clusterName()); } { Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/", "GET"); headers.addViaCopy("test_header", "test"); - EXPECT_EQ("local_service_with_headers", config.routeForRequest(headers, 0)->clusterName()); + EXPECT_EQ("local_service_with_headers", config.route(headers, 0)->routeEntry()->clusterName()); } { @@ -483,33 +489,35 @@ TEST(RouteMatcherTest, HeaderMatchedRouting) { headers.addViaCopy("test_header_multiple1", "test1"); headers.addViaCopy("test_header_multiple2", "test2"); EXPECT_EQ("local_service_with_multiple_headers", - config.routeForRequest(headers, 0)->clusterName()); + config.route(headers, 0)->routeEntry()->clusterName()); } { Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/", "GET"); headers.addViaCopy("non_existent_header", "foo"); - EXPECT_EQ("local_service_without_headers", config.routeForRequest(headers, 0)->clusterName()); + EXPECT_EQ("local_service_without_headers", + config.route(headers, 0)->routeEntry()->clusterName()); } { Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/", "GET"); headers.addViaCopy("test_header_presence", "test"); EXPECT_EQ("local_service_with_empty_headers", - config.routeForRequest(headers, 0)->clusterName()); + config.route(headers, 0)->routeEntry()->clusterName()); } { Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/", "GET"); headers.addViaCopy("test_header_pattern", "user=test-1223"); EXPECT_EQ("local_service_with_header_pattern_set_regex", - config.routeForRequest(headers, 0)->clusterName()); + config.route(headers, 0)->routeEntry()->clusterName()); } { Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/", "GET"); headers.addViaCopy("test_header_pattern", "customer=test-1223"); - EXPECT_EQ("local_service_without_headers", config.routeForRequest(headers, 0)->clusterName()); + EXPECT_EQ("local_service_without_headers", + config.route(headers, 0)->routeEntry()->clusterName()); } } @@ -547,19 +555,19 @@ TEST(RouteMatcherTest, ContentType) { { EXPECT_EQ("local_service", - config.routeForRequest(genHeaders("www.lyft.com", "/", "GET"), 0)->clusterName()); + config.route(genHeaders("www.lyft.com", "/", "GET"), 0)->routeEntry()->clusterName()); } { Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/", "GET"); headers.addViaCopy("content-type", "application/grpc"); - EXPECT_EQ("local_service_grpc", config.routeForRequest(headers, 0)->clusterName()); + EXPECT_EQ("local_service_grpc", config.route(headers, 0)->routeEntry()->clusterName()); } { Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/", "GET"); headers.addViaCopy("content-type", "foo"); - EXPECT_EQ("local_service", config.routeForRequest(headers, 0)->clusterName()); + EXPECT_EQ("local_service", config.route(headers, 0)->routeEntry()->clusterName()); } } @@ -602,11 +610,11 @@ TEST(RouteMatcherTest, Runtime) { EXPECT_CALL(snapshot, featureEnabled("some_key", 50, 10)).WillOnce(Return(true)); EXPECT_EQ("something_else", - config.routeForRequest(genHeaders("www.lyft.com", "/", "GET"), 10)->clusterName()); + config.route(genHeaders("www.lyft.com", "/", "GET"), 10)->routeEntry()->clusterName()); EXPECT_CALL(snapshot, featureEnabled("some_key", 50, 20)).WillOnce(Return(false)); EXPECT_EQ("www2", - config.routeForRequest(genHeaders("www.lyft.com", "/", "GET"), 20)->clusterName()); + config.route(genHeaders("www.lyft.com", "/", "GET"), 20)->routeEntry()->clusterName()); } TEST(RouteMatcherTest, ShadowClusterNotFound) { @@ -679,24 +687,30 @@ TEST(RouteMatcherTest, Shadow) { EXPECT_TRUE(config.usesRuntime()); - EXPECT_EQ("some_cluster", config.routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0) + EXPECT_EQ("some_cluster", config.route(genHeaders("www.lyft.com", "/foo", "GET"), 0) + ->routeEntry() ->shadowPolicy() .cluster()); - EXPECT_EQ("", config.routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0) + EXPECT_EQ("", config.route(genHeaders("www.lyft.com", "/foo", "GET"), 0) + ->routeEntry() ->shadowPolicy() .runtimeKey()); - EXPECT_EQ("some_cluster2", config.routeForRequest(genHeaders("www.lyft.com", "/bar", "GET"), 0) + EXPECT_EQ("some_cluster2", config.route(genHeaders("www.lyft.com", "/bar", "GET"), 0) + ->routeEntry() ->shadowPolicy() .cluster()); - EXPECT_EQ("foo", config.routeForRequest(genHeaders("www.lyft.com", "/bar", "GET"), 0) + EXPECT_EQ("foo", config.route(genHeaders("www.lyft.com", "/bar", "GET"), 0) + ->routeEntry() ->shadowPolicy() .runtimeKey()); - EXPECT_EQ("", config.routeForRequest(genHeaders("www.lyft.com", "/baz", "GET"), 0) + EXPECT_EQ("", config.route(genHeaders("www.lyft.com", "/baz", "GET"), 0) + ->routeEntry() ->shadowPolicy() .cluster()); - EXPECT_EQ("", config.routeForRequest(genHeaders("www.lyft.com", "/baz", "GET"), 0) + EXPECT_EQ("", config.route(genHeaders("www.lyft.com", "/baz", "GET"), 0) + ->routeEntry() ->shadowPolicy() .runtimeKey()); } @@ -741,27 +755,34 @@ TEST(RouteMatcherTest, Retry) { EXPECT_FALSE(config.usesRuntime()); - EXPECT_EQ(1U, config.routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0) + EXPECT_EQ(1U, config.route(genHeaders("www.lyft.com", "/foo", "GET"), 0) + ->routeEntry() ->retryPolicy() .numRetries()); EXPECT_EQ(RetryPolicy::RETRY_ON_CONNECT_FAILURE, - config.routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0) + config.route(genHeaders("www.lyft.com", "/foo", "GET"), 0) + ->routeEntry() ->retryPolicy() .retryOn()); - EXPECT_EQ(0U, config.routeForRequest(genHeaders("www.lyft.com", "/bar", "GET"), 0) + EXPECT_EQ(0U, config.route(genHeaders("www.lyft.com", "/bar", "GET"), 0) + ->routeEntry() ->retryPolicy() .numRetries()); - EXPECT_EQ(0U, config.routeForRequest(genHeaders("www.lyft.com", "/bar", "GET"), 0) + EXPECT_EQ(0U, config.route(genHeaders("www.lyft.com", "/bar", "GET"), 0) + ->routeEntry() ->retryPolicy() .retryOn()); - EXPECT_EQ(3U, config.routeForRequest(genHeaders("www.lyft.com", "/", "GET"), 0) + EXPECT_EQ(3U, config.route(genHeaders("www.lyft.com", "/", "GET"), 0) + ->routeEntry() ->retryPolicy() .numRetries()); - EXPECT_EQ( - RetryPolicy::RETRY_ON_CONNECT_FAILURE | RetryPolicy::RETRY_ON_5XX, - config.routeForRequest(genHeaders("www.lyft.com", "/", "GET"), 0)->retryPolicy().retryOn()); + EXPECT_EQ(RetryPolicy::RETRY_ON_CONNECT_FAILURE | RetryPolicy::RETRY_ON_5XX, + config.route(genHeaders("www.lyft.com", "/", "GET"), 0) + ->routeEntry() + ->retryPolicy() + .retryOn()); } TEST(RouteMatcherTest, TestBadDefaultConfig) { @@ -903,47 +924,93 @@ TEST(RouteMatcherTest, Redirect) { EXPECT_FALSE(config.usesRuntime()); - EXPECT_EQ(nullptr, - config.redirectRequest(genRedirectHeaders("www.foo.com", "/foo", true, true), 0)); - EXPECT_EQ(nullptr, - config.routeForRequest(genRedirectHeaders("www.foo.com", "/foo", true, true), 0)); + EXPECT_EQ(nullptr, config.route(genRedirectHeaders("www.foo.com", "/foo", true, true), 0)); { Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/foo", true, true); - EXPECT_EQ(nullptr, config.redirectRequest(headers, 0)); + EXPECT_EQ(nullptr, config.route(headers, 0)->redirectEntry()); } { Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/foo", false, false); - EXPECT_EQ("https://www.lyft.com/foo", config.redirectRequest(headers, 0)->newPath(headers)); + const Route* route = config.route(headers, 0); + EXPECT_EQ("https://www.lyft.com/foo", route->redirectEntry()->newPath(headers)); } { Http::TestHeaderMapImpl headers = genRedirectHeaders("api.lyft.com", "/foo", false, true); - EXPECT_EQ(nullptr, config.redirectRequest(headers, 0)); + EXPECT_EQ(nullptr, config.route(headers, 0)->redirectEntry()); } { Http::TestHeaderMapImpl headers = genRedirectHeaders("api.lyft.com", "/foo", false, false); - EXPECT_EQ("https://api.lyft.com/foo", config.redirectRequest(headers, 0)->newPath(headers)); + EXPECT_EQ("https://api.lyft.com/foo", + config.route(headers, 0)->redirectEntry()->newPath(headers)); } { Http::TestHeaderMapImpl headers = genRedirectHeaders("redirect.lyft.com", "/foo", false, false); - EXPECT_EQ("http://new.lyft.com/foo", config.redirectRequest(headers, 0)->newPath(headers)); + EXPECT_EQ("http://new.lyft.com/foo", + config.route(headers, 0)->redirectEntry()->newPath(headers)); } { Http::TestHeaderMapImpl headers = genRedirectHeaders("redirect.lyft.com", "/bar", true, false); EXPECT_EQ("https://redirect.lyft.com/new_bar", - config.redirectRequest(headers, 0)->newPath(headers)); + config.route(headers, 0)->redirectEntry()->newPath(headers)); } { Http::TestHeaderMapImpl headers = genRedirectHeaders("redirect.lyft.com", "/baz", true, false); - EXPECT_EQ("https://new.lyft.com/new_baz", config.redirectRequest(headers, 0)->newPath(headers)); + EXPECT_EQ("https://new.lyft.com/new_baz", + config.route(headers, 0)->redirectEntry()->newPath(headers)); + } +} + +TEST(RouteMatcherTest, ExclusiveRouteEntryOrRedirectEntry) { + std::string json = R"EOF( +{ + "virtual_hosts": [ + { + "name": "www2", + "domains": ["www.lyft.com"], + "routes": [ + { + "prefix": "/", + "cluster": "www2" + } + ] + }, + { + "name": "redirect", + "domains": ["redirect.lyft.com"], + "routes": [ + { + "prefix": "/foo", + "host_redirect": "new.lyft.com" + } + ] + } + ] +} + )EOF"; + + Json::ObjectPtr loader = Json::Factory::LoadFromString(json); + NiceMock runtime; + NiceMock cm; + ConfigImpl config(*loader, runtime, cm); + + { + Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/foo", true, true); + EXPECT_EQ(nullptr, config.route(headers, 0)->redirectEntry()); + EXPECT_EQ("www2", config.route(headers, 0)->routeEntry()->clusterName()); + } + { + Http::TestHeaderMapImpl headers = genRedirectHeaders("redirect.lyft.com", "/foo", false, false); + EXPECT_EQ("http://new.lyft.com/foo", + config.route(headers, 0)->redirectEntry()->newPath(headers)); + EXPECT_EQ(nullptr, config.route(headers, 0)->routeEntry()); } } TEST(NullConfigImplTest, All) { NullConfigImpl config; Http::TestHeaderMapImpl headers = genRedirectHeaders("redirect.lyft.com", "/baz", true, false); - EXPECT_EQ(nullptr, config.redirectRequest(headers, 0)); - EXPECT_EQ(nullptr, config.routeForRequest(headers, 0)); + EXPECT_EQ(nullptr, config.route(headers, 0)); EXPECT_EQ(0UL, config.internalOnlyHeaders().size()); EXPECT_EQ(0UL, config.responseHeadersToAdd().size()); EXPECT_EQ(0UL, config.responseHeadersToRemove().size()); diff --git a/test/common/router/router_ratelimit_test.cc b/test/common/router/router_ratelimit_test.cc index 6261c2ac6911..cb2f0d346e59 100644 --- a/test/common/router/router_ratelimit_test.cc +++ b/test/common/router/router_ratelimit_test.cc @@ -132,7 +132,8 @@ TEST_F(RateLimitConfiguration, NoRateLimit) { SetUpTest(json); - EXPECT_EQ(0U, config_->routeForRequest(genHeaders("www.lyft.com", "/bar", "GET"), 0) + EXPECT_EQ(0U, config_->route(genHeaders("www.lyft.com", "/bar", "GET"), 0) + ->routeEntry() ->rateLimitPolicy() .getApplicableRateLimit(0) .size()); @@ -168,7 +169,7 @@ TEST_F(RateLimitConfiguration, TestGetApplicationRateLimit) { SetUpTest(json); std::string address = "10.0.0.1"; - route_ = config_->routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0); + route_ = config_->route(genHeaders("www.lyft.com", "/foo", "GET"), 0)->routeEntry(); std::vector> rate_limits = route_->rateLimitPolicy().getApplicableRateLimit(0); EXPECT_EQ(1U, rate_limits.size()); @@ -210,7 +211,7 @@ TEST_F(RateLimitConfiguration, TestVirtualHost) { SetUpTest(json); - route_ = config_->routeForRequest(genHeaders("www.lyft.com", "/bar", "GET"), 0); + route_ = config_->route(genHeaders("www.lyft.com", "/bar", "GET"), 0)->routeEntry(); std::vector> rate_limits = route_->virtualHost().rateLimitPolicy().getApplicableRateLimit(0); EXPECT_EQ(1U, rate_limits.size()); @@ -262,7 +263,7 @@ TEST_F(RateLimitConfiguration, TestMultipleRateLimits) { SetUpTest(json); std::string address = "10.0.0.1"; - route_ = config_->routeForRequest(genHeaders("www.lyft.com", "/foo", "GET"), 0); + route_ = config_->route(genHeaders("www.lyft.com", "/foo", "GET"), 0)->routeEntry(); std::vector> rate_limits = route_->rateLimitPolicy().getApplicableRateLimit(0); EXPECT_EQ(2U, rate_limits.size()); diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 6c2074f4d25a..4618c9b0a7f9 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -83,14 +83,14 @@ TEST_F(RouterTest, RouteNotFound) { Http::TestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.route_table_, routeForRequest(_)).WillOnce(Return(nullptr)); + EXPECT_CALL(callbacks_.route_table_, route(_)).WillOnce(Return(nullptr)); router_.decodeHeaders(headers, true); } TEST_F(RouterTest, PoolFailureWithPriority) { NiceMock route_entry; - EXPECT_CALL(callbacks_.route_table_, routeForRequest(_)).WillOnce(Return(&route_entry)); + EXPECT_CALL(callbacks_.route_table_.route_, routeEntry()).WillOnce(Return(&route_entry)); route_entry.virtual_cluster_.priority_ = Upstream::ResourcePriority::High; EXPECT_CALL(cm_, httpConnPoolForCluster(_, Upstream::ResourcePriority::High)); @@ -603,8 +603,8 @@ TEST_F(RouterTest, RetryUpstream5xxNotComplete) { } TEST_F(RouterTest, Shadow) { - callbacks_.route_table_.route_entry_.shadow_policy_.cluster_ = "foo"; - callbacks_.route_table_.route_entry_.shadow_policy_.runtime_key_ = "bar"; + callbacks_.route_table_.route_.route_entry_.shadow_policy_.cluster_ = "foo"; + callbacks_.route_table_.route_.route_entry_.shadow_policy_.runtime_key_ = "bar"; ON_CALL(callbacks_, streamId()).WillByDefault(Return(43)); NiceMock encoder; @@ -644,7 +644,7 @@ TEST_F(RouterTest, Shadow) { TEST_F(RouterTest, AltStatName) { // Also test no upstream timeout here. NiceMock route_entry; - EXPECT_CALL(callbacks_.route_table_, routeForRequest(_)).WillOnce(Return(&route_entry)); + EXPECT_CALL(callbacks_.route_table_.route_, routeEntry()).WillOnce(Return(&route_entry)); EXPECT_CALL(route_entry, timeout()).WillOnce(Return(std::chrono::milliseconds(0))); EXPECT_CALL(callbacks_.dispatcher_, createTimer_(_)).Times(0); @@ -688,7 +688,7 @@ TEST_F(RouterTest, AltStatName) { TEST_F(RouterTest, Redirect) { MockRedirectEntry redirect; EXPECT_CALL(redirect, newPath(_)).WillOnce(Return("hello")); - EXPECT_CALL(callbacks_.route_table_, redirectRequest(_)).WillOnce(Return(&redirect)); + EXPECT_CALL(callbacks_.route_table_.route_, redirectEntry()).WillRepeatedly(Return(&redirect)); Http::TestHeaderMapImpl response_headers{{":status", "301"}, {"location", "hello"}}; EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); @@ -805,7 +805,7 @@ TEST(RouterFilterUtilityTest, shouldShadow) { TEST_F(RouterTest, CanaryStatusTrue) { NiceMock route_entry; - EXPECT_CALL(callbacks_.route_table_, routeForRequest(_)).WillOnce(Return(&route_entry)); + EXPECT_CALL(callbacks_.route_table_.route_, routeEntry()).WillOnce(Return(&route_entry)); EXPECT_CALL(route_entry, timeout()).WillOnce(Return(std::chrono::milliseconds(0))); EXPECT_CALL(callbacks_.dispatcher_, createTimer_(_)).Times(0); @@ -836,7 +836,7 @@ TEST_F(RouterTest, CanaryStatusTrue) { TEST_F(RouterTest, CanaryStatusFalse) { NiceMock route_entry; - EXPECT_CALL(callbacks_.route_table_, routeForRequest(_)).WillOnce(Return(&route_entry)); + EXPECT_CALL(callbacks_.route_table_.route_, routeEntry()).WillOnce(Return(&route_entry)); EXPECT_CALL(route_entry, timeout()).WillOnce(Return(std::chrono::milliseconds(0))); EXPECT_CALL(callbacks_.dispatcher_, createTimer_(_)).Times(0); diff --git a/test/mocks/router/mocks.cc b/test/mocks/router/mocks.cc index 766a2c18b42f..3d2b97974515 100644 --- a/test/mocks/router/mocks.cc +++ b/test/mocks/router/mocks.cc @@ -61,8 +61,11 @@ MockConfig::MockConfig() { MockConfig::~MockConfig() {} +MockRoute::MockRoute() { ON_CALL(*this, routeEntry()).WillByDefault(Return(&route_entry_)); } +MockRoute::~MockRoute() {} + MockStableRouteTable::MockStableRouteTable() { - ON_CALL(*this, routeForRequest(_)).WillByDefault(Return(&route_entry_)); + ON_CALL(*this, route(_)).WillByDefault(Return(&route_)); } MockStableRouteTable::~MockStableRouteTable() {} diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 5b72b10144f5..f8cef84dbf49 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -155,10 +155,7 @@ class MockConfig : public Config { ~MockConfig(); // Router::Config - MOCK_CONST_METHOD2(redirectRequest, - const RedirectEntry*(const Http::HeaderMap& headers, uint64_t random_value)); - MOCK_CONST_METHOD2(routeForRequest, - const RouteEntry*(const Http::HeaderMap&, uint64_t random_value)); + MOCK_CONST_METHOD2(route, const Route*(const Http::HeaderMap&, uint64_t random_value)); MOCK_CONST_METHOD0(internalOnlyHeaders, const std::list&()); MOCK_CONST_METHOD0(responseHeadersToAdd, const std::list>&()); @@ -170,16 +167,27 @@ class MockConfig : public Config { std::list response_headers_to_remove_; }; +class MockRoute : public Route { +public: + MockRoute(); + ~MockRoute(); + + // Router::Route + MOCK_CONST_METHOD0(redirectEntry, const RedirectEntry*()); + MOCK_CONST_METHOD0(routeEntry, const RouteEntry*()); + + testing::NiceMock route_entry_; +}; + class MockStableRouteTable : public StableRouteTable { public: MockStableRouteTable(); ~MockStableRouteTable(); // Router::StableRouteTable - MOCK_CONST_METHOD1(redirectRequest, const RedirectEntry*(const Http::HeaderMap& headers)); - MOCK_CONST_METHOD1(routeForRequest, const RouteEntry*(const Http::HeaderMap&)); + MOCK_CONST_METHOD1(route, const Route*(const Http::HeaderMap&)); - testing::NiceMock route_entry_; + testing::NiceMock route_; }; } // Router