Revert "dynamic_modules: add support for disabling HTTP per-route filter (envoyproxy#42766)" (envoyproxy#42771)

wbpcode · web-flow · commit cfcffd041f6d · 2025-12-25T11:51:51.000+08:00
Commit Message: Revert "dynamic_modules: add support for disabling HTTP per-route filter (envoyproxy#42766)" Additional Description: We have provided common disabled flag in the route configuration and this API may be unnecessary (and new api shepherd reviewed it. We need to revisit it before put int into our main. Risk Level: low. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a.
diff --git a/api/envoy/extensions/filters/http/dynamic_modules/v3/dynamic_modules.proto b/api/envoy/extensions/filters/http/dynamic_modules/v3/dynamic_modules.proto
@@ -120,11 +120,4 @@ message DynamicModuleFilterPerRoute {
   //    value: aGVsbG8= # echo -n "hello" | base64
   //
   google.protobuf.Any filter_config = 3;
-
-  // Disable the filter for this particular vhost or route.
-  // If this field is specified in multiple per-filter-configs, the most specific
-  // one will be used.
-  //
-  // If this field is not specified, the filter would remain enabled.
-  bool disabled = 4;
 }
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -281,11 +281,6 @@ new_features:
   change: |
     Added support for loading dynamic modules globally by setting :ref:`load_globally
     <envoy_v3_api_field_extensions.dynamic_modules.v3.DynamicModuleConfig.load_globally>` to ``true``.
-- area: dynamic modules
-  change: |
-    Added :ref:`disabled <envoy_v3_api_field_extensions.filters.http.dynamic_modules.v3.DynamicModuleFilterPerRoute.disabled>`
-    field to per-route configuration for the dynamic modules HTTP filter, enabling dynamic modules to be disabled on a
-    per-route basis.
 - area: dynamic modules
   change: |
     Added :ref:`network filter <envoy_v3_api_msg_extensions.filters.network.dynamic_modules.v3.DynamicModuleNetworkFilter>`
diff --git a/source/extensions/filters/http/dynamic_modules/BUILD b/source/extensions/filters/http/dynamic_modules/BUILD
@@ -25,7 +25,6 @@ envoy_cc_library(
     hdrs = ["filter.h"],
     deps = [
         ":filter_config_lib",
-        "//source/common/http:utility_lib",
         "//source/extensions/filters/http/common:pass_through_filter_lib",
     ],
 )
diff --git a/source/extensions/filters/http/dynamic_modules/factory.cc b/source/extensions/filters/http/dynamic_modules/factory.cc
@@ -44,6 +44,7 @@ absl::StatusOr<Http::FilterFactoryCb> DynamicModuleConfigFactory::createFilterFa
     auto filter =
         std::make_shared<Envoy::Extensions::DynamicModules::HttpFilters::DynamicModuleHttpFilter>(
             config, config->stats_scope_->symbolTable());
+    filter->initializeInModuleFilter();
     callbacks.addStreamFilter(filter);
   };
 }
