http: refactory Http::Utility::resolveMostSpecificPerFilterConfig method

source/common/http/utility.cc

@@ -888,18 +888,6 @@ void Utility::transformUpgradeResponseFromH2toH1(ResponseHeaderMap& headers,
   }
 }
 
-const Router::RouteSpecificFilterConfig*
-Utility::resolveMostSpecificPerFilterConfigGeneric(const std::string& filter_name,
-                                                   const Router::RouteConstSharedPtr& route) {
-
-  const Router::RouteSpecificFilterConfig* maybe_filter_config{};
-  traversePerFilterConfigGeneric(
-      filter_name, route, [&maybe_filter_config](const Router::RouteSpecificFilterConfig& cfg) {
-        maybe_filter_config = &cfg;
-      });
-  return maybe_filter_config;
-}
-
 void Utility::traversePerFilterConfigGeneric(
     const std::string& filter_name, const Router::RouteConstSharedPtr& route,
     std::function<void(const Router::RouteSpecificFilterConfig&)> cb) {

source/common/http/utility.h

@@ -487,9 +487,10 @@ const ConfigType* resolveMostSpecificPerFilterConfig(const std::string& filter_n
                                                      const Router::RouteConstSharedPtr& route) {
   static_assert(std::is_base_of<Router::RouteSpecificFilterConfig, ConfigType>::value,
                 "ConfigType must be a subclass of Router::RouteSpecificFilterConfig");
-  const Router::RouteSpecificFilterConfig* generic_config =
-      resolveMostSpecificPerFilterConfigGeneric(filter_name, route);
-  return dynamic_cast<const ConfigType*>(generic_config);
+  if (!route || !route->routeEntry()) {
+    return nullptr;
+  }
+  return route->routeEntry()->mostSpecificPerFilterConfigTyped<ConfigType>(filter_name);
 }
 
 /**

source/extensions/filters/http/aws_lambda/aws_lambda_filter.cc

@@ -120,12 +120,8 @@ Filter::Filter(const FilterSettings& settings, const FilterStats& stats,
     : settings_(settings), stats_(stats), sigv4_signer_(sigv4_signer) {}
 
 absl::optional<FilterSettings> Filter::getRouteSpecificSettings() const {
-  if (!decoder_callbacks_->route() || !decoder_callbacks_->route()->routeEntry()) {
-    return absl::nullopt;
-  }
-  const auto* route_entry = decoder_callbacks_->route()->routeEntry();
-  const auto* settings = route_entry->mostSpecificPerFilterConfigTyped<FilterSettings>(
-      HttpFilterNames::get().AwsLambda);
+  const auto* settings = Http::Utility::resolveMostSpecificPerFilterConfig<FilterSettings>(
+      HttpFilterNames::get().AwsLambda, decoder_callbacks_->route());
   if (!settings) {
     return absl::nullopt;
   }

source/extensions/filters/http/buffer/buffer_filter.cc

@@ -42,17 +42,10 @@ BufferFilter::BufferFilter(BufferFilterConfigSharedPtr config)
 void BufferFilter::initConfig() {
   ASSERT(!config_initialized_);
   config_initialized_ = true;
-
   settings_ = config_->settings();
 
-  if (!callbacks_->route() || !callbacks_->route()->routeEntry()) {
-    return;
-  }
-
-  const std::string& name = HttpFilterNames::get().Buffer;
-  const auto* entry = callbacks_->route()->routeEntry();
-  const auto* route_local = entry->mostSpecificPerFilterConfigTyped<BufferFilterSettings>(name);
-
+  const auto* route_local = Http::Utility::resolveMostSpecificPerFilterConfig<BufferFilterSettings>(
+      HttpFilterNames::get().Buffer, callbacks_->route());
   settings_ = route_local ? route_local : settings_;
 }
 

source/extensions/filters/http/dynamic_forward_proxy/BUILD

@@ -15,6 +15,7 @@ envoy_cc_library(
     hdrs = ["proxy_filter.h"],
     deps = [
         "//include/envoy/http:filter_interface",
+        "//source/common/http:header_utility_lib",
         "//source/extensions/clusters:well_known_names",
         "//source/extensions/common/dynamic_forward_proxy:dns_cache_interface",
         "//source/extensions/filters/http:well_known_names",

source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc

@@ -4,6 +4,8 @@
 #include "envoy/config/core/v3/base.pb.h"
 #include "envoy/extensions/filters/http/dynamic_forward_proxy/v3/dynamic_forward_proxy.pb.h"
 
+#include "common/http/utility.h"
+
 #include "extensions/clusters/well_known_names.h"
 #include "extensions/common/dynamic_forward_proxy/dns_cache.h"
 #include "extensions/filters/http/well_known_names.h"
@@ -87,8 +89,9 @@ Http::FilterHeadersStatus ProxyFilter::decodeHeaders(Http::RequestHeaderMap& hea
   }
 
   // Check for per route filter config.
-  const auto* config = route_entry->mostSpecificPerFilterConfigTyped<ProxyPerRouteConfig>(
-      HttpFilterNames::get().DynamicForwardProxy);
+  const auto* config = Http::Utility::resolveMostSpecificPerFilterConfig<ProxyPerRouteConfig>(
+      HttpFilterNames::get().DynamicForwardProxy, route);
+
   if (config != nullptr) {
     const auto& host_rewrite = config->hostRewrite();
     if (!host_rewrite.empty()) {

source/extensions/filters/http/fault/fault_filter.cc

@@ -113,14 +113,9 @@ Http::FilterHeadersStatus FaultFilter::decodeHeaders(Http::RequestHeaderMap& hea
   // faults. In other words, runtime is supported only when faults are
   // configured at the filter level.
   fault_settings_ = config_->settings();
-  if (decoder_callbacks_->route() && decoder_callbacks_->route()->routeEntry()) {
-    const std::string& name = Extensions::HttpFilters::HttpFilterNames::get().Fault;
-    const auto* route_entry = decoder_callbacks_->route()->routeEntry();
-
-    const auto* per_route_settings =
-        route_entry->mostSpecificPerFilterConfigTyped<FaultSettings>(name);
-    fault_settings_ = per_route_settings ? per_route_settings : fault_settings_;
-  }
+  const auto* per_route_settings = Http::Utility::resolveMostSpecificPerFilterConfig<FaultSettings>(
+      Extensions::HttpFilters::HttpFilterNames::get().Fault, decoder_callbacks_->route());
+  fault_settings_ = per_route_settings ? per_route_settings : fault_settings_;
 
   if (!matchesTargetUpstreamCluster()) {
     return Http::FilterHeadersStatus::Continue;

source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc

@@ -419,14 +419,8 @@ JsonTranscoderConfig::translateProtoMessageToJson(const Protobuf::Message& messa
 JsonTranscoderFilter::JsonTranscoderFilter(JsonTranscoderConfig& config) : config_(config) {}
 
 void JsonTranscoderFilter::initPerRouteConfig() {
-  if (!decoder_callbacks_->route() || !decoder_callbacks_->route()->routeEntry()) {
-    per_route_config_ = &config_;
-    return;
-  }
-
-  const std::string& name = HttpFilterNames::get().GrpcJsonTranscoder;
-  const auto* entry = decoder_callbacks_->route()->routeEntry();
-  const auto* route_local = entry->mostSpecificPerFilterConfigTyped<JsonTranscoderConfig>(name);
+  const auto* route_local = Http::Utility::resolveMostSpecificPerFilterConfig<JsonTranscoderConfig>(
+      HttpFilterNames::get().GrpcJsonTranscoder, decoder_callbacks_->route());
 
   per_route_config_ = route_local ? route_local : &config_;
 }

source/extensions/filters/http/kill_request/kill_request_filter.cc

@@ -6,6 +6,7 @@
 #include "envoy/extensions/filters/http/kill_request/v3/kill_request.pb.h"
 #include "envoy/http/header_map.h"
 
+#include "common/http/utility.h"
 #include "common/protobuf/utility.h"
 
 #include "extensions/filters/http/well_known_names.h"
@@ -52,19 +53,15 @@ Http::FilterHeadersStatus KillRequestFilter::decodeHeaders(Http::RequestHeaderMa
   }
 
   // Route-level configuration overrides filter-level configuration.
-  if (decoder_callbacks_->route() && decoder_callbacks_->route()->routeEntry()) {
-    const std::string& name = Extensions::HttpFilters::HttpFilterNames::get().KillRequest;
-    const auto* route_entry = decoder_callbacks_->route()->routeEntry();
-
-    const auto* per_route_kill_settings =
-        route_entry->mostSpecificPerFilterConfigTyped<KillSettings>(name);
-
-    if (per_route_kill_settings) {
-      is_correct_direction = per_route_kill_settings->getDirection() == KillRequest::REQUEST;
-      // Allocate the probability into kill_request in case the lifetime of
-      // per_route_kill_settings does not match that of kill_request_
-      kill_request_.mutable_probability()->CopyFrom(per_route_kill_settings->getProbability());
-    }
+  const auto* per_route_kill_settings =
+      Http::Utility::resolveMostSpecificPerFilterConfig<KillSettings>(
+          Extensions::HttpFilters::HttpFilterNames::get().KillRequest, decoder_callbacks_->route());
+
+  if (per_route_kill_settings) {
+    is_correct_direction = per_route_kill_settings->getDirection() == KillRequest::REQUEST;
+    // Allocate the probability into kill_request in case the lifetime of
+    // per_route_kill_settings does not match that of kill_request_
+    kill_request_.mutable_probability()->CopyFrom(per_route_kill_settings->getProbability());
   }
 
   if (is_kill_request && is_correct_direction && isKillRequestEnabled()) {

source/extensions/filters/http/rbac/rbac_filter.cc

@@ -26,15 +26,8 @@ RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig(
 const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl*
 RoleBasedAccessControlFilterConfig::engine(const Router::RouteConstSharedPtr route,
                                            Filters::Common::RBAC::EnforcementMode mode) const {
-  if (!route || !route->routeEntry()) {
-    return engine(mode);
-  }
-
-  const std::string& name = HttpFilterNames::get().Rbac;
-  const auto* entry = route->routeEntry();
-  const auto* route_local =
-      entry->mostSpecificPerFilterConfigTyped<RoleBasedAccessControlRouteSpecificFilterConfig>(
-          name);
+  const auto* route_local = Http::Utility::resolveMostSpecificPerFilterConfig<
+      RoleBasedAccessControlRouteSpecificFilterConfig>(HttpFilterNames::get().Rbac, route);
 
   if (route_local) {
     return route_local->engine(mode);

test/common/http/utility_test.cc

@@ -872,37 +872,21 @@ TEST(HttpUtility, ResetReasonToString) {
             Utility::resetReasonToString(Http::StreamResetReason::ConnectError));
 }
 
-// Verify that it resolveMostSpecificPerFilterConfigGeneric works with nil routes.
-TEST(HttpUtility, ResolveMostSpecificPerFilterConfigNilRoute) {
-  EXPECT_EQ(nullptr, Utility::resolveMostSpecificPerFilterConfigGeneric("envoy.filter", nullptr));
-}
-
 class TestConfig : public Router::RouteSpecificFilterConfig {
 public:
   int state_;
   void merge(const TestConfig& other) { state_ += other.state_; }
 };
 
-// Verify that resolveMostSpecificPerFilterConfig works and we get back the original type.
-TEST(HttpUtility, ResolveMostSpecificPerFilterConfig) {
-  TestConfig testConfig;
-
-  const std::string filter_name = "envoy.filter";
-  NiceMock<Http::MockStreamDecoderFilterCallbacks> filter_callbacks;
-
-  // make the file callbacks return our test config
-  ON_CALL(*filter_callbacks.route_, perFilterConfig(filter_name))
-      .WillByDefault(Return(&testConfig));
-
-  // test the we get the same object back (as this goes through the dynamic_cast)
-  auto resolved_filter_config = Utility::resolveMostSpecificPerFilterConfig<TestConfig>(
-      filter_name, filter_callbacks.route());
-  EXPECT_EQ(&testConfig, resolved_filter_config);
+// Verify that it resolveMostSpecificPerFilterConfig works with nil routes.
+TEST(HttpUtility, ResolveMostSpecificPerFilterConfigNilRoute) {
+  EXPECT_EQ(nullptr,
+            Utility::resolveMostSpecificPerFilterConfig<TestConfig>("envoy.filter", nullptr));
 }
 
-// Verify that resolveMostSpecificPerFilterConfigGeneric indeed returns the most specific per
+// Verify that resolveMostSpecificPerFilterConfig indeed returns the most specific per
 // filter config.
-TEST(HttpUtility, ResolveMostSpecificPerFilterConfigGeneric) {
+TEST(HttpUtility, ResolveMostSpecificPerFilterConfig) {
   const std::string filter_name = "envoy.filter";
   NiceMock<Http::MockStreamDecoderFilterCallbacks> filter_callbacks;
 
@@ -911,28 +895,34 @@ TEST(HttpUtility, ResolveMostSpecificPerFilterConfigGeneric) {
   const Router::RouteSpecificFilterConfig three;
 
   // Test when there's nothing on the route
-  EXPECT_EQ(nullptr, Utility::resolveMostSpecificPerFilterConfigGeneric(filter_name,
-                                                                        filter_callbacks.route()));
+  EXPECT_EQ(nullptr, Utility::resolveMostSpecificPerFilterConfig<Router::RouteSpecificFilterConfig>(
+                         filter_name, filter_callbacks.route()));
 
   // Testing in reverse order, so that the method always returns the last object.
+  // Testing per-virtualhost typed filter config
   ON_CALL(filter_callbacks.route_->route_entry_.virtual_host_, perFilterConfig(filter_name))
       .WillByDefault(Return(&one));
-  EXPECT_EQ(&one, Utility::resolveMostSpecificPerFilterConfigGeneric(filter_name,
-                                                                     filter_callbacks.route()));
+  EXPECT_EQ(&one, Utility::resolveMostSpecificPerFilterConfig<Router::RouteSpecificFilterConfig>(
+                      filter_name, filter_callbacks.route()));
 
+  // Testing per-route typed filter config
   ON_CALL(*filter_callbacks.route_, perFilterConfig(filter_name)).WillByDefault(Return(&two));
-  EXPECT_EQ(&two, Utility::resolveMostSpecificPerFilterConfigGeneric(filter_name,
-                                                                     filter_callbacks.route()));
+  ON_CALL(filter_callbacks.route_->route_entry_, perFilterConfig(filter_name))
+      .WillByDefault(Invoke(
+          [&](const std::string& name) { return filter_callbacks.route_->perFilterConfig(name); }));
+  EXPECT_EQ(&two, Utility::resolveMostSpecificPerFilterConfig<Router::RouteSpecificFilterConfig>(
+                      filter_name, filter_callbacks.route()));
 
+  // Testing per-route entry typed filter config
   ON_CALL(filter_callbacks.route_->route_entry_, perFilterConfig(filter_name))
       .WillByDefault(Return(&three));
-  EXPECT_EQ(&three, Utility::resolveMostSpecificPerFilterConfigGeneric(filter_name,
-                                                                       filter_callbacks.route()));
+  EXPECT_EQ(&three, Utility::resolveMostSpecificPerFilterConfig<Router::RouteSpecificFilterConfig>(
+                        filter_name, filter_callbacks.route()));
 
   // Cover the case of no route entry
   ON_CALL(*filter_callbacks.route_, routeEntry()).WillByDefault(Return(nullptr));
-  EXPECT_EQ(&two, Utility::resolveMostSpecificPerFilterConfigGeneric(filter_name,
-                                                                     filter_callbacks.route()));
+  EXPECT_EQ(nullptr, Utility::resolveMostSpecificPerFilterConfig<Router::RouteSpecificFilterConfig>(
+                         filter_name, filter_callbacks.route()));
 }
 
 // Verify that traversePerFilterConfigGeneric traverses in the order of specificity.

test/extensions/filters/http/ext_authz/ext_authz_test.cc

@@ -1498,7 +1498,8 @@ TEST_P(HttpFilterTestParam, ContextExtensions) {
       "value_route";
   // Initialize the route's per filter config.
   FilterConfigPerRoute auth_per_route(settingsroute);
-  ON_CALL(*filter_callbacks_.route_, perFilterConfig(HttpFilterNames::get().ExtAuthorization))
+  ON_CALL(filter_callbacks_.route_->route_entry_,
+          perFilterConfig(HttpFilterNames::get().ExtAuthorization))
       .WillByDefault(Return(&auth_per_route));
 
   prepareCheck();
@@ -1528,7 +1529,8 @@ TEST_P(HttpFilterTestParam, DisabledOnRoute) {
 
   prepareCheck();
 
-  ON_CALL(*filter_callbacks_.route_, perFilterConfig(HttpFilterNames::get().ExtAuthorization))
+  ON_CALL(filter_callbacks_.route_->route_entry_,
+          perFilterConfig(HttpFilterNames::get().ExtAuthorization))
       .WillByDefault(Return(&auth_per_route));
 
   auto test_disable = [&](bool disabled) {
@@ -1559,7 +1561,8 @@ TEST_P(HttpFilterTestParam, DisabledOnRouteWithRequestBody) {
   envoy::extensions::filters::http::ext_authz::v3::ExtAuthzPerRoute settings;
   FilterConfigPerRoute auth_per_route(settings);
 
-  ON_CALL(*filter_callbacks_.route_, perFilterConfig(HttpFilterNames::get().ExtAuthorization))
+  ON_CALL(filter_callbacks_.route_->route_entry_,
+          perFilterConfig(HttpFilterNames::get().ExtAuthorization))
       .WillByDefault(Return(&auth_per_route));
 
   auto test_disable = [&](bool disabled) {
@@ -2137,7 +2140,8 @@ TEST_P(HttpFilterTestParam, NoCluster) {
       "value_route";
   // Initialize the route's per filter config.
   FilterConfigPerRoute auth_per_route(settingsroute);
-  ON_CALL(*filter_callbacks_.route_, perFilterConfig(HttpFilterNames::get().ExtAuthorization))
+  ON_CALL(filter_callbacks_.route_->route_entry_,
+          perFilterConfig(HttpFilterNames::get().ExtAuthorization))
       .WillByDefault(Return(&auth_per_route));
 
   prepareCheck();
@@ -2163,7 +2167,8 @@ TEST_P(HttpFilterTestParam, DisableRequestBodyBufferingOnRoute) {
   envoy::extensions::filters::http::ext_authz::v3::ExtAuthzPerRoute settings;
   FilterConfigPerRoute auth_per_route(settings);
 
-  ON_CALL(*filter_callbacks_.route_, perFilterConfig(HttpFilterNames::get().ExtAuthorization))
+  ON_CALL(filter_callbacks_.route_->route_entry_,
+          perFilterConfig(HttpFilterNames::get().ExtAuthorization))
       .WillByDefault(Return(&auth_per_route));
 
   auto test_disable_request_body_buffering = [&](bool bypass) {

test/extensions/filters/http/grpc_http1_reverse_bridge/reverse_bridge_test.cc

@@ -616,7 +616,7 @@ TEST_F(ReverseBridgeTest, FilterConfigPerRouteDisabled) {
   filter_config_per_route.set_disabled(true);
   FilterConfigPerRoute filterConfigPerRoute(filter_config_per_route);
 
-  ON_CALL(*decoder_callbacks_.route_,
+  ON_CALL(decoder_callbacks_.route_->route_entry_,
           perFilterConfig(HttpFilterNames::get().GrpcHttp1ReverseBridge))
       .WillByDefault(testing::Return(&filterConfigPerRoute));
 
@@ -645,7 +645,7 @@ TEST_F(ReverseBridgeTest, FilterConfigPerRouteEnabled) {
   filter_config_per_route.set_disabled(false);
   FilterConfigPerRoute filterConfigPerRoute(filter_config_per_route);
 
-  ON_CALL(*decoder_callbacks_.route_,
+  ON_CALL(decoder_callbacks_.route_->route_entry_,
           perFilterConfig(HttpFilterNames::get().GrpcHttp1ReverseBridge))
       .WillByDefault(testing::Return(&filterConfigPerRoute));
 
@@ -733,7 +733,7 @@ TEST_F(ReverseBridgeTest, RouteWithTrailers) {
   filter_config_per_route.set_disabled(false);
   FilterConfigPerRoute filterConfigPerRoute(filter_config_per_route);
 
-  ON_CALL(*decoder_callbacks_.route_,
+  ON_CALL(decoder_callbacks_.route_->route_entry_,
           perFilterConfig(HttpFilterNames::get().GrpcHttp1ReverseBridge))
       .WillByDefault(testing::Return(&filterConfigPerRoute));
 

test/per_file_coverage.sh

@@ -33,7 +33,7 @@ declare -a KNOWN_LOW_COVERAGE=(
 "source/extensions/filters/common/rbac:87.5"
 "source/extensions/filters/http/cache:92.4"
 "source/extensions/filters/http/cache/simple_http_cache:95.2"
-"source/extensions/filters/http/grpc_json_transcoder:95.7"
+"source/extensions/filters/http/grpc_json_transcoder:95.6"
 "source/extensions/filters/http/ip_tagging:91.2"
 "source/extensions/filters/http/kill_request:85.0" # Death tests don't report LCOV
 "source/extensions/filters/listener/tls_inspector:92.4"