From 359b54b7c6eb4a184eda0a758aab6fae125eba40 Mon Sep 17 00:00:00 2001
From: John Howard <john.howard@solo.io>
Date: Fri, 13 Dec 2024 04:08:12 -0800
Subject: [PATCH] set_filter_state: enable per-route configuration override
 (#37507)

Commit Message: set_filter_state: enable per-route configuration
override
Additional Description:
This extends this filter to support usage in route configuration. When
present, both the listener and route level settings are applied
(listener first, then route).

Risk Level: Low
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

---------

Signed-off-by: John Howard <john.howard@solo.io>
---
 changelogs/current.yaml                       |  3 +
 .../common/set_filter_state/filter_config.h   |  3 +-
 .../filters/http/set_filter_state/config.cc   | 23 ++++-
 .../filters/http/set_filter_state/config.h    |  5 ++
 .../http/set_filter_state/integration_test.cc | 85 +++++++++++++++++++
 5 files changed, 117 insertions(+), 2 deletions(-)

diff --git a/changelogs/current.yaml b/changelogs/current.yaml
index 030fddfa2054..40dca57f6422 100644
--- a/changelogs/current.yaml
+++ b/changelogs/current.yaml
@@ -332,6 +332,9 @@ new_features:
   change: |
     Added new health check filter stats including total requests, successful/failed checks, cached responses, and
     cluster health status counters. These stats help track health check behavior and cluster health state.
+- area: filters
+  change: |
+    Updatd the ``set_filter_state`` :ref:`filter <config_http_filters_set_filter_state>` to support per-route overrides.
 - area: grpc-json
   change: |
     Added a new http filter for :ref:`gRPC to JSON transcoding <config_http_filters_grpc_json_reverse_transcoder>`.
diff --git a/source/extensions/filters/common/set_filter_state/filter_config.h b/source/extensions/filters/common/set_filter_state/filter_config.h
index 9ea6f1d20954..adb962237a98 100644
--- a/source/extensions/filters/common/set_filter_state/filter_config.h
+++ b/source/extensions/filters/common/set_filter_state/filter_config.h
@@ -28,7 +28,8 @@ struct Value {
   Formatter::FormatterConstSharedPtr value_;
 };
 
-class Config : public Logger::Loggable<Logger::Id::config> {
+class Config : public ::Envoy::Router::RouteSpecificFilterConfig,
+               public Logger::Loggable<Logger::Id::config> {
 public:
   Config(const Protobuf::RepeatedPtrField<FilterStateValueProto>& proto_values, LifeSpan life_span,
          Server::Configuration::GenericFactoryContext& context)
diff --git a/source/extensions/filters/http/set_filter_state/config.cc b/source/extensions/filters/http/set_filter_state/config.cc
index d0edcb486fe6..ae2acc5b69e6 100644
--- a/source/extensions/filters/http/set_filter_state/config.cc
+++ b/source/extensions/filters/http/set_filter_state/config.cc
@@ -7,6 +7,7 @@
 #include "envoy/formatter/substitution_formatter.h"
 #include "envoy/registry/registry.h"
 
+#include "source/common/http/utility.h"
 #include "source/common/protobuf/utility.h"
 #include "source/server/generic_factory_context.h"
 
@@ -19,7 +20,15 @@ SetFilterState::SetFilterState(const Filters::Common::SetFilterState::ConfigShar
     : config_(config) {}
 
 Http::FilterHeadersStatus SetFilterState::decodeHeaders(Http::RequestHeaderMap& headers, bool) {
-  config_->updateFilterState({&headers}, decoder_callbacks_->streamInfo());
+  // Apply listener level configuration first.
+  config_.get()->updateFilterState({&headers}, decoder_callbacks_->streamInfo());
+
+  // If configured, apply virtual host and then route level configuration next.
+  auto policies = Http::Utility::getAllPerFilterConfig<Filters::Common::SetFilterState::Config>(
+      decoder_callbacks_);
+  for (auto policy : policies) {
+    policy.get().updateFilterState({&headers}, decoder_callbacks_->streamInfo());
+  }
   return Http::FilterHeadersStatus::Continue;
 }
 
@@ -35,6 +44,18 @@ Http::FilterFactoryCb SetFilterStateConfig::createFilterFactoryFromProtoTyped(
   };
 }
 
+absl::StatusOr<Router::RouteSpecificFilterConfigConstSharedPtr>
+SetFilterStateConfig::createRouteSpecificFilterConfigTyped(
+    const envoy::extensions::filters::http::set_filter_state::v3::Config& proto_config,
+    Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor&) {
+
+  Server::GenericFactoryContextImpl generic_context(context, context.messageValidationVisitor());
+
+  return std::make_shared<const Filters::Common::SetFilterState::Config>(
+      proto_config.on_request_headers(), StreamInfo::FilterState::LifeSpan::FilterChain,
+      generic_context);
+}
+
 Http::FilterFactoryCb SetFilterStateConfig::createFilterFactoryFromProtoWithServerContextTyped(
     const envoy::extensions::filters::http::set_filter_state::v3::Config& proto_config,
     const std::string&, Server::Configuration::ServerFactoryContext& context) {
diff --git a/source/extensions/filters/http/set_filter_state/config.h b/source/extensions/filters/http/set_filter_state/config.h
index be0d72a6098f..7088f49b13d2 100644
--- a/source/extensions/filters/http/set_filter_state/config.h
+++ b/source/extensions/filters/http/set_filter_state/config.h
@@ -38,6 +38,11 @@ class SetFilterStateConfig
       const envoy::extensions::filters::http::set_filter_state::v3::Config& proto_config,
       const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override;
 
+  absl::StatusOr<Router::RouteSpecificFilterConfigConstSharedPtr>
+  createRouteSpecificFilterConfigTyped(
+      const envoy::extensions::filters::http::set_filter_state::v3::Config& proto_config,
+      Server::Configuration::ServerFactoryContext&, ProtobufMessage::ValidationVisitor&) override;
+
   Http::FilterFactoryCb createFilterFactoryFromProtoWithServerContextTyped(
       const envoy::extensions::filters::http::set_filter_state::v3::Config& proto_config,
       const std::string& stats_prefix,
diff --git a/test/extensions/filters/http/set_filter_state/integration_test.cc b/test/extensions/filters/http/set_filter_state/integration_test.cc
index 73c3763d8cd6..4a348a68affd 100644
--- a/test/extensions/filters/http/set_filter_state/integration_test.cc
+++ b/test/extensions/filters/http/set_filter_state/integration_test.cc
@@ -14,6 +14,7 @@
 #include "gtest/gtest.h"
 
 using testing::NiceMock;
+using testing::Return;
 using testing::ReturnRef;
 
 namespace Envoy {
@@ -66,6 +67,45 @@ class SetMetadataIntegrationTest : public testing::Test {
     EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter->decodeHeaders(headers_, true));
   }
 
+  void runPerRouteFilter(const std::string& filter_yaml_config,
+                         const std::string& per_route_yaml_config) {
+    Server::GenericFactoryContextImpl generic_context(context_);
+
+    envoy::extensions::filters::http::set_filter_state::v3::Config filter_proto_config;
+    TestUtility::loadFromYaml(filter_yaml_config, filter_proto_config);
+    auto filter_config = std::make_shared<Filters::Common::SetFilterState::Config>(
+        filter_proto_config.on_request_headers(), StreamInfo::FilterState::LifeSpan::FilterChain,
+        generic_context);
+
+    envoy::extensions::filters::http::set_filter_state::v3::Config route_proto_config;
+    TestUtility::loadFromYaml(per_route_yaml_config, route_proto_config);
+    Filters::Common::SetFilterState::Config route_config(
+        route_proto_config.on_request_headers(), StreamInfo::FilterState::LifeSpan::FilterChain,
+        generic_context);
+
+    NiceMock<Http::MockStreamDecoderFilterCallbacks> decoder_callbacks;
+
+    EXPECT_CALL(decoder_callbacks, perFilterConfigs())
+        .WillOnce(testing::Invoke(
+            [&]() -> Router::RouteSpecificFilterConfigs { return {&route_config}; }));
+    auto filter = std::make_shared<SetFilterState>(filter_config);
+    filter->setDecoderFilterCallbacks(decoder_callbacks);
+    EXPECT_CALL(decoder_callbacks, streamInfo()).WillRepeatedly(ReturnRef(info_));
+    EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter->decodeHeaders(headers_, true));
+
+    // Test the factory method.
+    {
+      NiceMock<Server::Configuration::MockServerFactoryContext> context;
+      SetFilterStateConfig factory;
+      Router::RouteSpecificFilterConfigConstSharedPtr route_config =
+          factory
+              .createRouteSpecificFilterConfig(route_proto_config, context,
+                                               ProtobufMessage::getNullValidationVisitor())
+              .value();
+      EXPECT_TRUE(route_config.get());
+    }
+  }
+
   NiceMock<Server::Configuration::MockFactoryContext> context_;
   Http::TestRequestHeaderMapImpl headers_{{"test-header", "test-value"}};
   NiceMock<StreamInfo::MockStreamInfo> info_;
@@ -85,6 +125,51 @@ TEST_F(SetMetadataIntegrationTest, FromHeader) {
   EXPECT_EQ(foo->serializeAsString(), "test-value");
 }
 
+TEST_F(SetMetadataIntegrationTest, RouteLevel) {
+  const std::string filter_config = R"EOF(
+  on_request_headers:
+  - object_key: both
+    factory_key: envoy.string
+    format_string:
+      text_format_source:
+        inline_string: "filter-%REQ(test-header)%"
+  - object_key: filter-only
+    factory_key: envoy.string
+    format_string:
+      text_format_source:
+        inline_string: "filter"
+  )EOF";
+  const std::string route_config = R"EOF(
+  on_request_headers:
+  - object_key: both
+    factory_key: envoy.string
+    format_string:
+      text_format_source:
+        inline_string: "route-%REQ(test-header)%"
+  - object_key: route-only
+    factory_key: envoy.string
+    format_string:
+      text_format_source:
+        inline_string: "route"
+  )EOF";
+  runPerRouteFilter(filter_config, route_config);
+
+  const auto* both = info_.filterState()->getDataReadOnly<Router::StringAccessor>("both");
+  ASSERT_NE(nullptr, both);
+  // Route takes precedence
+  EXPECT_EQ(both->serializeAsString(), "route-test-value");
+
+  const auto* filter = info_.filterState()->getDataReadOnly<Router::StringAccessor>("filter-only");
+  ASSERT_NE(nullptr, filter);
+  // Only set on filter
+  EXPECT_EQ(filter->serializeAsString(), "filter");
+
+  const auto* route = info_.filterState()->getDataReadOnly<Router::StringAccessor>("route-only");
+  ASSERT_NE(nullptr, route);
+  // Only set on route
+  EXPECT_EQ(route->serializeAsString(), "route");
+}
+
 } // namespace SetFilterState
 } // namespace HttpFilters
 } // namespace Extensions