@@ -72,8 +73,7 @@ DynamicModuleConfigFactory::createRouteSpecificFilterConfigTyped(
                      DynamicModuleHttpPerRouteFilterConfigConstSharedPtr>
       filter_config =
           Envoy::Extensions::DynamicModules::HttpFilters::newDynamicModuleHttpPerRouteConfig(
-              proto_config.per_route_config_name(), config, std::move(dynamic_module.value()),
-              proto_config.disabled());
+              proto_config.per_route_config_name(), config, std::move(dynamic_module.value()));
 
   if (!filter_config.ok()) {
     return absl::InvalidArgumentError("Failed to create pre-route filter config: " +
diff --git a/source/extensions/filters/http/dynamic_modules/filter.cc b/source/extensions/filters/http/dynamic_modules/filter.cc
@@ -3,8 +3,6 @@
 #include <cstdint>
 #include <memory>
 
-#include "source/common/http/utility.h"
-
 #include "absl/container/inlined_vector.h"
 
 namespace Envoy {
@@ -15,16 +13,10 @@ namespace HttpFilters {
 DynamicModuleHttpFilter::~DynamicModuleHttpFilter() { destroy(); }
 
 void DynamicModuleHttpFilter::initializeInModuleFilter() {
-  if (in_module_filter_ != nullptr) {
-    return;
-  }
   in_module_filter_ = config_->on_http_filter_new_(config_->in_module_config_, thisAsVoidPtr());
 }
 
 void DynamicModuleHttpFilter::onStreamComplete() {
-  if (in_module_filter_ == nullptr) {
-    return;
-  }
   config_->on_http_filter_stream_complete_(thisAsVoidPtr(), in_module_filter_);
 }
 
@@ -82,35 +74,13 @@ void DynamicModuleHttpFilter::destroy() {
 }
 
 FilterHeadersStatus DynamicModuleHttpFilter::decodeHeaders(RequestHeaderMap&, bool end_of_stream) {
-  if (decoder_callbacks_->route()) {
-    const auto* per_route_config =
-        Http::Utility::resolveMostSpecificPerFilterConfig<DynamicModuleHttpPerRouteFilterConfig>(
-            decoder_callbacks_);
-    if (per_route_config && per_route_config->disabled_) {
-      is_disabled_ = true;
-      in_continue_ = true;
-      return FilterHeadersStatus::Continue;
-    }
-  }
-
-  if (in_module_filter_ == nullptr) {
-    initializeInModuleFilter();
-  }
-
   const envoy_dynamic_module_type_on_http_filter_request_headers_status status =
       config_->on_http_filter_request_headers_(thisAsVoidPtr(), in_module_filter_, end_of_stream);
   in_continue_ = status == envoy_dynamic_module_type_on_http_filter_request_headers_status_Continue;
   return static_cast<FilterHeadersStatus>(status);
 };
 
 FilterDataStatus DynamicModuleHttpFilter::decodeData(Buffer::Instance& chunk, bool end_of_stream) {
-  if (is_disabled_) {
-    if (end_of_stream && decoder_callbacks_->decodingBuffer()) {
-      decoder_callbacks_->addDecodedData(chunk, false);
-    }
-    in_continue_ = true;
-    return FilterDataStatus::Continue;
-  }
   if (end_of_stream && decoder_callbacks_->decodingBuffer()) {
     // To make the very last chunk of the body available to the filter when buffering is enabled,
     // we need to call addDecodedData. See the code comment there for more details.
@@ -125,10 +95,6 @@ FilterDataStatus DynamicModuleHttpFilter::decodeData(Buffer::Instance& chunk, bo
 };
 
 FilterTrailersStatus DynamicModuleHttpFilter::decodeTrailers(RequestTrailerMap&) {
-  if (is_disabled_) {
-    in_continue_ = true;
-    return FilterTrailersStatus::Continue;
-  }
   const envoy_dynamic_module_type_on_http_filter_request_trailers_status status =
       config_->on_http_filter_request_trailers_(thisAsVoidPtr(), in_module_filter_);
   in_continue_ =
@@ -150,30 +116,8 @@ Filter1xxHeadersStatus DynamicModuleHttpFilter::encode1xxHeaders(ResponseHeaderM
 
 FilterHeadersStatus DynamicModuleHttpFilter::encodeHeaders(ResponseHeaderMap&, bool end_of_stream) {
   if (sent_local_reply_) { // See the comment on the flag.
-    in_continue_ = true;
     return FilterHeadersStatus::Continue;
   }
-
-  // Check per-route config for disabled flag if not already checked in decodeHeaders.
-  if (in_module_filter_ == nullptr) {
-    if (encoder_callbacks_->route()) {
-      const auto* per_route_config =
-          Http::Utility::resolveMostSpecificPerFilterConfig<DynamicModuleHttpPerRouteFilterConfig>(
-              encoder_callbacks_);
-      if (per_route_config && per_route_config->disabled_) {
-        is_disabled_ = true;
-        in_continue_ = true;
-        return FilterHeadersStatus::Continue;
-      }
-    }
-    initializeInModuleFilter();
-  }
-
-  if (is_disabled_) {
-    in_continue_ = true;
-    return FilterHeadersStatus::Continue;
-  }
-
   const envoy_dynamic_module_type_on_http_filter_response_headers_status status =
       config_->on_http_filter_response_headers_(thisAsVoidPtr(), in_module_filter_, end_of_stream);
   in_continue_ =
@@ -182,11 +126,7 @@ FilterHeadersStatus DynamicModuleHttpFilter::encodeHeaders(ResponseHeaderMap&, b
 };
 
 FilterDataStatus DynamicModuleHttpFilter::encodeData(Buffer::Instance& chunk, bool end_of_stream) {
-  if (sent_local_reply_ || is_disabled_) { // See the comment on the flag.
-    if (end_of_stream && encoder_callbacks_->encodingBuffer()) {
-      encoder_callbacks_->addEncodedData(chunk, false);
-    }
-    in_continue_ = true;
+  if (sent_local_reply_) { // See the comment on the flag.
     return FilterDataStatus::Continue;
   }
   if (end_of_stream && encoder_callbacks_->encodingBuffer()) {
@@ -203,8 +143,7 @@ FilterDataStatus DynamicModuleHttpFilter::encodeData(Buffer::Instance& chunk, bo
 };
 
 FilterTrailersStatus DynamicModuleHttpFilter::encodeTrailers(ResponseTrailerMap&) {
-  if (sent_local_reply_ || is_disabled_) { // See the comment on the flag.
-    in_continue_ = true;
+  if (sent_local_reply_) { // See the comment on the flag.
     return FilterTrailersStatus::Continue;
   }
   const envoy_dynamic_module_type_on_http_filter_response_trailers_status status =
diff --git a/source/extensions/filters/http/dynamic_modules/filter.h b/source/extensions/filters/http/dynamic_modules/filter.h
@@ -230,8 +230,6 @@ class DynamicModuleHttpFilter : public Http::StreamFilter,
   // multiple mutable borrows of the same object. In practice, a module shouldn't need encodeHeaders
   // and encodeData to be called for local reply contents, so we just skip them with this flag.
   bool sent_local_reply_ = false;
-  // This is set to true when the filter is disabled by the per-route configuration.
-  bool is_disabled_ = false;
 
   const DynamicModuleHttpFilterConfigSharedPtr config_ = nullptr;
   envoy_dynamic_module_type_http_filter_module_ptr in_module_filter_ = nullptr;
diff --git a/source/extensions/filters/http/dynamic_modules/filter_config.cc b/source/extensions/filters/http/dynamic_modules/filter_config.cc
@@ -23,21 +23,13 @@ DynamicModuleHttpFilterConfig::~DynamicModuleHttpFilterConfig() {
 }
 
 DynamicModuleHttpPerRouteFilterConfig::~DynamicModuleHttpPerRouteFilterConfig() {
-  if (destroy_) {
-    (*destroy_)(config_);
-  }
+  (*destroy_)(config_);
 }
 
 absl::StatusOr<DynamicModuleHttpPerRouteFilterConfigConstSharedPtr>
 newDynamicModuleHttpPerRouteConfig(const absl::string_view per_route_config_name,
                                    const absl::string_view filter_config,
-                                   Extensions::DynamicModules::DynamicModulePtr dynamic_module,
-                                   bool disabled) {
-  if (disabled) {
-    return std::make_shared<const DynamicModuleHttpPerRouteFilterConfig>(
-        nullptr, nullptr, std::move(dynamic_module), disabled);
-  }
-
+                                   Extensions::DynamicModules::DynamicModulePtr dynamic_module) {
   auto constructor =
       dynamic_module
           ->getFunctionPointer<decltype(&envoy_dynamic_module_on_http_filter_per_route_config_new)>(
@@ -56,7 +48,7 @@ newDynamicModuleHttpPerRouteConfig(const absl::string_view per_route_config_name
   }
 
   return std::make_shared<const DynamicModuleHttpPerRouteFilterConfig>(
-      filter_config_envoy_ptr, destroy.value(), std::move(dynamic_module), disabled);
+      filter_config_envoy_ptr, destroy.value(), std::move(dynamic_module));
 }
 
 absl::StatusOr<DynamicModuleHttpFilterConfigSharedPtr> newDynamicModuleHttpFilterConfig(
diff --git a/source/extensions/filters/http/dynamic_modules/filter_config.h b/source/extensions/filters/http/dynamic_modules/filter_config.h
@@ -309,13 +309,11 @@ class DynamicModuleHttpPerRouteFilterConfig : public Router::RouteSpecificFilter
   DynamicModuleHttpPerRouteFilterConfig(
       envoy_dynamic_module_type_http_filter_config_module_ptr config,
       OnHttpPerRouteConfigDestroyType destroy,
-      Extensions::DynamicModules::DynamicModulePtr dynamic_module, bool disabled)
-      : config_(config), disabled_(disabled), destroy_(destroy),
-        dynamic_module_(std::move(dynamic_module)) {}
+      Extensions::DynamicModules::DynamicModulePtr dynamic_module)
+      : config_(config), destroy_(destroy), dynamic_module_(std::move(dynamic_module)) {}
   ~DynamicModuleHttpPerRouteFilterConfig() override;
 
   envoy_dynamic_module_type_http_filter_config_module_ptr config_;
-  const bool disabled_;
 
 private:
   OnHttpPerRouteConfigDestroyType destroy_;
@@ -329,8 +327,7 @@ using DynamicModuleHttpPerRouteFilterConfigConstSharedPtr =
 absl::StatusOr<DynamicModuleHttpPerRouteFilterConfigConstSharedPtr>
 newDynamicModuleHttpPerRouteConfig(const absl::string_view per_route_config_name,
                                    const absl::string_view filter_config,
-                                   Extensions::DynamicModules::DynamicModulePtr dynamic_module,
-                                   bool disabled);
+                                   Extensions::DynamicModules::DynamicModulePtr dynamic_module);
 
 /**
  * Creates a new DynamicModuleHttpFilterConfig for given configuration.
diff --git a/test/extensions/dynamic_modules/http/filter_test.cc b/test/extensions/dynamic_modules/http/filter_test.cc
diff --git a/test/extensions/dynamic_modules/http/integration_test.cc b/test/extensions/dynamic_modules/http/integration_test.cc

Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,6 @@ envoy_cc_library(`
`25`	`25`	`hdrs = ["filter.h"],`
`26`	`26`	`deps = [`
`27`	`27`	`":filter_config_lib",`
`28`		`- "//source/common/http:utility_lib",`
`29`	`28`	`"//source/extensions/filters/http/common:pass_through_filter_lib",`
`30`	`29`	`],`
`31`	`30`	`)`