Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion api/envoy/config/route/v3/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ message WeightedCluster {
}
}

// [#next-free-field: 14]
// [#next-free-field: 15]
message RouteMatch {
option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.RouteMatch";

Expand Down Expand Up @@ -513,6 +513,17 @@ message RouteMatch {
// Note that CONNECT support is currently considered alpha in Envoy.
// [#comment: TODO(htuch): Replace the above comment with an alpha tag.]
ConnectMatcher connect_matcher = 12;

// If specified, the route is a path-separated prefix rule meaning that the
// ``:path`` header (without the query string) must either exactly match the
// ``path_separated_prefix`` or have it as a prefix, followed by ``/``
//
// For example, ``/api/dev`` would match
// ``/api/dev``, ``/api/dev/``, ``/api/dev/v1``, and ``/api/dev?param=true``
// but would not match ``/api/developer``
//
// Expect the value to not contain ``?`` or ``#`` and not to end in ``/``
string path_separated_prefix = 14 [(validate.rules).string = {pattern: "^[^?#]+[^?#/]$"}];
}

// Indicates that prefix/path matching should be case sensitive. The default
Expand Down
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ New Features
* matching: the matching API can now express a match tree that will always match by omitting a matcher at the top level.
* outlier_detection: :ref:`max_ejection_time_jitter<envoy_v3_api_field_config.cluster.v3.OutlierDetection.base_ejection_time>` configuration added to allow adding a random value to the ejection time to prevent 'thundering herd' scenarios. Defaults to 0 so as to not break or change the behavior of existing deployments.
* redis: support for hostnames returned in `cluster slots` response is now available.
* router: added a path-separated prefix matcher, to make route creation more efficient. :ref:`path_separated_prefix <envoy_v3_api_field_config.route.v3.RouteMatch.path_separated_prefix>`.
* schema_validator_tool: added ``bootstrap`` checking to the
:ref:`schema validator check tool <install_tools_schema_validator_check_tool>`.
* schema_validator_tool: added ``--fail-on-deprecated`` and ``--fail-on-wip`` to the
Expand Down
40 changes: 40 additions & 0 deletions source/common/router/config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ RouteEntryImplBaseConstSharedPtr createAndValidateRoute(
route = std::make_shared<ConnectRouteEntryImpl>(vhost, route_config, optional_http_filters,
factory_context, validator);
break;
case envoy::config::route::v3::RouteMatch::PathSpecifierCase::kPathSeparatedPrefix: {
route = std::make_shared<PathSeparatedPrefixRouteEntryImpl>(
vhost, route_config, optional_http_filters, factory_context, validator);
break;
}
case envoy::config::route::v3::RouteMatch::PathSpecifierCase::PATH_SPECIFIER_NOT_SET:
break; // throw the error below.
}
Expand Down Expand Up @@ -1394,6 +1399,41 @@ RouteConstSharedPtr ConnectRouteEntryImpl::matches(const Http::RequestHeaderMap&
return nullptr;
}

PathSeparatedPrefixRouteEntryImpl::PathSeparatedPrefixRouteEntryImpl(
const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route,
const OptionalHttpFilters& optional_http_filters,
Server::Configuration::ServerFactoryContext& factory_context,
ProtobufMessage::ValidationVisitor& validator)
: RouteEntryImplBase(vhost, route, optional_http_filters, factory_context, validator),
prefix_(route.match().path_separated_prefix()),
path_matcher_(Matchers::PathMatcher::createPrefix(prefix_, !case_sensitive_)) {}

void PathSeparatedPrefixRouteEntryImpl::rewritePathHeader(Http::RequestHeaderMap& headers,
bool insert_envoy_original_path) const {
finalizePathHeader(headers, prefix_, insert_envoy_original_path);
}

absl::optional<std::string> PathSeparatedPrefixRouteEntryImpl::currentUrlPathAfterRewrite(
const Http::RequestHeaderMap& headers) const {
return currentUrlPathAfterRewriteWithMatchedPath(headers, prefix_);
}

RouteConstSharedPtr
PathSeparatedPrefixRouteEntryImpl::matches(const Http::RequestHeaderMap& headers,
const StreamInfo::StreamInfo& stream_info,
uint64_t random_value) const {
if (!RouteEntryImplBase::matchRoute(headers, stream_info, random_value)) {
return nullptr;
Comment thread
tpetkov-VMW marked this conversation as resolved.
}
absl::string_view path = Http::PathUtil::removeQueryAndFragment(headers.getPathValue());
if (path.size() > prefix_.size() && path_matcher_->match(path) && path[prefix_.size()] == '/') {
return clusterEntry(headers, random_value);
} else if (case_sensitive_ ? path == prefix_ : absl::EqualsIgnoreCase(path, prefix_)) {
Comment thread
tpetkov-VMW marked this conversation as resolved.
Outdated
return clusterEntry(headers, random_value);
}
return nullptr;
}

VirtualHostImpl::VirtualHostImpl(
const envoy::config::route::v3::VirtualHost& virtual_host,
const OptionalHttpFilters& optional_http_filters, const ConfigImpl& global_route_config,
Expand Down
33 changes: 33 additions & 0 deletions source/common/router/config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,39 @@ class ConnectRouteEntryImpl : public RouteEntryImplBase {
bool supportsPathlessHeaders() const override { return true; }
};

/**
* Route entry implementation for path separated prefix match routing.
*/
class PathSeparatedPrefixRouteEntryImpl : public RouteEntryImplBase {
public:
PathSeparatedPrefixRouteEntryImpl(const VirtualHostImpl& vhost,
const envoy::config::route::v3::Route& route,
const OptionalHttpFilters& optional_http_filters,
Server::Configuration::ServerFactoryContext& factory_context,
ProtobufMessage::ValidationVisitor& validator);

// Router::PathMatchCriterion
const std::string& matcher() const override { return prefix_; }
PathMatchType matchType() const override { return PathMatchType::None; }
Comment thread
tpetkov-VMW marked this conversation as resolved.
Outdated
Comment thread
tpetkov-VMW marked this conversation as resolved.
Outdated

// Router::Matchable
RouteConstSharedPtr matches(const Http::RequestHeaderMap& headers,
const StreamInfo::StreamInfo& stream_info,
uint64_t random_value) const override;

// Router::DirectResponseEntry
void rewritePathHeader(Http::RequestHeaderMap& headers,
bool insert_envoy_original_path) const override;

// Router::RouteEntry
absl::optional<std::string>
currentUrlPathAfterRewrite(const Http::RequestHeaderMap& headers) const override;

private:
const std::string prefix_;
const Matchers::PathMatcherConstSharedPtr path_matcher_;
};

// Contextual information used to construct the route actions for a match tree.
struct RouteActionContext {
const VirtualHostImpl& vhost;
Expand Down
30 changes: 30 additions & 0 deletions source/extensions/filters/http/jwt_authn/matcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "source/common/common/logger.h"
#include "source/common/common/matchers.h"
#include "source/common/common/regex.h"
#include "source/common/http/path_utility.h"
#include "source/common/router/config_impl.h"

#include "absl/strings/match.h"
Expand Down Expand Up @@ -154,6 +155,33 @@ class ConnectMatcherImpl : public BaseMatcherImpl {
return false;
}
};

class PathSeparatedPrefixMatcherImpl : public BaseMatcherImpl {
public:
PathSeparatedPrefixMatcherImpl(const RequirementRule& rule)
: BaseMatcherImpl(rule), prefix_(rule.match().path_separated_prefix()),
path_matcher_(Matchers::PathMatcher::createPrefix(prefix_, !case_sensitive_)) {}

bool matches(const Http::RequestHeaderMap& headers) const override {
if (!BaseMatcherImpl::matchRoute(headers)) {
return false;
}
absl::string_view path = Http::PathUtil::removeQueryAndFragment(headers.getPathValue());
if (path.size() > prefix_.size() && path_matcher_->match(path) && path[prefix_.size()] == '/') {
Comment thread
tpetkov-VMW marked this conversation as resolved.
Outdated
ENVOY_LOG(debug, "Path-separated prefix requirement '{}' matched.", prefix_);
return true;
} else if (case_sensitive_ ? path == prefix_ : absl::EqualsIgnoreCase(path, prefix_)) {
ENVOY_LOG(debug, "Path-separated prefix requirement '{}' matched.", prefix_);
return true;
}
return false;
}

private:
// prefix string
const std::string prefix_;
const Matchers::PathMatcherConstSharedPtr path_matcher_;
};
} // namespace

MatcherConstPtr Matcher::create(const RequirementRule& rule) {
Expand All @@ -166,6 +194,8 @@ MatcherConstPtr Matcher::create(const RequirementRule& rule) {
return std::make_unique<RegexMatcherImpl>(rule);
case RouteMatch::PathSpecifierCase::kConnectMatcher:
return std::make_unique<ConnectMatcherImpl>(rule);
case RouteMatch::PathSpecifierCase::kPathSeparatedPrefix:
return std::make_unique<PathSeparatedPrefixMatcherImpl>(rule);
case RouteMatch::PathSpecifierCase::PATH_SPECIFIER_NOT_SET:
break; // Fall through to PANIC.
}
Expand Down
157 changes: 157 additions & 0 deletions test/common/router/config_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7585,6 +7585,163 @@ TEST_F(RouteMatcherTest, TlsContextMatching) {
}
}

TEST_F(RouteMatcherTest, PathSeparatedPrefixMatch) {

std::string yaml = R"EOF(
virtual_hosts:
- name: path_prefix
domains: ["*"]
routes:
- match:
path_separated_prefix: "/rest/Api"
case_sensitive: true
route: { cluster: case-sensitive-cluster}
- match:
path_separated_prefix: "/rest/api"
route: { cluster: path-separated-cluster}
- match:
path_separated_prefix: "/rewrite"
route:
prefix_rewrite: "/new/api"
cluster: rewrite-cluster
- match:
prefix: "/"
route: { cluster: default-cluster}
)EOF";

factory_context_.cluster_manager_.initializeClusters(
{"path-separated-cluster", "case-sensitive-cluster", "default-cluster", "rewrite-cluster"},
{});
TestConfigImpl config(parseRouteConfigurationFromYaml(yaml), factory_context_, true);

EXPECT_EQ("path-separated-cluster",
config.route(genHeaders("path.prefix.com", "/rest/api", "GET"), 0)
->routeEntry()
->clusterName());
EXPECT_EQ("path-separated-cluster",
config.route(genHeaders("path.prefix.com", "/rest/api?param=true", "GET"), 0)
->routeEntry()
->clusterName());
EXPECT_EQ("path-separated-cluster",
config.route(genHeaders("path.prefix.com", "/rest/api#fragment", "GET"), 0)
->routeEntry()
->clusterName());

EXPECT_EQ("path-separated-cluster",
config.route(genHeaders("path.prefix.com", "/rest/api/", "GET"), 0)
->routeEntry()
->clusterName());
EXPECT_EQ("path-separated-cluster",
config.route(genHeaders("path.prefix.com", "/rest/api/thing?param=true", "GET"), 0)
->routeEntry()
->clusterName());
EXPECT_EQ("path-separated-cluster",
config.route(genHeaders("path.prefix.com", "/rest/api/thing#fragment", "GET"), 0)
->routeEntry()
->clusterName());

EXPECT_EQ("case-sensitive-cluster",
config.route(genHeaders("path.prefix.com", "/rest/Api", "GET"), 0)
->routeEntry()
->clusterName());
EXPECT_EQ("case-sensitive-cluster",
config.route(genHeaders("path.prefix.com", "/rest/Api?param=true", "GET"), 0)
->routeEntry()
->clusterName());
EXPECT_EQ("case-sensitive-cluster",
config.route(genHeaders("path.prefix.com", "/rest/Api/", "GET"), 0)
->routeEntry()
->clusterName());
EXPECT_EQ("case-sensitive-cluster",
config.route(genHeaders("path.prefix.com", "/rest/Api/thing?param=true", "GET"), 0)
->routeEntry()
->clusterName());

EXPECT_EQ("default-cluster",
config.route(genHeaders("path.prefix.com", "/rest/apithing", "GET"), 0)
->routeEntry()
->clusterName());
EXPECT_EQ("default-cluster",
config.route(genHeaders("path.prefix.com", "/rest/Apithing", "GET"), 0)
->routeEntry()
->clusterName());

{ // Prefix rewrite exact match
NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
Http::TestRequestHeaderMapImpl headers =
genHeaders("path.prefix.com", "/rewrite?param=true#fragment", "GET");
const RouteEntry* route = config.route(headers, 0)->routeEntry();
EXPECT_EQ("/new/api?param=true#fragment", route->currentUrlPathAfterRewrite(headers));
route->finalizeRequestHeaders(headers, stream_info, true);
EXPECT_EQ("rewrite-cluster", route->clusterName());
EXPECT_EQ("/new/api?param=true#fragment", headers.get_(Http::Headers::get().Path));
EXPECT_EQ("path.prefix.com", headers.get_(Http::Headers::get().Host));
}

{ // Prefix rewrite long match
NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
Http::TestRequestHeaderMapImpl headers =
genHeaders("path.prefix.com", "/rewrite/this?param=true#fragment", "GET");
const RouteEntry* route = config.route(headers, 0)->routeEntry();
EXPECT_EQ("/new/api/this?param=true#fragment", route->currentUrlPathAfterRewrite(headers));
route->finalizeRequestHeaders(headers, stream_info, true);
EXPECT_EQ("rewrite-cluster", route->clusterName());
EXPECT_EQ("/new/api/this?param=true#fragment", headers.get_(Http::Headers::get().Path));
EXPECT_EQ("path.prefix.com", headers.get_(Http::Headers::get().Host));
}
}

