diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 186c3318934dc..0e3a2d41b9a9e 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -23,24 +23,25 @@ namespace Envoy { namespace Router { /** - * A routing primitive that creates a redirect path. + * A routing primitive that specifies a direct (non-proxied) HTTP response. */ -class RedirectEntry { +class DirectResponseEntry { public: - virtual ~RedirectEntry() {} + virtual ~DirectResponseEntry() {} /** - * Returns the redirect path based on the request headers. - * @param headers supplies the request headers. - * @return std::string the redirect URL. + * Returns the HTTP status code to return. + * @return Http::Code the response Code. */ - virtual std::string newPath(const Http::HeaderMap& headers) const PURE; + virtual Http::Code responseCode() const PURE; /** - * Returns the HTTP status code to use when redirecting a request. - * @return Http::Code the redirect response Code. + * Returns the redirect path based on the request headers. + * @param headers supplies the request headers. + * @return std::string the redirect URL if this DirectResponseEntry is a redirect, + * or an empty string otherwise. */ - virtual Http::Code redirectResponseCode() const PURE; + virtual std::string newPath(const Http::HeaderMap& headers) const PURE; }; /** @@ -416,16 +417,16 @@ class Decorator { typedef std::unique_ptr DecoratorConstPtr; /** - * An interface that holds a RedirectEntry or a RouteEntry for a request. + * An interface that holds a DirectResponseEntry or RouteEntry for a request. */ class Route { public: virtual ~Route() {} /** - * @return the redirect entry or nullptr if there is no redirect needed for the request. + * @return the direct response entry or nullptr if there is no direct response for the request. */ - virtual const RedirectEntry* redirectEntry() const PURE; + virtual const DirectResponseEntry* directResponseEntry() const PURE; /** * @return the route entry or nullptr if there is no matching route for the request. @@ -449,7 +450,7 @@ class Config { /** * Based on the incoming HTTP request headers, determine the target route (containing either a - * route entry or a redirect entry) for the request. + * route entry or a direct response 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. diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index 84ded953566ef..437443b9173e6 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -190,7 +190,7 @@ class AsyncStreamImpl : public AsyncClient::Stream, : route_entry_(cluster_name, timeout) {} // Router::Route - const Router::RedirectEntry* redirectEntry() const override { return nullptr; } + const Router::DirectResponseEntry* directResponseEntry() const override { return nullptr; } const Router::RouteEntry* routeEntry() const override { return &route_entry_; } const Router::Decorator* decorator() const override { return nullptr; } diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 744fb0e2292b7..3cf63eb2c840f 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -237,8 +237,7 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, response_headers_parser_(HeaderParser::configure(route.route().response_headers_to_add(), route.route().response_headers_to_remove())), opaque_config_(parseOpaqueConfig(route)), decorator_(parseDecorator(route)), - redirect_response_code_( - ConfigUtility::parseRedirectResponseCode(route.redirect().response_code())) { + direct_response_code_(ConfigUtility::parseDirectResponseCode(route)) { if (route.route().has_metadata_match()) { const auto filter_it = route.route().metadata_match().filter_metadata().find( Envoy::Config::MetadataFilters::get().ENVOY_LB); @@ -425,9 +424,10 @@ DecoratorConstPtr RouteEntryImplBase::parseDecorator(const envoy::api::v2::Route return ret; } -const RedirectEntry* RouteEntryImplBase::redirectEntry() const { - // A route for a request can exclusively be a route entry or a redirect entry. - if (isRedirect()) { +const DirectResponseEntry* RouteEntryImplBase::directResponseEntry() const { + // A route for a request can exclusively be a route entry, a direct response entry, + // or a redirect entry. + if (isDirectResponse()) { return this; } else { return nullptr; @@ -435,8 +435,9 @@ const RedirectEntry* RouteEntryImplBase::redirectEntry() const { } const RouteEntry* RouteEntryImplBase::routeEntry() const { - // A route for a request can exclusively be a route entry or a redirect entry. - if (isRedirect()) { + // A route for a request can exclusively be a route entry, a direct response entry, + // or a redirect entry. + if (isRedirect() || isDirectResponse()) { return nullptr; } else { return this; @@ -448,7 +449,7 @@ RouteConstSharedPtr RouteEntryImplBase::clusterEntry(const Http::HeaderMap& head // Gets the route object chosen from the list of weighted clusters // (if there is one) or returns self. if (weighted_clusters_.empty()) { - if (!cluster_name_.empty() || isRedirect()) { + if (!cluster_name_.empty() || isRedirect() || isDirectResponse()) { return shared_from_this(); } else { ASSERT(!cluster_header_name_.get().empty()); @@ -489,7 +490,7 @@ RouteConstSharedPtr RouteEntryImplBase::clusterEntry(const Http::HeaderMap& head } void RouteEntryImplBase::validateClusters(Upstream::ClusterManager& cm) const { - if (isRedirect()) { + if (isRedirect() || isDirectResponse()) { return; } diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index e2dc5e6782a0d..234bcb6ce5723 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -47,19 +47,19 @@ class RouteEntryImplBase; typedef std::shared_ptr RouteEntryImplBaseConstSharedPtr; /** - * Redirect entry that does an SSL redirect. + * Direct response entry that does an SSL redirect. */ -class SslRedirector : public RedirectEntry { +class SslRedirector : public DirectResponseEntry { public: - // Router::RedirectEntry + // Router::DirectResponseEntry std::string newPath(const Http::HeaderMap& headers) const override; - Http::Code redirectResponseCode() const override { return Http::Code::MovedPermanently; } + Http::Code responseCode() const override { return Http::Code::MovedPermanently; } }; class SslRedirectRoute : public Route { public: // Router::Route - const RedirectEntry* redirectEntry() const override { return &SSL_REDIRECTOR; } + const DirectResponseEntry* directResponseEntry() const override { return &SSL_REDIRECTOR; } const RouteEntry* routeEntry() const override { return nullptr; } const Decorator* decorator() const override { return nullptr; } @@ -286,7 +286,7 @@ class DecoratorImpl : public Decorator { */ class RouteEntryImplBase : public RouteEntry, public Matchable, - public RedirectEntry, + public DirectResponseEntry, public Route, public std::enable_shared_from_this { public: @@ -294,6 +294,7 @@ class RouteEntryImplBase : public RouteEntry, Runtime::Loader& loader); bool isRedirect() const { return !host_redirect_.empty() || !path_redirect_.empty(); } + bool isDirectResponse() const { return direct_response_code_.valid(); } bool matchRoute(const Http::HeaderMap& headers, uint64_t random_value) const; void validateClusters(Upstream::ClusterManager& cm) const; @@ -329,12 +330,12 @@ class RouteEntryImplBase : public RouteEntry, } bool includeVirtualHostRateLimits() const override { return include_vh_rate_limits_; } - // Router::RedirectEntry + // Router::DirectResponseEntry std::string newPath(const Http::HeaderMap& headers) const override; - Http::Code redirectResponseCode() const override { return redirect_response_code_; } + Http::Code responseCode() const override { return direct_response_code_.value(); } // Router::Route - const RedirectEntry* redirectEntry() const override; + const DirectResponseEntry* directResponseEntry() const override; const RouteEntry* routeEntry() const override; const Decorator* decorator() const override { return decorator_.get(); } @@ -402,7 +403,7 @@ class RouteEntryImplBase : public RouteEntry, } // Router::Route - const RedirectEntry* redirectEntry() const override { return nullptr; } + const DirectResponseEntry* directResponseEntry() const override { return nullptr; } const RouteEntry* routeEntry() const override { return this; } const Decorator* decorator() const override { return nullptr; } @@ -487,7 +488,7 @@ class RouteEntryImplBase : public RouteEntry, const std::multimap opaque_config_; const DecoratorConstPtr decorator_; - const Http::Code redirect_response_code_; + const Optional direct_response_code_; }; /** diff --git a/source/common/router/config_utility.cc b/source/common/router/config_utility.cc index 831aff75708a0..09acc121f64e5 100644 --- a/source/common/router/config_utility.cc +++ b/source/common/router/config_utility.cc @@ -89,6 +89,15 @@ Http::Code ConfigUtility::parseRedirectResponseCode( } } +Optional ConfigUtility::parseDirectResponseCode(const envoy::api::v2::Route& route) { + if (route.has_redirect()) { + return parseRedirectResponseCode(route.redirect().response_code()); + } else if (route.has_direct_response()) { + return static_cast(route.direct_response().status()); + } + return Optional(); +} + Http::Code ConfigUtility::parseClusterNotFoundResponseCode( const envoy::api::v2::RouteAction::ClusterNotFoundResponseCode& code) { switch (code) { diff --git a/source/common/router/config_utility.h b/source/common/router/config_utility.h index 09aa683a515f1..8d4fa8de4c2d4 100644 --- a/source/common/router/config_utility.h +++ b/source/common/router/config_utility.h @@ -4,6 +4,7 @@ #include #include +#include "envoy/common/optional.h" #include "envoy/http/codes.h" #include "envoy/json/json_object.h" #include "envoy/upstream/resource_manager.h" @@ -103,6 +104,15 @@ class ConfigUtility { static Http::Code parseRedirectResponseCode(const envoy::api::v2::RedirectAction::RedirectResponseCode& code); + /** + * Returns the HTTP Status Code enum parsed from the route's redirect or direct_response. + * @param route supplies the Route configuration. + * @return Optional the HTTP status from the route's direct_response if specified, + * or the HTTP status code from the route's redirect if specified, + * or an empty Option otherwise. + */ + static Optional parseDirectResponseCode(const envoy::api::v2::Route& route); + /** * Returns the HTTP Status Code enum parsed from proto. * @param code supplies the ClusterNotFoundResponseCode enum. diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 43d7fae456c01..fc7da1348c4f1 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -195,7 +195,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e // that get handled by earlier filters. config_.stats_.rq_total_.inc(); - // Determine if there is a route entry or a redirect for the request. + // Determine if there is a route entry or a direct response for the request. route_ = callbacks_->route(); if (!route_) { config_.stats_.no_route_.inc(); @@ -209,11 +209,18 @@ 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), - route_->redirectEntry()->redirectResponseCode()); + // Determine if there is a direct response for the request. + if (route_->directResponseEntry()) { + auto response_code = route_->directResponseEntry()->responseCode(); + if (response_code >= Http::Code::MultipleChoices && response_code < Http::Code::BadRequest) { + config_.stats_.rq_redirect_.inc(); + Http::Utility::sendRedirect(*callbacks_, route_->directResponseEntry()->newPath(headers), + response_code); + return Http::FilterHeadersStatus::StopIteration; + } + config_.stats_.rq_direct_response_.inc(); + sendLocalReply(route_->directResponseEntry()->responseCode(), "", false); + // TODO(brian-pane) support sending a response body and response_headers_to_add. return Http::FilterHeadersStatus::StopIteration; } diff --git a/source/common/router/router.h b/source/common/router/router.h index 5d2a25ceb4f75..bda04e9923c57 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -36,6 +36,7 @@ namespace Router { COUNTER(no_route) \ COUNTER(no_cluster) \ COUNTER(rq_redirect) \ + COUNTER(rq_direct_response) \ COUNTER(rq_total) // clang-format on diff --git a/test/common/http/filter/cors_filter_test.cc b/test/common/http/filter/cors_filter_test.cc index 038c05e807265..f1b277e037a94 100644 --- a/test/common/http/filter/cors_filter_test.cc +++ b/test/common/http/filter/cors_filter_test.cc @@ -50,7 +50,7 @@ class CorsFilterTest : public testing::Test { Buffer::OwnedImpl data_; TestHeaderMapImpl request_headers_; std::unique_ptr cors_policy_; - Router::MockRedirectEntry redirect_entry_; + Router::MockDirectResponseEntry direct_response_entry_; }; TEST_F(CorsFilterTest, RequestWithoutOrigin) { @@ -329,7 +329,8 @@ TEST_F(CorsFilterTest, EncodeWithNonMatchingOrigin) { } TEST_F(CorsFilterTest, RedirectRoute) { - ON_CALL(*decoder_callbacks_.route_, redirectEntry()).WillByDefault(Return(&redirect_entry_)); + ON_CALL(*decoder_callbacks_.route_, directResponseEntry()) + .WillByDefault(Return(&direct_response_entry_)); EXPECT_EQ(FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers_, false)); EXPECT_EQ(FilterDataStatus::Continue, filter_.decodeData(data_, false)); diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 3688326681508..f8d9a012443ef 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -2023,8 +2023,8 @@ static Http::TestHeaderMapImpl genRedirectHeaders(const std::string& host, const return headers; } -TEST(RouteMatcherTest, Redirect) { - std::string json = R"EOF( +TEST(RouteMatcherTest, DirectResponse) { + static const std::string v1_json = R"EOF( { "virtual_hosts": [ { @@ -2072,48 +2072,104 @@ TEST(RouteMatcherTest, Redirect) { } )EOF"; + // A superset of v1_json, with API v2 direct-response configuration added. + static const std::string v2_yaml = R"EOF( +name: foo +virtual_hosts: + - name: www2 + domains: [www.lyft.com] + require_tls: all + routes: + - match: { prefix: "/" } + route: { cluster: www2 } + - name: api + domains: [api.lyft.com] + require_tls: external_only + routes: + - match: { prefix: "/" } + route: { cluster: www2 } + - name: redirect + domains: [redirect.lyft.com] + routes: + - match: { prefix: /foo } + redirect: { host_redirect: new.lyft.com } + - match: { prefix: /bar } + redirect: { path_redirect: /new_bar } + - match: { prefix: /baz } + redirect: { host_redirect: new.lyft.com, path_redirect: /new_baz } + - name: direct + domains: [direct.example.com] + routes: + - match: { prefix: /gone } + direct_response: { status: 410 } + - match: { prefix: / } + route: { cluster: www2 } + )EOF"; + + auto testConfig = [](const ConfigImpl& config, bool test_v2 = false) { + 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.route(headers, 0)->directResponseEntry()); + } + { + Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/foo", false, false); + EXPECT_EQ("https://www.lyft.com/foo", + config.route(headers, 0)->directResponseEntry()->newPath(headers)); + } + { + Http::TestHeaderMapImpl headers = genRedirectHeaders("api.lyft.com", "/foo", false, true); + EXPECT_EQ(nullptr, config.route(headers, 0)->directResponseEntry()); + } + { + Http::TestHeaderMapImpl headers = genRedirectHeaders("api.lyft.com", "/foo", false, false); + EXPECT_EQ("https://api.lyft.com/foo", + config.route(headers, 0)->directResponseEntry()->newPath(headers)); + } + { + Http::TestHeaderMapImpl headers = + genRedirectHeaders("redirect.lyft.com", "/foo", false, false); + EXPECT_EQ("http://new.lyft.com/foo", + config.route(headers, 0)->directResponseEntry()->newPath(headers)); + } + { + Http::TestHeaderMapImpl headers = + genRedirectHeaders("redirect.lyft.com", "/bar", true, false); + EXPECT_EQ("https://redirect.lyft.com/new_bar", + config.route(headers, 0)->directResponseEntry()->newPath(headers)); + } + { + Http::TestHeaderMapImpl headers = + genRedirectHeaders("redirect.lyft.com", "/baz", true, false); + EXPECT_EQ("https://new.lyft.com/new_baz", + config.route(headers, 0)->directResponseEntry()->newPath(headers)); + } + if (!test_v2) { + return; + } + { + Http::TestHeaderMapImpl headers = + genRedirectHeaders("direct.example.com", "/gone", true, false); + EXPECT_EQ(Http::Code::Gone, config.route(headers, 0)->directResponseEntry()->responseCode()); + } + { + Http::TestHeaderMapImpl headers = + genRedirectHeaders("direct.example.com", "/other", true, false); + EXPECT_EQ(nullptr, config.route(headers, 0)->directResponseEntry()); + } + }; + NiceMock runtime; NiceMock cm; - ConfigImpl config(parseRouteConfigurationFromJson(json), runtime, cm, true); - EXPECT_EQ(nullptr, config.route(genRedirectHeaders("www.foo.com", "/foo", true, true), 0)); + ConfigImpl v1_json_config(parseRouteConfigurationFromJson(v1_json), runtime, cm, true); + testConfig(v1_json_config); - { - Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/foo", true, true); - 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.route(headers, 0)->redirectEntry()->newPath(headers)); - } - { - Http::TestHeaderMapImpl headers = genRedirectHeaders("api.lyft.com", "/foo", false, true); - 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.route(headers, 0)->redirectEntry()->newPath(headers)); - } - { - Http::TestHeaderMapImpl headers = genRedirectHeaders("redirect.lyft.com", "/foo", false, false); - 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.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.route(headers, 0)->redirectEntry()->newPath(headers)); - } + ConfigImpl v2_yaml_config(parseRouteConfigurationFromV2Yaml(v2_yaml), runtime, cm, true); + testConfig(v2_yaml_config, true); } -TEST(RouteMatcherTest, ExclusiveRouteEntryOrRedirectEntry) { +TEST(RouteMatcherTest, ExclusiveRouteEntryOrDirectResponseEntry) { std::string json = R"EOF( { "virtual_hosts": [ @@ -2147,18 +2203,18 @@ TEST(RouteMatcherTest, ExclusiveRouteEntryOrRedirectEntry) { { Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/foo", true, true); - EXPECT_EQ(nullptr, config.route(headers, 0)->redirectEntry()); + EXPECT_EQ(nullptr, config.route(headers, 0)->directResponseEntry()); 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)); + config.route(headers, 0)->directResponseEntry()->newPath(headers)); EXPECT_EQ(nullptr, config.route(headers, 0)->routeEntry()); } } -TEST(RouteMatcherTest, ExclusiveWeightedClustersEntryOrRedirectEntry) { +TEST(RouteMatcherTest, ExclusiveWeightedClustersEntryOrDirectResponseEntry) { std::string json = R"EOF( { "virtual_hosts": [ @@ -2194,14 +2250,14 @@ TEST(RouteMatcherTest, ExclusiveWeightedClustersEntryOrRedirectEntry) { { Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/foo", true, true); - EXPECT_EQ(nullptr, config.route(headers, 0)->redirectEntry()); + EXPECT_EQ(nullptr, config.route(headers, 0)->directResponseEntry()); 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)); + config.route(headers, 0)->directResponseEntry()->newPath(headers)); EXPECT_EQ(nullptr, config.route(headers, 0)->routeEntry()); } } @@ -2253,7 +2309,7 @@ TEST(RouteMatcherTest, WeightedClusters) { { Http::TestHeaderMapImpl headers = genRedirectHeaders("www1.lyft.com", "/foo", true, true); - EXPECT_EQ(nullptr, config.route(headers, 0)->redirectEntry()); + EXPECT_EQ(nullptr, config.route(headers, 0)->directResponseEntry()); } // Weighted Cluster with no runtime @@ -3253,7 +3309,7 @@ TEST(RouteEntryMetadataMatchTest, ParsesMetadata) { { Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/both", true, true); - EXPECT_EQ(nullptr, config.route(headers, 0)->redirectEntry()); + EXPECT_EQ(nullptr, config.route(headers, 0)->directResponseEntry()); auto* route_entry = config.route(headers, 0)->routeEntry(); EXPECT_EQ("www1", route_entry->clusterName()); @@ -3267,7 +3323,7 @@ TEST(RouteEntryMetadataMatchTest, ParsesMetadata) { { Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/cluster-only", true, true); - EXPECT_EQ(nullptr, config.route(headers, 0)->redirectEntry()); + EXPECT_EQ(nullptr, config.route(headers, 0)->directResponseEntry()); auto* route_entry = config.route(headers, 0)->routeEntry(); EXPECT_EQ("www2", route_entry->clusterName()); @@ -3279,7 +3335,7 @@ TEST(RouteEntryMetadataMatchTest, ParsesMetadata) { { Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/route-only", true, true); - EXPECT_EQ(nullptr, config.route(headers, 0)->redirectEntry()); + EXPECT_EQ(nullptr, config.route(headers, 0)->directResponseEntry()); auto* route_entry = config.route(headers, 0)->routeEntry(); EXPECT_EQ("www3", route_entry->clusterName()); @@ -3292,7 +3348,7 @@ TEST(RouteEntryMetadataMatchTest, ParsesMetadata) { { Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/cluster-passthrough", true, true); - EXPECT_EQ(nullptr, config.route(headers, 0)->redirectEntry()); + EXPECT_EQ(nullptr, config.route(headers, 0)->directResponseEntry()); auto* route_entry = config.route(headers, 0)->routeEntry(); EXPECT_EQ("www4", route_entry->clusterName()); @@ -3339,9 +3395,9 @@ name: foo { Http::TestHeaderMapImpl headers = genRedirectHeaders("redirect.lyft.com", "/foo", false, false); EXPECT_EQ("http://new.lyft.com/foo", - config.route(headers, 0)->redirectEntry()->newPath(headers)); + config.route(headers, 0)->directResponseEntry()->newPath(headers)); EXPECT_EQ(Http::Code::TemporaryRedirect, - config.route(headers, 0)->redirectEntry()->redirectResponseCode()); + config.route(headers, 0)->directResponseEntry()->responseCode()); } } diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 49d239a72db04..6fe0fde32d50f 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -1449,10 +1449,10 @@ TEST_F(RouterTest, AltStatName) { } TEST_F(RouterTest, Redirect) { - MockRedirectEntry redirect; - EXPECT_CALL(redirect, newPath(_)).WillOnce(Return("hello")); - EXPECT_CALL(redirect, redirectResponseCode()).WillOnce(Return(Http::Code::MovedPermanently)); - EXPECT_CALL(*callbacks_.route_, redirectEntry()).WillRepeatedly(Return(&redirect)); + MockDirectResponseEntry direct_response; + EXPECT_CALL(direct_response, newPath(_)).WillOnce(Return("hello")); + EXPECT_CALL(direct_response, responseCode()).WillOnce(Return(Http::Code::MovedPermanently)); + EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response)); Http::TestHeaderMapImpl response_headers{{":status", "301"}, {"location", "hello"}}; EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); @@ -1463,10 +1463,10 @@ TEST_F(RouterTest, Redirect) { } TEST_F(RouterTest, RedirectFound) { - MockRedirectEntry redirect; - EXPECT_CALL(redirect, newPath(_)).WillOnce(Return("hello")); - EXPECT_CALL(redirect, redirectResponseCode()).WillOnce(Return(Http::Code::Found)); - EXPECT_CALL(*callbacks_.route_, redirectEntry()).WillRepeatedly(Return(&redirect)); + MockDirectResponseEntry direct_response; + EXPECT_CALL(direct_response, newPath(_)).WillOnce(Return("hello")); + EXPECT_CALL(direct_response, responseCode()).WillOnce(Return(Http::Code::Found)); + EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response)); Http::TestHeaderMapImpl response_headers{{":status", "302"}, {"location", "hello"}}; EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); @@ -1476,6 +1476,20 @@ TEST_F(RouterTest, RedirectFound) { EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); } +TEST_F(RouterTest, DirectResponse) { + MockDirectResponseEntry direct_response; + EXPECT_CALL(direct_response, responseCode()).WillRepeatedly(Return(Http::Code::OK)); + EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response)); + + Http::TestHeaderMapImpl response_headers{{":status", "200"}}; + EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); + Http::TestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, true); + EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); + EXPECT_EQ(1UL, config_.stats_.rq_direct_response_.value()); +} + TEST(RouterFilterUtilityTest, finalTimeout) { { NiceMock route; diff --git a/test/config/utility.h b/test/config/utility.h index 90f6737cb868f..2547ac7c2d790 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -5,6 +5,8 @@ #include #include +#include "envoy/http/codes.h" + #include "common/network/address_impl.h" #include "api/base.pb.h" diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index efb5e4388f5c5..554b0b34b37d0 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -331,6 +331,28 @@ void HttpIntegrationTest::testRouterRedirect() { response->headers().get(Http::Headers::get().Location)->value().c_str()); } +void HttpIntegrationTest::testRouterDirectResponse() { + static const std::string domain("direct.example.com"); + static const std::string prefix("/"); + static const Http::Code status(Http::Code::NoContent); + config_helper_.addConfigModifier( + [&](envoy::api::v2::filter::network::HttpConnectionManager& hcm) -> void { + auto* route_config = hcm.mutable_route_config(); + auto* virtual_host = route_config->add_virtual_hosts(); + virtual_host->set_name(domain); + virtual_host->add_domains(domain); + virtual_host->add_routes()->mutable_match()->set_prefix(prefix); + virtual_host->mutable_routes(0)->mutable_direct_response()->set_status( + static_cast(status)); + }); + initialize(); + + BufferingStreamDecoderPtr response = IntegrationUtil::makeSingleRequest( + lookupPort("http"), "GET", "/", "", downstream_protocol_, version_, "direct.example.com"); + EXPECT_TRUE(response->complete()); + EXPECT_STREQ("204", response->headers().Status()->value().c_str()); +} + // Add a health check filter and verify correct behavior when draining. void HttpIntegrationTest::testDrainClose() { config_helper_.addFilter(ConfigHelper::DEFAULT_HEALTH_CHECK_FILTER); diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h index eb670b5d5a2f6..403f2be501caa 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -104,6 +104,7 @@ class HttpIntegrationTest : public BaseIntegrationTest { typedef std::function ConnectionCreationFunction; void testRouterRedirect(); + void testRouterDirectResponse(); void testRouterNotFound(); void testRouterNotFoundWithBody(); void testRouterClusterNotFound404(); diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index 332a14c5ee10a..fbb3869fbaf00 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -32,6 +32,8 @@ TEST_P(IntegrationTest, RouterClusterNotFound503) { testRouterClusterNotFound503 TEST_P(IntegrationTest, RouterRedirect) { testRouterRedirect(); } +TEST_P(IntegrationTest, RouterDirectResponse) { testRouterDirectResponse(); } + TEST_P(IntegrationTest, DrainClose) { testDrainClose(); } TEST_P(IntegrationTest, ConnectionClose) { diff --git a/test/mocks/router/mocks.cc b/test/mocks/router/mocks.cc index 9e077af58415a..15381f3cea97e 100644 --- a/test/mocks/router/mocks.cc +++ b/test/mocks/router/mocks.cc @@ -14,8 +14,8 @@ using testing::_; namespace Envoy { namespace Router { -MockRedirectEntry::MockRedirectEntry() {} -MockRedirectEntry::~MockRedirectEntry() {} +MockDirectResponseEntry::MockDirectResponseEntry() {} +MockDirectResponseEntry::~MockDirectResponseEntry() {} MockRetryState::MockRetryState() {} diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 3636d65a0dd78..e0240895e6bb6 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -27,14 +27,14 @@ namespace Envoy { namespace Router { -class MockRedirectEntry : public RedirectEntry { +class MockDirectResponseEntry : public DirectResponseEntry { public: - MockRedirectEntry(); - ~MockRedirectEntry(); + MockDirectResponseEntry(); + ~MockDirectResponseEntry(); - // Router::Config + // DirectResponseEntry MOCK_CONST_METHOD1(newPath, std::string(const Http::HeaderMap& headers)); - MOCK_CONST_METHOD0(redirectResponseCode, Http::Code()); + MOCK_CONST_METHOD0(responseCode, Http::Code()); }; class TestCorsPolicy : public CorsPolicy { @@ -243,7 +243,7 @@ class MockRoute : public Route { ~MockRoute(); // Router::Route - MOCK_CONST_METHOD0(redirectEntry, const RedirectEntry*()); + MOCK_CONST_METHOD0(directResponseEntry, const DirectResponseEntry*()); MOCK_CONST_METHOD0(routeEntry, const RouteEntry*()); MOCK_CONST_METHOD0(decorator, const Decorator*()); diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 516be9d7ae87c..785ad9b075667 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -187,8 +187,8 @@ bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::str bool RouterCheckTool::compareRedirectPath(ToolConfig& tool_config, const std::string& expected) { std::string actual = ""; - if (tool_config.route_->redirectEntry() != nullptr) { - actual = tool_config.route_->redirectEntry()->newPath(*tool_config.headers_); + if (tool_config.route_->directResponseEntry() != nullptr) { + actual = tool_config.route_->directResponseEntry()->newPath(*tool_config.headers_); } return compareResults(actual, expected, "path_redirect");