From f210831b7a341e4d654db9cdbc7ed2ceda8c9a84 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Sun, 4 Feb 2018 02:56:08 -0500 Subject: [PATCH 1/3] Expose path match criterion via route entry Signed-off-by: Matt Rice --- include/envoy/router/router.h | 33 ++++++++++++++++++++++ source/common/http/async_client_impl.cc | 2 ++ source/common/http/async_client_impl.h | 9 ++++++ source/common/router/config_impl.cc | 3 +- source/common/router/config_impl.h | 18 ++++++++++++ test/common/http/async_client_impl_test.cc | 18 ++++++++++++ test/common/router/config_impl_test.cc | 25 ++++++++++++++-- test/mocks/router/mocks.h | 1 + 8 files changed, 105 insertions(+), 4 deletions(-) diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 48278da08473..831cab67024c 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -306,6 +306,34 @@ class MetadataMatchCriteria { metadataMatchCriteria() const PURE; }; +/** + * Type of path matching that a route entry uses. + */ +enum class PathMatchType { + None, + Prefix, + Exact, + Regex, +}; + +/** + * Criterion that a route entry uses for matching a particular path. + */ +class PathMatchCriterion { +public: + virtual ~PathMatchCriterion() {} + + /** + * @return PathMatchType type of path match. + */ + virtual PathMatchType matchType() const PURE; + + /** + * @return const std::string& the string with which to compare paths. + */ + virtual const std::string& matcher() const PURE; +}; + /** * An individual resolved route entry. */ @@ -415,6 +443,11 @@ class RouteEntry : public ResponseEntry { * this route. */ virtual const envoy::api::v2::core::Metadata& metadata() const PURE; + + /** + * @return const PathMatchCriterion& the match criterion for this route. + */ + virtual const PathMatchCriterion& pathMatchCriterion() const PURE; }; /** diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index 8cfaf3f3f3a7..eacc672dea78 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -22,6 +22,8 @@ const AsyncStreamImpl::NullVirtualHost AsyncStreamImpl::RouteEntryImpl::virtual_ const AsyncStreamImpl::NullRateLimitPolicy AsyncStreamImpl::NullVirtualHost::rate_limit_policy_; const std::multimap AsyncStreamImpl::RouteEntryImpl::opaque_config_; const envoy::api::v2::core::Metadata AsyncStreamImpl::RouteEntryImpl::metadata_; +const AsyncStreamImpl::NullPathMatchCriterion + AsyncStreamImpl::RouteEntryImpl::path_match_criterion_; AsyncClientImpl::AsyncClientImpl(const Upstream::ClusterInfo& cluster, Stats::Store& stats_store, Event::Dispatcher& dispatcher, diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index 04ea3bd92f9b..b931e007eeae 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -135,6 +135,11 @@ class AsyncStreamImpl : public AsyncClient::Stream, static const NullRateLimitPolicy rate_limit_policy_; }; + struct NullPathMatchCriterion : public Router::PathMatchCriterion { + Router::PathMatchType matchType() const override { return Router::PathMatchType::None; } + const std::string& matcher() const override { return EMPTY_STRING; } + }; + struct RouteEntryImpl : public Router::RouteEntry { RouteEntryImpl(const std::string& cluster_name, const Optional& timeout) @@ -175,6 +180,9 @@ class AsyncStreamImpl : public AsyncClient::Stream, bool useWebSocket() const override { return false; } bool includeVirtualHostRateLimits() const override { return true; } const envoy::api::v2::core::Metadata& metadata() const override { return metadata_; } + const Router::PathMatchCriterion& pathMatchCriterion() const override { + return path_match_criterion_; + } static const NullRateLimitPolicy rate_limit_policy_; static const NullRetryPolicy retry_policy_; @@ -182,6 +190,7 @@ class AsyncStreamImpl : public AsyncClient::Stream, static const NullVirtualHost virtual_host_; static const std::multimap opaque_config_; static const envoy::api::v2::core::Metadata metadata_; + static const NullPathMatchCriterion path_match_criterion_; const std::string& cluster_name_; Optional timeout_; diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 5787929f15f0..317c530c7cbb 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -592,7 +592,8 @@ RegexRouteEntryImpl::RegexRouteEntryImpl(const VirtualHostImpl& vhost, const envoy::api::v2::route::Route& route, Runtime::Loader& loader) : RouteEntryImplBase(vhost, route, loader), - regex_(RegexUtil::parseRegex(route.match().regex().c_str())) {} + regex_(RegexUtil::parseRegex(route.match().regex().c_str())), + regex_str_(route.match().regex()) {} void RegexRouteEntryImpl::finalizeRequestHeaders( Http::HeaderMap& headers, const RequestInfo::RequestInfo& request_info) const { diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index 5c00a16fefb0..5b6483ff61b6 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -290,6 +290,7 @@ class RouteEntryImplBase : public RouteEntry, public Matchable, public DirectResponseEntry, public Route, + public PathMatchCriterion, public std::enable_shared_from_this { public: /** @@ -334,6 +335,7 @@ class RouteEntryImplBase : public RouteEntry, } bool includeVirtualHostRateLimits() const override { return include_vh_rate_limits_; } const envoy::api::v2::core::Metadata& metadata() const override { return metadata_; } + const PathMatchCriterion& pathMatchCriterion() const override { return *this; } // Router::DirectResponseEntry std::string newPath(const Http::HeaderMap& headers) const override; @@ -408,6 +410,9 @@ class RouteEntryImplBase : public RouteEntry, return parent_->includeVirtualHostRateLimits(); } const envoy::api::v2::core::Metadata& metadata() const override { return parent_->metadata(); } + const PathMatchCriterion& pathMatchCriterion() const override { + return parent_->pathMatchCriterion(); + } // Router::Route const DirectResponseEntry* directResponseEntry() const override { return nullptr; } @@ -512,6 +517,10 @@ class PrefixRouteEntryImpl : public RouteEntryImplBase { void finalizeRequestHeaders(Http::HeaderMap& headers, const RequestInfo::RequestInfo& request_info) const override; + // Router::PathMatchCriterion + const std::string& matcher() const override { return prefix_; } + PathMatchType matchType() const override { return PathMatchType::Prefix; } + // Router::Matchable RouteConstSharedPtr matches(const Http::HeaderMap& headers, uint64_t random_value) const override; @@ -531,6 +540,10 @@ class PathRouteEntryImpl : public RouteEntryImplBase { void finalizeRequestHeaders(Http::HeaderMap& headers, const RequestInfo::RequestInfo& request_info) const override; + // Router::PathMatchCriterion + const std::string& matcher() const override { return path_; } + PathMatchType matchType() const override { return PathMatchType::Exact; } + // Router::Matchable RouteConstSharedPtr matches(const Http::HeaderMap& headers, uint64_t random_value) const override; @@ -550,11 +563,16 @@ class RegexRouteEntryImpl : public RouteEntryImplBase { void finalizeRequestHeaders(Http::HeaderMap& headers, const RequestInfo::RequestInfo& request_info) const override; + // Router::PathMatchCriterion + const std::string& matcher() const override { return regex_str_; } + PathMatchType matchType() const override { return PathMatchType::Regex; } + // Router::Matchable RouteConstSharedPtr matches(const Http::HeaderMap& headers, uint64_t random_value) const override; private: const std::regex regex_; + const std::string regex_str_; }; /** diff --git a/test/common/http/async_client_impl_test.cc b/test/common/http/async_client_impl_test.cc index c4273f1b4242..3147a5cb7af1 100644 --- a/test/common/http/async_client_impl_test.cc +++ b/test/common/http/async_client_impl_test.cc @@ -863,6 +863,24 @@ TEST_F(AsyncClientImplTest, WatermarkCallbacks) { EXPECT_CALL(stream_callbacks_, onReset()); } +TEST_F(AsyncClientImplTest, NullPathMatchCriterion) { + TestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + AsyncClient::Stream* stream = + client_.start(stream_callbacks_, Optional(), false); + stream->sendHeaders(headers, false); + Http::StreamDecoderFilterCallbacks* filter_callbacks = + static_cast(stream); + auto route = filter_callbacks->route(); + ASSERT_NE(nullptr, route); + auto route_entry = route->routeEntry(); + ASSERT_NE(nullptr, route_entry); + auto& path_match_criterion = route_entry->pathMatchCriterion(); + EXPECT_EQ("", path_match_criterion.matcher()); + EXPECT_EQ(Router::PathMatchType::None, path_match_criterion.matchType()); + EXPECT_CALL(stream_callbacks_, onReset()); +} + } // namespace } // namespace Http } // namespace Envoy diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index d93ffd61e934..79dececefc85 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -3773,13 +3773,27 @@ name: foo EnvoyException, "response body size is 4097 bytes; maximum is 4096"); } -TEST(RouteConfigurationV2, Metadata) { +void checkPathMatchCriterion(const Route* route, const std::string& expected_matcher, + PathMatchType expected_type) { + ASSERT_NE(nullptr, route); + auto route_entry = route->routeEntry(); + ASSERT_NE(nullptr, route_entry); + auto& match_criterion = route_entry->pathMatchCriterion(); + EXPECT_EQ(expected_matcher, match_criterion.matcher()); + EXPECT_EQ(expected_type, match_criterion.matchType()); +} + +TEST(RouteConfigurationV2, RouteConfigGetters) { std::string yaml = R"EOF( name: foo virtual_hosts: - name: bar domains: ["*"] routes: + - match: { regex: "/rege[xy]" } + route: { cluster: ww2 } + - match: { path: "/exact-path" } + route: { cluster: ww2 } - match: { prefix: "/"} route: { cluster: www2 } metadata: { filter_metadata: { com.bar.foo: { baz: test_value } } } @@ -3789,9 +3803,14 @@ name: foo NiceMock cm; ConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), runtime, cm, true); - auto* route_entry = config.route(genHeaders("www.foo.com", "/", "GET"), 0)->routeEntry(); + checkPathMatchCriterion(config.route(genHeaders("www.foo.com", "/regex", "GET"), 0).get(), + "/rege[xy]", PathMatchType::Regex); + checkPathMatchCriterion(config.route(genHeaders("www.foo.com", "/exact-path", "GET"), 0).get(), + "/exact-path", PathMatchType::Exact); + auto route = config.route(genHeaders("www.foo.com", "/", "GET"), 0); + checkPathMatchCriterion(route.get(), "/", PathMatchType::Prefix); - const auto& metadata = route_entry->metadata(); + const auto& metadata = route->routeEntry()->metadata(); EXPECT_EQ("test_value", Envoy::Config::Metadata::metadataValue(metadata, "com.bar.foo", "baz").string_value()); diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 716871a142b7..e369e10fa09a 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -216,6 +216,7 @@ class MockRouteEntry : public RouteEntry { MOCK_CONST_METHOD0(includeVirtualHostRateLimits, bool()); MOCK_CONST_METHOD0(corsPolicy, const CorsPolicy*()); MOCK_CONST_METHOD0(metadata, const envoy::api::v2::core::Metadata&()); + MOCK_CONST_METHOD0(pathMatchCriterion, const PathMatchCriterion&()); std::string cluster_name_{"fake_cluster"}; std::multimap opaque_config_; From acdf74cdf036aa77c2c9d84a723e8743cdd25b07 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Mon, 5 Feb 2018 17:44:15 -0500 Subject: [PATCH 2/3] Fixed compile error Signed-off-by: Matt Rice --- test/common/router/config_impl_test.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 680fac120a78..bf423b48de3d 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -3810,7 +3810,8 @@ name: foo auto route = config.route(genHeaders("www.foo.com", "/", "GET"), 0); checkPathMatchCriterion(route.get(), "/", PathMatchType::Prefix); - const auto& metadata = route->routeEntry()->metadata(); + auto route_entry = route->routeEntry(); + const auto& metadata = route_entry->metadata(); EXPECT_EQ("test_value", Envoy::Config::Metadata::metadataValue(metadata, "com.bar.foo", "baz").string_value()); From c272cf0bf0ae245733a11b91a06ba36839d1ecd9 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Mon, 5 Feb 2018 22:47:28 -0500 Subject: [PATCH 3/3] Corrected const-ness in test Signed-off-by: Matt Rice --- test/common/router/config_impl_test.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index bf423b48de3d..ff91e8c60453 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -3776,9 +3776,9 @@ name: foo void checkPathMatchCriterion(const Route* route, const std::string& expected_matcher, PathMatchType expected_type) { ASSERT_NE(nullptr, route); - auto route_entry = route->routeEntry(); + const auto route_entry = route->routeEntry(); ASSERT_NE(nullptr, route_entry); - auto& match_criterion = route_entry->pathMatchCriterion(); + const auto& match_criterion = route_entry->pathMatchCriterion(); EXPECT_EQ(expected_matcher, match_criterion.matcher()); EXPECT_EQ(expected_type, match_criterion.matchType()); } @@ -3801,16 +3801,16 @@ name: foo NiceMock runtime; NiceMock cm; - ConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), runtime, cm, true); + const ConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), runtime, cm, true); checkPathMatchCriterion(config.route(genHeaders("www.foo.com", "/regex", "GET"), 0).get(), "/rege[xy]", PathMatchType::Regex); checkPathMatchCriterion(config.route(genHeaders("www.foo.com", "/exact-path", "GET"), 0).get(), "/exact-path", PathMatchType::Exact); - auto route = config.route(genHeaders("www.foo.com", "/", "GET"), 0); + const auto route = config.route(genHeaders("www.foo.com", "/", "GET"), 0); checkPathMatchCriterion(route.get(), "/", PathMatchType::Prefix); - auto route_entry = route->routeEntry(); + const auto route_entry = route->routeEntry(); const auto& metadata = route_entry->metadata(); EXPECT_EQ("test_value",