TEST_F(RouteMatcherTest, PathSeparatedPrefixMatchTrailingSlash) {

std::string yaml = R"EOF(
Comment thread
tpetkov-VMW marked this conversation as resolved.
Outdated
virtual_hosts:
- name: path_prefix
domains: ["*"]
routes:
- match:
path_separated_prefix: "/rest/api/"
route: { cluster: some-cluster }
)EOF";

factory_context_.cluster_manager_.initializeClusters({"some-cluster"}, {});
EXPECT_THROW(TestConfigImpl(parseRouteConfigurationFromYaml(yaml), factory_context_, true),
EnvoyException);
}

TEST_F(RouteMatcherTest, PathSeparatedPrefixMatchQueryParam) {

std::string yaml = R"EOF(
virtual_hosts:
- name: path_prefix
domains: ["*"]
routes:
- match:
path_separated_prefix: "/rest/api?query=1"
route: { cluster: some-cluster }
)EOF";

factory_context_.cluster_manager_.initializeClusters({"some-cluster"}, {});
EXPECT_THROW(TestConfigImpl(parseRouteConfigurationFromYaml(yaml), factory_context_, true),
EnvoyException);
}

TEST_F(RouteMatcherTest, PathSeparatedPrefixMatchFragment) {

std::string yaml = R"EOF(
virtual_hosts:
- name: path_prefix
domains: ["*"]
routes:
- match:
path_separated_prefix: "/rest/api#fragment"
route: { cluster: some-cluster }
)EOF";

factory_context_.cluster_manager_.initializeClusters({"some-cluster"}, {});
EXPECT_THROW(TestConfigImpl(parseRouteConfigurationFromYaml(yaml), factory_context_, true),
EnvoyException);
}

TEST_F(RouteConfigurationV2, RegexPrefixWithNoRewriteWorksWhenPathChanged) {

// Setup regex route entry. the regex is trivial, that's ok as we only want to test that
Expand Down
Loading