Revert "router: Remove PathMatchType and PathMatchCriterion (envoypro…

…xy#20649)"

This reverts commit 7014ce6.

Signed-off-by: Adi Suissa-Peleg <[email protected]>

envoy/router/router.h

@@ -719,6 +719,35 @@ class TlsContextMatchCriteria {
 
 using TlsContextMatchCriteriaConstPtr = std::unique_ptr<const TlsContextMatchCriteria>;
 
+/**
+ * Type of path matching that a route entry uses.
+ */
+enum class PathMatchType {
+  None,
+  Prefix,
+  Exact,
+  Regex,
+  PathSeparatedPrefix,
+};
+
+/**
+ * Criterion that a route entry uses for matching a particular path.
+ */
+class PathMatchCriterion {
+public:
+  virtual ~PathMatchCriterion() = default;
+
+  /**
+   * @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;
+};
+
 /**
  * Base class for all route typed metadata factories.
  */
@@ -922,6 +951,11 @@ class RouteEntry : public ResponseEntry {
    */
   virtual const TlsContextMatchCriteria* tlsContextMatchCriteria() const PURE;
 
+  /**
+   * @return const PathMatchCriterion& the match criterion for this route.
+   */
+  virtual const PathMatchCriterion& pathMatchCriterion() const PURE;
+
   /**
    * True if the virtual host this RouteEntry belongs to is configured to include the attempt
    * count header.

source/common/http/async_client_impl.cc

@@ -25,6 +25,8 @@ const AsyncStreamImpl::NullVirtualHost AsyncStreamImpl::RouteEntryImpl::virtual_
 const AsyncStreamImpl::NullRateLimitPolicy AsyncStreamImpl::NullVirtualHost::rate_limit_policy_;
 const AsyncStreamImpl::NullConfig AsyncStreamImpl::NullVirtualHost::route_configuration_;
 const std::multimap<std::string, std::string> AsyncStreamImpl::RouteEntryImpl::opaque_config_;
+const AsyncStreamImpl::NullPathMatchCriterion
+    AsyncStreamImpl::RouteEntryImpl::path_match_criterion_;
 const absl::optional<envoy::config::route::v3::RouteAction::UpgradeConfig::ConnectConfig>
     AsyncStreamImpl::RouteEntryImpl::connect_config_nullopt_;
 const std::list<LowerCaseString> AsyncStreamImpl::NullConfig::internal_only_headers_;

source/common/http/async_client_impl.h

@@ -169,6 +169,11 @@ class AsyncStreamImpl : public AsyncClient::Stream,
     static const NullConfig route_configuration_;
   };
 
+  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(
         AsyncClientImpl& parent, const absl::optional<std::chrono::milliseconds>& timeout,
@@ -265,6 +270,9 @@ class AsyncStreamImpl : public AsyncClient::Stream,
     bool autoHostRewrite() const override { return false; }
     bool appendXfh() const override { return false; }
     bool includeVirtualHostRateLimits() const override { return true; }
+    const Router::PathMatchCriterion& pathMatchCriterion() const override {
+      return path_match_criterion_;
+    }
 
     const absl::optional<ConnectConfig>& connectConfig() const override {
       return connect_config_nullopt_;
@@ -283,6 +291,7 @@ class AsyncStreamImpl : public AsyncClient::Stream,
     static const std::vector<Router::ShadowPolicyPtr> shadow_policies_;
     static const NullVirtualHost virtual_host_;
     static const std::multimap<std::string, std::string> opaque_config_;
+    static const NullPathMatchCriterion path_match_criterion_;
 
     Router::RouteEntry::UpgradeMap upgrade_map_;
     const std::string& cluster_name_;

source/common/router/config_impl.h

@@ -497,6 +497,7 @@ class RouteEntryImplBase : public RouteEntry,
                            public Matchable,
                            public DirectResponseEntry,
                            public Route,
+                           public PathMatchCriterion,
                            public std::enable_shared_from_this<RouteEntryImplBase>,
                            Logger::Loggable<Logger::Id::router> {
 public:
@@ -586,6 +587,7 @@ class RouteEntryImplBase : public RouteEntry,
   bool includeVirtualHostRateLimits() const override { return include_vh_rate_limits_; }
   const envoy::config::core::v3::Metadata& metadata() const override { return metadata_; }
   const Envoy::Config::TypedMetadata& typedMetadata() const override { return typed_metadata_; }
+  const PathMatchCriterion& pathMatchCriterion() const override { return *this; }
   bool includeAttemptCountInRequest() const override {
     return vhost_.includeAttemptCountInRequest();
   }
@@ -747,6 +749,9 @@ class RouteEntryImplBase : public RouteEntry,
     const Envoy::Config::TypedMetadata& typedMetadata() const override {
       return parent_->typedMetadata();
     }
+    const PathMatchCriterion& pathMatchCriterion() const override {
+      return parent_->pathMatchCriterion();
+    }
 
     bool includeAttemptCountInRequest() const override {
       return parent_->includeAttemptCountInRequest();
@@ -983,6 +988,10 @@ class PrefixRouteEntryImpl : public RouteEntryImplBase {
                        Server::Configuration::ServerFactoryContext& factory_context,
                        ProtobufMessage::ValidationVisitor& validator);
 
+  // Router::PathMatchCriterion
+  const std::string& matcher() const override { return prefix_; }
+  PathMatchType matchType() const override { return PathMatchType::Prefix; }
+
   // Router::Matchable
   RouteConstSharedPtr matches(const Http::RequestHeaderMap& headers,
                               const StreamInfo::StreamInfo& stream_info,
@@ -1011,6 +1020,10 @@ class PathRouteEntryImpl : public RouteEntryImplBase {
                      Server::Configuration::ServerFactoryContext& factory_context,
                      ProtobufMessage::ValidationVisitor& validator);
 
+  // Router::PathMatchCriterion
+  const std::string& matcher() const override { return path_; }
+  PathMatchType matchType() const override { return PathMatchType::Exact; }
+
   // Router::Matchable
   RouteConstSharedPtr matches(const Http::RequestHeaderMap& headers,
                               const StreamInfo::StreamInfo& stream_info,
@@ -1039,6 +1052,10 @@ class RegexRouteEntryImpl : public RouteEntryImplBase {
                       Server::Configuration::ServerFactoryContext& factory_context,
                       ProtobufMessage::ValidationVisitor& validator);
 
+  // Router::PathMatchCriterion
+  const std::string& matcher() const override { return regex_str_; }
+  PathMatchType matchType() const override { return PathMatchType::Regex; }
+
   // Router::Matchable
   RouteConstSharedPtr matches(const Http::RequestHeaderMap& headers,
                               const StreamInfo::StreamInfo& stream_info,
@@ -1067,6 +1084,10 @@ class ConnectRouteEntryImpl : public RouteEntryImplBase {
                         Server::Configuration::ServerFactoryContext& factory_context,
                         ProtobufMessage::ValidationVisitor& validator);
 
+  // Router::PathMatchCriterion
+  const std::string& matcher() const override { return EMPTY_STRING; }
+  PathMatchType matchType() const override { return PathMatchType::None; }
+
   // Router::Matchable
   RouteConstSharedPtr matches(const Http::RequestHeaderMap& headers,
                               const StreamInfo::StreamInfo& stream_info,
@@ -1093,6 +1114,10 @@ class PathSeparatedPrefixRouteEntryImpl : public RouteEntryImplBase {
                                     Server::Configuration::ServerFactoryContext& factory_context,
                                     ProtobufMessage::ValidationVisitor& validator);
 
+  // Router::PathMatchCriterion
+  const std::string& matcher() const override { return prefix_; }
+  PathMatchType matchType() const override { return PathMatchType::PathSeparatedPrefix; }
+
   // Router::Matchable
   RouteConstSharedPtr matches(const Http::RequestHeaderMap& headers,
                               const StreamInfo::StreamInfo& stream_info,

source/common/router/delegating_route_impl.cc

@@ -150,6 +150,10 @@ const TlsContextMatchCriteria* DelegatingRouteEntry::tlsContextMatchCriteria() c
   return base_route_->routeEntry()->tlsContextMatchCriteria();
 }
 
+const PathMatchCriterion& DelegatingRouteEntry::pathMatchCriterion() const {
+  return base_route_->routeEntry()->pathMatchCriterion();
+}
+
 bool DelegatingRouteEntry::includeAttemptCountInRequest() const {
   return base_route_->routeEntry()->includeAttemptCountInRequest();
 }

source/common/router/delegating_route_impl.h

@@ -103,6 +103,7 @@ class DelegatingRouteEntry : public Router::RouteEntry {
   const std::multimap<std::string, std::string>& opaqueConfig() const override;
   bool includeVirtualHostRateLimits() const override;
   const TlsContextMatchCriteria* tlsContextMatchCriteria() const override;
+  const PathMatchCriterion& pathMatchCriterion() const override;
   bool includeAttemptCountInRequest() const override;
   bool includeAttemptCountInResponse() const override;
   const UpgradeMap& upgradeMap() const override;

test/common/http/async_client_impl_test.cc

@@ -1674,6 +1674,9 @@ TEST_F(AsyncClientImplTest, RdsGettersTest) {
   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());
   const auto& route_config = route_entry->virtualHost().routeConfig();
   EXPECT_EQ("", route_config.name());
   EXPECT_EQ(0, route_config.internalOnlyHeaders().size());

test/common/http/conn_manager_impl_test.cc

@@ -1074,6 +1074,11 @@ TEST_F(HttpConnectionManagerImplTest, DelegatingRouteEntryAllCalls) {
         EXPECT_EQ(default_route->routeEntry()->tlsContextMatchCriteria(),
                   delegating_route_foo->routeEntry()->tlsContextMatchCriteria());
 
+        EXPECT_EQ(default_route->routeEntry()->pathMatchCriterion().matcher(),
+                  delegating_route_foo->routeEntry()->pathMatchCriterion().matcher());
+        EXPECT_EQ(default_route->routeEntry()->pathMatchCriterion().matchType(),
+                  delegating_route_foo->routeEntry()->pathMatchCriterion().matchType());
+
         EXPECT_EQ(default_route->routeEntry()->includeAttemptCountInRequest(),
                   delegating_route_foo->routeEntry()->includeAttemptCountInRequest());
         EXPECT_EQ(default_route->routeEntry()->includeAttemptCountInResponse(),

test/common/router/config_impl_test.cc

@@ -3114,6 +3114,7 @@ TEST_F(RouteMatcherTest, ClusterHeader) {
     route->routeEntry()->virtualCluster(headers);
     route->routeEntry()->virtualHost();
     route->routeEntry()->virtualHost().rateLimitPolicy();
+    route->routeEntry()->pathMatchCriterion();
     route->routeEntry()->hedgePolicy();
     route->routeEntry()->maxGrpcTimeout();
     route->routeEntry()->grpcTimeoutOffset();
@@ -6787,6 +6788,16 @@ TEST_F(RouteConfigurationV2, DirectResponseTooLarge) {
       EnvoyException, "response body size is 4097 bytes; maximum is 4096");
 }
 
+void checkPathMatchCriterion(const Route* route, const std::string& expected_matcher,
+                             PathMatchType expected_type) {
+  ASSERT_NE(nullptr, route);
+  const auto route_entry = route->routeEntry();
+  ASSERT_NE(nullptr, route_entry);
+  const auto& match_criterion = route_entry->pathMatchCriterion();
+  EXPECT_EQ(expected_matcher, match_criterion.matcher());
+  EXPECT_EQ(expected_type, match_criterion.matchType());
+}
+
 // Test loading broken config throws EnvoyException.
 TEST_F(RouteConfigurationV2, BrokenTypedMetadata) {
   const std::string yaml = R"EOF(
@@ -6831,7 +6842,15 @@ name: foo
   factory_context_.cluster_manager_.initializeClusters({"ww2", "www2"}, {});
   const TestConfigImpl config(parseRouteConfigurationFromYaml(yaml), factory_context_, 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);
+  checkPathMatchCriterion(
+      config.route(genHeaders("www.foo.com", "/path/separated", "GET"), 0).get(), "/path/separated",
+      PathMatchType::PathSeparatedPrefix);
   const auto route = config.route(genHeaders("www.foo.com", "/", "GET"), 0);
+  checkPathMatchCriterion(route.get(), "/", PathMatchType::Prefix);
 
   const auto route_entry = route->routeEntry();
   const auto& metadata = route->metadata();

test/mocks/router/mocks.cc

@@ -87,6 +87,13 @@ MockMetadataMatchCriteria::~MockMetadataMatchCriteria() = default;
 MockTlsContextMatchCriteria::MockTlsContextMatchCriteria() = default;
 MockTlsContextMatchCriteria::~MockTlsContextMatchCriteria() = default;
 
+MockPathMatchCriterion::MockPathMatchCriterion() {
+  ON_CALL(*this, matchType()).WillByDefault(ReturnPointee(&type_));
+  ON_CALL(*this, matcher()).WillByDefault(ReturnPointee(&matcher_));
+}
+
+MockPathMatchCriterion::~MockPathMatchCriterion() = default;
+
 MockRouteEntry::MockRouteEntry() {
   ON_CALL(*this, clusterName()).WillByDefault(ReturnRef(cluster_name_));
   ON_CALL(*this, opaqueConfig()).WillByDefault(ReturnRef(opaque_config_));
@@ -100,6 +107,7 @@ MockRouteEntry::MockRouteEntry() {
   ON_CALL(*this, virtualCluster(_)).WillByDefault(Return(&virtual_cluster_));
   ON_CALL(*this, virtualHost()).WillByDefault(ReturnRef(virtual_host_));
   ON_CALL(*this, includeVirtualHostRateLimits()).WillByDefault(Return(true));
+  ON_CALL(*this, pathMatchCriterion()).WillByDefault(ReturnRef(path_match_criterion_));
   ON_CALL(*this, upgradeMap()).WillByDefault(ReturnRef(upgrade_map_));
   ON_CALL(*this, hedgePolicy()).WillByDefault(ReturnRef(hedge_policy_));
   ON_CALL(*this, routeName()).WillByDefault(ReturnRef(route_name_));

test/mocks/router/mocks.h

@@ -329,6 +329,19 @@ class MockTlsContextMatchCriteria : public TlsContextMatchCriteria {
   MOCK_METHOD(const absl::optional<bool>&, validated, (), (const));
 };
 
+class MockPathMatchCriterion : public PathMatchCriterion {
+public:
+  MockPathMatchCriterion();
+  ~MockPathMatchCriterion() override;
+
+  // Router::PathMatchCriterion
+  MOCK_METHOD(PathMatchType, matchType, (), (const));
+  MOCK_METHOD(const std::string&, matcher, (), (const));
+
+  PathMatchType type_;
+  std::string matcher_;
+};
+
 class MockRouteEntry : public RouteEntry {
 public:
   MockRouteEntry();
@@ -375,6 +388,7 @@ class MockRouteEntry : public RouteEntry {
   MOCK_METHOD(const CorsPolicy*, corsPolicy, (), (const));
   MOCK_METHOD(absl::optional<std::string>, currentUrlPathAfterRewrite,
               (const Http::RequestHeaderMap&), (const));
+  MOCK_METHOD(const PathMatchCriterion&, pathMatchCriterion, (), (const));
   MOCK_METHOD(bool, includeAttemptCountInRequest, (), (const));
   MOCK_METHOD(bool, includeAttemptCountInResponse, (), (const));
   MOCK_METHOD(const absl::optional<ConnectConfig>&, connectConfig, (), (const));
@@ -395,6 +409,7 @@ class MockRouteEntry : public RouteEntry {
   MockMetadataMatchCriteria metadata_matches_criteria_;
   MockTlsContextMatchCriteria tls_context_matches_criteria_;
   TestCorsPolicy cors_policy_;
+  testing::NiceMock<MockPathMatchCriterion> path_match_criterion_;
   UpgradeMap upgrade_map_;
   absl::optional<ConnectConfig> connect_config_;
 };