composite_filter: Demonstrate the usage of route-level config (envoyproxy#30442)

* valid per route config

Signed-off-by: tyxia <[email protected]>

* tweak names

Signed-off-by: tyxia <[email protected]>

* add new line

Signed-off-by: tyxia <[email protected]>

* update test

Signed-off-by: tyxia <[email protected]>

---------

Signed-off-by: tyxia <[email protected]>

Author: tyxia

test/extensions/filters/http/composite/composite_filter_integration_test.cc

@@ -33,6 +33,7 @@ using envoy::extensions::common::matching::v3::ExtensionWithMatcherPerRoute;
 using envoy::extensions::filters::http::composite::v3::ExecuteFilterAction;
 using envoy::type::matcher::v3::HttpRequestHeaderMatchInput;
 using test::integration::filters::SetResponseCodeFilterConfig;
+using test::integration::filters::SetResponseCodePerRouteFilterConfig;
 using xds::type::matcher::v3::Matcher_OnMatch;
 
 class CompositeFilterIntegrationTest : public testing::TestWithParam<Network::Address::IpVersion>,
@@ -87,6 +88,25 @@ class CompositeFilterIntegrationTest : public testing::TestWithParam<Network::Ad
         });
   }
 
+  void addResponseCodeFilterPerRouteConfig(const std::string& filter_name,
+                                           const std::string& route_prefix, const int& code,
+                                           bool response_prefix = false) {
+    SetResponseCodePerRouteFilterConfig set_response_code_per_route_config;
+    set_response_code_per_route_config.set_code(code);
+    if (response_prefix) {
+      set_response_code_per_route_config.set_prefix("skipLocalReplyAndContinue");
+    }
+    config_helper_.addConfigModifier([set_response_code_per_route_config, filter_name,
+                                      route_prefix](ConfigHelper::HttpConnectionManager& cm) {
+      auto* vh = cm.mutable_route_config()->mutable_virtual_hosts(0);
+      auto* route = vh->mutable_routes()->Mutable(0);
+      route->mutable_match()->set_prefix(route_prefix);
+      route->mutable_route()->set_cluster("cluster_0");
+      (*route->mutable_typed_per_filter_config())[filter_name].PackFrom(
+          set_response_code_per_route_config);
+    });
+  }
+
   void prependCompositeFilter(const std::string& name = "composite") {
     config_helper_.prependFilter(absl::StrFormat(R"EOF(
       name: %s
@@ -166,6 +186,23 @@ TEST_P(CompositeFilterIntegrationTest, TestPerRoute) {
   EXPECT_THAT(response->headers(), Http::HttpStatusIs("401"));
 }
 
+// Verifies set_response_code filter's per-route config overrides the filter config.
+TEST_P(CompositeFilterIntegrationTest, TestPerRouteResponseCodeConfig) {
+  std::string top_level_filter_name = "match_delegate_filter";
+  prependCompositeFilter(top_level_filter_name);
+
+  addResponseCodeFilterPerRouteConfig(/*filter_name=*/top_level_filter_name,
+                                      /*route_prefix=*/"/somepath",
+                                      /*code=*/406);
+  initialize();
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+
+  auto response = codec_client_->makeRequestWithBody(match_request_headers_, 1024);
+  ASSERT_TRUE(response->waitForEndStream());
+  // Verifies that 406 from per route config is used, rather than 403 from filter config.
+  EXPECT_THAT(response->headers(), Http::HttpStatusIs("406"));
+}
+
 // Test an empty match tree resolving with a per route config.
 TEST_P(CompositeFilterIntegrationTest, TestPerRouteEmptyMatcher) {
   config_helper_.prependFilter(R"EOF(

test/integration/BUILD

@@ -722,7 +722,7 @@ envoy_cc_test(
     ]),
     deps = [
         ":http_integration_lib",
-        "//test/integration/filters:set_response_code_filter_lib",
+        "//test/integration/filters:set_route_filter_lib",
     ],
 )
 

test/integration/filters/BUILD

@@ -487,6 +487,7 @@ envoy_cc_test_library(
         ":set_response_code_filter_config_proto_cc_proto",
         "//envoy/http:filter_interface",
         "//envoy/registry",
+        "//source/common/http:utility_lib",
         "//source/extensions/filters/http/common:factory_base_lib",
         "//source/extensions/filters/http/common:pass_through_filter_lib",
     ],

test/integration/filters/set_response_code_filter.cc

@@ -3,6 +3,7 @@
 #include "envoy/http/filter.h"
 #include "envoy/registry/registry.h"
 
+#include "source/common/http/utility.h"
 #include "source/extensions/filters/http/common/factory_base.h"
 #include "source/extensions/filters/http/common/pass_through_filter.h"
 
@@ -27,14 +28,45 @@ class SetResponseCodeFilterConfig {
   ThreadLocal::TypedSlot<> tls_slot_;
 };
 
+class SetResponseCodeFilterRouteSpecificConfig : public Envoy::Router::RouteSpecificFilterConfig {
+public:
+  SetResponseCodeFilterRouteSpecificConfig(const std::string& prefix, uint32_t code,
+                                           const std::string& body,
+                                           Server::Configuration::FactoryContextBase& context)
+      : prefix_(prefix), code_(code), body_(body), tls_slot_(context.threadLocal()) {}
+
+  const std::string prefix_;
+  const uint32_t code_;
+  const std::string body_;
+  // Allocate a slot to validate that it is destroyed on a main thread only.
+  ThreadLocal::TypedSlot<> tls_slot_;
+};
+
 class SetResponseCodeFilter : public Http::PassThroughFilter {
 public:
   SetResponseCodeFilter(std::shared_ptr<SetResponseCodeFilterConfig> config) : config_(config) {}
 
   Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool) override {
-    if (absl::StartsWith(headers.Path()->value().getStringView(), config_->prefix_)) {
-      decoder_callbacks_->sendLocalReply(static_cast<Http::Code>(config_->code_), config_->body_,
-                                         nullptr, absl::nullopt, "");
+    const auto* per_route_config = Envoy::Http::Utility::resolveMostSpecificPerFilterConfig<
+        SetResponseCodeFilterRouteSpecificConfig>(decoder_callbacks_);
+
+    std::string prefix;
+    uint32_t code;
+    std::string body;
+    // Route level config takes precedence over filter level config, if present.
+    if (per_route_config != nullptr) {
+      prefix = per_route_config->prefix_;
+      code = per_route_config->code_;
+      body = per_route_config->body_;
+    } else {
+      prefix = config_->prefix_;
+      code = config_->code_;
+      body = config_->body_;
+    }
+
+    if (absl::StartsWith(headers.Path()->value().getStringView(), prefix)) {
+      decoder_callbacks_->sendLocalReply(static_cast<Http::Code>(code), body, nullptr,
+                                         absl::nullopt, "");
       return Http::FilterHeadersStatus::StopIteration;
     }
     return Http::FilterHeadersStatus::Continue;
@@ -44,8 +76,10 @@ class SetResponseCodeFilter : public Http::PassThroughFilter {
   const std::shared_ptr<SetResponseCodeFilterConfig> config_;
 };
 
-class SetResponseCodeFilterFactory : public Extensions::HttpFilters::Common::FactoryBase<
-                                         test::integration::filters::SetResponseCodeFilterConfig> {
+class SetResponseCodeFilterFactory
+    : public Extensions::HttpFilters::Common::FactoryBase<
+          test::integration::filters::SetResponseCodeFilterConfig,
+          test::integration::filters::SetResponseCodePerRouteFilterConfig> {
 public:
   SetResponseCodeFilterFactory() : FactoryBase("set-response-code-filter") {}
 
@@ -68,6 +102,14 @@ class SetResponseCodeFilterFactory : public Extensions::HttpFilters::Common::Fac
       callbacks.addStreamFilter(std::make_shared<SetResponseCodeFilter>(filter_config));
     };
   }
+
+  Router::RouteSpecificFilterConfigConstSharedPtr createRouteSpecificFilterConfigTyped(
+      const test::integration::filters::SetResponseCodePerRouteFilterConfig& proto_config,
+      Server::Configuration::ServerFactoryContext& context,
+      ProtobufMessage::ValidationVisitor&) override {
+    return std::make_shared<const SetResponseCodeFilterRouteSpecificConfig>(
+        proto_config.prefix(), proto_config.code(), proto_config.body(), context);
+  }
 };
 
 REGISTER_FACTORY(SetResponseCodeFilterFactory, Server::Configuration::NamedHttpFilterConfigFactory);

test/integration/filters/set_response_code_filter_config.proto

@@ -9,3 +9,10 @@ message SetResponseCodeFilterConfig {
   uint32 code = 2 [(validate.rules).uint32 = {lt: 600 gte: 200}];
   string body = 3;
 }
+
+// Per route config overrides filter config, if present.
+message SetResponseCodePerRouteFilterConfig {
+  string prefix = 1;
+  uint32 code = 2 [(validate.rules).uint32 = {lt: 600 gte: 200}];
+  string body = 3;
+}

test/integration/http_typed_per_filter_config_test.cc

@@ -1,6 +1,6 @@
 #include "envoy/config/route/v3/route_components.pb.h"
 
-#include "test/integration/filters/set_response_code_filter_config.pb.h"
+#include "test/integration/filters/set_route_filter_config.pb.h"
 #include "test/integration/http_integration.h"
 
 #include "gtest/gtest.h"
@@ -18,21 +18,20 @@ TEST_F(HTTPTypedPerFilterConfigTest, RejectUnsupportedTypedPerFilterConfig) {
   config_helper_.addConfigModifier(
       [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
               hcm) {
-        test::integration::filters::SetResponseCodeFilterConfig response_code;
-        response_code.set_code(403);
+        test::integration::filters::SetRouteFilterConfig set_route_config;
 
         auto* virtual_host = hcm.mutable_route_config()->mutable_virtual_hosts(0);
         auto* config = virtual_host->mutable_typed_per_filter_config();
-        (*config)["set-response-code-filter"].PackFrom(response_code);
+        (*config)["set-route-filter"].PackFrom(set_route_config);
 
         auto* filter = hcm.mutable_http_filters()->Add();
-        filter->set_name("set-response-code-filter");
-        filter->mutable_typed_config()->PackFrom(response_code);
+        filter->set_name("set-route-filter");
+        filter->mutable_typed_config()->PackFrom(set_route_config);
         // keep router the last
         auto size = hcm.http_filters_size();
         hcm.mutable_http_filters()->SwapElements(size - 2, size - 1);
       });
-  EXPECT_DEATH(initialize(), "The filter set-response-code-filter doesn't support virtual host or "
+  EXPECT_DEATH(initialize(), "The filter set-route-filter doesn't support virtual host or "
                              "route specific configurations");
 }