envoyproxy · mattklein123 · Apr 6, 2022 · Apr 4, 2022 · Apr 5, 2022 · Apr 5, 2022
diff --git a/envoy/router/router.h b/envoy/router/router.h
@@ -719,35 +719,6 @@ 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.
  */
@@ -951,11 +922,6 @@ 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.

diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc
@@ -25,8 +25,6 @@ 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_;

diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h
@@ -169,11 +169,6 @@ 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,
@@ -270,9 +265,6 @@ 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_;
@@ -291,7 +283,6 @@ 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_;

diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h
@@ -497,7 +497,6 @@ 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:
@@ -587,7 +586,6 @@ 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();
   }
@@ -749,9 +747,6 @@ 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();
@@ -988,10 +983,6 @@ 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,
@@ -1020,10 +1011,6 @@ 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,
@@ -1052,10 +1039,6 @@ 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,
@@ -1084,10 +1067,6 @@ 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,
@@ -1114,10 +1093,6 @@ 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,

diff --git a/source/common/router/delegating_route_impl.cc b/source/common/router/delegating_route_impl.cc
@@ -150,10 +150,6 @@ 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();
 }

diff --git a/source/common/router/delegating_route_impl.h b/source/common/router/delegating_route_impl.h
@@ -103,7 +103,6 @@ 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;

diff --git a/test/common/http/async_client_impl_test.cc b/test/common/http/async_client_impl_test.cc
@@ -1674,9 +1674,6 @@ 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());

diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc
@@ -1074,11 +1074,6 @@ 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(),

diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc
@@ -3114,7 +3114,6 @@ 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();
@@ -6788,16 +6787,6 @@ 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(
@@ -6842,15 +6831,7 @@ 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();

diff --git a/test/mocks/router/mocks.cc b/test/mocks/router/mocks.cc
@@ -87,13 +87,6 @@ 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_));
@@ -107,7 +100,6 @@ 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_));

diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h
@@ -329,19 +329,6 @@ 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();
@@ -388,7 +375,6 @@ 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));
@@ -409,7 +395,6 @@ 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_;
 };